nbudin / devise_cas_authenticatable

CAS authentication support for Devise
MIT License
285 stars 117 forks source link

Devise.cas_action_url fails when used within an engine #43

Open bradseefeld opened 12 years ago

bradseefeld commented 12 years ago

All of our authentication is encapsulated in an engine so that various products can just include the engine and automatically be setup to work with devise + CAS. Our paths should look something like:

The problem is that /engine_name/ gets wiped from the url because cas_action_url explicitly sets the path. Also, the current mechanism is fairly brittle since if we mapped /service to something else via our routes, cas_service_url would break.

nbudin commented 12 years ago

I agree this is potentially a problem, but I'm unsure what to do about it. Ideally it'd be great to be able to do something like creating a class that inherits the cas_action_url method and overrides it, but it's a global module-level method, so that doesn't seem possible without refactoring.

Or perhaps it's possible to detect whether we're in a sub-path somehow? Does your engine expose that information?

bradseefeld commented 12 years ago

The route is normally found by referencing the engine name. E.g., engine_name.user_service_url.

I think its best to use the Rails routing mechanism. AFAIK, this means all routes must be set in the controller where the route helpers exist. Right now, I am doing something like:

  module Devise
    def self.cas_service_url=(service_url)
      @cas_service_url = service_url
    end

    ##
    # Override the CAS service url method to use the value that was generated by routes. 
    def self.cas_service_url(base_url, mapping)
      @cas_service_url
    end
  end

In the sessions controller:

  before_filter: set_devise_service_url

   ##
   # Instead of building a URL manually via string parts, just let the
   # Routes handle it.
   def set_devise_service_url
     Devise.cas_service_url = my_engine.user_service_url
   end

I dont think it should ever be up to the module to generate the URL, but I am also not sure if the URL is ever needed outside of a controller context. So far this has worked for me (I have only been able to test in a dev env so far).

Preferably, these URLs would be set in the Devise::CasSessionsController and if we need to further customize the routes (as I do for the engine routes), we would just sub-class and over-ride. I believe this requires no extra configuration for most users, honors the routes in routes.rb (instead of recreating them) and provides the flexibility for them to be changed. If you agree, I dont mind submitting a pull request.

nbudin commented 12 years ago

The reason the module sets the URL right now is because it needs to be accessed by a Warden strategy (in this case, Devise::Strategies::CasAuthenticatable). AFAIK, there's no good way to access Rails routing from inside a strategy, since the strategy has no controller to use.

If you know of a good way to do that, I'd be very, very happy to get rid of hardcoded-ish URLs.

bradseefeld commented 12 years ago

Can you point me in the direction where Warden requests these URL's? Instead of setting them in a controller, we should probably set them in an initialization task (perhaps by making use of a Rails::Engine). The question is really how do we get our hands on the Rails routes during this initialization. If I knew more about how/when Warden needs the URL's I can start to look into this.

nbudin commented 12 years ago

There's basically two reasons the strategy needs to use URLs:

  1. If you need to do a redirect from a Warden strategy, you use the redirect! method, which accepts a URL as its parameter. See line 29 of strategy.rb for where we do this.
  2. In the particular case of CAS authentication, the CAS ticket itself needs to make reference to a service URL so that the CAS server can redirect the user back after performing authentication. We do this in the read_ticket method.