Skip to content

feat(auth): implement PKCE S256 for all OAuth providers (closes #730)#731

Merged
camathieu merged 1 commit into
masterfrom
feat/pkce-oauth-providers
Apr 5, 2026
Merged

feat(auth): implement PKCE S256 for all OAuth providers (closes #730)#731
camathieu merged 1 commit into
masterfrom
feat/pkce-oauth-providers

Conversation

@camathieu
Copy link
Copy Markdown
Member

Description

What

Implements RFC 7636 PKCE (Proof Key for Code Exchange) S256 across all three OAuth2 login flows: OIDC, Google OAuth, and GitHub OAuth.

Why

Closes #730. Providers like Keycloak (with pkce.code.challenge.method=S256) and Kanidm require PKCE and will reject token exchanges that omit the code_verifier. Without this, OIDC login simply fails against enforcing providers.

Changes

Server handlers (server/handlers/)

  • oidc.go, google.go, github.go: Login generates a verifier via oauth2.GenerateVerifier(), embeds it as pkceVerifier in the signed JWT state, and passes oauth2.S256ChallengeOption(verifier) to AuthCodeURL. Callback extracts pkceVerifier from the state and passes oauth2.VerifierOption(pkceVerifier) to conf.Exchange.
  • Backward-compatible: if pkceVerifier is absent from the state (old clients / rolling deploy), the verifier option is simply not sent.

Keycloak e2e test suite (testing/keycloak/)

  • run.sh: client configured with pkce.code.challenge.method: S256 — Keycloak now rejects any exchange missing a verifier.
  • The existing TestOIDCLoginBrowser passes, proving the happy path works.
  • New TestOIDCLoginBrowserPKCEEnforced: obtains a real authorization code from Keycloak, then calls Plik's callback with a forged state JWT that has no pkceVerifier. Keycloak returns invalid_grant "Code not valid", confirming enforcement is active and the test would fail if PKCE were removed.

Unit tests (server/handlers/*_test.go)

  • Login assertions: code_challenge, code_challenge_method=S256, and pkceVerifier in the state JWT — for all three providers.
  • TestOIDCCallbackPKCEVerifier: verifies the verifier is forwarded to the token endpoint.
  • TestOIDCCallbackPKCEEnforced: mock token server rejects missing verifier.

Testing

  • NO_RACE=1 make test — all packages pass
  • make test-backend keycloak — all 3 Keycloak e2e tests pass including the new enforcement test
  • make lint — clean
  • make docs — builds without errors

Add RFC 7636 PKCE S256 support to all three OAuth2 login flows (OIDC,
Google, GitHub). The implementation is always-on and stateless: a
code_verifier is generated in each Login handler, embedded in the signed
JWT state, and extracted in the Callback handler to pass to conf.Exchange.
No configuration changes are required.

Keycloak's PKCE enforcement is enabled in the e2e test suite
(pkce.code.challenge.method=S256) so that the existing TestOIDCLoginBrowser
test validates the full flow end-to-end. A dedicated negative test
(TestOIDCLoginBrowserPKCEEnforced) verifies that Keycloak rejects an
exchange that omits the code_verifier (real code + forged stateless JWT),
proving the enforcement setting is active.
@camathieu
Copy link
Copy Markdown
Member Author

docker deploy

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 4, 2026

🚀 Deployment Successful!

The instance at plik.root.gg has been redeployed with image rootgg/plik:pr-731.

@Glandos
Copy link
Copy Markdown
Contributor

Glandos commented Apr 4, 2026

That was fast!

I'm using the APT repository, so no Docker image for me. Maybe just a binary? I don't know if go install is able to fetch a PR?

@camathieu
Copy link
Copy Markdown
Member Author

camathieu commented Apr 4, 2026 via email

@Glandos
Copy link
Copy Markdown
Contributor

Glandos commented Apr 4, 2026

That's exactly what I was doing just before reading your comment 🤓

Anyway, I've tested it, after kanidm system oauth2 enable-pkce plik to re-enable PKCE, and now: it works perfectly for me 🥳

@camathieu
Copy link
Copy Markdown
Member Author

camathieu commented Apr 4, 2026 via email

@camathieu camathieu merged commit 9410a29 into master Apr 5, 2026
8 checks passed
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.

PKCE for OAuth

2 participants