Document custom grant flow registration in the README, closes #764 - #1888
Conversation
…#764] The grant flow registry added in 5.5.0 [doorkeeper-gem#1418] resolved the technical ask of doorkeeper-gem#764 — grant types are mapped to strategy classes through explicit registration instead of camelize/constantize, so URN/URI-shaped grant types such as RFC 7522's urn:ietf:params:oauth:grant-type:saml2-bearer work — but the feature was never documented outside the CHANGELOG, and the issue's opening question was precisely where the documentation is. Add a README section walking through Doorkeeper::GrantFlow.register with a SAML 2.0 bearer assertion (RFC 7522) example: flow registration and grant_flows configuration, the Request::Strategy subclass, and an OAuth::BaseRequest subclass with grant-specific validations. Note that subclasses must declare attr_reader :access_token themselves — BaseRequest#authorize reads it but does not define it. Pin the example with an end-to-end request spec exercising a URN-shaped grant type: token issuance for a valid assertion, invalid_grant for an unverifiable one, and unsupported_grant_type for unregistered types — the registry previously had unit specs only.
There was a problem hiding this comment.
Pull request overview
This PR documents Doorkeeper’s custom grant flow registry (added in #1418) by adding a README walkthrough for registering a URN/URI-shaped grant type (SAML 2.0 bearer assertion, RFC 7522) and backs it with an end-to-end request spec that exercises /oauth/token for a registered URN grant type.
Changes:
- Add a new README section (“Custom Grant Flows”) explaining
Doorkeeper::GrantFlow.register, strategies, andDoorkeeper::OAuth::BaseRequest-based request objects. - Add a request spec that registers a URN-shaped custom grant type and verifies successful issuance,
invalid_grant, andunsupported_grant_typebehaviors. - Add a changelog entry under
## mainreferencing the documentation + spec addition and closing #764.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/requests/flows/custom_grant_flow_spec.rb | Adds an end-to-end request spec demonstrating URN-shaped custom grant_type selection via the grant flow registry. |
| README.md | Documents how to register and enable custom grant flows, with a SAML bearer assertion example and pointers to related registry features. |
| CHANGELOG.md | Notes the new documentation/spec coverage as a user-visible documentation/testing improvement and links it to #764. |
Suppressed comments (2)
README.md:199
- After adding
validate :client_supports_grant_flow, the example also needs the correspondingvalidate_client_supports_grant_flowimplementation; otherwise the sample won’t run as written.
def validate_client
client.present?
end
spec/requests/flows/custom_grant_flow_spec.rb:48
- After adding
validate :client_supports_grant_flow, the example request needs the correspondingvalidate_client_supports_grant_flowmethod to keep the spec runnable and to mirror how built-in requests enforce per-client grant restrictions.
def validate_client
client.present?
end
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3e8c5d7 to
9d878e2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
README.md:184
- The README example maps
assertion.name_idtoUser.find_by(email: ...), but the repo’s dummy User model only has anamecolumn (and this PR’s end-to-end spec mirrors the example usingname). Keeping the README snippet aligned with the runnable example avoids confusion and makes the “README example is verified by the spec” claim accurate.
def resource_owner
# Map the assertion's subject to a resource owner.
@resource_owner ||= User.find_by(email: assertion.name_id)
end
The example assertion exposes the SAML subject as name_id, but the spec looks the resource owner up by User#name because the dummy app's User model has no other identifying column, while the README example maps the NameID onto email. Spell that out where the struct is defined so the three terms don't read as an inconsistency.
844ceef to
7e60f3e
Compare
|
Yeah this will be cool @ThisIsMissEm , thanks! |
|
@55728 actually, given you're freelance, would you consider setting up GitHub Sponsors? I'd be happy to throw you some money for the assist on getting the large PRs I was working on to land. |
|
Thanks for the nudge @ThisIsMissEm 😊 — just set it up! https://github.com/sponsors/55728 Really appreciate the kind words and the thoughtful reviews on these PRs. Happy to keep pushing things forward together 🦀 |
|
And thank you for the generous sponsorship — it genuinely means a lot, especially as a freelancer. Looking forward to landing these together 🦀 |
|
@55728 freelancer to freelancer and OSS contributor to OSS contributor, I get it. Consider it perhaps my way of paying forward the generosity I've received from others, if that makes sense? it was actually doing a massive context switching between freelance work and OSS work which was why I couldn't pick these PRs up again as easily |
|
That makes a lot of sense 😊 — and it’s exactly the kind of cycle that keeps open source going. The context-switching struggle is real; glad I could help carry some of that load. 🦀 |
Summary
Closes #764.
The technical ask of #764 — support grant types whose names are URNs/URIs, such as the SAML 2.0 bearer assertion grant (
urn:ietf:params:oauth:grant-type:saml2-bearer, RFC 7522 §2.1) — was resolved back in 5.5.0 by the grant flow registry (#1418): grant types are mapped to strategy classes through explicit registration (grant_type_matchesaccepts anyStringorRegexp) instead of the old camelize/constantize resolution the issue complained about.What never happened is documentation: the issue's opening question was literally "Is there any documentation for creating custom grant types?", and to this day the registry appears in neither the README nor the wiki — only in the CHANGELOG and the source. This PR closes that gap:
Doorkeeper::GrantFlow.registerwith a SAML 2.0 bearer assertion (RFC 7522) example — flow registration,grant_flowsconfiguration, theDoorkeeper::Request::Strategysubclass, and aDoorkeeper::OAuth::BaseRequestsubclass with grant-specific validations. Also points atresponse_type_matches/response_type_strategyandregister_aliasfor authorization-endpoint flows and extension aliases./oauth/tokenlevel (token issuance for a valid assertion,invalid_grantfor an unverifiable one,unsupported_grant_typefor unregistered types). The registry so far had unit specs only.The README example is verified by the added spec. One subtlety it documents:
BaseRequest#authorizereadsaccess_tokenbut doesn't define the reader, so subclasses must declareattr_reader :access_tokenthemselves (asPasswordAccessTokenRequestdoes).Docs/test-only change — no behavior changes.