-
-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Devise.cas_action_url fails when used within an engine #43
Comments
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? |
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. |
The reason the module sets the URL right now is because it needs to be accessed by a Warden strategy (in this case, If you know of a good way to do that, I'd be very, very happy to get rid of hardcoded-ish URLs. |
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. |
There's basically two reasons the strategy needs to use URLs:
|
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.
The text was updated successfully, but these errors were encountered: