Skip to content
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

Add OIDC PKCE functionality #4804

Merged
merged 2 commits into from Jan 27, 2024
Merged

Add OIDC PKCE functionality #4804

merged 2 commits into from Jan 27, 2024

Conversation

ssddanbrown
Copy link
Member

@ssddanbrown ssddanbrown commented Jan 25, 2024

Related to #4734.

Todo

  • Check full implementation and library against RFC spec.
  • Cover with PHPUnit testing.
  • Auth system checks (Check each with and without enforcement for post/after compatibility):
    • Jumpcloud
      • Works fine, Could not force PKCE, fails when invalid.
    • Okta
      • Works fine, allows forcing PKCE. Fails in expected cases of invalid/missing value, including invalid when not enforced.
    • Auth0
      • Could not find option to force PKCE. Does fail with invalid PKCE in auth code request.
    • Keycloak
      • Works fine, allows forcing PKCE (via specifying method) and fails without, and invalid when not enforced.
    • Authentik
      • Could not find easy option to force PKCE, but does fail with invalid PKCE.
    • Azure
      • Could not find option to force PKCE, but does fail with invalid PKCE.

Docs Updates

  • Update OIDC guidance to indicate support of PKCE, and advise enforcement where possible for extra security.
  • Update advisory to advise enforcement? Probably a good idea.

Related to #4734.
Uses core logic from League AbstractProvider.
Also compared full flow to RFC spec during this process
@ssddanbrown ssddanbrown merged commit 415663a into development Jan 27, 2024
21 checks passed
@ssddanbrown ssddanbrown deleted the oidc_pkce branch January 27, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

1 participant