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

Introduce polymorphic resource owner #1355

Merged
merged 1 commit into from
Feb 7, 2020
Merged

Conversation

nbulaj
Copy link
Member

@nbulaj nbulaj commented Jan 31, 2020

Adds long-awaited ability to set polymorphic Resource Owner for Access Token & Grant (and not just it's ID).

Fixes #1069
Fixes #651
Fixes #456
Fixes #233

TODO

  • Add configuration option to enable polymorphic resource owner
  • Refactor existing code base to use resource owner instance and no just it's ID
    • Make it backward compatible as much as possible
  • Add generator for new migration with required columns
  • Document changes, add to initializer
  • Cover it all with specs

How to use it

Enable configuration option:

Doorkeeper.configure do
  orm :active_record

  use_polymorphic_resource_owner
end

Generate migration to add polymorphic columns to tables:

$ bundle exec rails g doorkeeper:enable_polymorphic_resource_owner

If you previously set foreign key for :resource_owner_id column - you need to drop it as now it would be a polymorphic association. Add it to the migration generated above.

Run the migration:

$ bundle exec rails db:migrate

Use it! 🤠

⚠️ ⚠️ [ATTENTION]: If you enabled this option on existing project - you need to manually set polymorphic type column with the class of the Resource Owner. Otherwise your existing users/clients could face issues and spam you logs with errors.

⚠️ ⚠️ [ATTENTION]: Also this feature could be a breaking change for those users that patched Doorkeeper internals, specially models. You need to fix your patches to user resource_owner instance instead of it's ID (like it was before).

Known projects affected

@nbulaj nbulaj force-pushed the polymorphic-resource-owner branch from 1bfe828 to ae0f27f Compare January 31, 2020 12:14
@jturkel
Copy link

jturkel commented Jan 31, 2020

It's so awesome to see this supported out of the box! Are there any plans to support restricting oauth applications by resource owner type e.g. one set of oauth applications for "users" and a disjoint set for "operators"? I guess #1354 would make this possible with an appropriately configured block.

@nbulaj nbulaj added the WIP Work in progress label Feb 1, 2020
@nbulaj nbulaj force-pushed the polymorphic-resource-owner branch from ae0f27f to 6d35199 Compare February 1, 2020 14:29
@nbulaj nbulaj force-pushed the polymorphic-resource-owner branch from 6d35199 to 6c3fd03 Compare February 5, 2020 10:45
@nbulaj nbulaj force-pushed the polymorphic-resource-owner branch from fd7ef6c to 015a062 Compare February 6, 2020 15:41
@nbulaj
Copy link
Member Author

nbulaj commented Feb 7, 2020

OK, I'll try to add some specs later, need to think more about how to reload mixins and models that depends on Doorkeeper config (and polymcorphic resource owner option).

@nbulaj nbulaj merged commit 2e48a90 into master Feb 7, 2020
@nbulaj nbulaj deleted the polymorphic-resource-owner branch February 7, 2020 06:39
@nbulaj nbulaj removed the WIP Work in progress label Mar 6, 2020
@nbulaj nbulaj added this to the 5.3 milestone Mar 6, 2020
@nbulaj nbulaj mentioned this pull request Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants