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

feat: Add a flag to enforce PKCE only for public clients #431

Merged
merged 1 commit into from
May 9, 2020

Conversation

pjcdawkins
Copy link
Contributor

Related issue

Alternative proposal for the issue discussed in #389 and #391, where enforcement of PKCE is wanted only for certain clients.

Proposed changes

Add a new flag EnforcePKCEForPublicClients which enforces PKCE only for public clients. The error hint is slightly different, as it mentions PKCE is enforced for "this client" rather than "clients". (It intentionally does not mention why it's enforced, as I think basing it on public clients is an implementation detail that servers may want to change without adding to the error hints)

I didn't use an enum, as suggested in #391, because I imagine it would be a backwards compatibility problem. I could be convinced otherwise.

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)

@pjcdawkins pjcdawkins changed the title Add EnforcePKCEForPublicClients flag add(pkce): Add a flag to enforce PKCE only for public clients May 7, 2020
@pjcdawkins pjcdawkins changed the title add(pkce): Add a flag to enforce PKCE only for public clients add: A flag to enforce PKCE only for public clients May 7, 2020
@pjcdawkins pjcdawkins changed the title add: A flag to enforce PKCE only for public clients feat: Add a flag to enforce PKCE only for public clients May 7, 2020
@pjcdawkins pjcdawkins force-pushed the enforce-pkce-public-clients branch from dd3c89d to ac4b9e9 Compare May 7, 2020 10:44
@aeneasr
Copy link
Member

aeneasr commented May 9, 2020

Thank you! I'll release this right after merge if you need this in Hydra :)

@aeneasr aeneasr merged commit 9f53c84 into ory:master May 9, 2020
@pjcdawkins pjcdawkins deleted the enforce-pkce-public-clients branch May 9, 2020 13:49
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.

2 participants