Add Client Ownership about doorkeeper HOT 14 CLOSED

What about this:

• Add an owner_id integer column to :oauth_applications table
• Add a belongs_to :owner association to the Application model. The class for the owner association needs to be configurable from the initializer config block. Something like

owner_class_name 'User'

The provider app can build its own association to the Application model. Likely:

class User
  has_many :applications, :foreign_key => "owner_id"

That should do it. If you want to have more complex ownership, link to an owner class that is a bridge table.

from doorkeeper.

Cool! Thanks. Keep us posted

from doorkeeper.

This is definitely a key feature - the current implementation of Doorkeeper is extremely restrictive in this regard and wouldn't support an OAuth API in the most common use cases (ala Facebook, Twitter, Foursquare, etc.). As I'm trying to use it as such this is on my list to get finished in the next couple of weeks.

A couple of comments:

i) I think this needs to be really flexible - Doorkeeper is an engine and should not tie itself to any fixed user model.

ii) A key piece of the flexibility has to be support for both many owners of a client, and multiple levels of ownership. Both of these ideas are supported by Facebook. In Facebook's API a client application doesn't necessarily belong to a single user account, but may be owned by several. And there may be additional accounts that can view the client application data but not edit it.

None of this needs to be inherent to Doorkeeper, but Doorkeeper as an engine shouldn't make it impossible or overly difficult to implement.

iii) The feature implementation needs to allow an application using doorkeeper to enable client credentials authentication (as per section 4.4 of the spec). That may be incompatible with some choices for user ownership (a many to one user to client application owner model), but the engine should definitely not rule out implementation of section 4.4 of the spec.

iv) As I mention above, I need this feature so I've been thinking about it. I've been leaning towards implementing this in the initializer/doorkeeper.rb, using a proc or just a method name as symbol as the configuration value. The code itself could then invoke the proc or method name to do client owner resolution. This is one option that would keep the underlying engine and the application in which it is mounted minimally coupled. Just a thought on implementation, and hardly the only option.

As I said, I'm very interested in this feature so I'd love to take a look at the pull request when you submit it.

from doorkeeper.

One way to provide this flexibility is to generate the application model into the rails app on install. This would simplify customization.

Another way would abstract the current application model behaviour into a module, so it could be easily included in any model.

Something like this:

rails generate doorkeeper:application [NAME]

This will generate proper migration and the model.

class AnyUserModel < ActiveRecord::Base
  doorkeeper_application # or whatever the name of the method. 
end

The drawback is that this will add one more step in the installation process

from doorkeeper.

Generating the application model on install in an unencapsulated way is potentially dangerous, as the Application model is key piece of the OAuth flow. It has responsibilities (validating redirect URIs, confidential vs. public behavior, etc.) as part of that flow, and exposing the entirety of the model would make it more likely that applications using Doorkeeper would inadvertently break OAuth. So I definitely wouldn't recommend copying the raw Application code in a user app.

The latter idea - incorporating the OAuth-relevant code into a module and providing some optional hooks - sounds a lot more promising to me. It'll be harder for the user to accidentally break OAuth.

from doorkeeper.

Yes, that was my concern. The latter seems harder to implement, but it's definitely the most wise way.

from doorkeeper.

@teeparham yes, this seems to be enough for most of the use cases.

Also, internally we would need to change ApplicationController to require at least an user when registering an application. Right now if you do not configure the admin_authenticator block, anyone can create applications(!).

Thanks!

from doorkeeper.

I think that client ownership is a needed feature, but it shouldn't be required.

For example, in one of the projects I'm working now, the application developers don't create new applications on our platform, but they do it directly at a third-party developer portal, namely http://3scale.net -- then we copy the application data into our application so we can do the authorizations.

In this case, we don't have any model that owns the application; it's just the admins who are in charge of creating the applications.

from doorkeeper.

My suggestion here would be a nullable, polymorphic belongs_to relationship on Application. This would make the ownership optional from the model perspective.

Requiring ownership could be handled in the OAuth provider configuration. Doorkeeper's configuration would allow the resource owner to specify whether applications require owners, and this behavior could be enforced either in the model in a validator or in the controller.

That should handle all the cases specified to date. I'm going to try and put this together tonight to see how well it works.

Thoughts?

from doorkeeper.

A nullable, polymorphic belongs_to makes sense to me. Nullable makes ownership optional. Polymorphic means clients don't need to specify an owner class in an initializer, they can simply do this is their owner model:

has_many :applications, :as => :owner

Agreed that the ownership requirement should be a Doorkeeper configuration. I would enforce it as a validation in the Application model.

from doorkeeper.

And how would that configuration look like? Any ideas?

from doorkeeper.

So I think configuration could be as simple as

Doorkeeper.configure do
  should_confirm_application_owner true
end

Then inside the Doorkeeper::Application we add

validates_presence_of :owner, :if => :should_confirm?

def should_confirm?
  Doorkeeper.configuration.should_confirm_application_owner?
end

from doorkeeper.

+1
I'm happy to contribute time & code on this if need be.

from doorkeeper.

Closing this.

For now, refer to #78.

from doorkeeper.