diff --git a/packages/core/src/libraries/verification.ts b/packages/core/src/libraries/verification.ts index b8057efd92d..1838b8171ee 100644 --- a/packages/core/src/libraries/verification.ts +++ b/packages/core/src/libraries/verification.ts @@ -1,22 +1,38 @@ +import RequestError from '../errors/RequestError/index.js'; import { expirationTime } from '../queries/verification-records.js'; import { buildVerificationRecord, verificationRecordDataGuard, + type VerificationRecordMap, } from '../routes/experience/classes/verifications/index.js'; import { type VerificationRecord } from '../routes/experience/classes/verifications/verification-record.js'; +import { VerificationRecordsMap } from '../routes/experience/classes/verifications/verification-records-map.js'; import type Libraries from '../tenants/Libraries.js'; import type Queries from '../tenants/Queries.js'; import assertThat from '../utils/assert-that.js'; -export const buildUserVerificationRecordById = async ( - userId: string, - id: string, - queries: Queries, - libraries: Libraries -) => { - const record = await queries.verificationRecords.findUserActiveVerificationRecordById(userId, id); +/** + * Builds a verification record by its id. + * The `userId` is optional and is only used for user sensitive permission verifications. + */ +const buildVerificationRecordById = async ({ + id, + queries, + libraries, + userId, +}: { + id: string; + queries: Queries; + libraries: Libraries; + userId?: string; +}) => { + const record = await queries.verificationRecords.findActiveVerificationRecordById(id); assertThat(record, 'verification_record.not_found'); + if (userId) { + assertThat(record.userId === userId, 'verification_record.not_found'); + } + const result = verificationRecordDataGuard.safeParse({ ...record.data, id: record.id, @@ -27,17 +43,86 @@ export const buildUserVerificationRecordById = async ( return buildVerificationRecord(libraries, queries, result.data); }; -export const saveVerificationRecord = async ( - userId: string, +/** + * Verifies the user sensitive permission by checking if the verification record is valid + * and associated with the user. + */ +export const verifyUserSensitivePermission = async ({ + userId, + id, + queries, + libraries, +}: { + userId: string; + id: string; + queries: Queries; + libraries: Libraries; +}): Promise => { + try { + const record = await buildVerificationRecordById({ id, queries, libraries, userId }); + + assertThat(record.isVerified, 'verification_record.not_found'); + } catch (error) { + if (error instanceof RequestError) { + throw new RequestError({ code: 'verification_record.permission_denied', status: 403 }); + } + + throw error; + } +}; + +/** + * Builds a user verification record by its id and type. + * This is used to build a verification record for new identifier verifications, + * and may not be associated with a user. + */ +export const buildVerificationRecordByIdAndType = async ({ + type, + id, + queries, + libraries, +}: { + type: K; + id: string; + queries: Queries; + libraries: Libraries; +}): Promise => { + const records = new VerificationRecordsMap(); + records.setValue(await buildVerificationRecordById({ id, queries, libraries })); + + const instance = records.get(type); + + assertThat(instance?.type === type, 'verification_record.not_found'); + + return instance; +}; + +export const insertVerificationRecord = async ( verificationRecord: VerificationRecord, - queries: Queries + queries: Queries, + // For new identifier verifications, the user id should be empty + userId?: string ) => { const { id, ...rest } = verificationRecord.toJson(); - return queries.verificationRecords.upsertRecord({ + return queries.verificationRecords.insert({ id, userId, data: rest, expiresAt: new Date(Date.now() + expirationTime).valueOf(), }); }; + +// The upsert query can not update JSONB fields, so we need to use the update query +export const updateVerificationRecord = async ( + verificationRecord: VerificationRecord, + queries: Queries +) => { + const { id, ...rest } = verificationRecord.toJson(); + + return queries.verificationRecords.update({ + where: { id }, + set: { data: rest }, + jsonbMode: 'replace', + }); +}; diff --git a/packages/core/src/queries/verification-records.ts b/packages/core/src/queries/verification-records.ts index 0ad7cb729ce..4cd2e1dcadc 100644 --- a/packages/core/src/queries/verification-records.ts +++ b/packages/core/src/queries/verification-records.ts @@ -18,24 +18,16 @@ export class VerificationRecordQueries { returning: true, }); - public upsertRecord = buildInsertIntoWithPool(this.pool)(VerificationRecords, { - onConflict: { - fields: [fields.id], - setExcludedFields: [fields.expiresAt], - }, - }); - public readonly update = buildUpdateWhereWithPool(this.pool)(VerificationRecords, true); public readonly find = buildFindEntityByIdWithPool(this.pool)(VerificationRecords); constructor(public readonly pool: CommonQueryMethods) {} - public findUserActiveVerificationRecordById = async (userId: string, id: string) => { + public findActiveVerificationRecordById = async (id: string) => { return this.pool.maybeOne(sql` select * from ${table} - where ${fields.userId} = ${userId} - and ${fields.id} = ${id} + where ${fields.id} = ${id} and ${fields.expiresAt} > now() `); }; diff --git a/packages/core/src/routes/profile/index.openapi.json b/packages/core/src/routes/profile/index.openapi.json index 383be79e92f..465a3eaf88b 100644 --- a/packages/core/src/routes/profile/index.openapi.json +++ b/packages/core/src/routes/profile/index.openapi.json @@ -50,7 +50,7 @@ "post": { "operationId": "UpdatePassword", "summary": "Update password", - "description": "Update password for the user, a verification record is required.", + "description": "Update password for the user, a verification record is required for checking sensitive permissions.", "requestBody": { "content": { "application/json": { @@ -60,7 +60,7 @@ "description": "The new password for the user." }, "verificationRecordId": { - "description": "The verification record ID." + "description": "The verification record ID for checking sensitive permissions." } } } @@ -71,8 +71,45 @@ "204": { "description": "The password was updated successfully." }, + "403": { + "description": "Permission denied, the verification record is invalid." + } + } + } + }, + "/api/profile/primary-email": { + "post": { + "operationId": "UpdatePrimaryEmail", + "summary": "Update primary email", + "description": "Update primary email for the user, a verification record is required for checking sensitive permissions, and a new identifier verification record is required for the new email ownership verification.", + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "email": { + "description": "The new email for the user." + }, + "verificationRecordId": { + "description": "The verification record ID for checking sensitive permissions." + }, + "newIdentifierVerificationRecordId": { + "description": "The identifier verification record ID for the new email ownership verification." + } + } + } + } + } + }, + "responses": { + "204": { + "description": "The primary email was updated successfully." + }, "400": { - "description": "The verification record is invalid." + "description": "The new verification record is invalid." + }, + "403": { + "description": "Permission denied, the verification record is invalid." } } } diff --git a/packages/core/src/routes/profile/index.ts b/packages/core/src/routes/profile/index.ts index f7d346b08c0..ed6298d2aa6 100644 --- a/packages/core/src/routes/profile/index.ts +++ b/packages/core/src/routes/profile/index.ts @@ -1,4 +1,5 @@ -import { usernameRegEx, UserScope } from '@logto/core-kit'; +import { emailRegEx, usernameRegEx, UserScope } from '@logto/core-kit'; +import { VerificationType } from '@logto/schemas'; import { conditional } from '@silverhand/essentials'; import { z } from 'zod'; @@ -6,7 +7,10 @@ import koaGuard from '#src/middleware/koa-guard.js'; import { EnvSet } from '../../env-set/index.js'; import { encryptUserPassword } from '../../libraries/user.utils.js'; -import { buildUserVerificationRecordById } from '../../libraries/verification.js'; +import { + buildVerificationRecordByIdAndType, + verifyUserSensitivePermission, +} from '../../libraries/verification.js'; import assertThat from '../../utils/assert-that.js'; import type { UserRouter, RouterInitArgs } from '../types.js'; @@ -70,7 +74,7 @@ export default function profileRoutes( '/profile/password', koaGuard({ body: z.object({ password: z.string().min(1), verificationRecordId: z.string() }), - status: [204, 400], + status: [204, 400, 403], }), async (ctx, next) => { const { id: userId } = ctx.auth; @@ -78,13 +82,12 @@ export default function profileRoutes( // TODO(LOG-9947): apply password policy - const verificationRecord = await buildUserVerificationRecordById( + await verifyUserSensitivePermission({ userId, - verificationRecordId, + id: verificationRecordId, queries, - libraries - ); - assertThat(verificationRecord.isVerified, 'verification_record.not_found'); + libraries, + }); const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); const updatedUser = await updateUserById(userId, { @@ -99,4 +102,49 @@ export default function profileRoutes( return next(); } ); + + router.post( + '/profile/primary-email', + koaGuard({ + body: z.object({ + email: z.string().regex(emailRegEx), + verificationRecordId: z.string(), + newIdentifierVerificationRecordId: z.string(), + }), + status: [204, 400, 403], + }), + async (ctx, next) => { + const { id: userId, scopes } = ctx.auth; + const { email, verificationRecordId, newIdentifierVerificationRecordId } = ctx.guard.body; + + assertThat(scopes.has(UserScope.Email), 'auth.unauthorized'); + + await verifyUserSensitivePermission({ + userId, + id: verificationRecordId, + queries, + libraries, + }); + + // 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(); + } + ); } diff --git a/packages/core/src/routes/verification/index.openapi.json b/packages/core/src/routes/verification/index.openapi.json index f422da80ae8..236886ecd72 100644 --- a/packages/core/src/routes/verification/index.openapi.json +++ b/packages/core/src/routes/verification/index.openapi.json @@ -36,6 +36,71 @@ } } } + }, + "/api/verifications/verification-code": { + "post": { + "operationId": "CreateVerificationByVerificationCode", + "summary": "Create a record by verification code", + "description": "Create a verification record and send the code to the specified identifier. The code verification can be used to verify the given identifier.", + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "identifier": { + "description": "The identifier (email address or phone number) to send the verification code to." + } + } + } + } + } + }, + "responses": { + "201": { + "description": "The verification code has been successfully sent." + }, + "501": { + "description": "The connector for sending the verification code is not configured." + } + } + } + }, + "/api/verifications/verification-code/verify": { + "post": { + "operationId": "VerifyVerificationByVerificationCode", + "summary": "Verify verification code", + "description": "Verify the provided verification code against the identifier. If successful, the verification record will be marked as verified.", + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "code": { + "description": "The verification code to be verified." + }, + "identifier": { + "description": "The identifier (email address or phone number) to verify the code against. Must match the identifier used to send the verification code." + }, + "verificationId": { + "description": "The verification ID of the CodeVerification record." + } + } + } + } + } + }, + "responses": { + "200": { + "description": "The verification code has been successfully verified." + }, + "400": { + "description": "The verification code is invalid or the maximum number of attempts has been exceeded. Check the error message for details." + }, + "501": { + "description": "The connector for sending the verification code is not configured." + } + } + } } } } diff --git a/packages/core/src/routes/verification/index.ts b/packages/core/src/routes/verification/index.ts index 6a55fd96e8d..363de19491e 100644 --- a/packages/core/src/routes/verification/index.ts +++ b/packages/core/src/routes/verification/index.ts @@ -1,16 +1,28 @@ -import { AdditionalIdentifier, SentinelActivityAction } from '@logto/schemas'; +import { + AdditionalIdentifier, + InteractionEvent, + SentinelActivityAction, + SignInIdentifier, + verificationCodeIdentifierGuard, + VerificationType, +} from '@logto/schemas'; import { z } from 'zod'; import koaGuard from '#src/middleware/koa-guard.js'; import { EnvSet } from '../../env-set/index.js'; -import { saveVerificationRecord } from '../../libraries/verification.js'; +import { + buildVerificationRecordByIdAndType, + insertVerificationRecord, + updateVerificationRecord, +} from '../../libraries/verification.js'; import { withSentinel } from '../experience/classes/libraries/sentinel-guard.js'; +import { createNewCodeVerificationRecord } from '../experience/classes/verifications/code-verification.js'; import { PasswordVerification } from '../experience/classes/verifications/password-verification.js'; import type { UserRouter, RouterInitArgs } from '../types.js'; export default function verificationRoutes( - ...[router, { provider, queries, libraries, sentinel }]: RouterInitArgs + ...[router, { queries, libraries, sentinel }]: RouterInitArgs ) { if (!EnvSet.values.isDevFeaturesEnabled) { return; @@ -46,7 +58,7 @@ export default function verificationRoutes( passwordVerification.verify(password) ); - await saveVerificationRecord(userId, passwordVerification, queries); + await insertVerificationRecord(passwordVerification, queries, userId); ctx.body = { verificationRecordId: passwordVerification.id }; ctx.status = 201; @@ -54,4 +66,88 @@ export default function verificationRoutes( return next(); } ); + + router.post( + '/verifications/verification-code', + koaGuard({ + body: z.object({ + identifier: verificationCodeIdentifierGuard, + }), + response: z.object({ verificationRecordId: z.string() }), + status: [201, 501], + }), + async (ctx, next) => { + const { id: userId } = ctx.auth; + const { identifier } = ctx.guard.body; + + const user = await queries.users.findUserById(userId); + + const codeVerification = createNewCodeVerificationRecord( + libraries, + queries, + identifier, + // TODO(LOG-10148): Add new event + InteractionEvent.SignIn + ); + + await codeVerification.sendVerificationCode(); + + await insertVerificationRecord( + codeVerification, + queries, + // If the identifier is the primary email or phone, the verification record is associated with the user. + (identifier.type === SignInIdentifier.Email && identifier.value === user.primaryEmail) || + (identifier.type === SignInIdentifier.Phone && identifier.value === user.primaryPhone) + ? userId + : undefined + ); + + ctx.body = { verificationRecordId: codeVerification.id }; + ctx.status = 201; + + return next(); + } + ); + + router.post( + '/verifications/verification-code/verify', + koaGuard({ + body: z.object({ + identifier: verificationCodeIdentifierGuard, + verificationId: z.string(), + code: z.string(), + }), + response: z.object({ verificationRecordId: z.string() }), + // 501: connector not found + status: [200, 400, 501], + }), + async (ctx, next) => { + const { identifier, code, verificationId } = ctx.guard.body; + + const codeVerification = await buildVerificationRecordByIdAndType({ + type: VerificationType.EmailVerificationCode, + id: verificationId, + queries, + libraries, + }); + + await withSentinel( + { + sentinel, + action: SentinelActivityAction.VerificationCode, + identifier, + payload: { + verificationId: codeVerification.id, + }, + }, + codeVerification.verify(identifier, code) + ); + + await updateVerificationRecord(codeVerification, queries); + + ctx.body = { verificationRecordId: codeVerification.id }; + + return next(); + } + ); } diff --git a/packages/integration-tests/src/api/profile.ts b/packages/integration-tests/src/api/profile.ts index 2a6bc44e531..4a1e5d42990 100644 --- a/packages/integration-tests/src/api/profile.ts +++ b/packages/integration-tests/src/api/profile.ts @@ -7,6 +7,16 @@ export const updatePassword = async ( password: string ) => api.post('api/profile/password', { json: { password, verificationRecordId } }); +export const updatePrimaryEmail = async ( + api: KyInstance, + email: string, + verificationRecordId: string, + newIdentifierVerificationRecordId: string +) => + api.post('api/profile/primary-email', { + json: { email, verificationRecordId, newIdentifierVerificationRecordId }, + }); + export const updateUser = async (api: KyInstance, body: Record) => api.patch('api/profile', { json: body }).json<{ name?: string; diff --git a/packages/integration-tests/src/api/verification-record.ts b/packages/integration-tests/src/api/verification-record.ts index d7a71cd6d0d..b4660aef996 100644 --- a/packages/integration-tests/src/api/verification-record.ts +++ b/packages/integration-tests/src/api/verification-record.ts @@ -1,5 +1,8 @@ +import { type SignInIdentifier } from '@logto/schemas'; import { type KyInstance } from 'ky'; +import { readConnectorMessage } from '#src/helpers/index.js'; + export const createVerificationRecordByPassword = async (api: KyInstance, password: string) => { const { verificationRecordId } = await api .post('api/verifications/password', { @@ -11,3 +14,57 @@ export const createVerificationRecordByPassword = async (api: KyInstance, passwo return verificationRecordId; }; + +const createVerificationCode = async ( + api: KyInstance, + identifier: { type: SignInIdentifier; value: string } +) => { + const { verificationRecordId } = await api + .post('api/verifications/verification-code', { + json: { + identifier: { + type: identifier.type, + value: identifier.value, + }, + }, + }) + .json<{ verificationRecordId: string }>(); + + return verificationRecordId; +}; + +const verifyVerificationCode = async ( + api: KyInstance, + code: string, + identifier: { type: SignInIdentifier; value: string }, + verificationId: string +) => { + const { verificationRecordId } = await api + .post('api/verifications/verification-code/verify', { + json: { + code, + identifier, + verificationId, + }, + }) + .json<{ verificationRecordId: string }>(); + + return verificationRecordId; +}; + +export const createAndVerifyVerificationCode = async ( + api: KyInstance, + identifier: { type: SignInIdentifier; value: string } +) => { + const verificationRecordId = await createVerificationCode(api, identifier); + const { code, phone, address } = await readConnectorMessage( + identifier.type === 'email' ? 'Email' : 'Sms' + ); + + expect(code).toBeTruthy(); + expect(identifier.type === 'email' ? address : phone).toBe(identifier.value); + + await verifyVerificationCode(api, code, identifier, verificationRecordId); + + return verificationRecordId; +}; diff --git a/packages/integration-tests/src/helpers/profile.ts b/packages/integration-tests/src/helpers/profile.ts index 69f76cfce0c..c4444dc7195 100644 --- a/packages/integration-tests/src/helpers/profile.ts +++ b/packages/integration-tests/src/helpers/profile.ts @@ -9,12 +9,18 @@ import api, { baseApi, authedAdminApi } from '../api/api.js'; import { initClient } from './client.js'; -export const createDefaultTenantUserWithPassword = async () => { +export const createDefaultTenantUserWithPassword = async ({ + primaryEmail, + primaryPhone, +}: { + primaryEmail?: string; + primaryPhone?: string; +} = {}) => { const username = generateUsername(); const password = generatePassword(); const user = await authedAdminApi .post('users', { - json: { username, password }, + json: { username, password, primaryEmail, primaryPhone }, }) .json(); 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 new file mode 100644 index 00000000000..1981e6c353b --- /dev/null +++ b/packages/integration-tests/src/tests/api/profile/email-and-phone.test.ts @@ -0,0 +1,138 @@ +import { UserScope } from '@logto/core-kit'; +import { SignInIdentifier } from '@logto/schemas'; + +import { authedAdminApi } from '#src/api/api.js'; +import { getUserInfo, updatePrimaryEmail } from '#src/api/profile.js'; +import { + createAndVerifyVerificationCode, + createVerificationRecordByPassword, +} from '#src/api/verification-record.js'; +import { setEmailConnector } from '#src/helpers/connector.js'; +import { expectRejects } from '#src/helpers/index.js'; +import { + createDefaultTenantUserWithPassword, + deleteDefaultTenantUser, + signInAndGetUserApi, +} from '#src/helpers/profile.js'; +import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js'; +import { devFeatureTest, generateEmail } from '#src/utils.js'; + +const { describe, it } = devFeatureTest; + +describe('profile (email and phone)', () => { + beforeAll(async () => { + await enableAllPasswordSignInMethods(); + await setEmailConnector(authedAdminApi); + }); + + describe('POST /profile/primary-email', () => { + it('should fail if scope is missing', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password); + const newEmail = generateEmail(); + + await expectRejects( + updatePrimaryEmail( + api, + newEmail, + 'invalid-verification-record-id', + 'new-verification-record-id' + ), + { + 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], + }); + const newEmail = generateEmail(); + + await expectRejects( + updatePrimaryEmail( + api, + newEmail, + 'invalid-verification-record-id', + 'new-verification-record-id' + ), + { + code: 'verification_record.permission_denied', + status: 403, + } + ); + + await deleteDefaultTenantUser(user.id); + }); + + it('should fail if new identifier verification record is invalid', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const newEmail = generateEmail(); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + + await expectRejects( + updatePrimaryEmail(api, newEmail, verificationRecordId, 'new-verification-record-id'), + { + code: 'verification_record.not_found', + status: 400, + } + ); + + await deleteDefaultTenantUser(user.id); + }); + + it('should be able to update primary email by verifying password', async () => { + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const newEmail = generateEmail(); + const verificationRecordId = await createVerificationRecordByPassword(api, password); + const newVerificationRecordId = await createAndVerifyVerificationCode(api, { + type: SignInIdentifier.Email, + value: newEmail, + }); + + await updatePrimaryEmail(api, newEmail, verificationRecordId, newVerificationRecordId); + + const userInfo = await getUserInfo(api); + expect(userInfo).toHaveProperty('email', newEmail); + + await deleteDefaultTenantUser(user.id); + }); + + it('should be able to update primary email by verifying existing email', async () => { + const primaryEmail = generateEmail(); + const { user, username, password } = await createDefaultTenantUserWithPassword({ + primaryEmail, + }); + const api = await signInAndGetUserApi(username, password, { + scopes: [UserScope.Profile, UserScope.Email], + }); + const newEmail = generateEmail(); + const verificationRecordId = await createAndVerifyVerificationCode(api, { + type: SignInIdentifier.Email, + value: primaryEmail, + }); + const newVerificationRecordId = await createAndVerifyVerificationCode(api, { + type: SignInIdentifier.Email, + value: newEmail, + }); + + await updatePrimaryEmail(api, newEmail, verificationRecordId, newVerificationRecordId); + + const userInfo = await getUserInfo(api); + expect(userInfo).toHaveProperty('email', newEmail); + + 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 c44d1d06ddf..b16b8acfffd 100644 --- a/packages/integration-tests/src/tests/api/profile/index.test.ts +++ b/packages/integration-tests/src/tests/api/profile/index.test.ts @@ -121,8 +121,8 @@ describe('profile', () => { const newPassword = generatePassword(); await expectRejects(updatePassword(api, 'invalid-varification-record-id', newPassword), { - code: 'verification_record.not_found', - status: 400, + code: 'verification_record.permission_denied', + status: 403, }); await deleteDefaultTenantUser(user.id); diff --git a/packages/phrases/src/locales/en/errors/verification-record.ts b/packages/phrases/src/locales/en/errors/verification-record.ts index fca3dee41fc..9601bc320b1 100644 --- a/packages/phrases/src/locales/en/errors/verification-record.ts +++ b/packages/phrases/src/locales/en/errors/verification-record.ts @@ -1,5 +1,6 @@ const verification_record = { not_found: 'Verification record not found.', + permission_denied: 'Permission denied.', }; export default Object.freeze(verification_record);