Add resource indicators

[danielcooper]

Summary

Adds resource indicator support (#1413) as an optional extension.

Other Information

I've got a few implementation questions:

1. Should this be an optional extension (with a migration to generate) or just added into the default setup in a disabled state? I'm not clear how best to test the apps when the new migration hasn't been run/generated?

2. The way resource is specified in the RFC is quite inconvenient:

  GET /as/authorization.oauth2?response_type=code
     &client_id=s6BhdRkqt3
     &state=tNwzQ87pC6llebpmac_IDeeq-mCR2wLDYljHUZUAWuI
     &redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb
     &scope=calendar%20contacts
     &resource=https%3A%2F%2Fcal.example.com%2F
     &resource=https%3A%2F%2Fcontacts.example.com%2F HTTP/1.1
  Host: authorization-server.example.com

Out of the box rails params won't work in this case. You can see I've hacked together a quick fix, do you have any thoughts on a better way of doing this? Maybe it'd be better to only support one resource.

[houndci-bot]

Style/Documentation: Missing top-level class documentation comment.

[houndci-bot]

Lint/MissingCopEnableDirective: Re-enable Style/BlockDelimiters cop with # rubocop:enable after disabling it.

[guardrails]

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

Insecure Processing of Data (1)

Docs	Details
💡	Title: Potential XSS (unquoted template variable), Severity: Medium doorkeeper/lib/generators/doorkeeper/templates/add_resource_indicators_to_access_grants_and_access_tokens.rb.erb Line 3 in 4f299dd class AddResourceIndicatorsToAccessGrantsAndAccessTokens < ActiveRecord::Migration<%= migration_version %>

More info on how to fix Insecure Processing of Data in Ruby.

---

👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

[nbulaj]

hey @danielcooper , great work 👍

Should this be an optional extension (with a migration to generate) or just added into the default setup in a disabled state? I'm not clear how best to test the apps when the new migration hasn't been run/generated?

Yep, I'm for keeping such functionality as an extension. The main idea is to not pollute core gem with everything, but try to make it more flexible and modular.

The way resource is specified in the RFC is quite inconvenient:

Hm, good question 🤔 I don't remember when I done something like this with a collection-like params in Rails. But something tells me there should be a god way of doing it

[danielcooper]

@nbulaj Thanks for taking a look - what can I do to help get this merged?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[danielcooper]

@nbulaj Sorry to pester.

Are you generally not keen on this feature, the implementation, or is it just a matter of time to review?

[nbulaj]

Hey @danielcooper 👋

I love the feature for sure, I just wanted to have such features as a extensions (just like doorkeeper_openid_connect)