Skip to content

Commit 2314eb9

Browse files
authored
Merge pull request #1479 from Favouratambi/fix/db-backed-confirmation-tokens
fix: back confirmation tokens with database instead of process-local Map
2 parents 5663acb + 3db9cea commit 2314eb9

4 files changed

Lines changed: 1044 additions & 37 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Migration: confirmation_tokens
3+
*
4+
* Stores dual-control confirmation tokens for destructive admin actions.
5+
* Previously stored in a process-local Map, which broke multi-instance deployments.
6+
* See issue #1033.
7+
*/
8+
exports.up = async function up(knex) {
9+
await knex.schema.createTable('confirmation_tokens', (table) => {
10+
table.uuid('id').primary().defaultTo(knex.raw('gen_random_uuid()'))
11+
table.string('token_id', 36).notNullable().unique()
12+
table.string('user_id', 255).notNullable()
13+
table.string('action', 255).notNullable()
14+
table.string('scope', 255).nullable()
15+
table.timestamp('expires_at', { useTz: true }).notNullable()
16+
table.boolean('used').notNullable().defaultTo(false)
17+
table.boolean('dual_control_required').notNullable().defaultTo(false)
18+
table.string('approved_by', 255).nullable()
19+
table.timestamp('approved_at', { useTz: true }).nullable()
20+
table.timestamp('created_at', { useTz: true }).notNullable().defaultTo(knex.fn.now())
21+
22+
table.index(['token_id'], 'idx_confirmation_tokens_token_id')
23+
table.index(['user_id'], 'idx_confirmation_tokens_user_id')
24+
table.index(['expires_at'], 'idx_confirmation_tokens_expires_at')
25+
})
26+
}
27+
28+
exports.down = async function down(knex) {
29+
await knex.schema.dropTableIfExists('confirmation_tokens')
30+
}

src/middleware/confirmationToken.ts

Lines changed: 80 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { NextFunction, Request, Response } from 'express'
22
import { randomUUID } from 'node:crypto'
3+
import db from '../db/index.js'
34

45
// Configurable via env: comma-separated list of actions requiring a second-admin approval
56
export const DUAL_CONTROL_ACTIONS = new Set(
@@ -33,71 +34,108 @@ export interface ConfirmationTokenEntry {
3334
createdAt: number
3435
}
3536

36-
// In-memory store — same pattern as STEP_UP_NONCES in auth.service.ts
37-
const CONFIRMATION_TOKENS = new Map<string, ConfirmationTokenEntry>()
37+
function rowToEntry(row: any): ConfirmationTokenEntry {
38+
return {
39+
tokenId: row.token_id,
40+
userId: row.user_id,
41+
action: row.action,
42+
scope: row.scope ?? undefined,
43+
expiresAt: new Date(row.expires_at).getTime(),
44+
used: row.used,
45+
dualControlRequired: row.dual_control_required,
46+
approvedBy: row.approved_by ?? undefined,
47+
approvedAt: row.approved_at ? new Date(row.approved_at).getTime() : undefined,
48+
createdAt: new Date(row.created_at).getTime(),
49+
}
50+
}
3851

39-
// Test helper — mirrors clearProcessedOverrides from admin.ts
40-
export const clearConfirmationTokens = (): void => {
41-
CONFIRMATION_TOKENS.clear()
52+
export const clearConfirmationTokens = async (): Promise<void> => {
53+
await db('confirmation_tokens').delete()
4254
}
4355

4456
export const isDualControlRequired = (action: string): boolean =>
4557
DUAL_CONTROL_ACTIONS.has(action)
4658

47-
export const issueConfirmationToken = (
59+
export const issueConfirmationToken = async (
4860
userId: string,
4961
action: string,
5062
scope?: string,
51-
): ConfirmationTokenEntry => {
63+
): Promise<ConfirmationTokenEntry> => {
5264
const dualControlRequired = isDualControlRequired(action)
5365
const ttlMs = dualControlRequired ? DUAL_CONTROL_TTL_MS : SINGLE_CONTROL_TTL_MS
54-
const entry: ConfirmationTokenEntry = {
55-
tokenId: randomUUID(),
66+
const tokenId = randomUUID()
67+
const expiresAt = new Date(Date.now() + ttlMs)
68+
69+
await db('confirmation_tokens').insert({
70+
token_id: tokenId,
71+
user_id: userId,
72+
action,
73+
scope: scope ?? null,
74+
expires_at: expiresAt.toISOString(),
75+
used: false,
76+
dual_control_required: dualControlRequired,
77+
})
78+
79+
return {
80+
tokenId,
5681
userId,
5782
action,
5883
scope,
59-
expiresAt: Date.now() + ttlMs,
84+
expiresAt: expiresAt.getTime(),
6085
used: false,
6186
dualControlRequired,
6287
createdAt: Date.now(),
6388
}
64-
CONFIRMATION_TOKENS.set(entry.tokenId, entry)
65-
return entry
6689
}
6790

6891
export type ApproveResult =
6992
| { ok: true; entry: ConfirmationTokenEntry }
7093
| { ok: false; reason: string }
7194

72-
export const approveConfirmationToken = (tokenId: string, approverId: string): ApproveResult => {
73-
const entry = CONFIRMATION_TOKENS.get(tokenId)
74-
if (!entry) return { ok: false, reason: 'token_not_found' }
95+
export const approveConfirmationToken = async (tokenId: string, approverId: string): Promise<ApproveResult> => {
96+
const row = await db('confirmation_tokens').where({ token_id: tokenId }).first()
97+
if (!row) return { ok: false, reason: 'token_not_found' }
98+
99+
const entry = rowToEntry(row)
75100
if (entry.used) return { ok: false, reason: 'token_already_used' }
76101
if (entry.expiresAt < Date.now()) return { ok: false, reason: 'token_expired' }
77102
if (!entry.dualControlRequired) return { ok: false, reason: 'action_does_not_require_approval' }
78103
if (entry.approvedBy) return { ok: false, reason: 'already_approved' }
79-
// Self-approval is prohibited — the approver must be a different admin
80104
if (entry.userId === approverId) return { ok: false, reason: 'self_approval_not_allowed' }
105+
106+
const now = new Date()
107+
await db('confirmation_tokens')
108+
.where({ token_id: tokenId })
109+
.update({
110+
approved_by: approverId,
111+
approved_at: now.toISOString(),
112+
})
113+
81114
entry.approvedBy = approverId
82-
entry.approvedAt = Date.now()
115+
entry.approvedAt = now.getTime()
83116
return { ok: true, entry }
84117
}
85118

86-
export const validateConfirmationToken = (
119+
export const validateConfirmationToken = async (
87120
tokenId: string,
88121
userId: string,
89122
action: string,
90-
): ConfirmationTokenEntry | null => {
91-
const entry = CONFIRMATION_TOKENS.get(tokenId)
92-
if (!entry) return null
123+
): Promise<ConfirmationTokenEntry | null> => {
124+
const row = await db('confirmation_tokens').where({ token_id: tokenId }).first()
125+
if (!row) return null
126+
127+
const entry = rowToEntry(row)
93128
if (entry.used) return null
94129
if (entry.expiresAt < Date.now()) return null
95130
if (entry.userId !== userId) return null
96131
if (entry.action !== action) return null
97-
// Dual-control tokens must be approved before use
98132
if (entry.dualControlRequired && !entry.approvedBy) return null
133+
134+
await db('confirmation_tokens')
135+
.where({ token_id: tokenId })
136+
.update({ used: true })
137+
99138
entry.used = true
100-
CONFIRMATION_TOKENS.delete(tokenId)
101139
return entry
102140
}
103141

@@ -121,17 +159,24 @@ export const requireConfirmationToken =
121159
return
122160
}
123161

124-
const entry = validateConfirmationToken(tokenId, userId, action)
125-
if (!entry) {
126-
res.status(403).json({
127-
error: 'Invalid, expired, wrong-scope, or already-used confirmation token',
128-
confirmationRequired: true,
129-
action,
130-
prepareUrl: '/api/admin/confirm/prepare',
131-
})
132-
return
133-
}
162+
// Await is handled by wrapping the middleware
163+
validateConfirmationToken(tokenId, userId, action)
164+
.then((entry) => {
165+
if (!entry) {
166+
res.status(403).json({
167+
error: 'Invalid, expired, wrong-scope, or already-used confirmation token',
168+
confirmationRequired: true,
169+
action,
170+
prepareUrl: '/api/admin/confirm/prepare',
171+
})
172+
return
173+
}
134174

135-
;(req as any).confirmationTokenEntry = entry
136-
next()
175+
;(req as any).confirmationTokenEntry = entry
176+
next()
177+
})
178+
.catch((err) => {
179+
console.error('Confirmation token validation error:', err)
180+
res.status(500).json({ error: 'Internal server error during token validation' })
181+
})
137182
}

src/routes/admin.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ adminRouter.post('/confirm/prepare', async (req: Request, res: Response) => {
174174
return
175175
}
176176

177-
const entry = issueConfirmationToken(req.user!.userId, action, scope ?? undefined)
177+
const entry = await issueConfirmationToken(req.user!.userId, action, scope ?? undefined)
178178

179179
await createAuditLog({
180180
actor_user_id: req.user!.userId,
@@ -208,7 +208,7 @@ adminRouter.post('/confirm/prepare', async (req: Request, res: Response) => {
208208
*/
209209
adminRouter.post('/confirm/approve/:tokenId', async (req: Request, res: Response) => {
210210
const { tokenId } = req.params
211-
const result = approveConfirmationToken(tokenId, req.user!.userId)
211+
const result = await approveConfirmationToken(tokenId, req.user!.userId)
212212

213213
if (!result.ok) {
214214
const status = result.reason === 'token_not_found' ? 404 : 409

0 commit comments

Comments
 (0)