feat(auth): make automatic logout an opt-in per-user setting (#1536)
* feat(auth): make automatic logout an opt-in per-user setting Session timeouts (30 min idle / 12 h absolute on hosted) now apply only to users who enable "Automatic logout" in Settings > Security. Default is off: sessions live for the full Supabase refresh-token lifetime, the behavior from before the 2026-07 session hardening. - user_preferences.auto_logout (migration, default false), toggled via the extended /api/user/preferences route - The opt-in is snapshotted into the signed timeout cookie at mint, so enforcement stays DB-read-free per request; the preferences route clears the cookie on change so a toggle takes effect immediately - Pre-toggle cookies are authentic-but-stale: re-minted preserving their timers, never routed down the tamper path, so the rollout does not log anyone out - NEXT_PUBLIC_SESSION_TIMEOUT_FORCE_ALL=true enforces timeouts for every user regardless of preference (emergency lever, also plumbed through the Docker image); self-hosted stays disabled by default Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(auth): resolve PR #1536 review findings - Replace the spread upsert in /api/user/preferences with one literal payload per field: the phantom-column schema guard cannot resolve spread payloads (Unit tests 3/4 ceiling failure) - Map the preferences 500 through getErrorMessage so the user-facing text is Swedish (CodeRabbit) - fetchAutoLogoutPreference now returns null on a FAILED read instead of a fail-open false: callers skip minting so an unknown preference is never persisted into the year-long signed cookie, and the next request retries; failures log at error level, distinct from the normal opt-out path (compliance swarm GDPR Art.32(1)(b) / ISO A.8.5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(auth): write multi-field preference updates as one atomic upsert A request carrying both hide_assistant_fab and auto_logout previously issued two sequential writes, so a failure of the second returned 500 after half the request had persisted (CodeRabbit, PR #1536). One literal upsert per accepted field combination keeps the write atomic and stays resolvable for the phantom-column schema guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
2e2a64dd0a
commit
11995b1b0c
@@ -26,12 +26,24 @@ vi.mock('next/headers', () => ({
|
||||
|
||||
import { GET, POST } from '../route'
|
||||
|
||||
const preference = { autoLogout: false, error: null as unknown }
|
||||
|
||||
const supabase = {
|
||||
auth: {
|
||||
getClaims: vi.fn(async () => ({
|
||||
data: { claims: { session_id: 'session-1' } },
|
||||
})),
|
||||
},
|
||||
from: vi.fn(() => ({
|
||||
select: vi.fn(() => ({
|
||||
eq: vi.fn(() => ({
|
||||
maybeSingle: vi.fn(async () => ({
|
||||
data: preference.error ? null : { auto_logout: preference.autoLogout },
|
||||
error: preference.error,
|
||||
})),
|
||||
})),
|
||||
})),
|
||||
})),
|
||||
}
|
||||
|
||||
describe('session heartbeat route', () => {
|
||||
@@ -47,18 +59,23 @@ describe('session heartbeat route', () => {
|
||||
error: null,
|
||||
})
|
||||
mocks.cookieValue = undefined
|
||||
preference.autoLogout = false
|
||||
preference.error = null
|
||||
})
|
||||
|
||||
async function setState(args?: {
|
||||
startedAt?: number
|
||||
lastActivityAt?: number
|
||||
sessionId?: string
|
||||
autoLogout?: boolean
|
||||
}) {
|
||||
const state = {
|
||||
...createSessionTimeoutState({
|
||||
userId: 'user-1',
|
||||
sessionId: args?.sessionId ?? 'session-1',
|
||||
method: 'password',
|
||||
// Default the opt-in to true: most cases here exercise enforcement.
|
||||
autoLogout: args?.autoLogout ?? true,
|
||||
now: args?.startedAt ?? Date.now(),
|
||||
}),
|
||||
...(args?.lastActivityAt === undefined
|
||||
@@ -127,6 +144,67 @@ describe('session heartbeat route', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('reports enabled: false and never expires an opted-out session', async () => {
|
||||
const now = Date.now()
|
||||
// Times that would be long expired if enforcement applied.
|
||||
await setState({
|
||||
startedAt: now - 120_000,
|
||||
lastActivityAt: now - 120_000,
|
||||
autoLogout: false,
|
||||
})
|
||||
|
||||
const response = await GET()
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
data: { enabled: false },
|
||||
})
|
||||
})
|
||||
|
||||
it('upgrades a pre-toggle cookie in place, keeping its timers', async () => {
|
||||
preference.autoLogout = true
|
||||
const startedAt = Date.now() - 5_000
|
||||
const legacy: Record<string, unknown> = {
|
||||
...createSessionTimeoutState({
|
||||
userId: 'user-1',
|
||||
sessionId: 'session-1',
|
||||
method: 'password',
|
||||
autoLogout: true,
|
||||
now: startedAt,
|
||||
}),
|
||||
}
|
||||
delete legacy.autoLogout
|
||||
mocks.cookieValue =
|
||||
(await signSessionTimeoutState(
|
||||
legacy as Parameters<typeof signSessionTimeoutState>[0],
|
||||
)) ?? undefined
|
||||
|
||||
const response = await GET()
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
const upgraded = response.cookies.get(SESSION_TIMEOUT_COOKIE)?.value
|
||||
expect(upgraded).toBeTruthy()
|
||||
await expect(verifySessionTimeoutState(upgraded)).resolves.toMatchObject({
|
||||
startedAt,
|
||||
autoLogout: true,
|
||||
})
|
||||
})
|
||||
|
||||
it('answers without persisting a snapshot when the preference read fails', async () => {
|
||||
preference.error = { message: 'connection reset' }
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
const response = await GET()
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
data: { enabled: false },
|
||||
})
|
||||
// No cookie: the unknown preference must not be baked into a snapshot.
|
||||
expect(response.cookies.get(SESSION_TIMEOUT_COOKIE)).toBeUndefined()
|
||||
errorSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('passes through the existing authentication error', async () => {
|
||||
mocks.requireAuth.mockResolvedValue({
|
||||
user: null,
|
||||
|
||||
@@ -5,8 +5,10 @@ import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import {
|
||||
createSessionTimeoutState,
|
||||
evaluateSessionTimeout,
|
||||
fetchAutoLogoutPreference,
|
||||
getSessionTimeoutConfig,
|
||||
sessionStateMatchesUser,
|
||||
sessionStateNeedsRemint,
|
||||
sessionTimeoutCookieOptions,
|
||||
signSessionTimeoutState,
|
||||
toSessionTimeoutClientState,
|
||||
@@ -71,28 +73,40 @@ async function heartbeat(updateActivity: boolean): Promise<NextResponse> {
|
||||
const state = await verifySessionTimeoutState(encodedState)
|
||||
const sessionId = await getSessionId(auth.supabase)
|
||||
|
||||
if (!state || !sessionStateMatchesUser(state, auth.user.id, sessionId)) {
|
||||
// Mirror middleware initialization: a missing or session-mismatched
|
||||
// cookie means the timeout state has not been established for this
|
||||
// session yet, not that the session expired.
|
||||
const stateMatches =
|
||||
state !== null && sessionStateMatchesUser(state, auth.user.id, sessionId)
|
||||
|
||||
if (!state || !stateMatches || sessionStateNeedsRemint(state)) {
|
||||
// Mirror middleware initialization: a missing, session-mismatched, or
|
||||
// pre-toggle cookie means the timeout state has not been established
|
||||
// for this session yet, not that the session expired.
|
||||
const hintedMethod = cookieStore.get(SESSION_AUTH_METHOD_HINT_COOKIE)?.value
|
||||
const freshState = createSessionTimeoutState({
|
||||
userId: auth.user.id,
|
||||
sessionId,
|
||||
method: isSessionAuthMethod(hintedMethod) ? hintedMethod : 'password',
|
||||
now,
|
||||
})
|
||||
const autoLogout = await fetchAutoLogoutPreference(auth.supabase, auth.user.id)
|
||||
const freshState = state && stateMatches
|
||||
? { ...state, autoLogout: autoLogout ?? false }
|
||||
: createSessionTimeoutState({
|
||||
userId: auth.user.id,
|
||||
sessionId,
|
||||
method: isSessionAuthMethod(hintedMethod) ? hintedMethod : 'password',
|
||||
autoLogout: autoLogout ?? false,
|
||||
now,
|
||||
})
|
||||
const response = NextResponse.json({
|
||||
data: toSessionTimeoutClientState(freshState, config, now),
|
||||
})
|
||||
response.headers.set('Cache-Control', 'no-store')
|
||||
const signedFresh = await signSessionTimeoutState(freshState)
|
||||
if (signedFresh) {
|
||||
response.cookies.set(
|
||||
SESSION_TIMEOUT_COOKIE,
|
||||
signedFresh,
|
||||
sessionTimeoutCookieOptions(),
|
||||
)
|
||||
// Unknown preference (failed read): answer this poll without persisting
|
||||
// a fail-open snapshot; the next resync retries the read (mirrors the
|
||||
// middleware).
|
||||
if (autoLogout !== null) {
|
||||
const signedFresh = await signSessionTimeoutState(freshState)
|
||||
if (signedFresh) {
|
||||
response.cookies.set(
|
||||
SESSION_TIMEOUT_COOKIE,
|
||||
signedFresh,
|
||||
sessionTimeoutCookieOptions(),
|
||||
)
|
||||
}
|
||||
}
|
||||
return response
|
||||
}
|
||||
|
||||
@@ -21,7 +21,9 @@ function unauthed() {
|
||||
})
|
||||
}
|
||||
|
||||
function authedForGet(row: { hide_assistant_fab: boolean } | null) {
|
||||
function authedForGet(
|
||||
row: { hide_assistant_fab?: boolean; auto_logout?: boolean } | null,
|
||||
) {
|
||||
const maybeSingle = vi.fn().mockResolvedValue({ data: row, error: null })
|
||||
const supabase = {
|
||||
from: vi.fn(() => ({
|
||||
@@ -56,19 +58,19 @@ describe('GET /api/user/preferences', () => {
|
||||
expect(res.status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns the stored preference', async () => {
|
||||
authedForGet({ hide_assistant_fab: true })
|
||||
it('returns the stored preferences', async () => {
|
||||
authedForGet({ hide_assistant_fab: true, auto_logout: true })
|
||||
const res = await GET()
|
||||
const { status, body } = await parseJsonResponse<{ data: unknown }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data).toEqual({ hide_assistant_fab: true })
|
||||
expect(body.data).toEqual({ hide_assistant_fab: true, auto_logout: true })
|
||||
})
|
||||
|
||||
it('defaults to false when no preferences row exists', async () => {
|
||||
authedForGet(null)
|
||||
const res = await GET()
|
||||
const { body } = await parseJsonResponse<{ data: unknown }>(res)
|
||||
expect(body.data).toEqual({ hide_assistant_fab: false })
|
||||
expect(body.data).toEqual({ hide_assistant_fab: false, auto_logout: false })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -103,6 +105,48 @@ describe('PATCH /api/user/preferences', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('writes a multi-field request as one atomic upsert', async () => {
|
||||
const { upsert } = authedForPatch()
|
||||
const res = await PATCH(
|
||||
patchRequest({ hide_assistant_fab: true, auto_logout: false })
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
expect(upsert).toHaveBeenCalledTimes(1)
|
||||
expect(upsert).toHaveBeenCalledWith(
|
||||
{ user_id: 'user-1', hide_assistant_fab: true, auto_logout: false },
|
||||
{ onConflict: 'user_id' }
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects an empty body with 400', async () => {
|
||||
authedForPatch()
|
||||
const res = await PATCH(patchRequest({}))
|
||||
expect(res.status).toBe(400)
|
||||
})
|
||||
|
||||
it('upserts auto_logout and resets the session timeout cookie', async () => {
|
||||
const { upsert } = authedForPatch()
|
||||
const res = await PATCH(patchRequest({ auto_logout: true }))
|
||||
const { status, body } = await parseJsonResponse<{ data: unknown }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data).toEqual({ auto_logout: true })
|
||||
expect(upsert).toHaveBeenCalledWith(
|
||||
{ user_id: 'user-1', auto_logout: true },
|
||||
{ onConflict: 'user_id' }
|
||||
)
|
||||
// The signed timeout cookie caches the opt-in; a change must clear it so
|
||||
// the middleware re-mints with the new preference on the next request.
|
||||
const setCookie = res.headers.get('set-cookie') ?? ''
|
||||
expect(setCookie).toContain('gnubok-session-timeout=;')
|
||||
})
|
||||
|
||||
it('does not touch the session timeout cookie for unrelated preferences', async () => {
|
||||
authedForPatch()
|
||||
const res = await PATCH(patchRequest({ hide_assistant_fab: true }))
|
||||
expect(res.status).toBe(200)
|
||||
expect(res.headers.get('set-cookie')).toBeNull()
|
||||
})
|
||||
|
||||
it('returns 500 when the upsert fails', async () => {
|
||||
authedForPatch({ message: 'boom' })
|
||||
const res = await PATCH(patchRequest({ hide_assistant_fab: false }))
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { z } from 'zod'
|
||||
import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { sessionTimeoutClearCookieOptions } from '@/lib/auth/session-timeout'
|
||||
import { SESSION_TIMEOUT_COOKIE } from '@/lib/auth/session-timeout-shared'
|
||||
|
||||
// User-level UI preferences (not company-scoped), stored on user_preferences.
|
||||
// Mirrors the /api/user/locale pattern: requireAuth directly because these
|
||||
@@ -8,9 +11,13 @@ import { requireAuth } from '@/lib/auth/require-auth'
|
||||
|
||||
const BodySchema = z
|
||||
.object({
|
||||
hide_assistant_fab: z.boolean(),
|
||||
hide_assistant_fab: z.boolean().optional(),
|
||||
auto_logout: z.boolean().optional(),
|
||||
})
|
||||
.strict()
|
||||
.refine((value) => Object.keys(value).length > 0, {
|
||||
message: 'At least one preference is required',
|
||||
})
|
||||
|
||||
export async function GET() {
|
||||
const { user, supabase, error } = await requireAuth()
|
||||
@@ -18,12 +25,15 @@ export async function GET() {
|
||||
|
||||
const { data } = await supabase
|
||||
.from('user_preferences')
|
||||
.select('hide_assistant_fab')
|
||||
.select('hide_assistant_fab, auto_logout')
|
||||
.eq('user_id', user.id)
|
||||
.maybeSingle()
|
||||
|
||||
return NextResponse.json({
|
||||
data: { hide_assistant_fab: data?.hide_assistant_fab ?? false },
|
||||
data: {
|
||||
hide_assistant_fab: data?.hide_assistant_fab ?? false,
|
||||
auto_logout: data?.auto_logout ?? false,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
@@ -43,16 +53,56 @@ export async function PATCH(request: Request) {
|
||||
return NextResponse.json({ error: 'Invalid preferences' }, { status: 400 })
|
||||
}
|
||||
|
||||
const { error: upsertError } = await supabase
|
||||
.from('user_preferences')
|
||||
.upsert(
|
||||
{ user_id: user.id, hide_assistant_fab: parsed.data.hide_assistant_fab },
|
||||
{ onConflict: 'user_id' }
|
||||
)
|
||||
const { hide_assistant_fab, auto_logout } = parsed.data
|
||||
|
||||
if (upsertError) {
|
||||
return NextResponse.json({ error: 'Could not save preference' }, { status: 500 })
|
||||
// One literal upsert per accepted field combination: the phantom-column
|
||||
// schema guard cannot resolve spread payloads, and a single write keeps a
|
||||
// multi-field request atomic.
|
||||
let upsertError: unknown = null
|
||||
if (hide_assistant_fab !== undefined && auto_logout !== undefined) {
|
||||
const { error } = await supabase
|
||||
.from('user_preferences')
|
||||
.upsert(
|
||||
{ user_id: user.id, hide_assistant_fab, auto_logout },
|
||||
{ onConflict: 'user_id' },
|
||||
)
|
||||
upsertError = error
|
||||
} else if (hide_assistant_fab !== undefined) {
|
||||
const { error } = await supabase
|
||||
.from('user_preferences')
|
||||
.upsert({ user_id: user.id, hide_assistant_fab }, { onConflict: 'user_id' })
|
||||
upsertError = error
|
||||
} else if (auto_logout !== undefined) {
|
||||
const { error } = await supabase
|
||||
.from('user_preferences')
|
||||
.upsert({ user_id: user.id, auto_logout }, { onConflict: 'user_id' })
|
||||
upsertError = error
|
||||
}
|
||||
|
||||
return NextResponse.json({ data: parsed.data })
|
||||
if (upsertError) {
|
||||
return NextResponse.json(
|
||||
{
|
||||
error: getErrorMessage(upsertError, {
|
||||
context: 'settings',
|
||||
statusCode: 500,
|
||||
}),
|
||||
},
|
||||
{ status: 500 },
|
||||
)
|
||||
}
|
||||
|
||||
const response = NextResponse.json({ data: parsed.data })
|
||||
|
||||
if (parsed.data.auto_logout !== undefined) {
|
||||
// The middleware caches the opt-in inside the signed timeout cookie.
|
||||
// Clearing it forces a re-mint on the next request, so the change takes
|
||||
// effect immediately instead of at the next login.
|
||||
response.cookies.set(
|
||||
SESSION_TIMEOUT_COOKIE,
|
||||
'',
|
||||
sessionTimeoutClearCookieOptions(),
|
||||
)
|
||||
}
|
||||
|
||||
return response
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user