Remove non-standard *_endpoint_auth_method (bluesky-social#2729)

porkytheblack · Aug 20, 2024 · 35a1264 · 35a1264
1 parent 5131b02
commit 35a1264
Show file tree

Hide file tree

Showing 14 changed files with 117 additions and 186 deletions.
diff --git a/.changeset/cool-moose-argue.md b/.changeset/cool-moose-argue.md
@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": patch
+"@atproto/oauth-client": patch
+---
+
+The non-standard `introspection_endpoint_auth_method`, and `introspection_endpoint_auth_signing_alg` client metadata properties were removed. The client's `token_endpoint_auth_method`, and `token_endpoint_auth_signing_alg` properties are now used as the only indication of how a client must authenticate at the introspection endpoint.
diff --git a/.changeset/eight-poems-pump.md b/.changeset/eight-poems-pump.md
@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": patch
+"@atproto/oauth-client": patch
+---
+
+The non-standard `revocation_endpoint_auth_method`, and `revocation_endpoint_auth_signing_alg` client metadata properties were removed. The client's `token_endpoint_auth_method`, and `token_endpoint_auth_signing_alg` properties are now used as the only indication of how a client must authenticate at the revocation endpoint.
diff --git a/.changeset/hungry-queens-walk.md b/.changeset/hungry-queens-walk.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-types": patch
+---
+
+Avoid code duplication in the definition of OAuthEndpointName
diff --git a/.changeset/modern-pears-camp.md b/.changeset/modern-pears-camp.md
@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": patch
+"@atproto/oauth-client": patch
+---
+
+The non-standard `pushed_authorization_request_endpoint_auth_method`, and `pushed_authorization_request_endpoint_auth_signing_alg` client metadata properties were removed. The client's `token_endpoint_auth_method`, and `token_endpoint_auth_signing_alg` properties are now used as the only indication of how a client must authenticate at the introspection endpoint.
diff --git a/packages/oauth/oauth-client/src/oauth-server-agent.ts b/packages/oauth/oauth-client/src/oauth-server-agent.ts
@@ -206,12 +206,9 @@ export class OAuthServerAgent {
     payload: OAuthClientIdentification
   }> {
     const methodSupported =
-      this.serverMetadata[`${endpoint}_endpoint_auth_methods_supported`] ||
       this.serverMetadata[`token_endpoint_auth_methods_supported`]
 
-    const method =
-      this.clientMetadata[`${endpoint}_endpoint_auth_method`] ||
-      this.clientMetadata[`token_endpoint_auth_method`]
+    const method = this.clientMetadata[`token_endpoint_auth_method`]
 
     if (
       method === 'private_key_jwt' ||
@@ -223,13 +220,9 @@ export class OAuthServerAgent {
 
       try {
         const alg =
-          this.serverMetadata[
-            `${endpoint}_endpoint_auth_signing_alg_values_supported`
-          ] ??
           this.serverMetadata[
             `token_endpoint_auth_signing_alg_values_supported`
-          ] ??
-          FALLBACK_ALG
+          ] ?? FALLBACK_ALG
 
         // If jwks is defined, make sure to only sign using a key that exists in
         // the jwks. If jwks_uri is defined, we can't be sure that the key we're

diff --git a/packages/oauth/oauth-client/src/validate-client-metadata.ts b/packages/oauth/oauth-client/src/validate-client-metadata.ts
@@ -1,16 +1,10 @@
 import { Keyset } from '@atproto/jwk'
-import {
-  OAUTH_AUTHENTICATED_ENDPOINT_NAMES,
-  OAuthClientMetadataInput,
-} from '@atproto/oauth-types'
+import { OAuthClientMetadataInput } from '@atproto/oauth-types'
 
 import { ClientMetadata, clientMetadataSchema } from './types.js'
 
-// Improve bundle size by using concatenation
-const _ENDPOINT_AUTH_METHOD = '_endpoint_auth_method'
-const _ENDPOINT_AUTH_SIGNING_ALG = '_endpoint_auth_signing_alg'
-
-const TOKEN_ENDPOINT_AUTH_METHOD = `token${_ENDPOINT_AUTH_METHOD}`
+const TOKEN_ENDPOINT_AUTH_METHOD = `token_endpoint_auth_method`
+const TOKEN_ENDPOINT_AUTH_SIGNING_ALG = `token_endpoint_auth_signing_alg`
 
 export function validateClientMetadata(
   input: OAuthClientMetadataInput,
@@ -43,36 +37,33 @@ export function validateClientMetadata(
     throw new TypeError(`client_id must be a valid URL`, { cause })
   }
 
-  if (!metadata[TOKEN_ENDPOINT_AUTH_METHOD]) {
-    throw new TypeError(`${TOKEN_ENDPOINT_AUTH_METHOD} must be provided`)
-  }
-
-  for (const endpointName of OAUTH_AUTHENTICATED_ENDPOINT_NAMES) {
-    const method = metadata[`${endpointName}${_ENDPOINT_AUTH_METHOD}`]
-    switch (method) {
-      case undefined:
-      case 'none':
-        if (metadata[`${endpointName}${_ENDPOINT_AUTH_SIGNING_ALG}`]) {
-          throw new TypeError(
-            `${endpointName}${_ENDPOINT_AUTH_SIGNING_ALG} must not be provided`,
-          )
-        }
-        break
-      case 'private_key_jwt':
-        if (!keyset) {
-          throw new TypeError(`Keyset is required for ${method} method`)
-        }
-        if (!metadata[`${endpointName}${_ENDPOINT_AUTH_SIGNING_ALG}`]) {
-          throw new TypeError(
-            `${endpointName}${_ENDPOINT_AUTH_SIGNING_ALG} must be provided`,
-          )
-        }
-        break
-      default:
+  const method = metadata[TOKEN_ENDPOINT_AUTH_METHOD]
+  switch (method) {
+    case undefined:
+      throw new TypeError(`${TOKEN_ENDPOINT_AUTH_METHOD} must be provided`)
+    case 'none':
+      if (metadata[TOKEN_ENDPOINT_AUTH_SIGNING_ALG]) {
         throw new TypeError(
-          `Invalid "${endpointName}${_ENDPOINT_AUTH_METHOD}" value: ${method}`,
+          `${TOKEN_ENDPOINT_AUTH_SIGNING_ALG} must not be provided when ${TOKEN_ENDPOINT_AUTH_METHOD} is "${method}"`,
         )
-    }
+      }
+      break
+    case 'private_key_jwt':
+      if (!keyset?.size) {
+        throw new TypeError(
+          `A non-empty keyset must be provided when ${TOKEN_ENDPOINT_AUTH_METHOD} is "${method}"`,
+        )
+      }
+      if (!metadata[TOKEN_ENDPOINT_AUTH_SIGNING_ALG]) {
+        throw new TypeError(
+          `${TOKEN_ENDPOINT_AUTH_SIGNING_ALG} must be provided when ${TOKEN_ENDPOINT_AUTH_METHOD} is "${method}"`,
+        )
+      }
+      break
+    default:
+      throw new TypeError(
+        `Invalid "token_endpoint_auth_method" value: ${method}`,
+      )
   }
 
   return metadata

diff --git a/packages/oauth/oauth-provider/src/client/client-manager.ts b/packages/oauth/oauth-provider/src/client/client-manager.ts
@@ -17,7 +17,6 @@ import {
   isLoopbackUrl,
   isOAuthClientIdDiscoverable,
   isOAuthClientIdLoopback,
-  OAUTH_AUTHENTICATED_ENDPOINT_NAMES,
   OAuthClientIdDiscoverable,
   OAuthClientIdLoopback,
   OAuthClientMetadata,
@@ -271,49 +270,43 @@ export class ClientManager {
       )
     }
 
-    if (!metadata[`token_endpoint_auth_method`]) {
-      throw new InvalidClientMetadataError(
-        'Missing token_endpoint_auth_method client metadata',
-      )
-    }
-
-    for (const endpoint of OAUTH_AUTHENTICATED_ENDPOINT_NAMES) {
-      const method =
-        metadata[`${endpoint}_endpoint_auth_method`] ||
-        metadata[`token_endpoint_auth_method`]
-
-      switch (method) {
-        case 'none':
-          if (metadata.token_endpoint_auth_signing_alg) {
-            throw new InvalidClientMetadataError(
-              `${endpoint}_endpoint_auth_method "none" must not have ${endpoint}_endpoint_auth_signing_alg`,
-            )
-          }
-          break
+    const method = metadata[`token_endpoint_auth_method`]
+    switch (method) {
+      case undefined:
+        throw new InvalidClientMetadataError(
+          'Missing token_endpoint_auth_method client metadata',
+        )
 
-        case 'private_key_jwt':
-          if (!metadata.jwks && !metadata.jwks_uri) {
-            throw new InvalidClientMetadataError(
-              `private_key_jwt auth method requires jwks or jwks_uri`,
-            )
-          }
-          if (metadata.jwks?.keys.length === 0) {
-            throw new InvalidClientMetadataError(
-              `private_key_jwt auth method requires at least one key in jwks`,
-            )
-          }
-          if (!metadata.token_endpoint_auth_signing_alg) {
-            throw new InvalidClientMetadataError(
-              `Missing token_endpoint_auth_signing_alg client metadata`,
-            )
-          }
-          break
+      case 'none':
+        if (metadata.token_endpoint_auth_signing_alg) {
+          throw new InvalidClientMetadataError(
+            `token_endpoint_auth_method "none" must not have token_endpoint_auth_signing_alg`,
+          )
+        }
+        break
 
-        default:
+      case 'private_key_jwt':
+        if (!metadata.jwks && !metadata.jwks_uri) {
           throw new InvalidClientMetadataError(
-            `${method} is not a supported "${endpoint}_endpoint_auth_method". Use "private_key_jwt" or "none".`,
+            `private_key_jwt auth method requires jwks or jwks_uri`,
           )
-      }
+        }
+        if (metadata.jwks?.keys.length === 0) {
+          throw new InvalidClientMetadataError(
+            `private_key_jwt auth method requires at least one key in jwks`,
+          )
+        }
+        if (!metadata.token_endpoint_auth_signing_alg) {
+          throw new InvalidClientMetadataError(
+            `Missing token_endpoint_auth_signing_alg client metadata`,
+          )
+        }
+        break
+
+      default:
+        throw new InvalidClientMetadataError(
+          `${method} is not a supported "token_endpoint_auth_method". Use "private_key_jwt" or "none".`,
+        )
     }
 
     if (metadata.authorization_encrypted_response_enc) {
@@ -693,16 +686,11 @@ export class ClientManager {
       )
     }
 
-    for (const endpoint of OAUTH_AUTHENTICATED_ENDPOINT_NAMES) {
-      const method =
-        metadata[`${endpoint}_endpoint_auth_method`] ||
-        metadata[`token_endpoint_auth_method`]
-
-      if (method !== 'none') {
-        throw new InvalidClientMetadataError(
-          `Loopback clients are not allowed to use "${endpoint}_endpoint_auth_method" ${method}`,
-        )
-      }
+    const method = metadata[`token_endpoint_auth_method`]
+    if (method !== 'none') {
+      throw new InvalidClientMetadataError(
+        `Loopback clients are not allowed to use "token_endpoint_auth_method" ${method}`,
+      )
     }
 
     for (const redirectUri of metadata.redirect_uris) {
@@ -766,16 +754,14 @@ export class ClientManager {
       }
     }
 
-    for (const endpoint of OAUTH_AUTHENTICATED_ENDPOINT_NAMES) {
-      const method = metadata[`${endpoint}_endpoint_auth_method`]
-      switch (method) {
-        case 'client_secret_post':
-        case 'client_secret_basic':
-        case 'client_secret_jwt':
-          throw new InvalidClientMetadataError(
-            `Client authentication method "${method}" is not allowed for discoverable clients`,
-          )
-      }
+    const method = metadata[`token_endpoint_auth_method`]
+    switch (method) {
+      case 'client_secret_post':
+      case 'client_secret_basic':
+      case 'client_secret_jwt':
+        throw new InvalidClientMetadataError(
+          `Client authentication method "${method}" is not allowed for discoverable clients`,
+        )
     }
 
     for (const redirectUri of metadata.redirect_uris) {

diff --git a/packages/oauth/oauth-provider/src/client/client.ts b/packages/oauth/oauth-provider/src/client/client.ts
@@ -3,7 +3,6 @@ import {
   CLIENT_ASSERTION_TYPE_JWT_BEARER,
   OAuthClientIdentification,
   OAuthClientMetadata,
-  OAuthEndpointName,
 } from '@atproto/oauth-types'
 import {
   UnsecuredJWT,
@@ -101,21 +100,13 @@ export class Client {
     })
   }
 
-  protected getAuthMethod(endpoint: OAuthEndpointName) {
-    return (
-      this.metadata[`${endpoint}_endpoint_auth_method`] ||
-      this.metadata[`token_endpoint_auth_method`]
-    )
-  }
-
   /**
    * @see {@link https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1}
    * @see {@link https://datatracker.ietf.org/doc/html/draft-ietf-oauth-jwt-bearer-11#section-3}
    * @see {@link https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#token-endpoint-auth-method}
    */
   public async verifyCredentials(
     input: OAuthClientIdentification,
-    endpoint: OAuthEndpointName,
     checks: {
       audience: string
     },
@@ -124,7 +115,7 @@ export class Client {
     // for replay protection
     nonce?: string
   }> {
-    const method = this.getAuthMethod(endpoint)
+    const method = this.metadata[`token_endpoint_auth_method`]
 
     if (method === 'none') {
       const clientAuth: ClientAuth = { method: 'none' }
@@ -192,7 +183,7 @@ export class Client {
     }
 
     throw new InvalidClientMetadataError(
-      `Unsupported ${endpoint}_endpoint_auth_method "${method}"`,
+      `Unsupported token_endpoint_auth_method "${method}"`,
     )
   }
 
@@ -204,11 +195,11 @@ export class Client {
    */
   public async validateClientAuth(clientAuth: ClientAuth): Promise<boolean> {
     if (clientAuth.method === 'none') {
-      return this.getAuthMethod('token') === 'none'
+      return this.metadata[`token_endpoint_auth_method`] === 'none'
     }
 
     if (clientAuth.method === CLIENT_ASSERTION_TYPE_JWT_BEARER) {
-      if (this.getAuthMethod('token') !== 'private_key_jwt') {
+      if (this.metadata[`token_endpoint_auth_method`] !== 'private_key_jwt') {
         return false
       }
       try {

diff --git a/packages/oauth/oauth-provider/src/metadata/build-metadata.ts b/packages/oauth/oauth-provider/src/metadata/build-metadata.ts
@@ -127,28 +127,14 @@ export function buildMetadata(
     token_endpoint_auth_signing_alg_values_supported: [...VERIFY_ALGOS],
 
     revocation_endpoint: new URL('/oauth/revoke', issuer).href,
-    revocation_endpoint_auth_methods_supported: [
-      ...Client.AUTH_METHODS_SUPPORTED,
-    ],
-    revocation_endpoint_auth_signing_alg_values_supported: [...VERIFY_ALGOS],
 
     introspection_endpoint: new URL('/oauth/introspect', issuer).href,
-    introspection_endpoint_auth_methods_supported: [
-      ...Client.AUTH_METHODS_SUPPORTED,
-    ],
-    introspection_endpoint_auth_signing_alg_values_supported: [...VERIFY_ALGOS],
 
     userinfo_endpoint: new URL('/oauth/userinfo', issuer).href,
     // end_session_endpoint: new URL('/oauth/logout', issuer).href,
 
     // https://datatracker.ietf.org/doc/html/rfc9126#section-5
     pushed_authorization_request_endpoint: new URL('/oauth/par', issuer).href,
-    pushed_authorization_request_endpoint_auth_methods_supported: [
-      ...Client.AUTH_METHODS_SUPPORTED,
-    ],
-    pushed_authorization_request_endpoint_auth_signing_alg_values_supported: [
-      ...VERIFY_ALGOS,
-    ],
 
     require_pushed_authorization_requests: true,