Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure presence of DPoP related response headers #2711

Merged
merged 3 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/giant-starfishes-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/xrpc-server": patch
---

Expose the request context for AuthVerifier and StreamAuthVerifier as distinct types
5 changes: 5 additions & 0 deletions .changeset/happy-eggs-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Properly authenticate OAuth requests in catch all handler.
36 changes: 19 additions & 17 deletions packages/pds/src/auth-verifier.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,28 @@
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,
WWWAuthenticateError,
} 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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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] },
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/pds/src/pipethrough.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
Expand Down
16 changes: 12 additions & 4 deletions packages/xrpc-server/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 AuthVerifier = (
ctx: AuthVerifierContext,
) => Promise<AuthOutput> | AuthOutput

export type StreamAuthVerifier = (ctx: {
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
Expand Down