Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): add password algorithm transition #5481

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@ -248,13 +194,75 @@
};

const addUserMfaVerification = async (userId: string, payload: BindMfa) => {
// TODO @sijie use jsonb array append

Check warning on line 197 in packages/core/src/libraries/user.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/libraries/user.ts#L197

[no-warning-comments] Unexpected 'todo' comment: 'TODO @sijie use jsonb array append'.
const { mfaVerifications } = await findUserById(userId);
await updateUserById(userId, {
mfaVerifications: [...mfaVerifications, converBindMfaToMfaVerification(payload)],
});
};

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 });
gao-sun marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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 @@
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
Loading