Ensure presence of DPoP related response headers (#2711)

* fix(pds): ensure presence of DPoP related response headers

* Expose the request context for AuthVerifier and StreamAuthVerifier as distinct types

* Properly type ReqCtx for stream auth

.changeset/giant-starfishes-fry.md

@@ -0,0 +1,5 @@
+---
+"@atproto/xrpc-server": patch
+---
+
+Expose the request context for AuthVerifier and StreamAuthVerifier as distinct types

.changeset/happy-eggs-swim.md

@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Properly authenticate OAuth requests in catch all handler.

packages/pds/src/auth-verifier.ts

@@ -1,5 +1,8 @@
 import { KeyObject, createPublicKey, createSecretKey } from 'node:crypto'
+import { IncomingMessage, ServerResponse } from 'node:http'
 
+import { getVerificationMaterial } from '@atproto/common'
+import { IdResolver, getDidKeyFromMultibase } from '@atproto/identity'
 import {
   OAuthError,
   OAuthVerifier,
@@ -7,24 +10,19 @@ import {
 } from '@atproto/oauth-provider'
 import {
   AuthRequiredError,
+  AuthVerifierContext,
   ForbiddenError,
   InvalidRequestError,
+  StreamAuthVerifierContext,
   XRPCError,
   verifyJwt as verifyServiceJwt,
 } from '@atproto/xrpc-server'
-import { IdResolver, getDidKeyFromMultibase } from '@atproto/identity'
-import express from 'express'
 import * as jose from 'jose'
 import KeyEncoder from 'key-encoder'
 import { AccountManager } from './account-manager'
 import { softDeleted } from './db'
-import { getVerificationMaterial } from '@atproto/common'
 
-type ReqCtx = {
-  req: express.Request
-  // StreamAuthVerifier does not have "res"
-  res?: express.Response
-}
+type ReqCtx = AuthVerifierContext | StreamAuthVerifierContext
 
 // @TODO sync-up with current method names, consider backwards compat.
 export enum AuthScope {
@@ -462,7 +460,8 @@ export class AuthVerifier {
 
     this.setAuthHeaders(ctx)
 
-    const { req, res } = ctx
+    const { req } = ctx
+    const res = 'res' in ctx ? ctx.res : null
 
     // https://datatracker.ietf.org/doc/html/rfc9449#section-8.2
     if (res) {
@@ -474,9 +473,11 @@ export class AuthVerifier {
     }
 
     try {
-      const url = new URL(req.originalUrl || req.url, this._publicUrl)
+      const originalUrl =
+        ('originalUrl' in req && req.originalUrl) || req.url || '/'
+      const url = new URL(originalUrl, this._publicUrl)
       const result = await this.oauthVerifier.authenticateRequest(
-        req.method,
+        req.method || 'GET',
         url,
         req.headers,
         { audience: [this.dids.pds] },
@@ -619,7 +620,8 @@ export class AuthVerifier {
     }
   }
 
-  protected setAuthHeaders({ res }: ReqCtx) {
+  protected setAuthHeaders(ctx: ReqCtx) {
+    const res = 'res' in ctx ? ctx.res : null
     if (res) {
       res.setHeader('Cache-Control', 'private')
       vary(res, 'Authorization')
@@ -661,22 +663,22 @@ export const parseAuthorizationHeader = (
   )
 }
 
-const isAccessToken = (req: express.Request): boolean => {
+const isAccessToken = (req: IncomingMessage): boolean => {
   const [type] = parseAuthorizationHeader(req.headers.authorization)
   return type === AuthType.BEARER || type === AuthType.DPOP
 }
 
-const isBearerToken = (req: express.Request): boolean => {
+const isBearerToken = (req: IncomingMessage): boolean => {
   const [type] = parseAuthorizationHeader(req.headers.authorization)
   return type === AuthType.BEARER
 }
 
-const isBasicToken = (req: express.Request): boolean => {
+const isBasicToken = (req: IncomingMessage): boolean => {
   const [type] = parseAuthorizationHeader(req.headers.authorization)
   return type === AuthType.BASIC
 }
 
-const bearerTokenFromReq = (req: express.Request) => {
+const bearerTokenFromReq = (req: IncomingMessage) => {
   const [type, token] = parseAuthorizationHeader(req.headers.authorization)
   return type === AuthType.BEARER ? token : null
 }
@@ -715,7 +717,7 @@ export const createPublicKeyObject = (publicKeyHex: string): KeyObject => {
 
 const keyEncoder = new KeyEncoder('secp256k1')
 
-function vary(res: express.Response, value: string) {
+function vary(res: ServerResponse, value: string) {
   const current = res.getHeader('Vary')
   if (current == null || typeof current === 'number') {
     res.setHeader('Vary', value)

packages/pds/src/pipethrough.ts

@@ -21,7 +21,7 @@ export const proxyHandler = (ctx: AppContext): CatchallHandler => {
   return async (req, res, next) => {
     try {
       const { url, aud, nsid } = await formatUrlAndAud(ctx, req)
-      const auth = await accessStandard({ req })
+      const auth = await accessStandard({ req, res })
       if (!auth.credentials.isPrivileged && PRIVILEGED_METHODS.has(nsid)) {
         throw new InvalidRequestError('Bad token method', 'InvalidToken')
       }

packages/xrpc-server/src/types.ts

@@ -90,14 +90,22 @@ export type XRPCStreamHandler = (ctx: {
 
 export type AuthOutput = HandlerAuth | HandlerError
 
-export type AuthVerifier = (ctx: {
+export interface AuthVerifierContext {
   req: express.Request
   res: express.Response
-}) => Promise<AuthOutput> | AuthOutput
+}
 
-export type StreamAuthVerifier = (ctx: {
+export type AuthVerifier = (
+  ctx: AuthVerifierContext,
+) => Promise<AuthOutput> | AuthOutput
+
+export interface StreamAuthVerifierContext {
   req: IncomingMessage
-}) => Promise<AuthOutput> | AuthOutput
+}
+
+export type StreamAuthVerifier = (
+  ctx: StreamAuthVerifierContext,
+) => Promise<AuthOutput> | AuthOutput
 
 export type CalcKeyFn = (ctx: XRPCReqContext) => string | null
 export type CalcPointsFn = (ctx: XRPCReqContext) => number