From 93a4c8443f9b6b3b3c5cd8f5d8d0e4f7c29ee795 Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Mon, 19 Feb 2024 16:55:11 +0100 Subject: [PATCH] feat(oauth-provider): various improvements - export error classes - add support for Boom & XRPC errors when formatting responses - check uniqueness of JAR request objects - use signed JAR as client credentials - remove "jti" from clientAuth - distinguish "device-less login" (password_grant) in store implementation --- .../src/account/account-manager.ts | 50 ++--- .../src/account/account-store.ts | 33 ++-- .../oauth-provider/src/client/client-auth.ts | 1 - packages/oauth-provider/src/client/client.ts | 42 ++-- packages/oauth-provider/src/index.ts | 1 + packages/oauth-provider/src/oauth-errors.ts | 17 ++ packages/oauth-provider/src/oauth-provider.ts | 181 +++++++++++++++--- packages/oauth-provider/src/oauth-store.ts | 2 +- .../src/output/build-error-payload.ts | 77 +++++++- .../src/request/request-manager.ts | 53 ++--- packages/oauth-provider/src/signer/signer.ts | 6 +- .../oauth-provider/src/token/token-data.ts | 2 +- .../oauth-provider/src/token/token-manager.ts | 22 +-- packages/oauth-provider/src/util/crypto.ts | 5 + 14 files changed, 357 insertions(+), 135 deletions(-) create mode 100644 packages/oauth-provider/src/oauth-errors.ts diff --git a/packages/oauth-provider/src/account/account-manager.ts b/packages/oauth-provider/src/account/account-manager.ts index 1768c28f948..58cd6528fe8 100644 --- a/packages/oauth-provider/src/account/account-manager.ts +++ b/packages/oauth-provider/src/account/account-manager.ts @@ -1,18 +1,31 @@ -import { ClientId } from '../client/client-id.js' import { DeviceId } from '../device/device-id.js' import { InvalidRequestError } from '../errors/invalid-request-error.js' import { Sub } from '../oidc/sub.js' -import { AccountStore, DeviceAccountInfo } from './account-store.js' +import { + AccountLoginCredentials, + AccountStore, + DeviceAccountInfo, +} from './account-store.js' import { Account } from './account.js' export class AccountManager { constructor(protected readonly store: AccountStore) {} - public async list( + public async login( + credentials: AccountLoginCredentials, + deviceId: DeviceId | null, + ): Promise { + const account = await this.store.login(credentials, deviceId) + if (!account) throw new InvalidRequestError('Invalid credentials') + return account + } + + public async add( deviceId: DeviceId, - ): Promise<{ account: Account; info: DeviceAccountInfo }[]> { - const result = await this.store.listAccounts(deviceId) - return result.filter(({ info }) => info.remember) + sub: Sub, + info: DeviceAccountInfo, + ): Promise { + return this.store.addAccount(deviceId, sub, info) } public async get( @@ -24,25 +37,18 @@ export class AccountManager { return result } - public async login( - credentials: { username: string; password: string }, + public async update( deviceId: DeviceId, - ): Promise<{ account: Account; info: DeviceAccountInfo }> { - const result = await this.store.addAccount(deviceId, credentials) - if (!result) throw new InvalidRequestError('Invalid credentials') - return result + sub: Sub, + info: Partial, + ): Promise { + await this.store.updateAccount(deviceId, sub, info) } - public async addTrustedClient( + public async list( deviceId: DeviceId, - sub: Sub, - clientId: ClientId, - ): Promise { - const { info } = await this.get(deviceId, sub) - if (!info.trustedClients.includes(clientId)) { - await this.store.updateAccountInfo(deviceId, sub, { - trustedClients: [...info.trustedClients, clientId], - }) - } + ): Promise<{ account: Account; info: DeviceAccountInfo }[]> { + const result = await this.store.listAccounts(deviceId) + return result.filter(({ info }) => info.remember) } } diff --git a/packages/oauth-provider/src/account/account-store.ts b/packages/oauth-provider/src/account/account-store.ts index d51f0db02f2..a20c5e1a932 100644 --- a/packages/oauth-provider/src/account/account-store.ts +++ b/packages/oauth-provider/src/account/account-store.ts @@ -4,19 +4,29 @@ import { Sub } from '../oidc/sub.js' import { Awaitable } from '../util/awaitable.js' import { Account } from './account.js' -export type DeviceAccountCredentials = { +export type AccountLoginCredentials = { username: string password: string - remember?: boolean } export type DeviceAccountInfo = { + /** + * If false, the account will not be available next time a user logs in on the + * same device. Note that even though the account is not remembered, the + * DeviceAccountInfo entity must still be returned by the `listAccounts` + * method (with "remember" set to false). + */ remember: boolean authTime: Date trustedClients: readonly ClientId[] } export interface AccountStore { + login( + credentials: AccountLoginCredentials, + deviceId: DeviceId | null, + ): Awaitable + /** * @note The `deviceId` might not correspond to a real device meaning that * {@link SessionStore['createSession']} might never have been called for a @@ -25,21 +35,22 @@ export interface AccountStore { */ addAccount( deviceId: DeviceId, - credentials: DeviceAccountCredentials, - ): Awaitable - - listAccounts( - deviceId: DeviceId, - sub?: Sub, - ): Awaitable<{ account: Account; info: DeviceAccountInfo }[]> + sub: Sub, + info: DeviceAccountInfo, + ): Awaitable - updateAccountInfo( + updateAccount( deviceId: DeviceId, sub: Sub, info: Partial, ): Awaitable removeAccount(deviceId: DeviceId, sub: Sub): Awaitable + + listAccounts( + deviceId: DeviceId, + sub?: Sub, + ): Awaitable<{ account: Account; info: DeviceAccountInfo }[]> } export function isAccountStore( @@ -48,7 +59,7 @@ export function isAccountStore( return ( typeof implementation.addAccount === 'function' && typeof implementation.listAccounts === 'function' && - typeof implementation.updateAccountInfo === 'function' && + typeof implementation.updateAccount === 'function' && typeof implementation.removeAccount === 'function' ) } diff --git a/packages/oauth-provider/src/client/client-auth.ts b/packages/oauth-provider/src/client/client-auth.ts index 0876546f932..9a274243908 100644 --- a/packages/oauth-provider/src/client/client-auth.ts +++ b/packages/oauth-provider/src/client/client-auth.ts @@ -7,5 +7,4 @@ export type ClientAuth = alg: string kid: string jkt: string - jti: string } diff --git a/packages/oauth-provider/src/client/client.ts b/packages/oauth-provider/src/client/client.ts index 08d16814667..5b33b240a27 100644 --- a/packages/oauth-provider/src/client/client.ts +++ b/packages/oauth-provider/src/client/client.ts @@ -19,6 +19,7 @@ import { JOSEError } from 'jose/errors' import { JAR_MAX_AGE } from '../constants.js' import { InvalidClientMetadataError } from '../errors/invalid-client-metadata-error.js' import { InvalidRequestError } from '../errors/invalid-request-error.js' +import { keyJwkThumbprint } from '../util/crypto.js' import { ClientAuth } from './client-auth.js' import { CLIENT_ASSERTION_TYPE_JWT_BEARER, @@ -35,7 +36,7 @@ export class Client { */ static readonly AUTH_METHODS_SUPPORTED = ['none', 'private_key_jwt'] as const - private readonly keygetter: JWTVerifyGetKey + private readonly keyGetter: JWTVerifyGetKey constructor( public readonly id: ClientId, @@ -43,7 +44,7 @@ export class Client { jwks: undefined | Jwks = metadata.jwks, ) { // If the remote JWKS content is provided, we don't need to fetch it again. - this.keygetter = + this.keyGetter = jwks || !metadata.jwks_uri ? // @ts-expect-error https://github.com/panva/jose/issues/634 createLocalJWKSet(jwks || { keys: [] }) @@ -53,7 +54,7 @@ export class Client { async decodeRequestObject(jar: string) { switch (this.metadata.request_object_signing_alg) { case 'none': - return this.jwtVerifyUnsafe(jar, { + return this.jwtVerifyUnsecured(jar, { maxTokenAge: JAR_MAX_AGE / 1000, }) case undefined: @@ -71,7 +72,7 @@ export class Client { } } - async jwtVerifyUnsafe( + async jwtVerifyUnsecured( token: string, options?: Omit, ): Promise> { @@ -85,7 +86,7 @@ export class Client { token: string, options?: Omit, ): Promise & ResolvedKey> { - return jwtVerify(token, this.keygetter, { + return jwtVerify(token, this.keyGetter, { ...options, issuer: this.id, }) @@ -110,11 +111,16 @@ export class Client { audience: string maxTokenAge: number }, - ): Promise { + ): Promise<{ + clientAuth: ClientAuth + // for replay protection + nonce?: string + }> { const method = this.getAuthMethod(endpoint) if (method === 'none') { - return { method } + const clientAuth: ClientAuth = { method: 'none' } + return { clientAuth } } if (method === 'private_key_jwt') { @@ -135,7 +141,6 @@ export class Client { audience: options.audience, subject: this.id, maxTokenAge: options.maxTokenAge, - requiredClaims: ['jti'], }).catch((err) => { if (err instanceof JOSEError) { const msg = `Invalid "client_assertion": ${err.message}` @@ -146,16 +151,21 @@ export class Client { }) if (!result.protectedHeader.kid) { - throw new InvalidRequestError('Missing "kid" in header') + throw new InvalidRequestError(`"kid" required in client_assertion`) } - return { + if (!result.payload.jti) { + throw new InvalidRequestError(`"jti" required in client_assertion`) + } + + const clientAuth: ClientAuth = { method: CLIENT_ASSERTION_TYPE_JWT_BEARER, jkt: await calculateJwkThumbprint(await exportJWK(result.key)), - jti: result.payload.jti, alg: result.protectedHeader.alg, kid: result.protectedHeader.kid, } + + return { clientAuth, nonce: result.payload.jti } } throw new InvalidRequestError( @@ -165,7 +175,11 @@ export class Client { // @ts-expect-error Ensure to keep Client.AUTH_METHODS_SUPPORTED in sync if (Client.AUTH_METHODS_SUPPORTED.includes(method)) { - throw new Error('Implemntation missmatch') + throw new Error( + `verifyCredentials() should implement all of ${[ + Client.AUTH_METHODS_SUPPORTED, + ]}`, + ) } throw new InvalidClientMetadataError( @@ -189,14 +203,14 @@ export class Client { return false } try { - const key = await this.keygetter( + const key = await this.keyGetter( { kid: clientAuth.kid, alg: clientAuth.alg, }, { payload: '', signature: '' }, ) - const jtk = await calculateJwkThumbprint(await exportJWK(key)) + const jtk = await keyJwkThumbprint(key) return jtk === clientAuth.jkt } catch (e) { diff --git a/packages/oauth-provider/src/index.ts b/packages/oauth-provider/src/index.ts index 2eb95e5a951..6512fd220e2 100644 --- a/packages/oauth-provider/src/index.ts +++ b/packages/oauth-provider/src/index.ts @@ -4,3 +4,4 @@ export { OAuthProvider } from './oauth-provider.js' export * from './client/client-utils.js' export * from './oauth-hooks.js' export * from './oauth-store.js' +export * from './oauth-errors.js' diff --git a/packages/oauth-provider/src/oauth-errors.ts b/packages/oauth-provider/src/oauth-errors.ts new file mode 100644 index 00000000000..cc90d4147b8 --- /dev/null +++ b/packages/oauth-provider/src/oauth-errors.ts @@ -0,0 +1,17 @@ +// Root Error class +export { OAuthError } from './errors/oauth-error.js' + +export { AccessDeniedError } from './errors/access-denied-error.js' +export { AccountSelectionRequiredError } from './errors/account-selection-required-error.js' +export { ConsentRequiredError } from './errors/consent-required-error.js' +export { InvalidAuthorizationDetailsError } from './errors/invalid-authorization-details-error.js' +export { InvalidClientError } from './errors/invalid-client-error.js' +export { InvalidClientMetadataError } from './errors/invalid-client-metadata-error.js' +export { InvalidDpopProofError } from './errors/invalid-dpop-proof-error.js' +export { InvalidRedirectUriError } from './errors/invalid-redirect-uri-error.js' +export { InvalidRequestError } from './errors/invalid-request-error.js' +export { InvalidTokenError } from './errors/invalid-token-error.js' +export { LoginRequiredError } from './errors/login-required-error.js' +export { UnauthenticatedError } from './errors/unauthenticated-error.js' +export { UnauthorizedClientError } from './errors/unauthorized-client-error.js' +export { UseDpopNonceError } from './errors/use-dpop-nonce-error.js' diff --git a/packages/oauth-provider/src/oauth-provider.ts b/packages/oauth-provider/src/oauth-provider.ts index c4977be6615..3e2dfab1140 100644 --- a/packages/oauth-provider/src/oauth-provider.ts +++ b/packages/oauth-provider/src/oauth-provider.ts @@ -16,7 +16,7 @@ import { writeJson, } from '@atproto/http-util' import { Jwks, Jwt, Keyset, jwtSchema } from '@atproto/jwk' -import { decodeJwt } from 'jose' +import { JWTHeaderParameters, ResolvedKey, decodeJwt } from 'jose' import { z } from 'zod' import { AccountManager } from './account/account-manager.js' @@ -27,13 +27,16 @@ import { } from './account/account-store.js' import { Account } from './account/account.js' import { ClientAuth } from './client/client-auth.js' -import { ClientIdentification } from './client/client-credentials.js' +import { + CLIENT_ASSERTION_TYPE_JWT_BEARER, + ClientIdentification, +} from './client/client-credentials.js' import { ClientId, clientIdSchema } from './client/client-id.js' import { ClientDataHook, ClientManager } from './client/client-manager.js' import { ClientStore, asClientStore } from './client/client-store.js' import { Client } from './client/client.js' import { AUTH_MAX_AGE } from './constants.js' -import { DeviceId, generateDeviceId } from './device/device-id.js' +import { DeviceId } from './device/device-id.js' import { DpopManager } from './dpop/dpop-manager.js' import { DpopNonce } from './dpop/dpop-nonce.js' import { AccessDeniedError } from './errors/access-denied-error.js' @@ -65,7 +68,10 @@ import { sendAuthorizeRedirect, } from './output/send-authorize-redirect.js' import { sendErrorPage } from './output/send-error-page.js' -import { AuthorizationParameters } from './parameters/authorization-parameters.js' +import { + AuthorizationParameters, + authorizationParametersSchema, +} from './parameters/authorization-parameters.js' import { ReplayManager } from './replay/replay-manager.js' import { ReplayStore, asReplayStore } from './replay/replay-store.js' import { @@ -76,6 +82,7 @@ import { RequestStoreMemory } from './request/request-store-memory.js' import { RequestStore, isRequestStore } from './request/request-store.js' import { RequestUri, requestUriSchema } from './request/request-uri.js' import { + AuthorizationRequestJar, AuthorizationRequestQuery, PushedAuthorizationRequest, authorizationRequestQuerySchema, @@ -104,6 +111,7 @@ import { tokenRequestSchema, } from './token/types.js' import { parseAuthorizationHeader } from './util/authorization-header.js' +import { keyJwkThumbprint } from './util/crypto.js' import { dateToRelativeSeconds } from './util/date.js' import { formatWWWAuthenticateHeader } from './util/www-authenticate.js' @@ -257,18 +265,74 @@ export class OAuthProvider { ): Promise { const maxTokenAge = 60 - const details = await client.verifyCredentials(credentials, endpoint, { - audience: this.issuer, - maxTokenAge, - }) + const { clientAuth, nonce } = await client.verifyCredentials( + credentials, + endpoint, + { + audience: this.issuer, + maxTokenAge, + }, + ) + + if (nonce != null) { + const unique = await this.replayManager.unique(nonce, maxTokenAge) + if (!unique) { + throw new UnauthorizedClientError(`${clientAuth.method} jti reused`) + } + } + + return clientAuth + } + + async decodeJAR( + client: Client, + input: AuthorizationRequestJar, + ): Promise< + | { + payload: AuthorizationParameters + protectedHeader?: undefined + key?: undefined + } + | { + payload: AuthorizationParameters + protectedHeader: JWTHeaderParameters & { kid: string } + key: ResolvedKey['key'] + } + > { + const result = await client.decodeRequestObject(input.request) + + if (!result.payload.jti) { + throw new InvalidRequestError('Request object must contain a jti claim') + } + + if (!(await this.replayManager.unique(result.payload.jti, 300))) { + throw new InvalidRequestError('Request object jti is not unique') + } + + const payload = authorizationParametersSchema.parse(result.payload) + + if ('protectedHeader' in result) { + if (!result.protectedHeader.kid) { + throw new InvalidRequestError('Missing "kid" in header') + } + + return { + key: result.key, + payload, + protectedHeader: result.protectedHeader as JWTHeaderParameters & { + kid: string + }, + } + } - if ('jti' in details) { - const unique = await this.replayManager.unique(details.jti, maxTokenAge) - if (!unique) - throw new UnauthorizedClientError(`${details.method} jti reused`) + if ('header' in result) { + return { + payload, + } } - return details + // Should never happen + throw new Error('Invalid request object') } /** @@ -281,11 +345,17 @@ export class OAuthProvider { const client = await this.clientManager.getClient(input.client_id) const clientAuth = await this.authenticateClient(client, 'token', input) + // TODO (?) should we allow using signed JAR for client authentication? + const { payload: parameters } = + 'request' in input // Handle JAR + ? await this.decodeJAR(client, input) + : { payload: input } + const { uri, expiresAt } = await this.requestManager.pushedAuthorizationRequest( client, clientAuth, - input, + parameters, dpop_jkt, ) @@ -295,6 +365,54 @@ export class OAuthProvider { } } + private async setupAuthorizationRequest( + client: Client, + deviceId: DeviceId, + input: AuthorizationRequestQuery, + ) { + // Load PAR + if ('request_uri' in input) { + return this.requestManager.get(client, input.request_uri, deviceId) + } + + // Handle JAR + if ('request' in input) { + const requestObject = await this.decodeJAR(client, input) + + if (requestObject.protectedHeader) { + // Allow using signed JAR during "/authorize" as client authentication. + // This allows clients to skip PAR to initiate trusted sessions. + const clientAuth: ClientAuth = { + method: CLIENT_ASSERTION_TYPE_JWT_BEARER, + kid: requestObject.protectedHeader.kid, + alg: requestObject.protectedHeader.alg, + jkt: await keyJwkThumbprint(requestObject.key), + } + + return this.requestManager.authorizationRequest( + client, + clientAuth, + requestObject.payload, + deviceId, + ) + } + + return this.requestManager.authorizationRequest( + client, + { method: 'none' }, + requestObject.payload, + deviceId, + ) + } + + return this.requestManager.authorizationRequest( + client, + { method: 'none' }, + input, + deviceId, + ) + } + async authorize( deviceId: DeviceId, input: AuthorizationRequestQuery, @@ -304,7 +422,7 @@ export class OAuthProvider { try { const { uri, parameters, clientAuth } = - await this.requestManager.authorize(client, input, deviceId) + await this.setupAuthorizationRequest(client, deviceId, input) try { const sessions = await this.getSessions( @@ -442,11 +560,11 @@ export class OAuthProvider { info, ) - await this.accountManager.addTrustedClient( - deviceId, - account.sub, - client.id, - ) + if (!info.trustedClients.includes(client.id)) { + await this.accountManager.update(deviceId, account.sub, { + trustedClients: [...info.trustedClients, client.id], + }) + } return { issuer, client, parameters, redirect } } catch (err) { @@ -543,8 +661,8 @@ export class OAuthProvider { return await this.tokenManager.create( client, clientAuth, - deviceId, account, + deviceId, info, parameters, input, @@ -574,12 +692,6 @@ export class OAuthProvider { input: PasswordGrantRequest, dpop_jkt?: string, ): Promise { - // Generate a factis device id for the exchange. This device, not linked to - // any actual device/browser session, represents the "password" grant. - - // TODO (?) find a better way to represent this. - const deviceId = await generateDeviceId() - const parameters = await this.requestManager.validate( client, clientAuth, @@ -593,14 +705,14 @@ export class OAuthProvider { false, ) - const { account, info } = await this.accountManager.login(input, deviceId) + const account = await this.accountManager.login(input, null) return this.tokenManager.create( client, clientAuth, - deviceId, account, - info, + null, + null, parameters, input, dpop_jkt, @@ -1002,10 +1114,17 @@ export class OAuthProvider { ) const { deviceId } = await server.sessionManager.load(req, res, true) - const { account, info } = await server.accountManager.login( + const account = await server.accountManager.login( input.credentials, deviceId, ) + + const info = await server.accountManager.add(deviceId, account.sub, { + remember: input.credentials.remember === true, + trustedClients: [], + authTime: new Date(), + }) + return writeJson(res, { account, info }) }) diff --git a/packages/oauth-provider/src/oauth-store.ts b/packages/oauth-provider/src/oauth-store.ts index f27a6b2ae81..51477a63c1d 100644 --- a/packages/oauth-provider/src/oauth-store.ts +++ b/packages/oauth-provider/src/oauth-store.ts @@ -4,7 +4,7 @@ */ export { - type DeviceAccountCredentials as AccountLoginCredentials, + type AccountLoginCredentials, type AccountStore, type DeviceAccountInfo, } from './account/account-store.js' diff --git a/packages/oauth-provider/src/output/build-error-payload.ts b/packages/oauth-provider/src/output/build-error-payload.ts index c0d039c0b84..ead353f283e 100644 --- a/packages/oauth-provider/src/output/build-error-payload.ts +++ b/packages/oauth-provider/src/output/build-error-payload.ts @@ -6,9 +6,12 @@ import { OAuthError } from '../errors/oauth-error.js' const { JOSEError } = errors +const INVALID_REQUEST = 'invalid_request' +const SERVER_ERROR = 'server_error' + export function buildErrorStatus(error: unknown): number { if (error instanceof OAuthError) { - return 400 + return error.statusCode } if (error instanceof ZodError) { @@ -23,6 +26,14 @@ export function buildErrorStatus(error: unknown): number { return 400 } + if (isBoom(error)) { + return error.output.statusCode + } + + if (isXrpcError(error)) { + return error.type + } + if (isHttpError(error)) { return error.status } @@ -43,34 +54,88 @@ export function buildErrorPayload(error: unknown): { if (error instanceof ZodError) { return { - error: 'invalid_request', + error: INVALID_REQUEST, error_description: error.issues[0]?.message || 'Invalid request', } } if (error instanceof JOSEError) { return { - error: 'invalid_request', + error: INVALID_REQUEST, error_description: error.message, } } if (error instanceof TypeError) { return { - error: 'invalid_request', + error: INVALID_REQUEST, error_description: error.message, } } + if (isBoom(error)) { + return { + error: error.output.statusCode <= 500 ? INVALID_REQUEST : SERVER_ERROR, + error_description: + error.output.statusCode <= 500 + ? isPayloadLike(error.output?.payload) + ? error.output.payload.message + : error.message + : 'Server error', + } + } + + if (isXrpcError(error)) { + return { + error: error.type <= 500 ? INVALID_REQUEST : SERVER_ERROR, + error_description: error.payload.message, + } + } + if (isHttpError(error)) { return { - error: error.status < 500 ? 'invalid_request' : 'server_error', + error: error.status < 500 ? INVALID_REQUEST : SERVER_ERROR, error_description: error.expose ? error.message : 'Server error', } } return { - error: 'server_error', + error: SERVER_ERROR, error_description: 'Server error', } } + +function isBoom(v: unknown): v is Error & { + isBoom: true + output: { statusCode: number; payload: unknown } +} { + return ( + v instanceof Error && + (v as any).isBoom === true && + isHttpErrorCode(v['output']?.['statusCode']) + ) +} + +function isXrpcError(v: unknown): v is Error & { + type: number + payload: { error: string; message: string } +} { + return ( + v instanceof Error && + isHttpErrorCode(v['type']) && + isPayloadLike(v['payload']) + ) +} + +function isHttpErrorCode(v: unknown): v is number { + return typeof v === 'number' && v >= 400 && v < 600 && v === (v | 0) +} + +function isPayloadLike(v: unknown): v is { error: string; message: string } { + return ( + v != null && + typeof v === 'object' && + typeof v['error'] === 'string' && + typeof v['message'] === 'string' + ) +} diff --git a/packages/oauth-provider/src/request/request-manager.ts b/packages/oauth-provider/src/request/request-manager.ts index 7105e2aac86..b3942033983 100644 --- a/packages/oauth-provider/src/request/request-manager.ts +++ b/packages/oauth-provider/src/request/request-manager.ts @@ -12,10 +12,7 @@ import { AccessDeniedError } from '../errors/access-denied-error.js' import { InvalidRequestError } from '../errors/invalid-request-error.js' import { OIDC_SCOPE_CLAIMS } from '../oidc/claims.js' import { Sub } from '../oidc/sub.js' -import { - AuthorizationParameters, - authorizationParametersSchema, -} from '../parameters/authorization-parameters.js' +import { AuthorizationParameters } from '../parameters/authorization-parameters.js' import { Signer } from '../signer/signer.js' import { Awaitable } from '../util/awaitable.js' import { dateToEpoch } from '../util/date.js' @@ -28,7 +25,6 @@ import { decodeRequestUri, encodeRequestUri, } from './request-uri.js' -import { AuthorizationRequestJar, AuthorizationRequestQuery } from './types.js' export type RequestInfo = { id: RequestId @@ -71,25 +67,21 @@ export class RequestManager { async pushedAuthorizationRequest( client: Client, clientAuth: ClientAuth, - input: AuthorizationRequestJar | AuthorizationParameters, + input: AuthorizationParameters, dpop_jkt?: string, ): Promise { const params = await this.validate(client, clientAuth, input, dpop_jkt) return this.create(client, clientAuth, params, null) } - async authorize( + async authorizationRequest( client: Client, - input: AuthorizationRequestQuery, + clientAuth: ClientAuth, + input: AuthorizationParameters, deviceId: DeviceId, ): Promise { - if ('request_uri' in input) { - return this.get(client, input.request_uri, deviceId) - } else { - const clientAuth: ClientAuth = { method: 'none' } - const params = await this.validate(client, clientAuth, input) - return this.create(client, clientAuth, params, deviceId) - } + const params = await this.validate(client, clientAuth, input) + return this.create(client, clientAuth, params, deviceId) } protected async create( @@ -118,17 +110,10 @@ export class RequestManager { async validate( client: Client, clientAuth: ClientAuth, - input: AuthorizationRequestJar | AuthorizationParameters, + parameters: Readonly, dpop_jkt?: string, pkceRequired = this.pkceRequired, ): Promise { - const parameters = - 'request' in input - ? authorizationParametersSchema.parse( - (await client.decodeRequestObject(input.request)).payload, - ) - : structuredClone(input) - const scopes = parameters.scope?.split(' ') const responseTypes = parameters.response_type.split(' ') @@ -160,10 +145,10 @@ export class RequestManager { } // https://datatracker.ietf.org/doc/html/rfc9449#section-10 - if (parameters.dpop_jkt && parameters.dpop_jkt !== dpop_jkt) { + if (!parameters.dpop_jkt) { + parameters = { ...parameters, dpop_jkt } + } else if (parameters.dpop_jkt !== dpop_jkt) { throw new InvalidRequestError('DPoP header and dpop_jkt do not match') - } else { - parameters.dpop_jkt ||= dpop_jkt } if ( @@ -224,7 +209,9 @@ export class RequestManager { // TODO Validate parameters agains client metadata !!!! - parameters.scope ||= client.metadata.scope + if (!parameters.scope) { + parameters = { ...parameters, scope: client.metadata.scope } + } if (scopes) { const cScopes = client.metadata.scope?.split(' ') @@ -377,13 +364,11 @@ export class RequestManager { }) const id_token = responseType.includes('id_token') - ? await this.signer.idToken( - client, - data.parameters, - account, - info.authTime, - { exp: dateToEpoch(this.createTokenExpiry()), code }, - ) + ? await this.signer.idToken(client, data.parameters, account, { + auth_time: info.authTime, + exp: dateToEpoch(this.createTokenExpiry()), + code, + }) : undefined return { code, id_token } diff --git a/packages/oauth-provider/src/signer/signer.ts b/packages/oauth-provider/src/signer/signer.ts index e451eaec267..98d4989f57f 100644 --- a/packages/oauth-provider/src/signer/signer.ts +++ b/packages/oauth-provider/src/signer/signer.ts @@ -119,9 +119,9 @@ export class Signer { client: Client, params: AuthorizationParameters, account: Account, - authTime: Date, extra: { exp: number + auth_time?: Date code?: string access_token?: string }, @@ -167,8 +167,8 @@ export class Signer { auth_time: client.metadata.require_auth_time || params.max_age != null || - claimRequested(params, 'id_token', 'auth_time', true) - ? epoch(authTime) + claimRequested(params, 'id_token', 'auth_time', extra.auth_time != null) + ? epoch(extra.auth_time!) : undefined, } diff --git a/packages/oauth-provider/src/token/token-data.ts b/packages/oauth-provider/src/token/token-data.ts index 898dd122283..d38a01ca666 100644 --- a/packages/oauth-provider/src/token/token-data.ts +++ b/packages/oauth-provider/src/token/token-data.ts @@ -10,7 +10,7 @@ export type TokenData = { authorizedAt: Date clientId: ClientId clientAuth: ClientAuth - deviceId: DeviceId + deviceId: DeviceId | null sub: Sub parameters: AuthorizationParameters lastRefreshedAt?: Date diff --git a/packages/oauth-provider/src/token/token-manager.ts b/packages/oauth-provider/src/token/token-manager.ts index 85f65eea0a9..d9a2cfc6b9d 100644 --- a/packages/oauth-provider/src/token/token-manager.ts +++ b/packages/oauth-provider/src/token/token-manager.ts @@ -102,9 +102,9 @@ export class TokenManager { async create( client: Client, clientAuth: ClientAuth, - deviceId: DeviceId, account: Account, - info: DeviceAccountInfo, + deviceId: DeviceId | null, + deviceInfo: DeviceAccountInfo | null, parameters: AuthorizationParameters, input: CodeGrantRequest | PasswordGrantRequest, dpop_jkt?: string, @@ -243,9 +243,11 @@ export class TokenManager { const responseTypes = parameters.response_type.split(' ') const id_token = responseTypes.includes('id_token') - ? await this.signer.idToken(client, parameters, account, info.authTime, { - access_token, + ? await this.signer.idToken(client, parameters, account, { exp: dateToEpoch(expiresAt), + // If there is no deviceInfo, we are in a "password_grant" context + auth_time: deviceInfo?.authTime || new Date(), + access_token, code: 'code' in input ? input.code : undefined, }) : undefined @@ -361,13 +363,11 @@ export class TokenManager { const responseTypes = parameters.response_type.split(' ') const id_token = responseTypes.includes('id_token') - ? await this.signer.idToken( - client, - parameters, - account, - info.authTime, - { exp: dateToEpoch(expiresAt), access_token }, - ) + ? await this.signer.idToken(client, parameters, account, { + exp: dateToEpoch(expiresAt), + auth_time: info.authTime, + access_token, + }) : undefined const tokenResponse: TokenResponse = { diff --git a/packages/oauth-provider/src/util/crypto.ts b/packages/oauth-provider/src/util/crypto.ts index ef18ab0f6f1..96b4c501b53 100644 --- a/packages/oauth-provider/src/util/crypto.ts +++ b/packages/oauth-provider/src/util/crypto.ts @@ -1,4 +1,5 @@ import { randomBytes } from 'node:crypto' +import { KeyLike, calculateJwkThumbprint, exportJWK } from 'jose' export async function randomHexId(bytesLength = 16) { return new Promise((resolve, reject) => { @@ -9,6 +10,10 @@ export async function randomHexId(bytesLength = 16) { }) } +export async function keyJwkThumbprint(key: Uint8Array | KeyLike) { + return calculateJwkThumbprint(await exportJWK(key)) +} + // Basically all algorithms supported by "jose"'s jwtVerify(). // TODO: Is there a way to get this list from the runtime instead of hardcoding it? export const VERIFY_ALGOS = [