Polymorphic user model associations

[milgner]

Just like in this this SO question, I'm using doorkeeper scopes to hand out access tokens for multiple models.

The problem with the accepted answer there is that doorkeeper doesn't persist the information of the model on the token, causing problems with scoping of the resource_owner_id column in methods like AccessToken.revoke_all_for.

Are there any plans to support a polymorphic user model association within doorkeeper? Would you accept a pull request towards to accomplish this? My plan would be to just add a resource_owner_type column, adapt the access token mixin and set polymorphic: true on the ActiveRecord association. I'd need to investigate the other ORM integrations separately on how to accomplish this but I assume that they have similar features.

[nbulaj]

Hi @milgner . Yep, I think it would be nice to have such feature. Moreover, it will help OAuth Token Introspection to add additional info for the response. Feel free to send a PR, but be sure that we wouldn't break backward compatibility for legacy applications.

[nbulaj]

Hi @milgner . Any news on this issue?

[nbulaj]

My plan would be to just add a resource_owner_type column, adapt the access token mixin and set polymorphic: true on the ActiveRecord association. I'd need to investigate the other ORM integrations separately on how to accomplish this but I assume that they have similar features.

It's not so simple as many parts of the Doorkeeper are tied to Resource Owner. I've created a branch with some stubs for polymorphic Resource Owner, currently it has broken test suite and requires some changes. This implementation couldn't be merged to master because of incompatibility issues with old versions of Doorkeeper. So if such feature ever would be released, it requires to be compatible with old versions of the gem.

[nbulaj]

Closing this stale issue. If somebody will need it and find this issue, take a look at the branch mentioned above and feel free to send a PR, but keep in mind to do it backward compatible (it must be a configuration option, like polymorphic_owner true).

[top1st]

can you give me example doorkeeper configuration for polymorphic_owner true?
I am getting error

undefined method `polymorphic_owner' for #Doorkeeper::Config::Builder:0x000000000b3cc2c8 (NoMethodError)

when generate doorkeeper:migration

[nbulaj]

@top1st you are using my branch for polymorphic owner or what? Need more details

Current Doorkeeper version doesn't support polymorphic owner (in terms of Rails)

[top1st]

Yes. I am using yours
I forked your branch polymorphic code to clone and added gem path.
I have make doorkeeper:generate migration
but the tables did not generated like the spec/dummy/schma.rb etc
also i have tried add polymorhpic_owner ture in config/initalizers/tindoorkeerper.rb
but get above error

[nbulaj]

Honestly - I don't remember what we have in this branch. And I didn't ever tried it on real project, it's just a stub for those who wants to work on polymorphic associations :)

[top1st]

guess i need to customize my self ,
but not sure can works or not :)

[phlegx]

Another issue with the same feature request is here #651. Here a gist with a workaround. This issue asks for the same feature #456. But the solution with resource_owner_uid instead of resource_owner_id and resource_owner_type is bad. Another issue with same feature request here #233.

[modosc]

i did a poc of this here. the change to look at is here - essentially everything that took a resource_owner_id now takes a resource_owner. the entire test suite passes and i've tested it out in our app and it works.

i think that a global config switch could make this approach backwards compatible - essentially the resource_owner_type column could be added but only used with a specific flag in the config. not sure how this would work with the initial generated db migrations though, but i think in that case backwards compatibility doesn't matter. the flag can also be used for the polymorphic: parameter on the ar relationship. also the whole thing could be migrated to use the resource_owner_or_id pattern everywhere with the assumption that passing in an id means no polymorphic support (i suppose a runtime exception would be useful here).

i'm curious though - is the REST api the only "public" api, or does compatibility need to be maintained with the ruby methods as well?

[nbulaj]

i'm curious though - is the REST api the only "public" api, or does compatibility need to be maintained with the ruby methods as well?

Yep, many projects that uses Doorkeeper could (and they are!) patch some Doorkeeper internals. GitLab is one of them. So it's not so easy to just change everything and not break any existing project.

[modosc]

@nbulaj is there anything i can do to help #1355 move forward?

[nbulaj]

@modosc it requires additional tests, but it's hard to implement them just because AR mixins and models must be reloaded at run-time with a new option. Also many specs reloads Doorkeeper configuration and must be fixed too. I don't sure if somebody could help here

[nbulaj]

Implemented in #1355 and would be released with Doorkeeper 5.4 RC (not release ETA for now)