diff --git a/docs/verifier-queue-authorization.md b/docs/verifier-queue-authorization.md new file mode 100644 index 00000000..5225d3df --- /dev/null +++ b/docs/verifier-queue-authorization.md @@ -0,0 +1,83 @@ +# Verifier queue authorization + +Verifier actions are authorization-sensitive state transitions. The actor in +the signed authentication principal is the only identity used for the check; +wallet headers, request body fields, and role-shaped client headers are not +trusted as assignment evidence. + +## Decision table + +| Action | Queue shape | Required assignment | Result when stale or missing | +| --- | --- | --- | --- | +| `verify` | single verifier | exact current `verifierId` | `403`, no mutation | +| `validate` | single verifier | exact current `verifierId` | `403`, no mutation | +| `approve` | single verifier | exact current `verifierId` | `403`, no mutation | +| `approve` | M-of-N | approved verifier pool | `403` for non-approved/suspended actor | + +The M-of-N exception is intentional. `verifierId` identifies the queue +coordinator, while individual approvals come from the configured approved +verifier pool. M-of-N does not broaden `verify` or single-verifier +`validate`. + +## Authorization boundary + +`authorizeVerifierQueueAction` is a pure guard in +`src/services/verifierTransitions.ts`. It is called before mutation and +before any external side effect. It distinguishes these stable failure +codes: + +- `ACTOR_REQUIRED`: the request has no authenticated actor; +- `UNASSIGNED_QUEUE_ITEM`: no current verifier owns the item; +- `STALE_ASSIGNMENT`: the item was reassigned after the actor received it; +- `ALREADY_SETTLED`: a terminal item is being verified or validated again; +- `INVALID_TRANSITION`: the requested lifecycle edge is not monotonic. + +The guard does not accept a proposed assignee from the request. A reassignment +therefore invalidates an old queue response immediately, even if that old +response is replayed with a fresh HTTP request. + +## Atomicity and auditability + +Routes perform the guard before changing the compatibility in-memory milestone +record or inserting an approval. The lifecycle service performs the same +guard when an actor is supplied, so direct service callers cannot skip the +authorization boundary. Failed guards do not increment the lifecycle +sequence, set `verified`, insert an approval, or emit a success event. + +Every successful state-changing path emits one ordered milestone event: + +1. `milestone.verified` for the direct verification endpoint; +2. `milestone.validated` for evidence-backed validation; +3. `milestone.approval.recorded` for each approval vote; +4. `milestone.settled` when the approval threshold settles the milestone; +5. `milestone.lifecycle.*` for explicit lifecycle transitions. + +The event ledger is append-only for the process lifetime, has a monotonically +increasing per-milestone sequence, and is reset only by test fixtures. This +makes the order of accepted state changes observable without exposing +evidence hashes or other sensitive values. + +## Operational trade-offs + +The compatibility milestone service remains synchronous because existing +clients and tests depend on its in-memory representation. The authorization +guard is deliberately synchronous and side-effect-free for that reason. The +database-backed approval insert still uses its unique constraint as the final +concurrency barrier; the route-level guard supplies the authorization barrier +before that insert. A future database-native queue consumer should call the +same guard after selecting the row with a transaction lock and include its +audit insert in the transaction. + +## Test matrix + +The regression suite covers: + +- current assignee success for all single-verifier actions; +- missing actor and unassigned queue item failures; +- stale actor failures after reassignment; +- M-of-N approval-pool behavior; +- settled-item rejection; +- all permitted lifecycle edges; +- backward, self, and skipped lifecycle transitions; +- stable machine-readable error details. + diff --git a/src/routes/milestones.ts b/src/routes/milestones.ts index b53bc4ea..09508958 100644 --- a/src/routes/milestones.ts +++ b/src/routes/milestones.ts @@ -14,6 +14,8 @@ import { getMilestoneById, verifyMilestone, validateMilestone, + validateMilestoneMultiVerifier, + addMilestoneEvent, allMilestonesVerified, allMilestonesMetThreshold, } from '../services/milestones.js' @@ -42,6 +44,7 @@ import { DuplicateVerifierVoteError, getVerifierProfile, } from '../services/verifiers.js' +import { VerifierAuthorizationError } from '../services/verifierTransitions.js' const milestoneRepo = new MilestoneRepositoryEnhanced(db) @@ -308,7 +311,17 @@ milestonesRouter.patch('/:id/verify', authenticate, requireWalletIdentity, requi throw AppError.notFound('Milestone not found') } - const verified = verifyMilestone(id) + let verified + try { + verified = verifyMilestone(id, actorUserId) + } catch (error) { + if (error instanceof VerifierAuthorizationError) { + throw error.code === 'ALREADY_SETTLED' + ? AppError.conflict(error.message) + : AppError.forbidden(error.message) + } + throw error + } if (!verified) { throw AppError.notFound('Milestone not found') } @@ -370,7 +383,10 @@ milestonesRouter.post('/:id/validate', authenticate, requireWalletIdentity, requ if (result.error === 'Milestone already validated') { throw AppError.conflict('Milestone already validated') } - if (result.error === 'Unauthorized: only assigned verifier can validate') { + if (/already settled|already validated/i.test(result.error ?? '')) { + throw AppError.conflict(result.error!) + } + if (/assigned verifier|not assigned/i.test(result.error ?? '')) { throw AppError.forbidden('Unauthorized: only assigned verifier can validate') } throw AppError.badRequest(result.error!) @@ -434,6 +450,15 @@ milestonesRouter.post('/:id/approve', authenticate, requireWalletIdentity, requi throw AppError.forbidden('Only approved verifiers may cast milestone approvals') } + const authorization = validateMilestoneMultiVerifier( + id, + verifierUserId, + verifier?.status, + ) + if (!authorization.success) { + throw AppError.forbidden(authorization.error ?? 'Verifier is not authorized for this queue item') + } + // Check if verifier has already voted (duplicate vote prevention) const hasVoted = await hasVerifierVoted(id, verifierUserId) @@ -453,6 +478,14 @@ milestonesRouter.post('/:id/approve', authenticate, requireWalletIdentity, requi // Record the approval const approval = await recordMilestoneApproval(id, verifierUserId, approvalStatus as any) + addMilestoneEvent({ + userId: verifierUserId, + vaultId, + name: 'milestone.approval.recorded', + status: 'success', + timestamp: new Date().toISOString(), + }) + // Get updated approval progress const approvalProgress = await getMilestoneApprovalProgress(id, approvalThreshold, totalVerifiers) @@ -466,6 +499,14 @@ milestonesRouter.post('/:id/approve', authenticate, requireWalletIdentity, requi milestone.verifiedAt = new Date().toISOString() milestone.verifiedBy = verifierUserId + addMilestoneEvent({ + userId: verifierUserId, + vaultId, + name: 'milestone.settled', + status: 'success', + timestamp: milestone.verifiedAt, + }) + // Build approval/rejection counts for veto-aware vault check const vaultMilestones = getMilestonesByVaultId(vaultId) const approvalCounts: Record = {} diff --git a/src/services/milestones.ts b/src/services/milestones.ts index 8c5ea206..35ed2dfc 100644 --- a/src/services/milestones.ts +++ b/src/services/milestones.ts @@ -12,6 +12,11 @@ export interface Milestone { dueDate: string | null } +import { + authorizeVerifierQueueAction, + assertVerifierLifecycleTransition, +} from './verifierTransitions.js' + const milestonesTable: Milestone[] = [] export const createMilestone = (vaultId: string, description: string, verifierId?: string | null, dueDate?: string | null): Milestone => { @@ -40,12 +45,27 @@ export const getMilestoneById = (id: string): Milestone | undefined => { return milestonesTable.find((m) => m.id === id) } -export const verifyMilestone = (id: string): Milestone | null => { +export const verifyMilestone = (id: string, verifierUserId?: string): Milestone | null => { const milestone = milestonesTable.find((m) => m.id === id) if (!milestone) return null + if (verifierUserId !== undefined) { + authorizeVerifierQueueAction(milestone, verifierUserId, 'verify') + } + milestone.verified = true milestone.verifiedAt = new Date().toISOString() + milestone.verifiedBy = verifierUserId ?? milestone.verifiedBy + + if (verifierUserId !== undefined) { + addMilestoneEvent({ + userId: verifierUserId, + vaultId: milestone.vaultId, + name: 'milestone.verified', + status: 'success', + timestamp: new Date().toISOString(), + }) + } return milestone } @@ -53,8 +73,16 @@ export const validateMilestone = (id: string, validatorUserId: string, evidenceH const milestone = milestonesTable.find((m) => m.id === id) if (!milestone) return { success: false, error: 'Milestone not found' } - if (milestone.verifierId && milestone.verifierId !== validatorUserId) { - return { success: false, error: 'Unauthorized: only assigned verifier can validate' } + // Preserve the public replay contract for the current assignee while still + // checking assignment before an unverified item can be mutated. + if (milestone.verified && milestone.verifierId === validatorUserId) { + return { success: false, error: 'Milestone already validated' } + } + + try { + authorizeVerifierQueueAction(milestone, validatorUserId, 'validate') + } catch (error) { + return { success: false, error: error instanceof Error ? error.message : 'Verifier is not authorized' } } if (milestone.verified) { @@ -157,6 +185,16 @@ export const transitionMilestone = ( const from: MilestoneLifecycleState = lifecycleState[id] ?? 'created' + // Submission is created by the vault workflow, not a verifier queue action. + // Authorization begins when a verifier changes the submitted item. + if (opts?.actor !== undefined && to !== 'submitted') { + const action = to === 'settled' ? 'approve' : 'validate' + // A replay must be acknowledged before checking the now-advanced state. + if (!opts.idempotencyKey || !appliedIdempotencyKeys[id]?.has(opts.idempotencyKey)) { + assertVerifierLifecycleTransition(milestone, opts.actor, action, from, to) + } + } + // Duplicate-request safety: the same idempotency key may only apply once // per milestone. A retry with the same key is acknowledged without // re-applying the transition (exactly-once semantics). Checked before the @@ -412,10 +450,12 @@ export const validateMilestoneMultiVerifier = ( } // For thresholds > 1, multiple verifiers should be able to approve - if (milestone.approvalThreshold === 1 && milestone.verifierId && milestone.verifierId !== validatorUserId) { + try { + authorizeVerifierQueueAction(milestone, validatorUserId, 'approve') + } catch (error) { return { success: false, - error: 'Unauthorized: only assigned verifier can validate this milestone', + error: error instanceof Error ? error.message : 'Verifier is not authorized', milestone, canApprove: false, } diff --git a/src/services/verifierTransitions.test.ts b/src/services/verifierTransitions.test.ts new file mode 100644 index 00000000..aee549f1 --- /dev/null +++ b/src/services/verifierTransitions.test.ts @@ -0,0 +1,203 @@ +import { + assertVerifierLifecycleTransition, + authorizeVerifierQueueAction, + VerifierAuthorizationError, +} from './verifierTransitions.js' +import { describe, expect, it } from '@jest/globals' + +const item = (overrides: Record = {}) => ({ + id: 'ms-1498-test', + verifierId: 'verifier-1', + approvalThreshold: 1, + verified: false, + ...overrides, +}) + +const expectAuthorizationError = ( + action: 'verify' | 'validate' | 'approve', + queueItem: Record, + actor: unknown, + code: string, +) => { + try { + authorizeVerifierQueueAction(queueItem, actor, action) + throw new Error('expected authorization to fail') + } catch (error) { + expect(error).toBeInstanceOf(VerifierAuthorizationError) + expect((error as VerifierAuthorizationError).code).toBe(code) + expect((error as VerifierAuthorizationError).action).toBe(action) + expect((error as VerifierAuthorizationError).itemId).toBe('ms-1498-test') + } +} + +describe('verifier queue authorization', () => { + it.each(['verify', 'validate', 'approve'] as const)( + 'allows the current assignee to %s a single-verifier item', + (action) => { + expect(() => authorizeVerifierQueueAction(item(), 'verifier-1', action)).not.toThrow() + }, + ) + + it.each(['verify', 'validate', 'approve'] as const)( + 'rejects an empty actor before any queue action (%s)', + (action) => { + expectAuthorizationError(action, item(), '', 'ACTOR_REQUIRED') + expectAuthorizationError(action, item(), null, 'ACTOR_REQUIRED') + expectAuthorizationError(action, item(), 42, 'ACTOR_REQUIRED') + }, + ) + + it.each(['verify', 'validate', 'approve'] as const)( + 'rejects an unassigned single-verifier queue item (%s)', + (action) => { + expectAuthorizationError(action, item({ verifierId: null }), 'verifier-1', 'UNASSIGNED_QUEUE_ITEM') + expectAuthorizationError(action, item({ verifierId: '' }), 'verifier-1', 'UNASSIGNED_QUEUE_ITEM') + expectAuthorizationError(action, item({ verifierId: undefined }), 'verifier-1', 'UNASSIGNED_QUEUE_ITEM') + }, + ) + + it.each(['verify', 'validate', 'approve'] as const)( + 'rejects a stale actor after reassignment (%s)', + (action) => { + const reassigned = item({ verifierId: 'verifier-2' }) + expectAuthorizationError(action, reassigned, 'verifier-1', 'STALE_ASSIGNMENT') + expect(() => authorizeVerifierQueueAction(reassigned, 'verifier-2', action)).not.toThrow() + }, + ) + + it('permits approved pool members on an explicitly multi-verifier queue', () => { + const multi = item({ verifierId: 'verifier-coordinator', approvalThreshold: 2 }) + expect(() => authorizeVerifierQueueAction(multi, 'verifier-1', 'approve')).not.toThrow() + expect(() => authorizeVerifierQueueAction(multi, 'verifier-2', 'approve')).not.toThrow() + }) + + it('does not broaden verification or validation for multi-verifier queues', () => { + const multi = item({ verifierId: 'verifier-coordinator', approvalThreshold: 2 }) + expectAuthorizationError('verify', multi, 'verifier-1', 'STALE_ASSIGNMENT') + expectAuthorizationError('validate', multi, 'verifier-1', 'STALE_ASSIGNMENT') + }) + + it('rejects settled queue items before a second verification transition', () => { + expectAuthorizationError('verify', item({ verified: true }), 'verifier-1', 'ALREADY_SETTLED') + expectAuthorizationError('validate', item({ verified: true }), 'verifier-1', 'ALREADY_SETTLED') + }) +}) + +describe('verifier lifecycle transition contract', () => { + it('allows each forward transition for the current assignee', () => { + expect(() => assertVerifierLifecycleTransition(item(), 'verifier-1', 'validate', 'created', 'submitted')).not.toThrow() + expect(() => assertVerifierLifecycleTransition(item(), 'verifier-1', 'validate', 'submitted', 'validated')).not.toThrow() + expect(() => assertVerifierLifecycleTransition(item(), 'verifier-1', 'approve', 'validated', 'settled')).not.toThrow() + }) + + it.each([ + ['created', 'created'], + ['created', 'validated'], + ['submitted', 'submitted'], + ['submitted', 'settled'], + ['validated', 'submitted'], + ['settled', 'validated'], + ] as const)('rejects invalid transition %s -> %s without mutation', (from, to) => { + try { + assertVerifierLifecycleTransition(item(), 'verifier-1', 'validate', from, to) + throw new Error('expected transition to fail') + } catch (error) { + expect(error).toBeInstanceOf(VerifierAuthorizationError) + expect((error as VerifierAuthorizationError).code).toBe('INVALID_TRANSITION') + } + }) + + it('checks authorization before transition validity so stale work cannot probe state', () => { + expect(() => assertVerifierLifecycleTransition( + item({ verifierId: 'verifier-2' }), + 'verifier-1', + 'validate', + 'settled', + 'created' as never, + )).toThrow(/currently assigned verifier/i) + }) +}) + +describe('error shape', () => { + it('keeps stable machine-readable details for API adapters', () => { + try { + authorizeVerifierQueueAction(item({ verifierId: null }), 'verifier-1', 'validate') + } catch (error) { + const typed = error as VerifierAuthorizationError + expect(typed.name).toBe('VerifierAuthorizationError') + expect(typed.code).toBe('UNASSIGNED_QUEUE_ITEM') + expect(typed.action).toBe('validate') + expect(typed.itemId).toBe('ms-1498-test') + expect(typed.message).toMatch(/not assigned/i) + } + }) +}) + +describe('queue boundary invariants', () => { + it('trims actor identity but never trims or normalizes the stored assignment', () => { + expect(() => authorizeVerifierQueueAction(item(), ' verifier-1 ', 'validate')).not.toThrow() + expectAuthorizationError('validate', item({ verifierId: 'verifier-1 ' }), 'verifier-1', 'STALE_ASSIGNMENT') + }) + + it('treats malformed approval thresholds as single-verifier work', () => { + for (const approvalThreshold of [0, -1, 1.5, Number.NaN, '2']) { + expectAuthorizationError( + 'approve', + item({ approvalThreshold, verifierId: null }), + 'verifier-1', + 'UNASSIGNED_QUEUE_ITEM', + ) + } + }) + + it('does not mutate a queue item during any authorization decision', () => { + const queueItem = item() + const before = { ...queueItem } + expect(() => authorizeVerifierQueueAction(queueItem, 'verifier-1', 'verify')).not.toThrow() + expect(() => authorizeVerifierQueueAction(queueItem, 'verifier-2', 'validate')).toThrow() + expect(queueItem).toEqual(before) + }) + + it('keeps the multi-verifier exception limited to approval actions', () => { + const queueItem = item({ approvalThreshold: 3, verifierId: null }) + expect(() => authorizeVerifierQueueAction(queueItem, 'verifier-9', 'approve')).not.toThrow() + expectAuthorizationError('verify', queueItem, 'verifier-9', 'UNASSIGNED_QUEUE_ITEM') + expectAuthorizationError('validate', queueItem, 'verifier-9', 'UNASSIGNED_QUEUE_ITEM') + }) + + it('rejects duplicate and skipped state edges for a stale actor without exposing them as valid', () => { + const queueItem = item({ verifierId: 'verifier-2' }) + for (const [from, to] of [ + ['created', 'created'], + ['created', 'validated'], + ['submitted', 'settled'], + ['validated', 'submitted'], + ] as const) { + expect(() => assertVerifierLifecycleTransition( + queueItem, + 'verifier-1', + 'validate', + from, + to as 'submitted' | 'validated' | 'settled', + )).toThrow(/currently assigned verifier/i) + } + }) + + it('permits only the documented lifecycle edges after authorization', () => { + const queueItem = item() + const edges = [ + ['created', 'submitted'], + ['submitted', 'validated'], + ['validated', 'settled'], + ] as const + for (const [from, to] of edges) { + expect(() => assertVerifierLifecycleTransition( + queueItem, + 'verifier-1', + to === 'settled' ? 'approve' : 'validate', + from, + to, + )).not.toThrow() + } + }) +}) diff --git a/src/services/verifierTransitions.ts b/src/services/verifierTransitions.ts new file mode 100644 index 00000000..87c6aa95 --- /dev/null +++ b/src/services/verifierTransitions.ts @@ -0,0 +1,138 @@ +/** + * Shared authorization boundary for verifier queue actions. + * + * A verifier is allowed to change a queue item only when the item is still + * assigned to that verifier. Multi-verifier milestones are the deliberate + * exception: their threshold is greater than one and the assignment denotes + * the queue's coordinating verifier, while the approved verifier pool may + * cast the individual votes. + * + * Keeping this decision in one small, side-effect-free module is important. + * HTTP routes, background consumers, and future GraphQL mutations must not + * each grow subtly different stale-assignment rules. + */ + +export type VerifierQueueAction = 'verify' | 'validate' | 'approve' + +export interface VerifierQueueItem { + id: string + verifierId?: string | null + approvalThreshold?: number + verified?: boolean +} + +export type VerifierAuthorizationCode = + | 'ACTOR_REQUIRED' + | 'UNASSIGNED_QUEUE_ITEM' + | 'STALE_ASSIGNMENT' + | 'ALREADY_SETTLED' + | 'INVALID_TRANSITION' + +export class VerifierAuthorizationError extends Error { + readonly code: VerifierAuthorizationCode + readonly action: VerifierQueueAction + readonly itemId: string + + constructor( + code: VerifierAuthorizationCode, + action: VerifierQueueAction, + itemId: string, + message: string, + ) { + super(message) + this.name = 'VerifierAuthorizationError' + this.code = code + this.action = action + this.itemId = itemId + } +} + +const thresholdFor = (item: VerifierQueueItem): number => { + const threshold = Number(item.approvalThreshold ?? 1) + return Number.isSafeInteger(threshold) && threshold > 0 ? threshold : 1 +} + +/** + * Assert that an authenticated verifier may perform the requested action. + * This function intentionally performs no mutation, so callers can invoke it + * before a transaction or before changing an in-memory compatibility record. + */ +export const authorizeVerifierQueueAction = ( + item: VerifierQueueItem, + actorUserId: unknown, + action: VerifierQueueAction, +): void => { + if (typeof actorUserId !== 'string' || actorUserId.trim() === '') { + throw new VerifierAuthorizationError( + 'ACTOR_REQUIRED', + action, + item.id, + 'Authentication required for verifier queue action', + ) + } + + if (item.verified === true && action !== 'approve') { + throw new VerifierAuthorizationError( + 'ALREADY_SETTLED', + action, + item.id, + 'Milestone is already settled', + ) + } + + // An M-of-N approval queue is intentionally shared by its approved pool. + // Single-verifier validation and verification always require an exact, + // current assignment, including after an administrator reassigns the item. + const multiVerifierApproval = action === 'approve' && thresholdFor(item) > 1 + if (multiVerifierApproval) return + + if (typeof item.verifierId !== 'string' || item.verifierId.trim() === '') { + throw new VerifierAuthorizationError( + 'UNASSIGNED_QUEUE_ITEM', + action, + item.id, + 'Verifier queue item is not assigned to an active verifier', + ) + } + + if (item.verifierId !== actorUserId.trim()) { + throw new VerifierAuthorizationError( + 'STALE_ASSIGNMENT', + action, + item.id, + 'Unauthorized: only the currently assigned verifier can perform this action', + ) + } +} + +/** + * Validate a monotonic state change before the persistence layer runs it. + * The route still owns the concrete state transition; this helper makes the + * allowed transition contract explicit and testable for every caller. + */ +export const assertVerifierLifecycleTransition = ( + item: VerifierQueueItem, + actorUserId: unknown, + action: VerifierQueueAction, + from: 'created' | 'submitted' | 'validated' | 'settled', + to: 'submitted' | 'validated' | 'settled', +): void => { + authorizeVerifierQueueAction(item, actorUserId, action) + + const allowed: Record = { + created: ['submitted'], + submitted: ['validated'], + validated: ['settled'], + settled: [], + } + + if (!allowed[from]?.includes(to)) { + throw new VerifierAuthorizationError( + 'INVALID_TRANSITION', + action, + item.id, + `Invalid verifier lifecycle transition: ${from} -> ${to}`, + ) + } +} + diff --git a/src/services/verifiers.ts b/src/services/verifiers.ts index f3111228..e1fa1431 100644 --- a/src/services/verifiers.ts +++ b/src/services/verifiers.ts @@ -752,8 +752,8 @@ export const getMilestoneApprovalProgress = async ( const pending = approvals.pending.length const totalVoted = approved + rejected + pending - // Hostile-input boundary: clamp thresholds to sane ranges so zero, negative, - // or non-numeric inputs can never produce a degenerate veto/complete result. + // Hostile-input boundary: clamp totals to sane ranges so malformed input + // cannot produce a degenerate veto/complete result. const safeTotal = totalVerifiers !== undefined && totalVerifiers > 0 ? Math.max(1, Math.floor(Number(totalVerifiers)))