search

doorkeeper-gem / doorkeeper

Doorkeeper is an OAuth 2 provider for Ruby on Rails / Grape.
https://doorkeeper.gitbook.io/guides/
MIT License
5.34k stars 1.07k forks source link

Add pkce_code_challenge_methods option #1735

Closed ThisIsMissEm closed 1 month ago

ThisIsMissEm commented 1 month ago

Summary

This fixes #1717, allowing a user to prevent using the plain method for PKCE code challenges, since using plain is deemed insecure (as noted in the PKCE RFC)

This is based off of #1732, I also have some local changes that pull in irb and debug gems for interactive debugging, which I found helpful when testing in the console via:

$ docker run -it --rm doorkeeper:test /srv/bin/console

Other Information

In validate_pkce_code_challenge_methods I did want to log an error if the database hadn't been configured for PKCE, however, due to how this code is instantiated, I wasn't able to get that working due to the model class not yet being loaded by bin/console (so I'm assuming issues may happen in actual usage too).

The error message does list the supported methods always as plain or S256 — to fix that, we'd need to parameterize the localisation message to receive the supported code challenge methods. I wasn't sure if I should make such a change here.

This also limits us to only ever supporting plain or S256, but to my knowledge no other PKCE code challenge methods exist, and doorkeeper wouldn't have support for them anyway due to how it hardcodes which ones it supports.

ransombriggs commented 2 weeks ago

I pushed up https://github.com/doorkeeper-gem/doorkeeper/pull/1747 to try and fix the error message when plain is passed.