Closed eriko closed 10 years ago
Just so that I understand the problem, you are saying that the url
option should not be static, it should be computed each time it is needed?
So as far as I can tell the issue is more the deprecated use of def initialize in the strategy breaks the ability to dynamically setup the strategy. In the current state it only works when configured at startup.
On to your question. Effectively yes that is correct but that is coming from a different direction. Using the lambda setup method https://github.com/intridea/omniauth/wiki/Dynamic-Providers allows a running application to change settings and have them come into effect immediately . The two applications of this that I am after are:
In the case of Discourse there are multiple authentications in use in the same application. CAS is generally not the first one setup and the configuration is done in the application and not via a config file. Because this strategy does not allow the use of the lambda methods, unlike the other strategies in use. Due to this any changes require restarting all instances of the application.
The second case is when hosting multiple sites out of one application, multisite, you need to be able to support settings per site which needs the lambda method.
It is possible that there is a way to keep using the :url, which is nice if your CAS setup is normal, and make lambda work. I just could not find it quickly.
Ok here is the basic deal. While the :url option was nice it was only worked in when when you had a default cas setup. If there was anything odd it would break. Also the use of the overriding initialize method broke using the lambda method of the setting up the authentication dynamically. This also broke the steps Omniauth says to use in updating a strategy to 1.0 in https://github.com/intridea/omniauth/wiki/Adapting-strategies-for-1.0 . I needed to use the lambda in setting up a CAS authentication plugin for Discourse. The current version of omni_auth would only work at startup and would not update if you you changed settings in the app. This would not be to bad except that Discourse supports multisite (hosting multiple domains with one code tree) and needs to use different settings for different site.