diff --git a/.changeset/oidc-session-bind.md b/.changeset/oidc-session-bind.md new file mode 100644 index 00000000..b5ffea86 --- /dev/null +++ b/.changeset/oidc-session-bind.md @@ -0,0 +1,7 @@ +--- +'seamless-auth-api': minor +--- + +The API now owns which sessions a provider sign-in was made on. The provider service binds a session to a client with `POST /oidc/sessions/:id/bind`, presenting the user's own access token for that session, and the API answers account and auth context lookups, signs provider tokens, and accepts a session revoke only for bound sessions. Account notices need the same binding. Device-flow tokens no longer carry `amr` or `auth_time`, as they already did not carry `acr`. + +This raises the provider service contract to version 3, so `seamless-auth-oidc-provider` must be upgraded with the API. Sessions signed in to provider clients before the upgrade need to sign in again to get new provider tokens. diff --git a/openapi.json b/openapi.json index 2c74d8d2..0dd2578e 100644 --- a/openapi.json +++ b/openapi.json @@ -13481,6 +13481,143 @@ } } }, + "/oidc/sessions/{id}/bind": { + "post": { + "summary": "Bind a session to the client a provider sign-in is for", + "tags": ["OpenID Provider service"], + "security": [{ "serviceToken": [] }], + "parameters": [ + { + "schema": { "type": "string", "minLength": 1, "maxLength": 512 }, + "required": true, + "name": "id", + "in": "path" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "clientId": { "type": "string", "minLength": 1, "maxLength": 512 }, + "accessToken": { "type": "string", "minLength": 1, "maxLength": 8192 } + }, + "required": ["clientId", "accessToken"] + } + } + } + }, + "responses": { + "400": { + "description": "HTTP 400", + "content": { + "application/json": { + "example": { + "error": "string", + "message": "string", + "details": { "issues": [null] } + }, + "schema": { + "type": "object", + "properties": { + "error": { "type": "string" }, + "message": { "type": "string" }, + "details": { + "type": "object", + "properties": { + "issues": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": { + "type": "array", + "items": { "anyOf": [{ "type": "string" }, { "type": "number" }] } + }, + "code": { "type": "string" }, + "message": { "type": "string" } + }, + "required": ["path", "code", "message"] + } + } + }, + "required": ["issues"] + } + }, + "required": ["error"] + } + } + } + }, + "401": { + "description": "HTTP 401", + "content": { + "application/json": { + "example": { "message": "string", "error": "string" }, + "schema": { + "type": "object", + "properties": { "message": { "type": "string" }, "error": { "type": "string" } }, + "required": ["error"] + } + } + } + }, + "403": { + "description": "HTTP 403", + "content": { + "application/json": { + "example": { "message": "string", "error": "string" }, + "schema": { + "type": "object", + "properties": { "message": { "type": "string" }, "error": { "type": "string" } }, + "required": ["error"] + } + } + } + }, + "404": { + "description": "HTTP 404", + "content": { + "application/json": { + "example": { "message": "string", "error": "string" }, + "schema": { + "type": "object", + "properties": { "message": { "type": "string" }, "error": { "type": "string" } }, + "required": ["error"] + } + } + } + }, + "429": { + "description": "HTTP 429", + "content": { + "application/json": { + "example": { "message": "string", "error": "string" }, + "schema": { + "type": "object", + "properties": { "message": { "type": "string" }, "error": { "type": "string" } }, + "required": ["error"] + } + } + } + }, + "500": { + "description": "HTTP 500", + "content": { + "application/json": { + "example": { "message": "string", "error": "string" }, + "schema": { + "type": "object", + "properties": { "message": { "type": "string" }, "error": { "type": "string" } }, + "required": ["error"] + } + } + } + } + } + } + }, "/oidc/sessions/{id}/revoke": { "post": { "summary": "Revoke the session behind a provider sign-in", diff --git a/package-lock.json b/package-lock.json index e7552d12..5cb27ea5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,19 +1,19 @@ { "name": "seamless-auth-api", - "version": "0.21.0", + "version": "0.21.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "seamless-auth-api", - "version": "0.21.0", + "version": "0.21.1", "license": "Apache-2.0", "dependencies": { "@asteasolutions/zod-to-openapi": "^9.1.0", "@seamless-auth/messaging": "^0.3.1", "@seamless-auth/messaging-aws": "^0.3.1", "@seamless-auth/messaging-twilio": "^0.3.1", - "@seamless-auth/types": "^0.34.0", + "@seamless-auth/types": "^0.35.0", "@simplewebauthn/server": "^14.0.3", "base64url": "^3.0.1", "bcrypt-ts": "^9.0.2", @@ -2914,7 +2914,7 @@ "node": ">=18" } }, - "node_modules/@seamless-auth/types": { + "node_modules/@seamless-auth/messaging/node_modules/@seamless-auth/types": { "version": "0.34.0", "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.34.0.tgz", "integrity": "sha512-bTgZkEdpXwAejxwnA1J8vTGAq+FW9L6J4n97q2MI6SqWWIOqXyaF6zMTkzeKqNTvfgF0NIn2DHh9qXL+92FN/A==", @@ -2926,6 +2926,18 @@ "node": ">=22" } }, + "node_modules/@seamless-auth/types": { + "version": "0.35.0", + "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.35.0.tgz", + "integrity": "sha512-T/1E72d43M+Bzivgo9k8yxaSBRpaXO9XJYlsenCwYVPb71Qnis+w10nXS7X/Qh/zKp7otOjRys2T3jMgP1L6Eg==", + "license": "Apache-2.0", + "dependencies": { + "zod": "^4.5.0" + }, + "engines": { + "node": ">=22" + } + }, "node_modules/@simple-libs/child-process-utils": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@simple-libs/child-process-utils/-/child-process-utils-2.0.0.tgz", diff --git a/package.json b/package.json index 207d0b6a..515dff5a 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "@seamless-auth/messaging": "^0.3.1", "@seamless-auth/messaging-aws": "^0.3.1", "@seamless-auth/messaging-twilio": "^0.3.1", - "@seamless-auth/types": "^0.34.0", + "@seamless-auth/types": "^0.35.0", "@simplewebauthn/server": "^14.0.3", "base64url": "^3.0.1", "bcrypt-ts": "^9.0.2", diff --git a/src/controllers/oidcEdge.ts b/src/controllers/oidcEdge.ts index 405931c1..04bdc7b2 100644 --- a/src/controllers/oidcEdge.ts +++ b/src/controllers/oidcEdge.ts @@ -16,6 +16,7 @@ import { type OidcPayloadUidParams, type OidcPayloadUpsertRequest, type OidcPayloadUserCodeParams, + type OidcSessionBindRequest, type OidcSessionIdParams, type OidcSessionQuery, type OidcSessionRevokeRequest, @@ -39,6 +40,7 @@ import { verifyOidcClientSecret, } from '../services/oidcClientRegistry.js'; import { + bindOidcSession, consumeOidcPayload, destroyOidcPayload, ExpiryTooLongError, @@ -47,12 +49,13 @@ import { findOidcPayloadByUid, findOidcPayloadByUserCode, InvalidSessionIdError, + isOidcSessionBound, revokeOidcPayloadsByGrant, upsertOidcPayload, } from '../services/oidcPayloadStore.js'; import { getOidcPublicJwks } from '../services/oidcProviderKeys.js'; import { signForOidcEdge } from '../services/oidcSigning.js'; -import { revokeSessionChain } from '../services/sessionService.js'; +import { revokeSessionChain, validateBearerToken } from '../services/sessionService.js'; import getLogger from '../utils/logger.js'; const logger = getLogger('oidcEdge'); @@ -212,13 +215,40 @@ export async function getOidcSessionAuthContext(req: Req, r return context ? res.status(200).json(context) : res.status(404).json(NOT_FOUND); } +/** + * The edge holds a user's access token only while that user signs in through it, so a + * binding made with one is a sign-in the user really made, for the session the token was + * issued under. + */ +export async function postOidcSessionBind( + req: Req, + res: Response, +) { + const { clientId, accessToken } = req.body; + const validated = await validateBearerToken(accessToken, 'access'); + + if (!validated || validated.sessionId !== req.params.id) { + return res.status(403).json({ error: 'invalid_access_token' }); + } + if (validated.recovery) { + return res.status(403).json({ error: 'enrollment_required' }); + } + if (!(await findActiveOidcClient(clientId))) return res.status(404).json(NOT_FOUND); + + await bindOidcSession(req.params.id, validated.user.id, clientId); + return res.status(204).end(); +} + export async function postOidcSessionRevoke( req: Req, res: Response, ) { const session = isUuid(req.params.id) ? await Session.findByPk(req.params.id) : null; - if (!session) return res.status(404).json(NOT_FOUND); + // Only a session someone signed in to a client with through the edge is the edge's to end. + if (!session || !(await isOidcSessionBound(session.id))) { + return res.status(404).json(NOT_FOUND); + } // From the recorded row forward, so the rotations the edge's adapter made since the // provider sign-in are revoked with it. diff --git a/src/generated/api.ts b/src/generated/api.ts index af40ebac..c395fa42 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -13412,6 +13412,162 @@ export interface paths { patch?: never; trace?: never; }; + '/oidc/sessions/{id}/bind': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Bind a session to the client a provider sign-in is for */ + post: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: { + content: { + 'application/json': { + clientId: string; + accessToken: string; + }; + }; + }; + responses: { + /** @description HTTP 400 */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "error": "string", + * "message": "string", + * "details": { + * "issues": [ + * null + * ] + * } + * } + */ + 'application/json': { + error: string; + message?: string; + details?: { + issues: { + path: (string | number)[]; + code: string; + message: string; + }[]; + }; + }; + }; + }; + /** @description HTTP 401 */ + 401: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "message": "string", + * "error": "string" + * } + */ + 'application/json': { + message?: string; + error: string; + }; + }; + }; + /** @description HTTP 403 */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "message": "string", + * "error": "string" + * } + */ + 'application/json': { + message?: string; + error: string; + }; + }; + }; + /** @description HTTP 404 */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "message": "string", + * "error": "string" + * } + */ + 'application/json': { + message?: string; + error: string; + }; + }; + }; + /** @description HTTP 429 */ + 429: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "message": "string", + * "error": "string" + * } + */ + 'application/json': { + message?: string; + error: string; + }; + }; + }; + /** @description HTTP 500 */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + /** + * @example { + * "message": "string", + * "error": "string" + * } + */ + 'application/json': { + message?: string; + error: string; + }; + }; + }; + }; + }; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; '/oidc/sessions/{id}/revoke': { parameters: { query?: never; diff --git a/src/routes/oidc.routes.ts b/src/routes/oidc.routes.ts index 9b515c74..a58c8505 100644 --- a/src/routes/oidc.routes.ts +++ b/src/routes/oidc.routes.ts @@ -23,6 +23,7 @@ import { OidcPayloadUpsertRequestSchema, OidcPayloadUpsertResponseSchema, OidcPayloadUserCodeParamsSchema, + OidcSessionBindRequestSchema, OidcSessionIdParamsSchema, OidcSessionQuerySchema, OidcSessionRevokeRequestSchema, @@ -47,6 +48,7 @@ import { getOidcSessionAuthContext, postOidcEvent, postOidcPayloadConsume, + postOidcSessionBind, postOidcSessionRevoke, postOidcSign, postOidcVerifySecret, @@ -269,6 +271,20 @@ oidcRouter.get( getOidcSessionAuthContext, ); +oidcRouter.post( + '/sessions/:id/bind', + { + ...edge, + summary: 'Bind a session to the client a provider sign-in is for', + schemas: { + params: OidcSessionIdParamsSchema, + body: OidcSessionBindRequestSchema, + response: { 403: ErrorSchema, ...guarded }, + }, + }, + postOidcSessionBind, +); + oidcRouter.post( '/sessions/:id/revoke', { diff --git a/src/services/oidcPayloadStore.ts b/src/services/oidcPayloadStore.ts index 49e8a5ce..654556df 100644 --- a/src/services/oidcPayloadStore.ts +++ b/src/services/oidcPayloadStore.ts @@ -146,10 +146,56 @@ async function findOne( }; } -/** Whether a live provider grant for this client was stored against the session. */ +// Written only by bindOidcSession, after the API has checked the user's own access token for +// the session, and kept under a model the edge's payload routes cannot name. The edge's own +// Grant rows say whatever the edge sends, so on their own they bind nothing. +const SESSION_BINDING = 'SessionBinding'; + +/** + * Records that a provider sign-in for the client happened on the session. Lasts as long as + * the longest provider grant can, and goes with the session's other provider records when + * the session is revoked. + */ +export async function bindOidcSession(sessionId: string, userId: string, clientId: string) { + await OidcPayload.upsert({ + id: `${sessionId}:${clientId}`, + model: SESSION_BINDING, + payload: { accountId: userId, clientId }, + grantId: null, + userCode: null, + uid: null, + sessionId, + expiresAt: new Date(Date.now() + MAX_EXPIRES_IN_SECONDS * 1000), + }); +} + +function findBinding(where: Record) { + return OidcPayload.findOne({ + where: { + ...where, + model: SESSION_BINDING, + expiresAt: { [Op.gt]: new Date() }, + } as WhereOptions, + }); +} + +/** Whether a provider sign-in for any client was bound to the session. */ +export async function isOidcSessionBound(sessionId: string) { + if (!isUuid(sessionId)) return false; + + return Boolean(await findBinding({ sessionId })); +} + +/** + * Whether the session is bound to this client and a live provider grant for the client was + * stored against it. The grant is still required so that revoking it ends access. + */ export async function hasOidcGrantForSession(sessionId: string, clientId: string) { if (!isUuid(sessionId)) return false; + const binding = await findBinding({ id: `${sessionId}:${clientId}` }); + if (binding?.payload.clientId !== clientId) return false; + // One first-party session can sign in to several clients, so the client is part of the // query rather than checked on whichever grant comes back first. const grant = await OidcPayload.findOne({ @@ -164,10 +210,15 @@ export async function hasOidcGrantForSession(sessionId: string, clientId: string return grant?.payload.clientId === clientId; } -/** Whether the user holds a live provider grant for the client, through any session. */ +/** Whether the user signed in to the client and still holds a live provider grant for it. */ export async function hasOidcGrantForUser(userId: string, clientId: string) { if (!isUuid(userId)) return false; + const binding = await findBinding({ payload: { accountId: userId, clientId } }); + if (binding?.payload.accountId !== userId || binding.payload.clientId !== clientId) { + return false; + } + const grant = await OidcPayload.findOne({ where: { model: 'Grant', diff --git a/src/services/oidcSigning.ts b/src/services/oidcSigning.ts index 2d8739f3..0ce3b588 100644 --- a/src/services/oidcSigning.ts +++ b/src/services/oidcSigning.ts @@ -275,7 +275,10 @@ async function checkUserToken( // ADR 0001, D10. The passkey proves who approved the device, not who holds its tokens, so // a device-flow token never states the assurance the approving session earned. - if (device && claims.acr !== undefined) { + if ( + device && + (claims.acr !== undefined || claims.amr !== undefined || claims.auth_time !== undefined) + ) { return refuse('auth_context_mismatch', session.userId); } diff --git a/tests/integration/applications/accountNotices.spec.ts b/tests/integration/applications/accountNotices.spec.ts index 072f642e..af54a67f 100644 --- a/tests/integration/applications/accountNotices.spec.ts +++ b/tests/integration/applications/accountNotices.spec.ts @@ -125,9 +125,15 @@ describe('POST /applications/account-notices', () => { ); }); - it('only checks a grant held with the asking client', async () => { + it('only checks a sign-in and a grant held with the asking client', async () => { await send(); + expect(OidcPayload.findOne).toHaveBeenCalledWith({ + where: expect.objectContaining({ + model: 'SessionBinding', + payload: { accountId: USER, clientId: 'locksley' }, + }), + }); expect(OidcPayload.findOne).toHaveBeenCalledWith({ where: expect.objectContaining({ model: 'Grant', diff --git a/tests/integration/oidc/oidcEdge.spec.ts b/tests/integration/oidc/oidcEdge.spec.ts index 8fd99462..bdd37254 100644 --- a/tests/integration/oidc/oidcEdge.spec.ts +++ b/tests/integration/oidc/oidcEdge.spec.ts @@ -16,6 +16,8 @@ vi.mock('../../../src/services/oidcPayloadStore.js', async (importOriginal) => ( destroyOidcPayload: vi.fn(), revokeOidcPayloadsByGrant: vi.fn(), findGrantBoundSession: vi.fn(), + bindOidcSession: vi.fn(), + isOidcSessionBound: vi.fn(), })); vi.mock('../../../src/services/oidcBackchannelLogout.js', () => ({ @@ -59,6 +61,7 @@ import { verifyOidcClientSecret, } from '../../../src/services/oidcClientRegistry.js'; import { + bindOidcSession, consumeOidcPayload, destroyOidcPayload, findOidcPayload, @@ -67,12 +70,13 @@ import { findGrantBoundSession, findOidcPayloadByUserCode, InvalidSessionIdError, + isOidcSessionBound, revokeOidcPayloadsByGrant, upsertOidcPayload, } from '../../../src/services/oidcPayloadStore.js'; import { getOidcPublicJwks } from '../../../src/services/oidcProviderKeys.js'; import { signForOidcEdge } from '../../../src/services/oidcSigning.js'; -import { revokeSessionChain } from '../../../src/services/sessionService.js'; +import { revokeSessionChain, validateBearerToken } from '../../../src/services/sessionService.js'; import { buildSystemConfig } from '../../factories/systemConfigFactory'; import { mintInternalServiceToken, @@ -173,7 +177,7 @@ describe('GET /oidc/config', () => { const res = await edge('get', '/oidc/config'); expect(res.body).toEqual({ - contractVersion: 2, + contractVersion: 3, issuer: ISSUER, appName: 'SeamlessAuth', allowClientSecrets: false, @@ -459,9 +463,65 @@ describe('sessions', () => { expect((await edge('get', `/oidc/sessions/${SESSION}/auth-context`)).status).toBe(400); }); + describe('binding a session to a client', () => { + const path = `/oidc/sessions/${SESSION}/bind`; + const body = { clientId: 'app', accessToken: 'user-access-token' }; + + beforeEach(() => { + vi.mocked(validateBearerToken).mockResolvedValue({ + user: { id: USER }, + sessionId: SESSION, + } as never); + vi.mocked(findActiveOidcClient).mockResolvedValue({ id: 'app' } as never); + }); + + it("binds with the user's own access token for that session", async () => { + expect((await edge('post', path).send(body)).status).toBe(204); + expect(validateBearerToken).toHaveBeenCalledWith('user-access-token', 'access'); + expect(bindOidcSession).toHaveBeenCalledWith(SESSION, USER, 'app'); + }); + + it('refuses a token that is not an access token for this session', async () => { + vi.mocked(validateBearerToken).mockResolvedValueOnce(null); + expect((await edge('post', path).send(body)).body).toEqual({ + error: 'invalid_access_token', + }); + + vi.mocked(validateBearerToken).mockResolvedValueOnce({ + user: { id: USER }, + sessionId: '33333333-3333-4333-8333-333333333333', + } as never); + const res = await edge('post', path).send(body); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'invalid_access_token' }); + expect(bindOidcSession).not.toHaveBeenCalled(); + }); + + it('refuses a recovery session and an inactive client', async () => { + vi.mocked(validateBearerToken).mockResolvedValueOnce({ + user: { id: USER }, + sessionId: SESSION, + recovery: true, + } as never); + const recovery = await edge('post', path).send(body); + expect(recovery.status).toBe(403); + expect(recovery.body).toEqual({ error: 'enrollment_required' }); + + vi.mocked(findActiveOidcClient).mockResolvedValueOnce(null); + expect((await edge('post', path).send(body)).status).toBe(404); + expect(bindOidcSession).not.toHaveBeenCalled(); + }); + + it('requires the client and the token', async () => { + expect((await edge('post', path).send({ clientId: 'app' })).status).toBe(400); + expect((await edge('post', path).send({ accessToken: 't' })).status).toBe(400); + }); + }); + it('revokes the chain and the provider records behind it', async () => { const session = { id: SESSION, userId: 'u1' }; vi.mocked(Session.findByPk).mockResolvedValue(session as never); + vi.mocked(isOidcSessionBound).mockResolvedValue(true); const res = await edge('post', `/oidc/sessions/${SESSION}/revoke`).send({ reason: 'refresh_token_reuse', @@ -498,6 +558,20 @@ describe('sessions', () => { expect.objectContaining({ type: 'oidc_session_revoked', metadata: expect.anything() }), ); }); + + it('refuses to revoke a session nobody signed in to a client with', async () => { + vi.mocked(Session.findByPk).mockResolvedValue({ id: SESSION, userId: 'u1' } as never); + vi.mocked(isOidcSessionBound).mockResolvedValue(false); + + const res = await edge('post', `/oidc/sessions/${SESSION}/revoke`).send({ + reason: 'provider_logout', + }); + + expect(res.status).toBe(404); + expect(isOidcSessionBound).toHaveBeenCalledWith(SESSION); + expect(revokeSessionChain).not.toHaveBeenCalled(); + expect(OidcPayload.destroy).not.toHaveBeenCalled(); + }); }); describe('POST /oidc/events', () => { diff --git a/tests/postgres/oidcProvider.spec.ts b/tests/postgres/oidcProvider.spec.ts index 0b5dd260..f46cd6e2 100644 --- a/tests/postgres/oidcProvider.spec.ts +++ b/tests/postgres/oidcProvider.spec.ts @@ -5,11 +5,14 @@ import { getSequelize, initializeModels } from '../../src/models/index.js'; import { OidcClient } from '../../src/models/oidcClients.js'; import { OidcPayload } from '../../src/models/oidcPayloads.js'; import { + bindOidcSession, claimOAuthStateUse, consumeOidcPayload, findGrantBoundSession, findOidcPayload, findOidcPayloadByUid, + hasOidcGrantForUser, + isOidcSessionBound, purgeExpiredOidcPayloads, revokeOidcPayloadsByGrant, upsertOidcPayload, @@ -168,11 +171,39 @@ describe('grant binding', () => { expiresIn: 600, sessionId: session.id, }); + await bindOidcSession(session.id, user.id, 'app'); expect((await findGrantBoundSession(session.id, 'app'))?.id).toBe(session.id); expect(await findGrantBoundSession(session.id, 'other')).toBeNull(); }); + it('ignores grants the edge stored without an API binding', async () => { + const { User } = await import('../../src/models/users.js'); + const { Session } = await import('../../src/models/sessions.js'); + const user = await User.create({ email: `${randomUUID()}@example.com` } as never); + const session = await Session.create({ + userId: user.id, + mode: 'server', + expiresAt: new Date(Date.now() + 3_600_000), + idleExpiresAt: new Date(Date.now() + 3_600_000), + }); + + await upsertOidcPayload('Grant', randomUUID(), { + payload: { clientId: 'app', accountId: user.id }, + expiresIn: 600, + sessionId: session.id, + }); + + expect(await findGrantBoundSession(session.id, 'app')).toBeNull(); + expect(await hasOidcGrantForUser(user.id, 'app')).toBe(false); + expect(await isOidcSessionBound(session.id)).toBe(false); + + await bindOidcSession(session.id, user.id, 'other'); + expect(await findGrantBoundSession(session.id, 'app')).toBeNull(); + expect(await hasOidcGrantForUser(user.id, 'app')).toBe(false); + expect(await isOidcSessionBound(session.id)).toBe(true); + }); + it('binds every client a session signed in to, not just one of them', async () => { const { User } = await import('../../src/models/users.js'); const { Session } = await import('../../src/models/sessions.js'); @@ -190,6 +221,7 @@ describe('grant binding', () => { expiresIn: 600, sessionId: session.id, }); + await bindOidcSession(session.id, user.id, clientId); } for (const clientId of ['first', 'second', 'third']) { diff --git a/tests/unit/services/oidcPayloadStore.spec.ts b/tests/unit/services/oidcPayloadStore.spec.ts index 57715c8a..d2cc8b6d 100644 --- a/tests/unit/services/oidcPayloadStore.spec.ts +++ b/tests/unit/services/oidcPayloadStore.spec.ts @@ -24,6 +24,7 @@ import { OidcPayload } from '../../../src/models/oidcPayloads'; import { recordBackchannelLogoutTargets } from '../../../src/services/oidcBackchannelLogout'; import { resolveCurrentSession } from '../../../src/services/oidcAuthContext'; import { + bindOidcSession, claimOAuthStateUse, consumeOidcPayload, destroyOidcPayload, @@ -35,6 +36,7 @@ import { hasOidcGrantForSession, hasOidcGrantForUser, InvalidSessionIdError, + isOidcSessionBound, MAX_EXPIRES_IN_SECONDS, purgeExpiredOidcPayloads, revokeOidcPayloadsByGrant, @@ -201,21 +203,73 @@ describe('finding records', () => { }); }); +describe('bindOidcSession', () => { + it('stores the binding under a model the edge cannot name, for the longest grant', async () => { + vi.useFakeTimers({ now: new Date('2030-01-01T00:00:00.000Z') }); + await bindOidcSession(SESSION, 'u1', 'app'); + vi.useRealTimers(); + + expect(OidcPayload.upsert).toHaveBeenCalledWith({ + id: `${SESSION}:app`, + model: 'SessionBinding', + payload: { accountId: 'u1', clientId: 'app' }, + grantId: null, + userCode: null, + uid: null, + sessionId: SESSION, + expiresAt: new Date(Date.parse('2030-01-01T00:00:00.000Z') + MAX_EXPIRES_IN_SECONDS * 1000), + }); + }); +}); + +describe('isOidcSessionBound', () => { + it('needs a live binding for the session', async () => { + vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ payload: {} } as never); + expect(await isOidcSessionBound(SESSION)).toBe(true); + expect(vi.mocked(OidcPayload.findOne).mock.calls[0][0]).toMatchObject({ + where: { model: 'SessionBinding', sessionId: SESSION }, + }); + + vi.mocked(OidcPayload.findOne).mockResolvedValueOnce(null); + expect(await isOidcSessionBound(SESSION)).toBe(false); + }); + + it('refuses a malformed session id without a query', async () => { + expect(await isOidcSessionBound('nope')).toBe(false); + expect(OidcPayload.findOne).not.toHaveBeenCalled(); + }); +}); + +const binding = (payload: Record) => ({ payload }) as never; + describe('hasOidcGrantForSession', () => { - it('needs a live grant for this client stored against the session', async () => { - vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ payload: { clientId: 'app' } } as never); + it('needs the API binding and a live grant for this client stored against the session', async () => { + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ clientId: 'app' })) + .mockResolvedValueOnce({ payload: { clientId: 'app' } } as never); expect(await hasOidcGrantForSession(SESSION, 'app')).toBe(true); expect(vi.mocked(OidcPayload.findOne).mock.calls[0][0]).toMatchObject({ + where: { model: 'SessionBinding', id: `${SESSION}:app` }, + }); + expect(vi.mocked(OidcPayload.findOne).mock.calls[1][0]).toMatchObject({ where: { model: 'Grant', sessionId: SESSION, payload: { clientId: 'app' } }, }); - vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ - payload: { clientId: 'other' }, - } as never); + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ clientId: 'app' })) + .mockResolvedValueOnce({ payload: { clientId: 'other' } } as never); + expect(await hasOidcGrantForSession(SESSION, 'app')).toBe(false); + + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ clientId: 'app' })) + .mockResolvedValueOnce(null); expect(await hasOidcGrantForSession(SESSION, 'app')).toBe(false); + }); + it('ignores a grant the edge stored for a session the API never bound', async () => { vi.mocked(OidcPayload.findOne).mockResolvedValueOnce(null); expect(await hasOidcGrantForSession(SESSION, 'app')).toBe(false); + expect(OidcPayload.findOne).toHaveBeenCalledTimes(1); }); it('refuses a malformed session id without a query', async () => { @@ -225,22 +279,38 @@ describe('hasOidcGrantForSession', () => { }); describe('hasOidcGrantForUser', () => { - it('needs a live grant held by the user for this client', async () => { - vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ - payload: { accountId: SESSION, clientId: 'app' }, - } as never); + it('needs the API binding and a live grant held by the user for this client', async () => { + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ accountId: SESSION, clientId: 'app' })) + .mockResolvedValueOnce({ payload: { accountId: SESSION, clientId: 'app' } } as never); expect(await hasOidcGrantForUser(SESSION, 'app')).toBe(true); expect(vi.mocked(OidcPayload.findOne).mock.calls[0][0]).toMatchObject({ + where: { model: 'SessionBinding', payload: { accountId: SESSION, clientId: 'app' } }, + }); + expect(vi.mocked(OidcPayload.findOne).mock.calls[1][0]).toMatchObject({ where: { model: 'Grant', payload: { accountId: SESSION, clientId: 'app' } }, }); - vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ - payload: { accountId: SESSION, clientId: 'other' }, - } as never); + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ accountId: SESSION, clientId: 'app' })) + .mockResolvedValueOnce({ payload: { accountId: SESSION, clientId: 'other' } } as never); + expect(await hasOidcGrantForUser(SESSION, 'app')).toBe(false); + + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ accountId: SESSION, clientId: 'app' })) + .mockResolvedValueOnce(null); expect(await hasOidcGrantForUser(SESSION, 'app')).toBe(false); + }); + it('ignores a grant the edge stored for a user who never signed in to the client', async () => { vi.mocked(OidcPayload.findOne).mockResolvedValueOnce(null); expect(await hasOidcGrantForUser(SESSION, 'app')).toBe(false); + + vi.mocked(OidcPayload.findOne).mockResolvedValueOnce( + binding({ accountId: SESSION, clientId: 'other' }), + ); + expect(await hasOidcGrantForUser(SESSION, 'app')).toBe(false); + expect(OidcPayload.findOne).toHaveBeenCalledTimes(2); }); it('refuses a malformed user id without a query', async () => { @@ -252,7 +322,9 @@ describe('hasOidcGrantForUser', () => { describe('findGrantBoundSession', () => { it('returns the current session only when it holds a grant for the client', async () => { vi.mocked(resolveCurrentSession).mockResolvedValue({ id: SESSION, userId: 'u1' } as never); - vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ payload: { clientId: 'app' } } as never); + vi.mocked(OidcPayload.findOne) + .mockResolvedValueOnce(binding({ clientId: 'app' })) + .mockResolvedValueOnce({ payload: { clientId: 'app' } } as never); expect(await findGrantBoundSession(SESSION, 'app')).toEqual({ id: SESSION, userId: 'u1' }); vi.mocked(OidcPayload.findOne).mockResolvedValueOnce(null); diff --git a/tests/unit/services/oidcSigning.spec.ts b/tests/unit/services/oidcSigning.spec.ts index 1f87db29..ab35d269 100644 --- a/tests/unit/services/oidcSigning.spec.ts +++ b/tests/unit/services/oidcSigning.spec.ts @@ -421,14 +421,26 @@ describe('signForOidcEdge', () => { } as never); } - it('signs one that leaves acr out', async () => { + it('signs one that leaves acr, amr and auth_time out', async () => { deviceClient(); - const { acr: _acr, ...claims } = idTokenClaims(); + const { acr: _acr, amr: _amr, auth_time: _authTime, ...claims } = idTokenClaims(); expect((await signForOidcEdge({ ...idTokenRequest(claims), grantType: DEVICE })).ok).toBe( true, ); }); + it('never signs the amr or auth_time of the approving session', async () => { + deviceClient(); + const { acr: _acr, ...withAmr } = idTokenClaims(); + const { auth_time: _authTime, ...amrOnly } = withAmr; + const { amr: _amr, ...authTimeOnly } = withAmr; + for (const claims of [amrOnly, authTimeOnly]) { + expect( + await signForOidcEdge({ ...idTokenRequest(claims), grantType: DEVICE }), + ).toMatchObject({ ok: false, code: 'auth_context_mismatch' }); + } + }); + it('never signs the acr the approving session earned', async () => { deviceClient(); expect(await signForOidcEdge({ ...idTokenRequest(), grantType: DEVICE })).toMatchObject({