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

Enable OAuth2 PKCE extension in Doorkeeper #2312

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Enable OAuth2 PKCE extension in Doorkeeper #2312

merged 2 commits into from
Aug 22, 2023

Commits on Aug 22, 2023

  1. Add test to exercise OAuth2 flow

    This test demonstrates/exercises the OAuth2 interactions involved in the
    Authorization Code Grant type[1]. This is the "happy path" that's
    followed when the user has the signin permission in the application
    they're authorizing to make requests on their behalf.
    
    I'm adding this to give me confidence that I'm not breaking this
    behaviour when I enable the PKCE extension.
    
    Note that some of our "integration" tests are using Rails'
    RequestHelpers[2] (e.g `get` and `post`) while others are using the
    Capybara methods (e.g. `visit`). I'm using the Rails RequestHelpers as I
    need to simulate requests that happen outside of the browser (e.g.  the
    request for the Access Token).
    
    Note that CSRF protection was enabled in our integration tests in
    c69288d. It's not particularly useful
    in these tests so I'm disabling it rather than working out how to
    get/set the authenticity token.
    
    I'm using `ActionDispatch::Integration::Runner#reset!`[3] to reset the
    session between the user approving the Authorization Grant (in the form
    of the Authorization Code) and the client using that Authorization Grant
    to request an Access Token. My hope is that this makes it slightly
    clearer that the Authorization Code request is performed by the user's
    User Agent and the Access Token request, and subsequent request for the
    protected resource, is performed by the client.
    
    [1]: https://oauth.net/2/grant-types/authorization-code/
    [2]: https://api.rubyonrails.org/v7.0.7/classes/ActionDispatch/Integration/RequestHelpers.html
    [3]: https://api.rubyonrails.org/v7.0.7/classes/ActionDispatch/Integration/Runner.html#method-i-reset-21
    chrisroos committed Aug 22, 2023
    Configuration menu
    Copy the full SHA
    2a79ea8 View commit details
    Browse the repository at this point in the history
  2. Enable OAuth2 PKCE extension in Doorkeeper

    I ran `bundle exec rails generate doorkeeper:pkce` to enable the PKCE
    extension[1] in Doorkeeper[2]. Rubocop complained about the migration that
    was generated so I changed it to use `change_table` instead of two calls
    to `add_column`.
    
    The PKCE extension isn't mandatory at this point (as demonstrated by the
    "oauth2 authorization code grant type" test continuing to pass) but it
    will be used if the client supplies a `code_challenge_method` and
    `code_challenge` when requesting the Authorization Code.
    
    It's our intention to mandate the use of PKCE once we can be sure all
    clients of Signon are using it.
    
    The Doorkeeper docs[2] include the following 2 points to consider:
    
    1. If you overrode the doorkeeper/authorizations/new.html.erb view, make
       sure that you have the code_challenge and code_challenge_method hidden
       form fields.
    
    We never render this template and haven't overridden it so this is of no
    concern to us.
    
    2. Also, in case your client is public (e.g. mobile app, single page
       app) note that the Doorkeeper::Application for the client should have
       confidential: false.
    
    We only support confidential clients at the moment so this is of no
    concern to us. Note that we set `Doorkeeper::Application#confidential`
    to true in schema.rb[3] and don't allow it to be overridden in the UI.
    
    [1]: https://datatracker.ietf.org/doc/html/rfc7636
    [2]: https://doorkeeper.gitbook.io/guides/ruby-on-rails/pkce-flow
    [3]: https://github.com/alphagov/signon/blob/78ebf57fe0a787d1e462b39f2475d3b74f085511/db/schema.rb#L114
    chrisroos committed Aug 22, 2023
    Configuration menu
    Copy the full SHA
    74354ec View commit details
    Browse the repository at this point in the history