Polymorphic ResourceOwner support

[thibaudgg]

Hey,

My App needs to support authentication of both User and Operator model (we use two different Devise scopes).

It would be great if we could add a resource_owner_type field to Doorkeeper::AccessToken for supporting a polymorphic association.
Owner type could be passed as an optional argument on the POST /oauth/token?owner_type=operator request if multiple owner type are supported.

It could be related to #461, but I would gladly fire up a new PR for that.

Thoughts?

PS: Thanks for this awesome gems ❤️!

[jasl]

You mean User and Operator are independent models, Not STI?

[thibaudgg]

Yes, exactly. They have both there own ids.

[jasl]

Let Doorkeeper supports multiple-models is necessary, but adding an optional arguments may have potential issue, because Doorkeeper support many flows, if we want to add one optional argument, we need letting all flows to support.

I think there have another way to reach the goal. I have an idea, we could support multi-models by customizing resource_owner_id , for example User_123 or Operator_123(depends you, just keeping unique), by this way, we just adding two option that told Doorkeeper how to generateresource_owner_id` and how to find the document, no need to expand APIs.

What do you think?

[thibaudgg]

I think that using User_123 or Operator_123 is kind of hackish when Rails polymorphic association exists. I would really prefer following the convention and rely on resource_owner_type.
We could add a new configuration setting for this feature (like enable_polymorphic_resource_owner) so the resource_owner_type field is not mandatory for everybody (easy upgrade for existing users).

Concerning the optional request argument, I think we need a way to specify the owner type when requesting an access_token no? Without it, there's no way to know against which model authenticate the username/password credentials.

[tute]

Concerning the optional request argument, I think we need a way to specify the owner type when requesting an access_token no? Without it, there's no way to know against which model authenticate the username/password credentials.

Do the user types belong in the client apps or in the provider? If in the provider, we can discuss further, otherwise, I don't feel very positive about including this in doorkeeper.

[thibaudgg]

@tute The owner types belong in the provider (our Rails app), the client should be able to authenticate the owner as User or Operator.

[tute]

In that case nothing has to change in the request, as the application already knows what the resource_owner is, and it can query it's .class (see the resource_owner_authenticator configuration block in your doorkeeper initializer). Actually, we might not need to query for class, we could consider the relationship between tokens and owners polymorphic always, let Rails fill in the type for us, and even when we only use one (the typical case, if not the only one, until now).

This would of course be a backwards incompatible way, and it would happen when we release doorkeeper 3, which I don't expect to happen very soon.

Also, given it's the first time this is thought about in doorkeeper's life that I know, I'm not sure it should be added into the project. For example, a potential issue is it would potentially harm projects that use other frameworks (like Grape) or ORMs, that don't solve this "out of the box". That would imply that we need to add code to support an uncommon case, which I'm against of.

Let me think further about this. Thank you!

[thibaudgg]

I should have precised that we use the Resource Owner Password Credentials, so we need a way to know the owner type on the resource_owner_from_credentials block.

Here the request that we would like do to for getting the access token:

POST /oauth/token?grant_type=password&client_id=CLIENT_ID&client_secret=CLIENT_SECRET&owner_type=OWNER_TYPE&username=OWNER_EMAIL&password=OWNER_PASSWORD 

Does it make any sense?

[tute]

It does make sense, and we won't support it in doorkeeper in the foreseeable future. Thanks for your input!

[thibaudgg]

Ok thanks for your feedback.
In that case, I guess one solution for us would be to add an extra layer between Doorkeeper::AccessToken and our User/Operator models like a OauthResourceOwner model that will handle the polymorphic association.

[fernandes]

Making some tests on how to support different owners (I'm using token authentication), I got into something like this (my idea was touch less possible of Doorkeeper):

I created different routes for each scope

use_doorkeeper do
  controllers :tokens => 'doorkeeper/manager_tokens'
  as tokens: 'manager_token'
  skip_controllers :applications, :authorized_applications, :authorizations
end

use_doorkeeper :scope => 'oauth' do
  controllers :tokens => 'doorkeeper/admin_tokens'
  as tokens: 'admin_token'
  skip_controllers :applications, :authorized_applications, :authorizations
end

Then created controllers for each route:

class Doorkeeper::AdminTokensController < Doorkeeper::TokensController
end

class Doorkeeper::ManagerTokensController < Doorkeeper::TokensController
end

Then on config I set user based on each controller used on request.

resource_owner_from_credentials do |routes|
  user = Manager.find_for_database_authentication(:email => params[:username]) if params[:controller] == "doorkeeper/manager_tokens"
  user = Admin.find_for_database_authentication(:email => params[:username]) if params[:controller] == "doorkeeper/admin_tokens"
  if user && user.valid_for_authentication? { user.valid_password?(params[:password]) }
    user
  end
end

This is far from a good solution (I wonder), I've tested my application on Manager/Admin getting tokens on the right route, and not getting tokens on not owned routes.

Seems to be working perfectly for token authentication.

Any thoughts?

[lucaspiller]

This would be something I'd like to see too. In our case some users are authenticated via LDAP and others are authenticated via Devise, but the way they interact with the API is the same. Currently we use HTTP Basic auth, but would like to switch to oAuth2.

Another solution would be to change resource_owner_id to accept a string, in which case it could use the Global ID functionality in Rails 4.2.

[thibaudgg]

If everyone is interested here the solution we ended up with: https://gist.github.com/thibaudgg/3aa7e3276fbcdd712cbf

[mspanc]

I would also like to see that in Doorkeeper.

In our system we have two totally distinct user types, but one pool of access tokens. It would be awesome if we could assign the token to the different user classes.

[tute]

If we can make this change backwards compatible I will consider it.

[penso]

👍 I'd love to see this change as well.

[jufemaiz]

Hi @tute just wondering if there's been any movement on getting this on the roadmap or if @thibaudgg's solution ( https://gist.github.com/thibaudgg/3aa7e3276fbcdd712cbf ) is still the unofficial recommended approach to resolving this?

We've got the use case where our application needs to support OAuth, has two different user models (much like above), and it seems that many of us have adopted this path for a platform where segregation of management and users (for want of different role names) is enforced at a stricter level through using two different models.

[johnnyshields]

This should be possible without introducing an intermediate model as @thibaudgg has done; AccessToken.resource_owner itself can be made polymorphic and this can be done in a backwards compatible way (will require a migration to ensure AccessToken.resource_owner_type is properly set in the DB). Our team will make a PR.

[johnnyshields]

Investigated this further. At least in MongoDB where primary keys are GUIDs (guaranteed to be unique across tables), it is not necessary to introduce a polymorphic relation; Doorkeeper gem can be used as-is. I solved this as follows in the doorkeeper.rb initializer:

  resource_owner_from_credentials do |routes|
    case routes

      when TokensControllerOne
        u = UserOne.find_for_database_authentication # etc etc...

      when TokensControllerTwo
        u = UserTwo.find_for_database_authentication # etc etc...

      else
        fail('Doorkeeper unknown route')
    end
  end

If you want to use the same controller you could introduce a param as suggested above.