Skip to content

Conversation

@Ross65536
Copy link

Implement PKCE for authorization code grant type, as per RFC-7637:

  • add string fields code_challenge and code_challenge_method to oauth_access_grants table which will contain the PKCE information. Add instructions for upgrading.
  • add config option use_pkce. If true when issuing grants the code_challenge and code_challenge_method query fields are mandatory and are saved to the grant model. If false these fields are ignored and the corresponding grant models are set to nil
  • when emitting an access token, the grant model is checking for having the code_challenge_method field set. If set to a value not nil then the code_verifier query field is mandatory and it's used to check against the grant's code_challenge field. If set to nil this query parameter is ignored and the grant acts as though PKCE is disabled.

Partially inspired by #61

@dagumak
Copy link

dagumak commented Dec 19, 2021

Aww man! I was going to implement this, but you beat me to it.

@dagumak
Copy link

dagumak commented Dec 19, 2021

I think it would be a good idea to add a test with the examples in Appendix A and Appendix B.

@Ross65536
Copy link
Author

Appendix B

I'll see If I have time later for this. But feel free to contribute with the tests if you need it right away :)

@dagumak
Copy link

dagumak commented Dec 31, 2021

Appendix B

I'll see If I have time later for this. But feel free to contribute with the tests if you need it right away :)

I'm still new to Elixir, but I may give it a try later :)

@lukyanov
Copy link

Waiting for this to be merged to upstream. A very good PR!

Just one note. use_pkce: true forces PKCE for all applications. There are use cases when you want to enable it for some applications only. For example, I want to force it for javascript apps, but don't for server-side apps. What if instead of using the global config parameter we do these things instead:

  1. If code_challenge is present in the authorization request, we assume the client wants PKCE check and enable it.
  2. We add a field to the oauth_applications table something like require_pkce, and if it's true, we fail the authorization if the parameters are not present.

What do you think?

@lukyanov
Copy link

@Ross65536 Made a PR to your repo :)
Ross65536#1

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.

3 participants