Skip to content

Commit

Permalink
feat(oauth-provider): various improvements
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
matthieusieben committed Feb 19, 2024
1 parent 4432e6b commit 93a4c84
Show file tree
Hide file tree
Showing 14 changed files with 357 additions and 135 deletions.
50 changes: 28 additions & 22 deletions packages/oauth-provider/src/account/account-manager.ts
Original file line number Diff line number Diff line change
@@ -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<Account> {
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<void> {
return this.store.addAccount(deviceId, sub, info)
}

public async get(
Expand All @@ -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<DeviceAccountInfo>,
): Promise<void> {
await this.store.updateAccount(deviceId, sub, info)
}

public async addTrustedClient(
public async list(
deviceId: DeviceId,
sub: Sub,
clientId: ClientId,
): Promise<void> {
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)
}
}
33 changes: 22 additions & 11 deletions packages/oauth-provider/src/account/account-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Account | null>

/**
* @note The `deviceId` might not correspond to a real device meaning that
* {@link SessionStore['createSession']} might never have been called for a
Expand All @@ -25,21 +35,22 @@ export interface AccountStore {
*/
addAccount(
deviceId: DeviceId,
credentials: DeviceAccountCredentials,
): Awaitable<null | { account: Account; info: DeviceAccountInfo }>

listAccounts(
deviceId: DeviceId,
sub?: Sub,
): Awaitable<{ account: Account; info: DeviceAccountInfo }[]>
sub: Sub,
info: DeviceAccountInfo,
): Awaitable<void>

updateAccountInfo(
updateAccount(
deviceId: DeviceId,
sub: Sub,
info: Partial<DeviceAccountInfo>,
): Awaitable<void>

removeAccount(deviceId: DeviceId, sub: Sub): Awaitable<void>

listAccounts(
deviceId: DeviceId,
sub?: Sub,
): Awaitable<{ account: Account; info: DeviceAccountInfo }[]>
}

export function isAccountStore(
Expand All @@ -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'
)
}
Expand Down
1 change: 0 additions & 1 deletion packages/oauth-provider/src/client/client-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@ export type ClientAuth =
alg: string
kid: string
jkt: string
jti: string
}
42 changes: 28 additions & 14 deletions packages/oauth-provider/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -35,15 +36,15 @@ 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,
public readonly metadata: ClientMetadata,
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: [] })
Expand All @@ -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:
Expand All @@ -71,7 +72,7 @@ export class Client {
}
}

async jwtVerifyUnsafe<PayloadType = JWTPayload>(
async jwtVerifyUnsecured<PayloadType = JWTPayload>(
token: string,
options?: Omit<JWTVerifyOptions, 'issuer'>,
): Promise<UnsecuredResult<PayloadType>> {
Expand All @@ -85,7 +86,7 @@ export class Client {
token: string,
options?: Omit<JWTVerifyOptions, 'issuer'>,
): Promise<JWTVerifyResult<PayloadType> & ResolvedKey<KeyLike>> {
return jwtVerify<PayloadType>(token, this.keygetter, {
return jwtVerify<PayloadType>(token, this.keyGetter, {
...options,
issuer: this.id,
})
Expand All @@ -110,11 +111,16 @@ export class Client {
audience: string
maxTokenAge: number
},
): Promise<ClientAuth> {
): 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') {
Expand All @@ -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}`
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions packages/oauth-provider/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
17 changes: 17 additions & 0 deletions packages/oauth-provider/src/oauth-errors.ts
Original file line number Diff line number Diff line change
@@ -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'
Loading

0 comments on commit 93a4c84

Please sign in to comment.