Override authorization approval decision

[miyagawa]

This is a proposal rather than a bug report.

How about providing a hook to override the authorization check flow, especially:

  def new
    if authorization.valid?
      if authorization.access_token_exists?  # <- this
        authorization.authorize
        redirect_to authorization.success_redirect_uri
      end

to override authorization.access_token_exists?.

I have two use cases in mind.

One is to force re-confirmation of the authorization screen even when the same set of scopes were granted previously. This is somewhat related to the security consideration section of the spec (10.2):

10.2.  Client Impersonation
...
   The authorization server SHOULD NOT process repeated authorization
   requests automatically (without active resource owner interaction)
   without authenticating the client or relying on other measures to
   ensure the repeated request comes from the original client and not an
   impersonator.

especially with the Implicit Grant flow (response_type=token) it would be better to re-authorize users with a user interaction, since it could be vulnerable to the URL sniffing in an embedded browser situation with a client impersonation.

The other use case is the opposite i.e. skip the confirmation screen even when this is the first time use of the application and there was no previous authorization. This would be useful when handling in-house applications (which btw requires #37 :D) as a kind of "superuser" applications and assume all of these applications are already approved.

Arguably this could be a rare use case, but the spec merely says (4.1.1):

   ... If the request is valid, the
   authorization server authenticates the resource owner and obtains an
   authorization decision (by asking the resource owner or by
   establishing approval via other means).

and this is to support "via other means" :)

To implement this, I guess we can rename the access_token_exists? to something like pre_approved? and let users override it with some configuration block that returns true or false.

Thoughts?

[felipeelias]

Apologizes for the delay.

Awesome idea :D

We could either use a configuration block or give the ability to extend this with a class/module. I think the method name could be skip_authorization? and the configuration could receive the resource owner and client and make the decision based on those parameters:

# authorization.skip_authorization? in controller
skip_authorization do |resource_owner, client|
  client.superapp? || resource_owner.admin?
end

Not sure how this would fit in the first use case that you mention though (which seems more frequent than the second one).

Just wondering, when you configure this block, should it replace the default behaviour (skips the form when previously granted)?

[miyagawa]

Sorry about slow reply on my side - yes, skip_authorization? sounds sane and configuration block like that allows flexible customization in which case should be able to skip the authorization screen.

As for the question whether it should replace the default - i guess it would be nice if we can fallback to the default behavior, like with super in plain old ruby code, so that authorized application would skip the screen. Wonder how that would work in config blocks like this.

[felipeelias]

I'll give it a try today on this 😄

[matthewford]

Hi Guys,

I was just wondering if there has been any work on it, otherwise i'll implement it in my branch.

My main use case is to use doorkeeper just for sso within a group of internal apps - so users should either skip authorization or pre authorize on create.

[matthewford]

Had a crack at it anyway:

https://github.com/bitzesty/doorkeeper/compare/petergoldstein:feature/mongoid_3_0_x_support...bitzesty:bitzesty

Needs specs and I'm working of the mongoid 3 branch

[felipeelias]

@matthewford thanks. Few questions: Is this just true/false configuration? What's the usage?

[matthewford]

Yes, it's just true or false. I'm using to pre authorize a bunch of apps, as these are all internal - we should not need to preauthorize with the SSO server when logging in.

[belguzmani]

Where should we set this value ??

[felipeelias]

It's only available on @matthewford fork. You should set the configuration to

Doorkeeper.configure
  skip_authorization true
end

[belguzmani]

I needed this way actually

  skip_authorization do |routes|
    current_site = App.find_by(:auth_app_id => params['client_id'])
    current_user.apps.where(:id => current_site.id).count > 0
  end

I changed i little bit @matthewford version (good job btw)

belguzmani@95d307f

[matthewford]

As I said on the commit having a proc is probably the better solution - I just needed on/off for the time being but I see that your way is better probably for the majority of people

[matthewford]

my branch had one too many conflicts with the latest HEAD so I've used adbeelitamar's branch and updated it to HEAD here: https://github.com/bitzesty/doorkeeper/tree/skip-auth

[gustavokloh]

Do you know when this feature will be developed?

Thanks.

[vicentemundim]

+1 for this feature

[felipeelias]

Not focusing on this feature right now.

Pull requests are welcome 😄

[epchris]

@matthewford Is there something outstanding before this is submitted as a pull request? I'm interested in seeing this behavior put in place too.

[epchris]

I created a fork and merged in @matthewford changes with a couple tweaks here: https://github.com/centro/doorkeeper/tree/skip-auth if anyone wants to take a look

[epchris]

Opened pull request #166 based on the work in this discussion.