Infrastructure/logto
0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2025-01-27 21:39:16 -05:00
Code Issues Activity

feat(core,schemas): add mandatory password guard on register (#6368)

Browse source 
* refactor(core): refactor backup code generate flow

refactor backup code generate flow

* fix(core): fix api payload

fix api payload

* fix(core): fix rebase issue

fix rebase issue

* feat(core,schemas): add mandatory password guard on register

add mandatory password guard on register
This commit is contained in:
simeng-li 2024-08-01 10:20:23 +08:00 • committed by GitHub
parent 5fd5353383
commit dab06cb1a9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 360 additions and 122 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
packages/core/src/routes/experience/classes/experience-interaction.ts
View file

@ -1,6 +1,12 @@
/* eslint-disable max-lines */
import { type ToZodObject } from '@logto/connector-kit';
import { InteractionEvent, type User } from '@logto/schemas';
import {
InteractionEvent,
SignInIdentifier,
VerificationType,
type InteractionIdentifier,
type User,
} from '@logto/schemas';
import { conditional } from '@silverhand/essentials';
import { z } from 'zod';
@ -28,6 +34,7 @@ import { SignInExperienceValidator } from './libraries/sign-in-experience-valida
import { Mfa, mfaDataGuard, userMfaDataKey, type MfaData } from './mfa.js';
import { Profile } from './profile.js';
import { toUserSocialIdentityData } from './utils.js';
import { identifierCodeVerificationTypeMap } from './verifications/code-verification.js';
import {
buildVerificationRecord,
verificationRecordDataGuard,
@ -456,13 +463,28 @@ export default class ExperienceInteraction {
/**
* Create a new user using the verification record.
*
* @throws {RequestError} with 400 if the verification record is invalid for creating a new user or not verified
* @throws {RequestError} with 422 if a new password identity verification is provided, but identifier (email/phone) is not verified
* @throws {RequestError} with 400 if the verification record can not be used for creating a new user or not verified
* @throws {RequestError} with 422 if the profile data is not unique across users
* @throws {RequestError} with 422 if the password is required for the sign-up settings but only email/phone verification record is provided
*/
private async createNewUser(verificationRecord: VerificationRecord) {
if (verificationRecord.type === VerificationType.NewPasswordIdentity) {
const { identifier } = verificationRecord;
assertThat(
this.isIdentifierVerified(identifier),
new RequestError(
{ code: 'session.identifier_not_verified', status: 422 },
{ identifier: identifier.value }
)
);
}
const newProfile = await getNewUserProfileFromVerificationRecord(verificationRecord);
await this.profile.profileValidator.guardProfileUniquenessAcrossUsers(newProfile);
await this.signInExperienceValidator.guardMandatoryPasswordOnRegister(verificationRecord);
const user = await this.provisionLibrary.createUser(newProfile);
this.userId = user.id;
@ -500,6 +522,20 @@ export default class ExperienceInteraction {
return this.verificationRecordsArray.find((record) => record.id === verificationId);
}
private isIdentifierVerified(identifier: InteractionIdentifier) {
const { type, value } = identifier;
if (type === SignInIdentifier.Username) {
return true;
}
const verificationRecord = this.verificationRecords.get(
identifierCodeVerificationTypeMap[type]
);
return verificationRecord?.identifier.value === value && verificationRecord.isVerified;
}
/**
* Clean up the interaction storage.
*/

123
packages/core/src/routes/experience/classes/libraries/sign-in-experience-validator.test.ts
View file

@ -42,6 +42,15 @@ const newPasswordIdentityVerificationRecord = NewPasswordIdentityVerification.cr
}
);
const emailNewPasswordIdentityVerificationRecord = NewPasswordIdentityVerification.create(
mockTenant.libraries,
mockTenant.queries,
{
type: SignInIdentifier.Email,
value: `foo@${emailDomain}`,
}
);
const passwordVerificationRecords = Object.fromEntries(
Object.values(SignInIdentifier).map((identifier) => [
identifier,
@ -509,5 +518,119 @@ describe('SignInExperienceValidator', () => {
).rejects.toMatchError(expectError);
});
});
describe('guardMandatoryPasswordOnRegister', () => {
const testCases: Record<
string,
{
signInExperience: SignInExperience;
cases: Array<{ verificationRecord: VerificationRecord; accepted: boolean }>;
}
> = Object.freeze({
'should throw error for CodeVerification Records if password is required': {
signInExperience: {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email],
password: true,
verify: true,
},
},
cases: [
{
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Email],
accepted: false,
},
{
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Phone],
accepted: false,
},
],
},
'should not throw error for CodeVerification Records if password is not required': {
signInExperience: {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email],
password: false,
verify: true,
},
},
cases: [
{
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Email],
accepted: true,
},
{
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Phone],
accepted: true,
},
],
},
'should not throw error for NewPasswordIdentity verification record': {
signInExperience: {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Username],
password: true,
verify: true,
},
},
cases: [
{
verificationRecord: newPasswordIdentityVerificationRecord,
accepted: true,
},
{
verificationRecord: emailNewPasswordIdentityVerificationRecord,
accepted: true,
},
],
},
'should not throw error for Social and SSO verification records': {
signInExperience: {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email],
password: true,
verify: true,
},
},
cases: [
{
verificationRecord: socialVerificationRecord,
accepted: true,
},
{
verificationRecord: enterpriseSsoVerificationRecords,
accepted: true,
},
],
},
});
describe.each(Object.keys(testCases))(`%s`, (testCase) => {
const { signInExperience, cases } = testCases[testCase]!;
it.each(cases)('guard verification record %p', async ({ verificationRecord, accepted }) => {
signInExperiences.findDefaultSignInExperience.mockResolvedValueOnce(signInExperience);
const signInExperienceSettings = new SignInExperienceValidator(
mockTenant.libraries,
mockTenant.queries
);
await (accepted
? expect(
signInExperienceSettings.guardMandatoryPasswordOnRegister(verificationRecord)
).resolves.not.toThrow()
: expect(
signInExperienceSettings.guardMandatoryPasswordOnRegister(verificationRecord)
).rejects.toMatchError(
new RequestError({ code: 'user.password_required_in_profile', status: 422 })
));
});
});
});
});
/* eslint-enable max-lines */

19
packages/core/src/routes/experience/classes/libraries/sign-in-experience-validator.ts
View file

@ -162,6 +162,25 @@ export class SignInExperienceValidator {
return mandatoryUserProfile;
}
/**
* If password is enabled in the sign-up settings,
* guard the verification record contains password (NewPasswordIdentity).
*
* - Password is not required for social and SSO verification records.
*/
public async guardMandatoryPasswordOnRegister({ type }: VerificationRecord) {
const { signUp } = await this.getSignInExperienceData();
if (
signUp.password &&
[VerificationType.EmailVerificationCode, VerificationType.PhoneVerificationCode].includes(
type
)
) {
throw new RequestError({ code: 'user.password_required_in_profile', status: 422 });
}
}
/**
* Guard the verification records contains email identifier with SSO enabled
*

3
packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts
View file

@ -45,8 +45,7 @@ export const newPasswordIdentityVerificationRecordDataGuard = z.object({
*
* @remarks This verification record can only be used for new user registration.
* By default this verification record allows all types of identifiers, username, email, and phone.
* But in our current product design, only username + password registration is supported. The identifier type
* will be guarded at the API level.
* For email and phone identifiers, a `CodeVerification` record is required.
*/
export class NewPasswordIdentityVerification
implements VerificationRecord<VerificationType.NewPasswordIdentity>

4
packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts
View file

@ -1,4 +1,4 @@
import { newPasswordIdentityVerificationPayloadGuard, VerificationType } from '@logto/schemas';
import { passwordVerificationPayloadGuard, VerificationType } from '@logto/schemas';
import { Action } from '@logto/schemas/lib/types/log/interaction.js';
import type Router from 'koa-router';
import { z } from 'zod';
@ -17,7 +17,7 @@ export default function newPasswordIdentityVerificationRoutes<
router.post(
`${experienceRoutes.verification}/new-password-identity`,
koaGuard({
body: newPasswordIdentityVerificationPayloadGuard,
body: passwordVerificationPayloadGuard,
status: [200, 400, 422],
response: z.object({
verificationId: z.string(),

5
packages/integration-tests/src/client/experience/index.ts
View file

@ -3,7 +3,6 @@ import {
type IdentificationApiPayload,
type InteractionEvent,
type MfaFactor,
type NewPasswordIdentityVerificationPayload,
type PasswordVerificationPayload,
type UpdateProfileApiPayload,
type VerificationCodeIdentifier,
@ -196,9 +195,7 @@ export class ExperienceClient extends MockClient {
.json<{ verificationId: string }>();
}
public async createNewPasswordIdentityVerification(
payload: NewPasswordIdentityVerificationPayload
) {
public async createNewPasswordIdentityVerification(payload: PasswordVerificationPayload) {
return api
.post(`${experienceRoutes.verification}/new-password-identity`, {
headers: { cookie: this.interactionCookie },

23
packages/integration-tests/src/helpers/experience/index.ts
View file

@ -109,28 +109,29 @@ export const registerNewUserWithVerificationCode = async (
interactionEvent: InteractionEvent.Register,
});
const verifiedVerificationId = await successfullyVerifyVerificationCode(client, {
await successfullyVerifyVerificationCode(client, {
identifier,
verificationId,
code,
});
await client.identifyUser({
verificationId: verifiedVerificationId,
});
if (options?.fulfillPassword) {
await expectRejects(client.submitInteraction(), {
code: 'user.missing_profile',
await expectRejects(client.identifyUser({ verificationId }), {
code: 'user.password_required_in_profile',
status: 422,
});
const password = generatePassword();
await client.updateProfile({
type: 'password',
value: password,
});
const { verificationId: newPasswordIdentityVerificationId } =
await client.createNewPasswordIdentityVerification({
identifier,
password,
});
await client.identifyUser({ verificationId: newPasswordIdentityVerificationId });
} else {
await client.identifyUser({ verificationId });
}
const { redirectTo } = await client.submitInteraction();

136
packages/integration-tests/src/tests/api/experience-api/register-interaction/verification-code.test.ts
View file

@ -15,7 +15,7 @@ import {
import { expectRejects } from '#src/helpers/index.js';
import { enableAllVerificationCodeSignInMethods } from '#src/helpers/sign-in-experience.js';
import { generateNewUser } from '#src/helpers/user.js';
import { devFeatureTest, generateEmail, generatePhone } from '#src/utils.js';
import { devFeatureTest, generateEmail, generatePassword, generatePhone } from '#src/utils.js';
const verificationIdentifierType: readonly [SignInIdentifier.Email, SignInIdentifier.Phone] =
Object.freeze([SignInIdentifier.Email, SignInIdentifier.Phone]);
@ -47,56 +47,68 @@ devFeatureTest.describe('Register interaction with verification code happy path'
}
);
it.each(verificationIdentifierType)(
'Should fail to sign-up with existing %p identifier and directly sign-in instead ',
async (identifierType) => {
const { userProfile, user } = await generateNewUser({
[identifiersTypeToUserProfile[identifierType]]: true,
describe('sign-up with existing identifier', () => {
beforeAll(async () => {
await Promise.all([setEmailConnector(), setSmsConnector()]);
await enableAllVerificationCodeSignInMethods({
identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone],
password: true,
verify: true,
});
});
const identifier: VerificationCodeIdentifier = {
type: identifierType,
value: userProfile[identifiersTypeToUserProfile[identifierType]]!,
};
it.each(verificationIdentifierType)(
'Should fail to sign-up with existing %p identifier and directly sign-in instead ',
async (identifierType) => {
const { userProfile, user } = await generateNewUser({
[identifiersTypeToUserProfile[identifierType]]: true,
password: true,
});
const client = await initExperienceClient(InteractionEvent.Register);
const identifier: VerificationCodeIdentifier = {
type: identifierType,
value: userProfile[identifiersTypeToUserProfile[identifierType]]!,
};
const { verificationId, code } = await successfullySendVerificationCode(client, {
identifier,
interactionEvent: InteractionEvent.Register,
});
const client = await initExperienceClient(InteractionEvent.Register);
await successfullyVerifyVerificationCode(client, {
identifier,
verificationId,
code,
});
const { verificationId, code } = await successfullySendVerificationCode(client, {
identifier,
interactionEvent: InteractionEvent.Register,
});
await expectRejects(
client.identifyUser({
await successfullyVerifyVerificationCode(client, {
identifier,
verificationId,
}),
{
code: `user.${identifierType}_already_in_use`,
status: 422,
}
);
code,
});
await client.updateInteractionEvent({
interactionEvent: InteractionEvent.SignIn,
});
await expectRejects(
client.identifyUser({
verificationId,
}),
{
code: `user.${identifierType}_already_in_use`,
status: 422,
}
);
await client.identifyUser({
verificationId,
});
await client.updateInteractionEvent({
interactionEvent: InteractionEvent.SignIn,
});
const { redirectTo } = await client.submitInteraction();
await processSession(client, redirectTo);
await logoutClient(client);
await client.identifyUser({
verificationId,
});
await deleteUser(user.id);
}
);
const { redirectTo } = await client.submitInteraction();
await processSession(client, redirectTo);
await logoutClient(client);
await deleteUser(user.id);
}
);
});
describe('fulfill password', () => {
beforeAll(async () => {
@ -107,6 +119,50 @@ devFeatureTest.describe('Register interaction with verification code happy path'
});
});
it.each(verificationIdentifierType)(
'Should throw identifier not verified error when trying to fulfill password without verifying %p identifier',
async (identifier) => {
const client = await initExperienceClient(InteractionEvent.Register);
const interactionIdentifier = {
type: identifier,
value: identifier === SignInIdentifier.Email ? generateEmail() : generatePhone(),
};
const { verificationId } = await client.createNewPasswordIdentityVerification({
identifier: interactionIdentifier,
password: generatePassword(),
});
await expectRejects(client.identifyUser({ verificationId }), {
code: 'session.identifier_not_verified',
status: 422,
});
const { verificationId: codeVerificationId, code } = await successfullySendVerificationCode(
client,
{
identifier: interactionIdentifier,
interactionEvent: InteractionEvent.Register,
}
);
await successfullyVerifyVerificationCode(client, {
identifier: interactionIdentifier,
verificationId: codeVerificationId,
code,
});
await client.identifyUser({ verificationId });
const { redirectTo } = await client.submitInteraction();
const userId = await processSession(client, redirectTo);
await logoutClient(client);
await deleteUser(userId);
}
);
it.each(verificationIdentifierType)(
'Should register with verification code using %p and fulfill the password successfully',
async (identifier) => {

105
packages/integration-tests/src/tests/api/experience-api/verifications/new-password-identity-verification.test.ts
View file

@ -3,22 +3,12 @@ import { SignInIdentifier } from '@logto/schemas';
import { updateSignInExperience } from '#src/api/sign-in-experience.js';
import { initExperienceClient } from '#src/helpers/client.js';
import { expectRejects } from '#src/helpers/index.js';
import { generateNewUser } from '#src/helpers/user.js';
import { generateNewUserProfile, UserApiTest } from '#src/helpers/user.js';
import { devFeatureTest, randomString } from '#src/utils.js';
const invalidIdentifiers = Object.freeze([
{
type: SignInIdentifier.Email,
value: 'email',
},
{
type: SignInIdentifier.Phone,
value: 'phone',
},
]);
devFeatureTest.describe('password verifications', () => {
const username = 'test_' + randomString();
const userApi = new UserApiTest();
beforeAll(async () => {
await updateSignInExperience({
@ -35,6 +25,10 @@ devFeatureTest.describe('password verifications', () => {
});
});
afterEach(async () => {
await userApi.cleanUp();
});
afterAll(async () => {
await updateSignInExperience({
// Need to reset password policy to default value otherwise it will affect other tests.
@ -42,44 +36,77 @@ devFeatureTest.describe('password verifications', () => {
});
});
it.each(invalidIdentifiers)(
'should fail to verify with password using %p',
async (identifier) => {
describe('invalid identifier check', () => {
it('should throw error if username is registered', async () => {
const { username, password } = generateNewUserProfile({ username: true, password: true });
await userApi.create({ username, password });
const client = await initExperienceClient();
await expectRejects(
client.createNewPasswordIdentityVerification({
// @ts-expect-error
identifier,
password: 'password',
identifier: {
type: SignInIdentifier.Username,
value: username,
},
password,
}),
{
code: 'guard.invalid_input',
status: 400,
code: 'user.username_already_in_use',
status: 422,
}
);
}
);
});
it('should throw error if username is registered', async () => {
const { userProfile } = await generateNewUser({ username: true, password: true });
const { username } = userProfile;
it('should throw error if email is registered', async () => {
const { primaryEmail, password } = generateNewUserProfile({
primaryEmail: true,
password: true,
});
const client = await initExperienceClient();
await userApi.create({ primaryEmail, password });
await expectRejects(
client.createNewPasswordIdentityVerification({
identifier: {
type: SignInIdentifier.Username,
value: username,
},
password: 'password',
}),
{
code: 'user.username_already_in_use',
status: 422,
}
);
const client = await initExperienceClient();
await expectRejects(
client.createNewPasswordIdentityVerification({
identifier: {
type: SignInIdentifier.Email,
value: primaryEmail,
},
password,
}),
{
code: 'user.email_already_in_use',
status: 422,
}
);
});
it('should throw error if phone is registered', async () => {
const { primaryPhone, password } = generateNewUserProfile({
primaryPhone: true,
password: true,
});
await userApi.create({ primaryPhone, password });
const client = await initExperienceClient();
await expectRejects(
client.createNewPasswordIdentityVerification({
identifier: {
type: SignInIdentifier.Phone,
value: primaryPhone,
},
password,
}),
{
code: 'user.phone_already_in_use',
status: 422,
}
);
});
});
describe('password policy check', () => {

2
packages/phrases/src/locales/en/errors/session.ts
View file

@ -27,6 +27,8 @@ const session = {
not_supported_for_forgot_password: 'This operation is not supported for forgot password.',
identity_conflict:
'Identity mismatch detected. Please initiate a new session to proceed with a different identity.',
identifier_not_verified:
'The provided identifier {{identifier}} has not been verified. Please create a verification record for this identifier and complete the verification process.',
mfa: {
require_mfa_verification: 'Mfa verification is required to sign in.',
mfa_sign_in_only: 'Mfa is only available for sign-in interaction.',

22
packages/schemas/src/types/interactions.ts
View file

@ -1,4 +1,3 @@
/* eslint-disable max-lines */
import { emailRegEx, phoneRegEx, usernameRegEx } from '@logto/core-kit';
import { z } from 'zod';
@ -121,26 +120,6 @@ export const backupCodeVerificationVerifyPayloadGuard = z.object({
code: z.string().min(1),
}) satisfies ToZodObject<BackupCodeVerificationVerifyPayload>;
/**
* Payload type for `POST /api/experience/verification/new-password-identity`.
* @remarks Currently we only support username identifier for new password identity registration.
* For email and phone new identity registration, a `CodeVerification` record is required.
*/
export type NewPasswordIdentityVerificationPayload = {
identifier: {
type: SignInIdentifier.Username;
value: string;
};
password: string;
};
export const newPasswordIdentityVerificationPayloadGuard = z.object({
identifier: z.object({
type: z.literal(SignInIdentifier.Username),
value: z.string(),
}),
password: z.string().min(1),
}) satisfies ToZodObject<NewPasswordIdentityVerificationPayload>;
/** Payload type for `POST /api/experience/identification`. */
export type IdentificationApiPayload = {
/** The ID of the verification record that is used to identify the user. */
@ -432,4 +411,3 @@ export const verifyMfaResultGuard = z.object({
});
export type VerifyMfaResult = z.infer<typeof verifyMfaResultGuard>;
/* eslint-enable max-lines */