Skip to content

feat: add @ascorbic/atproto-oauth-provider package#31

Merged
ascorbic merged 31 commits into
mainfrom
claude/oauth-provider-implementation-IeqVp
Dec 30, 2025
Merged

feat: add @ascorbic/atproto-oauth-provider package#31
ascorbic merged 31 commits into
mainfrom
claude/oauth-provider-implementation-IeqVp

Conversation

@ascorbic
Copy link
Copy Markdown
Owner

Implement OAuth 2.1 provider with AT Protocol extensions:

  • PKCE verification (S256 method)
  • DPoP proof verification using jose library (RFC 9449)
  • Pushed Authorization Requests (PAR) handler (RFC 9126)
  • DID-based client discovery
  • Token generation and validation
  • Authorization consent UI
  • Core OAuth provider class orchestrating the flow

All 49 tests pass covering:

  • PKCE challenge generation and verification
  • DPoP proof verification with key thumbprint
  • PAR push and retrieve flow
  • Full OAuth authorization code flow with DPoP

Implement OAuth 2.1 provider with AT Protocol extensions:

- PKCE verification (S256 method)
- DPoP proof verification using jose library (RFC 9449)
- Pushed Authorization Requests (PAR) handler (RFC 9126)
- DID-based client discovery
- Token generation and validation
- Authorization consent UI
- Core OAuth provider class orchestrating the flow

All 49 tests pass covering:
- PKCE challenge generation and verification
- DPoP proof verification with key thumbprint
- PAR push and retrieve flow
- Full OAuth authorization code flow with DPoP
- Add SqliteOAuthStorage implementing OAuthStorage interface for DO SQLite
- Create oauth.ts module to wire up ATProtoOAuthProvider with PDS
- Add OAuth routes for authorization, token, PAR, and revocation endpoints
- Update AccountDurableObject to initialize and expose OAuth storage
- Add @ascorbic/atproto-oauth-provider as workspace dependency
- Add OAuth RPC methods to AccountDurableObject for each storage operation
- Create DOProxyOAuthStorage class that delegates to RPC methods
- Fix serialization issue where SqliteOAuthStorage couldn't cross DO boundary
- Add 14 OAuth integration tests for endpoints
- Simplify createOAuthApp to take getAccountDO function directly
Security and code quality improvements:

- Use @atproto/syntax ensureValidDid() instead of custom regex for DID validation
- Extract duplicated base64UrlEncode into shared encoding.ts module
- Add randomString utility for cryptographic token generation
- Consolidate all encoding/random generation to single trusted source

This follows the pattern of using official atproto libraries where available.
Add a strict CSP to protect the consent UI from clickjacking and XSS:
- default-src 'none': Deny all by default
- style-src 'unsafe-inline': Allow inline styles (page has no external CSS)
- img-src https: data:: Allow client logos from HTTPS URLs
- form-action 'self': Form can only POST to same origin
- frame-ancestors 'none': Prevent clickjacking
- base-uri 'none': Prevent base tag injection

Export CONSENT_UI_CSP for consumers who may need to customize.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Dec 30, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
atproto-pds 7bb1ae4 Dec 30 2025, 08:50 PM

claude and others added 18 commits December 30, 2025 12:49
Replace custom base64url encoding with jose's well-tested implementation.
Since we already use jose for JWT operations, this reduces custom code
and ensures consistent encoding behavior.
Replace switch statement with a const map for cleaner, more declarative
algorithm parameter lookup.
…ypes

- Use OAuthClientMetadata and oauthClientMetadataSchema for client validation
- Use OAuthParResponse for PAR response type
- Add zod validation for fetched client metadata
- Use OAuthTokenResponse for buildTokenResponse return type
- Use OAuthAuthorizationServerMetadata for server discovery metadata
Add fields required by the atproto OAuth client:
- subject_types_supported: ["public"]
- authorization_response_iss_parameter_supported: true
- client_id_metadata_document_supported: true
The authorization form was losing OAuth params (client_id, redirect_uri,
etc.) on POST because they were originally in the query string for GET
but not included in the form. This fix:

- Adds oauthParams to ConsentUIOptions interface
- Renders OAuth params as hidden form fields in the consent UI
- Parses params from form data on POST requests
- Updates tests to include OAuth params in form data

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AT Protocol OAuth uses fragment mode by default for authorization
redirects (code and state go in URL hash instead of query params).
This matches what bsky.social does. Also includes the 'iss' parameter
in the redirect as required by the spec.

- Default response_mode is now 'fragment'
- Added response_modes_supported to server metadata
- Updated tests to parse hash params

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Similar to the PAR endpoint fix, the token endpoint now accepts both
application/json and application/x-www-form-urlencoded content types.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract body parsing logic into a reusable helper function that handles
both JSON and form-urlencoded content types. The token endpoint now
uses this helper. The helper is exported for potential use in other
modules.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…g error

Refactored the helper to throw RequestBodyError so each handler can
do its own error formatting. Both token endpoint and PAR handler now
use the shared helper.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Export parseRequestBody helper and RequestBodyError from oauth-provider
- Token revocation endpoint now accepts JSON in addition to form-urlencoded
- Updated tests to verify JSON is accepted in PAR and token endpoints

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AT Protocol OAuth requires the subject (user DID) in the token response.
Added sub to GeneratedTokens interface and buildTokenResponse.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a comprehensive OAuth 2.1 provider package (@ascorbic/atproto-oauth-provider) with AT Protocol extensions, implementing PKCE verification (S256), DPoP proof verification (RFC 9449), and Pushed Authorization Requests (RFC 9126). The implementation includes DID-based client discovery, token generation/validation, authorization consent UI, and full integration with the PDS via Durable Objects. All 49 tests pass, covering PKCE, DPoP, PAR, and the complete OAuth authorization code flow.

Key changes:

  • New packages/oauth-provider package with modular OAuth components (provider, storage, PKCE, DPoP, PAR, client resolver)
  • PDS integration via oauth.ts with DO-proxy storage and OAuth endpoints
  • SQLite-backed OAuth storage in Durable Objects with automatic cleanup
  • Authorization consent UI with CSP headers and XSS protection

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
packages/oauth-provider/package.json New package definition with dependencies on @atproto/oauth-types, @atproto/syntax, and jose
packages/oauth-provider/src/provider.ts Core OAuth 2.1 provider orchestrating authorization code flow with PKCE, DPoP, and PAR
packages/oauth-provider/src/pkce.ts PKCE verification implementation with S256 challenge method (RFC 7636)
packages/oauth-provider/src/dpop.ts DPoP proof verification using jose library (RFC 9449) with key thumbprint calculation
packages/oauth-provider/src/par.ts PAR handler implementing RFC 9126 for pushed authorization requests
packages/oauth-provider/src/tokens.ts Opaque token generation and validation with configurable TTLs
packages/oauth-provider/src/storage.ts Storage interface and in-memory implementation for testing
packages/oauth-provider/src/client-resolver.ts DID-based client metadata resolution with caching
packages/oauth-provider/src/ui.ts Authorization consent UI with XSS protection and CSP headers
packages/oauth-provider/src/encoding.ts Shared encoding utilities using jose's base64url
packages/oauth-provider/src/index.ts Package exports for public API
packages/pds/src/oauth.ts OAuth routes integration with PDS via DO-proxy storage
packages/pds/src/oauth-storage.ts SQLite OAuth storage implementation for Durable Objects
packages/pds/src/account-do.ts RPC methods added for OAuth storage operations
packages/pds/src/index.ts OAuth routes mounted to main app
packages/oauth-provider/test/*.test.ts Comprehensive tests for PKCE, DPoP, PAR, and full OAuth flow
packages/pds/test/oauth.test.ts Integration tests for PDS OAuth endpoints
pnpm-lock.yaml Dependency updates including new @atproto packages and jose library
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cachedAt: Date.now(),
};

// 9. Cache metadata
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment numbering skips from step 7 to step 9. Step 8 is missing. This should be renumbered for clarity and consistency.

Suggested change
// 9. Cache metadata
// 8. Cache metadata

Copilot uses AI. Check for mistakes.
Comment thread packages/oauth-provider/src/storage.ts Outdated
Comment on lines +300 to +306
setTimeout(
() => {
this.nonces.delete(nonce);
},
5 * 60 * 1000
);
return true;
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using setTimeout in the InMemoryOAuthStorage implementation is problematic for testing and Cloudflare Workers environments. In Workers, there's no guarantee that the setTimeout callback will execute if the Worker is idle, and timers can accumulate without cleanup. For a test-only implementation, consider either accepting that this is a limitation or documenting that cleanup should be done explicitly via the clear() method. In production, this should be handled by the SQLite storage cleanup() method instead.

Copilot uses AI. Check for mistakes.
Comment thread packages/pds/src/oauth.ts Outdated
Comment on lines +204 to +207
// Try to revoke the token
const accountDO = getAccountDO(c.env);
await accountDO.rpcRevokeToken(token);

Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revokeToken method only revokes by access token, but the token revocation endpoint (RFC 7009) accepts either access or refresh tokens. The OAuth revocation endpoint in oauth.ts passes the token parameter directly to rpcRevokeToken, which means refresh token revocations won't work. Consider adding logic to check both token types or renaming the method to clarify it only handles access tokens.

Copilot uses AI. Check for mistakes.
Comment thread packages/pds/src/oauth.ts
Comment on lines +173 to +201
oauth.post("/oauth/revoke", async (c) => {
// Parse the token from the request (accepts JSON or form-urlencoded)
const contentType = c.req.header("Content-Type") ?? "";
let token: string | undefined;

try {
if (contentType.includes("application/json")) {
const json = await c.req.json();
token = json.token;
} else if (contentType.includes("application/x-www-form-urlencoded")) {
const body = await c.req.text();
const params = Object.fromEntries(new URLSearchParams(body).entries());
token = params.token;
} else {
return c.json(
{ error: "invalid_request", error_description: "Content-Type must be application/json or application/x-www-form-urlencoded" },
400,
);
}
} catch {
return c.json(
{ error: "invalid_request", error_description: "Failed to parse request body" },
400,
);
}

if (!token) {
// Per RFC 7009, return 200 even if no token provided
return c.json({});
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token revocation endpoint accepts requests with empty bodies and returns success per RFC 7009 (line 200-201), but it also tries to parse the body first (lines 178-196) which may throw an error. If the Content-Type header is application/json or application/x-www-form-urlencoded but the body is empty, the parsing will succeed and token will be undefined, which is correctly handled. However, if the Content-Type is missing or invalid, the error response at lines 187-190 will be returned instead of the success response required by RFC 7009. Consider handling missing/invalid Content-Type more gracefully for empty bodies.

Copilot uses AI. Check for mistakes.
Comment thread packages/oauth-provider/src/par.ts Outdated
Comment on lines +78 to +91
// 3. Validate client_id is present
const clientId = params.client_id;
if (!clientId) {
return this.errorResponse("invalid_request", "Missing client_id parameter", 400);
}

// 4. Validate required OAuth parameters
for (const param of REQUIRED_PARAMS) {
if (!params[param]) {
return this.errorResponse("invalid_request", `Missing required parameter: ${param}`, 400);
}
}

// 5. Validate response_type is "code"
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment numbering skips from step 1 to step 3. Step 2 is missing. This should be renumbered for clarity and consistency.

Suggested change
// 3. Validate client_id is present
const clientId = params.client_id;
if (!clientId) {
return this.errorResponse("invalid_request", "Missing client_id parameter", 400);
}
// 4. Validate required OAuth parameters
for (const param of REQUIRED_PARAMS) {
if (!params[param]) {
return this.errorResponse("invalid_request", `Missing required parameter: ${param}`, 400);
}
}
// 5. Validate response_type is "code"
// 2. Validate client_id is present
const clientId = params.client_id;
if (!clientId) {
return this.errorResponse("invalid_request", "Missing client_id parameter", 400);
}
// 3. Validate required OAuth parameters
for (const param of REQUIRED_PARAMS) {
if (!params[param]) {
return this.errorResponse("invalid_request", `Missing required parameter: ${param}`, 400);
}
}
// 4. Validate response_type is "code"

Copilot uses AI. Check for mistakes.
Comment thread packages/oauth-provider/src/ui.ts Outdated
* @returns HTML string
*/
export function renderConsentUI(options: ConsentUIOptions): string {
const { client, scope, authorizeUrl, state, oauthParams, userHandle, showLogin, error } = options;
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable state.

Copilot uses AI. Check for mistakes.
Comment thread packages/oauth-provider/src/tokens.ts Outdated
scope,
dpopJkt,
accessTokenTtl = ACCESS_TOKEN_TTL,
refreshTokenTtl = REFRESH_TOKEN_TTL,
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable refreshTokenTtl.

Suggested change
refreshTokenTtl = REFRESH_TOKEN_TTL,

Copilot uses AI. Check for mistakes.
ascorbic and others added 3 commits December 30, 2025 19:05
- Add DPoP token verification in auth middleware alongside Bearer tokens
- Update XRPC proxy to verify DPoP tokens and create service JWTs
- Export getProvider from oauth.ts for use in auth and proxy
- Update test expectation for malformed auth header error message

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create test/helpers.ts with client-side test utilities
- Move createDpopProof, generateDpopKeyPair to test helpers
- Move generateCodeVerifier, generateCodeChallenge to test helpers
- Remove calculateKeyThumbprint wrapper (tests use jose directly)
- Update all tests to import from helpers file

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add non-null assertion for indexed access in writeDevVars

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/pds/src/oauth.ts Outdated
Comment on lines +25 to +27
* This is needed because the SqliteOAuthStorage object contains a SQL connection
* that can't be serialized across the DO RPC boundary. Instead, we delegate each
* storage operation to individual RPC methods that pass serializable data.
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 8 mentions that the package contains a SQL connection that can't be serialized, but this comment is describing SqliteOAuthStorage, not the DOProxyOAuthStorage class. The phrasing "This is needed because the SqliteOAuthStorage object contains a SQL connection" is slightly unclear - consider rephrasing to "This is needed because SqliteOAuthStorage instances contain a SQL connection that can't be serialized across the DO RPC boundary."

Suggested change
* This is needed because the SqliteOAuthStorage object contains a SQL connection
* that can't be serialized across the DO RPC boundary. Instead, we delegate each
* storage operation to individual RPC methods that pass serializable data.
* This is needed because SqliteOAuthStorage instances contain a SQL connection
* that can't be serialized across the DO RPC boundary. Instead, we delegate each
* storage operation to individual RPC methods that pass only serializable data.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +19
export const CONSENT_UI_CSP =
"default-src 'none'; style-src 'unsafe-inline'; img-src https: data:; form-action 'self'; frame-ancestors 'none'; base-uri 'none'";
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CSP on line 18-19 allows 'unsafe-inline' for styles, which could be avoided by moving the CSS to an external file or using a nonce/hash-based approach. While this is acceptable for a consent UI with inline styles, consider documenting why inline styles are necessary here, or exploring safer alternatives if this UI will be extended in the future.

Copilot uses AI. Check for mistakes.
Comment on lines +176 to +177
} else if (payload.ath !== undefined) {
throw new DpopError('DPoP "ath" claim not allowed without access token', "invalid_dpop");
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check on line 176 rejects requests where ath is present but no accessToken was provided for verification. However, according to RFC 9449 Section 4.3, the ath claim is only required when presenting an access token to a resource server, not at the token endpoint. This logic will incorrectly reject valid DPoP proofs at the token endpoint that don't include ath (which is correct behavior for token endpoint requests). Consider removing this check or making it conditional based on whether an access token is expected in the context.

Suggested change
} else if (payload.ath !== undefined) {
throw new DpopError('DPoP "ath" claim not allowed without access token', "invalid_dpop");

Copilot uses AI. Check for mistakes.
Comment thread packages/pds/src/oauth.ts Outdated
Comment on lines +195 to +197
} else {
return c.json(
{ error: "invalid_request", error_description: "Content-Type must be application/json or application/x-www-form-urlencoded" },
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message on line 197 states "Content-Type must be application/json or application/x-www-form-urlencoded", but according to RFC 7009 Section 2.1, the token revocation endpoint should accept application/x-www-form-urlencoded as the required content type. While accepting JSON is a reasonable extension, the current implementation returns an error for missing Content-Type headers even when the body might be empty (which is handled at line 208). Consider either removing the Content-Type requirement for empty bodies, or clarifying that JSON is an extension to the standard.

Suggested change
} else {
return c.json(
{ error: "invalid_request", error_description: "Content-Type must be application/json or application/x-www-form-urlencoded" },
} else if (!contentType && (!c.req.header("Content-Length") || c.req.header("Content-Length") === "0")) {
// No Content-Type and no body: treat as no token provided (RFC 7009 allows this)
token = undefined;
} else {
return c.json(
{
error: "invalid_request",
error_description:
"Content-Type must be application/x-www-form-urlencoded (per RFC 7009) or application/json",
},

Copilot uses AI. Check for mistakes.
ascorbic and others added 5 commits December 30, 2025 19:53
- Fix comment numbering (step 9 → 8) in client-resolver.ts
- Remove setTimeout from InMemoryOAuthStorage (test-only, not needed)
- Token revocation now handles both access and refresh tokens per RFC 7009

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix comment numbering in par.ts (was 1,3,4... now 1,2,3...)
- Remove unused 'state' variable in ui.ts
- Remove unused 'refreshTokenTtl' variable in tokens.ts
- Improve comment clarity in oauth.ts DOProxyOAuthStorage
- Handle missing Content-Type gracefully in token revocation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Numbered step comments are fragile and add noise. The code is
self-documenting and has proper JSDoc. Kept only comments that
reference RFC sections or explain non-obvious behavior.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make generateCodeChallenge internal (still used by verifyPkceChallenge)
- Remove generateCodeVerifier (tests use their own implementation)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ascorbic ascorbic merged commit c1619fe into main Dec 30, 2025
3 checks passed
@ascorbic ascorbic deleted the claude/oauth-provider-implementation-IeqVp branch December 30, 2025 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants