cloudflare / cloudflare/workers-oauth-provider
feat: surface internal reasons via onError.internal across all generic-error paths
- Dominant language
- TypeScript
- Stars
- 1.9k
- Forks
- 134
- PR merge metrics
- No merged PRs in 30d
Description
## Problem
`OAuthProviderOptions.onError` is the only error-observation channel exposed by the library, and historically its callback input has been tied 1:1 to the wire response: whatever the deployer's hook sees in `description` is what the client sees on the network.
That's a problem for any path where the wire response must stay generic for security reasons (RFC 6749 §5.2 — don't leak which check failed), but the deployer legitimately wants the rich reason for logs / alerting / SIEM:
- The EMA / JWT-bearer grant path (added in #208) — 25 typed validator reasons (`signature_failed`, `replayed`, `lifetime_too_long`, `issuer_not_trusted`, `aud_mismatch`, etc.) all collapse to `invalid_grant: "Invalid assertion"` on the wire. The deployer currently has no way to observe which one fired.
- Many existing throw sites that emit `OAuthError('invalid_grant', '...generic...')` — refresh-token failures, code redemption failures, DPoP/PKCE failures, mismatched redirect URIs, etc. The reason the library has for rejecting is often more specific than what reaches the wire.
## What #208 added
#208 introduced a non-breaking `internal` field on the `onError` input:
```ts
onError?: (error: {
code: string;
description: string;
status: number;
headers: Record;
internal?: { category: string; reason: string; detail?: unknown }; // <-- new, optional
}) => Response | void;
```
The EMA path emits:
```ts
this.createErrorResponse(
wire.code,
{ description: wire.message },
{ category: 'enterprise-managed-authorization', reason: result.error.reason, detail: result.error }
);
```
`createErrorResponse` takes an optional 3rd argument that becomes `error.internal` when forwarded to `onError`. Wire response is untouched.
## What this issue tracks
Adopt the same `internal: { category, reason, detail? }` shape at every existing site where the library deliberately emits a generic wire message but knows a more specific reason internally. Candidates to audit (non-exhaustive):
- `/token` — authorization_code path: grant lookup miss vs. expired vs. wrong redirect_uri vs. wrong PKCE vs. revoked
- `/token` — refresh_token path: token miss vs. expired vs. revoked vs. wrong client vs. reuse-detected
- `/token` — token_exchange path
- `/token` — client authentication: unknown client vs. wrong method vs. wrong secret
- `/authorize` — redirect_uri mismatch vs. invalid_request vs. unauthorized_client
- `/register` — DCR validation failures
- External-token resolution (`resolveExternalToken`)
For each, group categories with stable names (e.g. `category: 'authorization_code_grant'`, `category: 'refresh_token_grant'`, `category: 'client_authentication'`) and pick concise, stable `reason` slugs. Keep the wire response exactly as it is today — this issue is observability-only, not a behavior change.
## Constraints
- **Strictly backwards compatible.** `internal` stays optional on the callback shape; existing `onError` callbacks that ignore the field continue to work.
- **Never leaks on the wire.** The wire response is fully decoupled from `internal` — the field exists *only* on the path from `createErrorResponse` to `onError`.
- **Stable reason slugs.** Once advertised, `category` + `reason` strings are part of the public contract; treat them like enum members in semver.
## References
- #208 — first user of this channel
- RFC 6749 §5.2 — the rule for generic OAuth error responses
Contributor guide
Research direction
Start by reading createErrorResponse and auditing the listed /token, /authorize, /register, and resolveExternalToken paths for generic wire errors. Define stable category and reason slugs, attach them only to onError.internal, and verify that existing wire responses remain unchanged across every audited path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, authentication, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100