Allow custom data to be sent inside access tokens

[JeremyC-za]

Summary

I've added a new config option - custom_access_token_fields. By default, this will be an empty array and have no effect, and when in use, it will be an array of symbols. This array of symbols will then be permitted parameters keys when authorising and granting access to an application. A developer can then configure their OAuth consent screen to allow custom data to be submitted during authorisation. This custom data will then be saved to the Access Grant, and will be included in any subsequently generated access tokens.

Example Use Case

You have a multi-tenanted platform, where our example user has access to tenant_1, tenant_2, and tenant_3. The example user would like to grant an OAuth application access to their data, but only for tenant_2, and not the others. Using this new config option, the developer of the platform can specify that a tenant_id will be submitted when granting access - custom_access_token_fields [:tenant_id]. The developer can then add a tenant_id column to the oauth_access_grants and oauth_access_tokens database tables, and then add a tenant select dropdown to their OAuth consent screen, allowing the tenant_id to be selected and submitted and saved. This tenant_id is then included in access tokens. When receiving a request using one of these access tokens, then developer can add measures to check that the requested resource belongs to the tenant for which the access token was created.

[guardrails]

⚠️ We detected 1 security issue in this pull request:

Insecure Use of Language/Framework API (1)

Docs	Details
💡	Title: User Controlled Method Invocation, Severity: Medium doorkeeper/lib/doorkeeper/oauth/authorization_code_request.rb Line 39 in c144fb2 [field, grant.send(field)]

More info on how to fix Insecure Use of Language/Framework API in Ruby.

---

👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

[JeremyC-za]

@nbulaj let me know what you think about these changes. I'm not sure about the codeclimate issue - seems like the code will be worse if I try to refactor that 🤷‍♂️

[nbulaj]

Thanks @JeremyC-za , will check a little bit later, don't have enough time. I like the idea, but I have to think if it could be done with current implementation or we still need some changes to make it possible

[nbulaj]

OK, I've checked the changes. I'm afraid of some broken ORM and other extensions (like openid connect) while we changed a few internal class initializers. Maybe we should consider another way of doing it, I'll think more about it.

[jonathansimmons]

Beyond just adding a thumbs up, I'll say this would be a huge feature addition allowing us to prioritize the user experience in multi-tenant setups.

Happy to discuss or contribute, but I'm not sure I understand the doorkeeper DSL and flows enough yet.

[ashkulz]

@nbulaj can you elaborate on what exactly would break for other extensions?

[nbulaj]

Actually all the ORM extensions @ashkulz . Maybe more, have to be tested with doorkeeper-openid_connect as well (as it's also patches some Doorkeeper internals)

[nbulaj]

@JeremyC-za could you please rebase you branch on top of current main one ? Thanks 🙇

[JeremyC-za]

@nbulaj I've rebased and actioned your feedback. Let me know if there's anything else I can do.

[nbulaj]

LGTM 👍
Thanks!

[ashkulz]

Thanks for merging this and doing a followup PR (#1634), @nbulaj!

[ashkulz]

@nbulaj can you publish a release to RubyGems?

[nbulaj]

Released with 5.6.5

[raivil]

@JeremyC-za @nbulaj
I've tried adding this new feature and found one issue with rails migrations. Added to #1642