Deprecate query & fragment in DPoP proof htu claim (#3879)

* Properly validate JWK `htu` claim by enforcing URL without query or fragment

* type fix

* Return DPoP validation result from `authenticateRequest`

* Log clients using invalid "htu" claim in DPoP proof

* review comments

* fix lint

* tidy

* rename dpop result to dpop proof

.changeset/metal-oranges-sing.md

@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": minor
+---
+
+Improve validation of DPoP proofs

.changeset/nasty-knives-kick.md

@@ -0,0 +1,5 @@
+---
+"@atproto/jwk": minor
+---
+
+Properly validate JWK `htu` claim by enforcing URL without query or fragment

.changeset/sweet-ways-allow.md

@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Log clients using invalid "htu" claim in DPoP proof

.changeset/weak-cycles-speak.md

@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Return DPoP validation result from `authenticateRequest`

package.json

@@ -54,7 +54,7 @@
     "prettier": "^3.2.5",
     "prettier-config-standard": "^7.0.0",
     "prettier-plugin-tailwindcss": "^0.6.11",
-    "typescript": "^5.8.2"
+    "typescript": "^5.8.3"
   },
   "workspaces": {
     "packages": [

packages/oauth/jwk/src/jwt.ts

@@ -61,6 +61,50 @@ export const jwtHeaderSchema = z
 
 export type JwtHeader = z.infer<typeof jwtHeaderSchema>
 
+/**
+ * @see {@link https://www.rfc-editor.org/rfc/rfc9449.html#section-4.2-4.6}
+ * @see {@link https://www.rfc-editor.org/rfc/rfc9110#section-7.1}
+ */
+export const htuSchema = z.string().superRefine((value, ctx) => {
+  try {
+    const url = new URL(value)
+    if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'Only http: and https: protocols are allowed',
+      })
+    }
+
+    if (url.username || url.password) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'Credentials not allowed',
+      })
+    }
+
+    if (url.search) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'Query string not allowed',
+      })
+    }
+
+    if (url.hash) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'Fragment not allowed',
+      })
+    }
+  } catch (err) {
+    ctx.addIssue({
+      code: z.ZodIssueCode.invalid_string,
+      validation: 'url',
+    })
+  }
+
+  return value
+})
+
 // https://www.iana.org/assignments/jwt/jwt.xhtml
 export const jwtPayloadSchema = z
   .object({
@@ -72,7 +116,7 @@ export const jwtPayloadSchema = z
     iat: z.number().int().optional(),
     jti: z.string().optional(),
     htm: z.string().optional(),
-    htu: z.string().optional(),
+    htu: htuSchema.optional(),
     ath: z.string().optional(),
     acr: z.string().optional(),
     azp: z.string().optional(),

packages/oauth/oauth-provider/src/dpop/dpop-manager.ts

@@ -1,15 +1,18 @@
 import { createHash } from 'node:crypto'
 import { EmbeddedJWK, calculateJwkThumbprint, errors, jwtVerify } from 'jose'
 import { z } from 'zod'
+import { ValidationError } from '@atproto/jwk'
 import { DPOP_NONCE_MAX_AGE } from '../constants.js'
 import { InvalidDpopProofError } from '../errors/invalid-dpop-proof-error.js'
 import { UseDpopNonceError } from '../errors/use-dpop-nonce-error.js'
+import { ifURL } from '../lib/util/cast.js'
 import {
   DpopNonce,
   DpopSecret,
   dpopSecretSchema,
   rotationIntervalSchema,
 } from './dpop-nonce.js'
+import { DpopProof } from './dpop-proof.js'
 
 const { JOSEError } = errors
 
@@ -47,111 +50,163 @@ export class DpopManager {
    * @see {@link https://datatracker.ietf.org/doc/html/rfc9449#section-4.3}
    */
   async checkProof(
-    proof: unknown,
-    htm: string, // HTTP Method
-    htu: string | URL, // HTTP URL
-    accessToken?: string, // Access Token
-  ) {
-    if (Array.isArray(proof) && proof.length === 1) {
-      proof = proof[0]
+    httpMethod: string,
+    httpUrl: Readonly<URL>,
+    httpHeaders: Record<string, undefined | string | string[]>,
+    accessToken?: string,
+  ): Promise<null | DpopProof> {
+    // Fool proofing against use of empty string
+    if (!httpMethod) {
+      throw new TypeError('HTTP method is required')
     }
 
-    if (!proof || typeof proof !== 'string') {
-      throw new InvalidDpopProofError('DPoP proof required')
-    }
+    const proof = extractProof(httpHeaders)
+    if (!proof) return null
 
-    const { protectedHeader, payload } = await jwtVerify<{
-      iat: number
-      jti: string
-    }>(proof, EmbeddedJWK, {
+    const { protectedHeader, payload } = await jwtVerify(proof, EmbeddedJWK, {
       typ: 'dpop+jwt',
-      maxTokenAge: 10,
+      maxTokenAge: 10, // Will ensure presence & validity of "iat" claim
       clockTolerance: DPOP_NONCE_MAX_AGE / 1e3,
-      requiredClaims: ['iat', 'jti'],
     }).catch((err) => {
-      const message =
-        err instanceof JOSEError
-          ? `Invalid DPoP proof (${err.message})`
-          : 'Invalid DPoP proof'
-      throw new InvalidDpopProofError(message, err)
+      throw newInvalidDpopProofError('Failed to verify DPoP proof', err)
     })
 
-    if (!payload.jti || typeof payload.jti !== 'string') {
-      throw new InvalidDpopProofError('Invalid or missing jti property')
+    // @NOTE For legacy & backwards compatibility reason, we cannot use
+    // `jwtPayloadSchema` here as it will reject DPoP proofs containing a query
+    // or fragment component in the "htu" claim.
+
+    // const { ath, htm, htu, jti, nonce } = await jwtPayloadSchema
+    //   .parseAsync(payload)
+    //   .catch((err) => {
+    //     throw buildInvalidDpopProofError('Invalid DPoP proof', err)
+    //   })
+
+    // @TODO Uncomment previous lines (and remove redundant checks bellow) once
+    // we decide to drop legacy support.
+    const { ath, htm, htu, jti, nonce } = payload
+
+    if (nonce !== undefined && typeof nonce !== 'string') {
+      throw newInvalidDpopProofError('Invalid DPoP "nonce" type')
+    }
+
+    if (!jti || typeof jti !== 'string') {
+      throw newInvalidDpopProofError('DPoP "jti" missing')
     }
 
     // Note rfc9110#section-9.1 states that the method name is case-sensitive
-    if (!htm || htm !== payload['htm']) {
-      throw new InvalidDpopProofError('DPoP htm mismatch')
+    if (!htm || htm !== httpMethod) {
+      throw newInvalidDpopProofError('DPoP "htm" mismatch')
     }
 
-    if (
-      payload['nonce'] !== undefined &&
-      typeof payload['nonce'] !== 'string'
-    ) {
-      throw new InvalidDpopProofError('DPoP nonce must be a string')
+    if (!htu || typeof htu !== 'string') {
+      throw newInvalidDpopProofError('Invalid DPoP "htu" type')
     }
 
-    if (!payload['nonce'] && this.dpopNonce) {
+    // > To reduce the likelihood of false negatives, servers SHOULD employ
+    // > syntax-based normalization (Section 6.2.2 of [RFC3986]) and
+    // > scheme-based normalization (Section 6.2.3 of [RFC3986]) before
+    // > comparing the htu claim.
+    //
+    // RFC9449 section 4.3. Checking DPoP Proofs - https://datatracker.ietf.org/doc/html/rfc9449#section-4.3
+    if (!htu || parseHtu(htu) !== normalizeHtuUrl(httpUrl)) {
+      throw newInvalidDpopProofError('DPoP "htu" mismatch')
+    }
+
+    if (!nonce && this.dpopNonce) {
       throw new UseDpopNonceError()
     }
 
-    if (payload['nonce'] && !this.dpopNonce?.check(payload['nonce'])) {
-      throw new UseDpopNonceError('DPoP nonce mismatch')
-    }
-
-    const htuNorm = normalizeHtu(htu)
-    if (!htuNorm) {
-      throw new TypeError('Invalid "htu" argument')
-    }
-
-    if (htuNorm !== normalizeHtu(payload['htu'])) {
-      throw new InvalidDpopProofError('DPoP htu mismatch')
+    if (nonce && !this.dpopNonce?.check(nonce)) {
+      throw new UseDpopNonceError('DPoP "nonce" mismatch')
     }
 
     if (accessToken) {
-      const athBuffer = createHash('sha256').update(accessToken).digest()
-      if (payload['ath'] !== athBuffer.toString('base64url')) {
-        throw new InvalidDpopProofError('DPoP ath mismatch')
+      const accessTokenHash = createHash('sha256').update(accessToken).digest()
+      if (ath !== accessTokenHash.toString('base64url')) {
+        throw newInvalidDpopProofError('DPoP "ath" mismatch')
       }
-    } else if (payload['ath']) {
-      throw new InvalidDpopProofError('DPoP ath not allowed')
+    } else if (ath !== undefined) {
+      throw newInvalidDpopProofError('DPoP "ath" claim not allowed')
     }
 
-    try {
-      return {
-        protectedHeader,
-        payload,
-        jkt: await calculateJwkThumbprint(protectedHeader['jwk']!, 'sha256'), // EmbeddedJWK
+    // @NOTE we can assert there is a jwk because the jwtVerify used the
+    // EmbeddedJWK key getter mechanism.
+    const jwk = protectedHeader.jwk!
+    const jkt = await calculateJwkThumbprint(jwk, 'sha256').catch((err) => {
+      throw newInvalidDpopProofError('Failed to calculate jkt', err)
+    })
+
+    return { jti, jkt, htm, htu }
   }
-    } catch (err) {
-      const message =
-        err instanceof JOSEError ? err.message : 'Failed to calculate jkt'
-      throw new InvalidDpopProofError(message, err)
 }
+
+function extractProof(
+  httpHeaders: Record<string, undefined | string | string[]>,
+): string | null {
+  const dpopHeader = httpHeaders['dpop']
+  switch (typeof dpopHeader) {
+    case 'string':
+      if (dpopHeader) return dpopHeader
+      throw newInvalidDpopProofError('DPoP header cannot be empty')
+    case 'object':
+      // @NOTE the "0" case should never happen a node.js HTTP server will only
+      // return an array if the header is set multiple times.
+      if (dpopHeader.length === 1 && dpopHeader[0]) return dpopHeader[0]!
+      throw newInvalidDpopProofError('DPoP header must contain a single proof')
+    default:
+      return null
   }
 }
 
 /**
- * @note
- * > The htu claim matches the HTTP URI value for the HTTP request in which the
- * > JWT was received, ignoring any query and fragment parts.
+ * Constructs the HTTP URI (htu) claim as defined in RFC9449.
  *
- * > To reduce the likelihood of false negatives, servers SHOULD employ
- * > syntax-based normalization (Section 6.2.2 of [RFC3986]) and scheme-based
- * > normalization (Section 6.2.3 of [RFC3986]) before comparing the htu claim.
- * @see {@link https://datatracker.ietf.org/doc/html/rfc9449#section-4.3 | RFC9449 section 4.3. Checking DPoP Proofs}
+ * The htu claim is the normalized URL of the HTTP request, excluding the query
+ * string and fragment. This function ensures that the URL is normalized by
+ * removing the search and hash components, as well as by using an URL object to
+ * simplify the pathname (e.g. removing dot segments).
+ *
+ * @returns The normalized URL as a string.
+ * @see {@link https://datatracker.ietf.org/doc/html/rfc9449#section-4.3}
  */
-function normalizeHtu(htu: unknown): string | null {
-  // Optimization
-  if (!htu) return null
+function normalizeHtuUrl(url: Readonly<URL>): string {
+  // NodeJS's `URL` normalizes the pathname, so we can just use that.
+  return url.origin + url.pathname
+}
 
-  try {
-    const url = new URL(String(htu))
-    url.hash = ''
-    url.search = ''
-    return url.href
-  } catch {
-    return null
+function parseHtu(htu: string): string {
+  const url = ifURL(htu)
+  if (!url) {
+    throw newInvalidDpopProofError('DPoP "htu" is not a valid URL')
   }
+
+  // @NOTE the checks bellow can be removed once once jwtPayloadSchema is used
+  // to validate the DPoP proof payload as it already performs these checks
+  // (though the htuSchema).
+
+  if (url.password || url.username) {
+    throw newInvalidDpopProofError('DPoP "htu" must not contain credentials')
+  }
+
+  if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+    throw newInvalidDpopProofError('DPoP "htu" must be http or https')
+  }
+
+  // @NOTE For legacy & backwards compatibility reason, we allow a query and
+  // fragment in the DPoP proof's htu. This is not a standard behavior as the
+  // htu is not supposed to contain query or fragment.
+
+  // NodeJS's `URL` normalizes the pathname.
+  return normalizeHtuUrl(url)
+}
+
+function newInvalidDpopProofError(
+  title: string,
+  err?: unknown,
+): InvalidDpopProofError {
+  const msg =
+    err instanceof JOSEError || err instanceof ValidationError
+      ? `${title}: ${err.message}`
+      : title
+  return new InvalidDpopProofError(msg, err)
 }

packages/oauth/oauth-provider/src/dpop/dpop-proof.ts

@@ -0,0 +1,6 @@
+export type DpopProof = {
+  jti: string
+  jkt: string
+  htm: string
+  htu: string
+}

packages/oauth/oauth-provider/src/lib/util/authorization-header.ts

@@ -11,8 +11,8 @@ export const authorizationHeaderSchema = z.tuple([
   oauthAccessTokenSchema,
 ])
 
-export const parseAuthorizationHeader = (header?: string) => {
-  if (header == null) {
+export const parseAuthorizationHeader = (header: unknown) => {
+  if (typeof header !== 'string') {
     throw new WWWAuthenticateError(
       'invalid_request',
       'Authorization header required',

packages/oauth/oauth-provider/src/lib/util/cast.ts

@@ -2,3 +2,17 @@ export function asArray<T>(value: T | T[]): T[] {
   if (value == null) return []
   return Array.isArray(value) ? value : [value]
 }
+
+export function asURL(value: string | { toString: () => string }): URL {
+  return new URL(value)
+}
+
+export function ifURL(
+  value: string | { toString: () => string },
+): URL | undefined {
+  try {
+    return asURL(value)
+  } catch {
+    return undefined
+  }
+}

packages/oauth/oauth-provider/src/oauth-provider.ts

@@ -69,7 +69,11 @@ import { LocalizedString, MultiLangString } from './lib/util/locale.js'
 import { extractZodErrorMessage } from './lib/util/zod-error.js'
 import { CustomMetadata, buildMetadata } from './metadata/build-metadata.js'
 import { OAuthHooks } from './oauth-hooks.js'
-import { OAuthVerifier, OAuthVerifierOptions } from './oauth-verifier.js'
+import {
+  DpopProof,
+  OAuthVerifier,
+  OAuthVerifierOptions,
+} from './oauth-verifier.js'
 import { ReplayStore, ifReplayStore } from './replay/replay-store.js'
 import { codeSchema } from './request/code.js'
 import { RequestInfo } from './request/request-info.js'
@@ -458,7 +462,7 @@ export class OAuthProvider extends OAuthVerifier {
   public async pushedAuthorizationRequest(
     credentials: OAuthClientCredentials,
     authorizationRequest: OAuthAuthorizationRequestPar,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthParResponse> {
     try {
       const [client, clientAuth] = await this.authenticateClient(credentials)
@@ -474,7 +478,7 @@ export class OAuthProvider extends OAuthVerifier {
           clientAuth,
           parameters,
           null,
-          dpopJkt,
+          dpopProof,
         )
 
       return {
@@ -717,7 +721,7 @@ export class OAuthProvider extends OAuthVerifier {
     clientCredentials: OAuthClientCredentials,
     clientMetadata: RequestMetadata,
     request: OAuthTokenRequest,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthTokenResponse> {
     const [client, clientAuth] =
       await this.authenticateClient(clientCredentials)
@@ -740,7 +744,7 @@ export class OAuthProvider extends OAuthVerifier {
         clientAuth,
         clientMetadata,
         request,
-        dpopJkt,
+        dpopProof,
       )
     }
 
@@ -750,7 +754,7 @@ export class OAuthProvider extends OAuthVerifier {
         clientAuth,
         clientMetadata,
         request,
-        dpopJkt,
+        dpopProof,
       )
     }
 
@@ -764,7 +768,7 @@ export class OAuthProvider extends OAuthVerifier {
     clientAuth: ClientAuth,
     clientMetadata: RequestMetadata,
     input: OAuthAuthorizationCodeGrantTokenRequest,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthTokenResponse> {
     const code = codeSchema.parse(input.code)
     try {
@@ -807,7 +811,7 @@ export class OAuthProvider extends OAuthVerifier {
         deviceId,
         parameters,
         input,
-        dpopJkt,
+        dpopProof,
       )
     } catch (err) {
       // If a token is replayed, requestManager.findCode will throw. In that
@@ -835,14 +839,14 @@ export class OAuthProvider extends OAuthVerifier {
     clientAuth: ClientAuth,
     clientMetadata: RequestMetadata,
     input: OAuthRefreshTokenGrantTokenRequest,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthTokenResponse> {
     return this.tokenManager.refresh(
       client,
       clientAuth,
       clientMetadata,
       input,
-      dpopJkt,
+      dpopProof,
     )
   }
 
@@ -874,24 +878,24 @@ export class OAuthProvider extends OAuthVerifier {
   protected override async verifyToken(
     tokenType: OAuthTokenType,
     token: OAuthAccessToken,
-    dpopJkt: string | null,
+    dpopProof: null | DpopProof,
     verifyOptions?: VerifyTokenClaimsOptions,
   ): Promise<VerifyTokenClaimsResult> {
     if (this.accessTokenMode === AccessTokenMode.stateless) {
-      return super.verifyToken(tokenType, token, dpopJkt, verifyOptions)
+      return super.verifyToken(tokenType, token, dpopProof, verifyOptions)
     }
 
     if (this.accessTokenMode === AccessTokenMode.light) {
-      const { claims } = await super.verifyToken(
+      const { tokenClaims } = await super.verifyToken(
         tokenType,
         token,
-        dpopJkt,
+        dpopProof,
         // Do not verify the scope and audience in case of "light" tokens.
         // these will be checked through the tokenManager hereafter.
         undefined,
       )
 
-      const tokenId = claims.jti
+      const tokenId = tokenClaims.jti
 
       // In addition to verifying the signature (through the verifier above), we
       // also verify the tokenId is still valid using a database to fetch
@@ -900,7 +904,7 @@ export class OAuthProvider extends OAuthVerifier {
         token,
         tokenType,
         tokenId,
-        dpopJkt,
+        dpopProof,
         verifyOptions,
       )
     }

packages/oauth/oauth-provider/src/oauth-verifier.ts

@@ -8,6 +8,7 @@ import {
 } from '@atproto/oauth-types'
 import { DpopManager, DpopManagerOptions } from './dpop/dpop-manager.js'
 import { DpopNonce } from './dpop/dpop-nonce.js'
+import { DpopProof } from './dpop/dpop-proof.js'
 import { InvalidDpopProofError } from './errors/invalid-dpop-proof-error.js'
 import { InvalidTokenError } from './errors/invalid-token-error.js'
 import { UseDpopNonceError } from './errors/use-dpop-nonce-error.js'
@@ -50,7 +51,7 @@ export type OAuthVerifierOptions = Override<
 >
 
 export { DpopNonce, Key, Keyset }
-export type { RedisOptions, ReplayStore, VerifyTokenClaimsOptions }
+export type { DpopProof, RedisOptions, ReplayStore, VerifyTokenClaimsOptions }
 
 export class OAuthVerifier {
   public readonly issuer: OAuthIssuerIdentifier
@@ -95,30 +96,30 @@ export class OAuthVerifier {
   }
 
   public async checkDpopProof(
-    proof: unknown,
-    htm: string,
-    htu: string | URL,
+    httpMethod: string,
+    httpUrl: Readonly<URL>,
+    httpHeaders: Record<string, undefined | string | string[]>,
     accessToken?: string,
-  ): Promise<string | null> {
-    if (proof === undefined) return null
-
-    const { payload, jkt } = await this.dpopManager.checkProof(
-      proof,
-      htm,
-      htu,
+  ): Promise<null | DpopProof> {
+    const dpopProof = await this.dpopManager.checkProof(
+      httpMethod,
+      httpUrl,
+      httpHeaders,
       accessToken,
     )
 
-    const unique = await this.replayManager.uniqueDpop(payload.jti)
-    if (!unique) throw new InvalidDpopProofError('DPoP proof jti is not unique')
+    if (dpopProof) {
+      const unique = await this.replayManager.uniqueDpop(dpopProof.jti)
+      if (!unique) throw new InvalidDpopProofError('DPoP proof replayed')
+    }
 
-    return jkt
+    return dpopProof
   }
 
   protected async verifyToken(
     tokenType: OAuthTokenType,
     token: OAuthAccessToken,
-    dpopJkt: string | null,
+    dpopProof: null | DpopProof,
     verifyOptions?: VerifyTokenClaimsOptions,
   ): Promise<VerifyTokenClaimsResult> {
     if (!isSignedJwt(token)) {
@@ -135,35 +136,37 @@ export class OAuthVerifier {
       token,
       payload.jti,
       tokenType,
-      dpopJkt,
       payload,
+      dpopProof,
       verifyOptions,
     )
   }
 
   public async authenticateRequest(
-    method: string,
-    url: URL,
-    headers: {
-      authorization?: string
-      dpop?: unknown
-    },
+    httpMethod: string,
+    httpUrl: Readonly<URL>,
+    httpHeaders: Record<string, undefined | string | string[]>,
     verifyOptions?: VerifyTokenClaimsOptions,
-  ) {
-    const [tokenType, token] = parseAuthorizationHeader(headers.authorization)
+  ): Promise<VerifyTokenClaimsResult> {
+    const [tokenType, token] = parseAuthorizationHeader(
+      httpHeaders['authorization'],
+    )
     try {
-      const dpopJkt = await this.checkDpopProof(
-        headers.dpop,
-        method,
-        url,
+      const dpopProof = await this.checkDpopProof(
+        httpMethod,
+        httpUrl,
+        httpHeaders,
         token,
       )
 
-      if (tokenType === 'DPoP' && !dpopJkt) {
-        throw new InvalidDpopProofError(`DPoP proof required`)
-      }
+      const tokenResult = await this.verifyToken(
+        tokenType,
+        token,
+        dpopProof,
+        verifyOptions,
+      )
 
-      return await this.verifyToken(tokenType, token, dpopJkt, verifyOptions)
+      return tokenResult
     } catch (err) {
       if (err instanceof UseDpopNonceError) throw err.toWwwAuthenticateError()
       if (err instanceof WWWAuthenticateError) throw err

packages/oauth/oauth-provider/src/request/request-manager.ts

@@ -16,6 +16,8 @@ import { DeviceId } from '../device/device-id.js'
 import { AccessDeniedError } from '../errors/access-denied-error.js'
 import { ConsentRequiredError } from '../errors/consent-required-error.js'
 import { InvalidAuthorizationDetailsError } from '../errors/invalid-authorization-details-error.js'
+import { InvalidDpopKeyBindingError } from '../errors/invalid-dpop-key-binding-error.js'
+import { InvalidDpopProofError } from '../errors/invalid-dpop-proof-error.js'
 import { InvalidGrantError } from '../errors/invalid-grant-error.js'
 import { InvalidParametersError } from '../errors/invalid-parameters-error.js'
 import { InvalidRequestError } from '../errors/invalid-request-error.js'
@@ -23,6 +25,7 @@ import { InvalidScopeError } from '../errors/invalid-scope-error.js'
 import { RequestMetadata } from '../lib/http/request.js'
 import { callAsync } from '../lib/util/function.js'
 import { OAuthHooks } from '../oauth-hooks.js'
+import { DpopProof } from '../oauth-verifier.js'
 import { Signer } from '../signer/signer.js'
 import { Code, generateCode } from './code.js'
 import {
@@ -56,9 +59,9 @@ export class RequestManager {
     clientAuth: ClientAuth,
     input: Readonly<OAuthAuthorizationRequestParameters>,
     deviceId: null | DeviceId,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<RequestInfo> {
-    const parameters = await this.validate(client, clientAuth, input, dpopJkt)
+    const parameters = await this.validate(client, clientAuth, input, dpopProof)
     return this.create(client, clientAuth, parameters, deviceId)
   }
 
@@ -89,7 +92,7 @@ export class RequestManager {
     client: Client,
     clientAuth: ClientAuth,
     parameters: Readonly<OAuthAuthorizationRequestParameters>,
-    dpop_jkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<Readonly<OAuthAuthorizationRequestParameters>> {
     // -------------------------------
     // Validate unsupported parameters
@@ -196,12 +199,11 @@ export class RequestManager {
 
     // https://datatracker.ietf.org/doc/html/rfc9449#section-10
     if (!parameters.dpop_jkt) {
-      if (dpop_jkt) parameters = { ...parameters, dpop_jkt }
-    } else if (parameters.dpop_jkt !== dpop_jkt) {
-      throw new InvalidParametersError(
-        parameters,
-        '"dpop_jkt" parameters does not match the DPoP proof',
-      )
+      if (dpopProof) parameters = { ...parameters, dpop_jkt: dpopProof.jkt }
+    } else if (!dpopProof) {
+      throw new InvalidDpopProofError('DPoP proof required')
+    } else if (parameters.dpop_jkt !== dpopProof.jkt) {
+      throw new InvalidDpopKeyBindingError()
     }
 
     if (clientAuth.method === CLIENT_ASSERTION_TYPE_JWT_BEARER) {

packages/oauth/oauth-provider/src/router/create-oauth-middleware.ts

@@ -101,16 +101,16 @@ export function createOAuthMiddleware<
         .parseAsync(payload, { path: ['body'] })
         .catch(throwInvalidRequest)
 
-      const dpopJkt = await server.checkDpopProof(
-        req.headers['dpop'],
+      const dpopProof = await server.checkDpopProof(
         req.method!,
         this.url,
+        req.headers,
       )
 
       return server.pushedAuthorizationRequest(
         credentials,
         authorizationRequest,
-        dpopJkt,
+        dpopProof,
       )
     }, 201),
   )
@@ -138,17 +138,17 @@ export function createOAuthMiddleware<
         .parseAsync(payload, { path: ['body'] })
         .catch(throwInvalidGrant)
 
-      const dpopJkt = await server.checkDpopProof(
-        req.headers['dpop'],
+      const dpopProof = await server.checkDpopProof(
         req.method!,
         this.url,
+        req.headers,
       )
 
       return server.token(
         clientCredentials,
         clientMetadata,
         tokenRequest,
-        dpopJkt,
+        dpopProof,
       )
     }),
   )

packages/oauth/oauth-provider/src/token/token-manager.ts

@@ -32,6 +32,7 @@ import { RequestMetadata } from '../lib/http/request.js'
 import { dateToEpoch, dateToRelativeSeconds } from '../lib/util/date.js'
 import { callAsync } from '../lib/util/function.js'
 import { OAuthHooks } from '../oauth-hooks.js'
+import { DpopProof } from '../oauth-verifier.js'
 import { Sub } from '../oidc/sub.js'
 import { Code, isCode } from '../request/code.js'
 import { SignedTokenPayload } from '../signer/signed-token-payload.js'
@@ -104,12 +105,12 @@ export class TokenManager {
       | OAuthAuthorizationCodeGrantTokenRequest
       | OAuthClientCredentialsGrantTokenRequest
       | OAuthPasswordGrantTokenRequest,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthTokenResponse> {
     // @NOTE the atproto specific DPoP requirement is enforced though the
     // "dpop_bound_access_tokens" metadata, which is enforced by the
     // ClientManager class.
-    if (client.metadata.dpop_bound_access_tokens && !dpopJkt) {
+    if (client.metadata.dpop_bound_access_tokens && !dpopProof) {
       throw new InvalidDpopProofError('DPoP proof required')
     }
 
@@ -117,8 +118,10 @@ export class TokenManager {
       // Allow clients to bind their access tokens to a DPoP key during
       // token request if they didn't provide a "dpop_jkt" during the
       // authorization request.
-      if (dpopJkt) parameters = { ...parameters, dpop_jkt: dpopJkt }
-    } else if (parameters.dpop_jkt !== dpopJkt) {
+      if (dpopProof) parameters = { ...parameters, dpop_jkt: dpopProof.jkt }
+    } else if (!dpopProof) {
+      throw new InvalidDpopProofError('DPoP proof required')
+    } else if (parameters.dpop_jkt !== dpopProof.jkt) {
       throw new InvalidDpopKeyBindingError()
     }
 
@@ -347,7 +350,7 @@ export class TokenManager {
     clientAuth: ClientAuth,
     clientMetadata: RequestMetadata,
     input: OAuthRefreshTokenGrantTokenRequest,
-    dpopJkt: null | string,
+    dpopProof: null | DpopProof,
   ): Promise<OAuthTokenResponse> {
     const refreshTokenParsed = refreshTokenSchema.safeParse(input.refresh_token)
     if (!refreshTokenParsed.success) {
@@ -381,9 +384,9 @@ export class TokenManager {
       }
 
       if (parameters.dpop_jkt) {
-        if (!dpopJkt) {
+        if (!dpopProof) {
           throw new InvalidDpopProofError('DPoP proof required')
-        } else if (parameters.dpop_jkt !== dpopJkt) {
+        } else if (parameters.dpop_jkt !== dpopProof.jkt) {
           throw new InvalidDpopKeyBindingError()
         }
       }
@@ -531,7 +534,7 @@ export class TokenManager {
     token: OAuthAccessToken,
     tokenType: OAuthTokenType,
     tokenId: TokenId,
-    dpopJkt: string | null,
+    dpopProof: null | DpopProof,
     verifyOptions?: VerifyTokenClaimsOptions,
   ): Promise<VerifyTokenClaimsResult> {
     const tokenInfo = await this.getTokenInfo(tokenId).catch((err) => {
@@ -547,7 +550,7 @@ export class TokenManager {
     const { parameters } = data
 
     // Construct a list of claim, as if the token was a JWT.
-    const claims: SignedTokenPayload = {
+    const tokenClaims: SignedTokenPayload = {
       iss: this.signer.issuer,
       jti: tokenId,
       sub: account.sub,
@@ -566,8 +569,8 @@ export class TokenManager {
       token,
       tokenId,
       tokenType,
-      dpopJkt,
-      claims,
+      tokenClaims,
+      dpopProof,
       verifyOptions,
     )
   }

packages/oauth/oauth-provider/src/token/verify-token-claims.ts

@@ -3,9 +3,13 @@ import { InvalidDpopKeyBindingError } from '../errors/invalid-dpop-key-binding-e
 import { InvalidDpopProofError } from '../errors/invalid-dpop-proof-error.js'
 import { asArray } from '../lib/util/cast.js'
 import { InvalidTokenError } from '../oauth-errors.js'
+import { DpopProof } from '../oauth-verifier.js'
 import { SignedTokenPayload } from '../signer/signed-token-payload.js'
 import { TokenId } from './token-id.js'
 
+const BEARER = 'Bearer' satisfies OAuthTokenType
+const DPOP = 'DPoP' satisfies OAuthTokenType
+
 export type VerifyTokenClaimsOptions = {
   /** One of these audience must be included in the token audience(s) */
   audience?: [string, ...string[]]
@@ -17,48 +21,73 @@ export type VerifyTokenClaimsResult = {
   token: OAuthAccessToken
   tokenId: TokenId
   tokenType: OAuthTokenType
-  claims: SignedTokenPayload
+  tokenClaims: SignedTokenPayload
+  dpopProof: null | DpopProof
 }
 
 export function verifyTokenClaims(
   token: OAuthAccessToken,
   tokenId: TokenId,
   tokenType: OAuthTokenType,
-  dpopJkt: string | null,
-  claims: SignedTokenPayload,
+  tokenClaims: SignedTokenPayload,
+  dpopProof: null | DpopProof,
   options?: VerifyTokenClaimsOptions,
 ): VerifyTokenClaimsResult {
   const dateReference = Date.now()
-  const claimsJkt = claims.cnf?.jkt ?? null
 
-  const expectedTokenType: OAuthTokenType = claimsJkt ? 'DPoP' : 'Bearer'
-  if (expectedTokenType !== tokenType) {
-    throw new InvalidTokenError(expectedTokenType, `Invalid token type`)
+  if (tokenClaims.cnf?.jkt) {
+    // An access token with a cnf.jkt claim must be a DPoP token
+    if (tokenType !== DPOP) {
+      throw new InvalidTokenError(
+        DPOP,
+        `Access token is bound to a DPoP proof, but token type is ${tokenType}`,
+      )
     }
-  if (tokenType === 'DPoP' && !dpopJkt) {
-    throw new InvalidDpopProofError(`jkt is required for DPoP tokens`)
+
+    // DPoP token type must be used with a DPoP proof
+    if (!dpopProof) {
+      throw new InvalidDpopProofError(`DPoP proof required`)
     }
-  if (claimsJkt !== dpopJkt) {
+
+    // DPoP proof must be signed with the key that matches the "cnf" claim
+    if (tokenClaims.cnf.jkt !== dpopProof.jkt) {
       throw new InvalidDpopKeyBindingError()
     }
+  } else {
+    // An access token without a cnf.jkt claim must be a Bearer token
+    if (tokenType !== BEARER) {
+      throw new InvalidTokenError(
+        BEARER,
+        `Bearer token type must be used without a DPoP proof`,
+      )
+    }
+
+    // Unexpected DPoP proof received for a Bearer token
+    if (dpopProof) {
+      throw new InvalidTokenError(
+        BEARER,
+        `DPoP proof not expected for Bearer token type`,
+      )
+    }
+  }
 
   if (options?.audience) {
-    const aud = asArray(claims.aud)
+    const aud = asArray(tokenClaims.aud)
     if (!options.audience.some((v) => aud.includes(v))) {
       throw new InvalidTokenError(tokenType, `Invalid audience`)
     }
   }
 
   if (options?.scope) {
-    const scopes = claims.scope?.split(' ')
+    const scopes = tokenClaims.scope?.split(' ')
     if (!scopes || !options.scope.some((v) => scopes.includes(v))) {
       throw new InvalidTokenError(tokenType, `Invalid scope`)
     }
   }
 
-  if (claims.exp != null && claims.exp * 1000 <= dateReference) {
+  if (tokenClaims.exp != null && tokenClaims.exp * 1000 <= dateReference) {
     throw new InvalidTokenError(tokenType, `Token expired`)
   }
 
-  return { token, tokenId, tokenType, claims }
+  return { token, tokenId, tokenType, tokenClaims, dpopProof }
 }

packages/pds/src/auth-verifier.ts

@@ -490,19 +490,19 @@ export class AuthVerifier {
       const originalUrl =
         ('originalUrl' in req && req.originalUrl) || req.url || '/'
       const url = new URL(originalUrl, this._publicUrl)
-      const result = await this.oauthVerifier.authenticateRequest(
+      const { tokenClaims } = await this.oauthVerifier.authenticateRequest(
         req.method || 'GET',
         url,
         req.headers,
         { audience: [this.dids.pds] },
       )
 
-      const { sub } = result.claims
+      const { sub } = tokenClaims
       if (typeof sub !== 'string' || !sub.startsWith('did:')) {
         throw new InvalidRequestError('Malformed token', 'InvalidToken')
       }
 
-      const oauthScopes = new Set(result.claims.scope?.split(' '))
+      const oauthScopes = new Set(tokenClaims.scope?.split(' '))
 
       if (!oauthScopes.has('transition:generic')) {
         throw new AuthRequiredError(
@@ -535,7 +535,7 @@ export class AuthVerifier {
       return {
         credentials: {
           type: 'oauth',
-          did: result.claims.sub,
+          did: tokenClaims.sub,
           scope: scopeEquivalent,
           oauthScopes,
           isPrivileged: scopeEquivalent === AuthScope.AppPassPrivileged,