search

doorkeeper-gem / doorkeeper-grants_assertion

Assertion grant extension for Doorkeeper. Born from: https://github.com/doorkeeper-gem/doorkeeper/pull/249
MIT License
82 stars 53 forks source link

Colliding with other flows? #2

Closed christopherhein closed 10 years ago

christopherhein commented 10 years ago

So I'm not sure if we should actually be overriding Doorkeeper::ApplicationController here it seems like whenever this gem is included it overrides all other strategies by way of fully redefining the ApplicationController…

Can you confirm my assumption? Easy to reproduce start a new app setup doorkeeper and this gem and try to use the authorization code flow…

I'm assuming we might want to do something like class_eval or include a helper method?

module Doorkeeper
  class ApplicationController < ActionController::Base
    def resource_owner_from_assertion
      instance_eval(&Doorkeeper.configuration.resource_owner_from_assertion)
    end
  end
end
christopherhein commented 10 years ago

something along the lines of https://github.com/generalthings/doorkeeper-grants_assertion/compare/using-include-application-controller

gottfrois commented 10 years ago

+1 for @christopherhein fix. Otherwise you override the doorkeeper application controller and we are not able to access /oauth/applications for example.

maurobender commented 10 years ago

I made a fix of my own to avoid the "/oauth/applications" problem but I think @christopherhein's it's really better. +1

tute commented 10 years ago

Fixed in PR, thanks @christopherhein!