diff --git a/.changeset/admin-identity-change-notice.md b/.changeset/admin-identity-change-notice.md new file mode 100644 index 00000000..2ca07bc5 --- /dev/null +++ b/.changeset/admin-identity-change-notice.md @@ -0,0 +1,12 @@ +--- +'seamless-auth-api': patch +--- + +Changing a user's email or phone through `PATCH /admin/users/{userId}` now signs the user out and tells them (#475). + +- Every active session for the user is revoked, in the same transaction as the change. Access tokens already issued stay valid until they expire. +- A notice goes to the previous address and to the new one, by email for an email change and by SMS for a phone change, through the configured direct transports. A failed notice does not fail the request. +- The `internal_user_updated_by_owner` audit event records `identityChanges` (SHA-256 digests of the previous and new values), `revokedSessions` and the outcome of each notice. +- With `x-seamless-auth-delivery-mode: external` no notice is sent, because the delivery payload has no kind for account notices yet. Each is recorded as `not_sent_external_delivery`. + +The response shape is unchanged. diff --git a/docs/admin-operations.md b/docs/admin-operations.md index e8a42f96..f7a96e3c 100644 --- a/docs/admin-operations.md +++ b/docs/admin-operations.md @@ -56,6 +56,28 @@ no one able to administer it. Instances whose `available_roles` predates scoped roles and lists only `admin` get `admin` instead, which is equivalent in power, so the grant never silently becomes a no-op. +## Changing A User's Email Or Phone + +When `PATCH /admin/users/:userId` changes a user's email address or phone number, the API also: + +- revokes every active session for that user, in the same transaction as the change, so the + user is signed out everywhere (access tokens already issued stay valid until they expire) +- sends a notice to the previous address and to the new one, by email for an email change and + by SMS for a phone change +- records the change on the `internal_user_updated_by_owner` audit event as `identityChanges`, + with a SHA-256 digest of the previous and new values (audit metadata never stores addresses), + plus `revokedSessions` and the outcome of each notice in `notices` + +An email that differs only in letter case is not treated as a change. Setting `emailVerified`, +`phoneVerified` or roles on their own does not revoke sessions. + +Notices go through the configured direct email and SMS transports. The messaging contract has +no template for account notices, so the text is fixed by the API. A notice that fails to send +does not fail the request; it is recorded as `failed` in `notices`. A caller that sends +`x-seamless-auth-delivery-mode: external` owns delivery, and the delivery payload has no kind +for account notices yet, so in that mode no notice is sent and each is recorded as +`not_sent_external_delivery`. + ## Device Replacement Recovery Administrators with write access can prepare an account for device replacement: diff --git a/openapi.json b/openapi.json index 40587bc9..cb9c114a 100644 --- a/openapi.json +++ b/openapi.json @@ -4163,6 +4163,7 @@ "/admin/users/{userId}": { "patch": { "summary": "Update user", + "description": "Changing `email` or `phone` revokes all of the user's sessions and sends a notice to the previous and the new address. With `x-seamless-auth-delivery-mode: external` no notice is sent.", "tags": ["Admin"], "security": [{ "bearerAuth": [] }], "requestBody": { diff --git a/src/config/directMessaging.ts b/src/config/directMessaging.ts index d90a4240..118505c6 100644 --- a/src/config/directMessaging.ts +++ b/src/config/directMessaging.ts @@ -7,7 +7,9 @@ import { type AuthMessagingService, createAuthMessagingService, + type EmailTransport, MessagingConfigurationError, + type SmsTransport, } from '@seamless-auth/messaging'; import { createAwsEmailTransport, createAwsSmsTransport } from '@seamless-auth/messaging-aws'; import { createTwilioSmsTransport } from '@seamless-auth/messaging-twilio'; @@ -114,3 +116,11 @@ export function createDirectAuthMessagingService(appName: string): AuthMessaging sms: buildSmsTransport(), }); } + +export function createDirectEmailTransport(): EmailTransport { + return buildEmailTransport(); +} + +export function createDirectSmsTransport(): SmsTransport { + return buildSmsTransport(); +} diff --git a/src/controllers/admin.ts b/src/controllers/admin.ts index 657378a8..1a3e58ec 100644 --- a/src/controllers/admin.ts +++ b/src/controllers/admin.ts @@ -8,6 +8,7 @@ import { Request, Response } from 'express'; import { Op, UniqueConstraintError, WhereOptions } from 'sequelize'; import { getSystemConfig } from '../config/getSystemConfig.js'; +import { wantsExternalDelivery } from '../lib/externalDelivery.js'; import { unavailableRoles } from '../lib/scopedRoles.js'; import { AuthEvent, AuthEventAttributes } from '../models/authEvents.js'; import { Credential } from '../models/credentials.js'; @@ -34,6 +35,12 @@ import { streamAuthEventExport } from '../services/auditExport.js'; import { verifyAuthEventChain } from '../services/auditIntegrity.js'; import { serializeAuthEvents } from '../services/authEventSerialization.js'; import { AuthEventService } from '../services/authEventService.js'; +import { + auditIdentityChanges, + detectIdentityChanges, + revokeSessionsForIdentityChange, + sendIdentityChangeNotices, +} from '../services/identityChangeService.js'; import { revokeAllSessionsForUsers } from '../services/sessionRevocation.js'; import { hardRevokeSession } from '../services/sessionService.js'; import { importUsers as runUserImport } from '../services/userImportService.js'; @@ -355,8 +362,28 @@ export const updateUser = async (req: ServiceRequest, res: Response) => { }); } + const identityChanges = detectIdentityChanges(user, updateData); + try { - await user.update(updateData); + let revokedSessions = 0; + + if (identityChanges.length > 0) { + // One transaction, so a failed revocation cannot leave the new address stored + // and the change undetectable on a retry. + await User.sequelize!.transaction(async (transaction) => { + await user.update(updateData, { transaction }); + revokedSessions = await revokeSessionsForIdentityChange(user.id, transaction); + }); + } else { + await user.update(updateData); + } + + const notices = + identityChanges.length > 0 + ? await sendIdentityChangeNotices(identityChanges, { + external: wantsExternalDelivery(req), + }) + : []; await AuthEventService.log({ userId, @@ -367,6 +394,13 @@ export const updateUser = async (req: ServiceRequest, res: Response) => { before, after: updateData, targetUser: userId, + ...(identityChanges.length > 0 + ? { + identityChanges: auditIdentityChanges(identityChanges), + revokedSessions, + notices, + } + : {}), }, }); } catch (error) { diff --git a/src/generated/api.ts b/src/generated/api.ts index b3160ccc..bbe7df24 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -4068,7 +4068,10 @@ export interface paths { delete?: never; options?: never; head?: never; - /** Update user */ + /** + * Update user + * @description Changing `email` or `phone` revokes all of the user's sessions and sends a notice to the previous and the new address. With `x-seamless-auth-delivery-mode: external` no notice is sent. + */ patch: { parameters: { query?: never; diff --git a/src/routes/admin.routes.ts b/src/routes/admin.routes.ts index 1a3c941e..05594c60 100644 --- a/src/routes/admin.routes.ts +++ b/src/routes/admin.routes.ts @@ -546,6 +546,8 @@ adminRouter.patch( { auth: 'access', summary: 'Update user', + description: + "Changing `email` or `phone` revokes all of the user's sessions and sends a notice to the previous and the new address. With `x-seamless-auth-delivery-mode: external` no notice is sent.", tags: ['Admin'], middleware: [requireAdmin('write')], diff --git a/src/services/identityChangeService.ts b/src/services/identityChangeService.ts new file mode 100644 index 00000000..3db0f2bf --- /dev/null +++ b/src/services/identityChangeService.ts @@ -0,0 +1,147 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the Apache License, Version 2.0 + * See LICENSE file in the project root for full license information + */ + +import type { Transaction } from 'sequelize'; + +import { getSystemConfig } from '../config/getSystemConfig.js'; +import getLogger from '../utils/logger.js'; +import { hashSha256 } from '../utils/utils.js'; +import { DeliveryError } from './deliveryError.js'; +import { sendNoticeEmail, sendNoticeSms } from './messagingService.js'; +import { revokeAllSessionsForUsers } from './sessionRevocation.js'; + +const logger = getLogger('identityChange'); + +export type IdentityField = 'email' | 'phone'; + +export interface IdentityChange { + field: IdentityField; + previous: string | null; + next: string | null; +} + +export type IdentityNoticeStatus = 'sent' | 'failed' | 'not_sent_external_delivery'; + +export interface IdentityNoticeResult { + field: IdentityField; + recipient: 'previous' | 'new'; + status: IdentityNoticeStatus; +} + +function sameEmail(a: string | null, b: string | null) { + return (a ?? '').toLowerCase() === (b ?? '').toLowerCase(); +} + +export function detectIdentityChanges( + current: { email: string | null; phone: string | null }, + update: Record, +): IdentityChange[] { + const changes: IdentityChange[] = []; + + if (typeof update.email === 'string' && !sameEmail(current.email, update.email)) { + changes.push({ field: 'email', previous: current.email, next: update.email }); + } + + if ('phone' in update) { + const next = typeof update.phone === 'string' ? update.phone : null; + + if (next !== (current.phone ?? null)) { + changes.push({ field: 'phone', previous: current.phone ?? null, next }); + } + } + + return changes; +} + +/** + * Audit metadata is redacted of addresses before it is stored, so the values are kept + * as digests of the normalized form. An investigator can still confirm whether a given + * address was the one removed or added. + */ +export function auditIdentityChanges(changes: IdentityChange[]) { + const digest = (field: IdentityField, value: string | null) => + value === null ? null : hashSha256(field === 'email' ? value.toLowerCase() : value); + + return changes.map(({ field, previous, next }) => ({ + field, + previousSha256: digest(field, previous), + nextSha256: digest(field, next), + })); +} + +export async function revokeSessionsForIdentityChange( + userId: string, + transaction?: Transaction, +): Promise { + return revokeAllSessionsForUsers(userId, 'admin_identity_change', transaction); +} + +function noticeText(appName: string, field: IdentityField, recipient: 'previous' | 'new') { + const label = field === 'email' ? 'email address' : 'phone number'; + + if (recipient === 'previous') { + return `An administrator changed the ${label} on your ${appName} account, and every session on it was signed out. If you did not expect this change, contact your administrator.`; + } + + return `An administrator set this ${label} on an ${appName} account. If you did not expect this, contact your administrator.`; +} + +async function deliver( + field: IdentityField, + to: string, + appName: string, + recipient: 'previous' | 'new', +) { + const text = noticeText(appName, field, recipient); + + if (field === 'email') { + await sendNoticeEmail(to, `Your ${appName} sign-in email was changed`, text); + } else { + await sendNoticeSms(to, text); + } +} + +/** + * A notice that fails to send does not undo the change or fail the request: the change + * is already stored and the sessions are already revoked, so the outcome is recorded in + * the audit event instead. + */ +export async function sendIdentityChangeNotices( + changes: IdentityChange[], + { external }: { external: boolean }, +): Promise { + const results: IdentityNoticeResult[] = []; + const appName = (await getSystemConfig())?.app_name ?? 'Seamless Auth'; + + for (const change of changes) { + const targets: Array<['previous' | 'new', string | null]> = [ + ['previous', change.previous], + ['new', change.next], + ]; + + for (const [recipient, to] of targets) { + if (!to) continue; + + // The delivery contract has no kind for account notices, so a caller that owns + // delivery cannot be handed one. Sending directly would bypass that caller. + if (external) { + results.push({ field: change.field, recipient, status: 'not_sent_external_delivery' }); + continue; + } + + try { + await deliver(change.field, to, appName, recipient); + results.push({ field: change.field, recipient, status: 'sent' }); + } catch (error) { + if (!(error instanceof DeliveryError)) throw error; + logger.error(`Identity change notice failed for ${change.field} (${recipient})`); + results.push({ field: change.field, recipient, status: 'failed' }); + } + } + } + + return results; +} diff --git a/src/services/messagingService.ts b/src/services/messagingService.ts index 1d5ffb8b..e9b83140 100644 --- a/src/services/messagingService.ts +++ b/src/services/messagingService.ts @@ -4,9 +4,13 @@ * See LICENSE file in the project root for full license information */ -import type { AuthMessagingService } from '@seamless-auth/messaging'; +import type { AuthMessagingService, EmailTransport, SmsTransport } from '@seamless-auth/messaging'; -import { createDirectAuthMessagingService } from '../config/directMessaging.js'; +import { + createDirectAuthMessagingService, + createDirectEmailTransport, + createDirectSmsTransport, +} from '../config/directMessaging.js'; import { getSystemConfig } from '../config/getSystemConfig.js'; import getLogger from '../utils/logger.js'; import { normalizePhoneNumber } from '../utils/utils.js'; @@ -131,3 +135,48 @@ export const sendEnrollmentInviteEmail = async (to: string, signInUrl: string) = throw new DeliveryError('Failed to send enrollment invite email', error); } }; + +// The messaging contract has templates only for sign-in messages, so account notices go +// straight to the configured transport with text written here. +let noticeEmailTransport: EmailTransport | null = null; +let noticeSmsTransport: SmsTransport | null = null; + +export const sendNoticeEmail = async (to: string, subject: string, text: string) => { + logger.debug('Sending account notice email'); + + if (shouldBypassDirectMessaging()) { + logger.debug('Skipping direct notice email delivery in development'); + return; + } + + try { + noticeEmailTransport ??= createDirectEmailTransport(); + await noticeEmailTransport.send({ to, subject, text }); + } catch (error) { + logger.error(`Failed to send account notice email ${error}`); + throw new DeliveryError('Failed to send account notice email', error); + } +}; + +export const sendNoticeSms = async (to: string, body: string) => { + logger.debug('Sending account notice SMS'); + + if (shouldBypassDirectMessaging()) { + logger.debug('Skipping direct notice SMS delivery in development'); + return; + } + + try { + const normalizedPhone = normalizePhoneNumber(to); + + if (!normalizedPhone) { + throw new Error('Invalid phone number for direct SMS delivery'); + } + + noticeSmsTransport ??= createDirectSmsTransport(); + await noticeSmsTransport.send({ to: normalizedPhone, body }); + } catch (error) { + logger.error(`Failed to send account notice SMS ${error}`); + throw new DeliveryError('Failed to send account notice SMS', error); + } +}; diff --git a/tests/integration/admin/admin.spec.ts b/tests/integration/admin/admin.spec.ts index 5555b36f..7cb1184e 100644 --- a/tests/integration/admin/admin.spec.ts +++ b/tests/integration/admin/admin.spec.ts @@ -921,6 +921,83 @@ describe('PATCH /admin/users/:userId', () => { expect(User.findByPk).not.toHaveBeenCalled(); }); + it('revokes sessions, notifies both addresses and audits an email change', async () => { + const { sendNoticeEmail } = await import('../../../src/services/messagingService.js'); + const user = buildUser({ email: 'old@example.com' }); + + (User.findByPk as any).mockResolvedValue(user); + (Session.update as any).mockResolvedValueOnce([2]); + + const res = await request(app).patch('/admin/users/user-1').send({ email: 'new@example.com' }); + + expect(res.status).toBe(200); + expect(User.sequelize!.transaction).toHaveBeenCalled(); + expect(user.update).toHaveBeenCalledWith({ email: 'new@example.com' }, expect.anything()); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'admin_identity_change' }, + expect.objectContaining({ where: { userId: user.id, revokedAt: null } }), + ); + // The cutoff, so a refresh racing the change cannot leave a session behind. + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + expect.objectContaining({ where: { id: user.id } }), + ); + expect(sendNoticeEmail).toHaveBeenCalledWith( + 'old@example.com', + expect.any(String), + expect.any(String), + ); + expect(sendNoticeEmail).toHaveBeenCalledWith( + 'new@example.com', + expect.any(String), + expect.any(String), + ); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'internal_user_updated_by_owner', + metadata: expect.objectContaining({ + revokedSessions: 2, + identityChanges: [ + expect.objectContaining({ field: 'email', previousSha256: expect.any(String) }), + ], + notices: [ + { field: 'email', recipient: 'previous', status: 'sent' }, + { field: 'email', recipient: 'new', status: 'sent' }, + ], + }), + }), + ); + }); + + it('does not send notices itself when the caller owns delivery', async () => { + const { sendNoticeSms } = await import('../../../src/services/messagingService.js'); + const user = buildUser({ phone: '+14155552671' }); + + (User.findByPk as any).mockResolvedValue(user); + + const res = await request(app) + .patch('/admin/users/user-1') + .set('x-seamless-auth-delivery-mode', 'external') + .send({ phone: '+14155552672' }); + + expect(res.status).toBe(200); + expect(Session.update).toHaveBeenCalled(); + expect(sendNoticeSms).not.toHaveBeenCalled(); + }); + + it('leaves sessions alone when no identifier changes', async () => { + const user = buildUser({ email: 'same@example.com' }); + + (User.findByPk as any).mockResolvedValue(user); + + const res = await request(app) + .patch('/admin/users/user-1') + .send({ email: 'same@example.com', emailVerified: true }); + + expect(res.status).toBe(200); + expect(Session.update).not.toHaveBeenCalled(); + }); + it('clears phone state when phone is removed', async () => { const user = buildUser({ phone: '+14155552671', phoneVerified: true }); @@ -929,12 +1006,15 @@ describe('PATCH /admin/users/:userId', () => { const res = await request(app).patch('/admin/users/user-1').send({ phone: null }); expect(res.status).toBe(200); - expect(user.update).toHaveBeenCalledWith({ - phone: null, - phoneVerified: false, - phoneVerificationToken: null, - phoneVerificationTokenExpiry: null, - }); + expect(user.update).toHaveBeenCalledWith( + { + phone: null, + phoneVerified: false, + phoneVerificationToken: null, + phoneVerificationTokenExpiry: null, + }, + expect.anything(), + ); }); it('rejects phoneVerified true without a phone number', async () => { @@ -992,6 +1072,7 @@ describe('PATCH /admin/users/:userId', () => { expect(res.status).toBe(200); expect(user.update).toHaveBeenCalledWith( expect.objectContaining({ phone: '+14155552672', phoneVerified: false }), + expect.anything(), ); }); @@ -1007,6 +1088,7 @@ describe('PATCH /admin/users/:userId', () => { expect(res.status).toBe(200); expect(user.update).toHaveBeenCalledWith( expect.objectContaining({ phone: '+14155552672', phoneVerified: true }), + expect.anything(), ); }); diff --git a/tests/setup/mocks.ts b/tests/setup/mocks.ts index d7ce324e..e9af6cb4 100644 --- a/tests/setup/mocks.ts +++ b/tests/setup/mocks.ts @@ -272,6 +272,8 @@ vi.mock('../../src/models/magicLinks.js', () => ({ vi.mock('../../src/services/messagingService.js', () => ({ sendMagicLinkEmail: vi.fn(), sendEnrollmentInviteEmail: vi.fn(), + sendNoticeEmail: vi.fn(), + sendNoticeSms: vi.fn(), })); vi.mock('crypto', async () => { diff --git a/tests/unit/config/directMessaging.spec.ts b/tests/unit/config/directMessaging.spec.ts index 72461d4e..c2322a45 100644 --- a/tests/unit/config/directMessaging.spec.ts +++ b/tests/unit/config/directMessaging.spec.ts @@ -75,6 +75,18 @@ describe('directMessaging config', () => { expect(result).toEqual({ name: 'messaging-service' }); }); + it('builds the email and SMS transports on their own for account notices', async () => { + process.env.AWS_REGION = 'us-east-1'; + process.env.SES_EMAIL = 'noreply@example.com'; + + const { createDirectEmailTransport, createDirectSmsTransport } = + await import('../../../src/config/directMessaging.js'); + + expect(createDirectEmailTransport()).toEqual({ name: 'aws-email' }); + expect(createDirectSmsTransport()).toEqual({ name: 'aws-sms' }); + expect(createAuthMessagingServiceMock).not.toHaveBeenCalled(); + }); + it('builds Twilio SMS transport when configured', async () => { process.env.MESSAGING_AWS_REGION = 'us-west-2'; process.env.MESSAGING_EMAIL_FROM = 'alerts@example.com'; diff --git a/tests/unit/services/identityChangeService.spec.ts b/tests/unit/services/identityChangeService.spec.ts new file mode 100644 index 00000000..22e97f90 --- /dev/null +++ b/tests/unit/services/identityChangeService.spec.ts @@ -0,0 +1,160 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { getSystemConfig } from '../../../src/config/getSystemConfig.js'; +import { Session } from '../../../src/models/sessions.js'; +import { DeliveryError } from '../../../src/services/deliveryError.js'; +import { + auditIdentityChanges, + detectIdentityChanges, + revokeSessionsForIdentityChange, + sendIdentityChangeNotices, +} from '../../../src/services/identityChangeService.js'; +import { sendNoticeEmail, sendNoticeSms } from '../../../src/services/messagingService.js'; +import { hashSha256 } from '../../../src/utils/utils.js'; + +const current = { email: 'old@example.com', phone: '+14155552671' }; + +beforeEach(() => { + vi.clearAllMocks(); + (getSystemConfig as any).mockResolvedValue({ app_name: 'Acme' }); +}); + +describe('detectIdentityChanges', () => { + it('reports a changed email', () => { + expect(detectIdentityChanges(current, { email: 'new@example.com' })).toEqual([ + { field: 'email', previous: 'old@example.com', next: 'new@example.com' }, + ]); + }); + + it('ignores an email that differs only in case', () => { + expect(detectIdentityChanges(current, { email: 'OLD@example.com' })).toEqual([]); + }); + + it('reports a changed, removed or added phone', () => { + expect(detectIdentityChanges(current, { phone: '+14155552672' })).toEqual([ + { field: 'phone', previous: '+14155552671', next: '+14155552672' }, + ]); + expect(detectIdentityChanges(current, { phone: null })).toEqual([ + { field: 'phone', previous: '+14155552671', next: null }, + ]); + expect(detectIdentityChanges({ email: null, phone: null }, { phone: '+14155552672' })).toEqual([ + { field: 'phone', previous: null, next: '+14155552672' }, + ]); + }); + + it('ignores an unchanged phone and fields that are not identifiers', () => { + expect(detectIdentityChanges(current, { phone: '+14155552671', roles: ['admin'] })).toEqual([]); + }); +}); + +describe('auditIdentityChanges', () => { + it('records digests of the normalized values rather than the values', () => { + const audited = auditIdentityChanges([ + { field: 'email', previous: 'Old@Example.com', next: 'new@example.com' }, + { field: 'phone', previous: '+14155552671', next: null }, + ]); + + expect(audited).toEqual([ + { + field: 'email', + previousSha256: hashSha256('old@example.com'), + nextSha256: hashSha256('new@example.com'), + }, + { field: 'phone', previousSha256: hashSha256('+14155552671'), nextSha256: null }, + ]); + expect(JSON.stringify(audited)).not.toContain('example.com'); + }); +}); + +describe('revokeSessionsForIdentityChange', () => { + it('revokes every live session for the user', async () => { + (Session.update as any).mockResolvedValueOnce([3]); + + await expect(revokeSessionsForIdentityChange('user-1')).resolves.toBe(3); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'admin_identity_change' }, + { where: { userId: 'user-1', revokedAt: null }, transaction: undefined }, + ); + }); +}); + +describe('sendIdentityChangeNotices', () => { + it('notifies the previous and the new address', async () => { + const results = await sendIdentityChangeNotices( + [ + { field: 'email', previous: 'old@example.com', next: 'new@example.com' }, + { field: 'phone', previous: '+14155552671', next: null }, + ], + { external: false }, + ); + + expect(sendNoticeEmail).toHaveBeenCalledWith( + 'old@example.com', + 'Your Acme sign-in email was changed', + expect.stringContaining('signed out'), + ); + expect(sendNoticeEmail).toHaveBeenCalledWith( + 'new@example.com', + 'Your Acme sign-in email was changed', + expect.stringContaining('set this email address'), + ); + expect(sendNoticeSms).toHaveBeenCalledTimes(1); + expect(sendNoticeSms).toHaveBeenCalledWith('+14155552671', expect.stringContaining('phone')); + expect(results).toEqual([ + { field: 'email', recipient: 'previous', status: 'sent' }, + { field: 'email', recipient: 'new', status: 'sent' }, + { field: 'phone', recipient: 'previous', status: 'sent' }, + ]); + }); + + it('records a failed delivery without throwing', async () => { + (sendNoticeEmail as any).mockRejectedValueOnce(new DeliveryError('down', new Error('x'))); + + const results = await sendIdentityChangeNotices( + [{ field: 'email', previous: 'old@example.com', next: 'new@example.com' }], + { external: false }, + ); + + expect(results).toEqual([ + { field: 'email', recipient: 'previous', status: 'failed' }, + { field: 'email', recipient: 'new', status: 'sent' }, + ]); + }); + + it('rethrows an error that is not a delivery failure', async () => { + (sendNoticeEmail as any).mockRejectedValueOnce(new Error('bug')); + + await expect( + sendIdentityChangeNotices([{ field: 'email', previous: 'old@example.com', next: null }], { + external: false, + }), + ).rejects.toThrow('bug'); + }); + + it('sends nothing directly when the caller owns delivery', async () => { + const results = await sendIdentityChangeNotices( + [{ field: 'email', previous: 'old@example.com', next: 'new@example.com' }], + { external: true }, + ); + + expect(sendNoticeEmail).not.toHaveBeenCalled(); + expect(results.map((result) => result.status)).toEqual([ + 'not_sent_external_delivery', + 'not_sent_external_delivery', + ]); + }); + + it('falls back to a default app name', async () => { + (getSystemConfig as any).mockResolvedValueOnce(undefined); + + await sendIdentityChangeNotices([{ field: 'email', previous: null, next: 'new@example.com' }], { + external: false, + }); + + expect(sendNoticeEmail).toHaveBeenCalledWith( + 'new@example.com', + 'Your Seamless Auth sign-in email was changed', + expect.any(String), + ); + }); +}); diff --git a/tests/unit/services/messagingService.spec.ts b/tests/unit/services/messagingService.spec.ts index 6d91fdd8..b976a92b 100644 --- a/tests/unit/services/messagingService.spec.ts +++ b/tests/unit/services/messagingService.spec.ts @@ -11,9 +11,19 @@ const createDirectAuthMessagingServiceMock = vi.fn(() => ({ sendEnrollmentInviteEmail: sendEnrollmentInviteEmailMock, })); +const emailTransportSendMock = vi.fn(); +const smsTransportSendMock = vi.fn(); +const createDirectEmailTransportMock = vi.fn(() => ({ + name: 'email', + send: emailTransportSendMock, +})); +const createDirectSmsTransportMock = vi.fn(() => ({ name: 'sms', send: smsTransportSendMock })); + vi.unmock('../../../src/services/messagingService'); vi.mock('../../../src/config/directMessaging', () => ({ createDirectAuthMessagingService: createDirectAuthMessagingServiceMock, + createDirectEmailTransport: createDirectEmailTransportMock, + createDirectSmsTransport: createDirectSmsTransportMock, })); vi.mock('../../../src/config/getSystemConfig', () => ({ getSystemConfig: vi.fn(async () => ({ @@ -171,4 +181,51 @@ describe('messagingService', () => { await expect(sendOTPSMS('+14155552671', 123456)).resolves.toBeUndefined(); expect(sendOtpSmsMock).toHaveBeenCalledWith({ to: '+14155552671', token: 123456 }); }); + + it('does not send account notices in development', async () => { + process.env.NODE_ENV = 'development'; + + const { sendNoticeEmail, sendNoticeSms } = + await import('../../../src/services/messagingService'); + + await sendNoticeEmail('test@example.com', 'Subject', 'Body'); + await sendNoticeSms('+14155552671', 'Body'); + expect(createDirectEmailTransportMock).not.toHaveBeenCalled(); + expect(createDirectSmsTransportMock).not.toHaveBeenCalled(); + }); + + it('sends account notices through the configured transports and reuses them', async () => { + process.env.NODE_ENV = 'production'; + + const { sendNoticeEmail, sendNoticeSms } = + await import('../../../src/services/messagingService'); + + await sendNoticeEmail('test@example.com', 'Subject', 'Body'); + await sendNoticeEmail('other@example.com', 'Subject', 'Body'); + await sendNoticeSms('+14155552671', 'Body'); + + expect(emailTransportSendMock).toHaveBeenCalledWith({ + to: 'test@example.com', + subject: 'Subject', + text: 'Body', + }); + expect(createDirectEmailTransportMock).toHaveBeenCalledTimes(1); + expect(smsTransportSendMock).toHaveBeenCalledWith({ to: '+14155552671', body: 'Body' }); + }); + + it('reports failed account notices as delivery errors', async () => { + process.env.NODE_ENV = 'production'; + emailTransportSendMock.mockRejectedValueOnce(new Error('email down')); + smsTransportSendMock.mockRejectedValueOnce(new Error('sms down')); + + const { sendNoticeEmail, sendNoticeSms } = + await import('../../../src/services/messagingService'); + const { DeliveryError } = await import('../../../src/services/deliveryError'); + + await expect(sendNoticeEmail('test@example.com', 'S', 'B')).rejects.toBeInstanceOf( + DeliveryError, + ); + await expect(sendNoticeSms('+14155552671', 'B')).rejects.toBeInstanceOf(DeliveryError); + await expect(sendNoticeSms('not-a-phone', 'B')).rejects.toBeInstanceOf(DeliveryError); + }); });