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

Option to specify supported PKCE code_challenge_methods supported #1717

Open
ThisIsMissEm opened this issue Jul 24, 2024 · 7 comments
Open

Comments

@ThisIsMissEm
Copy link
Contributor

In OAuth 2.0 Security Best Current Practices, the PKCE code_challenge_method of plain MUST NOT be used, as it leaks the code_verifier in the authorization request:

When using PKCE, clients SHOULD use PKCE code challenge methods that do not expose the PKCE verifier in the authorization request. Otherwise, attackers that can read the authorization request (cf. Attacker A4 in Section 3) can break the security provided by PKCE. Currently, S256 is the only such method.

Doorkeeper's PKCE support currently supports both plain and S256 and there is no easy way to disable 'plain' to be inline with security best current practices.

Potential Resolution

Add a code_challenge_methods_supported option that is an array of methods, defaulting to ['plain', 'S256'] which is then used when validating the code_challenge_method.

Additionally this should be exposed via Doorkeeper.configuration.code_challenge_methods_supported, such that servers implementing OAuth Authorization Server Metadata (also now recommended, issue #1587) can return code_challenge_methods_supported property.

@ThisIsMissEm
Copy link
Contributor Author

In this Mastodon pull request, I've worked around this by extending Doorkeeper's Doorkeeper::OAuth::PreAuthorization

@ransombriggs
Copy link
Contributor

I am also interested in this feature since I did a similar, but less elegant workaround in my own codebase to disable plain

@ThisIsMissEm
Copy link
Contributor Author

@nbulaj would you be interested in a pull request to add said code_challenge_methods_supported configuration option?

@nbulaj
Copy link
Member

nbulaj commented Aug 1, 2024

Yeah @ThisIsMissEm , all the above sounds reasonable to me 👍

@ThisIsMissEm
Copy link
Contributor Author

(Now I just need to figure out #1721 such that I can contribute this feature change)

@nbulaj
Copy link
Member

nbulaj commented Aug 2, 2024

TBH I don't use docker for local development so not sure which setup we have currently for it 😺 On my local machine I'm able to run the test suite without issues 🤔

@kmayer
Copy link
Contributor

kmayer commented Aug 2, 2024

I develop in an environment where I need multiple versions of Ruby, Rails, Gems and on and on and on. 17 year-old legacy systems are like that. I've found that the only way to get a predictable development environment is to isolate each one in devcontainer. It's saved me over and over again. It also saved colleagues who were reviewing my code, but weren't otherwise in active development of it. It was sooo much easier for them to "just" launch the container than install all of the things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants