Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
554 changes: 277 additions & 277 deletions .beads/issues.jsonl

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ EPDS_LINK_BASE_URL=https://auth.pds.example/auth/verify
SESSION_EXPIRES_IN=604800
SESSION_UPDATE_AGE=86400

# OTP code length — number of characters in the email verification code (default: 8)
# Must be between 4 and 12 characters. Applies to login, recovery, and account settings OTP flows.
# OTP_LENGTH=8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kzoeps Please can you check if you need to add these two vars to scripts/setup.sh?


# OTP character set — 'numeric' for digits only (0-9), 'alphanumeric' for uppercase letters + digits (A-Z, 0-9)
# Default: numeric. Alphanumeric codes have higher entropy but require text input instead of numeric keyboard.
# OTP_CHARSET=numeric

# GOOGLE_CLIENT_ID=
# GOOGLE_CLIENT_SECRET=
# GITHUB_CLIENT_ID=
Expand Down
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ import { AuthServiceContext } from './context.js'
- All epds-callback redirects must be HMAC-SHA256 signed using
`signCallback()` / `verifyCallback()` from `@certified-app/shared`.
- Use `timingSafeEqual()` for all secret/token comparisons.
- OTP codes: 8-digit, single-use, managed by better-auth.
- OTP codes: configurable length (4–12, default 8, via `OTP_LENGTH`) and charset
(`numeric` or `alphanumeric`, default `numeric`, via `OTP_CHARSET`), single-use,
managed by better-auth. Expiry is hardcoded at 600 s with 5 allowed attempts.
- Internal service-to-service calls use `x-internal-secret` header.

## Task Tracking
Expand Down
8 changes: 8 additions & 0 deletions packages/auth-service/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ EPDS_LINK_BASE_URL=https://auth.pds.example/auth/verify
SESSION_EXPIRES_IN=604800
SESSION_UPDATE_AGE=86400

# OTP code length — number of characters in the email verification code (default: 8)
# Must be between 4 and 12. Applies to login, recovery, and account settings OTP flows.
# OTP_LENGTH=8

# OTP character set — 'numeric' for digits only (0-9), 'alphanumeric' for uppercase letters + digits (A-Z, 0-9)
# Default: numeric. Alphanumeric codes have higher entropy but require text input instead of numeric keyboard.
# OTP_CHARSET=numeric

# Social login providers (both ID and SECRET must be set to enable)
# Google: https://console.cloud.google.com/
# GOOGLE_CLIENT_ID=
Expand Down
2 changes: 2 additions & 0 deletions packages/auth-service/src/__tests__/consent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ function makeMockContext(db: EpdsDb): AuthServiceContext {
fromName: 'Test PDS',
},
dbLocation: ':memory:',
otpLength: 8,
otpCharset: 'numeric',
}

return {
Expand Down
3 changes: 2 additions & 1 deletion packages/auth-service/src/__tests__/email-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
import { describe, it, expect, vi, afterEach } from 'vitest'
import { EmailSender } from '../email/sender.js'
import { formatOtpHtmlGrouped } from '@certified-app/shared'
import type { EmailConfig } from '@certified-app/shared'

const originalFetch = globalThis.fetch
Expand Down Expand Up @@ -173,7 +174,7 @@ describe('EmailSender', () => {
// Default template uses pdsName in subject, not client name
expect(mailOpts.subject).toContain('Test PDS')
// Default template contains the sign-in code block
expect(mailOpts.html).toContain('44444444')
expect(mailOpts.html).toContain(formatOtpHtmlGrouped('44444444'))
// Should NOT contain the broken template content
expect(mailOpts.html).not.toContain('broken-template')
// From name should be the default config, not the client name
Expand Down
65 changes: 65 additions & 0 deletions packages/auth-service/src/__tests__/otp-input.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* Tests for buildOtpInputProps — derives HTML input attributes from OTP config.
*
* Covers:
* 1. Correct pattern and placeholder for numeric charset
* 2. Correct pattern and placeholder for alphanumeric charset
* 3. Pattern and placeholder length match the requested otpLength
* 4. Numeric pattern rejects letters
* 5. Alphanumeric pattern accepts both letters and digits
*/
import { describe, it, expect } from 'vitest'
import { buildOtpInputProps } from '../otp-input.js'

describe('Recovery flow: OTP input props', () => {
it('numeric charset produces digit-only pattern and zero placeholder', () => {
const props = buildOtpInputProps(8, 'numeric')
expect(props.pattern).toBe('[0-9]{8}')
expect(props.placeholder).toBe('00000000')
expect(props.inputmode).toBe('numeric')
expect(props.autocapitalize).toBe('off')
})

it('alphanumeric charset produces alphanumeric pattern and X placeholder', () => {
const props = buildOtpInputProps(8, 'alphanumeric')
expect(props.pattern).toBe('[A-Z0-9]{8}')
expect(props.placeholder).toBe('XXXXXXXX')
expect(props.inputmode).toBe('text')
expect(props.autocapitalize).toBe('characters')
})

it('pattern and placeholder length match otpLength', () => {
const numeric = buildOtpInputProps(6, 'numeric')
expect(numeric.pattern).toBe('[0-9]{6}')
expect(numeric.placeholder).toHaveLength(6)
const numericRe = new RegExp(`^${numeric.pattern}$`)
expect(numericRe.test('123456')).toBe(true)
expect(numericRe.test('12345')).toBe(false) // too short
expect(numericRe.test('1234567')).toBe(false) // too long

const alpha = buildOtpInputProps(6, 'alphanumeric')
expect(alpha.pattern).toBe('[A-Z0-9]{6}')
expect(alpha.placeholder).toHaveLength(6)
const alphaRe = new RegExp(`^${alpha.pattern}$`)
expect(alphaRe.test('A1B2C3')).toBe(true)
expect(alphaRe.test('A1B2C')).toBe(false) // too short
expect(alphaRe.test('A1B2C3D')).toBe(false) // too long
})

it('numeric pattern does not accept letters', () => {
const { pattern } = buildOtpInputProps(8, 'numeric')
const re = new RegExp(`^${pattern}$`)
expect(re.test('12345678')).toBe(true)
expect(re.test('1234567A')).toBe(false)
})

it('alphanumeric pattern accepts both letters and digits', () => {
const { pattern } = buildOtpInputProps(8, 'alphanumeric')
const re = new RegExp(`^${pattern}$`)
expect(re.test('A1B2C3D4')).toBe(true)
expect(re.test('12345678')).toBe(true)
expect(re.test('ABCDEFGH')).toBe(true)
expect(re.test('abcdefgh')).toBe(false) // lowercase rejected
expect(re.test('A1B2C3D')).toBe(false) // one short
})
})
24 changes: 0 additions & 24 deletions packages/auth-service/src/__tests__/recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,30 +119,6 @@ describe('Recovery flow: auth_flow creation for request_uri threading', () => {
})
})

describe('Recovery flow: OTP pattern (8 digits)', () => {
it('OTP sent by better-auth is 8 digits (configured in emailOTP plugin)', () => {
// Verify the configured OTP length matches what users expect
const OTP_LENGTH = 8
const pattern = new RegExp(`^[0-9]{${OTP_LENGTH}}$`)

// Simulate an 8-digit OTP
const otp = '12345678'
expect(pattern.test(otp)).toBe(true)

// 6 digits should not match (old format)
expect(pattern.test('123456')).toBe(false)
})

it('OTP entry form uses maxlength=8 and pattern [0-9]{8}', () => {
// This is a documentation test confirming the UI constraints
// match the better-auth configuration (otpLength: 8)
const maxlength = 8
const pattern = '[0-9]{8}'
expect(maxlength).toBe(8)
expect(pattern).toContain('8')
})
})

describe('Recovery flow: /auth/complete bridge integration', () => {
let db: EpdsDb
let dbPath: string
Expand Down
39 changes: 31 additions & 8 deletions packages/auth-service/src/better-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@
* The instance is mounted at /api/auth/* alongside the existing custom routes.
* No existing behavior is changed — this is a foundation-only step.
*/
import Database from 'better-sqlite3'
import type { EpdsDb } from '@certified-app/shared'
import { createLogger } from '@certified-app/shared'
import { betterAuth } from 'better-auth'
import { generateRandomString } from 'better-auth/crypto'
import { getMigrations } from 'better-auth/db'
import { emailOTP } from 'better-auth/plugins'
import { createLogger } from '@certified-app/shared'
import type { EpdsDb } from '@certified-app/shared'
import Database from 'better-sqlite3'
import type { EmailSender } from './email/sender.js'
import { getDidByEmail } from './lib/get-did-by-email.js'

export type BetterAuthInstance = ReturnType<typeof createBetterAuth>

const logger = createLogger('auth:better-auth')

const AUTH_FLOW_COOKIE = 'epds_auth_flow'
Expand Down Expand Up @@ -68,10 +71,16 @@ export let socialProviders: Record<
* Run better-auth migrations at startup — creates user, session, account,
* and verification tables if they don't exist yet. Safe to call on every
* startup (no-ops when tables are already present).
*
* The otpLength parameter is accepted for API consistency but does not affect
* the schema — better-auth stores OTP codes as hashed strings regardless of
* length, so the column definition is the same for any valid otpLength.
*/
export async function runBetterAuthMigrations(
dbLocation: string,
authHostname: string,
otpLength: number,
otpCharset: 'numeric' | 'alphanumeric' = 'numeric',
): Promise<void> {
Comment thread
Kzoeps marked this conversation as resolved.
const betterAuthDb = new Database(dbLocation)
const tempAuth = betterAuth({
Expand All @@ -81,10 +90,13 @@ export async function runBetterAuthMigrations(
basePath: '/api/auth',
plugins: [
emailOTP({
otpLength: 8,
otpLength,
expiresIn: 600,
allowedAttempts: 5,
storeOTP: 'hashed',
...(otpCharset === 'alphanumeric'
? { generateOTP: () => generateRandomString(otpLength, 'A-Z', '0-9') }
: {}),
async sendVerificationOTP() {},
}),
],
Expand All @@ -108,8 +120,12 @@ export async function runBetterAuthMigrations(
betterAuthDb.close()
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function createBetterAuth(emailSender: EmailSender, db: EpdsDb): any {
export function createBetterAuth(
emailSender: EmailSender,
db: EpdsDb,
otpLength: number,
otpCharset: 'numeric' | 'alphanumeric' = 'numeric',
) {
const dbLocation = process.env.DB_LOCATION ?? './data/epds.sqlite'
const authHostname = process.env.AUTH_HOSTNAME ?? 'auth.localhost'
const pdsName = process.env.SMTP_FROM_NAME ?? 'ePDS'
Expand All @@ -133,7 +149,11 @@ export function createBetterAuth(emailSender: EmailSender, db: EpdsDb): any {
// Use AUTH_SESSION_SECRET so better-auth doesn't fall back to its
// default secret (which throws in production).
secret: process.env.AUTH_SESSION_SECRET,
database: betterAuthDb,
// TS4058: BetterSqlite3.Database leaks into the inferred return type when
// passed directly; casting to `any` breaks the inference chain so declaration
// emit succeeds without casting the entire createBetterAuth return value.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
database: betterAuthDb as any,
baseURL: `https://${authHostname}`,
basePath: '/api/auth',

Expand All @@ -146,10 +166,13 @@ export function createBetterAuth(emailSender: EmailSender, db: EpdsDb): any {

plugins: [
emailOTP({
otpLength: 8,
otpLength,
expiresIn: 600,
allowedAttempts: 5,
storeOTP: 'hashed',
...(otpCharset === 'alphanumeric'
? { generateOTP: () => generateRandomString(otpLength, 'A-Z', '0-9') }
Comment thread
aspiers marked this conversation as resolved.
: {}),

/**
* Wire OTP sending to the existing EmailSender.
Expand Down
2 changes: 2 additions & 0 deletions packages/auth-service/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export interface AuthServiceConfig {
fromName: string
}
dbLocation: string
otpLength: number
otpCharset: 'numeric' | 'alphanumeric'
}

const logger = createLogger('auth-service')
Expand Down
18 changes: 11 additions & 7 deletions packages/auth-service/src/email/sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import * as nodemailer from 'nodemailer'
import { createLogger } from '@certified-app/shared'
import type { Transporter } from 'nodemailer'
import type { EmailConfig } from '@certified-app/shared'
import { escapeHtml } from '@certified-app/shared'
import {
escapeHtml,
formatOtpPlain,
formatOtpHtmlGrouped,
} from '@certified-app/shared'
import { resolveClientMetadata } from '../lib/client-metadata.js'

const logger = createLogger('auth:email')
Expand Down Expand Up @@ -215,9 +219,9 @@ export class EmailSender {
app_name: appName,
})
} else if (isNewUser) {
subject = `${code} — Welcome to ${appName}`
subject = `${formatOtpPlain(code)} — Welcome to ${appName}`
} else {
subject = `${code} is your sign-in code for ${appName}`
subject = `${formatOtpPlain(code)} is your sign-in code for ${appName}`
}

const fromName = metadata.client_name || this.config.fromName
Expand Down Expand Up @@ -266,7 +270,7 @@ export class EmailSender {
}): Promise<void> {
const { to, code, clientAppName, pdsName, pdsDomain } = opts

const subject = `${code} is your sign-in code for ${pdsName}`
const subject = `${formatOtpPlain(code)} is your sign-in code for ${pdsName}`

const text = [
`Your sign-in code for ${clientAppName}:`,
Expand All @@ -288,7 +292,7 @@ export class EmailSender {
<p>Your sign-in code for <strong>${escapeHtml(clientAppName)}</strong>:</p>
<p style="margin: 30px 0; text-align: center;">
<span style="font-size: 32px; font-family: 'SF Mono', 'Menlo', 'Consolas', monospace; letter-spacing: 6px; background: #f5f5f5; padding: 16px 24px; border-radius: 8px; display: inline-block; font-weight: 600; color: #0f1828;">
${escapeHtml(code)}
${formatOtpHtmlGrouped(code)}
</span>
</p>
<p style="color: #666; font-size: 14px;">This code expires in 10 minutes.</p>
Expand All @@ -315,7 +319,7 @@ export class EmailSender {
}): Promise<void> {
const { to, code, pdsName, pdsDomain } = opts

const subject = `${code} — Welcome to ${pdsName}`
const subject = `${formatOtpPlain(code)} — Welcome to ${pdsName}`

const text = [
`Welcome to ${pdsName}!`,
Expand All @@ -342,7 +346,7 @@ export class EmailSender {
<p>Enter this code to confirm your email and create your account:</p>
<p style="margin: 30px 0; text-align: center;">
<span style="font-size: 32px; font-family: 'SF Mono', 'Menlo', 'Consolas', monospace; letter-spacing: 6px; background: #f5f5f5; padding: 16px 24px; border-radius: 8px; display: inline-block; font-weight: 600; color: #0f1828;">
${escapeHtml(code)}
${formatOtpHtmlGrouped(code)}
</span>
</p>
<p style="color: #666; font-size: 14px;">This code expires in 10 minutes.</p>
Expand Down
37 changes: 34 additions & 3 deletions packages/auth-service/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ export function createAuthService(config: AuthServiceConfig): {

// Mount better-auth BEFORE express.json() so it can parse its own request bodies.
// All better-auth endpoints live under /api/auth/*.
const betterAuthInstance = createBetterAuth(ctx.emailSender, ctx.db)
const betterAuthInstance = createBetterAuth(
ctx.emailSender,
ctx.db,
config.otpLength,
config.otpCharset,
)
app.all('/api/auth/*', toNodeHandler(betterAuthInstance))

// Middleware
Expand Down Expand Up @@ -75,7 +80,7 @@ export function createAuthService(config: AuthServiceConfig): {
app.use(createLoginPageRouter(ctx))
app.use(createConsentRouter(ctx))
app.use(createRecoveryRouter(ctx, betterAuthInstance))
app.use(createAccountLoginRouter(betterAuthInstance))
app.use(createAccountLoginRouter(betterAuthInstance, ctx))
app.use(createAccountSettingsRouter(ctx, betterAuthInstance))
app.use(createCompleteRouter(ctx, betterAuthInstance))
app.use(createChooseHandleRouter(ctx, betterAuthInstance))
Expand Down Expand Up @@ -132,9 +137,35 @@ async function main() {
fromName: process.env.SMTP_FROM_NAME || 'ePDS',
},
dbLocation: process.env.DB_LOCATION || './data/epds.sqlite',
otpLength: Number(process.env.OTP_LENGTH ?? '8'),
otpCharset: (process.env.OTP_CHARSET || 'numeric') as
| 'numeric'
| 'alphanumeric',
}

await runBetterAuthMigrations(config.dbLocation, config.hostname)
if (
isNaN(config.otpLength) ||
config.otpLength < 4 ||
config.otpLength > 12
) {
throw new Error(
`Invalid OTP_LENGTH: must be between 4 and 12, got "${process.env.OTP_LENGTH}"`,
)
}

const validCharsets = ['numeric', 'alphanumeric']
if (!validCharsets.includes(config.otpCharset)) {
throw new Error(
`Invalid OTP_CHARSET: must be 'numeric' or 'alphanumeric', got "${process.env.OTP_CHARSET}"`,
)
}
Comment on lines +146 to +161

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general please always look for opportunities to keep functions small and extract logic out into smaller chunks. Keeping everything smaller yields an enormous boost in code quality / maintainability / legibility (even for agents). For example these bits could be refactored as getValidOtpConfig() or something like that.

Feel free to adopt https://github.com/aspiers/ai-config/blob/main/.agents/skills/code-refactoring-small/SKILL.md


await runBetterAuthMigrations(
config.dbLocation,
config.hostname,
config.otpLength,
config.otpCharset,
)

const { app, ctx } = createAuthService(config)

Expand Down
Loading
Loading