Allow auto authorization per application #322

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@thomase
thomase commented Nov 22, 2013

Allow auto authorization per application

  • Allows to automatically skip authorization of resource owners on a per-app-basis
  • Removes Doorkeeper.configuration.skip_authorization configuration block
thomase added some commits Nov 22, 2013
@thomase thomase DRY allowed params 198351d
@thomase thomase Allow auto authorization per application
  * Allows to automatically skip authorization of resource owners
  * Removes Doorkeeper.configuration.skip_authorization configuration block
6d2c10e
@tute tute commented on the diff Dec 1, 2013
lib/generators/doorkeeper/templates/initializer.rb
@@ -58,10 +58,4 @@
#
# test_redirect_uri 'urn:ietf:wg:oauth:2.0:oob'
- # Under some circumstances you might want to have applications auto-approved,
- # so that the user skips the authorization step.
- # For example if dealing with trusted a application.
- # skip_authorization do |resource_owner, client|
- # client.superapp? or resource_owner.admin?
tute
tute Dec 1, 2013 Owner

Why don't we do the check in here, with current doorkeeper's capabilities? Something like, [1,4,8].include?(client.id)?

gigr
gigr Dec 7, 2013

The only problem with something like that is that ids might vary by environment.

Owner
tute commented Dec 1, 2013

I think we can solve the same problem using current skip_authorization configuration block.

thomase commented Dec 2, 2013

We could keep the skip_authorization configuration block because it offers even more flexibility for power-users and keep the checkbox in the web interface from this PR to enable easy configuration per app?

What do you think?

Owner
tute commented Dec 3, 2013

I'm not sure I like it. Having it easy in the web interface so that non-programmers can change it? If so, does the person know the implications of skipping this step? I'd rather leave only the skip_authorization config in code, which already does the trick, and requires a programmer to implement the decision.

thomase commented Dec 4, 2013

I understand and appreciate your concerns. The option would be nice for our use case - of course, considering the fact that people might accidentally disable authorization might be dangerous...

gigr commented Dec 7, 2013

@tute I see your point about being able to change the behaviour in the web interface. I think it comes down to whether or not you see doorkeeper's web interface being a place for trusted users. As it stands, a non-programmer can cause some damage with the current features that it provides (deleting applications, changing their redirect URIs, etc.) without fully understanding the implications.

thomase commented Dec 19, 2013

@gigr That's true - any user having access to the web interface can cause great damage.

Regarding the implementation: I guess having the skip_authorization for power users as well as the simple variant having a checkbox within the web interface would be nice (at least for my use case). I'm not sure if skipping authorization per application is needed by more people out there...

Owner
tute commented Apr 12, 2014

I won't make the UI "smarter" for now. Thank you for your work and input!

@tute tute closed this Apr 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment