refactor(oauth-provider): internalize oauth-provider-client-uri and A…

…TPROTO spec rules

packages/dev-env/src/pds.ts

@@ -45,7 +45,7 @@ export class TestPds {
       modServiceDid: 'did:example:invalid',
       plcRotationKeyK256PrivateKeyHex: plcRotationPriv,
       inviteRequired: false,
-      oauthDisableSsrf: true,
+      fetchDisableSsrf: true,
       oauthProviderName: 'PDS (dev)',
       oauthProviderPrimaryColor: '#ffcb1e',
       oauthProviderLogo:

packages/oauth-provider/package.json

@@ -32,13 +32,17 @@
     "@atproto/fetch-node": "workspace:*",
     "@atproto/jwk": "workspace:*",
     "@atproto/oauth-types": "workspace:*",
+    "@atproto/simple-store": "workspace:*",
+    "@atproto/simple-store-memory": "workspace:*",
+    "@atproto/transformer": "workspace:*",
     "@hapi/accept": "^6.0.3",
     "@hapi/bourne": "^3.0.0",
     "cookie": "^0.6.0",
     "http-errors": "^2.0.0",
     "jose": "^5.2.0",
     "keygrip": "^1.1.0",
     "oidc-token-hash": "^5.0.3",
+    "psl": "^1.9.0",
     "tslib": "^2.6.2",
     "zod": "^3.22.4"
   },

packages/oauth-provider-client-uri/src/oauth-client-uri-store.ts → packages/oauth-provider/src/client/client-store-uri.ts

@@ -1,11 +1,10 @@
-import { CachedGetter, SimpleStore } from '@atproto/simple-store'
-import { SimpleStoreMemory } from '@atproto/simple-store-memory'
 import {
   Fetch,
   fetchFailureHandler,
   fetchJsonProcessor,
   fetchOkProcessor,
 } from '@atproto/fetch'
+import { safeFetchWrap } from '@atproto/fetch-node'
 import { Jwks, jwksSchema } from '@atproto/jwk'
 import {
   Awaitable,
@@ -18,9 +17,13 @@ import {
   oauthClientMetadataSchema,
   parseRedirectUri,
 } from '@atproto/oauth-provider'
+import { CachedGetter, SimpleStore } from '@atproto/simple-store'
+import { SimpleStoreMemory } from '@atproto/simple-store-memory'
 import { compose } from '@atproto/transformer'
 
-import { buildWellknownUrl, isInternetHost, isLoopbackHost } from './util.js'
+import { isInternetHost, isLoopbackHost } from '../lib/util/hostname.js'
+import { isSubUrl } from '../lib/util/path.js'
+import { buildWellknownUrl } from '../lib/util/well-known.js'
 
 export type LoopbackMetadataGetter = (
   url: URL,
@@ -31,35 +34,42 @@ export type ClientMetadataValidator = (
   metadata: OAuthClientMetadata,
 ) => Awaitable<void>
 
-export type OAuthClientUriStoreConfig = {
+export type ClientStoreUriConfig = {
   /**
-   * In prod, it can be useful to enable SSRF & other kinds of protections.
-   * This can be done by providing a custom fetch function that enforces
-   * these protections. If you want to disable all protections, you can
-   * provide `globalThis.fetch` as fetch function.
+   * A custom fetch function that can be used to fetch the client metadata from
+   * the internet. By default, the fetch function is a safeFetchWrap() function
+   * that protects against SSRF attacks, large responses & known bad domains. If
+   * you want to disable all protections, you can provide `globalThis.fetch` as
+   * fetch function.
    */
-  fetch: Fetch
+  safeFetch?: Fetch
 
   /**
    * In order to speed up the client fetching process, you can provide a cache
    * to store HTTP responses.
    *
    * @note the cached entries should automatically expire after a certain time (typically 10 minutes)
    */
-  cache?: SimpleStore<string>
+  clientMetadataCache?: SimpleStore<string>
 
   /**
    * In order to enable loopback clients, you can provide a function that
    * returns the client metadata for a given loopback URL. This is useful for
    * development and testing purposes. This function is not called for internet
    * clients.
+   *
+   * @default is as specified by ATPROTO
    */
   loopbackMetadata?: null | false | LoopbackMetadataGetter
 
   /**
    * A custom function to validate the client metadata. This is useful for
    * enforcing custom rules on the client metadata. This function is called for
    * both loopback and internet clients.
+   *
+   * @default rules defined by the ATPROTO spec in addition to the OAuth spec
+   * rules already enforced by {@link ClientStoreUri.validateMetadata} and
+   * {@link ClientManager}.
    */
   validateMetadata?: null | false | ClientMetadataValidator
 }
@@ -68,23 +78,95 @@ export type OAuthClientUriStoreConfig = {
  * This class is responsible for fetching client data based on it's ID. Since
  * clients are not pre-registered, we need to fetch their data from the network.
  */
-export class OAuthClientUriStore implements ClientStore {
+export class ClientStoreUri implements ClientStore {
   #jsonFetch: CachedGetter<string>
 
   protected readonly loopbackMetadata?: LoopbackMetadataGetter
   protected readonly validateMetadataCustom?: ClientMetadataValidator
 
   constructor({
-    fetch,
-    cache = new SimpleStoreMemory({ maxSize: 50_000_000, ttl: 600e3 }),
-    loopbackMetadata,
-    validateMetadata,
-  }: OAuthClientUriStoreConfig) {
+    safeFetch = safeFetchWrap(),
+    clientMetadataCache = new SimpleStoreMemory({
+      maxSize: 50_000_000,
+      ttl: 600e3,
+    }),
+    loopbackMetadata = ({ href }) => ({
+      client_name: 'Loopback client',
+      client_uri: href,
+      response_types: ['code', 'code id_token'],
+      grant_types: ['authorization_code'],
+      scope: 'openid profile',
+      redirect_uris: ['127.0.0.1', '[::1]'].map(
+        (ip) => Object.assign(new URL(href), { hostname: ip }).href,
+      ) as [string, string],
+      token_endpoint_auth_method: 'none',
+      application_type: 'native',
+      dpop_bound_access_tokens: true,
+    }),
+    validateMetadata = (clientId, clientUrl, metadata) => {
+      // ATPROTO spec requires the use of DPoP (OAuth spec defaults to false)
+      if (metadata.dpop_bound_access_tokens !== true) {
+        throw new InvalidClientMetadataError(
+          '"dpop_bound_access_tokens" must be true',
+        )
+      }
+
+      // ATPROTO spec requires the use of PKCE
+      if (
+        metadata.response_types.some((rt) => rt.split(' ').includes('token'))
+      ) {
+        throw new InvalidClientMetadataError(
+          '"token" response type is not compatible with PKCE (use "code" instead)',
+        )
+      }
+
+      for (const redirectUri of metadata.redirect_uris) {
+        const uri = parseRedirectUri(redirectUri)
+
+        switch (true) {
+          case uri.protocol === 'http:':
+            // Only loopback redirect URIs are allowed to use HTTP
+            switch (uri.hostname) {
+              // ATPROTO spec requires that the IP is used in case of loopback redirect URIs
+              case '127.0.0.1':
+              case '[::1]':
+                continue
+
+              // ATPROTO spec forbids use of localhost as redirect URI hostname
+              case 'localhost':
+                throw new InvalidRedirectUriError(
+                  `Loopback redirect URI ${uri} is not allowed (use explicit IPs instead)`,
+                )
+            }
+
+            // ATPROTO spec forbids http redirects (except for loopback, covered before)
+            throw new InvalidRedirectUriError(
+              `Redirect URI ${uri} must use HTTPS`,
+            )
+
+          // ATPROTO spec requires that the redirect URI is a sub-url of the client URL
+          case uri.protocol === 'https:':
+            if (!isSubUrl(clientUrl, uri)) {
+              throw new InvalidRedirectUriError(
+                `Redirect URI ${uri} must be a sub-url of ${clientUrl}`,
+              )
+            }
+            continue
+
+          // Custom URI schemes are allowed by ATPROTO, following the rules
+          // defined in the spec & current best practices. These are already
+          // enforced by ClientManager & ClientStoreUri
+          default:
+            continue
+        }
+      }
+    },
+  }: ClientStoreUriConfig) {
     this.loopbackMetadata = loopbackMetadata || undefined
     this.validateMetadataCustom = validateMetadata || undefined
 
     const jsonFetch = compose(
-      fetch,
+      safeFetch,
       fetchOkProcessor(),
       fetchJsonProcessor('application/json', false),
       (r: { json: unknown }) => r.json,
@@ -99,7 +181,7 @@ export class OAuthClientUriStore implements ClientStore {
         redirect: 'error',
       })
       return jsonFetch(request).catch(fetchFailureHandler)
-    }, cache)
+    }, clientMetadataCache)
   }
 
   public async findClient(clientId: OAuthClientId): Promise<ClientData> {

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

@@ -16,11 +16,21 @@ export function isClientStore(
   return typeof implementation.findClient === 'function'
 }
 
+export function ifClientStore(
+  implementation?: Record<string, unknown> & Partial<ClientStore>,
+): ClientStore | undefined {
+  if (implementation && isClientStore(implementation)) {
+    return implementation
+  }
+
+  return undefined
+}
+
 export function asClientStore(
   implementation?: Record<string, unknown> & Partial<ClientStore>,
 ): ClientStore {
-  if (!implementation || !isClientStore(implementation)) {
-    throw new Error('Invalid ClientStore implementation')
-  }
-  return implementation
+  const store = ifClientStore(implementation)
+  if (store) return store
+
+  throw new Error('Invalid ClientStore implementation')
 }

packages/oauth-provider-client-uri/src/util.ts → packages/oauth-provider/src/lib/util/hostname.ts

@@ -8,12 +8,3 @@ export function isInternetHost(host: string): boolean {
   const parsed = pslParse(host)
   return 'listed' in parsed && parsed.listed === true
 }
-
-export function buildWellknownUrl(url: URL, name: string): URL {
-  const path =
-    url.pathname === '/'
-      ? `/.well-known/${name}`
-      : `${url.pathname.replace(/\/+$/, '')}/${name}`
-
-  return new URL(path, url)
-}

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

@@ -0,0 +1,17 @@
+import { isAbsolute, relative } from 'node:path'
+
+export function isSubUrl(reference: URL, url: URL): boolean {
+  if (url.origin !== reference.origin) return false
+  if (url.username !== reference.username) return false
+  if (url.password !== reference.password) return false
+
+  return (
+    reference.pathname === url.pathname ||
+    isSubPath(reference.pathname, url.pathname)
+  )
+}
+
+function isSubPath(reference: string, path: string): boolean {
+  const rel = relative(reference, path)
+  return !rel.startsWith('..') && !isAbsolute(rel)
+}

packages/oauth-provider/src/lib/util/redirect-uri.ts

@@ -1,3 +1,5 @@
+import { isLoopbackHost } from './hostname.js'
+
 /**
  *
  * @see {@link https://datatracker.ietf.org/doc/html/rfc8252#section-8.4}
@@ -17,11 +19,7 @@ export function compareRedirectUri(
 
   // https://datatracker.ietf.org/doc/html/rfc8252#section-7.3
   const allowedUri = new URL(allowed_uri)
-  if (
-    allowedUri.hostname === 'localhost' ||
-    allowedUri.hostname === '127.0.0.1' ||
-    allowedUri.hostname === '[::1]'
-  ) {
+  if (isLoopbackHost(allowedUri.hostname)) {
     const requestUri = new URL(request_uri)
 
     // > The authorization server MUST allow any port to be specified at the

packages/oauth-provider/src/lib/util/well-known.ts

@@ -0,0 +1,8 @@
+export function buildWellknownUrl(url: URL, name: string): URL {
+  const path =
+    url.pathname === '/'
+      ? `/.well-known/${name}`
+      : `${url.pathname.replace(/\/+$/, '')}/${name}`
+
+  return new URL(path, url)
+}