diff --git a/src/oidcserver.js b/src/oidcserver.js index 137048c42..77970560b 100644 --- a/src/oidcserver.js +++ b/src/oidcserver.js @@ -389,7 +389,7 @@ async function interactionLogin(req, res, next) { // Handle passkey verification if provided if (!verifyError && user && !user.ghost && passkeyResponse && !totpToken) { - const userPasskeys = await passkeys.list(user.id); + const userPasskeys = await passkeys.listByUserId(user.id); if (userPasskeys.length > 0) { const [passkeyError] = await safe(passkeys.verifyAuthentication(user, passkeyResponse)); if (passkeyError) { @@ -402,14 +402,14 @@ async function interactionLogin(req, res, next) { // If password verified but 2FA is required and not provided, return challenge if (!verifyError && user && !user.ghost && !totpToken && !passkeyResponse) { - const userPasskeys = await passkeys.list(user.id); + const userPasskeys = await passkeys.listByUserId(user.id); const has2FA = user.twoFactorAuthenticationEnabled || userPasskeys.length > 0; if (has2FA) { // Generate passkey options if user has passkeys let passkeyOptions = null; if (userPasskeys.length > 0) { - const [optionsError, options] = await safe(passkeys.generateAuthenticationOptions(user)); + const [optionsError, options] = await safe(passkeys.getAuthenticationOptions(user)); if (!optionsError) passkeyOptions = options; } diff --git a/src/passkeys.js b/src/passkeys.js index 31c954ffa..277691c75 100644 --- a/src/passkeys.js +++ b/src/passkeys.js @@ -6,16 +6,15 @@ import database from './database.js'; import debugModule from 'debug'; import safe from 'safetydance'; import { - generateRegistrationOptions as generateWebAuthnRegistrationOptions, + generateRegistrationOptions, verifyRegistrationResponse, - generateAuthenticationOptions as generateWebAuthnAuthenticationOptions, + generateAuthenticationOptions, verifyAuthenticationResponse } from '@simplewebauthn/server'; import _ from './underscore.js'; const debug = debugModule('box:passkeys'); - const PASSKEY_FIELDS = [ 'id', 'userId', 'credentialId', 'publicKey', 'counter', 'transports', 'name', 'creationTime', 'lastUsedTime' ].join(','); // In-memory challenge store with expiration (challenges are short-lived) @@ -32,17 +31,17 @@ function postProcess(passkey) { return passkey; } -async function list(userId) { +async function listByUserId(userId) { assert.strictEqual(typeof userId, 'string'); - const results = await database.query('SELECT ' + PASSKEY_FIELDS + ' FROM passkeys WHERE userId = ? ORDER BY creationTime', [ userId ]); + const results = await database.query(`SELECT ${PASSKEY_FIELDS} FROM passkeys WHERE userId = ? ORDER BY creationTime`, [ userId ]); return results.map(postProcess); } async function get(id) { assert.strictEqual(typeof id, 'string'); - const result = await database.query('SELECT ' + PASSKEY_FIELDS + ' FROM passkeys WHERE id = ?', [ id ]); + const result = await database.query(`SELECT ${PASSKEY_FIELDS} FROM passkeys WHERE id = ?`, [ id ]); if (result.length === 0) return null; return postProcess(result[0]); } @@ -50,7 +49,7 @@ async function get(id) { async function getByCredentialId(credentialId) { assert.strictEqual(typeof credentialId, 'string'); - const result = await database.query('SELECT ' + PASSKEY_FIELDS + ' FROM passkeys WHERE credentialId = ?', [ credentialId ]); + const result = await database.query(`SELECT ${PASSKEY_FIELDS} FROM passkeys WHERE credentialId = ?`, [ credentialId ]); if (result.length === 0) return null; return postProcess(result[0]); } @@ -76,14 +75,14 @@ async function add(userId, credentialId, publicKey, counter, transports, name) { return { id }; } -async function del(id, userId) { +async function del(id) { assert.strictEqual(typeof id, 'string'); - assert.strictEqual(typeof userId, 'string'); - const result = await database.query('DELETE FROM passkeys WHERE id = ? AND userId = ?', [ id, userId ]); + const result = await database.query('DELETE FROM passkeys WHERE id = ?', [ id ]); if (result.affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'Passkey not found'); } +// this counter prevents replay attacks async function updateCounter(id, counter) { assert.strictEqual(typeof id, 'string'); assert.strictEqual(typeof counter, 'number'); @@ -96,22 +95,20 @@ async function delAll() { } function storeChallenge(userId, challenge) { - const key = `${userId}`; - gChallenges.set(key, { + gChallenges.set(userId, { challenge, expiresAt: Date.now() + CHALLENGE_EXPIRY_MS }); - // Clean up expired challenges periodically + // clean up expired challenges for (const [k, v] of gChallenges) { if (v.expiresAt < Date.now()) gChallenges.delete(k); } } function getAndDeleteChallenge(userId) { - const key = `${userId}`; - const entry = gChallenges.get(key); - gChallenges.delete(key); + const entry = gChallenges.get(userId); + gChallenges.delete(userId); if (!entry) return null; if (entry.expiresAt < Date.now()) return null; @@ -129,23 +126,17 @@ async function getRpInfo() { }; } -async function generateRegistrationOptions(user) { +async function getRegistrationOptions(user) { assert.strictEqual(typeof user, 'object'); - // Only one passkey per user is allowed - const existingPasskeys = await list(user.id); - if (existingPasskeys.length > 0) { - throw new BoxError(BoxError.ALREADY_EXISTS, 'User already has a passkey registered'); - } + const existingPasskeys = await listByUserId(user.id); + if (existingPasskeys.length > 0) throw new BoxError(BoxError.ALREADY_EXISTS, 'User already has a passkey registered'); - // Cannot register passkey if TOTP is enabled (user must choose one or the other) - if (user.twoFactorAuthenticationEnabled) { - throw new BoxError(BoxError.ALREADY_EXISTS, 'Cannot register passkey when TOTP is enabled'); - } + if (user.twoFactorAuthenticationEnabled) throw new BoxError(BoxError.ALREADY_EXISTS, 'Cannot register passkey when TOTP is enabled'); const { rpId, rpName } = await getRpInfo(); - const options = await generateWebAuthnRegistrationOptions({ + const options = await generateRegistrationOptions({ rpName, rpID: rpId, userName: user.username || user.email, @@ -160,7 +151,7 @@ async function generateRegistrationOptions(user) { storeChallenge(user.id, options.challenge); - debug(`generateRegistrationOptions: generated for user ${user.id}`); + debug(`getRegistrationOptions: generated for user ${user.id}`); return options; } @@ -170,16 +161,10 @@ async function verifyRegistration(user, response, name) { assert.strictEqual(typeof response, 'object'); assert.strictEqual(typeof name, 'string'); - // Defense in depth: check again before adding - const existingPasskeys = await list(user.id); - if (existingPasskeys.length > 0) { - throw new BoxError(BoxError.ALREADY_EXISTS, 'User already has a passkey registered'); - } - - // Cannot register passkey if TOTP is enabled - if (user.twoFactorAuthenticationEnabled) { - throw new BoxError(BoxError.ALREADY_EXISTS, 'Cannot register passkey when TOTP is enabled'); - } + // check should ideally be in a transaction + const existingPasskeys = await listByUserId(user.id); + if (existingPasskeys.length > 0) throw new BoxError(BoxError.ALREADY_EXISTS, 'User already has a passkey registered'); + if (user.twoFactorAuthenticationEnabled) throw new BoxError(BoxError.ALREADY_EXISTS, 'Cannot register passkey when TOTP is enabled'); const expectedChallenge = getAndDeleteChallenge(user.id); if (!expectedChallenge) throw new BoxError(BoxError.BAD_FIELD, 'Challenge expired or not found'); @@ -198,9 +183,7 @@ async function verifyRegistration(user, response, name) { throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); } - if (!verification.verified || !verification.registrationInfo) { - throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); - } + if (!verification.verified || !verification.registrationInfo) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); const { credential } = verification.registrationInfo; @@ -222,17 +205,15 @@ async function verifyRegistration(user, response, name) { return result; } -async function generateAuthenticationOptions(user) { +async function getAuthenticationOptions(user) { assert.strictEqual(typeof user, 'object'); const { rpId } = await getRpInfo(); - const existingPasskeys = await list(user.id); - if (existingPasskeys.length === 0) { - throw new BoxError(BoxError.NOT_FOUND, 'No passkeys registered'); - } + const existingPasskeys = await listByUserId(user.id); + if (existingPasskeys.length === 0) throw new BoxError(BoxError.NOT_FOUND, 'No passkeys registered'); - const options = await generateWebAuthnAuthenticationOptions({ + const options = await generateAuthenticationOptions({ rpID: rpId, allowCredentials: existingPasskeys.map(pk => ({ id: pk.credentialId, @@ -243,7 +224,7 @@ async function generateAuthenticationOptions(user) { storeChallenge(user.id, options.challenge); - debug(`generateAuthenticationOptions: generated for user ${user.id}`); + debug(`getAuthenticationOptions: generated for user ${user.id}`); return options; } @@ -271,7 +252,6 @@ async function verifyAuthentication(user, response) { throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey does not belong to this user'); } - // Convert stored base64 public key back to Uint8Array const publicKey = new Uint8Array(Buffer.from(passkey.publicKey, 'base64')); const [error, verification] = await safe(verifyAuthenticationResponse({ @@ -292,11 +272,8 @@ async function verifyAuthentication(user, response) { throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); } - if (!verification.verified) { - throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); - } + if (!verification.verified) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Passkey verification failed'); - // Update the counter to prevent replay attacks await updateCounter(passkey.id, verification.authenticationInfo.newCounter); debug(`verifyAuthentication: passkey verified for user ${user.id}`); @@ -305,19 +282,17 @@ async function verifyAuthentication(user, response) { } export default { - list, + listByUserId, get, getByCredentialId, add, del, - updateCounter, - // this is only for dashboard origin changes delAll, - generateRegistrationOptions, + getRegistrationOptions, verifyRegistration, - generateAuthenticationOptions, + getAuthenticationOptions, verifyAuthentication, removePrivateFields diff --git a/src/routes/profile.js b/src/routes/profile.js index c61e661d7..e765c688d 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -235,7 +235,7 @@ async function destroyUserSession(req, res, next) { async function getPasskey(req, res, next) { assert.strictEqual(typeof req.user, 'object'); - const [error, result] = await safe(passkeys.list(req.user.id)); + const [error, result] = await safe(passkeys.listByUserId(req.user.id)); if (error) return next(BoxError.toHttpError(error)); // Only one passkey per user - return first one or null @@ -246,7 +246,7 @@ async function getPasskey(req, res, next) { async function getPasskeyRegistrationOptions(req, res, next) { assert.strictEqual(typeof req.user, 'object'); - const [error, options] = await safe(passkeys.generateRegistrationOptions(req.user)); + const [error, options] = await safe(passkeys.getRegistrationOptions(req.user)); if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(200, options)); @@ -271,12 +271,12 @@ async function deletePasskey(req, res, next) { assert.strictEqual(typeof req.user, 'object'); // Get the user's passkey (only one allowed) - const [listError, result] = await safe(passkeys.list(req.user.id)); + const [listError, result] = await safe(passkeys.listByUserId(req.user.id)); if (listError) return next(BoxError.toHttpError(listError)); if (result.length === 0) return next(new HttpError(404, 'No passkey registered')); - const [error] = await safe(passkeys.del(result[0].id, req.user.id)); + const [error] = await safe(passkeys.del(result[0].id)); if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(204)); diff --git a/src/test/passkeys-test.js b/src/test/passkeys-test.js index 25f23b8ea..61e027d6e 100644 --- a/src/test/passkeys-test.js +++ b/src/test/passkeys-test.js @@ -14,7 +14,7 @@ import webauthnHelper from './webauthn-helper.js'; /* global after:false */ describe('Passkeys', function () { - const { domainSetup, cleanup, admin, user, auditSource, dashboardFqdn } = common; + const { domainSetup, cleanup, admin, auditSource, dashboardFqdn } = common; const origin = `https://${dashboardFqdn}`; async function cleanupUsers() { @@ -36,7 +36,7 @@ describe('Passkeys', function () { before(createOwner); it('list returns empty', async function () { - const result = await passkeys.list(admin.id); + const result = await passkeys.listByUserId(admin.id); expect(result).to.be.an(Array); expect(result.length).to.be(0); }); @@ -48,7 +48,7 @@ describe('Passkeys', function () { }); it('list returns one passkey', async function () { - const result = await passkeys.list(admin.id); + const result = await passkeys.listByUserId(admin.id); expect(result.length).to.be(1); expect(result[0].userId).to.be(admin.id); expect(result[0].credentialId).to.be('credId123'); @@ -59,7 +59,7 @@ describe('Passkeys', function () { }); it('can get by id', async function () { - const list = await passkeys.list(admin.id); + const list = await passkeys.listByUserId(admin.id); const result = await passkeys.get(list[0].id); expect(result).to.not.be(null); expect(result.credentialId).to.be('credId123'); @@ -87,23 +87,15 @@ describe('Passkeys', function () { expect(error.reason).to.be(BoxError.ALREADY_EXISTS); }); - it('can update counter', async function () { - const list = await passkeys.list(admin.id); - await passkeys.updateCounter(list[0].id, 42); - const updated = await passkeys.get(list[0].id); - expect(updated.counter).to.be(42); - expect(updated.lastUsedTime).to.not.be(null); - }); - it('can delete a passkey', async function () { - const list = await passkeys.list(admin.id); - await passkeys.del(list[0].id, admin.id); - const afterDel = await passkeys.list(admin.id); + const list = await passkeys.listByUserId(admin.id); + await passkeys.del(list[0].id); + const afterDel = await passkeys.listByUserId(admin.id); expect(afterDel.length).to.be(0); }); it('delete throws for unknown passkey', async function () { - const [error] = await safe(passkeys.del('pk-nonexistent', admin.id)); + const [error] = await safe(passkeys.del('pk-nonexistent')); expect(error).to.not.be(null); expect(error.reason).to.be(BoxError.NOT_FOUND); }); @@ -111,7 +103,7 @@ describe('Passkeys', function () { it('delAll clears all passkeys', async function () { await passkeys.add(admin.id, 'cred1', 'pk1', 0, [], 'Key1'); await passkeys.delAll(); - const result = await passkeys.list(admin.id); + const result = await passkeys.listByUserId(admin.id); expect(result.length).to.be(0); }); @@ -127,7 +119,7 @@ describe('Passkeys', function () { expect(filtered.counter).to.be(undefined); expect(filtered.userId).to.be(undefined); - await passkeys.del(id, admin.id); + await passkeys.del(id); }); }); @@ -138,7 +130,7 @@ describe('Passkeys', function () { it('can generate registration options', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateRegistrationOptions(adminUser); + const options = await passkeys.getRegistrationOptions(adminUser); expect(options).to.be.an(Object); expect(options.challenge).to.be.a('string'); expect(options.rp).to.be.an(Object); @@ -151,7 +143,7 @@ describe('Passkeys', function () { authenticator = webauthnHelper.createVirtualAuthenticator(); const adminUser = await users.get(admin.id); - const options = await passkeys.generateRegistrationOptions(adminUser); + const options = await passkeys.getRegistrationOptions(adminUser); const response = await webauthnHelper.createRegistrationResponse(authenticator, options, origin); const result = await passkeys.verifyRegistration(adminUser, response, 'My Test Key'); @@ -159,14 +151,14 @@ describe('Passkeys', function () { expect(result.id).to.be.a('string'); expect(result.id).to.match(/^pk-/); - const list = await passkeys.list(admin.id); + const list = await passkeys.listByUserId(admin.id); expect(list.length).to.be(1); expect(list[0].name).to.be('My Test Key'); }); it('rejects duplicate registration', async function () { const adminUser = await users.get(admin.id); - const [error] = await safe(passkeys.generateRegistrationOptions(adminUser)); + const [error] = await safe(passkeys.getRegistrationOptions(adminUser)); expect(error).to.not.be(null); expect(error.reason).to.be(BoxError.ALREADY_EXISTS); }); @@ -180,7 +172,7 @@ describe('Passkeys', function () { authenticator = webauthnHelper.createVirtualAuthenticator(); // generate options to get a challenge, but use a different challenge in the response - const options = await passkeys.generateRegistrationOptions(adminUser); + const options = await passkeys.getRegistrationOptions(adminUser); options.challenge = 'tampered-challenge-value'; const response = await webauthnHelper.createRegistrationResponse(authenticator, options, origin); @@ -198,7 +190,7 @@ describe('Passkeys', function () { await users.enableTwoFactorAuthentication(adminUser, totpToken, auditSource); adminUser.twoFactorAuthenticationEnabled = true; - const [error] = await safe(passkeys.generateRegistrationOptions(adminUser)); + const [error] = await safe(passkeys.getRegistrationOptions(adminUser)); expect(error).to.not.be(null); expect(error.reason).to.be(BoxError.ALREADY_EXISTS); @@ -217,14 +209,14 @@ describe('Passkeys', function () { authenticator = webauthnHelper.createVirtualAuthenticator(); const adminUser = await users.get(admin.id); - const options = await passkeys.generateRegistrationOptions(adminUser); + const options = await passkeys.getRegistrationOptions(adminUser); const response = await webauthnHelper.createRegistrationResponse(authenticator, options, origin); await passkeys.verifyRegistration(adminUser, response, 'Auth Test Key'); }); it('can generate authentication options', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateAuthenticationOptions(adminUser); + const options = await passkeys.getAuthenticationOptions(adminUser); expect(options).to.be.an(Object); expect(options.challenge).to.be.a('string'); expect(options.allowCredentials).to.be.an(Array); @@ -233,7 +225,7 @@ describe('Passkeys', function () { it('can authenticate with virtual authenticator', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateAuthenticationOptions(adminUser); + const options = await passkeys.getAuthenticationOptions(adminUser); const response = await webauthnHelper.createAuthenticationResponse(authenticator, options, origin); const result = await passkeys.verifyAuthentication(adminUser, response); @@ -243,26 +235,26 @@ describe('Passkeys', function () { }); it('counter was updated after authentication', async function () { - const list = await passkeys.list(admin.id); + const list = await passkeys.listByUserId(admin.id); expect(list[0].counter).to.be(1); expect(list[0].lastUsedTime).to.not.be(null); }); it('can authenticate again (counter increments)', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateAuthenticationOptions(adminUser); + const options = await passkeys.getAuthenticationOptions(adminUser); const response = await webauthnHelper.createAuthenticationResponse(authenticator, options, origin); const result = await passkeys.verifyAuthentication(adminUser, response); expect(result.verified).to.be(true); - const list = await passkeys.list(admin.id); + const list = await passkeys.listByUserId(admin.id); expect(list[0].counter).to.be(2); }); it('rejects authentication with wrong credential', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateAuthenticationOptions(adminUser); + const options = await passkeys.getAuthenticationOptions(adminUser); // create a different authenticator and try to auth with it const fakeAuth = webauthnHelper.createVirtualAuthenticator(); @@ -275,7 +267,7 @@ describe('Passkeys', function () { it('rejects authentication with tampered challenge', async function () { const adminUser = await users.get(admin.id); - const options = await passkeys.generateAuthenticationOptions(adminUser); + const options = await passkeys.getAuthenticationOptions(adminUser); options.challenge = 'tampered-challenge'; const response = await webauthnHelper.createAuthenticationResponse(authenticator, options, origin); @@ -287,7 +279,7 @@ describe('Passkeys', function () { await passkeys.delAll(); const adminUser = await users.get(admin.id); - const [error] = await safe(passkeys.generateAuthenticationOptions(adminUser)); + const [error] = await safe(passkeys.getAuthenticationOptions(adminUser)); expect(error).to.not.be(null); expect(error.reason).to.be(BoxError.NOT_FOUND); }); @@ -300,7 +292,7 @@ describe('Passkeys', function () { // register a passkey const authenticator = webauthnHelper.createVirtualAuthenticator(); const adminUser = await users.get(admin.id); - const options = await passkeys.generateRegistrationOptions(adminUser); + const options = await passkeys.getRegistrationOptions(adminUser); const response = await webauthnHelper.createRegistrationResponse(authenticator, options, origin); await passkeys.verifyRegistration(adminUser, response, 'Exclusion Test'); @@ -326,7 +318,7 @@ describe('Passkeys', function () { await users.enableTwoFactorAuthentication(adminUser, totpToken, auditSource); adminUser.twoFactorAuthenticationEnabled = true; - const [error] = await safe(passkeys.generateRegistrationOptions(adminUser)); + const [error] = await safe(passkeys.getRegistrationOptions(adminUser)); expect(error).to.not.be(null); expect(error.reason).to.be(BoxError.ALREADY_EXISTS); diff --git a/src/users.js b/src/users.js index 2d07aa5f8..227fc6718 100644 --- a/src/users.js +++ b/src/users.js @@ -921,7 +921,7 @@ async function enableTwoFactorAuthentication(user, totpToken, auditSource) { if (user.source === 'ldap' && externalLdap.supports2FA(externalLdapConfig)) throw new BoxError(BoxError.BAD_STATE, 'Cannot enable 2FA of external auth user'); // Cannot enable TOTP if user has a passkey (user must choose one or the other) - const userPasskeys = await passkeys.list(user.id); + const userPasskeys = await passkeys.listByUserId(user.id); if (userPasskeys.length > 0) throw new BoxError(BoxError.ALREADY_EXISTS, 'Cannot enable TOTP when passkey is registered'); const verified = speakeasy.totp.verify({ secret: user.twoFactorAuthenticationSecret, encoding: 'base32', token: totpToken, window: 2 });