Infrastructure/logto
0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2024-12-30 20:33:54 -05:00
Code Issues Activity

refactor(core): make password optional in NewPasswordIdentity (#6377)

Browse source 
refactor(core): make password optional in NewPasswordIdentity verification

make password optioanl in NewPasswordIdentity verification
This commit is contained in:
simeng-li 2024-08-01 17:17:51 +08:00 committed by GitHub
parent 323a5650f0
commit a1f6009cc5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 170 additions and 167 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -483,8 +483,6 @@ export default class ExperienceInteraction {
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;

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

@ -1,4 +1,5 @@
/* eslint-disable max-lines */
import { type LogtoErrorCode } from '@logto/phrases';
import {
InteractionEvent,
type SignInExperience,
@ -339,7 +340,11 @@ describe('SignInExperienceValidator', () => {
string,
{
signInExperience: SignInExperience;
cases: Array<{ verificationRecord: VerificationRecord; accepted: boolean }>;
cases: Array<{
verificationRecord: VerificationRecord;
accepted: boolean;
errorCode?: LogtoErrorCode;
}>;
}
> = Object.freeze({
'only username is enabled for sign-up': {
@ -364,7 +369,7 @@ describe('SignInExperienceValidator', () => {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email],
password: true,
password: false,
verify: true,
},
},
@ -377,6 +382,10 @@ describe('SignInExperienceValidator', () => {
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Phone],
accepted: false,
},
{
verificationRecord: emailNewPasswordIdentityVerificationRecord,
accepted: false,
},
],
},
'email and phone are enabled for sign-up': {
@ -384,7 +393,7 @@ describe('SignInExperienceValidator', () => {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone],
password: true,
password: false,
verify: true,
},
},
@ -399,6 +408,27 @@ describe('SignInExperienceValidator', () => {
},
],
},
'email are enabled for sign-up but password is required': {
signInExperience: {
...mockSignInExperience,
signUp: {
identifiers: [SignInIdentifier.Email],
password: true,
verify: true,
},
},
cases: [
{
verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Email],
accepted: false,
errorCode: 'user.password_required_in_profile',
},
{
verificationRecord: emailNewPasswordIdentityVerificationRecord,
accepted: true,
},
],
},
'enterprise sso enabled': {
signInExperience: {
...mockSignInExperience,
@ -428,30 +458,36 @@ describe('SignInExperienceValidator', () => {
describe.each(Object.keys(registerVerificationTestCases))(`%s`, (testCase) => {
const { signInExperience, cases } = registerVerificationTestCases[testCase]!;
it.each(cases)('guard verification record %p', async ({ verificationRecord, accepted }) => {
signInExperiences.findDefaultSignInExperience.mockResolvedValueOnce(signInExperience);
it.each(cases)(
'guard verification record %p',
async ({ verificationRecord, accepted, errorCode }) => {
signInExperiences.findDefaultSignInExperience.mockResolvedValueOnce(signInExperience);
const signInExperienceSettings = new SignInExperienceValidator(
mockTenant.libraries,
mockTenant.queries
);
const signInExperienceSettings = new SignInExperienceValidator(
mockTenant.libraries,
mockTenant.queries
);
await (accepted
? expect(
signInExperienceSettings.verifyIdentificationMethod(
InteractionEvent.Register,
verificationRecord
)
).resolves.not.toThrow()
: expect(
signInExperienceSettings.verifyIdentificationMethod(
InteractionEvent.Register,
verificationRecord
)
).rejects.toMatchError(
new RequestError({ code: 'user.sign_up_method_not_enabled', status: 422 })
));
});
await (accepted
? expect(
signInExperienceSettings.verifyIdentificationMethod(
InteractionEvent.Register,
verificationRecord
)
).resolves.not.toThrow()
: expect(
signInExperienceSettings.verifyIdentificationMethod(
InteractionEvent.Register,
verificationRecord
)
).rejects.toMatchError(
new RequestError({
code: errorCode ?? 'user.sign_up_method_not_enabled',
status: 422,
})
));
}
);
});
});
@ -518,119 +554,5 @@ 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 */

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

@ -162,25 +162,6 @@ 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
*
@ -283,6 +264,11 @@ export class SignInExperienceValidator {
signUp.identifiers.includes(type) && signUp.verify,
new RequestError({ code: 'user.sign_up_method_not_enabled', status: 422 })
);
assertThat(
!signUp.password,
new RequestError({ code: 'user.password_required_in_profile', status: 422 })
);
break;
}
case VerificationType.Social: {

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

@ -93,12 +93,21 @@ export class NewPasswordIdentityVerification
*
* - Check if the identifier is unique across users
* - Validate the password against the password policy
*
* @throws {RequestError} with status 422 if the identifier is in use by another user
* @throws {RequestError} with status 422 if the password is not provided
* @throws {RequestError} with status 422 if the password does not meet the password policy
*/
async verify(password: string) {
async verify(password?: string) {
const { identifier } = this;
const identifierProfile = interactionIdentifierToUserProfile(identifier);
await this.profileValidator.guardProfileUniquenessAcrossUsers(identifierProfile);
assertThat(
password,
new RequestError({ code: 'user.password_required_in_profile', status: 422 })
);
const passwordPolicy = await this.signInExperienceValidator.getPasswordPolicy();
const passwordValidator = new PasswordValidator(passwordPolicy);
await passwordValidator.validatePassword(password, identifierProfile);

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

@ -1,4 +1,4 @@
import { passwordVerificationPayloadGuard, VerificationType } from '@logto/schemas';
import { interactionIdentifierGuard, 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,10 @@ export default function newPasswordIdentityVerificationRoutes<
router.post(
`${experienceRoutes.verification}/new-password-identity`,
koaGuard({
body: passwordVerificationPayloadGuard,
body: z.object({
identifier: interactionIdentifierGuard,
password: z.string().optional(),
}),
status: [200, 400, 422],
response: z.object({
verificationId: z.string(),

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

@ -195,7 +195,9 @@ export class ExperienceClient extends MockClient {
.json<{ verificationId: string }>();
}
public async createNewPasswordIdentityVerification(payload: PasswordVerificationPayload) {
public async createNewPasswordIdentityVerification(
payload: Pick<PasswordVerificationPayload, 'identifier'> & { password?: string }
) {
return api
.post(`${experienceRoutes.verification}/new-password-identity`, {
headers: { cookie: this.interactionCookie },

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

@ -52,13 +52,13 @@ devFeatureTest.describe('Register interaction with verification code happy path'
await Promise.all([setEmailConnector(), setSmsConnector()]);
await enableAllVerificationCodeSignInMethods({
identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone],
password: true,
password: false,
verify: true,
});
});
it.each(verificationIdentifierType)(
'Should fail to sign-up with existing %p identifier and directly sign-in instead ',
'Should fail to sign-up with existing %p identifier and directly sign-in instead',
async (identifierType) => {
const { userProfile, user } = await generateNewUser({
[identifiersTypeToUserProfile[identifierType]]: true,
@ -110,7 +110,7 @@ devFeatureTest.describe('Register interaction with verification code happy path'
);
});
describe('fulfill password', () => {
describe('password enabled', () => {
beforeAll(async () => {
await enableAllVerificationCodeSignInMethods({
identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone],
@ -163,6 +163,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,
password: true,
});
const identifier: VerificationCodeIdentifier = {
type: identifierType,
value: userProfile[identifiersTypeToUserProfile[identifierType]]!,
};
const client = await initExperienceClient(InteractionEvent.Register);
const { verificationId, code } = await successfullySendVerificationCode(client, {
identifier,
interactionEvent: InteractionEvent.Register,
});
await successfullyVerifyVerificationCode(client, {
identifier,
verificationId,
code,
});
await expectRejects(
client.identifyUser({
verificationId,
}),
{
code: `user.password_required_in_profile`,
status: 422,
}
);
await expectRejects(
client.createNewPasswordIdentityVerification({
identifier,
}),
{
code: `user.${identifierType}_already_in_use`,
status: 422,
}
);
await client.updateInteractionEvent({
interactionEvent: InteractionEvent.SignIn,
});
await client.identifyUser({
verificationId,
});
const { redirectTo } = await client.submitInteraction();
await processSession(client, redirectTo);
await logoutClient(client);
await deleteUser(user.id);
}
);
it.each(verificationIdentifierType)(
'Should register with verification code using %p and fulfill the password successfully',
async (identifier) => {

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

@ -119,6 +119,27 @@ devFeatureTest.describe('password verifications', () => {
[username, 'userInfo'],
];
it('should throw error if password is not provided', async () => {
const { primaryEmail } = generateNewUserProfile({
primaryEmail: true,
});
const client = await initExperienceClient();
await expectRejects(
client.createNewPasswordIdentityVerification({
identifier: {
type: SignInIdentifier.Email,
value: primaryEmail,
},
}),
{
code: 'user.password_required_in_profile',
status: 422,
}
);
});
it.each(invalidPasswords)('should reject invalid password %p', async (password) => {
const client = await initExperienceClient();