diff --git a/packages/core/src/routes/profile/email-and-phone.ts b/packages/core/src/routes/profile/email-and-phone.ts new file mode 100644 index 000000000..accd2aa78 --- /dev/null +++ b/packages/core/src/routes/profile/email-and-phone.ts @@ -0,0 +1,192 @@ +import { emailRegEx, phoneRegEx, UserScope } from '@logto/core-kit'; +import { VerificationType, AccountCenterControlValue, SignInIdentifier } from '@logto/schemas'; +import { z } from 'zod'; + +import koaGuard from '#src/middleware/koa-guard.js'; + +import RequestError from '../../errors/RequestError/index.js'; +import { buildVerificationRecordByIdAndType } from '../../libraries/verification.js'; +import assertThat from '../../utils/assert-that.js'; +import type { UserRouter, RouterInitArgs } from '../types.js'; + +export default function emailAndPhoneRoutes(...args: RouterInitArgs) { + const [router, { queries, libraries }] = args; + const { + users: { updateUserById, findUserById }, + signInExperiences: { findDefaultSignInExperience }, + } = queries; + + const { + users: { checkIdentifierCollision }, + } = libraries; + + router.post( + '/profile/primary-email', + koaGuard({ + body: z.object({ + email: z.string().regex(emailRegEx), + newIdentifierVerificationRecordId: z.string(), + }), + status: [204, 400, 401], + }), + async (ctx, next) => { + const { id: userId, scopes, identityVerified } = ctx.auth; + assertThat( + identityVerified, + new RequestError({ code: 'verification_record.permission_denied', status: 401 }) + ); + const { email, newIdentifierVerificationRecordId } = ctx.guard.body; + const { fields } = ctx.accountCenter; + assertThat( + fields.email === AccountCenterControlValue.Edit, + 'account_center.filed_not_editable' + ); + + assertThat(scopes.has(UserScope.Email), 'auth.unauthorized'); + + // Check new identifier + const newVerificationRecord = await buildVerificationRecordByIdAndType({ + type: VerificationType.EmailVerificationCode, + id: newIdentifierVerificationRecordId, + queries, + libraries, + }); + assertThat(newVerificationRecord.isVerified, 'verification_record.not_found'); + assertThat(newVerificationRecord.identifier.value === email, 'verification_record.not_found'); + + await checkIdentifierCollision({ primaryEmail: email }, userId); + + const updatedUser = await updateUserById(userId, { primaryEmail: email }); + + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); + + ctx.status = 204; + + return next(); + } + ); + + router.delete( + '/profile/primary-email', + koaGuard({ + status: [204, 400, 401], + }), + async (ctx, next) => { + const { id: userId, scopes, identityVerified } = ctx.auth; + assertThat( + identityVerified, + new RequestError({ code: 'verification_record.permission_denied', status: 401 }) + ); + const { fields } = ctx.accountCenter; + assertThat( + fields.email === AccountCenterControlValue.Edit, + 'account_center.filed_not_editable' + ); + + assertThat(scopes.has(UserScope.Email), 'auth.unauthorized'); + + const { signUp } = await findDefaultSignInExperience(); + + if (signUp.identifiers.includes(SignInIdentifier.Email)) { + // If email is the only sign-up identifier, we need to keep the email + assertThat(signUp.identifiers.includes(SignInIdentifier.Phone), 'user.email_required'); + // If phone is also a sign-up identifier, check if phone is set + const user = await findUserById(userId); + assertThat(user.primaryPhone, 'user.email_or_phone_required'); + } + + const updatedUser = await updateUserById(userId, { primaryEmail: null }); + + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); + + ctx.status = 204; + + return next(); + } + ); + + router.post( + '/profile/primary-phone', + koaGuard({ + body: z.object({ + phone: z.string().regex(phoneRegEx), + newIdentifierVerificationRecordId: z.string(), + }), + status: [204, 400, 401], + }), + async (ctx, next) => { + const { id: userId, scopes, identityVerified } = ctx.auth; + assertThat( + identityVerified, + new RequestError({ code: 'verification_record.permission_denied', status: 401 }) + ); + const { phone, newIdentifierVerificationRecordId } = ctx.guard.body; + const { fields } = ctx.accountCenter; + assertThat( + fields.phone === AccountCenterControlValue.Edit, + 'account_center.filed_not_editable' + ); + + assertThat(scopes.has(UserScope.Phone), 'auth.unauthorized'); + + // Check new identifier + const newVerificationRecord = await buildVerificationRecordByIdAndType({ + type: VerificationType.PhoneVerificationCode, + id: newIdentifierVerificationRecordId, + queries, + libraries, + }); + assertThat(newVerificationRecord.isVerified, 'verification_record.not_found'); + assertThat(newVerificationRecord.identifier.value === phone, 'verification_record.not_found'); + + await checkIdentifierCollision({ primaryPhone: phone }, userId); + + const updatedUser = await updateUserById(userId, { primaryPhone: phone }); + + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); + + ctx.status = 204; + + return next(); + } + ); + + router.delete( + '/profile/primary-phone', + koaGuard({ + status: [204, 400, 401], + }), + async (ctx, next) => { + const { id: userId, scopes, identityVerified } = ctx.auth; + assertThat( + identityVerified, + new RequestError({ code: 'verification_record.permission_denied', status: 401 }) + ); + const { fields } = ctx.accountCenter; + assertThat( + fields.phone === AccountCenterControlValue.Edit, + 'account_center.filed_not_editable' + ); + + assertThat(scopes.has(UserScope.Phone), 'auth.unauthorized'); + + const { signUp } = await findDefaultSignInExperience(); + + if (signUp.identifiers.includes(SignInIdentifier.Phone)) { + // If phone is the only sign-up identifier, we need to keep the phone + assertThat(signUp.identifiers.includes(SignInIdentifier.Email), 'user.phone_required'); + // If email is also a sign-up identifier, check if email is set + const user = await findUserById(userId); + assertThat(user.primaryEmail, 'user.email_or_phone_required'); + } + + const updatedUser = await updateUserById(userId, { primaryPhone: null }); + + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); + + ctx.status = 204; + + return next(); + } + ); +} diff --git a/packages/core/src/routes/profile/index.openapi.json b/packages/core/src/routes/profile/index.openapi.json index f0849d742..d6aea6bcd 100644 --- a/packages/core/src/routes/profile/index.openapi.json +++ b/packages/core/src/routes/profile/index.openapi.json @@ -174,6 +174,16 @@ "description": "Permission denied, the verification record is invalid." } } + }, + "delete": { + "operationId": "DeletePrimaryEmail", + "summary": "Delete primary email", + "description": "Delete primary email for the user, a verification-record-id in header is required for checking sensitive permissions.", + "responses": { + "204": { + "description": "The primary email was deleted successfully." + } + } } }, "/api/profile/primary-phone": { @@ -208,6 +218,16 @@ "description": "Permission denied, the verification record is invalid." } } + }, + "delete": { + "operationId": "DeletePrimaryPhone", + "summary": "Delete primary phone", + "description": "Delete primary phone for the user, a verification-record-id in header is required for checking sensitive permissions.", + "responses": { + "204": { + "description": "The primary phone was deleted successfully." + } + } } }, "/api/profile/identities": { diff --git a/packages/core/src/routes/profile/index.ts b/packages/core/src/routes/profile/index.ts index 998ce465f..ec74c7c48 100644 --- a/packages/core/src/routes/profile/index.ts +++ b/packages/core/src/routes/profile/index.ts @@ -1,9 +1,9 @@ -import { emailRegEx, phoneRegEx, usernameRegEx, UserScope } from '@logto/core-kit'; +import { usernameRegEx, UserScope } from '@logto/core-kit'; import { - VerificationType, userProfileResponseGuard, userProfileGuard, AccountCenterControlValue, + SignInIdentifier, } from '@logto/schemas'; import { z } from 'zod'; @@ -12,11 +12,11 @@ import koaGuard from '#src/middleware/koa-guard.js'; import { EnvSet } from '../../env-set/index.js'; import RequestError from '../../errors/RequestError/index.js'; import { encryptUserPassword } from '../../libraries/user.utils.js'; -import { buildVerificationRecordByIdAndType } from '../../libraries/verification.js'; import assertThat from '../../utils/assert-that.js'; import { PasswordValidator } from '../experience/classes/libraries/password-validator.js'; import type { UserRouter, RouterInitArgs } from '../types.js'; +import emailAndPhoneRoutes from './email-and-phone.js'; import identitiesRoutes from './identities.js'; import koaAccountCenter from './middlewares/koa-account-center.js'; import { getAccountCenterFilteredProfile, getScopedProfile } from './utils/get-scoped-profile.js'; @@ -24,7 +24,7 @@ import { getAccountCenterFilteredProfile, getScopedProfile } from './utils/get-s export default function profileRoutes(...args: RouterInitArgs) { const [router, { queries, libraries }] = args; const { - users: { updateUserById, findUserById, deleteUserIdentity }, + users: { updateUserById, findUserById }, signInExperiences: { findDefaultSignInExperience }, } = queries; @@ -58,7 +58,7 @@ export default function profileRoutes(...args: RouterInitA body: z.object({ name: z.string().nullable().optional(), avatar: z.string().url().nullable().optional(), - username: z.string().regex(usernameRegEx).optional(), + username: z.string().regex(usernameRegEx).nullable().optional(), }), response: userProfileResponseGuard.partial(), status: [200, 400, 422], @@ -84,7 +84,15 @@ export default function profileRoutes(...args: RouterInitA assertThat(scopes.has(UserScope.Profile), 'auth.unauthorized'); if (username !== undefined) { - await checkIdentifierCollision({ username }, userId); + if (username === null) { + const { signUp } = await findDefaultSignInExperience(); + assertThat( + !signUp.identifiers.includes(SignInIdentifier.Username), + 'user.username_required' + ); + } else { + await checkIdentifierCollision({ username }, userId); + } } const updatedUser = await updateUserById(userId, { @@ -175,97 +183,6 @@ export default function profileRoutes(...args: RouterInitA } ); - router.post( - '/profile/primary-email', - koaGuard({ - body: z.object({ - email: z.string().regex(emailRegEx), - newIdentifierVerificationRecordId: z.string(), - }), - status: [204, 400, 401], - }), - async (ctx, next) => { - const { id: userId, scopes, identityVerified } = ctx.auth; - assertThat( - identityVerified, - new RequestError({ code: 'verification_record.permission_denied', status: 401 }) - ); - const { email, newIdentifierVerificationRecordId } = ctx.guard.body; - const { fields } = ctx.accountCenter; - assertThat( - fields.email === AccountCenterControlValue.Edit, - 'account_center.filed_not_editable' - ); - - assertThat(scopes.has(UserScope.Email), 'auth.unauthorized'); - - // Check new identifier - const newVerificationRecord = await buildVerificationRecordByIdAndType({ - type: VerificationType.EmailVerificationCode, - id: newIdentifierVerificationRecordId, - queries, - libraries, - }); - assertThat(newVerificationRecord.isVerified, 'verification_record.not_found'); - assertThat(newVerificationRecord.identifier.value === email, 'verification_record.not_found'); - - await checkIdentifierCollision({ primaryEmail: email }, userId); - - const updatedUser = await updateUserById(userId, { primaryEmail: email }); - - ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); - - ctx.status = 204; - - return next(); - } - ); - - router.post( - '/profile/primary-phone', - koaGuard({ - body: z.object({ - phone: z.string().regex(phoneRegEx), - newIdentifierVerificationRecordId: z.string(), - }), - status: [204, 400, 401], - }), - async (ctx, next) => { - const { id: userId, scopes, identityVerified } = ctx.auth; - assertThat( - identityVerified, - new RequestError({ code: 'verification_record.permission_denied', status: 401 }) - ); - const { phone, newIdentifierVerificationRecordId } = ctx.guard.body; - const { fields } = ctx.accountCenter; - assertThat( - fields.phone === AccountCenterControlValue.Edit, - 'account_center.filed_not_editable' - ); - - assertThat(scopes.has(UserScope.Phone), 'auth.unauthorized'); - - // Check new identifier - const newVerificationRecord = await buildVerificationRecordByIdAndType({ - type: VerificationType.PhoneVerificationCode, - id: newIdentifierVerificationRecordId, - queries, - libraries, - }); - assertThat(newVerificationRecord.isVerified, 'verification_record.not_found'); - assertThat(newVerificationRecord.identifier.value === phone, 'verification_record.not_found'); - - await checkIdentifierCollision({ primaryPhone: phone }, userId); - - const updatedUser = await updateUserById(userId, { primaryPhone: phone }); - - ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); - - ctx.status = 204; - - return next(); - } - ); - + emailAndPhoneRoutes(...args); identitiesRoutes(...args); } diff --git a/packages/integration-tests/src/api/profile.ts b/packages/integration-tests/src/api/profile.ts index cf6ccec67..fee1c82e7 100644 --- a/packages/integration-tests/src/api/profile.ts +++ b/packages/integration-tests/src/api/profile.ts @@ -24,6 +24,11 @@ export const updatePrimaryEmail = async ( headers: { [verificationRecordIdHeader]: verificationRecordId }, }); +export const deletePrimaryEmail = async (api: KyInstance, verificationRecordId: string) => + api.delete('api/profile/primary-email', { + headers: { [verificationRecordIdHeader]: verificationRecordId }, + }); + export const updatePrimaryPhone = async ( api: KyInstance, phone: string, @@ -35,6 +40,11 @@ export const updatePrimaryPhone = async ( headers: { [verificationRecordIdHeader]: verificationRecordId }, }); +export const deletePrimaryPhone = async (api: KyInstance, verificationRecordId: string) => + api.delete('api/profile/primary-phone', { + headers: { [verificationRecordIdHeader]: verificationRecordId }, + }); + export const updateIdentities = async ( api: KyInstance, verificationRecordId: string, diff --git a/packages/integration-tests/src/tests/api/profile/email-and-phone.test.ts b/packages/integration-tests/src/tests/api/profile/email-and-phone.test.ts index 619197cdf..e24c7cd55 100644 --- a/packages/integration-tests/src/tests/api/profile/email-and-phone.test.ts +++ b/packages/integration-tests/src/tests/api/profile/email-and-phone.test.ts @@ -3,7 +3,13 @@ import { SignInIdentifier } from '@logto/schemas'; import { enableAllAccountCenterFields } from '#src/api/account-center.js'; import { authedAdminApi } from '#src/api/api.js'; -import { getUserInfo, updatePrimaryEmail, updatePrimaryPhone } from '#src/api/profile.js'; +import { + deletePrimaryEmail, + deletePrimaryPhone, + getUserInfo, + updatePrimaryEmail, + updatePrimaryPhone, +} from '#src/api/profile.js'; import { createAndVerifyVerificationCode, createVerificationRecordByPassword, @@ -135,6 +141,102 @@ describe('profile (email and phone)', () => { }); }); + describe('DELETE /profile/primary-email', () => { + it('should fail if scope is missing', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + + await expectRejects(deletePrimaryEmail(api, verificationRecordId), { + code: 'auth.unauthorized', + status: 400, + }); + + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if verification record is invalid', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + + await expectRejects(deletePrimaryEmail(api, 'invalid-verification-record-id'), { + code: 'verification_record.permission_denied', + status: 401, + }); + + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if email is the only sign-up identifier', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Email], + password: true, + verify: true, + }); + + await expectRejects(deletePrimaryEmail(api, verificationRecordId), { + code: 'user.email_required', + status: 400, + }); + + await enableAllPasswordSignInMethods(); + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if email or phone is the sign-up identifier', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone], + password: true, + verify: true, + }); + + await expectRejects(deletePrimaryEmail(api, verificationRecordId), { + code: 'user.email_or_phone_required', + status: 400, + }); + + await enableAllPasswordSignInMethods(); + await deleteDefaultTenantUser(user.id); + }); + + it('should be able to delete primary email', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + const newEmail = generateEmail(); + const newVerificationRecordId = await createAndVerifyVerificationCode(api, { + type: SignInIdentifier.Email, + value: newEmail, + }); + + await updatePrimaryEmail(api, newEmail, verificationRecordId, newVerificationRecordId); + + const userInfo = await getUserInfo(api); + expect(userInfo).toHaveProperty('primaryEmail', newEmail); + + await deletePrimaryEmail(api, verificationRecordId); + + const userInfoAfterDelete = await getUserInfo(api); + expect(userInfoAfterDelete).toHaveProperty('primaryEmail', null); + + await deleteDefaultTenantUser(user.id); + }); + }); + describe('POST /profile/primary-phone', () => { it('should fail if scope is missing', async () => { const { user, username, password } = await createDefaultTenantUserWithPassword(); @@ -241,4 +343,100 @@ describe('profile (email and phone)', () => { await deleteDefaultTenantUser(user.id); }); }); + + describe('DELETE /profile/primary-phone', () => { + it('should fail if scope is missing', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + + await expectRejects(deletePrimaryPhone(api, verificationRecordId), { + code: 'auth.unauthorized', + status: 400, + }); + + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if verification record is invalid', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Phone], + }); + + await expectRejects(deletePrimaryPhone(api, 'invalid-verification-record-id'), { + code: 'verification_record.permission_denied', + status: 401, + }); + + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if phone is the only sign-up identifier', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Phone], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Phone], + password: true, + verify: true, + }); + + await expectRejects(deletePrimaryPhone(api, verificationRecordId), { + code: 'user.phone_required', + status: 400, + }); + + await enableAllPasswordSignInMethods(); + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if email or phone is the sign-up identifier', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Phone], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone], + password: true, + verify: true, + }); + + await expectRejects(deletePrimaryPhone(api, verificationRecordId), { + code: 'user.email_or_phone_required', + status: 400, + }); + + await enableAllPasswordSignInMethods(); + await deleteDefaultTenantUser(user.id); + }); + + it('should be able to delete primary phone', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Phone], + }); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + const newPhone = generatePhone(); + const newVerificationRecordId = await createAndVerifyVerificationCode(api, { + type: SignInIdentifier.Phone, + value: newPhone, + }); + + await updatePrimaryPhone(api, newPhone, verificationRecordId, newVerificationRecordId); + + const userInfo = await getUserInfo(api); + expect(userInfo).toHaveProperty('primaryPhone', newPhone); + + await deletePrimaryPhone(api, verificationRecordId); + + const userInfoAfterDelete = await getUserInfo(api); + expect(userInfoAfterDelete).toHaveProperty('primaryPhone', null); + + await deleteDefaultTenantUser(user.id); + }); + }); }); diff --git a/packages/integration-tests/src/tests/api/profile/index.test.ts b/packages/integration-tests/src/tests/api/profile/index.test.ts index 766e45269..79e09e0f3 100644 --- a/packages/integration-tests/src/tests/api/profile/index.test.ts +++ b/packages/integration-tests/src/tests/api/profile/index.test.ts @@ -1,8 +1,9 @@ import { UserScope } from '@logto/core-kit'; -import { hookEvents } from '@logto/schemas'; +import { hookEvents, SignInIdentifier } from '@logto/schemas'; import { enableAllAccountCenterFields } from '#src/api/account-center.js'; import { getUserInfo, updateOtherProfile, updatePassword, updateUser } from '#src/api/profile.js'; +import { updateSignInExperience } from '#src/api/sign-in-experience.js'; import { createVerificationRecordByPassword } from '#src/api/verification-record.js'; import { WebHookApiTest } from '#src/helpers/hook.js'; import { expectRejects } from '#src/helpers/index.js'; @@ -174,6 +175,27 @@ describe('profile', () => { await deleteDefaultTenantUser(user.id); }); + it('should be able to update username to null', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password); + + await updateSignInExperience({ + signUp: { + identifiers: [SignInIdentifier.Email], + password: true, + verify: true, + }, + }); + const response = await updateUser(api, { username: null }); + expect(response).toMatchObject({ username: null }); + await enableAllPasswordSignInMethods(); + + const userInfo = await getUserInfo(api); + expect(userInfo).toHaveProperty('username', null); + + await deleteDefaultTenantUser(user.id); + }); + it('should fail if username is already in use', async () => { const { user, username, password } = await createDefaultTenantUserWithPassword(); const { user: user2, username: username2 } = await createDefaultTenantUserWithPassword(); diff --git a/packages/phrases/src/locales/en/errors/user.ts b/packages/phrases/src/locales/en/errors/user.ts index eb1843366..5db42127f 100644 --- a/packages/phrases/src/locales/en/errors/user.ts +++ b/packages/phrases/src/locales/en/errors/user.ts @@ -37,6 +37,11 @@ const user = { personal_access_token_name_exists: 'Personal access token name already exists.', totp_secret_invalid: 'Invalid TOTP secret supplied.', wrong_backup_code_format: 'Backup code format is invalid.', + username_required: 'Username is a required identifier, you can not set it to null.', + email_or_phone_required: + 'Email address or phone number is a required identifier, at least one is required.', + email_required: 'Email address is a required identifier, you can not set it to null.', + phone_required: 'Phone number is a required identifier, you can not set it to null.', }; export default Object.freeze(user);