diff --git a/packages/core/src/libraries/user.test.ts b/packages/core/src/libraries/user.test.ts index 1ab423b04..1b4e9630e 100644 --- a/packages/core/src/libraries/user.test.ts +++ b/packages/core/src/libraries/user.test.ts @@ -7,7 +7,7 @@ import { MockQueries } from '#src/test-utils/tenant.js'; const { jest } = import.meta; -const { encryptUserPassword, createUserLibrary, verifyUserPassword } = await import('./user.js'); +const { encryptUserPassword, createUserLibrary } = await import('./user.js'); const hasUserWithId = jest.fn(); const updateUserById = jest.fn(); @@ -70,6 +70,8 @@ describe('encryptUserPassword()', () => { }); describe('verifyUserPassword()', () => { + const { verifyUserPassword } = createUserLibrary(queries); + describe('Argon2', () => { it('resolves when password is correct', async () => { await expect( @@ -151,6 +153,22 @@ describe('verifyUserPassword()', () => { ); }); }); + + describe('Migrate other algorithms to Argon2', () => { + const user = { + ...mockUser, + passwordEncrypted: '5f4dcc3b5aa765d61d8327deb882cf99', + passwordEncryptionMethod: UsersPasswordEncryptionMethod.MD5, + }; + it('migrates password to Argon2', async () => { + await verifyUserPassword(user, 'password'); + expect(updateUserById).toHaveBeenCalledWith(user.id, { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + passwordEncrypted: expect.stringContaining('argon2'), + passwordEncryptionMethod: UsersPasswordEncryptionMethod.Argon2i, + }); + }); + }); }); describe('findUserScopesForResourceId()', () => { diff --git a/packages/core/src/libraries/user.ts b/packages/core/src/libraries/user.ts index 30012023f..19114697f 100644 --- a/packages/core/src/libraries/user.ts +++ b/packages/core/src/libraries/user.ts @@ -31,60 +31,6 @@ export const encryptUserPassword = async ( return { passwordEncrypted, passwordEncryptionMethod }; }; -export const verifyUserPassword = async (user: Nullable, password: string): Promise => { - assertThat(user, new RequestError({ code: 'session.invalid_credentials', status: 422 })); - const { passwordEncrypted, passwordEncryptionMethod } = user; - - assertThat( - passwordEncrypted && passwordEncryptionMethod, - new RequestError({ code: 'session.invalid_credentials', status: 422 }) - ); - - switch (passwordEncryptionMethod) { - case UsersPasswordEncryptionMethod.Argon2i: { - const result = await argon2Verify({ password, hash: passwordEncrypted }); - assertThat(result, new RequestError({ code: 'session.invalid_credentials', status: 422 })); - break; - } - case UsersPasswordEncryptionMethod.MD5: { - const expectedEncrypted = await md5(password); - assertThat( - expectedEncrypted === passwordEncrypted, - new RequestError({ code: 'session.invalid_credentials', status: 422 }) - ); - break; - } - case UsersPasswordEncryptionMethod.SHA1: { - const expectedEncrypted = await sha1(password); - assertThat( - expectedEncrypted === passwordEncrypted, - new RequestError({ code: 'session.invalid_credentials', status: 422 }) - ); - break; - } - case UsersPasswordEncryptionMethod.SHA256: { - const expectedEncrypted = await sha256(password); - assertThat( - expectedEncrypted === passwordEncrypted, - new RequestError({ code: 'session.invalid_credentials', status: 422 }) - ); - break; - } - case UsersPasswordEncryptionMethod.Bcrypt: { - const result = await bcryptVerify({ password, hash: passwordEncrypted }); - assertThat(result, new RequestError({ code: 'session.invalid_credentials', status: 422 })); - break; - } - default: { - throw new RequestError({ code: 'session.invalid_credentials', status: 422 }); - } - } - - // TODO(@sijie) migrate to use argon2 - - return user; -}; - /** * Convert bindMfa to mfaVerification, add common fields like "id" and "createdAt" * and transpile formats like "codes" to "code" for backup code @@ -255,6 +201,68 @@ export const createUserLibrary = (queries: Queries) => { }); }; + const verifyUserPassword = async (user: Nullable, password: string): Promise => { + assertThat(user, new RequestError({ code: 'session.invalid_credentials', status: 422 })); + const { passwordEncrypted, passwordEncryptionMethod, id } = user; + + assertThat( + passwordEncrypted && passwordEncryptionMethod, + new RequestError({ code: 'session.invalid_credentials', status: 422 }) + ); + + switch (passwordEncryptionMethod) { + case UsersPasswordEncryptionMethod.Argon2i: { + const result = await argon2Verify({ password, hash: passwordEncrypted }); + assertThat(result, new RequestError({ code: 'session.invalid_credentials', status: 422 })); + break; + } + case UsersPasswordEncryptionMethod.MD5: { + const expectedEncrypted = await md5(password); + assertThat( + expectedEncrypted === passwordEncrypted, + new RequestError({ code: 'session.invalid_credentials', status: 422 }) + ); + break; + } + case UsersPasswordEncryptionMethod.SHA1: { + const expectedEncrypted = await sha1(password); + assertThat( + expectedEncrypted === passwordEncrypted, + new RequestError({ code: 'session.invalid_credentials', status: 422 }) + ); + break; + } + case UsersPasswordEncryptionMethod.SHA256: { + const expectedEncrypted = await sha256(password); + assertThat( + expectedEncrypted === passwordEncrypted, + new RequestError({ code: 'session.invalid_credentials', status: 422 }) + ); + break; + } + case UsersPasswordEncryptionMethod.Bcrypt: { + const result = await bcryptVerify({ password, hash: passwordEncrypted }); + assertThat(result, new RequestError({ code: 'session.invalid_credentials', status: 422 })); + break; + } + default: { + throw new RequestError({ code: 'session.invalid_credentials', status: 422 }); + } + } + + // Migrate password to default algorithm: argon2i + if (passwordEncryptionMethod !== UsersPasswordEncryptionMethod.Argon2i) { + const { passwordEncrypted: newEncrypted, passwordEncryptionMethod: newMethod } = + await encryptUserPassword(password); + return updateUserById(id, { + passwordEncrypted: newEncrypted, + passwordEncryptionMethod: newMethod, + }); + } + + return user; + }; + return { generateUserId, insertUser, @@ -263,5 +271,6 @@ export const createUserLibrary = (queries: Queries) => { findUserScopesForResourceIndicator, findUserRoles, addUserMfaVerification, + verifyUserPassword, }; }; diff --git a/packages/core/src/routes-me/user.ts b/packages/core/src/routes-me/user.ts index 90fe834cc..c1e3faf9c 100644 --- a/packages/core/src/routes-me/user.ts +++ b/packages/core/src/routes-me/user.ts @@ -4,7 +4,7 @@ import { conditional, pick } from '@silverhand/essentials'; import { literal, object, string } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; -import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js'; +import { encryptUserPassword } from '#src/libraries/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; @@ -20,7 +20,7 @@ export default function userRoutes( users: { findUserById, updateUserById }, }, libraries: { - users: { checkIdentifierCollision }, + users: { checkIdentifierCollision, verifyUserPassword }, verificationStatuses: { createVerificationStatus, checkVerificationStatus }, }, } = tenant; diff --git a/packages/core/src/routes/admin-user/basics.test.ts b/packages/core/src/routes/admin-user/basics.test.ts index 77b03664d..280502541 100644 --- a/packages/core/src/routes/admin-user/basics.test.ts +++ b/packages/core/src/routes/admin-user/basics.test.ts @@ -69,17 +69,14 @@ const { revokeInstanceByUserId } = mockedQueries.oidcModelInstances; const { hasUser, findUserById, updateUserById, deleteUserIdentity, deleteUserById } = mockedQueries.users; -const { encryptUserPassword, verifyUserPassword } = await mockEsmWithActual( - '#src/libraries/user.js', - () => ({ - encryptUserPassword: jest.fn(() => ({ - passwordEncrypted: 'password', - passwordEncryptionMethod: 'Argon2i', - })), - verifyUserPassword: jest.fn(), - }) -); +const { encryptUserPassword } = await mockEsmWithActual('#src/libraries/user.js', () => ({ + encryptUserPassword: jest.fn(() => ({ + passwordEncrypted: 'password', + passwordEncryptionMethod: 'Argon2i', + })), +})); +const verifyUserPassword = jest.fn(); const usersLibraries = { generateUserId: jest.fn(async () => 'fooId'), insertUser: jest.fn( @@ -88,6 +85,7 @@ const usersLibraries = { ...removeUndefinedKeys(user), // No undefined values will be returned from database }) ), + verifyUserPassword, } satisfies Partial; const adminUserRoutes = await pickDefault(import('./basics.js')); diff --git a/packages/core/src/routes/admin-user/basics.ts b/packages/core/src/routes/admin-user/basics.ts index fe561a943..e6b83166f 100644 --- a/packages/core/src/routes/admin-user/basics.ts +++ b/packages/core/src/routes/admin-user/basics.ts @@ -9,7 +9,7 @@ import { conditional, pick, yes } from '@silverhand/essentials'; import { boolean, literal, nativeEnum, object, string } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; -import { encryptUserPassword, verifyUserPassword } from '#src/libraries/user.js'; +import { encryptUserPassword } from '#src/libraries/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; @@ -30,7 +30,7 @@ export default function adminUserBasicsRoutes(...args: R userSsoIdentities, } = queries; const { - users: { checkIdentifierCollision, generateUserId, insertUser }, + users: { checkIdentifierCollision, generateUserId, insertUser, verifyUserPassword }, } = libraries; router.get( diff --git a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts index d79e82d6d..237ccaed3 100644 --- a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts +++ b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts @@ -36,16 +36,15 @@ const { verifySocialIdentity } = mockEsm('../utils/social-verification.js', () = verifySocialIdentity: jest.fn().mockResolvedValue({ id: 'foo' }), })); -const { verifyUserPassword } = await mockEsmWithActual('#src/libraries/user.js', () => ({ - verifyUserPassword: jest.fn(), -})); - const identifierPayloadVerification = await pickDefault( import('./identifier-payload-verification.js') ); +const verifyUserPassword = jest.fn(); const logContext = createMockLogContext(); -const tenant = new MockTenant(); +const tenant = new MockTenant(undefined, undefined, undefined, { + users: { verifyUserPassword }, +}); describe('identifier verification', () => { const baseCtx = { diff --git a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts index f607b2011..4ba0925b4 100644 --- a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts +++ b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts @@ -12,7 +12,6 @@ import { type Optional, isKeyInObject } from '@silverhand/essentials'; import { sha256 } from 'hash-wasm'; import RequestError from '#src/errors/RequestError/index.js'; -import { verifyUserPassword } from '#src/libraries/user.js'; import type { WithLogContext } from '#src/middleware/koa-audit-log.js'; import type TenantContext from '#src/tenants/TenantContext.js'; import assertThat from '#src/utils/assert-that.js'; @@ -56,7 +55,7 @@ const verifyPasswordIdentifier = async ( log.append({ ...identity }); const user = await findUserByIdentifier(tenant, identity); - const verifiedUser = await verifyUserPassword(user, password); + const verifiedUser = await tenant.libraries.users.verifyUserPassword(user, password); const { isSuspended, id } = verifiedUser; diff --git a/packages/integration-tests/src/tests/api/interaction/sign-in-with-password-identifier/happy-path.test.ts b/packages/integration-tests/src/tests/api/interaction/sign-in-with-password-identifier/happy-path.test.ts index 98d1e23ae..dc872ae0c 100644 --- a/packages/integration-tests/src/tests/api/interaction/sign-in-with-password-identifier/happy-path.test.ts +++ b/packages/integration-tests/src/tests/api/interaction/sign-in-with-password-identifier/happy-path.test.ts @@ -1,4 +1,9 @@ -import { InteractionEvent, ConnectorType, SignInIdentifier } from '@logto/schemas'; +import { + InteractionEvent, + ConnectorType, + SignInIdentifier, + UsersPasswordEncryptionMethod, +} from '@logto/schemas'; import { putInteraction, @@ -13,12 +18,13 @@ import { setSmsConnector, setEmailConnector, } from '#src/helpers/connector.js'; -import { readConnectorMessage, expectRejects } from '#src/helpers/index.js'; +import { readConnectorMessage, expectRejects, createUserByAdmin } from '#src/helpers/index.js'; import { enableAllPasswordSignInMethods, enableAllVerificationCodeSignInMethods, } from '#src/helpers/sign-in-experience.js'; import { generateNewUser, generateNewUserProfile } from '#src/helpers/user.js'; +import { generateUsername } from '#src/utils.js'; describe('Sign-in flow using password identifiers', () => { beforeAll(async () => { @@ -52,6 +58,47 @@ describe('Sign-in flow using password identifiers', () => { await deleteUser(user.id); }); + it('sign-in with username and password twice to test algorithm transition', async () => { + const username = generateUsername(); + const password = 'password'; + const user = await createUserByAdmin({ + username, + passwordDigest: '5f4dcc3b5aa765d61d8327deb882cf99', + passwordAlgorithm: UsersPasswordEncryptionMethod.MD5, + }); + const client = await initClient(); + + await client.successSend(putInteraction, { + event: InteractionEvent.SignIn, + identifier: { + username, + password, + }, + }); + + const { redirectTo } = await client.submitInteraction(); + + await processSession(client, redirectTo); + await logoutClient(client); + + const client2 = await initClient(); + + await client2.successSend(putInteraction, { + event: InteractionEvent.SignIn, + identifier: { + username, + password, + }, + }); + + const { redirectTo: redirectTo2 } = await client2.submitInteraction(); + + await processSession(client2, redirectTo2); + await logoutClient(client2); + + await deleteUser(user.id); + }); + it('sign-in with email and password', async () => { const { userProfile, user } = await generateNewUser({ primaryEmail: true, password: true }); const client = await initClient();