Skip to content

Commit

Permalink
feat(core): add password algorithm transition (#5481)
Browse files Browse the repository at this point in the history
  • Loading branch information
wangsijie authored Mar 8, 2024
1 parent 1724119 commit 95f4ba1
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 79 deletions.
20 changes: 19 additions & 1 deletion packages/core/src/libraries/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -70,6 +70,8 @@ describe('encryptUserPassword()', () => {
});

describe('verifyUserPassword()', () => {
const { verifyUserPassword } = createUserLibrary(queries);

describe('Argon2', () => {
it('resolves when password is correct', async () => {
await expect(
Expand Down Expand Up @@ -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()', () => {
Expand Down
117 changes: 63 additions & 54 deletions packages/core/src/libraries/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,60 +31,6 @@ export const encryptUserPassword = async (
return { passwordEncrypted, passwordEncryptionMethod };
};

export const verifyUserPassword = async (user: Nullable<User>, password: string): Promise<User> => {
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
Expand Down Expand Up @@ -255,6 +201,68 @@ export const createUserLibrary = (queries: Queries) => {
});
};

const verifyUserPassword = async (user: Nullable<User>, password: string): Promise<User> => {
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,
Expand All @@ -263,5 +271,6 @@ export const createUserLibrary = (queries: Queries) => {
findUserScopesForResourceIndicator,
findUserRoles,
addUserMfaVerification,
verifyUserPassword,
};
};
4 changes: 2 additions & 2 deletions packages/core/src/routes-me/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -20,7 +20,7 @@ export default function userRoutes<T extends AuthedMeRouter>(
users: { findUserById, updateUserById },
},
libraries: {
users: { checkIdentifierCollision },
users: { checkIdentifierCollision, verifyUserPassword },
verificationStatuses: { createVerificationStatus, checkVerificationStatus },
},
} = tenant;
Expand Down
20 changes: 9 additions & 11 deletions packages/core/src/routes/admin-user/basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -88,6 +85,7 @@ const usersLibraries = {
...removeUndefinedKeys(user), // No undefined values will be returned from database
})
),
verifyUserPassword,
} satisfies Partial<Libraries['users']>;

const adminUserRoutes = await pickDefault(import('./basics.js'));
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/routes/admin-user/basics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -30,7 +30,7 @@ export default function adminUserBasicsRoutes<T extends AuthedRouter>(...args: R
userSsoIdentities,
} = queries;
const {
users: { checkIdentifierCollision, generateUserId, insertUser },
users: { checkIdentifierCollision, generateUserId, insertUser, verifyUserPassword },
} = libraries;

router.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { InteractionEvent, ConnectorType, SignInIdentifier } from '@logto/schemas';
import {
InteractionEvent,
ConnectorType,
SignInIdentifier,
UsersPasswordEncryptionMethod,
} from '@logto/schemas';

import {
putInteraction,
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 95f4ba1

Please sign in to comment.