Skip to content

feat(oauth): Add CORS support for OAuth device flow with origin validation#107731

Open
BYK wants to merge 16 commits intomasterfrom
feat/oauth-cors-public-clients
Open

feat(oauth): Add CORS support for OAuth device flow with origin validation#107731
BYK wants to merge 16 commits intomasterfrom
feat/oauth-cors-public-clients

Conversation

@BYK
Copy link
Member

@BYK BYK commented Feb 5, 2026

Summary

Adds CORS (Cross-Origin Resource Sharing) support for public OAuth clients running in browsers (SPAs, browser-based CLIs) using the device authorization flow. Origins are validated against the OAuth application's allowed_origins configuration.

Changes

  • Add OAuthCORSMixin for shared CORS handling logic
  • Add CORS headers to /oauth/device/code endpoint for device authorization flow
  • Add CORS headers to /oauth/token endpoint for token exchange
  • Validate request origins against the application's allowed_origins setting

Motivation

Browser-based applications using the OAuth device flow need CORS support to:

  1. Request device codes from /oauth/device/code
  2. Poll for tokens at /oauth/token

Implementation

OAuthCORSMixin (oauth_cors_mixin.py)

New mixin class that provides:

  • CORS preflight (OPTIONS) handling with Access-Control-Max-Age
  • Origin validation against application.get_allowed_origins()
  • Configurable cors_allowed_headers and cors_log_tag per endpoint
  • Warning logs when origins are rejected

OAuth Endpoints

Both oauth_device_authorization.py and oauth_token.py now use OAuthCORSMixin:

  • Device authorization: cors_allowed_headers = "Content-Type"
  • Token endpoint: cors_allowed_headers = "Content-Type, Authorization"

Origin Validation Behavior

Scenario Result
OPTIONS preflight Allow all origins (app unknown yet, POST validates)
POST with allowed_origins configured, origin matches ✅ Allow
POST with allowed_origins configured, origin doesn't match ❌ Reject (no CORS header)
POST with empty allowed_origins ❌ Reject (must be configured)
POST with no Origin header (native client) No CORS headers sent

Security Considerations

Security model:

  • Origins MUST be explicitly configured in allowed_origins for CORS to work
  • Native clients (no Origin header) work without CORS headers
  • Access-Control-Allow-Credentials is intentionally NOT set
  • Without credentials, browsers won't send cookies with requests

We intentionally do NOT:

  • Allow all origins by default (must be configured)
  • Set Access-Control-Allow-Credentials: true
  • Allow credentialed CORS requests from arbitrary origins

RFC References

  • RFC 8628 §5.6 - Native Clients: Public clients that cannot securely store a client secret
  • RFC 6749 §2.1 - Client Types: Defines public vs confidential clients

Usage

To enable browser CORS for an OAuth application:

  1. Go to the OAuth application settings
  2. Add allowed origins to the Allowed Origins field (space-separated)
  3. Browser requests from those origins will now work

Add CORS headers to OAuth device authorization and token endpoints to
enable browser-based public clients (SPAs, browser CLIs) to complete
the device authorization flow.

Changes:
- oauth_device_authorization.py: Add CORS headers and OPTIONS handling
- oauth_token.py: Add CORS headers and OPTIONS handling
- apitoken.py: Return ["*"] from get_allowed_origins() for public clients
- test_apitoken.py: Add tests for get_allowed_origins() behavior

Security: Access-Control-Allow-Credentials is intentionally NOT set.
Public clients use bearer tokens (not cookies), so CSRF protection via
origin checking is unnecessary - the token itself is the credential.

Refs:
- RFC 8628 §5.6: Native/public clients in device flow
- RFC 6749 §2.1: Public vs confidential client types
- RFC 6749 §10.3: Bearer token security model
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 5, 2026
@BYK BYK requested a review from dcramer February 5, 2026 22:52
@BYK BYK marked this pull request as ready for review February 5, 2026 22:52
@BYK BYK requested a review from a team as a code owner February 5, 2026 22:52
@dcramer
Copy link
Member

dcramer commented Feb 5, 2026

is this correct? we had cors support before via allowed domains. allowing any domain to use a public credential creates a phishing attack

@BYK
Copy link
Member Author

BYK commented Feb 5, 2026

is this correct? we had cors support before via allowed domains. allowing any domain to use a public credential creates a phishing attack

We don't have a website field for public apps. We can add one and limit it to that. Not sure I see the phishing attack angle tho as we limit the cookies?

@dcramer
Copy link
Member

dcramer commented Feb 5, 2026

Ah I understand - why would a public app (a CLI) be using the browser to query things? security vuln exists nomatter what here

@dcramer
Copy link
Member

dcramer commented Feb 5, 2026

attack being: i take your public client ID, create a domain like sentry.gg and trick you into logging into it, masquerading as the sentry cli

@BYK
Copy link
Member Author

BYK commented Feb 5, 2026

attack being: i take your public client ID, create a domain like sentry.gg and trick you into logging into it, masquerading as the sentry cli

Can I not do that by just adding sentry.gg as my domain in a confidential app?

@BYK BYK marked this pull request as draft February 5, 2026 23:09
RFC 8252 §8.6 recommends public clients be bound to a registered website
to prevent client impersonation attacks. An attacker could take a legitimate
public client_id and use it on a phishing domain like sentry.gg to trick
users into authenticating.

This change restricts public OAuth clients to only allow CORS requests from
their registered homepage_url. If no homepage_url is set, CORS is disabled
entirely (assuming native app using device flow only).

- Public client with homepage_url: CORS allowed only from that URL
- Public client without homepage_url: No CORS (device flow only)
- Confidential clients: Use configured allowed_origins (unchanged)
@BYK
Copy link
Member Author

BYK commented Feb 5, 2026

Security Fix: Bind Public Clients to homepage_url

Good catch @dcramer! You're right that returning ["*"] for all public clients creates a client impersonation vulnerability.

The Attack Vector

An attacker could:

  1. Take our legitimate public client_id (e.g., Sentry CLI's)
  2. Create a phishing domain like sentry.gg
  3. Complete OAuth device flow - user authenticates thinking it's the real CLI
  4. Make API requests with the token, impersonating the legitimate app

Why Confidential Clients Are Different

Can I not do that by just adding sentry.gg as my domain in a confidential app?

With confidential clients, the attacker would need both:

  • The client_id (public)
  • The client_secret (private, never shared)

Without the secret, they can't complete the OAuth flow. That's the key difference.

RFC-Compliant Solution

Per RFC 8252 §8.6:

"Measures such as claimed 'https' scheme redirects MAY be accepted by authorization servers as identity proof"

I've updated get_allowed_origins() to use the existing homepage_url field on ApiApplication:

Client Type homepage_url CORS Allowed From
Public https://cli.sentry.io Only that origin
Public None No CORS (device flow from native apps)
Confidential N/A Configured allowed_origins

For Browser CLI

The browser-based CLI will need to:

  1. Be hosted at a known domain (e.g., https://cli.sentry.io)
  2. Have that domain registered as homepage_url on its public OAuth app

This binds the public client to a specific origin, preventing impersonation.

The homepage_url binding for public clients needs more discussion.
For now, just add CORS headers to /oauth/device/code and /oauth/token
endpoints to enable browser-based OAuth device flow.
BYK and others added 2 commits February 6, 2026 00:49
- Add origin validation in device authorization and token endpoints
- If allowed_origins is configured, only those origins are permitted
- If allowed_origins is empty, all origins allowed (backwards compatible)
- OPTIONS preflight still allows all origins (app unknown at that point)
- Log warnings when origins are rejected for debugging
Remove backwards-compatible fallback that allowed all origins when
allowed_origins was empty. Now:
- allowed_origins MUST be configured for browser CORS to work
- Native clients (no Origin header) don't get CORS headers
- OPTIONS preflight still allows all (POST validates)
BYK and others added 2 commits February 6, 2026 01:30
- Create OAuthCORSMixin with shared CORS logic
- Reduce duplication between device authorization and token endpoints
- Configurable cors_allowed_headers and cors_log_tag per endpoint
@BYK BYK changed the title feat(oauth): Add CORS support for public OAuth clients in browser feat(oauth): Add CORS support for OAuth device flow with origin validation Feb 6, 2026
@BYK BYK marked this pull request as ready for review February 6, 2026 11:15
- Only allow permissive CORS headers for OPTIONS preflight requests
- POST error responses (before app validation) no longer get CORS headers
- Native clients without Origin header don't trigger spurious warnings

This fixes two security/correctness issues:
1. POST errors with invalid client_id no longer leak error details cross-origin
2. Native CLI clients no longer generate false CORS rejection warnings
The CORS origin validation was doing a direct string comparison which
failed when allowed_origins contained '*'. Now correctly allows all
origins when '*' is in the allowed_origins list, consistent with
is_valid_origin() behavior.
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

BYK and others added 2 commits February 6, 2026 14:42
Move Access-Control-Allow-Methods and Access-Control-Allow-Headers
to only be set when Access-Control-Allow-Origin is also set. This ensures:
- Native clients (no Origin header) receive no CORS headers
- Rejected origins receive no CORS headers
- Only valid requests (OPTIONS preflight and validated POST) get CORS headers

Extracted _set_cors_headers() helper to avoid duplication.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants