wip: upgrade legacy password hashes on login (#30)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: aberoham

lib/user/mysql.js

@@ -37,6 +37,16 @@ class UserRepoMySQL extends UserBase {
 
     for (const u of await Mysql.execute(query, [username, groupName])) {
       if (await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt)) {
+        if (this._needsUpgrade) {
+          const salt = this.generateSalt()
+          const hash = await this.hashAuthPbkdf2(authTry.password, salt)
+          await Mysql.execute(
+            ...Mysql.update('nt_user', `nt_user_id=${u.id}`, {
+              password: hash,
+              pass_salt: salt,
+            }),
+          )
+        }
         for (const f of ['password', 'pass_salt']) {
           delete u[f] // SECURITY: no longer needed
         }

lib/user/test.js

@@ -1,8 +1,10 @@
 import assert from 'node:assert/strict'
+import crypto from 'node:crypto'
 import { describe, it, after, before } from 'node:test'
 
 import User from './index.js'
 import Group from '../group/index.js'
+import Mysql from '../mysql.js'
 
 import userCase from '../test/user.json' with { type: 'json' }
 import groupCase from '../test/group.json' with { type: 'json' }
@@ -144,4 +146,117 @@ describe('user', function () {
       assert.ok(u)
     })
   })
+
+  describe('password upgrade on login', () => {
+    const upgradeUserId = 4200
+    const upgradeUser = {
+      nt_user_id: upgradeUserId,
+      nt_group_id: groupCase.id,
+      username: 'upgrade-test',
+      email: 'upgrade-test@example.com',
+      first_name: 'Upgrade',
+      last_name: 'Test',
+    }
+    const testPass = 'UpgradeMe!123'
+    const authCreds = {
+      username: `${upgradeUser.username}@${groupCase.name}`,
+      password: testPass,
+    }
+
+    async function getDbPassword() {
+      const rows = await Mysql.execute(
+        'SELECT password, pass_salt FROM nt_user WHERE nt_user_id = ?',
+        [upgradeUserId],
+      )
+      return rows[0]
+    }
+
+    async function insertUser(password, passSalt) {
+      await Mysql.execute(
+        'INSERT INTO nt_user (nt_user_id, nt_group_id, username, email, first_name, last_name, password, pass_salt) VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
+        [upgradeUserId, upgradeUser.nt_group_id, upgradeUser.username, upgradeUser.email, upgradeUser.first_name, upgradeUser.last_name, password, passSalt],
+      )
+    }
+
+    async function cleanup() {
+      await Mysql.execute(
+        'DELETE FROM nt_user WHERE nt_user_id = ?',
+        [upgradeUserId],
+      )
+    }
+
+    before(cleanup)
+    after(cleanup)
+
+    it('upgrades plain text password to PBKDF2 on login', async () => {
+      await cleanup()
+      await insertUser(testPass, null)
+
+      const result = await User.authenticate(authCreds)
+      assert.ok(result, 'login should succeed')
+
+      const row = await getDbPassword()
+      assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
+      assert.notEqual(row.password, testPass, 'password should be hashed')
+
+      // verify round-trip: can still log in with the upgraded hash
+      const again = await User.authenticate(authCreds)
+      assert.ok(again, 'login should succeed after upgrade')
+      await cleanup()
+    })
+
+    it('upgrades SHA1 password to PBKDF2 on login', async () => {
+      // authenticate() passes the full authTry.username (including @group) to
+      // validPassword(), so the HMAC key must match that full string
+      const sha1Hash = crypto
+        .createHmac('sha1', authCreds.username.toLowerCase())
+        .update(testPass)
+        .digest('hex')
+      await cleanup()
+      await insertUser(sha1Hash, null)
+
+      const result = await User.authenticate(authCreds)
+      assert.ok(result, 'login should succeed with SHA1 hash')
+
+      const row = await getDbPassword()
+      assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
+      assert.notEqual(row.password, sha1Hash, 'password should be re-hashed')
+
+      const again = await User.authenticate(authCreds)
+      assert.ok(again, 'login should succeed after upgrade')
+      await cleanup()
+    })
+
+    it('upgrades PBKDF2-5000 to current iterations on login', async () => {
+      const legacySalt = User.generateSalt()
+      const legacyHash = await User.hashAuthPbkdf2(testPass, legacySalt, 5000)
+      await cleanup()
+      await insertUser(legacyHash, legacySalt)
+
+      const result = await User.authenticate(authCreds)
+      assert.ok(result, 'login should succeed with legacy PBKDF2')
+
+      const row = await getDbPassword()
+      assert.notEqual(row.password, legacyHash, 'password should be re-hashed')
+      assert.notEqual(row.pass_salt, legacySalt, 'salt should be regenerated')
+
+      const again = await User.authenticate(authCreds)
+      assert.ok(again, 'login should succeed after upgrade')
+      await cleanup()
+    })
+
+    it('does not re-hash password already at current iterations', async () => {
+      const salt = User.generateSalt()
+      const hash = await User.hashAuthPbkdf2(testPass, salt)
+      await cleanup()
+      await insertUser(hash, salt)
+
+      await User.authenticate(authCreds)
+
+      const row = await getDbPassword()
+      assert.equal(row.password, hash, 'password should be unchanged')
+      assert.equal(row.pass_salt, salt, 'salt should be unchanged')
+      await cleanup()
+    })
+  })
 })

lib/user/userBase.js

@@ -1,5 +1,8 @@
 import crypto from 'node:crypto'
 
+const PBKDF2_ITERATIONS = parseInt(process.env.PBKDF2_ITERATIONS) || 220000
+const LEGACY_ITERATIONS = 5000
+
 /**
  * User domain class – pure attributes and password business logic.
  *
@@ -60,30 +63,45 @@ class UserBase {
     return salt
   }
 
-  async hashAuthPbkdf2(pass, salt) {
+  async hashAuthPbkdf2(pass, salt, iterations = PBKDF2_ITERATIONS) {
     return new Promise((resolve, reject) => {
-      // match the defaults for NicTool 2.x
-      crypto.pbkdf2(pass, salt, 5000, 32, 'sha512', (err, derivedKey) => {
+      crypto.pbkdf2(pass, salt, iterations, 32, 'sha512', (err, derivedKey) => {
         if (err) return reject(err)
         resolve(derivedKey.toString('hex'))
       })
     })
   }
 
   async validPassword(passTry, passDb, username, salt) {
-    if (!salt && passTry === passDb) return true // plain pass, TODO, encrypt!
+    this._needsUpgrade = false
+
+    if (!salt && passTry === passDb) {
+      this._needsUpgrade = true
+      return true
+    }
 
     if (salt) {
       const hashed = await this.hashAuthPbkdf2(passTry, salt)
       if (this.debug) console.log(`hashed: (${hashed === passDb}) ${hashed}`)
-      return hashed === passDb
+      if (hashed === passDb) return true
+      // fall back to NicTool 2 legacy iteration count
+      const legacy = await this.hashAuthPbkdf2(passTry, salt, LEGACY_ITERATIONS)
+      if (this.debug) console.log(`legacy: (${legacy === passDb}) ${legacy}`)
+      if (legacy === passDb) {
+        this._needsUpgrade = true
+        return true
+      }
+      return false
     }
 
     // Check for HMAC SHA-1 password
     if (/^[0-9a-f]{40}$/.test(passDb)) {
       const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex')
       if (this.debug) console.log(`digest: (${digest === passDb}) ${digest}`)
-      return digest === passDb
+      if (digest === passDb) {
+        this._needsUpgrade = true
+        return true
+      }
     }
 
     return false