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

Add ability to register custom Grant Flows #1418

Merged
merged 8 commits into from
May 27, 2020

Conversation

nbulaj
Copy link
Member

@nbulaj nbulaj commented May 26, 2020

Allow flexible extending of Doorkeeper with a new OAuth Grant Flows without adding them to the gem itself.

A new flow is registered like so:

Doorkeeper::GrantFlow.register :authorization_code,
  response_type_matches: 'code',
  response_type_strategy: Doorkeeper::Request::Code,
  grant_type_matches: 'authorization_code',
  grant_type_strategy: Doorkeeper::Request::AuthorizationCode

Based on #733 with additions & retro-compatibility for next extensions:

  • doorkeeper_openid-connect
  • doorkeeper-grant_assertions

This changes allows to fix / implement:

TODO:

  • Implement custom grant flows registration
  • Allow multiple flows aliased with a one name in grant_flows (like in openid_connect)
  • Add specs

Based on #733 - adds the possibility of adding strategy registration to Doorkeeper
lib/doorkeeper/grant_flow/registry.rb Show resolved Hide resolved
lib/doorkeeper/grant_flow/flow.rb Show resolved Hide resolved
lib/doorkeeper/grant_flow/fallback_flow.rb Show resolved Hide resolved
lib/doorkeeper/grant_flow.rb Show resolved Hide resolved
lib/doorkeeper/config.rb Show resolved Hide resolved
@nbulaj nbulaj force-pushed the add-grant-flows-customizations branch from 2863e34 to a0473f9 Compare May 26, 2020 21:40
@nbulaj nbulaj changed the title Add grant flows customizations Add new grant flows registration May 26, 2020
@nbulaj nbulaj added the WIP Work in progress label May 26, 2020
@doorkeeper-bot
Copy link

1 Warning
⚠️ Please squash all your commits to a single one

Generated by 🚫 Danger

@nbulaj nbulaj force-pushed the add-grant-flows-customizations branch 3 times, most recently from 81720a4 to 25e3e80 Compare May 27, 2020 12:40
@nbulaj nbulaj force-pushed the add-grant-flows-customizations branch from 19f80a7 to 9621845 Compare May 27, 2020 12:48
@nbulaj nbulaj force-pushed the add-grant-flows-customizations branch from c32eb15 to 4c37438 Compare May 27, 2020 16:37
@nbulaj nbulaj changed the title Add new grant flows registration Add ability to register custom Grant Flows May 27, 2020
@nbulaj nbulaj added this to the 5.5 milestone May 27, 2020
@nbulaj nbulaj merged commit 620d4af into master May 27, 2020
@nbulaj nbulaj deleted the add-grant-flows-customizations branch May 27, 2020 16:48
@nbulaj nbulaj mentioned this pull request May 27, 2020
@nbulaj nbulaj removed the WIP Work in progress label May 29, 2020
nbulaj added a commit that referenced this pull request May 29, 2020
@joeljunstrom
Copy link

@nbulaj this is pretty neat. I had missed that this was a new thing when I fixed a bug in the openid connect lib (a bug that I think exists here in the main repo as well).

The bug consists of when a response type that requires (by spec) that the fragment response mode is used but throws an error in the controller layer will use the default (query) mode instead of the fragment for the error redirect. This is due to no checking of the response type at the controller level raise / rescue handling.

Looking at this I think it might make sense to let the grant flow strategy own the information about which response modes are available / default or MUST be used.

Would you be open to such a patch? Or are there alternative ideas on how response modes should be handled going forward?

My fix is here doorkeeper-gem/doorkeeper-openid_connect#118 but that is just fixing the already brittle monkey patching.

@rafaelsales
Copy link
Contributor

@nbulaj We're about to use this feature in prod - would be nice to have this released in 5.5.0

@k-rudy
Copy link

k-rudy commented Jan 12, 2022

@nbulaj thank you so much for the job.

What's the preferred place to put Doorkeeper::GrantFlow.register now? Also should we still indicate the name of the custom grant in grant_flows %w[authorization_code ...] config setting?

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

Successfully merging this pull request may close these issues.

None yet

6 participants