diff --git a/.changeset/oauth-passkey-requirement.md b/.changeset/oauth-passkey-requirement.md new file mode 100644 index 00000000..0f33c282 --- /dev/null +++ b/.changeset/oauth-passkey-requirement.md @@ -0,0 +1,8 @@ +--- +'seamless-auth-api': patch +--- + +OAuth sign-in respects the per-account passkey requirement (#469). + +- With `passkey_login_fallback_enabled` off, `POST /oauth/:providerId/callback` for an account that holds a passkey now answers `403 login_method_disabled` and issues no session, as the email code, phone code, magic link and TOTP continuations already did. The refusal is recorded as `oauth_login_failed` with reason `passkey_required`. +- Session issuance now applies the same rule as its backstop, not only phishing-resistant-only mode, so a sign-in path that misses the check fails closed instead of issuing a session. diff --git a/.changeset/revoke-all-sessions-cutoff.md b/.changeset/revoke-all-sessions-cutoff.md new file mode 100644 index 00000000..f9ae2679 --- /dev/null +++ b/.changeset/revoke-all-sessions-cutoff.md @@ -0,0 +1,10 @@ +--- +'seamless-auth-api': patch +--- + +Log out everywhere can no longer miss a session a concurrent refresh was creating (#471). + +- `DELETE /logout/all`, `DELETE /sessions`, `DELETE /admin/sessions/:userId/revoke-all`, device-replacement recovery and OAuth provider retirement revoke with one bulk update instead of reading the sessions and revoking them one at a time. +- Each also records a per-user cutoff. A session created at or before it is refused on every authenticated request and on `POST /refresh`, which re-reads the cutoff after claiming the rotation, so a rotation that started before the revoke cannot produce a live session. The refusal is `401 invalid_refresh_token`, with `refusal: 'sessions_revoked'` on the `refresh_token_failed` auth event. +- Validating an access token now reads the cutoff from the session owner's row, one indexed lookup per authenticated request. +- Migration: `users` gains `sessions_revoked_before`. diff --git a/docs/admin-operations.md b/docs/admin-operations.md index ab910440..e8a42f96 100644 --- a/docs/admin-operations.md +++ b/docs/admin-operations.md @@ -85,7 +85,7 @@ It does not return credential private material, TOTP secrets, recovery codes, re ## Session Hygiene -Administrative session endpoints can list sessions and revoke individual or all sessions for a user. Use these endpoints when responding to suspicious account activity or user-requested device cleanup. +Administrative session endpoints can list sessions and revoke individual or all sessions for a user. Use these endpoints when responding to suspicious account activity or user-requested device cleanup. Revoking all of a user's sessions also records a cutoff on the account, so a session a concurrent refresh was creating at that moment is refused as well (see [Log out everywhere](./security-posture.md#log-out-everywhere)). ## Lockout Policy diff --git a/docs/architecture.md b/docs/architecture.md index 0f878412..eeb09c32 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -94,8 +94,10 @@ Supported login methods are controlled by `login_methods` system config: Accounts holding a passkey can be restricted to passkey-only continuation by disabling `passkey_login_fallback_enabled`. `phishing_resistant_only` goes further and restricts every account to passkeys, apart from the one session that verifies a new account's address. Both -are enforced on each continuation endpoint as well as in the `/login` method list, and session -issuance refuses a non-passkey factor in phishing-resistant-only mode as a backstop. The client sends a `passkeyAvailable` capability hint on +are enforced on each continuation endpoint, including the OAuth callback, as well as in the +`/login` method list. Session issuance applies both rules again as a backstop, refusing a +non-passkey factor whenever either one requires a passkey, so a sign-in path that forgets the +check fails closed instead of issuing a session. The client sends a `passkeyAvailable` capability hint on `POST /login`, but it is advisory: it can remove passkey from a set the policy already permits, never add a weaker method to a passkey-only one. diff --git a/docs/configuration.md b/docs/configuration.md index ad978387..0134479c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -79,7 +79,7 @@ first boot. | `RATE_LIMIT` | Yes | - | `rate_limit` | Global limit, positive integer. | | `DELAY_AFTER` | Yes | - | `delay_after` | Slow-down threshold, non-negative integer. | | `LOGIN_METHODS` | No | `passkey,magic_link` | `login_methods` | Any of `passkey,magic_link,email_otp,phone_otp,oauth`. `.env.example` ships `passkey,magic_link,email_otp` so a stock instance is CLI/headless-loginable; `email_otp` needs a configured messaging transport or the external-delivery path. | -| `PASSKEY_LOGIN_FALLBACK_ENABLED` | No | `true` | `passkey_login_fallback_enabled` | When `false`, an account that holds a passkey continues with passkey only. The client's `passkeyAvailable` hint on `POST /login` cannot widen that: a caller reporting no passkey support is still offered passkey only, because otherwise any caller could ask for a weaker method by claiming not to support passkeys. A browser that genuinely cannot run the ceremony cannot sign in, which is what this setting means. Accounts with no passkey are unaffected and keep the configured methods. The rule binds on every continuation endpoint, not only on the list `/login` returns, so a passkey holder calling an email code, phone code, magic link, TOTP or email verification endpoint directly gets `403 login_method_disabled`. | +| `PASSKEY_LOGIN_FALLBACK_ENABLED` | No | `true` | `passkey_login_fallback_enabled` | When `false`, an account that holds a passkey continues with passkey only. The client's `passkeyAvailable` hint on `POST /login` cannot widen that: a caller reporting no passkey support is still offered passkey only, because otherwise any caller could ask for a weaker method by claiming not to support passkeys. A browser that genuinely cannot run the ceremony cannot sign in, which is what this setting means. Accounts with no passkey are unaffected and keep the configured methods. The rule binds on every continuation endpoint, not only on the list `/login` returns, so a passkey holder calling an email code, phone code, magic link, TOTP or email verification endpoint directly, or finishing an OAuth sign-in, gets `403 login_method_disabled`. | | `PHISHING_RESISTANT_ONLY` | No | `false` | `phishing_resistant_only` | When `true`, a session starts only from a passkey. Email and phone codes, magic links, TOTP and OAuth are refused (`403 login_method_disabled`, and OAuth providers are hidden) whatever `LOGIN_METHODS` and `PASSKEY_LOGIN_FALLBACK_ENABLED` say, and `GET /system-config/public` reports `loginMethods: ["passkey"]`. The one exception is a new account: the email code that verifies its address starts one session so its first passkey can be enrolled. An account that never enrolls one cannot sign in again on its own and needs admin recovery. Off by default, so existing deployments are unaffected. | | `LOCKOUT_POLICY` | No | `{"enabled":true,"maxFailures":10,"windowSeconds":900,"lockoutSeconds":900}` | `lockout_policy` | JSON. Set `enabled:false` only when an upstream policy handles lockout. | | `SESSION_IDLE_TTL` | No | `8h` | `session_idle_ttl` | Format `\d+[smhd]`. How long a session may go unrefreshed. The absolute session lifetime is `REFRESH_TOKEN_TTL`, measured from sign-in and not extended by refresh; this only binds while it is the shorter of the two. | diff --git a/docs/oauth.md b/docs/oauth.md index 0f2fd3a4..06bdc9e1 100644 --- a/docs/oauth.md +++ b/docs/oauth.md @@ -52,6 +52,12 @@ Example provider: 5. `POST /oauth/:providerId/callback` with `code`, `state` and the `binding` from step 2. 6. Seamless Auth validates state and its binding, exchanges the code, fetches userinfo, resolves the local user, and issues a Seamless Auth session. +OAuth is a non-passkey factor, so the passkey rules bind here as they do on every other +continuation. When the resolved account has to sign in with a passkey (phishing-resistant-only +mode, or the account holds a passkey and `passkey_login_fallback_enabled` is off), the callback +answers 403 `login_method_disabled`, records `oauth_login_failed` with reason `passkey_required`, +and issues no session. + The `binding` ties the `state` to the client that started the sign-in. The state travels through the provider and the address bar, so without it anyone holding a state and its code could finish the sign-in in another browser and sign that browser in to their own account (login CSRF). A diff --git a/docs/security-posture.md b/docs/security-posture.md index f4393303..100c1ad8 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -413,6 +413,28 @@ of one token signs its user out, which is the direction this has to fail: the al is that whoever wins the race keeps a session, and the winner is not always the client that should have it. +### Log out everywhere + +"Log out everywhere" (`DELETE /logout/all`, `DELETE /sessions`) and the admin equivalents +(`DELETE /admin/sessions/:userId/revoke-all`, device-replacement recovery, and retiring an +OAuth provider for an organization) used to read a user's sessions and then revoke them one +row at a time. A refresh landing between the read and the writes created a replacement that +nothing revoked, so the user stayed signed in on the device they were trying to remove. + +All of them now go through +[`revokeAllSessionsForUsers`](../src/services/sessionRevocation.ts), which revokes with one +`UPDATE ... WHERE user_id = ? AND revoked_at IS NULL` and first records +`users.sessions_revoked_before`. A session created at or before that time is refused by +`validateSessionRecord` and by `POST /refresh` even if its own row was never revoked, and a +refresh re-reads the cutoff after it claims the rotation, so a rotation that started before +the revoke cannot hand out a working session. It answers `401 invalid_refresh_token`, revokes +the session with `revokedReason: 'sessions_revoked'`, and records `refresh_token_failed` with +`refusal: 'sessions_revoked'`. + +The comparison uses the API's clock for both sides. Instances with skewed clocks can refuse a +session created within the skew after a revoke-all, which signs that user in again rather than +leaving a session alive. + ### Absolute and idle lifetime Rotation does not extend a session. Every session in a chain carries `chainStartedAt`, the time of diff --git a/openapi.json b/openapi.json index b5e8bc7f..54e67aff 100644 --- a/openapi.json +++ b/openapi.json @@ -8047,7 +8047,7 @@ "/oauth/{providerId}/callback": { "post": { "summary": "Finish OAuth login", - "description": "Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider. A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet.", + "description": "Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider, and 403 `login_method_disabled` when the account has to sign in with a passkey (phishing-resistant-only mode, or the account holds a passkey and `passkey_login_fallback_enabled` is off). A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet.", "tags": ["OAuth"], "parameters": [ { diff --git a/resources/coverage-badge.svg b/resources/coverage-badge.svg index c931b02b..322cf29d 100644 --- a/resources/coverage-badge.svg +++ b/resources/coverage-badge.svg @@ -1,5 +1,5 @@ - - coverage: 99.2% + + coverage: 99.1% @@ -17,7 +17,7 @@ coverage coverage - 99.2% - 99.2% + 99.1% + 99.1% diff --git a/src/controllers/admin.ts b/src/controllers/admin.ts index ddafc72e..657378a8 100644 --- a/src/controllers/admin.ts +++ b/src/controllers/admin.ts @@ -34,6 +34,7 @@ import { streamAuthEventExport } from '../services/auditExport.js'; import { verifyAuthEventChain } from '../services/auditIntegrity.js'; import { serializeAuthEvents } from '../services/authEventSerialization.js'; import { AuthEventService } from '../services/authEventService.js'; +import { revokeAllSessionsForUsers } from '../services/sessionRevocation.js'; import { hardRevokeSession } from '../services/sessionService.js'; import { importUsers as runUserImport } from '../services/userImportService.js'; import type { AuthenticatedRequest } from '../types/types.js'; @@ -506,16 +507,7 @@ export const revokeAllUserSessions = async (req: RouteRequest, res: Response) => const { userId } = req.params; try { - const sessions = await Session.findAll({ - where: { - userId, - revokedAt: null, - }, - }); - - for (const session of sessions) { - await hardRevokeSession(session, 'admin_revoke_all'); - } + const revokedSessions = await revokeAllSessionsForUsers(userId, 'admin_revoke_all'); logger.info('All sessions revoked for user'); @@ -526,7 +518,7 @@ export const revokeAllUserSessions = async (req: RouteRequest, res: Response) => req, metadata: { targetUser: userId, - revokedSessions: sessions.length, + revokedSessions, scope: 'all', }, }); @@ -596,18 +588,7 @@ export const recoverUserForDeviceReplacement = async (req: RouteRequest, res: Re let disabledTotpCredentials = 0; if (revokeSessions) { - const sessions = await Session.findAll({ - where: { - userId, - revokedAt: null, - }, - }); - - for (const session of sessions) { - await hardRevokeSession(session, 'admin_device_replacement'); - } - - revokedSessions = sessions.length; + revokedSessions = await revokeAllSessionsForUsers(userId, 'admin_device_replacement'); } if (removePasskeys) { diff --git a/src/controllers/authentication.ts b/src/controllers/authentication.ts index dac23938..f10597e7 100644 --- a/src/controllers/authentication.ts +++ b/src/controllers/authentication.ts @@ -30,6 +30,7 @@ import { LoginMethod, resolveAvailableLoginMethods, } from '../services/loginPolicyService.js'; +import { isRevokedByCutoff, revokeAllSessionsForUsers } from '../services/sessionRevocation.js'; import { claimSessionRotation, classifyExpiredRefreshToken, @@ -455,15 +456,13 @@ export const logoutAllSessions = async (req: Request, res: Response) => { logger.info('User logged out all sessions'); try { - const sessions = await Session.findAll({ where: { userId: authUser.id, revokedAt: null } }); - - await Promise.all(sessions.map((session) => hardRevokeSession(session, 'user_logout_all'))); + const revokedSessions = await revokeAllSessionsForUsers(authUser.id, 'user_logout_all'); await AuthEventService.log({ userId: authUser.id, type: 'logout_success', req, - metadata: { scope: 'all_sessions', revokedSessions: sessions.length }, + metadata: { scope: 'all_sessions', revokedSessions }, }); } catch (error) { logger.error(`Error during logout: ${error}`); @@ -473,6 +472,18 @@ export const logoutAllSessions = async (req: Request, res: Response) => { return res.json({ message: 'Success' }); }; +async function refuseRevokedRefresh(req: Request, res: Response, userId: string, session: Session) { + await hardRevokeSession(session, 'sessions_revoked'); + await AuthEventService.log({ + userId, + sessionId: session.id, + type: 'refresh_token_failed', + req, + metadata: { reason: 'Sessions revoked for user', refusal: 'sessions_revoked' }, + }); + return res.status(401).json({ error: 'invalid_refresh_token' }); +} + export const refreshSession = async (req: Request, res: Response) => { logger.info(`Refreshing user token`); @@ -570,6 +581,10 @@ export const refreshSession = async (req: Request, res: Response) => { return res.status(401).json({ error: 'invalid_session' }); } + if (isRevokedByCutoff(session, user.sessionsRevokedBefore)) { + return refuseRevokedRefresh(req, res, user.id, session); + } + const { access_token_ttl, refresh_token_ttl, session_idle_ttl } = await getSystemConfig(); const chainStartedAt = session.chainStartedAt ?? session.createdAt ?? now; const { expiresAt, idleExpiresAt } = computeRotatedSessionTimes( @@ -648,6 +663,18 @@ export const refreshSession = async (req: Request, res: Response) => { return res.status(401).json({ error: 'refresh_token_reused' }); } + // Read again because a revoke-all can land while this rotation is in flight. Its cutoff + // covers the session the rotation started from, so the replacement is refused whether + // or not the bulk update reached its row. + const owner = await User.findOne({ + where: { id: user.id, revoked: false }, + attributes: ['id', 'sessionsRevokedBefore'], + }); + + if (!owner || isRevokedByCutoff(session, owner.sessionsRevokedBefore)) { + return refuseRevokedRefresh(req, res, user.id, newSession); + } + const token = await signAccessToken(newSession.id, user.id, user.roles, session.organizationId); if (token && newRefreshToken) { diff --git a/src/controllers/oauth.ts b/src/controllers/oauth.ts index 24503743..49ae3970 100644 --- a/src/controllers/oauth.ts +++ b/src/controllers/oauth.ts @@ -9,6 +9,7 @@ import { Request, Response } from 'express'; import { getSystemConfig } from '../config/getSystemConfig.js'; import { Credential } from '../models/credentials.js'; import { AuthEventService } from '../services/authEventService.js'; +import { isPasskeyRequiredForUser } from '../services/loginPolicyService.js'; import { buildOAuthAuthorizationUrl, consumeOAuthState, @@ -153,6 +154,16 @@ export async function finishOAuthLogin(req: RouteRequest, res: Response) { return res.status(403).json({ error: 'OAuth signup is disabled' }); } + if (await isPasskeyRequiredForUser(user.id)) { + await AuthEventService.log({ + userId: user.id, + type: 'oauth_login_failed', + req, + metadata: { providerId: provider.id, reason: 'passkey_required' }, + }); + return res.status(403).json({ error: 'login_method_disabled' }); + } + await AuthEventService.log({ userId: user.id, subjectEmail: user.email, diff --git a/src/controllers/sessions.ts b/src/controllers/sessions.ts index 8dff68e2..a15257ea 100644 --- a/src/controllers/sessions.ts +++ b/src/controllers/sessions.ts @@ -9,6 +9,7 @@ import { Op } from 'sequelize'; import { Session } from '../models/sessions.js'; import { serializeSession } from '../services/apiResponseSerializers.js'; +import { revokeAllSessionsForUsers } from '../services/sessionRevocation.js'; import { hardRevokeSession } from '../services/sessionService.js'; import { AuthenticatedRequest } from '../types/types.js'; import getLogger from '../utils/logger.js'; @@ -77,13 +78,7 @@ export const revokeAllSessions = async (req: Request, res: Response) => { return res.status(401).json({ error: 'Not allowed' }); } - const sessions = await Session.findAll({ - where: { userId: user.id, revokedAt: null }, - }); - - for (const session of sessions) { - await hardRevokeSession(session, 'user_logout_all'); - } + await revokeAllSessionsForUsers(user.id, 'user_logout_all'); logger.info(`All sessions revoked for user ${user.id}`); diff --git a/src/generated/api.ts b/src/generated/api.ts index 085dfd7e..a32bdbe4 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -8063,7 +8063,7 @@ export interface paths { put?: never; /** * Finish OAuth login - * @description Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider. A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet. + * @description Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider, and 403 `login_method_disabled` when the account has to sign in with a passkey (phishing-resistant-only mode, or the account holds a passkey and `passkey_login_fallback_enabled` is off). A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet. */ post: { parameters: { diff --git a/src/migrations/20261022130000-add-user-sessions-revoked-before.cjs b/src/migrations/20261022130000-add-user-sessions-revoked-before.cjs new file mode 100644 index 00000000..86b45040 --- /dev/null +++ b/src/migrations/20261022130000-add-user-sessions-revoked-before.cjs @@ -0,0 +1,27 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the Apache License, Version 2.0 + * See LICENSE file in the project root for full license information + */ + +'use strict'; + +/** + * When every session the user held was last revoked at once. A session created at or + * before it is dead even when its own row was missed, which closes the gap a refresh + * racing "log out everywhere" used to slip through. + * + * @type {import('sequelize-cli').Migration} + */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.addColumn('users', 'sessions_revoked_before', { + type: Sequelize.DATE, + allowNull: true, + }); + }, + + async down(queryInterface) { + await queryInterface.removeColumn('users', 'sessions_revoked_before'); + }, +}; diff --git a/src/models/users.ts b/src/models/users.ts index c556e6ec..bc2aa507 100644 --- a/src/models/users.ts +++ b/src/models/users.ts @@ -27,6 +27,7 @@ export interface UserAttributes { verified?: boolean; lastLogin?: Date; enrollmentInvitedAt?: Date | null; + sessionsRevokedBefore?: Date | null; createdAt?: Date; updatedAt?: Date; credentials?: Credential[]; @@ -51,6 +52,7 @@ export class User extends Model implements UserAttributes { declare roles?: string[]; declare lastLogin?: Date; declare enrollmentInvitedAt: Date | null; + declare sessionsRevokedBefore: Date | null; declare readonly createdAt: Date; declare readonly updatedAt: Date; declare readonly credentials?: Credential[]; @@ -162,6 +164,10 @@ const initializeUserModel = (sequelize: Sequelize) => { type: DataTypes.DATE, allowNull: true, }, + sessionsRevokedBefore: { + type: DataTypes.DATE, + allowNull: true, + }, }, { sequelize, diff --git a/src/routes/oauth.routes.ts b/src/routes/oauth.routes.ts index 0c710c55..7618ece9 100644 --- a/src/routes/oauth.routes.ts +++ b/src/routes/oauth.routes.ts @@ -63,7 +63,7 @@ oauthRouter.post( adapter: { issues: 'session' }, summary: 'Finish OAuth login', description: - 'Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider. A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet.', + 'Answers 403 with `code: oauth_provider_retired` when an organization the user belongs to has retired this provider, and 403 `login_method_disabled` when the account has to sign in with a passkey (phishing-resistant-only mode, or the account holds a passkey and `passkey_login_fallback_enabled` is off). A success carries `nextStep: enroll_passkey` when the provider has `promptPasskeyEnrollment` set and the user has no passkey yet.', tags: ['OAuth'], middleware: [oauthIpLimiter, oauthProviderLimiter], schemas: { diff --git a/src/services/organizationService.ts b/src/services/organizationService.ts index 7559fec7..0deca51f 100644 --- a/src/services/organizationService.ts +++ b/src/services/organizationService.ts @@ -9,8 +9,8 @@ import { Op, UniqueConstraintError, WhereOptions } from 'sequelize'; import { hasScopedRole } from '../lib/scopedRoles.js'; import { OrganizationMembership } from '../models/organizationMemberships.js'; import { Organization } from '../models/organizations.js'; -import { Session } from '../models/sessions.js'; import { User } from '../models/users.js'; +import { revokeAllSessionsForUsers } from './sessionRevocation.js'; export type OrganizationRole = 'owner' | 'admin' | 'member'; @@ -406,9 +406,10 @@ export async function setOAuthProviderRetired( const userIds = memberships.map((membership) => membership.userId); if (userIds.length > 0) { - [revokedSessions] = await Session.update( - { revokedAt: new Date(), revokedReason: 'oauth_provider_retired' }, - { where: { userId: userIds, revokedAt: null }, transaction }, + revokedSessions = await revokeAllSessionsForUsers( + userIds, + 'oauth_provider_retired', + transaction, ); } } diff --git a/src/services/sessionIssuance.ts b/src/services/sessionIssuance.ts index 4e571e54..dc4b1a51 100644 --- a/src/services/sessionIssuance.ts +++ b/src/services/sessionIssuance.ts @@ -11,7 +11,7 @@ import { createRefreshTokenLookup, generateRefreshToken, signAccessToken } from import { Session } from '../models/sessions.js'; import { computeSessionTimes, parseDurationToSeconds } from '../utils/utils.js'; import { enforceConcurrentSessionLimit } from './concurrentSessionPolicy.js'; -import { getLoginPolicy } from './loginPolicyService.js'; +import { isPasskeyRequiredForUser } from './loginPolicyService.js'; import { getDefaultOrganizationIdForUser } from './organizationService.js'; /** The factor that proved the user, as far as session issuance needs to know it. */ @@ -19,7 +19,7 @@ export type SessionMethod = 'passkey' | 'email_otp' | 'phone_otp' | 'magic_link' export class PasskeyRequiredError extends Error { constructor(method: SessionMethod) { - super(`Refused to issue a session from ${method} in phishing-resistant-only mode`); + super(`Refused to issue a session from ${method}: this sign-in has to be a passkey`); this.name = 'PasskeyRequiredError'; } } @@ -54,13 +54,12 @@ type IssueSessionParams = { export async function issueSessionAndRespond(params: IssueSessionParams): Promise { const { user, req, res, extraFields, method, accountVerification } = params; - // Every endpoint that reaches here with another factor refuses first. This is the - // backstop for one that does not, so it fails closed rather than trusting the gates. - if (method !== 'passkey' && !accountVerification) { - const { phishingResistantOnly } = await getLoginPolicy(); - if (phishingResistantOnly) { - throw new PasskeyRequiredError(method); - } + // Every endpoint that reaches here with another factor refuses first, with a 403. This + // is the backstop for one that does not, so it fails closed rather than trusting the + // gates. It covers both rules, phishing-resistant-only mode and an account that holds a + // passkey while fallback is off, because OAuth once shipped without the second. + if (method !== 'passkey' && !accountVerification && (await isPasskeyRequiredForUser(user.id))) { + throw new PasskeyRequiredError(method); } const refreshToken = generateRefreshToken(); diff --git a/src/services/sessionRevocation.ts b/src/services/sessionRevocation.ts new file mode 100644 index 00000000..5762d60c --- /dev/null +++ b/src/services/sessionRevocation.ts @@ -0,0 +1,55 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the Apache License, Version 2.0 + * See LICENSE file in the project root for full license information + */ + +import type { Transaction } from 'sequelize'; + +import { Session } from '../models/sessions.js'; +import { User } from '../models/users.js'; + +/** + * Ends every session the given users hold, including one a refresh is creating right now. + * + * Reading the sessions and revoking them row by row left a gap: a refresh landing between + * the read and the writes created a replacement nothing revoked. The bulk update closes + * most of that, and the per-user cutoff closes the rest, because a session created at or + * before it is refused wherever a session is accepted, whether or not its row was caught. + * + * The cutoff is written first so there is no moment where the rows are revoked and a + * session minted from one of them still passes. + */ +export async function revokeAllSessionsForUsers( + userIds: string | string[], + reason: string, + transaction?: Transaction, +): Promise { + const revokedAt = new Date(); + + await User.update( + { sessionsRevokedBefore: revokedAt }, + { where: { id: userIds }, ...(transaction ? { transaction } : {}) }, + ); + + const [revoked] = await Session.update( + { revokedAt, revokedReason: reason }, + { where: { userId: userIds, revokedAt: null }, ...(transaction ? { transaction } : {}) }, + ); + + return revoked; +} + +/** + * Whether a revoke-all for the session's owner happened at or after the session began. + * A session with no creation time is treated as revoked, so a malformed row fails closed. + */ +export function isRevokedByCutoff( + session: Pick, + sessionsRevokedBefore: Date | null | undefined, +): boolean { + if (!sessionsRevokedBefore) return false; + if (!session.createdAt) return true; + + return new Date(session.createdAt).getTime() <= new Date(sessionsRevokedBefore).getTime(); +} diff --git a/src/services/sessionService.ts b/src/services/sessionService.ts index 20a31da3..3f9355e0 100644 --- a/src/services/sessionService.ts +++ b/src/services/sessionService.ts @@ -14,6 +14,7 @@ import { User } from '../models/users.js'; import getLogger from '../utils/logger.js'; import { getPublicKeyByKid } from '../utils/signingKeyStore.js'; import { decoyPrincipalAsUser, decoyPrincipalForSubject } from './decoyPrincipal.js'; +import { isRevokedByCutoff } from './sessionRevocation.js'; const logger = getLogger('sessionService'); @@ -247,6 +248,13 @@ export async function validateSessionRecord(sessionId: string) { if (session.expiresAt < now) return null; if (session.idleExpiresAt < now) return null; + // Catches a session that a revoke-all missed because a refresh created it mid-revoke. + const owner = await User.findOne({ + where: { id: session.userId }, + attributes: ['id', 'sessionsRevokedBefore'], + }); + if (!owner || isRevokedByCutoff(session, owner.sessionsRevokedBefore)) return null; + return session; } diff --git a/tests/integration/admin/admin.spec.ts b/tests/integration/admin/admin.spec.ts index 79400cb0..5555b36f 100644 --- a/tests/integration/admin/admin.spec.ts +++ b/tests/integration/admin/admin.spec.ts @@ -227,12 +227,21 @@ describe('DELETE /admin/sessions/by-id/:id', () => { }); describe('DELETE /admin/sessions/:userId/revoke-all', () => { - it('revokes all sessions', async () => { - (Session.findAll as any).mockResolvedValue([{ id: 's1' }, { id: 's2' }]); + it('revokes all sessions in one update and records the cutoff', async () => { + (Session.update as any).mockResolvedValue([2]); const res = await request(app).delete(`/admin/sessions/${testGuid}/revoke-all`); expect(res.status).toBe(200); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: testGuid } }, + ); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'admin_revoke_all' }, + { where: { userId: testGuid, revokedAt: null } }, + ); + expect(Session.findAll).not.toHaveBeenCalled(); }); }); @@ -240,14 +249,13 @@ const validProofing = { method: 'in_person', evidenceRef: 'TICKET-1042' }; describe('POST /admin/users/:userId/recovery/device-replacement', () => { it('revokes sessions, removes passkeys, and disables TOTP for device replacement', async () => { - const sessions = [buildSession({ id: 's1' }), buildSession({ id: 's2' })]; const credentials = [buildCredential({ id: 'cred-1' }), buildCredential({ id: 'cred-2' })]; (Session.findOne as any).mockResolvedValue( buildSession({ stepUpVerifiedAt: new Date(), stepUpMethod: 'webauthn' }), ); (User.findByPk as any).mockResolvedValue(buildUser({ id: testGuid })); - (Session.findAll as any).mockResolvedValue(sessions); + (Session.update as any).mockResolvedValue([2]); (Credential.findAll as any).mockResolvedValue(credentials); (TotpCredential.update as any).mockResolvedValue([1]); @@ -256,7 +264,14 @@ describe('POST /admin/users/:userId/recovery/device-replacement', () => { .send({ proofing: validProofing }); expect(res.status).toBe(200); - expect(hardRevokeSession).toHaveBeenCalledTimes(2); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'admin_device_replacement' }, + { where: { userId: testGuid, revokedAt: null } }, + ); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: testGuid } }, + ); expect(credentials[0].destroy).toHaveBeenCalled(); expect(credentials[1].destroy).toHaveBeenCalled(); expect(TotpCredential.update).toHaveBeenCalledWith( @@ -389,10 +404,7 @@ describe('admin actions are attributed', () => { }); it('records a bulk session revoke', async () => { - (Session.findAll as any).mockResolvedValue([ - buildSession({ id: 's1' }), - buildSession({ id: 's2' }), - ]); + (Session.update as any).mockResolvedValue([2]); const res = await request(app).delete(`/admin/sessions/${testGuid}/revoke-all`); @@ -1189,7 +1201,7 @@ describe('admin session failure paths', () => { }); it('returns 500 when revoking all sessions fails', async () => { - (Session.findAll as any).mockRejectedValue(new Error('boom')); + (Session.update as any).mockRejectedValue(new Error('boom')); const res = await request(app).delete(`/admin/sessions/${testGuid}/revoke-all`); diff --git a/tests/integration/authentication/authentication.spec.ts b/tests/integration/authentication/authentication.spec.ts index 215c0a26..1dd747e2 100644 --- a/tests/integration/authentication/authentication.spec.ts +++ b/tests/integration/authentication/authentication.spec.ts @@ -540,22 +540,32 @@ describe('DELETE /logout', () => { }); describe('DELETE /logout/all', () => { - it('logs out all user sessions', async () => { - const sessions = [{ id: 'session-1' }, { id: 'session-2' }]; - (Session.findAll as any).mockResolvedValue(sessions); + it('logs out all user sessions in one update and records the cutoff', async () => { + (Session.update as any).mockResolvedValue([2]); const res = await request(app).delete('/logout/all'); expect(res.status).toBe(200); expect(res.body.message).toBe('Success'); - expect(Session.findAll).toHaveBeenCalledWith({ - where: { userId: expect.any(String), revokedAt: null }, - }); - expect(hardRevokeSession).toHaveBeenCalledTimes(2); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'user_logout_all' }, + { where: { userId: expect.any(String), revokedAt: null } }, + ); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: expect.any(String) } }, + ); + expect(Session.findAll).not.toHaveBeenCalled(); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'logout_success', + metadata: { scope: 'all_sessions', revokedSessions: 2 }, + }), + ); }); it('still succeeds when revoking all sessions throws', async () => { - (Session.findAll as any).mockRejectedValue(new Error('db down')); + (Session.update as any).mockRejectedValue(new Error('db down')); const res = await request(app).delete('/logout/all'); @@ -793,3 +803,156 @@ describe('POST /refresh', () => { expect(res.body.refreshTtl).toBe(86400); }); }); + +describe('POST /refresh racing DELETE /logout/all', () => { + type Row = { + id: string; + userId: string; + infraId: string; + mode: 'server'; + userAgent: string; + organizationId: string | null; + chainStartedAt: Date; + createdAt: Date; + revokedAt: Date | null; + revokedReason: string | null; + replacedBySessionId: string | null; + save: () => Promise; + reload: () => Promise; + }; + + const owner = buildUser(); + let rows: Row[]; + let cutoff: Date | null; + + function row(id: string, createdAt: Date): Row { + return { + id, + userId: owner.id, + infraId: 'app', + mode: 'server', + userAgent: 'agent', + organizationId: null, + chainStartedAt: createdAt, + createdAt, + revokedAt: null, + revokedReason: null, + replacedBySessionId: null, + save: async () => {}, + reload: async () => {}, + }; + } + + const liveRows = () => rows.filter((candidate) => candidate.revokedAt === null); + + // A store the handlers share, so the revoke-all runs against the rows the refresh is + // writing, at the point in the rotation each case names. + function installStore(raceAt: 'before_replacement' | 'after_claim') { + rows = [row('session-old', new Date(Date.now() - 60_000))]; + cutoff = null; + + const revokeAll = async () => { + const res = await request(app).delete('/logout/all'); + expect(res.status).toBe(200); + }; + const matches = (candidate: Row, where: Record) => + candidate.userId === where.userId && candidate.revokedAt === where.revokedAt; + + (findRefreshSessionByToken as any).mockImplementation(async () => rows[0]); + (User.findOne as any).mockImplementation(async () => ({ + ...owner, + sessionsRevokedBefore: cutoff, + })); + (User.update as any).mockImplementation(async (values: any) => { + cutoff = values.sessionsRevokedBefore; + return [1]; + }); + (Session.findAll as any).mockImplementation(async ({ where }: any) => + rows.filter((candidate) => matches(candidate, where)), + ); + (Session.update as any).mockImplementation(async (values: any, { where }: any) => { + const hit = rows.filter((candidate) => matches(candidate, where)); + hit.forEach((candidate) => Object.assign(candidate, values)); + return [hit.length]; + }); + (Session.create as any).mockImplementation(async () => { + if (raceAt === 'before_replacement') await revokeAll(); + const created = row('session-new', new Date()); + rows.push(created); + return created; + }); + (claimSessionRotation as any).mockImplementation(async (session: Row, replacement: string) => { + if (session.replacedBySessionId || session.revokedAt) return false; + session.replacedBySessionId = replacement; + if (raceAt === 'after_claim') await revokeAll(); + return true; + }); + (hardRevokeSession as any).mockImplementation(async (session: Row, reason: string) => { + session.revokedAt = new Date(); + session.revokedReason = reason; + }); + (revokeSessionChain as any).mockImplementation(async (session: Row) => { + let current: Row | undefined = session; + while (current) { + current.revokedAt ??= new Date(); + const next: string | null = current.replacedBySessionId; + current = rows.find((candidate) => candidate.id === next); + } + }); + (signAccessToken as any).mockResolvedValue('access'); + (generateRefreshToken as any).mockReturnValue('refresh'); + (createRefreshTokenLookup as any).mockReturnValue('refresh-lookup'); + (getSystemConfig as any).mockResolvedValue({ + access_token_ttl: '15m', + refresh_token_ttl: '1h', + session_idle_ttl: '8h', + }); + } + + it.each(['before_replacement', 'after_claim'] as const)( + 'leaves no live session when the revoke lands %s', + async (raceAt) => { + installStore(raceAt); + + const res = await request(app).post('/refresh').set('Authorization', 'Bearer refresh-token'); + + expect(res.status).toBe(401); + expect(res.body.refreshToken).toBeUndefined(); + expect(rows).toHaveLength(2); + expect(liveRows()).toEqual([]); + }, + ); + + it('refuses a session the revoke-all missed, by its cutoff', async () => { + installStore('after_claim'); + cutoff = new Date(); + + const res = await request(app).post('/refresh').set('Authorization', 'Bearer refresh-token'); + + expect(res.status).toBe(401); + expect(res.body.error).toBe('invalid_refresh_token'); + expect(Session.create).not.toHaveBeenCalled(); + expect(hardRevokeSession).toHaveBeenCalledWith(rows[0], 'sessions_revoked'); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'refresh_token_failed', + metadata: expect.objectContaining({ refusal: 'sessions_revoked' }), + }), + ); + }); + + it('refuses the rotation when the owner is gone by the time it completes', async () => { + installStore('before_replacement'); + (Session.create as any).mockImplementation(async () => { + const created = row('session-new', new Date()); + rows.push(created); + (User.findOne as any).mockResolvedValue(null); + return created; + }); + + const res = await request(app).post('/refresh').set('Authorization', 'Bearer refresh-token'); + + expect(res.status).toBe(401); + expect(hardRevokeSession).toHaveBeenCalledWith(rows[1], 'sessions_revoked'); + }); +}); diff --git a/tests/integration/oauth/oauth.spec.ts b/tests/integration/oauth/oauth.spec.ts index 58c8b4b5..06b1691f 100644 --- a/tests/integration/oauth/oauth.spec.ts +++ b/tests/integration/oauth/oauth.spec.ts @@ -707,6 +707,51 @@ describe('OAuth routes', () => { expect(Credential.count).not.toHaveBeenCalled(); }); + it('refuses an account that holds a passkey when passkey fallback is off', async () => { + (getSystemConfig as any).mockResolvedValue( + buildSystemConfig({ + login_methods: ['passkey', 'oauth'], + passkey_login_fallback_enabled: false, + oauth_providers: [provider], + }), + ); + (User.findOne as any).mockResolvedValue(buildUser({ id: 'user-1', phone: null })); + (Credential.count as any).mockResolvedValue(1); + + const res = await finishGoogleSignIn(); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'login_method_disabled' }); + expect(Session.create).not.toHaveBeenCalled(); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'oauth_login_failed', + userId: 'user-1', + metadata: expect.objectContaining({ reason: 'passkey_required' }), + }), + ); + expect(AuthEventService.log).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'oauth_login_success' }), + ); + }); + + it('signs in an account with no passkey when passkey fallback is off', async () => { + (getSystemConfig as any).mockResolvedValue( + buildSystemConfig({ + login_methods: ['passkey', 'oauth'], + passkey_login_fallback_enabled: false, + oauth_providers: [provider], + }), + ); + (User.findOne as any).mockResolvedValue(buildUser({ id: 'user-1', phone: null })); + (Credential.count as any).mockResolvedValue(0); + + const res = await finishGoogleSignIn(); + + expect(res.status).toBe(200); + expect(res.body.token).toBe('access-token'); + }); + it('refuses a member of an organization that retired the provider, before linking', async () => { const user = buildUser({ id: 'user-1', phone: null, verified: false }); (User.findOne as any).mockResolvedValue(user); diff --git a/tests/integration/organizations/organizations.spec.ts b/tests/integration/organizations/organizations.spec.ts index 65d67cdb..025b3b57 100644 --- a/tests/integration/organizations/organizations.spec.ts +++ b/tests/integration/organizations/organizations.spec.ts @@ -717,6 +717,13 @@ describe('admin OAuth provider retirement', () => { transaction: expect.anything(), }), ); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + expect.objectContaining({ + where: { id: ['user-1', otherUserId] }, + transaction: expect.anything(), + }), + ); expect(AuthEventService.log).toHaveBeenCalledWith( expect.objectContaining({ type: 'admin_oauth_provider_retired', diff --git a/tests/integration/session/session.security.spec.ts b/tests/integration/session/session.security.spec.ts index c38698fb..580997e0 100644 --- a/tests/integration/session/session.security.spec.ts +++ b/tests/integration/session/session.security.spec.ts @@ -4,6 +4,7 @@ import { Application } from 'express'; import { createApp } from '../../../src/app'; import { Session } from '../../../src/models/sessions.js'; +import { User } from '../../../src/models/users.js'; import { hardRevokeSession } from '../../../src/services/sessionService.js'; import { buildSession } from '../../factories/sessionFactory.js'; @@ -106,15 +107,21 @@ describe('Session Security - Revocation', () => { expect(hardRevokeSession).toHaveBeenCalledWith(session, 'user_revoked'); }); - it('revokes all active sessions', async () => { - const sessions = [buildSession({ id: '1' }), buildSession({ id: '2' })]; - - (Session.findAll as any).mockResolvedValue(sessions); + it('revokes all active sessions in one update and records the cutoff', async () => { + (Session.update as any).mockResolvedValue([2]); const res = await request(app).delete('/sessions'); expect(res.status).toBe(200); - expect(hardRevokeSession).toHaveBeenCalledTimes(2); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'user_logout_all' }, + { where: { userId: expect.any(String), revokedAt: null } }, + ); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: expect.any(String) } }, + ); + expect(Session.findAll).not.toHaveBeenCalled(); }); it('handles no sessions safely', async () => { diff --git a/tests/integration/session/session.spec.ts b/tests/integration/session/session.spec.ts index 138008dd..1760a0fb 100644 --- a/tests/integration/session/session.spec.ts +++ b/tests/integration/session/session.spec.ts @@ -5,6 +5,7 @@ import { Application } from 'express'; import { Op } from 'sequelize'; import { Session } from '../../../src/models/sessions.js'; +import { User } from '../../../src/models/users.js'; import { hardRevokeSession } from '../../../src/services/sessionService.js'; import { buildSession } from '../../factories/sessionFactory.js'; @@ -78,15 +79,21 @@ describe('DELETE /sessions/:id', () => { }); describe('DELETE /sessions', () => { - it('revokes all sessions', async () => { - const sessions = [buildSession({ id: '1' }), buildSession({ id: '2' })]; - - (Session.findAll as any).mockResolvedValue(sessions); + it('revokes all sessions in one update and records the cutoff', async () => { + (Session.update as any).mockResolvedValue([2]); const res = await request(app).delete('/sessions'); expect(res.status).toBe(200); - expect(hardRevokeSession).toHaveBeenCalledTimes(2); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'user_logout_all' }, + { where: { userId: expect.any(String), revokedAt: null } }, + ); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: expect.any(String) } }, + ); + expect(Session.findAll).not.toHaveBeenCalled(); }); it('handles no sessions gracefully', async () => { diff --git a/tests/postgres/sessionRevocation.spec.ts b/tests/postgres/sessionRevocation.spec.ts new file mode 100644 index 00000000..14806b9a --- /dev/null +++ b/tests/postgres/sessionRevocation.spec.ts @@ -0,0 +1,88 @@ +import { randomUUID } from 'crypto'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { getSequelize, initializeModels } from '../../src/models/index.js'; +import { revokeAllSessionsForUsers } from '../../src/services/sessionRevocation.js'; +import { claimSessionRotation, validateSessionRecord } from '../../src/services/sessionService.js'; + +type Models = Awaited>; + +let Session: Models['Session']; +let User: Models['User']; + +async function createUser() { + const id = randomUUID(); + await User.create({ id, email: `revoke-${id}@example.com`, phone: null }); + return id; +} + +function createSession(userId: string) { + const now = Date.now(); + return Session.create({ + userId, + mode: 'server', + refreshTokenLookup: randomUUID(), + expiresAt: new Date(now + 3_600_000), + idleExpiresAt: new Date(now + 3_600_000), + lastUsedAt: new Date(now), + }); +} + +beforeAll(async () => { + ({ Session, User } = await initializeModels()); +}); + +afterAll(async () => { + await getSequelize().close(); +}); + +describe('revoking every session for a user', () => { + it('revokes every live row in one update and records the cutoff', async () => { + const userId = await createUser(); + const other = await createUser(); + const sessions = await Promise.all([createSession(userId), createSession(userId)]); + const untouched = await createSession(other); + + const revoked = await revokeAllSessionsForUsers(userId, 'user_logout_all'); + + expect(revoked).toBe(2); + for (const session of sessions) { + await session.reload(); + expect(session.revokedReason).toBe('user_logout_all'); + } + await untouched.reload(); + expect(untouched.revokedAt).toBeNull(); + + const user = await User.findByPk(userId); + expect(user?.sessionsRevokedBefore).toBeInstanceOf(Date); + }); + + it('refuses a session the update missed and accepts one started afterwards', async () => { + const userId = await createUser(); + const missed = await createSession(userId); + + await revokeAllSessionsForUsers(userId, 'user_logout_all'); + // Stands in for a row a refresh wrote while the revoke ran, which the update never saw. + await Session.update({ revokedAt: null, revokedReason: null }, { where: { id: missed.id } }); + + expect(await validateSessionRecord(missed.id)).toBeNull(); + + await new Promise((resolve) => setTimeout(resolve, 5)); + const later = await createSession(userId); + expect(await validateSessionRecord(later.id)).not.toBeNull(); + }); + + it('leaves a rotation that races the revoke unable to claim its session', async () => { + const userId = await createUser(); + const current = await createSession(userId); + + const [replacement] = await Promise.all([ + createSession(userId), + revokeAllSessionsForUsers(userId, 'user_logout_all'), + ]); + const claimed = await claimSessionRotation(current, replacement.id); + + expect(claimed).toBe(false); + expect(await validateSessionRecord(current.id)).toBeNull(); + }); +}); diff --git a/tests/unit/services/sessionIssueService.spec.ts b/tests/unit/services/sessionIssueService.spec.ts index 872bdda0..695e68c2 100644 --- a/tests/unit/services/sessionIssueService.spec.ts +++ b/tests/unit/services/sessionIssueService.spec.ts @@ -35,6 +35,7 @@ vi.mock('../../../src/utils/utils.js', () => ({ })); import { getSystemConfig } from '../../../src/config/getSystemConfig.js'; +import { Credential } from '../../../src/models/credentials.js'; import { createRefreshTokenLookup, generateRefreshToken, @@ -152,6 +153,57 @@ describe('issueSessionAndRespond', () => { ); }); + describe('with passkey fallback off', () => { + beforeEach(() => { + (getSystemConfig as any).mockResolvedValue({ + access_token_ttl: '15m', + refresh_token_ttl: '1h', + session_idle_ttl: '8h', + login_methods: ['passkey', 'email_otp', 'oauth'], + passkey_login_fallback_enabled: false, + }); + }); + + it.each(['email_otp', 'phone_otp', 'magic_link', 'totp', 'oauth'] as const)( + 'refuses to start a session from %s for an account that holds a passkey', + async (method) => { + (Credential.count as any).mockResolvedValue(1); + + await expect( + issueSessionAndRespond({ method, user: mockUser, req: mockReq(), res: mockRes() }), + ).rejects.toBeInstanceOf(PasskeyRequiredError); + + expect(Credential.count).toHaveBeenCalledWith({ where: { userId: mockUser.id } }); + expect(Session.create).not.toHaveBeenCalled(); + }, + ); + + it('starts a session from another factor for an account with no passkey', async () => { + (Credential.count as any).mockResolvedValue(0); + + await issueSessionAndRespond({ + method: 'oauth', + user: mockUser, + req: mockReq(), + res: mockRes(), + }); + + expect(Session.create).toHaveBeenCalled(); + }); + + it('starts a session from a passkey without counting credentials', async () => { + await issueSessionAndRespond({ + method: 'passkey', + user: mockUser, + req: mockReq(), + res: mockRes(), + }); + + expect(Credential.count).not.toHaveBeenCalled(); + expect(Session.create).toHaveBeenCalled(); + }); + }); + describe('in phishing-resistant-only mode', () => { beforeEach(() => { (getSystemConfig as any).mockResolvedValue({ diff --git a/tests/unit/services/sessionRevocation.spec.ts b/tests/unit/services/sessionRevocation.spec.ts new file mode 100644 index 00000000..81dbade3 --- /dev/null +++ b/tests/unit/services/sessionRevocation.spec.ts @@ -0,0 +1,71 @@ +import { beforeEach, describe, expect, it } from 'vitest'; + +import { Session } from '../../../src/models/sessions.js'; +import { User } from '../../../src/models/users.js'; +import { + isRevokedByCutoff, + revokeAllSessionsForUsers, +} from '../../../src/services/sessionRevocation.js'; + +describe('revokeAllSessionsForUsers', () => { + beforeEach(() => { + (Session.update as any).mockResolvedValue([3]); + }); + + it('writes the cutoff, then revokes every live session in one update', async () => { + const revoked = await revokeAllSessionsForUsers('user-1', 'user_logout_all'); + + expect(revoked).toBe(3); + expect(User.update).toHaveBeenCalledWith( + { sessionsRevokedBefore: expect.any(Date) }, + { where: { id: 'user-1' } }, + ); + expect(Session.update).toHaveBeenCalledWith( + { revokedAt: expect.any(Date), revokedReason: 'user_logout_all' }, + { where: { userId: 'user-1', revokedAt: null } }, + ); + + const [{ sessionsRevokedBefore }] = (User.update as any).mock.calls[0]; + const [{ revokedAt }] = (Session.update as any).mock.calls[0]; + expect(revokedAt).toBe(sessionsRevokedBefore); + expect((User.update as any).mock.invocationCallOrder[0]).toBeLessThan( + (Session.update as any).mock.invocationCallOrder[0], + ); + }); + + it('runs both writes inside the caller transaction', async () => { + const transaction = {} as any; + + await revokeAllSessionsForUsers(['user-1', 'user-2'], 'oauth_provider_retired', transaction); + + expect(User.update).toHaveBeenCalledWith(expect.anything(), { + where: { id: ['user-1', 'user-2'] }, + transaction, + }); + expect(Session.update).toHaveBeenCalledWith(expect.anything(), { + where: { userId: ['user-1', 'user-2'], revokedAt: null }, + transaction, + }); + }); +}); + +describe('isRevokedByCutoff', () => { + const cutoff = new Date('2026-10-22T12:00:00Z'); + + it('is false when the user has never revoked everything', () => { + expect(isRevokedByCutoff({ createdAt: new Date(0) }, null)).toBe(false); + }); + + it('is true for a session created at or before the cutoff', () => { + expect(isRevokedByCutoff({ createdAt: new Date(cutoff.getTime() - 1) }, cutoff)).toBe(true); + expect(isRevokedByCutoff({ createdAt: cutoff }, cutoff)).toBe(true); + }); + + it('is false for a session created after the cutoff', () => { + expect(isRevokedByCutoff({ createdAt: new Date(cutoff.getTime() + 1) }, cutoff)).toBe(false); + }); + + it('fails closed for a session with no creation time', () => { + expect(isRevokedByCutoff({} as any, cutoff)).toBe(true); + }); +}); diff --git a/tests/unit/services/sessionService.spec.ts b/tests/unit/services/sessionService.spec.ts index c75ee3f4..48031c47 100644 --- a/tests/unit/services/sessionService.spec.ts +++ b/tests/unit/services/sessionService.spec.ts @@ -245,16 +245,66 @@ describe('sessionService', () => { it('returns session if valid', async () => { const { Session } = await import('../../../src/models/sessions'); + const { User } = await import('../../../src/models/users'); - const session = buildSession(); + const session = buildSession({ userId: 'user-1' }); (Session.findByPk as any).mockResolvedValue(session); + (User.findOne as any).mockResolvedValue({ id: 'user-1', sessionsRevokedBefore: null }); const { validateSessionRecord } = await import('../../../src/services/sessionService'); const result = await validateSessionRecord('id'); expect(result).toBe(session); + expect(User.findOne).toHaveBeenCalledWith({ + where: { id: 'user-1' }, + attributes: ['id', 'sessionsRevokedBefore'], + }); + }); + + describe('after a revoke-all', () => { + const revokedAt = new Date('2026-10-22T12:00:00Z'); + + async function validate(createdAt: Date | undefined) { + const { Session } = await import('../../../src/models/sessions'); + const { User } = await import('../../../src/models/users'); + + (Session.findByPk as any).mockResolvedValue(buildSession({ userId: 'user-1', createdAt })); + (User.findOne as any).mockResolvedValue({ id: 'user-1', sessionsRevokedBefore: revokedAt }); + + const { validateSessionRecord } = await import('../../../src/services/sessionService'); + + return validateSessionRecord('id'); + } + + it('refuses a session created before the cutoff that the revoke missed', async () => { + expect(await validate(new Date(revokedAt.getTime() - 1))).toBeNull(); + }); + + it('refuses a session created at the cutoff', async () => { + expect(await validate(revokedAt)).toBeNull(); + }); + + it('refuses a session with no creation time', async () => { + expect(await validate(undefined)).toBeNull(); + }); + + it('accepts a session started after the cutoff', async () => { + expect(await validate(new Date(revokedAt.getTime() + 1))).not.toBeNull(); + }); + }); + + it('refuses a session whose owner no longer exists', async () => { + const { Session } = await import('../../../src/models/sessions'); + const { User } = await import('../../../src/models/users'); + + (Session.findByPk as any).mockResolvedValue(buildSession({ userId: 'user-1' })); + (User.findOne as any).mockResolvedValue(null); + + const { validateSessionRecord } = await import('../../../src/services/sessionService'); + + expect(await validateSessionRecord('id')).toBeNull(); }); // The keyed fingerprint is what authenticates the token, so a row whose stored value @@ -542,12 +592,16 @@ describe('sessionService', () => { }); (Session.findByPk as any).mockResolvedValue(buildSession({ id: 'session-1', userId: 'other' })); + const { User } = await import('../../../src/models/users'); + (User.findOne as any).mockResolvedValue({ id: 'other', sessionsRevokedBefore: null }); const { validateBearerToken } = await import('../../../src/services/sessionService'); const result = await validateBearerToken('token'); expect(result).toBeNull(); + // Refused on the subject check, not earlier for want of an owner row. + expect(User.findOne).toHaveBeenCalledTimes(1); }); it('returns user when ephemeral bearer token is valid', async () => { @@ -738,11 +792,15 @@ describe('sessionService', () => { payload: { typ: 'access', sub: 'user', sid: 'session-1' }, }); (Session.findByPk as any).mockResolvedValue(buildSession({ id: 'session-1', userId: 'user' })); - (User.findOne as any).mockResolvedValue(null); + // The cutoff lookup finds the row; the revoked-user filter in getUserFromSession does not. + (User.findOne as any) + .mockResolvedValueOnce({ id: 'user', sessionsRevokedBefore: null }) + .mockResolvedValueOnce(null); const { validateBearerToken } = await import('../../../src/services/sessionService'); expect(await validateBearerToken('token')).toBeNull(); + expect(User.findOne).toHaveBeenCalledTimes(2); }); it('returns null when the access token session record is missing', async () => {