fix(account-delete): judge step-up auth by amr sign-in time, not token iat
This commit is contained in:
parent
043ef579a8
commit
83c2deb561
3 changed files with 167 additions and 19 deletions
|
|
@ -1,13 +1,13 @@
|
|||
import { corsHeaders, handleCorsPreflightRequest } from '../_shared/cors.ts'
|
||||
import { requireUser, authErrorResponse, type AuthError } from '../_shared/auth.ts'
|
||||
import { createServiceRoleClient } from '../_shared/quota.ts'
|
||||
import { hasRecentAuthentication } from './recent-auth.ts'
|
||||
|
||||
interface DeleteAccountRequest {
|
||||
confirmation: string
|
||||
}
|
||||
|
||||
const CONFIRMATION_PHRASE = 'DELETE_MY_ACCOUNT'
|
||||
const RECENT_AUTH_SECONDS = 10 * 60
|
||||
const STORAGE_BUCKETS = ['audio', 'exports', 'avatars'] as const
|
||||
|
||||
function jsonResponse(body: Record<string, unknown>, status = 200): Response {
|
||||
|
|
@ -17,22 +17,6 @@ function jsonResponse(body: Record<string, unknown>, status = 200): Response {
|
|||
})
|
||||
}
|
||||
|
||||
function decodeJwtIssuedAt(authorization: string | null): number | null {
|
||||
const token = authorization?.replace(/^Bearer\s+/i, '')
|
||||
if (!token) return null
|
||||
const payloadPart = token.split('.')[1]
|
||||
if (!payloadPart) return null
|
||||
|
||||
try {
|
||||
const normalized = payloadPart.replace(/-/g, '+').replace(/_/g, '/')
|
||||
const padded = normalized.padEnd(Math.ceil(normalized.length / 4) * 4, '=')
|
||||
const payload = JSON.parse(atob(padded)) as { iat?: unknown }
|
||||
return typeof payload.iat === 'number' ? payload.iat : null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
async function listStorageFiles(
|
||||
serviceClient: ReturnType<typeof createServiceRoleClient>,
|
||||
bucket: string,
|
||||
|
|
@ -81,9 +65,10 @@ Deno.serve(async (req: Request) => {
|
|||
return jsonResponse({ error: 'Explicit confirmation is required', code: 'CONFIRMATION_REQUIRED' }, 400)
|
||||
}
|
||||
|
||||
const issuedAt = decodeJwtIssuedAt(req.headers.get('Authorization'))
|
||||
// Step-up auth: judged by the session's amr authentication time, never the
|
||||
// access token iat (which every refresh_token grant resets).
|
||||
const nowSeconds = Math.floor(Date.now() / 1000)
|
||||
if (issuedAt === null || nowSeconds - issuedAt > RECENT_AUTH_SECONDS || issuedAt > nowSeconds + 60) {
|
||||
if (!hasRecentAuthentication(req.headers.get('Authorization'), nowSeconds)) {
|
||||
return jsonResponse({ error: 'Recent authentication is required', code: 'REAUTHENTICATION_REQUIRED' }, 403)
|
||||
}
|
||||
|
||||
|
|
|
|||
81
server/supabase/functions/account-delete/recent-auth.test.ts
Normal file
81
server/supabase/functions/account-delete/recent-auth.test.ts
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
import {
|
||||
decodeJwtPayload,
|
||||
extractBearerToken,
|
||||
hasRecentAuthentication,
|
||||
isRecentlyAuthenticated,
|
||||
parseSessionAuthClaims,
|
||||
RECENT_AUTH_SECONDS,
|
||||
resolveAuthenticatedAt,
|
||||
} from './recent-auth.ts'
|
||||
|
||||
function assert(condition: boolean, message: string): asserts condition {
|
||||
if (!condition) throw new Error(message)
|
||||
}
|
||||
|
||||
function base64UrlJson(value: unknown): string {
|
||||
return btoa(JSON.stringify(value)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/g, '')
|
||||
}
|
||||
|
||||
function bearer(payload: Record<string, unknown>): string {
|
||||
return `Bearer ${base64UrlJson({ alg: 'HS256', typ: 'JWT' })}.${base64UrlJson(payload)}.signature`
|
||||
}
|
||||
|
||||
const NOW = 1_800_000_000
|
||||
const MONTHS_AGO = NOW - 90 * 24 * 60 * 60
|
||||
|
||||
Deno.test('redteam r1-20: a refreshed token (fresh iat) from an old login is NOT recent authentication', () => {
|
||||
const refreshed = bearer({
|
||||
sub: 'user-1',
|
||||
iat: NOW - 5,
|
||||
exp: NOW + 3600,
|
||||
amr: [{ method: 'password', timestamp: MONTHS_AGO }],
|
||||
})
|
||||
assert(!hasRecentAuthentication(refreshed, NOW), 'refresh_token grant must not satisfy step-up auth')
|
||||
})
|
||||
|
||||
Deno.test('redteam r1-20: a token without amr claims is rejected even with a fresh iat', () => {
|
||||
assert(!hasRecentAuthentication(bearer({ sub: 'user-1', iat: NOW }), NOW), 'missing amr must fail closed')
|
||||
assert(!hasRecentAuthentication(bearer({ sub: 'user-1', iat: NOW, amr: [] }), NOW), 'empty amr must fail closed')
|
||||
assert(
|
||||
!hasRecentAuthentication(bearer({ sub: 'user-1', iat: NOW, amr: [{ method: 'password' }] }), NOW),
|
||||
'amr entries without timestamps must fail closed',
|
||||
)
|
||||
})
|
||||
|
||||
Deno.test('recent sign-in (amr timestamp inside the window) is accepted', () => {
|
||||
const fresh = bearer({ sub: 'user-1', iat: NOW - 30, amr: [{ method: 'password', timestamp: NOW - 30 }] })
|
||||
assert(hasRecentAuthentication(fresh, NOW), 'fresh login must pass')
|
||||
const edge = bearer({ sub: 'user-1', iat: NOW, amr: [{ method: 'otp', timestamp: NOW - RECENT_AUTH_SECONDS }] })
|
||||
assert(hasRecentAuthentication(edge, NOW), 'exactly at the window boundary must pass')
|
||||
const stale = bearer({ sub: 'user-1', iat: NOW, amr: [{ method: 'otp', timestamp: NOW - RECENT_AUTH_SECONDS - 1 }] })
|
||||
assert(!hasRecentAuthentication(stale, NOW), 'one second past the window must fail')
|
||||
})
|
||||
|
||||
Deno.test('the most recent amr method counts (e.g. a fresh TOTP step-up on an old password session)', () => {
|
||||
const claims = parseSessionAuthClaims({
|
||||
amr: [
|
||||
{ method: 'password', timestamp: MONTHS_AGO },
|
||||
{ method: 'totp', timestamp: NOW - 60 },
|
||||
'garbage',
|
||||
{ method: 'oauth', timestamp: 'nope' },
|
||||
],
|
||||
})
|
||||
assert(claims !== null && claims.amr.length === 2, 'invalid amr entries are skipped')
|
||||
assert(resolveAuthenticatedAt(claims) === NOW - 60, 'latest timestamp wins')
|
||||
assert(isRecentlyAuthenticated(resolveAuthenticatedAt(claims), NOW), 'step-up within window passes')
|
||||
})
|
||||
|
||||
Deno.test('future authentication times beyond clock skew are rejected', () => {
|
||||
assert(isRecentlyAuthenticated(NOW + 30, NOW), 'small skew tolerated')
|
||||
assert(!isRecentlyAuthenticated(NOW + 3600, NOW), 'far-future timestamp rejected')
|
||||
assert(!isRecentlyAuthenticated(null, NOW), 'null rejected')
|
||||
})
|
||||
|
||||
Deno.test('malformed Authorization headers fail closed', () => {
|
||||
assert(extractBearerToken(null) === null, 'null header')
|
||||
assert(extractBearerToken('Bearer ') === null, 'blank token')
|
||||
assert(decodeJwtPayload('not-a-jwt') === null, 'no payload segment')
|
||||
assert(decodeJwtPayload('a.!!!.c') === null, 'invalid base64')
|
||||
assert(decodeJwtPayload(`a.${base64UrlJson([1, 2])}.c`) === null, 'array payload')
|
||||
assert(!hasRecentAuthentication('Bearer a.!!!.c', NOW), 'malformed token rejected')
|
||||
})
|
||||
82
server/supabase/functions/account-delete/recent-auth.ts
Normal file
82
server/supabase/functions/account-delete/recent-auth.ts
Normal file
|
|
@ -0,0 +1,82 @@
|
|||
// server/supabase/functions/account-delete/recent-auth.ts
|
||||
// Step-up authentication policy for destructive account operations.
|
||||
//
|
||||
// The access token `iat` is NOT an authentication time: GoTrue mints a fresh
|
||||
// access token (new `iat`) on every refresh_token grant, so anyone holding a
|
||||
// refresh token can make `iat` "recent" without re-entering credentials.
|
||||
// The `amr` claim entries carry the time each authentication method was last
|
||||
// completed for the session and survive refresh, so the most recent `amr`
|
||||
// timestamp is the session's real authentication time.
|
||||
|
||||
export const RECENT_AUTH_SECONDS = 10 * 60
|
||||
/** Tolerated clock skew for timestamps slightly in the future. */
|
||||
export const CLOCK_SKEW_SECONDS = 60
|
||||
|
||||
export interface AmrEntry {
|
||||
method: string
|
||||
timestamp: number
|
||||
}
|
||||
|
||||
export interface SessionAuthClaims {
|
||||
amr: AmrEntry[]
|
||||
}
|
||||
|
||||
export function extractBearerToken(authorization: string | null): string | null {
|
||||
const token = authorization?.replace(/^Bearer\s+/i, '').trim()
|
||||
return token ? token : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Decodes (without verifying) the JWT payload. Callers must verify the token
|
||||
* first (requireUser -> auth.getUser()) before trusting these claims.
|
||||
*/
|
||||
export function decodeJwtPayload(token: string | null): Record<string, unknown> | null {
|
||||
if (!token) return null
|
||||
const payloadPart = token.split('.')[1]
|
||||
if (!payloadPart) return null
|
||||
|
||||
try {
|
||||
const normalized = payloadPart.replace(/-/g, '+').replace(/_/g, '/')
|
||||
const padded = normalized.padEnd(Math.ceil(normalized.length / 4) * 4, '=')
|
||||
const payload: unknown = JSON.parse(atob(padded))
|
||||
if (!payload || typeof payload !== 'object' || Array.isArray(payload)) return null
|
||||
return payload as Record<string, unknown>
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export function parseSessionAuthClaims(payload: Record<string, unknown> | null): SessionAuthClaims | null {
|
||||
if (!payload || !Array.isArray(payload.amr)) return null
|
||||
|
||||
const amr: AmrEntry[] = []
|
||||
for (const entry of payload.amr as unknown[]) {
|
||||
if (!entry || typeof entry !== 'object') continue
|
||||
const { method, timestamp } = entry as { method?: unknown; timestamp?: unknown }
|
||||
if (typeof method !== 'string' || typeof timestamp !== 'number' || !Number.isFinite(timestamp)) continue
|
||||
amr.push({ method, timestamp })
|
||||
}
|
||||
return { amr }
|
||||
}
|
||||
|
||||
/** The most recent time the session completed any authentication method, or null. */
|
||||
export function resolveAuthenticatedAt(claims: SessionAuthClaims | null): number | null {
|
||||
if (!claims || claims.amr.length === 0) return null
|
||||
return Math.max(...claims.amr.map((entry) => entry.timestamp))
|
||||
}
|
||||
|
||||
export function isRecentlyAuthenticated(
|
||||
authenticatedAt: number | null,
|
||||
nowSeconds: number,
|
||||
windowSeconds: number = RECENT_AUTH_SECONDS,
|
||||
): boolean {
|
||||
if (authenticatedAt === null) return false
|
||||
if (authenticatedAt > nowSeconds + CLOCK_SKEW_SECONDS) return false
|
||||
return nowSeconds - authenticatedAt <= windowSeconds
|
||||
}
|
||||
|
||||
/** Composes the policy from a raw Authorization header. */
|
||||
export function hasRecentAuthentication(authorization: string | null, nowSeconds: number): boolean {
|
||||
const claims = parseSessionAuthClaims(decodeJwtPayload(extractBearerToken(authorization)))
|
||||
return isRecentlyAuthenticated(resolveAuthenticatedAt(claims), nowSeconds)
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue