Rework of authorization request hooks (#3525)

* Rework of authorization request hooks

* add device details to `onAuthorized` hook

* Expose port number in DeviceDetails, allow passing options to DeviceManager

* tidy

* tidy

* tidy

* tidy

* changeset

* Expose client request metadata in hooks

* tidy

* tidy

* docs

.changeset/friendly-berries-kick.md

@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": minor
+---
+
+Rename `onClientInfo` and `onAuthorizationDetails` hooks to `getClientInfo` and `getAuthorizationDetails` respectively.
+

.changeset/gentle-islands-impress.md

@@ -0,0 +1,9 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Add the following hooks in the `OAuthProvider`:
+
+- `onAuthorized` which is triggered when the user "authorized" a client (a `code` is issued)
+- `onTokenCreated` which is triggered when the code is exchanged for a token
+- `onTokenRefreshed` which is triggered when a refresh token is exchanged for a new access token

.changeset/seven-falcons-pump.md

@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Dead code cleanup.

packages/oauth/oauth-provider/package.json

@@ -47,7 +47,6 @@
     "http-errors": "^2.0.0",
     "ioredis": "^5.3.2",
     "jose": "^5.2.0",
-    "keygrip": "^1.1.0",
     "psl": "^1.9.0",
     "zod": "^3.23.8"
   },
@@ -59,7 +58,6 @@
     "@rollup/plugin-terser": "^0.4.4",
     "@rollup/plugin-typescript": "^11.1.6",
     "@types/cookie": "^0.6.0",
-    "@types/keygrip": "^1.0.6",
     "@types/psl": "1.1.3",
     "@types/react": "^18.2.50",
     "@types/react-dom": "^18.2.18",

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

@@ -110,17 +110,15 @@ export class ClientManager {
         })
       : undefined
 
-    const partialInfo = this.hooks.onClientInfo
-      ? await callAsync(this.hooks.onClientInfo, clientId, {
-          metadata,
-          jwks,
-        }).catch((err) => {
-          throw InvalidClientMetadataError.from(
-            err,
-            `Rejected client information for "${clientId}"`,
-          )
-        })
-      : undefined
+    const partialInfo = await callAsync(this.hooks.getClientInfo, clientId, {
+      metadata,
+      jwks,
+    }).catch((err) => {
+      throw InvalidClientMetadataError.from(
+        err,
+        `Rejected client information for "${clientId}"`,
+      )
+    })
 
     const isFirstParty = partialInfo?.isFirstParty ?? false
     const isTrusted = partialInfo?.isTrusted ?? isFirstParty

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

@@ -1,10 +1,11 @@
 import { z } from 'zod'
-import { deviceDetailsSchema } from './device-details.js'
 import { sessionIdSchema } from './session-id.js'
 
-export const deviceDataSchema = deviceDetailsSchema.extend({
+export const deviceDataSchema = z.object({
   sessionId: sessionIdSchema,
   lastSeenAt: z.date(),
+  userAgent: z.string().nullable(),
+  ipAddress: z.string(),
 })
 
 export type DeviceData = z.infer<typeof deviceDataSchema>

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

@@ -1,84 +1,89 @@
-import { IncomingMessage, ServerResponse } from 'node:http'
+import type { IncomingMessage, ServerResponse } from 'node:http'
 import { serialize as serializeCookie } from 'cookie'
-import type Keygrip from 'keygrip'
 import { z } from 'zod'
 import { SESSION_FIXATION_MAX_AGE } from '../constants.js'
 import { appendHeader, parseHttpCookies } from '../lib/http/index.js'
+import { RequestMetadata, extractRequestMetadata } from '../lib/http/request.js'
 import { DeviceData } from './device-data.js'
-import { extractDeviceDetails } from './device-details.js'
 import { DeviceId, deviceIdSchema, generateDeviceId } from './device-id.js'
 import { DeviceStore } from './device-store.js'
 import { generateSessionId, sessionIdSchema } from './session-id.js'
 
-export const DEFAULT_OPTIONS = {
+/**
+ * @see {@link https://www.npmjs.com/package/keygrip | Keygrip}
+ */
+export const keygripSchema = z.object({
+  sign: z.function().args(z.any()).returns(z.string()),
+  verify: z.function().args(z.any(), z.string()).returns(z.boolean()),
+  index: z.function().args(z.any(), z.string()).returns(z.number()),
+})
+
+export const deviceManagerOptionsSchema = z.object({
   /**
    * Controls whether the IP address is read from the `X-Forwarded-For` header
    * (if `true`), or from the `req.socket.remoteAddress` property (if `false`).
    *
    * @default true // (nowadays, most requests are proxied)
    */
-  trustProxy: true,
-
+  trustProxy: z.boolean().default(true),
   /**
    * Amount of time (in ms) after which session IDs will be rotated
    *
    * @default 300e3 // (5 minutes)
    */
-  rotationRate: 5 * 60e3,
-
+  rotationRate: z.number().default(300e3),
   /**
    * Cookie options
    */
-  cookie: {
-    keys: undefined as undefined | Keygrip,
-
-    /**
-     * Name of the cookie used to identify the device
-     *
-     * @default 'session-id'
-     */
-    device: 'device-id',
-
-    /**
-     * Name of the cookie used to identify the session
-     *
-     * @default 'session-id'
-     */
-    session: 'session-id',
-
-    /**
-     * Url path for the cookie
-     *
-     * @default '/oauth/authorize'
-     */
-    path: '/oauth/authorize',
-
-    /**
-     * Amount of time (in ms) after which the session cookie will expire.
-     * If set to `null`, the cookie will be a session cookie (deleted when the
-     * browser is closed).
-     *
-     * @default 10 * 365.2 * 24 * 60 * 60e3 // 10 years (in ms)
-     */
-    age: <number | null>(10 * 365.2 * 24 * 60 * 60e3),
-
-    /**
-     * Controls whether the cookie is only sent over HTTPS (if `true`), or also
-     * over HTTP (if `false`). This should **NOT** be set to `false` in
-     * production.
-     */
-    secure: true,
-
-    /**
-     * Controls whether the cookie is sent along with cross-site requests.
-     *
-     * @default 'lax'
-     */
-    sameSite: 'lax' as 'lax' | 'strict',
-  },
-}
+  cookie: z
+    .object({
+      keys: keygripSchema.optional(),
+      /**
+       * Name of the cookie used to identify the device
+       *
+       * @default 'session-id'
+       */
+      device: z.string().default('device-id'),
+      /**
+       * Name of the cookie used to identify the session
+       *
+       * @default 'session-id'
+       */
+      session: z.string().default('session-id'),
+      /**
+       * Url path for the cookie
+       *
+       * @default '/oauth/authorize'
+       */
+      path: z.string().default('/oauth/authorize'),
+      /**
+       * Amount of time (in ms) after which the session cookie will expire.
+       * If set to `null`, the cookie will be a session cookie (deleted when the
+       * browser is closed).
+       *
+       * @default 10 years
+       */
+      age: z
+        .number()
+        .nullable()
+        .default(10 * 365.2 * 24 * 60 * 60e3),
+      /**
+       * Controls whether the cookie is only sent over HTTPS (if `true`), or also
+       * over HTTP (if `false`). This should **NOT** be set to `false` in
+       * production.
+       */
+      secure: z.boolean().default(true),
+      /**
+       * Controls whether the cookie is sent along with cross-site requests.
+       *
+       * @default 'lax'
+       */
+      sameSite: z.enum(['lax', 'strict']).default('lax'),
+    })
+    .default({}),
+})
 
-export type DeviceDeviceManagerOptions = typeof DEFAULT_OPTIONS
+export type DeviceManagerOptions = z.input<typeof deviceManagerOptionsSchema>
 
 const cookieValueSchema = z.tuple([deviceIdSchema, sessionIdSchema])
 type CookieValue = z.infer<typeof cookieValueSchema>
@@ -89,16 +94,23 @@ type CookieValue = z.infer<typeof cookieValueSchema>
  * identify the session.
  */
 export class DeviceManager {
+  private readonly options: z.infer<typeof deviceManagerOptionsSchema>
+
   constructor(
     private readonly store: DeviceStore,
-    private readonly options: DeviceDeviceManagerOptions = DEFAULT_OPTIONS,
-  ) {}
+    options: DeviceManagerOptions = {},
+  ) {
+    this.options = deviceManagerOptionsSchema.parse(options)
+  }
 
   public async load(
     req: IncomingMessage,
     res: ServerResponse,
     forceRotate = false,
-  ): Promise<{ deviceId: DeviceId }> {
+  ): Promise<{
+    deviceId: DeviceId
+    deviceMetadata: RequestMetadata
+  }> {
     const cookie = await this.getCookie(req)
     if (cookie) {
       return this.refresh(
@@ -115,8 +127,11 @@ export class DeviceManager {
   private async create(
     req: IncomingMessage,
     res: ServerResponse,
-  ): Promise<{ deviceId: DeviceId }> {
-    const { userAgent, ipAddress } = this.getDeviceDetails(req)
+  ): Promise<{
+    deviceId: DeviceId
+    deviceMetadata: RequestMetadata
+  }> {
+    const deviceMetadata = this.getRequestMetadata(req)
 
     const [deviceId, sessionId] = await Promise.all([
       generateDeviceId(),
@@ -126,21 +141,24 @@ export class DeviceManager {
     await this.store.createDevice(deviceId, {
       sessionId,
       lastSeenAt: new Date(),
-      userAgent,
-      ipAddress,
+      userAgent: deviceMetadata.userAgent,
+      ipAddress: deviceMetadata.ipAddress,
     })
 
     this.setCookie(res, [deviceId, sessionId])
 
-    return { deviceId }
+    return { deviceId, deviceMetadata }
   }
 
   private async refresh(
     req: IncomingMessage,
     res: ServerResponse,
     [deviceId, sessionId]: CookieValue,
     forceRotate = false,
-  ): Promise<{ deviceId: DeviceId }> {
+  ): Promise<{
+    deviceId: DeviceId
+    deviceMetadata: RequestMetadata
+  }> {
     const data = await this.store.readDevice(deviceId)
     if (!data) return this.create(req, res)
 
@@ -159,24 +177,24 @@ export class DeviceManager {
       }
     }
 
-    const details = this.getDeviceDetails(req)
+    const deviceMetadata = this.getRequestMetadata(req)
 
     if (
       forceRotate ||
-      details.ipAddress !== data.ipAddress ||
-      details.userAgent !== data.userAgent ||
+      deviceMetadata.ipAddress !== data.ipAddress ||
+      deviceMetadata.userAgent !== data.userAgent ||
       age > this.options.rotationRate
     ) {
       await this.rotate(req, res, deviceId, {
-        ipAddress: details.ipAddress,
-        userAgent: details.userAgent || data.userAgent,
+        ipAddress: deviceMetadata.ipAddress,
+        userAgent: deviceMetadata.userAgent || data.userAgent,
       })
     }
 
-    return { deviceId }
+    return { deviceId, deviceMetadata }
   }
 
-  public async rotate(
+  private async rotate(
     req: IncomingMessage,
     res: ServerResponse,
     deviceId: DeviceId,
@@ -284,7 +302,7 @@ export class DeviceManager {
     }
   }
 
-  private getDeviceDetails(req: IncomingMessage) {
-    return extractDeviceDetails(req, this.options.trustProxy)
+  public getRequestMetadata(req: IncomingMessage) {
+    return extractRequestMetadata(req, this.options)
   }
 }

packages/oauth/oauth-provider/src/lib/http/accept.ts

@@ -1,11 +1,7 @@
+import type { IncomingMessage, ServerResponse } from 'node:http'
 import { mediaType } from '@hapi/accept'
 import { SubCtx, subCtx } from './context.js'
-import {
-  IncomingMessage,
-  Middleware,
-  NextFunction,
-  ServerResponse,
-} from './types.js'
+import { Middleware, NextFunction } from './types.js'
 
 type View<
   T,