From 1b8c57ce7892d7d73c32d26641cf8fd0ecefe1a1 Mon Sep 17 00:00:00 2001 From: Volodymyr Zakhovaiko Date: Sun, 23 Aug 2026 00:14:00 +0200 Subject: [PATCH 1/4] feat(identity): verify registration email by otp and sign in on verify (BF-379) Registration mails a six-digit code instead of a magic link, and verifying that code is what mints the session. Sign-up stays sessionless, which is what keeps its duplicate-email answer indistinguishable, so a new player still lands signed in without giving up account enumeration. Email verification is no longer enforced on sign-in: it moves behind registration.requireEmailVerification, default off, so unverified players stay unrestricted while the KYC toggle is off. The RG and suspension gate is now one shared implementation in rg-guard.service.ts, used by password login, phone login, and the new verification path, so a session cannot be handed out through a gate that only some paths enforce. BREAKING CHANGE: verifyEmail takes { email, otp } and returns the session; sendEmailVerification takes { email } and is unauthenticated; the verifyEmail email template renders an otp instead of a url; registration.webUrl is replaced by registration.requireEmailVerification. --- docs/catalog.json | 4 + .../src/contracts/adapters/email-template.ts | 4 +- .../core/src/contracts/schemas/identity.ts | 8 +- .../src/contracts/schemas/platform-config.ts | 7 +- .../__tests__/identity.rate-limit.int.test.ts | 27 +- .../identity.registration-gates.int.test.ts | 2 +- .../default-email-template-renderer.test.ts | 10 +- .../core/src/pam/identity/contract/index.ts | 15 +- .../core/src/pam/identity/router/index.ts | 6 +- .../pam/identity/service/identity.service.ts | 263 ++++++++++-------- .../identity/service/phone-login.service.ts | 63 ++--- .../pam/identity/service/rg-guard.service.ts | 82 ++++++ .../src/server/auth/__tests__/auth.test.ts | 51 +++- packages/core/src/server/auth/auth.ts | 49 ++-- ...qa-wallet-auto-withdrawal-config-plugin.ts | 2 +- ...to-withdrawal-exclude-risk-flags-plugin.ts | 2 +- .../fixtures/test-kyc-config-plugin.ts | 2 +- ...allet-auto-withdrawal-cap-config-plugin.ts | 2 +- ...st-wallet-auto-withdrawal-config-plugin.ts | 2 +- .../src/__tests__/registration.e2e.test.ts | 99 ++++++- packages/testing/src/index.ts | 4 +- packages/testing/src/register.ts | 31 ++- .../src/test-registration-config-plugin.ts | 1 - 23 files changed, 490 insertions(+), 246 deletions(-) diff --git a/docs/catalog.json b/docs/catalog.json index 1e013fee..65ce1ba9 100644 --- a/docs/catalog.json +++ b/docs/catalog.json @@ -1903,6 +1903,10 @@ "name": "RequestPasswordResetInputSchema", "file": "packages/core/src/contracts/schemas/identity.ts" }, + { + "name": "ResendEmailVerificationInputSchema", + "file": "packages/core/src/contracts/schemas/identity.ts" + }, { "name": "ResetPasswordInputSchema", "file": "packages/core/src/contracts/schemas/identity.ts" diff --git a/packages/core/src/contracts/adapters/email-template.ts b/packages/core/src/contracts/adapters/email-template.ts index 12c78ac7..a93d60ea 100644 --- a/packages/core/src/contracts/adapters/email-template.ts +++ b/packages/core/src/contracts/adapters/email-template.ts @@ -10,7 +10,7 @@ export type EmailTemplateKey = | 'rgSelfExclusionLifted'; export type EmailTemplateData = { - verifyEmail: { url: string; token: string }; + verifyEmail: { otp: string }; resetPasswordOtp: { otp: string; email: string }; rgLimitUpdated: { period: string; type: string; description: string }; rgCoolingOffActivated: { expiresAt: Date }; @@ -52,7 +52,7 @@ export const DEFAULT_EMAIL_TEMPLATES: { } = { verifyEmail: (data) => ({ subject: 'Verify your email', - body: `Verify your email using this link: ${data.url}\n\nVerification token: ${data.token}`, + body: `Your email verification code is: ${data.otp}`, }), resetPasswordOtp: (data) => ({ subject: 'Reset your password', diff --git a/packages/core/src/contracts/schemas/identity.ts b/packages/core/src/contracts/schemas/identity.ts index 0bd0d41e..bbb4c194 100644 --- a/packages/core/src/contracts/schemas/identity.ts +++ b/packages/core/src/contracts/schemas/identity.ts @@ -136,8 +136,13 @@ export const VerifyPasswordResetOtpInputSchema = ResetPasswordInputSchema.pick({ otp: true, }); +export const ResendEmailVerificationInputSchema = z.object({ + email: z.email(), +}); + export const VerifyEmailInputSchema = z.object({ - token: z.string().min(1), + email: z.email(), + otp: z.string().length(OTP_CODE_LENGTH), }); export const UpdateProfileInputSchema = z @@ -178,6 +183,7 @@ export type Disable2faInput = z.infer; export type RequestPasswordResetInput = z.infer; export type ResetPasswordInput = z.infer; export type VerifyPasswordResetOtpInput = z.infer; +export type ResendEmailVerificationInput = z.infer; export type VerifyEmailInput = z.infer; export type UpdateProfileInput = z.infer; export type ChangePasswordInput = z.infer; diff --git a/packages/core/src/contracts/schemas/platform-config.ts b/packages/core/src/contracts/schemas/platform-config.ts index b512666a..7c76b77c 100644 --- a/packages/core/src/contracts/schemas/platform-config.ts +++ b/packages/core/src/contracts/schemas/platform-config.ts @@ -124,8 +124,11 @@ export const RegistrationConfigSchema = z .object({ /** Version recorded beside the player's affirmative terms acceptance. */ termsVersion: z.string().min(1), - /** Public consumer origin used in verification-email links. */ - webUrl: z.url(), + /** + * Blocks sign-in until the player has verified their address. Off by default: + * unverified players stay unrestricted while the KYC toggle is off. + */ + requireEmailVerification: z.boolean().default(false), }) .strict(); diff --git a/packages/core/src/pam/identity/__tests__/identity.rate-limit.int.test.ts b/packages/core/src/pam/identity/__tests__/identity.rate-limit.int.test.ts index 798665c7..63126a41 100644 --- a/packages/core/src/pam/identity/__tests__/identity.rate-limit.int.test.ts +++ b/packages/core/src/pam/identity/__tests__/identity.rate-limit.int.test.ts @@ -76,7 +76,7 @@ describe('IdentityService - rate limiting (real Redis)', () => { events, limiter, platformConfig: definePlatformConfig({ - registration: { termsVersion: '2026-08', webUrl: 'https://app.example.test' }, + registration: { termsVersion: '2026-08', requireEmailVerification: false }, }), }); @@ -158,29 +158,38 @@ describe('IdentityService - rate limiting on secret-guessing routes (ABC-208 fin ).rejects.toMatchObject({ code: 'TOO_MANY_REQUESTS' }); }); - it('rejects verifyEmail with a 429 once the per-caller limit is exhausted', async () => { + it('rejects verifyEmail with a 429 once the per-address limit is exhausted', async () => { const limiter = makeLimiter(); for (let i = 0; i < 5; i++) { - await limiter.consume('verify-email:203.0.113.5', { limit: 5, windowMs: 15 * 60 * 1000 }); + await limiter.consume('verify-email:target@e2e.test', { limit: 5, windowMs: 15 * 60 * 1000 }); } const svc = withTemplateRenderer({ drizzle, events, limiter }); + // Six digits are guessable, so the budget follows the address under attack. await expect( - svc.verifyEmail({ token: 'sometoken' }, { 'x-real-ip': '203.0.113.5' }), + svc.verifyEmail( + { email: 'target@e2e.test', otp: '000000' }, + { 'x-real-ip': '203.0.113.5' }, + new Headers(), + ), ).rejects.toMatchObject({ code: 'TOO_MANY_REQUESTS' }); }); - it('buckets unauthenticated verifyEmail callers separately, so one cannot stall the rest', async () => { + it('buckets verifyEmail addresses separately, so one target cannot stall the rest', async () => { const limiter = makeLimiter(); for (let i = 0; i < 5; i++) { - await limiter.consume('verify-email:203.0.113.6', { limit: 5, windowMs: 15 * 60 * 1000 }); + await limiter.consume('verify-email:target@e2e.test', { limit: 5, windowMs: 15 * 60 * 1000 }); } const svc = withTemplateRenderer({ drizzle, events, limiter }); - // Verification links are followed without a session. A shared bucket would let the - // exhausted caller above block every other sign-up in flight. + // Codes are entered without a session. A shared bucket would let the exhausted + // address above block every other sign-up in flight. await expect( - svc.verifyEmail({ token: 'sometoken' }, { 'x-real-ip': '203.0.113.7' }), + svc.verifyEmail( + { email: 'other@e2e.test', otp: '000000' }, + { 'x-real-ip': '203.0.113.7' }, + new Headers(), + ), ).rejects.not.toMatchObject({ code: 'TOO_MANY_REQUESTS' }); }); diff --git a/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts b/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts index 12aa958d..e1ce1a43 100644 --- a/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts +++ b/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts @@ -27,7 +27,7 @@ vi.mock('@openora/core/server', async (importOriginal) => { const events = makeEventBus(); const registrationConfig = definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'https://app.example.test' }, + registration: { termsVersion: 'test-v1', requireEmailVerification: false }, }); let db: TestDb; diff --git a/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts b/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts index be234fbd..2cf78c0e 100644 --- a/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts +++ b/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts @@ -4,16 +4,12 @@ import { DefaultEmailTemplateRenderer } from '../default-email-template-renderer describe('DefaultEmailTemplateRenderer', () => { const renderer = new DefaultEmailTemplateRenderer(); - it('renders the verifyEmail template with the url and token interpolated', () => { - const result = renderer.render( - 'verifyEmail', - { url: 'https://example.com/verify', token: 'tok123' }, - 'de', - ); + it('renders the verifyEmail template with the otp interpolated', () => { + const result = renderer.render('verifyEmail', { otp: '123456' }, 'de'); expect(result).toEqual({ subject: 'Verify your email', - body: 'Verify your email using this link: https://example.com/verify\n\nVerification token: tok123', + body: 'Your email verification code is: 123456', }); }); diff --git a/packages/core/src/pam/identity/contract/index.ts b/packages/core/src/pam/identity/contract/index.ts index 9164492d..88d877b3 100644 --- a/packages/core/src/pam/identity/contract/index.ts +++ b/packages/core/src/pam/identity/contract/index.ts @@ -14,6 +14,7 @@ import { RequestPasswordResetInputSchema, VerifyPasswordResetOtpInputSchema, ResetPasswordInputSchema, + ResendEmailVerificationInputSchema, VerifyEmailInputSchema, UpdateProfileInputSchema, ChangePasswordInputSchema, @@ -132,14 +133,26 @@ export const identityContract = { .input(ChangePasswordInputSchema) .output(IdentitySuccessSchema), + // Unauthenticated: the player has no session until the emailed code is verified. + // Always answers success, so it never reveals whether the address has an account. sendEmailVerification: oc .route({ method: 'POST', path: '/identity/email/verify/send' }) + .input(ResendEmailVerificationInputSchema) .output(IdentitySuccessSchema), + // Verifying the registration code is what mints the session (sign-up never does). A + // 2FA-enrolled account is verified but NOT signed in - it gets `twoFactorRedirect` and + // completes the challenge through `login`, exactly as that route signals it. verifyEmail: oc .route({ method: 'POST', path: '/identity/email/verify' }) .input(VerifyEmailInputSchema) - .output(IdentitySuccessSchema), + .output( + z.object({ + user: UserSchema.optional(), + session: SessionSchema.optional(), + twoFactorRedirect: z.boolean().optional(), + }), + ), changeEmail: oc .route({ method: 'POST', path: '/identity/email/change' }) diff --git a/packages/core/src/pam/identity/router/index.ts b/packages/core/src/pam/identity/router/index.ts index 2c4997d9..4a1fd9ce 100644 --- a/packages/core/src/pam/identity/router/index.ts +++ b/packages/core/src/pam/identity/router/index.ts @@ -121,12 +121,12 @@ export function createIdentityRouter( identity.changePassword(input, context.request.headers, context.resHeaders ?? new Headers()), ), - sendEmailVerification: os.sendEmailVerification.handler(({ context }) => - identity.sendEmailVerification(context.request.headers), + sendEmailVerification: os.sendEmailVerification.handler(({ input, context }) => + identity.sendEmailVerification(input, context.request.headers), ), verifyEmail: os.verifyEmail.handler(({ input, context }) => - identity.verifyEmail(input, context.request.headers), + identity.verifyEmail(input, context.request.headers, context.resHeaders ?? new Headers()), ), changeEmail: os.changeEmail.handler(({ input, context }) => diff --git a/packages/core/src/pam/identity/service/identity.service.ts b/packages/core/src/pam/identity/service/identity.service.ts index 4dbd7007..2ca1626d 100644 --- a/packages/core/src/pam/identity/service/identity.service.ts +++ b/packages/core/src/pam/identity/service/identity.service.ts @@ -15,7 +15,6 @@ import { import { parseCookies } from 'better-auth/cookies'; import { and, eq, isNull, sql } from 'drizzle-orm'; import { user, session, account, verification, twoFactor } from '../schema/index.js'; -import { player } from '@openora/core/pam/schema/profile'; import type { CacheAdapter, RateLimiterAdapter, @@ -30,6 +29,7 @@ import type { RequestPasswordResetInput, VerifyPasswordResetOtpInput, ResetPasswordInput, + ResendEmailVerificationInput, VerifyEmailInput, UpdateProfileInput, Theme, @@ -45,7 +45,7 @@ import type { } from '@openora/core/contracts'; import { RATE_LIMIT_KEYS, makeRateLimitKey } from '@openora/core/contracts'; import { assertSupportedLanguage } from '../../shared/language.js'; -import { isRgBlocked } from './rg-guard.service.js'; +import { assertAccountNotBlocked } from './rg-guard.service.js'; import { DEFAULT_LOCKOUT_DURATION_MS, DEFAULT_MAX_LOGIN_ATTEMPTS, @@ -134,8 +134,8 @@ type ExtendedAuthApi = { requestPasswordResetEmailOTP: AuthCall<{ email: string }>; checkVerificationOTP: AuthCall<{ email: string; type: 'forget-password'; otp: string }>; resetPasswordEmailOTP: AuthCall<{ email: string; otp: string; password: string }>; - sendVerificationEmail: AuthCall<{ email: string }>; - verifyEmail: AuthCall<{ token: string }>; + sendVerificationOTP: AuthCall<{ email: string; type: 'email-verification' }>; + verifyEmailOTP: AuthCall<{ email: string; otp: string }>; changePassword: AuthCall<{ currentPassword: string; newPassword: string }>; changeEmail: AuthCall<{ newEmail: string }>; updateUser: AuthCall<{ name?: string; image?: string | null; theme?: Theme; language?: string }>; @@ -206,8 +206,20 @@ const PASSWORD_RESET_VERIFY_RATE_LIMIT = { onUnavailable: 'deny', } as const; const VERIFY_2FA_RATE_LIMIT = { limit: 5, windowMs: 5 * MINUTE_MS, onUnavailable: 'deny' } as const; -const EMAIL_VERIFICATION_RATE_LIMIT = { limit: 3, windowMs: 15 * MINUTE_MS }; -const VERIFY_EMAIL_RATE_LIMIT = { limit: 5, windowMs: 15 * MINUTE_MS }; +// Fails closed with the verify budget below: better-auth issues a fresh code (and a fresh +// 3-attempt counter) per resend, so an unbounded resend loop is an unbounded guess budget. +const EMAIL_VERIFICATION_RATE_LIMIT = { + limit: 3, + windowMs: 15 * MINUTE_MS, + onUnavailable: 'deny', +} as const; +// Fails closed like the other secret-guessing budgets: the emailed code is six digits, +// so an unthrottled window is a brute-force window, not a degraded-UX window. +const VERIFY_EMAIL_RATE_LIMIT = { + limit: 5, + windowMs: 15 * MINUTE_MS, + onUnavailable: 'deny', +} as const; const CHANGE_PASSWORD_RATE_LIMIT = { limit: 5, windowMs: 15 * MINUTE_MS }; const TWO_FACTOR_PASSWORD_RATE_LIMIT = { limit: 5, windowMs: 5 * MINUTE_MS }; const FAKE_LOGIN_SHADOW_TTL_MS = 7 * 24 * 60 * 60 * 1000; @@ -318,7 +330,8 @@ export class IdentityService { ...(email ? { sendEmail: (args) => email.send(args) } : {}), templateRenderer: this.templateRenderer, getUserLanguage: (lookupEmail) => this.resolveUserLanguage(lookupEmail), - registrationWebUrl: this.platformConfig?.registration?.webUrl, + requireEmailVerification: + this.platformConfig?.registration?.requireEmailVerification ?? false, onExistingUserSignUp: async (existing) => { const response = await this.api.requestPasswordResetEmailOTP({ body: { email: existing.email }, @@ -374,6 +387,17 @@ export class IdentityService { return session?.user?.id ?? null; } + // Same gate as password login and phone login, from the one shared implementation. + private assertAccountNotBlocked( + account: { id: User['id']; rgBlocked: boolean; rgBlockedUntil: Date | null }, + meta: ClientMeta, + ): Promise { + return assertAccountNotBlocked({ drizzle: this.drizzle, events: this.events }, account, meta, { + revokeSessions: true, + errorData: { rgBlocked: { code: 'RG_BLOCKED' }, suspended: { code: 'ACCOUNT_SUSPENDED' } }, + }); + } + async register(input: RegisterInput, reqHeaders: NodeHeaders) { const registration = this.platformConfig?.registration; const provisioning = this.playerProvisioning; @@ -425,6 +449,22 @@ export class IdentityService { ip, userAgent, }); + // Sent here rather than by better-auth's sendOnSignUp hook: that hook also fires on + // the synthetic duplicate-email response, mailing a live code to an address whose + // owner never asked for it - and `verifyEmail` signs that code's bearer in. + // A mail failure is logged, never surfaced: the account exists either way and the + // player can ask for a new code, so failing the call here would only mislead them. + const otpResponse = await this.api.sendVerificationOTP({ + body: { email: input.email, type: 'email-verification' }, + headers, + asResponse: true, + }); + if (!otpResponse.ok) { + identityLogger.error( + { userId: body.user.id, status: otpResponse.status }, + 'verification code could not be sent - player must request a new one', + ); + } } return { status: 'check-email' as const }; } @@ -614,64 +654,12 @@ export class IdentityService { }); await ensureOk(authResponse); - // Resolved once (only when a user was found), before either gate below, so both - // the RG-block and the Backoffice-block events - and the eventual login success - // event - can all carry playerId. - let playerRow: Pick | undefined; - if (existingUser) { - [playerRow] = await this.drizzle.db - .select({ id: player.id, status: player.status }) - .from(player) - .where(eq(player.userId, existingUser.id)) - .limit(1); - } - - // RG login block, applied only AFTER credentials verify so a pre-auth probe can't - // distinguish a restricted account from a wrong password. Kill the session - // better-auth just issued and never forward its cookie. Cooling-off auto-expires - // here once rgBlockedUntil elapses (no unblock job). - if (existingUser && isRgBlocked(existingUser)) { - await this.drizzle.db - .update(session) - .set({ expiresAt: new Date() }) - .where(eq(session.userId, existingUser.id)); - this.events.emit('rg.exclusion.login_blocked', { - userId: existingUser.id, - playerId: playerRow?.id ?? null, - ip, - userAgent, - }); - throw new ORPCError('FORBIDDEN', { - message: 'Account access is currently restricted (responsible gambling).', - data: { code: 'RG_BLOCKED' }, - }); - } - - // Backoffice-initiated account block (status suspended/closed). Same shape as the - // RG gate above: applied only AFTER credentials verify, kills the just-issued - // session and withholds its cookie. Distinct mechanism from RG (self_excluded is - // out of scope here) - a suspended/closed player can never log back in. - if ( - existingUser && - playerRow && - (playerRow.status === 'suspended' || playerRow.status === 'closed') - ) { - await this.drizzle.db - .update(session) - .set({ expiresAt: new Date() }) - .where(eq(session.userId, existingUser.id)); - this.events.emit('player.login_blocked', { - userId: existingUser.id, - playerId: playerRow.id, - status: playerRow.status, - ip, - userAgent, - }); - throw new ORPCError('FORBIDDEN', { - message: 'This account has been suspended and can no longer be used.', - data: { code: 'ACCOUNT_SUSPENDED' }, - }); - } + // RG and backoffice blocks, applied only AFTER credentials verify so a pre-auth + // probe can't distinguish a restricted account from a wrong password. Resolves + // playerId once, so the eventual login success event can carry it too. + const playerId = existingUser + ? await this.assertAccountNotBlocked(existingUser, { ip, userAgent }) + : null; this.forwardCookies(authResponse, resHeaders); @@ -696,7 +684,7 @@ export class IdentityService { .where(eq(session.token, body.token)); this.events.emit('identity.user.login', { userId: body.user.id, - playerId: playerRow?.id ?? null, + playerId, ip, userAgent, }); @@ -1178,68 +1166,113 @@ export class IdentityService { return SUCCESS; } - async sendEmailVerification(reqHeaders: NodeHeaders) { - const headers = nodeHeadersToHeaders(reqHeaders); - const session = await this.auth.api.getSession({ headers }); - const email = session?.user?.email; - if (email) { - await assertRateLimit( - this.limiter, - `email-verify:${email.toLowerCase()}`, - EMAIL_VERIFICATION_RATE_LIMIT, - ); - await this.api.sendVerificationEmail({ body: { email }, headers, asResponse: true }); + /** + * Resends the registration code. Unauthenticated by design - the player has no session + * until the code is verified - so it answers SUCCESS for every address and only mails a + * code to an account that exists and is still unverified. Verified accounts are skipped + * deliberately: `verifyEmail` signs the code's bearer in, so resending to a verified + * address would turn this into passwordless sign-in for anyone who can read that inbox. + */ + async sendEmailVerification(input: ResendEmailVerificationInput, reqHeaders: NodeHeaders) { + const { ip } = extractClientMeta(reqHeaders); + const email = input.email.toLowerCase(); + await assertRateLimit(this.limiter, `email-verify:${email}`, EMAIL_VERIFICATION_RATE_LIMIT); + if (ip) { + await assertRateLimit(this.limiter, `email-verify-ip:${ip}`, EMAIL_VERIFICATION_RATE_LIMIT); + } + const [row] = await this.drizzle.db + .select({ emailVerified: user.emailVerified }) + .from(user) + .where(eq(user.email, email)) + .limit(1); + if (row && !row.emailVerified) { + await this.api.sendVerificationOTP({ + body: { email, type: 'email-verification' }, + headers: nodeHeadersToHeaders(reqHeaders), + asResponse: true, + }); } return SUCCESS; } - async verifyEmail(input: VerifyEmailInput, reqHeaders: NodeHeaders) { + /** + * Consumes the 6-digit code mailed by `register()` and signs the player in - + * better-auth's `autoSignInAfterVerification` mints the session here rather than at + * sign-up, which is what lets sign-up stay sessionless and therefore keep its + * duplicate-email response indistinguishable. Same post-credential gates as `login`: + * a code is proof of address ownership, not a bypass for an RG or backoffice block. + */ + async verifyEmail(input: VerifyEmailInput, reqHeaders: NodeHeaders, resHeaders: Headers) { const { ip, userAgent } = extractClientMeta(reqHeaders); const headers = nodeHeadersToHeaders(reqHeaders); - const sessionUserId = await this.currentUserId(headers); - // Verification links are followed without a session, so an `anonymous` bucket - // would be shared by every caller - one client could stall everyone's sign-up. - await assertRateLimit( - this.limiter, - `verify-email:${sessionUserId ?? ip ?? 'unknown'}`, - VERIFY_EMAIL_RATE_LIMIT, - ); - const res = await this.api.verifyEmail({ - query: { token: input.token }, + const email = input.email.toLowerCase(); + // Keyed on the address under attack (six digits are guessable) and on the caller, so + // one client can neither grind a single account nor sweep many. + await assertRateLimit(this.limiter, `verify-email:${email}`, VERIFY_EMAIL_RATE_LIMIT); + // Only when the caller's IP is actually known: an `unknown` bucket would be shared by + // every anonymous caller, letting one client stall everyone else's sign-up. + if (ip) { + await assertRateLimit(this.limiter, `verify-email-ip:${ip}`, VERIFY_EMAIL_RATE_LIMIT); + } + const res = await this.api.verifyEmailOTP({ + body: { email, otp: input.otp }, headers, asResponse: true, }); - await ensureOk(res); - const userId = sessionUserId ?? (await this.userIdFromVerificationToken(input.token)); - if (userId) { - this.events.emit('identity.email.verified', { - userId, - playerId: await this.identityReader.getPlayerIdByUserIdSafe(userId), - ip, - userAgent, - }); + await ensureOk(res, { genericMessage: 'Invalid or expired verification code' }); + const body = (await res.json()) as { token?: string | null; user: BetterAuthUser }; + + // better-auth has already committed `emailVerified` by now, so the audit record is + // emitted before any gate below can throw - a state change that leaves no trail is a + // compliance gap, see docs/standards/audit.md. + const playerId = await this.identityReader.getPlayerIdByUserIdSafe(body.user.id); + this.events.emit('identity.email.verified', { userId: body.user.id, playerId, ip, userAgent }); + + const [account] = await this.drizzle.db + .select({ + id: user.id, + rgBlocked: user.rgBlocked, + rgBlockedUntil: user.rgBlockedUntil, + twoFactorEnabled: user.twoFactorEnabled, + }) + .from(user) + .where(eq(user.id, body.user.id)) + .limit(1); + if (account) { + await this.assertAccountNotBlocked(account, { ip, userAgent }); } - return SUCCESS; - } - /** - * Better Auth answers an unauthenticated verification with `user: null`, so the - * subject is recovered from the token it just accepted - otherwise the audit event - * would never fire for the ordinary click-the-link flow. - */ - private async userIdFromVerificationToken(token: string) { - const payload = token.split('.')[1]; - if (!payload) { - return null; + // better-auth mints this session with `createSession`, which its twoFactor plugin only + // hooks on the sign-in routes - so an enrolled account would get a full session from + // the emailed code alone, bypassing its second factor. Verification still stands; the + // session does not, and the player signs in through `login` to face the challenge. + if (account?.twoFactorEnabled) { + await this.drizzle.db + .update(session) + .set({ expiresAt: new Date() }) + .where(eq(session.userId, body.user.id)); + return { twoFactorRedirect: true as const }; } - let email: unknown; - try { - email = (JSON.parse(Buffer.from(payload, 'base64url').toString()) as { email?: unknown }) - .email; - } catch { - return null; + + if (!body.token) { + throw new ORPCError('INTERNAL_SERVER_ERROR', { message: 'Verification did not sign you in' }); } - return typeof email === 'string' ? this.findUserIdByEmail(email) : null; + this.forwardCookies(res, resHeaders); + await this.drizzle.db + .update(session) + .set({ ipAddress: ip, userAgent }) + .where(eq(session.token, body.token)); + this.events.emit('identity.user.login', { userId: body.user.id, playerId, ip, userAgent }); + + const sessionDurationSeconds = + this.auth.options.session?.expiresIn ?? SESSION_DURATION_IN_SECONDS; + return { + user: toUser(body.user), + session: { + token: body.token, + expiresAt: new Date(Date.now() + sessionDurationSeconds * 1000).toISOString(), + }, + }; } async changeEmail(input: ChangeEmailInput, reqHeaders: NodeHeaders, resHeaders: Headers) { diff --git a/packages/core/src/pam/identity/service/phone-login.service.ts b/packages/core/src/pam/identity/service/phone-login.service.ts index fbb1159f..bdb02b15 100644 --- a/packages/core/src/pam/identity/service/phone-login.service.ts +++ b/packages/core/src/pam/identity/service/phone-login.service.ts @@ -25,8 +25,7 @@ import { ClientMeta, } from '@openora/core/contracts'; import { user, session, smsOtpSession } from '../schema/index.js'; -import { player } from '@openora/core/pam/schema/profile'; -import { isRgBlocked } from './rg-guard.service.js'; +import { assertAccountNotBlocked } from './rg-guard.service.js'; import { DEFAULT_MAX_LOGIN_ATTEMPTS, createAccountLockedError, @@ -380,48 +379,22 @@ export class PhoneLoginService { account = { ...account, failedLoginAttempts: 0, lockoutUntil: null }; } - // Resolved once, before either gate below, so both the RG-block and the - // Backoffice-block events - and the eventual phone-login success event - can all - // carry playerId. - const [playerRow] = await this.drizzle.db - .select({ id: player.id, status: player.status }) - .from(player) - .where(eq(player.userId, account.id)) - .limit(1); - - // RG login block applied only AFTER the OTP verifies so a probe can't distinguish - // a restricted account from a wrong code. - if (isRgBlocked(account)) { - this.events.emit('rg.exclusion.login_blocked', { - userId: account.id, - playerId: playerRow?.id ?? null, - ip, - userAgent, - }); - throw new ORPCError('FORBIDDEN', { - message: 'Account access is currently restricted (responsible gambling).', - data: { reason: PhoneLoginErrorReasonSchema.enum.rg_blocked }, - }); - } - - // Backoffice-initiated account block (status suspended/closed). Checked after the OTP - // verifies, before the session-minting transaction - no session exists yet, so unlike - // the email path there is nothing to revoke here; the OTP is left unconsumed so the - // gate reads the same as the RG block above. Distinct from RG (self_excluded is out of - // scope) - a suspended/closed player can never complete phone login. - if (playerRow && (playerRow.status === 'suspended' || playerRow.status === 'closed')) { - this.events.emit('player.login_blocked', { - userId: account.id, - playerId: playerRow.id, - status: playerRow.status, - ip, - userAgent, - }); - throw new ORPCError('FORBIDDEN', { - message: 'This account has been suspended and can no longer be used.', - data: { reason: PhoneLoginErrorReasonSchema.enum.account_suspended }, - }); - } + // Same RG and backoffice gates as password login, from the one shared implementation. + // Checked after the OTP verifies, before the session-minting transaction - no session + // exists yet, so unlike the email paths there is nothing to revoke here; the OTP is + // left unconsumed so a blocked account reads the same as any other rejection. + const playerId = await assertAccountNotBlocked( + { drizzle: this.drizzle, events: this.events }, + account, + { ip, userAgent }, + { + revokeSessions: false, + errorData: { + rgBlocked: { reason: PhoneLoginErrorReasonSchema.enum.rg_blocked }, + suspended: { reason: PhoneLoginErrorReasonSchema.enum.account_suspended }, + }, + }, + ); // Mint the session directly - bypasses better-auth's TOTP plugin chain by design, // so phone login never triggers a second factor. @@ -457,7 +430,7 @@ export class PhoneLoginService { this.events.emit('identity.user.phone_login', { userId: account.id, - playerId: playerRow?.id ?? null, + playerId, method: 'phone', ip, userAgent, diff --git a/packages/core/src/pam/identity/service/rg-guard.service.ts b/packages/core/src/pam/identity/service/rg-guard.service.ts index ad0a65a1..ffefd36b 100644 --- a/packages/core/src/pam/identity/service/rg-guard.service.ts +++ b/packages/core/src/pam/identity/service/rg-guard.service.ts @@ -1,3 +1,85 @@ +import { ORPCError } from '@orpc/server'; +import { eq } from 'drizzle-orm'; +import type { DrizzleService, EventBus } from '@openora/core/server'; +import type { ClientMeta, User } from '@openora/core/contracts'; +import { player } from '@openora/core/pam/schema/profile'; +import { session } from '../schema/index.js'; + export function isRgBlocked(u: { rgBlocked: boolean; rgBlockedUntil: Date | null }): boolean { return u.rgBlocked && (u.rgBlockedUntil === null || u.rgBlockedUntil > new Date()); } + +type BlockedAccount = { id: User['id']; rgBlocked: boolean; rgBlockedUntil: Date | null }; + +type AccountBlockGateOptions = { + /** + * Expire every session on the account before throwing. Needed wherever the caller has + * already let better-auth mint one (password login, email-code verification); the phone + * path checks the gate before minting, so it has nothing to revoke and passes false. + */ + revokeSessions: boolean; + /** `ORPCError.data` for each block, so each surface keeps its own error vocabulary. */ + errorData: { rgBlocked: Record; suspended: Record }; +}; + +/** + * The RG and backoffice (suspended/closed) gates for an account that has just proven a + * credential - never before, so a probe can't tell a restricted account apart from a + * wrong secret. Shared by every surface that hands out a session, so the three of them + * cannot drift: a gate that only some login paths enforce is not a gate. + * Returns the resolved playerId so callers can attach it to their own success event. + */ +export async function assertAccountNotBlocked( + { drizzle, events }: { drizzle: DrizzleService; events: EventBus }, + account: BlockedAccount, + { ip, userAgent }: ClientMeta, + { revokeSessions, errorData }: AccountBlockGateOptions, +): Promise { + const [playerRow] = await drizzle.db + .select({ id: player.id, status: player.status }) + .from(player) + .where(eq(player.userId, account.id)) + .limit(1); + + const revoke = async () => { + if (revokeSessions) { + await drizzle.db + .update(session) + .set({ expiresAt: new Date() }) + .where(eq(session.userId, account.id)); + } + }; + + if (isRgBlocked(account)) { + await revoke(); + events.emit('rg.exclusion.login_blocked', { + userId: account.id, + playerId: playerRow?.id ?? null, + ip, + userAgent, + }); + throw new ORPCError('FORBIDDEN', { + message: 'Account access is currently restricted (responsible gambling).', + data: errorData.rgBlocked, + }); + } + + // Distinct mechanism from RG (self_excluded is out of scope here) - a suspended or + // closed player can never log back in. + if (playerRow && (playerRow.status === 'suspended' || playerRow.status === 'closed')) { + await revoke(); + events.emit('player.login_blocked', { + userId: account.id, + playerId: playerRow.id, + status: playerRow.status, + ip, + userAgent, + }); + throw new ORPCError('FORBIDDEN', { + message: 'This account has been suspended and can no longer be used.', + data: errorData.suspended, + }); + } + + return playerRow?.id ?? null; +} diff --git a/packages/core/src/server/auth/__tests__/auth.test.ts b/packages/core/src/server/auth/__tests__/auth.test.ts index 92bac78b..298f87e3 100644 --- a/packages/core/src/server/auth/__tests__/auth.test.ts +++ b/packages/core/src/server/auth/__tests__/auth.test.ts @@ -60,6 +60,29 @@ describe('createAuth', () => { }); }); + it('renders the verification code template for an email-verification OTP', async () => { + const sendEmail = vi.fn().mockResolvedValue(undefined); + const templateRenderer = { + render: vi.fn().mockResolvedValue({ subject: 'Verify your email', body: 'code' }), + }; + + createAuth({ db: {} as never, sendEmail, templateRenderer }); + + const emailOtpOpts = emailOTPMock.mock.calls[0][0]; + await emailOtpOpts.sendVerificationOTP({ + email: 'test@example.com', + otp: '123456', + type: 'email-verification', + }); + + expect(templateRenderer.render).toHaveBeenCalledWith('verifyEmail', { otp: '123456' }, 'en'); + expect(sendEmail).toHaveBeenCalledWith({ + to: 'test@example.com', + subject: 'Verify your email', + body: 'code', + }); + }); + it('returns early without sending an email for other types', async () => { const sendEmail = vi.fn().mockResolvedValue(undefined); const templateRenderer = { @@ -78,7 +101,7 @@ describe('createAuth', () => { await emailOtpOpts.sendVerificationOTP({ email: 'test@example.com', otp: '123456', - type: 'verify-email', + type: 'sign-in', }); expect(getUserLanguage).not.toHaveBeenCalled(); @@ -87,6 +110,32 @@ describe('createAuth', () => { }); }); + describe('email verification gate', () => { + it('does not require a verified address by default', () => { + // Unverified players stay unrestricted while the KYC toggle is off. + createAuth({ db: {} as never }); + + expect(betterAuthMock.mock.calls[0][0].emailAndPassword.requireEmailVerification).toBe(false); + }); + + it('requires one when the operator turns the gate on', () => { + createAuth({ db: {} as never, requireEmailVerification: true }); + + expect(betterAuthMock.mock.calls[0][0].emailAndPassword.requireEmailVerification).toBe(true); + }); + + it('never lets better-auth mail the code on sign-up', () => { + // Its sign-up hook also fires on the synthetic duplicate-email response, which + // would mail a live code to an address whose owner never asked for it. + createAuth({ db: {} as never }); + + expect(betterAuthMock.mock.calls[0][0].emailVerification.sendOnSignUp).toBe(false); + expect(betterAuthMock.mock.calls[0][0].emailVerification.autoSignInAfterVerification).toBe( + true, + ); + }); + }); + describe('cookie domain', () => { it('leaves the session cookie host-only by default', () => { createAuth({ db: {} as never }); diff --git a/packages/core/src/server/auth/auth.ts b/packages/core/src/server/auth/auth.ts index c29ab73a..6370f17e 100644 --- a/packages/core/src/server/auth/auth.ts +++ b/packages/core/src/server/auth/auth.ts @@ -27,7 +27,12 @@ export type AuthOptions = { onPasswordReset?: (user: { id: string; email: string }) => Promise | void; templateRenderer?: EmailTemplateRenderer; getUserLanguage?: (email: string) => Promise; - registrationWebUrl?: string; + /** + * Blocks sign-in until the address is verified. Default off: unverified players stay + * unrestricted while the KYC toggle is off. Operators that need the stricter gate set + * `registration.requireEmailVerification` in platform config. + */ + requireEmailVerification?: boolean; onExistingUserSignUp?: (user: { id: string; email: string }) => Promise | void; cookieDomain?: string; }; @@ -87,8 +92,12 @@ export function createAuth(options: AuthOptions): BetterAuthType { }, emailAndPassword: { enabled: true, + // Sign-up never mints a session: better-auth only returns the indistinguishable + // duplicate-email response when autoSignIn is off (or verification is required, + // which is now operator-configurable). The session is minted by the OTP + // verification step instead - see `autoSignInAfterVerification` below. autoSignIn: false, - requireEmailVerification: true, + requireEmailVerification: options.requireEmailVerification ?? false, revokeSessionsOnPasswordReset: true, onExistingUserSignUp: options.onExistingUserSignUp ? async ({ user }) => { @@ -102,24 +111,11 @@ export function createAuth(options: AuthOptions): BetterAuthType { : undefined, }, emailVerification: { - sendOnSignUp: true, - expiresIn: 24 * 60 * 60, - sendVerificationEmail: async ({ user, url, token }) => { - const locale = (user as { language?: string }).language ?? 'en'; - const verificationUrl = options.registrationWebUrl - ? new URL('/verify-email', options.registrationWebUrl) - : undefined; - if (verificationUrl) { - verificationUrl.searchParams.set('token', token); - verificationUrl.searchParams.set('callbackURL', '/'); - } - const { subject, body } = await templateRenderer.render( - 'verifyEmail', - { url: verificationUrl?.toString() ?? url, token }, - locale, - ); - await sendEmail({ to: user.email, subject, body }); - }, + // The verification OTP is sent by IdentityService.register(), never by better-auth: + // its sign-up hook fires on the synthetic duplicate-email response too, which would + // mail a valid code to an existing account's address (takeover). + sendOnSignUp: false, + autoSignInAfterVerification: true, }, // Without this, better-auth's admin plugin defaults new signups to its own // 'user' role, which UserRoleSchema (player|admin) rejects everywhere downstream. @@ -131,15 +127,16 @@ export function createAuth(options: AuthOptions): BetterAuthType { otpLength: OTP_CODE_LENGTH, expiresIn: OTP_EXPIRES_IN_SEC, async sendVerificationOTP({ email, otp, type }) { - if (type !== 'forget-password') { + // Allow-list, not a fallback: an OTP type this app never issues (sign-in, + // change-email) must send nothing rather than borrow another template's copy. + if (type !== 'email-verification' && type !== 'forget-password') { return; } const locale = (await options.getUserLanguage?.(email)) ?? 'en'; - const { subject, body } = await templateRenderer.render( - 'resetPasswordOtp', - { otp, email }, - locale, - ); + const { subject, body } = + type === 'email-verification' + ? await templateRenderer.render('verifyEmail', { otp }, locale) + : await templateRenderer.render('resetPasswordOtp', { otp, email }, locale); await sendEmail({ to: email, subject, body }); }, }), diff --git a/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-config-plugin.ts b/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-config-plugin.ts index 4bcb4fbc..c1d574de 100644 --- a/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-config-plugin.ts +++ b/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-config-plugin.ts @@ -14,7 +14,7 @@ export default { register(ctx) { ctx.provide(PLATFORM_CONFIG, () => definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'http://localhost:3000' }, + registration: { termsVersion: 'test-v1' }, kyc: { gateWithdrawals: false }, autoWithdrawal: { enabled: true, diff --git a/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-exclude-risk-flags-plugin.ts b/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-exclude-risk-flags-plugin.ts index 2943e5e0..4ab472b7 100644 --- a/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-exclude-risk-flags-plugin.ts +++ b/packages/testing/src/__tests__/fixtures/qa-wallet-auto-withdrawal-exclude-risk-flags-plugin.ts @@ -14,7 +14,7 @@ export default { register(ctx) { ctx.provide(PLATFORM_CONFIG, () => definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'http://localhost:3000' }, + registration: { termsVersion: 'test-v1' }, kyc: { gateWithdrawals: false }, autoWithdrawal: { enabled: true, diff --git a/packages/testing/src/__tests__/fixtures/test-kyc-config-plugin.ts b/packages/testing/src/__tests__/fixtures/test-kyc-config-plugin.ts index d4801a63..a79138d7 100644 --- a/packages/testing/src/__tests__/fixtures/test-kyc-config-plugin.ts +++ b/packages/testing/src/__tests__/fixtures/test-kyc-config-plugin.ts @@ -49,7 +49,7 @@ export default { register(ctx) { ctx.provide(PLATFORM_CONFIG, () => definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'http://localhost:3000' }, + registration: { termsVersion: 'test-v1' }, kyc: { gateWithdrawals: true, reverifyThresholds: { USD: '10', EUR: '10' }, diff --git a/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-cap-config-plugin.ts b/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-cap-config-plugin.ts index 895fd328..733db3ae 100644 --- a/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-cap-config-plugin.ts +++ b/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-cap-config-plugin.ts @@ -11,7 +11,7 @@ export default { register(ctx) { ctx.provide(PLATFORM_CONFIG, () => definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'http://localhost:3000' }, + registration: { termsVersion: 'test-v1' }, autoWithdrawal: { enabled: true, dailyCapCount: 1 }, }), ); diff --git a/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-config-plugin.ts b/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-config-plugin.ts index 708da9aa..9a3c6281 100644 --- a/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-config-plugin.ts +++ b/packages/testing/src/__tests__/fixtures/test-wallet-auto-withdrawal-config-plugin.ts @@ -14,7 +14,7 @@ export default { register(ctx) { ctx.provide(PLATFORM_CONFIG, () => definePlatformConfig({ - registration: { termsVersion: 'test-v1', webUrl: 'http://localhost:3000' }, + registration: { termsVersion: 'test-v1' }, kyc: { gateWithdrawals: false }, autoWithdrawal: { enabled: true, diff --git a/packages/testing/src/__tests__/registration.e2e.test.ts b/packages/testing/src/__tests__/registration.e2e.test.ts index 4743d6cc..449f69b5 100644 --- a/packages/testing/src/__tests__/registration.e2e.test.ts +++ b/packages/testing/src/__tests__/registration.e2e.test.ts @@ -2,14 +2,15 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { randomUUID } from 'node:crypto'; import { eq } from 'drizzle-orm'; import { loadExtensions, DRIZZLE } from '@openora/core/server'; -import { user } from '@openora/core/pam/schema/identity'; +import { user, session } from '@openora/core/pam/schema/identity'; import { player } from '@openora/core/pam/schema/profile'; import { + verificationOtpFor, setupTestDb, bootTestApp, registerPlayer, submitRegistration, - verifyEmailByLink, + verifyEmailByOtp, capturedEmailsFor, seedMinimal, type TestDb, @@ -50,22 +51,97 @@ afterAll(async () => { await db?.dispose(); }); -describe('registration email verification gate', () => { - it('refuses login until the address is verified, then allows it', async () => { +describe('registration email verification', () => { + it('signs the player in when the emailed code is verified', async () => { const email = `reg-verify-${randomUUID()}@e2e.test`; const res = await submitRegistration(app, { email }); expect(res.status).toBe(200); expect(await res.json()).toEqual({ status: 'check-email' }); + // Sign-up stays sessionless - that is what keeps the duplicate-email answer + // indistinguishable. The session is minted by verification instead. expect(res.headers.get('set-cookie')).toBeNull(); - const beforeVerify = await login(email); - expect(beforeVerify.ok).toBe(false); + const verified = await verifyEmailByOtp(app, email); + expect(verified.headers.get('set-cookie')).toBeTruthy(); + const body = (await verified.json()) as { user: { email: string }; session: { token: string } }; + expect(body.user.email).toBe(email.toLowerCase()); + expect(body.session.token).toBeTruthy(); + }); + + it('refuses to sign in a blocked account that enters a valid code', async () => { + // An account can be RG-blocked or suspended between registering and entering the + // code. Verification still stands - the block is on the session, not the address. + const email = `reg-blocked-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); + const otp = verificationOtpFor(email); + const db = app.container.get(DRIZZLE).db; + const userId = await userIdFor(email); + await db + .update(user) + .set({ rgBlocked: true, rgBlockedUntil: null }) + .where(eq(user.id, userId!)); + + const res = await app.app.request('/identity/email/verify', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, otp }), + }); + + expect(res.status).toBe(403); + expect(res.headers.get('set-cookie')).toBeNull(); + const [row] = await db + .select({ emailVerified: user.emailVerified }) + .from(user) + .where(eq(user.id, userId!)); + expect(row?.emailVerified).toBe(true); + }); + + it('verifies but does not sign in a 2FA-enrolled account', async () => { + // better-auth mints the post-verification session with `createSession`, which its + // twoFactor plugin does not hook - so the code alone must not replace the second + // factor. The address is verified; the player still signs in through /identity/login. + const email = `reg-2fa-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); + const otp = verificationOtpFor(email); + const db = app.container.get(DRIZZLE).db; + const userId = await userIdFor(email); + await db.update(user).set({ twoFactorEnabled: true }).where(eq(user.id, userId!)); + + const res = await app.app.request('/identity/email/verify', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, otp }), + }); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ twoFactorRedirect: true }); + expect(res.headers.get('set-cookie')).toBeNull(); + const sessions = await db.select().from(session).where(eq(session.userId, userId!)); + expect(sessions.every((row) => row.expiresAt.getTime() <= Date.now())).toBe(true); + }); + + it('rejects a wrong code', async () => { + const email = `reg-badotp-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); + + const res = await app.app.request('/identity/email/verify', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, otp: '000000' }), + }); + expect(res.ok).toBe(false); + expect(res.headers.get('set-cookie')).toBeNull(); + }); - await verifyEmailByLink(app, email); + it('lets an unverified player sign in while the verification gate is off', async () => { + // Unverified players stay unrestricted until the operator turns the gate (or KYC) + // on, so registering and then signing in with the password must work. + const email = `reg-unverified-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); - const afterVerify = await login(email); - expect(afterVerify.ok).toBe(true); - expect(afterVerify.headers.get('set-cookie')).toBeTruthy(); + const res = await login(email); + expect(res.ok).toBe(true); + expect(res.headers.get('set-cookie')).toBeTruthy(); }); it('hides whether the email was already taken and sends a reset instead', async () => { @@ -88,6 +164,9 @@ describe('registration email verification gate', () => { .where(eq(player.userId, (await userIdFor(email)) ?? '')); expect(players).toHaveLength(1); expect(capturedEmailsFor(email).some((e) => /reset/i.test(e.subject))).toBe(true); + // Exactly one verification code was ever mailed - the one the real owner's own + // sign-up produced. A second would hand a stranger a code that signs them in. + expect(capturedEmailsFor(email).filter((e) => /verify/i.test(e.subject))).toHaveLength(1); }); it('records the terms and age acceptance on the player row at registration', async () => { diff --git a/packages/testing/src/index.ts b/packages/testing/src/index.ts index 18a8d01d..a5a7bb81 100644 --- a/packages/testing/src/index.ts +++ b/packages/testing/src/index.ts @@ -13,8 +13,8 @@ export { registerPlayer, registerAndMaterializePlayer, submitRegistration, - verificationTokenFor, - verifyEmailByLink, + verificationOtpFor, + verifyEmailByOtp, type RegisterPlayerInput, } from './register.js'; export { capturedEmailsFor, clearCapturedEmails, type CapturedEmail } from './captured-emails.js'; diff --git a/packages/testing/src/register.ts b/packages/testing/src/register.ts index 14a9892a..bc5ff1e2 100644 --- a/packages/testing/src/register.ts +++ b/packages/testing/src/register.ts @@ -27,34 +27,35 @@ export async function submitRegistration(app: TestApp, input: RegisterPlayerInpu }); } -/** The `token` query param better-auth put in the most recent verification email. */ -export function verificationTokenFor(email: string): string { +/** The 6-digit code in the most recent verification email. */ +export function verificationOtpFor(email: string): string { const [sent] = capturedEmailsFor(email); if (!sent) { throw new Error(`no verification email captured for ${email}`); } - const token = /[?&]token=([^&\s"']+)/.exec(sent.body)?.[1]; - if (!token) { - throw new Error(`no token in verification email for ${email}: ${sent.body}`); + const otp = /\b(\d{6})\b/.exec(sent.body)?.[1]; + if (!otp) { + throw new Error(`no verification code in email for ${email}: ${sent.body}`); } - return decodeURIComponent(token); + return otp; } /** - * Clicks the emailed verification link for real - the route consumes the token, - * rate-limits, and emits `identity.email.verified`, none of which a direct - * `email_verified` write would exercise. + * Enters the emailed code for real - the route consumes the OTP, rate-limits, mints the + * session and emits `identity.email.verified`, none of which a direct `email_verified` + * write would exercise. Returns the response so callers can assert on the session. */ -export async function verifyEmailByLink(app: TestApp, email: string) { +export async function verifyEmailByOtp(app: TestApp, email: string) { const res = await app.app.request('/identity/email/verify', { method: 'POST', - // Fresh client IP per click: the route buckets unauthenticated callers by IP. + // Fresh client IP per attempt: the route also buckets callers by IP. headers: registrationRequestHeaders(), - body: JSON.stringify({ token: verificationTokenFor(email) }), + body: JSON.stringify({ email, otp: verificationOtpFor(email) }), }); if (!res.ok) { throw new Error(`verify email failed (${res.status}): ${await res.text()}`); } + return res; } /** Escape hatch for tests that only need the verified state, not the route. */ @@ -67,8 +68,8 @@ export async function forceEmailVerified(app: TestApp, userId: string) { } /** - * Registers a player and returns its user id. Sign-in requires a verified address, - * so pass `verifyEmail: false` only when the test asserts on the unverified state. + * Registers a player and returns its user id. Verification is what mints the first + * session, so pass `verifyEmail: false` only when the test asserts on the unverified state. */ export async function registerPlayer( app: TestApp, @@ -87,7 +88,7 @@ export async function registerPlayer( throw new Error('registered user was not persisted'); } if (input.verifyEmail !== false) { - await verifyEmailByLink(app, input.email); + await verifyEmailByOtp(app, input.email); } return registered.id; } diff --git a/packages/testing/src/test-registration-config-plugin.ts b/packages/testing/src/test-registration-config-plugin.ts index 3ed390ba..5f35ef26 100644 --- a/packages/testing/src/test-registration-config-plugin.ts +++ b/packages/testing/src/test-registration-config-plugin.ts @@ -9,7 +9,6 @@ export default { definePlatformConfig({ registration: { termsVersion: 'test-v1', - webUrl: 'http://localhost:3000', }, }), ); From a6ae10dd4ba96b063c54d216bab3d8c951d53c84 Mon Sep 17 00:00:00 2001 From: Volodymyr Zakhovaiko Date: Mon, 24 Aug 2026 12:34:26 +0200 Subject: [PATCH 2/4] fix(identity): scope the 2fa verify sign-out to the session just minted Verifying the emailed code on a 2FA-enrolled account expired every session on the user, not only the one better-auth minted moments earlier, so the player was silently logged out of all their devices. The RG and backoffice branch keeps revoking everything - that one is deliberate. --- .../src/pam/identity/service/identity.service.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/core/src/pam/identity/service/identity.service.ts b/packages/core/src/pam/identity/service/identity.service.ts index 2ca1626d..b28fa8d7 100644 --- a/packages/core/src/pam/identity/service/identity.service.ts +++ b/packages/core/src/pam/identity/service/identity.service.ts @@ -1247,10 +1247,15 @@ export class IdentityService { // the emailed code alone, bypassing its second factor. Verification still stands; the // session does not, and the player signs in through `login` to face the challenge. if (account?.twoFactorEnabled) { - await this.drizzle.db - .update(session) - .set({ expiresAt: new Date() }) - .where(eq(session.userId, body.user.id)); + // Scoped to the token this call just minted: the player's other devices did nothing + // wrong, and an unrelated sign-out here would be indistinguishable from a session + // hijack. The RG/backoffice branch above is the one that revokes everything. + if (body.token) { + await this.drizzle.db + .update(session) + .set({ expiresAt: new Date() }) + .where(eq(session.token, body.token)); + } return { twoFactorRedirect: true as const }; } From 1583963792eac87791b4ca594e86aa3158c6d7a5 Mon Sep 17 00:00:00 2001 From: Damian Rzepka Date: Mon, 24 Aug 2026 21:11:43 +0200 Subject: [PATCH 3/4] feat(profile): player profile fields for the registration profile step (BF-379) The optional "complete your profile" step after email verification had nowhere to write: `player` carried only country and currency, so first name, last name, date of birth and a contact number were dropped on the floor. `player` gains all four as nullable columns, surfaced on `PlayerSchema` and writable through `PATCH /profile`. The step is skippable and pre-existing rows keep reading, so nullable is the honest shape. `phone` is deliberately not unique. `user.phoneNumber` is unique because it is a phone-login credential; making the self-declared contact number unique too would answer 409 for a number the caller does not own - a phone-enumeration oracle of exactly the kind `PhoneLoginService.requestOtp` shadow-responses to avoid - and would let anyone squat a stranger's number before they ever verify it. The two are different things: a contact detail and a credential. A future phone-verification flow promotes one to the other. `dateOfBirth` is a calendar date in string mode, not a timestamp: the default Date mode round-trips through a timezone and shifts the day for players either side of UTC. It is rejected on input under 18, compared calendar-wise so a leap day cannot move the boundary. Also fixes a 500 on the same route. `UpdatePlayerProfileInputSchema` accepted an update carrying no fields, which reached `db.update().set({})` and threw; it now requires at least one and answers 400, matching the identity module's own profile contract. `country` is held to an ISO 3166-1 alpha-2 code so it can be compared against the igaming config's jurisdictions and blocked-country lists. Adds the changeset the earlier OTP commit on this branch omitted, including the `registration.webUrl` -> `requireEmailVerification` migration note - the config schema is strict, so the stale key fails a consumer's boot rather than warning. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0184Pm2rzwzCHvDJxcpqYzXR --- .changeset/player-profile-fields.md | 13 + .changeset/registration-email-otp.md | 15 ++ .../src/contracts/__tests__/player.test.ts | 71 ++++++ packages/core/src/contracts/schemas/player.ts | 51 +++- .../__tests__/profile.service.int.test.ts | 51 ++++ .../migrations/0003_absurd_morbius.sql | 4 + .../migrations/meta/0003_snapshot.json | 224 ++++++++++++++++++ .../drizzle/migrations/meta/_journal.json | 7 + packages/core/src/pam/profile/schema/index.ts | 10 + packages/core/src/pam/shared/player-mapper.ts | 6 + .../src/__tests__/registration.e2e.test.ts | 53 +++++ packages/testing/src/seed-demo-data.ts | 12 + 12 files changed, 512 insertions(+), 5 deletions(-) create mode 100644 .changeset/player-profile-fields.md create mode 100644 .changeset/registration-email-otp.md create mode 100644 packages/core/src/contracts/__tests__/player.test.ts create mode 100644 packages/core/src/pam/profile/drizzle/migrations/0003_absurd_morbius.sql create mode 100644 packages/core/src/pam/profile/drizzle/migrations/meta/0003_snapshot.json diff --git a/.changeset/player-profile-fields.md b/.changeset/player-profile-fields.md new file mode 100644 index 00000000..5e2c4563 --- /dev/null +++ b/.changeset/player-profile-fields.md @@ -0,0 +1,13 @@ +--- +'@openora/core': minor +--- + +The player profile gains the fields a registration flow's optional profile step collects: `firstName`, `lastName`, `dateOfBirth`, and `phone` on the `player` table, surfaced on `PlayerSchema` and writable through `PATCH /profile`. All four are nullable - the step is skippable, and rows written before this release keep reading fine. + +- `phone` is the player's self-declared contact number and is deliberately **not** unique. `user.phoneNumber` stays the unique, verified phone-login credential; making this one unique too would turn an optional profile field into a phone-enumeration oracle (a duplicate would answer `409` for a number the caller does not own) and would let anyone permanently squat a stranger's number before they verify it. A future phone-verification flow promotes `player.phone` to `user.phoneNumber`. +- `dateOfBirth` is a plain calendar date (`YYYY-MM-DD`, no timezone) and is rejected on input when it puts the player under 18. +- `country` on this route is now held to an ISO 3166-1 alpha-2 code rather than any string, so it can actually be compared against the `jurisdictions` and `blockedCountries` lists in the igaming config. + +Fixes a `500` on the same route: `UpdatePlayerProfileInputSchema` accepted an update carrying no fields, which reached `db.update().set({})` and threw. It now requires at least one field and answers `400`, matching the identity module's own profile-update contract. + +Consumers apply the `profile` module's `0003` migration (`pnpm db:migrate`). diff --git a/.changeset/registration-email-otp.md b/.changeset/registration-email-otp.md new file mode 100644 index 00000000..c42253f6 --- /dev/null +++ b/.changeset/registration-email-otp.md @@ -0,0 +1,15 @@ +--- +'@openora/core': minor +--- + +Registration now mails a six-digit code instead of a magic link, and verifying that code is what mints the session. Sign-up stays sessionless, which is what keeps its duplicate-email answer indistinguishable, so a new player still lands signed in without giving up account enumeration. + +Email verification is no longer enforced on sign-in - it moved behind `registration.requireEmailVerification` (default off), so unverified players stay unrestricted while the KYC toggle is off. The RG and suspension gate is now one shared implementation used by password login, phone login, and the new verification path, so a session cannot be handed out through a gate that only some paths enforce. + +Upgrading a consumer takes three edits: + +- **`registration.webUrl` is gone, replaced by `registration.requireEmailVerification`.** `PlatformConfigSchema` is `.strict()`, so leaving the old key in place does not warn - it throws `Invalid platform config: registration: Unrecognized key: "webUrl"` the first time `PLATFORM_CONFIG` resolves, which is during router construction and therefore before the server ever listens. +- **`verifyEmail` takes `{ email, otp }` instead of `{ token }`** and returns `{ user?, session?, twoFactorRedirect? }` rather than `{ success: true }`. A 2FA-enrolled account is verified but deliberately not signed in: it gets `twoFactorRedirect` and completes the challenge through `login`. +- **`sendEmailVerification` takes `{ email }` and is unauthenticated**, since the player has no session until the code is verified. It always answers success, so it never reveals whether the address has an account. + +The `verifyEmail` email template renders an `otp` instead of a `url` - a consumer with its own template renderer must update it, or the mail goes out with an empty body. diff --git a/packages/core/src/contracts/__tests__/player.test.ts b/packages/core/src/contracts/__tests__/player.test.ts new file mode 100644 index 00000000..f4515d3d --- /dev/null +++ b/packages/core/src/contracts/__tests__/player.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect } from 'vitest'; +import { + MIN_PLAYER_AGE_YEARS, + UpdatePlayerProfileInputSchema, + isAdultDateOfBirth, +} from '../schemas/player.js'; + +const NOW = new Date('2026-08-24T00:00:00Z'); + +function isoBirthday(yearsAgo: number, offsetDays = 0) { + const d = new Date(NOW); + d.setUTCFullYear(d.getUTCFullYear() - yearsAgo); + d.setUTCDate(d.getUTCDate() + offsetDays); + return d.toISOString().slice(0, 10); +} + +describe('isAdultDateOfBirth', () => { + it('passes a player on their own 18th birthday', () => { + expect(isAdultDateOfBirth(isoBirthday(MIN_PLAYER_AGE_YEARS), NOW)).toBe(true); + }); + + it('rejects a player one day short of 18', () => { + expect(isAdultDateOfBirth(isoBirthday(MIN_PLAYER_AGE_YEARS, 1), NOW)).toBe(false); + }); + + it('rejects a date in the future', () => { + expect(isAdultDateOfBirth('2030-01-01', NOW)).toBe(false); + }); +}); + +describe('UpdatePlayerProfileInputSchema', () => { + it('accepts the full profile the signup step collects', () => { + const input = { + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + country: 'GB', + }; + + expect(UpdatePlayerProfileInputSchema.parse(input)).toEqual(input); + }); + + it('accepts null to clear an optional field', () => { + expect(UpdatePlayerProfileInputSchema.safeParse({ phone: null }).success).toBe(true); + }); + + // Without this the handler reaches `db.update().set({})`, which drizzle rejects at + // runtime - a 500 on a form where the player simply changed nothing. + it('rejects an update that carries no fields', () => { + expect(UpdatePlayerProfileInputSchema.safeParse({}).success).toBe(false); + }); + + it('rejects a country display name instead of an ISO 3166-1 alpha-2 code', () => { + expect(UpdatePlayerProfileInputSchema.safeParse({ country: 'United Kingdom' }).success).toBe( + false, + ); + }); + + it('rejects a phone number that is not E.164', () => { + expect(UpdatePlayerProfileInputSchema.safeParse({ phone: '+44 1632 960001' }).success).toBe( + false, + ); + }); + + it('rejects a date of birth under the minimum age', () => { + const result = UpdatePlayerProfileInputSchema.safeParse({ dateOfBirth: '2020-01-01' }); + + expect(result.success).toBe(false); + }); +}); diff --git a/packages/core/src/contracts/schemas/player.ts b/packages/core/src/contracts/schemas/player.ts index dda113ca..45ebcad2 100644 --- a/packages/core/src/contracts/schemas/player.ts +++ b/packages/core/src/contracts/schemas/player.ts @@ -1,6 +1,7 @@ import * as z from 'zod'; import { MoneyAmountSchema, TimestampSchema, UuidSchema } from './common.js'; -import { CurrencyCodeSchema } from './igaming-config.js'; +import { CountryCodeSchema, CurrencyCodeSchema } from './igaming-config.js'; +import { E164PhoneSchema } from './identity.js'; import { TagKeySchema } from './tag.js'; import { PageQuerySchema, SortOrderSchema } from '../kit.js'; @@ -45,6 +46,10 @@ export const PlayerSchema = z.object({ userId: UuidSchema, username: z.string(), email: z.string(), + firstName: z.string().nullable(), + lastName: z.string().nullable(), + dateOfBirth: z.iso.date().nullable(), + phone: z.string().nullable(), country: z.string().nullable(), currency: CurrencyCodeSchema, status: PlayerStatusSchema, @@ -90,9 +95,45 @@ export type KycStatus = z.infer; export type PaginatedPlayerListSearchArgs = z.infer; -export const UpdatePlayerProfileInputSchema = PlayerSchema.pick({ - country: true, - currency: true, -}).partial(); +export const MIN_PLAYER_AGE_YEARS = 18; + +/** + * Compares calendar dates, not elapsed milliseconds: a leap day between the birth date + * and the cutoff would shift a ms-based boundary by a day. The player's own 18th birthday + * passes. + */ +export function isAdultDateOfBirth(dateOfBirth: string, now = new Date()): boolean { + const cutoff = + (now.getUTCFullYear() - MIN_PLAYER_AGE_YEARS) * 10_000 + + (now.getUTCMonth() + 1) * 100 + + now.getUTCDate(); + return Number(dateOfBirth.replaceAll('-', '')) <= cutoff; +} + +/** + * Deliberately narrower than `PlayerSchema`: the read side stays tolerant of rows written + * before these columns existed, while a write is held to ISO country codes and E.164 + * phones. `null` clears a field; omitting it leaves the stored value alone. `phone` is the + * player's self-declared contact number - the verified login credential lives on the + * identity module's `user.phoneNumber` and is never written from here. + */ +export const UpdatePlayerProfileInputSchema = z + .object({ + firstName: z.string().min(1).max(100).nullable(), + lastName: z.string().min(1).max(100).nullable(), + dateOfBirth: z.iso + .date() + .refine((value) => isAdultDateOfBirth(value), { + message: `Player must be at least ${MIN_PLAYER_AGE_YEARS} years old`, + }) + .nullable(), + phone: E164PhoneSchema.nullable(), + country: CountryCodeSchema.nullable(), + currency: CurrencyCodeSchema, + }) + .partial() + .refine((v) => Object.values(v).some((x) => x !== undefined), { + message: 'Provide at least one field to update', + }); export type UpdatePlayerProfileInput = z.infer; diff --git a/packages/core/src/pam/profile/__tests__/profile.service.int.test.ts b/packages/core/src/pam/profile/__tests__/profile.service.int.test.ts index fe36f703..8b897d98 100644 --- a/packages/core/src/pam/profile/__tests__/profile.service.int.test.ts +++ b/packages/core/src/pam/profile/__tests__/profile.service.int.test.ts @@ -97,6 +97,57 @@ describe('ProfileService.updateMyProfile (real PG)', () => { expect(await playersFor(account.id)).toHaveLength(1); }); + it('persists the registration profile fields the signup step collects', async () => { + const svc = makeService(); + const account = await seedUser(db); + await seedPlayer(account.id); + + const result = await svc.updateMyProfile(account.id, { + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + country: 'GB', + }); + + expect(result).toMatchObject({ + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + country: 'GB', + }); + const [row] = await playersFor(account.id); + expect(row).toMatchObject({ + firstName: 'Ada', + lastName: 'Lovelace', + // A `date` column in string mode must come back as the calendar day it went in as, + // with no timezone shift. + dateOfBirth: '1990-05-17', + phone: '+441632960001', + }); + }); + + it('leaves omitted fields alone and clears the ones explicitly set to null', async () => { + const svc = makeService(); + const account = await seedUser(db); + await seedPlayer(account.id, { + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + }); + + const result = await svc.updateMyProfile(account.id, { firstName: 'Augusta', phone: null }); + + expect(result).toMatchObject({ + firstName: 'Augusta', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: null, + }); + }); + it('leaves other players untouched', async () => { const svc = makeService(); const account = await seedUser(db); diff --git a/packages/core/src/pam/profile/drizzle/migrations/0003_absurd_morbius.sql b/packages/core/src/pam/profile/drizzle/migrations/0003_absurd_morbius.sql new file mode 100644 index 00000000..8ea0fb71 --- /dev/null +++ b/packages/core/src/pam/profile/drizzle/migrations/0003_absurd_morbius.sql @@ -0,0 +1,4 @@ +ALTER TABLE "player" ADD COLUMN "first_name" text;--> statement-breakpoint +ALTER TABLE "player" ADD COLUMN "last_name" text;--> statement-breakpoint +ALTER TABLE "player" ADD COLUMN "date_of_birth" date;--> statement-breakpoint +ALTER TABLE "player" ADD COLUMN "phone" text; \ No newline at end of file diff --git a/packages/core/src/pam/profile/drizzle/migrations/meta/0003_snapshot.json b/packages/core/src/pam/profile/drizzle/migrations/meta/0003_snapshot.json new file mode 100644 index 00000000..f7917dd3 --- /dev/null +++ b/packages/core/src/pam/profile/drizzle/migrations/meta/0003_snapshot.json @@ -0,0 +1,224 @@ +{ + "id": "3a65a978-c6e6-4a7d-8a4b-138c529b8ba1", + "prevId": "f0525db2-f419-41f1-913d-ddf84c172e55", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.player": { + "name": "player", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "user_id": { + "name": "user_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "first_name": { + "name": "first_name", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "last_name": { + "name": "last_name", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "date_of_birth": { + "name": "date_of_birth", + "type": "date", + "primaryKey": false, + "notNull": false + }, + "phone": { + "name": "phone", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "country": { + "name": "country", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "currency": { + "name": "currency", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'USD'" + }, + "status": { + "name": "status", + "type": "player_status", + "typeSchema": "public", + "primaryKey": false, + "notNull": true, + "default": "'active'" + }, + "kyc_status": { + "name": "kyc_status", + "type": "kyc_status", + "typeSchema": "public", + "primaryKey": false, + "notNull": true, + "default": "'pending'" + }, + "level": { + "name": "level", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 1 + }, + "total_wagered": { + "name": "total_wagered", + "type": "numeric(18, 2)", + "primaryKey": false, + "notNull": true, + "default": "'0'" + }, + "total_deposits": { + "name": "total_deposits", + "type": "numeric(18, 2)", + "primaryKey": false, + "notNull": true, + "default": "'0'" + }, + "last_seen_at": { + "name": "last_seen_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "terms_version": { + "name": "terms_version", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "terms_accepted_at": { + "name": "terms_accepted_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "age_accepted_at": { + "name": "age_accepted_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "registration_ip": { + "name": "registration_ip", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "registration_user_agent": { + "name": "registration_user_agent", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "player_status_idx": { + "name": "player_status_idx", + "columns": [ + { + "expression": "status", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "player_created_at_idx": { + "name": "player_created_at_idx", + "columns": [ + { + "expression": "created_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "player_user_id_unique": { + "name": "player_user_id_unique", + "nullsNotDistinct": false, + "columns": ["user_id"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": { + "public.kyc_status": { + "name": "kyc_status", + "schema": "public", + "values": [ + "not_started", + "pending", + "approved", + "verified", + "rejected", + "resubmission_requested", + "manually_overridden" + ] + }, + "public.player_status": { + "name": "player_status", + "schema": "public", + "values": ["active", "dormant", "self_excluded", "suspended", "closed"] + } + }, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} diff --git a/packages/core/src/pam/profile/drizzle/migrations/meta/_journal.json b/packages/core/src/pam/profile/drizzle/migrations/meta/_journal.json index c7bb6b41..85bbd227 100644 --- a/packages/core/src/pam/profile/drizzle/migrations/meta/_journal.json +++ b/packages/core/src/pam/profile/drizzle/migrations/meta/_journal.json @@ -22,6 +22,13 @@ "when": 1787339175627, "tag": "0002_sour_alex_wilder", "breakpoints": true + }, + { + "idx": 3, + "version": "7", + "when": 1787589924992, + "tag": "0003_absurd_morbius", + "breakpoints": true } ] } diff --git a/packages/core/src/pam/profile/schema/index.ts b/packages/core/src/pam/profile/schema/index.ts index 39a392bc..e6d779d1 100644 --- a/packages/core/src/pam/profile/schema/index.ts +++ b/packages/core/src/pam/profile/schema/index.ts @@ -4,6 +4,7 @@ import { text, integer, decimal, + date, timestamp, pgEnum, index, @@ -18,6 +19,15 @@ export const player = pgTable( { id: uuid().primaryKey().defaultRandom(), userId: uuid().notNull().unique('player_user_id_unique'), + firstName: text(), + lastName: text(), + // `mode: 'string'` keeps a birth date a plain calendar date: the default Date mode + // round-trips through a timestamp and shifts the day for players east or west of UTC. + dateOfBirth: date({ mode: 'string' }), + // Self-declared contact number - deliberately not unique. `user.phoneNumber` is unique + // because it is a login credential; making this one unique too would turn an optional + // profile field into a phone-enumeration oracle and let anyone squat a stranger's number. + phone: text(), country: text(), currency: text().notNull().default('USD'), status: playerStatusEnum().notNull().default('active'), diff --git a/packages/core/src/pam/shared/player-mapper.ts b/packages/core/src/pam/shared/player-mapper.ts index bc48b494..0e634e73 100644 --- a/packages/core/src/pam/shared/player-mapper.ts +++ b/packages/core/src/pam/shared/player-mapper.ts @@ -10,6 +10,12 @@ export function toPlayer(row: typeof player.$inferSelect, email: string, usernam userId: row.userId, username, email, + firstName: row.firstName, + lastName: row.lastName, + // Already a 'YYYY-MM-DD' string - the column is a `date` in string mode, unlike the + // timestamps below. + dateOfBirth: row.dateOfBirth, + phone: row.phone, country: row.country, currency: row.currency, status: row.status, diff --git a/packages/testing/src/__tests__/registration.e2e.test.ts b/packages/testing/src/__tests__/registration.e2e.test.ts index 449f69b5..1d1dc1de 100644 --- a/packages/testing/src/__tests__/registration.e2e.test.ts +++ b/packages/testing/src/__tests__/registration.e2e.test.ts @@ -68,6 +68,59 @@ describe('registration email verification', () => { expect(body.session.token).toBeTruthy(); }); + it('lets the verified player fill in the optional profile step with that session', async () => { + // The modal's last screen: the session the code just minted is the only thing + // authorising the write, so this is the whole register -> code -> profile path. + const email = `reg-profile-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); + const verified = await verifyEmailByOtp(app, email); + const cookie = (verified.headers.get('set-cookie') ?? '').split(';')[0] ?? ''; + expect(cookie).toBeTruthy(); + + const res = await app.app.request('/profile', { + method: 'PATCH', + headers: { 'content-type': 'application/json', cookie }, + body: JSON.stringify({ + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + country: 'GB', + }), + }); + + expect(res.status).toBe(200); + const [row] = await app.container + .get(DRIZZLE) + .db.select() + .from(player) + .where(eq(player.userId, (await userIdFor(email)) ?? '')); + expect(row).toMatchObject({ + firstName: 'Ada', + lastName: 'Lovelace', + dateOfBirth: '1990-05-17', + phone: '+441632960001', + country: 'GB', + }); + }); + + it('rejects a profile update that carries no fields', async () => { + // `Skip` must not post an empty body: without the contract's own guard this reaches + // `db.update().set({})` and 500s. + const email = `reg-profile-empty-${randomUUID()}@e2e.test`; + await submitRegistration(app, { email }); + const verified = await verifyEmailByOtp(app, email); + const cookie = (verified.headers.get('set-cookie') ?? '').split(';')[0] ?? ''; + + const res = await app.app.request('/profile', { + method: 'PATCH', + headers: { 'content-type': 'application/json', cookie }, + body: JSON.stringify({}), + }); + + expect(res.status).toBe(400); + }); + it('refuses to sign in a blocked account that enters a valid code', async () => { // An account can be RG-blocked or suspended between registering and entering the // code. Verification still stands - the block is on the session, not the address. diff --git a/packages/testing/src/seed-demo-data.ts b/packages/testing/src/seed-demo-data.ts index ccc534cd..775d905a 100644 --- a/packages/testing/src/seed-demo-data.ts +++ b/packages/testing/src/seed-demo-data.ts @@ -511,8 +511,20 @@ export async function seedDemoData(options: SeedOptions): Promise { ? new Date(now - (30 + Math.floor(rng() * 60)) * dayMs) : null; + // Same `rng` stream as the rest of the row, so the seed stays reproducible. Ages 21-65 + // keep every demo player clear of the 18+ floor the profile contract enforces. + const dateOfBirth = new Date(now - (21 + Math.floor(rng() * 45)) * 365.25 * dayMs) + .toISOString() + .slice(0, 10); + await db.insert(player).values({ userId: playerUser.id, + firstName: first, + lastName: last, + dateOfBirth, + // The contact number mirrors the login credential for a demo player; a real one fills + // this in on the optional profile step long before any phone verification. + phone: phoneNumber, country, currency, status, From c610c58198bb281a8711bfb7d39d5a4faa13b749 Mon Sep 17 00:00:00 2001 From: Damian Rzepka Date: Tue, 25 Aug 2026 02:40:53 +0200 Subject: [PATCH 4/4] feat(identity): audit every registration attempt and explain the duplicate-email mail (BF-379) A log of registration activity showed only the attempts that worked. Nothing recorded a rejection: not the rate limits, not a geo block, not a taken username, not a consent write that had to roll the user back. The trail existed once, in a consumer overlay that recorded `registration.attempted` with its outcome; handing registration to core deleted the overlay and never replaced what it did. `identity.user.registration.failed` carries the address, the attempted handle, the origin and a typed reason, and the audit plugin files it under a `registration` resource with `result: 'failure'`. The known-address branch emits it too. That branch is the delicate one: no account is created, yet the caller is deliberately told the attempt succeeded, because a truthful answer there is an enumeration oracle. The audit log records what happened; the response does not. Two publications, not a contradiction. The accepted terms now ride along on the success event. They are omitted when the consent write was discarded because a player row already existed - a trail that implies evidence which was never stored is worse than one that stays quiet. Separately, whoever tried to sign up with an address that already had an account received "Your password reset code is: 123456" and nothing else. better-auth issues that mail and a self-service reset through the same `forget-password` OTP type, so no renderer could tell them apart - the consumer could not fix this from outside. `createAuth` now takes a predicate and picks a new `existingAccountSignUp` template, whose copy says no new account was created and offers the code as a reset. The predicate is fed by a set held only for the duration of the send; the call chain is synchronous, so it needs no TTL and no cache. Password rules are now shared by the flows that set one. Sign-up had no upper bound while reset capped at better-auth's 128, so an over-length password passed the contract and came back as a generic "Registration is unavailable". Sign-in stays uncapped - no longer password was ever storable, so bounding it there would narrow a contract for nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0184Pm2rzwzCHvDJxcpqYzXR --- .../registration-audit-and-duplicate-email.md | 17 ++ docs/catalog.json | 9 ++ .../src/audit/__tests__/map-event.test.ts | 31 ++++ packages/core/src/audit/plugin.ts | 8 + .../src/contracts/__tests__/identity.test.ts | 61 ++++++++ .../src/contracts/adapters/email-template.ts | 14 ++ packages/core/src/contracts/schemas/events.ts | 14 ++ .../core/src/contracts/schemas/identity.ts | 36 ++++- .../identity.registration-gates.int.test.ts | 29 ++++ .../default-email-template-renderer.test.ts | 14 ++ .../pam/identity/service/identity.service.ts | 145 +++++++++++++----- .../src/server/auth/__tests__/auth.test.ts | 30 ++++ packages/core/src/server/auth/auth.ts | 13 +- .../src/__tests__/registration.e2e.test.ts | 55 ++++++- packages/testing/src/register.ts | 8 +- 15 files changed, 436 insertions(+), 48 deletions(-) create mode 100644 .changeset/registration-audit-and-duplicate-email.md create mode 100644 packages/core/src/contracts/__tests__/identity.test.ts diff --git a/.changeset/registration-audit-and-duplicate-email.md b/.changeset/registration-audit-and-duplicate-email.md new file mode 100644 index 00000000..65d03d99 --- /dev/null +++ b/.changeset/registration-audit-and-duplicate-email.md @@ -0,0 +1,17 @@ +--- +'@openora/core': minor +--- + +Registration now leaves an audit trail whether or not it produces an account, and a sign-up on an address that already has one gets its own email instead of a bare password-reset code. + +**Rejected attempts are audited.** New `identity.user.registration.failed` domain event, emitted by every path in `IdentityService.register` that ends without an account - registration not configured, either rate limit, a geo block, a taken username, a failed sign-up, and a consent write that had to roll the user back. It carries the address, the attempted username, the origin (IP and user agent) and a typed `reason`; the audit plugin subscribes to it and files it under `resourceType: 'registration'` with `result: 'failure'`. Previously only successes were recorded, so a log of registration activity showed nothing but the attempts that worked. + +The known-address branch emits it too, with `reason: 'email_already_registered'`. The HTTP response is unchanged and still indistinguishable from a fresh sign-up - the audit log records what happened, the response deliberately does not, because a truthful answer there is an account-enumeration oracle. + +**The accepted terms reach the audit log.** `identity.user.registered` now carries `termsVersion`, `acceptedTerms` and `acceptedAge`. They are omitted when the consent write was discarded because a player row already existed, so the trail never implies evidence that was not stored. The `player` row already held the current values; it is not a record of the act. + +**New `existingAccountSignUp` email template key.** better-auth issues both the self-service reset and the duplicate-sign-up notice through the same `forget-password` OTP type, so a renderer could not tell them apart and the second one went out as "Your password reset code is: …" to a player who had asked for no such thing. `createAuth` takes an `isExistingAccountSignUp` predicate and picks the new key; the shipped copy explains that no new account was created and offers the code as a reset. + +Adding the key is minor-breaking for a consumer whose renderer is backed by a full `Record` map - it will not compile until the new entry exists. A renderer that switches on the key with a fallback is unaffected. + +**Password rules match across the flows that set one.** `PasswordSchema` (min 8, max 128) is now shared by sign-up, password change and password reset. Sign-up previously had no upper bound, so an over-length password passed the contract and was rejected by better-auth as a generic "Registration is unavailable". Sign-in is deliberately left uncapped: no longer password was ever storable, so a bound there could only narrow an existing contract. diff --git a/docs/catalog.json b/docs/catalog.json index 5dc95edd..e708917a 100644 --- a/docs/catalog.json +++ b/docs/catalog.json @@ -942,6 +942,7 @@ "identity.user.phone_login", "identity.user.reactivated", "identity.user.registered", + "identity.user.registration.failed", "identity.user.unauthorized_access", "identity.user.unlocked", "notifications.created", @@ -1739,6 +1740,10 @@ "name": "PaginatedPlayerSearchArgsSchema", "file": "packages/core/src/contracts/schemas/player.ts" }, + { + "name": "PasswordSchema", + "file": "packages/core/src/contracts/schemas/identity.ts" + }, { "name": "PaymentWebhookInputSchema", "file": "packages/core/src/wallet/contract/index.ts" @@ -1911,6 +1916,10 @@ "name": "RegistrationConfigSchema", "file": "packages/core/src/contracts/schemas/platform-config.ts" }, + { + "name": "RegistrationFailureReasonSchema", + "file": "packages/core/src/contracts/schemas/identity.ts" + }, { "name": "RegistrationsOverTimePointSchema", "file": "packages/core/src/admin-console/contract/index.ts" diff --git a/packages/core/src/audit/__tests__/map-event.test.ts b/packages/core/src/audit/__tests__/map-event.test.ts index 8b259b27..692562bc 100644 --- a/packages/core/src/audit/__tests__/map-event.test.ts +++ b/packages/core/src/audit/__tests__/map-event.test.ts @@ -29,3 +29,34 @@ describe('mapEventToRecord: identity.session.revoked', () => { expect(row).toMatchObject({ actorType: 'admin', actorId: adminId, resourceId: sessionId }); }); }); + +describe('mapEventToRecord: identity.user.registration.failed', () => { + it('records a rejected attempt as a failure against the registration resource', async () => { + const row = await mapEventToRecord('identity.user.registration.failed', { + email: 'taken@example.com', + username: 'taken_handle', + reason: 'username_taken', + ip: '203.0.113.7', + userAgent: 'Mozilla/5.0', + }); + + expect(row).toMatchObject({ + result: 'failure', + resourceType: 'registration', + actorType: 'system', + resourceId: null, + ip: '203.0.113.7', + userAgent: 'Mozilla/5.0', + }); + }); + + it('carries the address and reason through, since a rejected attempt has no actor', async () => { + const row = await mapEventToRecord('identity.user.registration.failed', { + email: 'blocked@example.com', + reason: 'geo_blocked', + }); + + expect(row.after).toMatchObject({ email: 'blocked@example.com', reason: 'geo_blocked' }); + expect(row.actorId).toBeUndefined(); + }); +}); diff --git a/packages/core/src/audit/plugin.ts b/packages/core/src/audit/plugin.ts index dbbe70af..fd06d82d 100644 --- a/packages/core/src/audit/plugin.ts +++ b/packages/core/src/audit/plugin.ts @@ -615,6 +615,13 @@ export async function mapEventToRecord( return { ...base, actorId: str(p['playerId']), actorType: 'player' }; } + // A rejected attempt has no account behind it, so there is no actor to name - only the + // address that was tried, which `after` already carries. `resourceType` is narrowed off + // the default 'identity' so an auditor can pull registration attempts on their own. + if (topic === 'identity.user.registration.failed') { + return { ...base, resourceType: 'registration' }; + } + // Shared identity self-action topics: the same `/identity/*` endpoints serve // both player and admin accounts, so playerId only resolves for a player. A // null playerId means the account has no player row - attribute to the @@ -645,6 +652,7 @@ export async function mapEventToRecord( const SUBSCRIBED_TOPICS: DomainEventName[] = [ 'identity.user.registered', + 'identity.user.registration.failed', 'identity.user.login', 'identity.user.login.failed', 'identity.user.lockout.triggered', diff --git a/packages/core/src/contracts/__tests__/identity.test.ts b/packages/core/src/contracts/__tests__/identity.test.ts new file mode 100644 index 00000000..001d096b --- /dev/null +++ b/packages/core/src/contracts/__tests__/identity.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest'; +import { + ChangePasswordInputSchema, + LoginInputSchema, + RegisterInputSchema, + ResetPasswordInputSchema, +} from '../schemas/identity.js'; + +const OVER_LENGTH = 'a'.repeat(129); + +const registerInput = (password: string) => ({ + email: 'player@example.com', + password, + username: 'new_player', + acceptedTerms: true as const, + acceptedAge: true as const, +}); + +describe('password rules', () => { + it('holds sign-up to the same bounds as the password reset flow', () => { + expect(RegisterInputSchema.safeParse(registerInput('short')).success).toBe(false); + expect(RegisterInputSchema.safeParse(registerInput('password123')).success).toBe(true); + }); + + // better-auth caps at 128 itself, but only after the fact: on sign-up that surfaces as + // a generic "Registration is unavailable", and on reset it burns the one-time code + // first. Both must be rejected at the contract instead. + it.each([ + ['register', () => RegisterInputSchema.safeParse(registerInput(OVER_LENGTH))], + [ + 'reset', + () => + ResetPasswordInputSchema.safeParse({ + email: 'player@example.com', + otp: '123456', + newPassword: OVER_LENGTH, + }), + ], + [ + 'change', + () => + ChangePasswordInputSchema.safeParse({ + currentPassword: 'password123', + newPassword: OVER_LENGTH, + }), + ], + ])('rejects a password over 128 characters on %s', (_name, parse) => { + expect(parse().success).toBe(false); + }); + + // Sign-in is deliberately left uncapped: no longer password was ever storable, so a + // bound there could only narrow an existing contract for nothing. + it('does not cap the password on sign-in', () => { + const result = LoginInputSchema.safeParse({ + email: 'player@example.com', + password: OVER_LENGTH, + }); + + expect(result.success).toBe(true); + }); +}); diff --git a/packages/core/src/contracts/adapters/email-template.ts b/packages/core/src/contracts/adapters/email-template.ts index a93d60ea..2b2be61a 100644 --- a/packages/core/src/contracts/adapters/email-template.ts +++ b/packages/core/src/contracts/adapters/email-template.ts @@ -3,6 +3,7 @@ import { createToken, type Token } from './token.js'; export type EmailTemplateKey = | 'verifyEmail' | 'resetPasswordOtp' + | 'existingAccountSignUp' | 'rgLimitUpdated' | 'rgCoolingOffActivated' | 'rgCoolingOffLifted' @@ -12,6 +13,7 @@ export type EmailTemplateKey = export type EmailTemplateData = { verifyEmail: { otp: string }; resetPasswordOtp: { otp: string; email: string }; + existingAccountSignUp: { otp: string; email: string }; rgLimitUpdated: { period: string; type: string; description: string }; rgCoolingOffActivated: { expiresAt: Date }; rgCoolingOffLifted: Record; @@ -58,6 +60,18 @@ export const DEFAULT_EMAIL_TEMPLATES: { subject: 'Reset your password', body: `Your password reset code is: ${data.otp}`, }), + // Sent when someone tries to sign up with an address that already has an account. It + // must never confirm or deny that the account exists to anyone but its owner, so the + // wording addresses the owner and the sign-up response stays identical either way. + // Carries a reset code rather than only a notice: whoever tried is most likely the + // owner having forgotten they registered. + existingAccountSignUp: (data) => ({ + subject: 'You already have an account', + body: + `Someone tried to create an account with this email address. ` + + `You already have one, so no new account was created. ` + + `If it was you, sign in as usual - or use this code to reset your password: ${data.otp}`, + }), rgLimitUpdated: (data) => ({ subject: 'Your gambling limit was updated', body: `A ${data.period} ${data.type} limit of ${data.description} is now active on your account.`, diff --git a/packages/core/src/contracts/schemas/events.ts b/packages/core/src/contracts/schemas/events.ts index 53e11245..4362b1ab 100644 --- a/packages/core/src/contracts/schemas/events.ts +++ b/packages/core/src/contracts/schemas/events.ts @@ -9,6 +9,7 @@ import { import { TagKeySchema } from './tag.js'; import { CurrencyCodeSchema, CountryCodeSchema } from './igaming-config.js'; import { PermissionLevelSchema } from './iam.js'; +import { RegistrationFailureReasonSchema, UsernameSchema } from './identity.js'; import { KycStatusSchema, KycStatusSourceSchema, PlayerStatusSchema } from './player.js'; // Optional request-origin metadata shared by HTTP-triggered events; both fields may be absent. @@ -53,6 +54,19 @@ export const domainEventSchemas = { 'identity.user.registered': authContextBase.extend({ userId: UuidSchema, playerId: UuidSchema.nullable(), + // The consent the player actually gave, carried so the audit trail holds it too - + // the `player` row alone is current state, not a record of the act. Absent when the + // consent write was discarded because a player row already existed. + termsVersion: z.string().optional(), + acceptedTerms: z.literal(true).optional(), + acceptedAge: z.literal(true).optional(), + }), + // No `userId`: registration is unauthenticated, so the address is the only subject + // there is - the same shape `identity.user.login.failed` settled on below. + 'identity.user.registration.failed': authContextBase.extend({ + email: z.email(), + username: UsernameSchema.optional(), + reason: RegistrationFailureReasonSchema, }), 'identity.user.login': authContextBase.extend({ userId: UuidSchema, diff --git a/packages/core/src/contracts/schemas/identity.ts b/packages/core/src/contracts/schemas/identity.ts index 01ebfc04..7224a0c4 100644 --- a/packages/core/src/contracts/schemas/identity.ts +++ b/packages/core/src/contracts/schemas/identity.ts @@ -57,6 +57,21 @@ export const PhoneLoginErrorReasonSchema = z.enum(PHONE_LOGIN_ERROR_REASONS); export const PHONE_LOGIN_OTP_INVALID_REASONS = ['expired', 'wrong_code'] as const; export const PhoneLoginOtpInvalidReasonSchema = z.enum(PHONE_LOGIN_OTP_INVALID_REASONS); +/** + * Why a registration attempt produced no account. `email_already_registered` is the odd + * one out: the caller is deliberately told the attempt succeeded, because saying otherwise + * is an account-enumeration oracle. The audit trail records what actually happened. + */ +export const REGISTRATION_FAILURE_REASONS = [ + 'registration_disabled', + 'rate_limited', + 'geo_blocked', + 'username_taken', + 'email_already_registered', + 'error', +] as const; +export const RegistrationFailureReasonSchema = z.enum(REGISTRATION_FAILURE_REASONS); + export const OrganizationSchema = z.object({ id: UuidSchema, name: z.string().min(1).max(255), @@ -74,6 +89,18 @@ export const MemberSchema = z.object({ createdAt: TimestampSchema, }); +/** + * The rule for every password the platform *sets*. Upper-bounded to better-auth's own + * `maxPasswordLength` default (128), which it enforces itself but only after the fact - + * on sign-up that surfaces as a generic "Registration is unavailable", and on reset it + * burns a valid one-time code before rejecting. Bounding it here fails the caller with + * the real reason instead. + * + * Sign-in deliberately does NOT use this: capping the input cannot help (no longer + * password was ever storable) and would only narrow an existing contract. + */ +export const PasswordSchema = z.string().min(8).max(128); + const credentialsBase = z.object({ email: z.email(), password: z.string().min(8), @@ -89,6 +116,7 @@ export const LoginSecurityStateSchema = z.object({ }); export const RegisterInputSchema = credentialsBase.extend({ + password: PasswordSchema, username: UsernameSchema, acceptedTerms: z.literal(true), acceptedAge: z.literal(true), @@ -125,10 +153,7 @@ export const ResetPasswordInputSchema = z.object({ email: z.email(), otp: z.string().length(OTP_CODE_LENGTH), token: z.string().min(1).optional(), - // Upper-bounded to better-auth's own maxPasswordLength default (128): better-auth checks - // this AFTER consuming the OTP, so an over-length password would burn a valid one-time - // code and still get rejected - reject it here instead, before the OTP is ever spent. - newPassword: z.string().min(8).max(128), + newPassword: PasswordSchema, }); export const VerifyPasswordResetOtpInputSchema = ResetPasswordInputSchema.pick({ @@ -158,7 +183,7 @@ export const UpdateProfileInputSchema = z export const ChangePasswordInputSchema = z.object({ currentPassword: z.string().min(8), - newPassword: z.string().min(8), + newPassword: PasswordSchema, }); export const ChangeEmailInputSchema = z.object({ @@ -195,3 +220,4 @@ export type PhoneLoginRequestOutput = z.infer; export type PhoneLoginErrorReason = z.infer; export type PhoneLoginOtpInvalidReason = z.infer; +export type RegistrationFailureReason = z.infer; diff --git a/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts b/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts index e1ce1a43..cd417ba5 100644 --- a/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts +++ b/packages/core/src/pam/identity/__tests__/identity.registration-gates.int.test.ts @@ -69,8 +69,14 @@ afterAll(async () => { beforeEach(async () => { await redis.flush(); + events.emit.mockClear(); }); +const failureReasons = () => + events.emit.mock.calls + .filter(([topic]) => topic === 'identity.user.registration.failed') + .map(([, payload]) => (payload as { reason: string }).reason); + describe('IdentityService.register - availability gates', () => { it('rejects registration when the operator has not configured it', async () => { const svc = makeService({ platformConfig: undefined }); @@ -78,6 +84,7 @@ describe('IdentityService.register - availability gates', () => { await expect(svc.register(validInput(), {})).rejects.toMatchObject({ code: 'FORBIDDEN', }); + expect(failureReasons()).toEqual(['registration_disabled']); }); it('rejects registration when no player provisioning port is bound', async () => { @@ -96,6 +103,7 @@ describe('IdentityService.register - availability gates', () => { code: 'FORBIDDEN', }); expect(checkRegistration).toHaveBeenCalledWith('203.0.113.7'); + expect(failureReasons()).toEqual(['geo_blocked']); }); it('throttles registrations coming from one address, across different emails', async () => { @@ -112,5 +120,26 @@ describe('IdentityService.register - availability gates', () => { await expect(svc.register(validInput(), headers)).rejects.toMatchObject({ code: 'TOO_MANY_REQUESTS', }); + expect(failureReasons()).toContain('rate_limited'); + }); + + // Every attempt has to leave a record with its origin, not only the ones that produce + // an account. A rejected attempt is unauthenticated, so the address is the only subject + // there is to record it against. + it('records the origin of a rejected attempt, since there is no account to attribute it to', async () => { + const svc = makeService({ platformConfig: undefined }); + const input = validInput(); + + await svc + .register(input, { 'x-real-ip': '203.0.113.7', 'user-agent': 'Mozilla/5.0' }) + .catch(() => undefined); + + expect(events.emit).toHaveBeenCalledWith('identity.user.registration.failed', { + email: input.email, + username: input.username, + reason: 'registration_disabled', + ip: '203.0.113.7', + userAgent: 'Mozilla/5.0', + }); }); }); diff --git a/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts b/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts index 2cf78c0e..36711476 100644 --- a/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts +++ b/packages/core/src/pam/identity/adapters/__tests__/default-email-template-renderer.test.ts @@ -26,6 +26,20 @@ describe('DefaultEmailTemplateRenderer', () => { }); }); + it('renders existingAccountSignUp without naming it a password reset', () => { + const result = renderer.render( + 'existingAccountSignUp', + { otp: '123456', email: 'test@example.com' }, + 'en', + ); + + expect(result.subject).toBe('You already have an account'); + expect(result.body).toContain('123456'); + // The whole point of the separate key: a player who never asked to reset anything + // must not be handed a bare reset code with no explanation. + expect(result.body).toContain('no new account was created'); + }); + it('ignores locale - always English', () => { const en = renderer.render( 'resetPasswordOtp', diff --git a/packages/core/src/pam/identity/service/identity.service.ts b/packages/core/src/pam/identity/service/identity.service.ts index b28fa8d7..c15d3510 100644 --- a/packages/core/src/pam/identity/service/identity.service.ts +++ b/packages/core/src/pam/identity/service/identity.service.ts @@ -40,6 +40,7 @@ import type { IdentityServiceOptions, PlatformConfig, ClientMeta, + RegistrationFailureReason, GeoCheckCommands, PlayerProvisioning, } from '@openora/core/contracts'; @@ -299,6 +300,12 @@ export class IdentityService { private readonly playerProvisioning?: PlayerProvisioning; private readonly geoCheck?: GeoCheckCommands; private readonly cache?: CacheAdapter; + // Addresses whose reset code is being sent because a sign-up hit an existing account. + // Held only for the duration of that send: `requestPasswordResetEmailOTP` drives + // `sendVerificationOTP` -> `render` inside the same await, so the flag is read before + // the `finally` clears it. Not cache-backed - it never has to outlive the call or + // cross a process. + private readonly existingAccountSignUps = new Set(); constructor({ drizzle, @@ -332,13 +339,21 @@ export class IdentityService { getUserLanguage: (lookupEmail) => this.resolveUserLanguage(lookupEmail), requireEmailVerification: this.platformConfig?.registration?.requireEmailVerification ?? false, + isExistingAccountSignUp: (lookupEmail) => + this.existingAccountSignUps.has(lookupEmail.toLowerCase()), onExistingUserSignUp: async (existing) => { - const response = await this.api.requestPasswordResetEmailOTP({ - body: { email: existing.email }, - headers: new Headers(), - asResponse: true, - }); - await ensureOk(response); + const key = existing.email.toLowerCase(); + this.existingAccountSignUps.add(key); + try { + const response = await this.api.requestPasswordResetEmailOTP({ + body: { email: existing.email }, + headers: new Headers(), + asResponse: true, + }); + await ensureOk(response); + } finally { + this.existingAccountSignUps.delete(key); + } }, onPasswordReset: async (resetUser) => { this.events.emit('identity.password.reset', { @@ -398,20 +413,51 @@ export class IdentityService { }); } + /** + * Emitted on every registration attempt that produced no account. The audit trail is + * the one place that records what actually happened - the HTTP response deliberately + * does not, because a truthful answer on a known address is an enumeration oracle. + */ + private emitRegistrationFailed( + reason: RegistrationFailureReason, + input: RegisterInput, + { ip, userAgent }: ClientMeta, + ) { + this.events.emit('identity.user.registration.failed', { + email: input.email, + username: input.username, + reason, + ip, + userAgent, + }); + } + async register(input: RegisterInput, reqHeaders: NodeHeaders) { + // Read before the config guard: a rejected attempt is audited too, and an audit row + // with no origin is barely a record. + const { ip, userAgent } = extractClientMeta(reqHeaders); + const meta = { ip, userAgent }; const registration = this.platformConfig?.registration; const provisioning = this.playerProvisioning; if (!registration || !provisioning) { + this.emitRegistrationFailed('registration_disabled', input, meta); throw new ORPCError('FORBIDDEN', { message: 'Registration is unavailable' }); } - const { ip, userAgent } = extractClientMeta(reqHeaders); - await assertRateLimit( - this.limiter, - `register:${input.email.toLowerCase()}`, - REGISTER_RATE_LIMIT, - ); - await assertRateLimit(this.limiter, `register-ip:${ip ?? 'unknown'}`, REGISTER_RATE_LIMIT); + // `assertRateLimit` throws and is shared by a dozen call sites, so the emit wraps it + // here rather than moving into it. + try { + await assertRateLimit( + this.limiter, + `register:${input.email.toLowerCase()}`, + REGISTER_RATE_LIMIT, + ); + await assertRateLimit(this.limiter, `register-ip:${ip ?? 'unknown'}`, REGISTER_RATE_LIMIT); + } catch (err) { + this.emitRegistrationFailed('rate_limited', input, meta); + throw err; + } if (this.geoCheck && !(await this.geoCheck.checkRegistration(ip)).allowed) { + this.emitRegistrationFailed('geo_blocked', input, meta); throw new ORPCError('FORBIDDEN', { message: 'Registration is unavailable' }); } const headers = nodeHeadersToHeaders(reqHeaders); @@ -429,42 +475,65 @@ export class IdentityService { // The `lower(username)` unique index is the only arbiter - a pre-flight check // could not close the race anyway, so the taken handle is read off the failure. if (await this.findUserIdByUsername(input.username)) { + this.emitRegistrationFailed('username_taken', input, meta); throw new UsernameConflictError(); } + // `ensureOk` always throws on a non-ok response, so emitting first needs no catch. + this.emitRegistrationFailed('error', input, meta); await ensureOk(authResponse, { genericMessage: 'Registration is unavailable' }); } const body = (await authResponse.json()) as { user: BetterAuthUser }; // A known address gets an indistinguishable success response (and a reset mail) // rather than a new account, so only a genuinely new user is provisioned. - if ((await this.findUserIdByEmail(input.email)) === body.user.id) { - const { playerId } = await this.recordRegistrationConsent( + if ((await this.findUserIdByEmail(input.email)) !== body.user.id) { + // The known-address branch: no account was created, so the attempt failed even + // though the caller is told it succeeded. Only the audit trail says so. + this.emitRegistrationFailed('email_already_registered', input, meta); + return { status: 'check-email' as const }; + } + let consent: Awaited>; + try { + consent = await this.recordRegistrationConsent( provisioning, body.user.id, registration.termsVersion, { ip, userAgent }, ); - this.events.emit('identity.user.registered', { - userId: body.user.id, - playerId: playerId ?? (await this.identityReader.getPlayerIdByUserIdSafe(body.user.id)), - ip, - userAgent, - }); - // Sent here rather than by better-auth's sendOnSignUp hook: that hook also fires on - // the synthetic duplicate-email response, mailing a live code to an address whose - // owner never asked for it - and `verifyEmail` signs that code's bearer in. - // A mail failure is logged, never surfaced: the account exists either way and the - // player can ask for a new code, so failing the call here would only mislead them. - const otpResponse = await this.api.sendVerificationOTP({ - body: { email: input.email, type: 'email-verification' }, - headers, - asResponse: true, - }); - if (!otpResponse.ok) { - identityLogger.error( - { userId: body.user.id, status: otpResponse.status }, - 'verification code could not be sent - player must request a new one', - ); - } + } catch (err) { + this.emitRegistrationFailed('error', input, meta); + throw err; + } + const { playerId, consentStored } = consent; + this.events.emit('identity.user.registered', { + userId: body.user.id, + playerId: playerId ?? (await this.identityReader.getPlayerIdByUserIdSafe(body.user.id)), + // Only claimed when the consent row was actually written - a discarded capture + // must not leave an audit trail implying evidence that does not exist. + ...(consentStored + ? { + termsVersion: registration.termsVersion, + acceptedTerms: input.acceptedTerms, + acceptedAge: input.acceptedAge, + } + : {}), + ip, + userAgent, + }); + // Sent here rather than by better-auth's sendOnSignUp hook: that hook also fires on + // the synthetic duplicate-email response, mailing a live code to an address whose + // owner never asked for it - and `verifyEmail` signs that code's bearer in. + // A mail failure is logged, never surfaced: the account exists either way and the + // player can ask for a new code, so failing the call here would only mislead them. + const otpResponse = await this.api.sendVerificationOTP({ + body: { email: input.email, type: 'email-verification' }, + headers, + asResponse: true, + }); + if (!otpResponse.ok) { + identityLogger.error( + { userId: body.user.id, status: otpResponse.status }, + 'verification code could not be sent - player must request a new one', + ); } return { status: 'check-email' as const }; } @@ -525,7 +594,7 @@ export class IdentityService { 'registration consent not stored - player row already existed', ); } - return { playerId: outcome.playerId ?? null }; + return { playerId: outcome.playerId ?? null, consentStored: outcome.created }; } async usernameAvailable(username: string, reqHeaders: NodeHeaders) { diff --git a/packages/core/src/server/auth/__tests__/auth.test.ts b/packages/core/src/server/auth/__tests__/auth.test.ts index 298f87e3..1142b781 100644 --- a/packages/core/src/server/auth/__tests__/auth.test.ts +++ b/packages/core/src/server/auth/__tests__/auth.test.ts @@ -60,6 +60,36 @@ describe('createAuth', () => { }); }); + it('renders the existing-account template when the reset came from a sign-up', async () => { + // better-auth issues both through the same `forget-password` type, so without the + // predicate a duplicate sign-up mails a bare "Reset your password" to someone who + // never asked to reset anything. + const sendEmail = vi.fn().mockResolvedValue(undefined); + const templateRenderer = { + render: vi.fn().mockResolvedValue({ subject: 'Exists', body: 'Code: 123456' }), + }; + + createAuth({ + db: {} as never, + sendEmail, + templateRenderer, + isExistingAccountSignUp: (email) => email === 'test@example.com', + }); + + const emailOtpOpts = emailOTPMock.mock.calls[0][0]; + await emailOtpOpts.sendVerificationOTP({ + email: 'test@example.com', + otp: '123456', + type: 'forget-password', + }); + + expect(templateRenderer.render).toHaveBeenCalledWith( + 'existingAccountSignUp', + { otp: '123456', email: 'test@example.com' }, + 'en', + ); + }); + it('renders the verification code template for an email-verification OTP', async () => { const sendEmail = vi.fn().mockResolvedValue(undefined); const templateRenderer = { diff --git a/packages/core/src/server/auth/auth.ts b/packages/core/src/server/auth/auth.ts index 6370f17e..0eaf3242 100644 --- a/packages/core/src/server/auth/auth.ts +++ b/packages/core/src/server/auth/auth.ts @@ -34,6 +34,13 @@ export type AuthOptions = { */ requireEmailVerification?: boolean; onExistingUserSignUp?: (user: { id: string; email: string }) => Promise | void; + /** + * True while the reset code being sent was triggered by a sign-up on an address that + * already has an account, rather than by its owner asking to reset. better-auth issues + * both through the same `forget-password` OTP type, so `sendVerificationOTP` cannot tell + * them apart on its own - and the two need different copy. + */ + isExistingAccountSignUp?: (email: string) => boolean; cookieDomain?: string; }; @@ -133,10 +140,14 @@ export function createAuth(options: AuthOptions): BetterAuthType { return; } const locale = (await options.getUserLanguage?.(email)) ?? 'en'; + // Three explicit branches, not a computed key: `render` is generic over the key, + // so a union of keys will not narrow the data argument to a single payload type. const { subject, body } = type === 'email-verification' ? await templateRenderer.render('verifyEmail', { otp }, locale) - : await templateRenderer.render('resetPasswordOtp', { otp, email }, locale); + : options.isExistingAccountSignUp?.(email) + ? await templateRenderer.render('existingAccountSignUp', { otp, email }, locale) + : await templateRenderer.render('resetPasswordOtp', { otp, email }, locale); await sendEmail({ to: email, subject, body }); }, }), diff --git a/packages/testing/src/__tests__/registration.e2e.test.ts b/packages/testing/src/__tests__/registration.e2e.test.ts index 1d1dc1de..467cc089 100644 --- a/packages/testing/src/__tests__/registration.e2e.test.ts +++ b/packages/testing/src/__tests__/registration.e2e.test.ts @@ -1,9 +1,10 @@ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { randomUUID } from 'node:crypto'; import { eq } from 'drizzle-orm'; import { loadExtensions, DRIZZLE } from '@openora/core/server'; import { user, session } from '@openora/core/pam/schema/identity'; import { player } from '@openora/core/pam/schema/profile'; +import { auditLog } from '@openora/core/audit/schema'; import { verificationOtpFor, setupTestDb, @@ -216,12 +217,62 @@ describe('registration email verification', () => { .from(player) .where(eq(player.userId, (await userIdFor(email)) ?? '')); expect(players).toHaveLength(1); - expect(capturedEmailsFor(email).some((e) => /reset/i.test(e.subject))).toBe(true); + // The mail must say why it arrived. A bare "Reset your password" reaches someone who + // never asked to reset anything and explains nothing about the sign-up they just tried. + const notice = capturedEmailsFor(email).find( + (e) => e.subject === 'You already have an account', + ); + expect(notice).toBeDefined(); + expect(notice?.body).toMatch(/\b\d{6}\b/); // Exactly one verification code was ever mailed - the one the real owner's own // sign-up produced. A second would hand a stranger a code that signs them in. expect(capturedEmailsFor(email).filter((e) => /verify/i.test(e.subject))).toHaveLength(1); }); + // The audit subscription is fire-and-forget, so this is the only level that proves the + // whole chain: emit -> SUBSCRIBED_TOPICS -> mapper -> row. A unit test on the mapper + // alone would still pass with the topic missing from the subscription list. + it('writes a rejected attempt to the audit log with its origin and outcome', async () => { + const email = `reg-audit-${randomUUID()}@e2e.test`; + const username = `dup_${randomUUID().replaceAll('-', '').slice(0, 10)}`; + await registerPlayer(app, { email: `reg-audit-first-${randomUUID()}@e2e.test`, username }); + + // Called directly rather than through `submitRegistration`, which picks its own + // client IP - the origin is what this test is about. + const res = await app.app.request('/identity/register', { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'x-real-ip': '203.0.113.42', + 'user-agent': 'AuditProbe/1.0', + }, + body: JSON.stringify({ + email, + password: 'password123', + username: username.toUpperCase(), + acceptedTerms: true, + acceptedAge: true, + }), + }); + expect(res.status).toBe(409); + + await vi.waitFor(async () => { + const rows = await app.container + .get(DRIZZLE) + .db.select() + .from(auditLog) + .where(eq(auditLog.action, 'identity.user.registration.failed')); + const row = rows.find((r) => (r.after as { email?: string })?.email === email); + expect(row).toMatchObject({ + result: 'failure', + resourceType: 'registration', + ip: '203.0.113.42', + userAgent: 'AuditProbe/1.0', + }); + expect(row?.after).toMatchObject({ reason: 'username_taken' }); + }); + }); + it('records the terms and age acceptance on the player row at registration', async () => { const email = `reg-consent-${randomUUID()}@e2e.test`; const userId = await registerPlayer(app, { email }); diff --git a/packages/testing/src/register.ts b/packages/testing/src/register.ts index bc5ff1e2..0d85f8ca 100644 --- a/packages/testing/src/register.ts +++ b/packages/testing/src/register.ts @@ -27,9 +27,13 @@ export async function submitRegistration(app: TestApp, input: RegisterPlayerInpu }); } -/** The 6-digit code in the most recent verification email. */ +/** + * The 6-digit code in the most recent verification email. Filtered by subject rather than + * taking the newest mail outright: a sign-up on an address that already has an account + * also mails a six-digit code, and picking that one up would silently test the wrong flow. + */ export function verificationOtpFor(email: string): string { - const [sent] = capturedEmailsFor(email); + const [sent] = capturedEmailsFor(email).filter((mail) => /verify/i.test(mail.subject)); if (!sent) { throw new Error(`no verification email captured for ${email}`); }