feat(oauth): Add PKCE support for OAuth2 authorization code flow #104418
Conversation
|
This PR has a migration; here is the generated SQL for for --
-- Add field code_challenge to apigrant
--
ALTER TABLE "sentry_apigrant" ADD COLUMN "code_challenge" varchar(128) NULL;
--
-- Add field code_challenge_method to apigrant
--
ALTER TABLE "sentry_apigrant" ADD COLUMN "code_challenge_method" varchar(10) NULL; |
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Moves PKCE validation and OAuth security checks from the endpoint layer to the model layer. The oauth_token endpoint now calls ApiToken.from_grant() instead of duplicating token creation logic with .objects.create(). Enhances from_grant() to handle redirect_uri binding and PKCE verification inside the lock, fixing TOCTOU race conditions where an application could be deactivated or grant could be reused between validation and token creation. All validations now properly invalidate grants on failure per RFC 6749 §10.5.
|
i think we're good to go |
michelletran-sentry
left a comment
There was a problem hiding this comment.
Generally LGTM. There's just the one question about removing the status filter for getting ApiApplication.
| # once they login, bind their user ID | ||
| if request.user.is_authenticated: | ||
| request.session["oa2"]["uid"] = request.user.id | ||
| # Regenerate session to prevent session fixation attacks |
There was a problem hiding this comment.
I think this prevents attackers (who somehow has the user's session id before they're logged in) from reusing that session after the user has logged in (so now, the attacker can call /authorize again and it will see the user as already authenticated). I'm not sure whether it's simple to get/inject a session id onto a Sentry cookie, so this might not be that exploitable. But from what I've read (on ChatGPT and other sources), cycling the session id after authenticating is a best practice on this endpoint.
|
YOLO |
Implements RFC 7636 Proof Key for Code Exchange (PKCE) to protect OAuth2 authorization code grants from interception attacks. Adds code_challenge and code_challenge_method support to the authorize endpoint, and validates code_verifier in the token endpoint. Supports both S256 and plain methods.
Refs #99002
Replaces #99452