-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
docs(api): Add OAuth2 documentation with PKCE support #15904
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
Conversation
Consolidate OAuth documentation by adding comprehensive OAuth2 section to auth.mdx covering authorization flow, token exchange, refresh tokens, and PKCE implementation. Streamline partnership OAuth page to reference the central documentation, reducing duplication. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
coolguyzone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
| "code": code, | ||
| "code_verifier": code_verifier | ||
| }) | ||
| tokens = response.json() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The OAuth callback example code does not check the HTTP status code before parsing the token response, causing a KeyError when the server returns an error.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The example code for the OAuth callback function makes a POST request to the token endpoint but does not validate the HTTP status code of the response. In common failure scenarios, such as an expired authorization code, the OAuth server will return an error response (e.g., HTTP 400) with a JSON body that does not contain access_token or refresh_token keys. The code proceeds to parse this JSON and immediately attempts to access tokens['access_token'], which raises a KeyError and causes an unhandled exception, crashing the application.
💡 Suggested Fix
Before parsing the JSON with response.json(), check if response.status_code is successful (e.g., response.raise_for_status() or if response.status_code == 200). Handle non-200 status codes gracefully instead of attempting to access token keys that will not be present in an error response.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: docs/api/auth.mdx#L200
Potential issue: The example code for the OAuth callback function makes a `POST` request
to the token endpoint but does not validate the HTTP status code of the response. In
common failure scenarios, such as an expired authorization code, the OAuth server will
return an error response (e.g., HTTP 400) with a JSON body that does not contain
`access_token` or `refresh_token` keys. The code proceeds to parse this JSON and
immediately attempts to access `tokens['access_token']`, which raises a `KeyError` and
causes an unhandled exception, crashing the application.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8192205
| "client_id": CLIENT_ID, | ||
| "client_secret": CLIENT_SECRET, | ||
| "grant_type": "refresh_token", | ||
| "refresh_token": session['refresh_token'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The refresh_token function accesses session['refresh_token'] without a check, causing a KeyError if the key is not present in the session.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The refresh_token function in the example code directly accesses session['refresh_token'] without first verifying its existence. If this function is called in a scenario where the refresh token has not yet been stored in the session (e.g., due to a previous error in the callback, an expired session, or being called out of sequence), the application will raise a KeyError. This results in an unhandled exception, crashing the token refresh process.
💡 Suggested Fix
Validate the presence of the refresh_token in the session before using it. Use a safe access method like session.get('refresh_token') and handle the case where the token is None. This prevents a KeyError if the function is called on a session without a valid token.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: docs/api/auth.mdx#L211
Potential issue: The `refresh_token` function in the example code directly accesses
`session['refresh_token']` without first verifying its existence. If this function is
called in a scenario where the refresh token has not yet been stored in the session
(e.g., due to a previous error in the callback, an expired session, or being called out
of sequence), the application will raise a `KeyError`. This results in an unhandled
exception, crashing the token refresh process.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8192205
docs/api/auth.mdxcovering authorization flow, token exchange, refresh tokens, and PKCE implementationRefs getsentry/sentry#104418