ChiliProject is not maintained anymore. Please be advised that there will be no more updates.
We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.
custom autologin cookie name not read (Bug #273)
Description
When you configure the autologin cookie name in configuration.yml, it is stored in the browser under the custom name, but it is not looked for during cookie authentication. The app still looks for default name.
Patch attached.
Possibly, the cookie name could be set globally somewhere.
Associated revisions
Get the autologin cookie name from the config #273
History
Updated by Felix Schäfer at 2011-03-20 08:35 am
Ted P wrote:
Possibly, the cookie name could be set globally somewhere.
There is one in Redmine::Configuration
, though I'm not really sure I like it. Anyway, I'm going to take a look at that.
- Target version set to 1.2.0
- Assignee set to Felix Schäfer
Updated by Felix Schäfer at 2011-03-20 09:46 am
If I didn't know better I'd say Redmine::Configuration
is a bad joke -_-
.
Anyway, I've updated your patch and made a pull request with it at https://github.com/chiliproject/chiliproject/pull/30.
Now, this had me working with Redmine::Configuration
, which strikes me as half-assed at best. While having a central configuration always reminds me of the often-loathed windows registry, I can also see that it has advantages.
What's happening with Redmine::Configuration
though apparently is that every place that reads a value from there also has to define a default in-place (cookie_name = Redmine::Configuration['autologin_cookie_name'] || 'autologin'
), meaning if you every want to change that default, you'd have to go through everywhere this configuration piece is used and change it. Not nice.
You'd think there's a place to put defaults for those, you open the corresponding file and oh joy, you stumble over a hash named @defaults
(source:/lib/redmine/configuration.rb#L21) with the comment "Configuration default values". Looks exactly like what you'd need, right? Well, nearly, the problem is that every key in source:/config/configuration.yml.example is in there uncommented, meaning it gets read with a value if nil
, which in turn overwrites the so-called "defaults" from @defaults
. To make matters worse, the "base" or "common" section in the source:/config/configuration.yml.example that gets loaded before any environment-specific configuration is called default
too, and that gets patched together programmatically in Redmine::Configuration
, instead of leveraging similar functionality provided by YAML directly (though I don't know whether the YAML-one does a flat or a deep merge…).
- Should we consider key with a
nil
value in the configuration file as unset? - Should we keep it as-is and uncomment the keys in source:/config/configuration.yml.example so that the
@defaults
in source:/lib/redmine/configuration.rb#L21 really is a default config? - Should we keep the defaults at the place where the variable gets used?
- Can we rename the "default" section in the config file to "common"?
- Should we switch the config file to using yaml references instead of merging it in in source:/lib/redmine/configuration.rb?
Updated by Felix Schäfer at 2011-03-20 10:05 am
- Assignee deleted (
Felix Schäfer) - Status changed from Open to Ready for review
Updated by Eric Davis at 2011-03-22 11:42 pm
Felix Schäfer wrote:
If I didn't know better I'd say
Redmine::Configuration
is a bad joke-_-
.Anyway, I've updated your patch and made a pull request with it at https://github.com/chiliproject/chiliproject/pull/30.
Pull request looks good to me as long as the tests are passing
Now, this had me working with
Redmine::Configuration
, which strikes me as half-assed at best.
We should discuss this in the forums. From what I saw Configuration included some of Setting which has it's own set of edge cases. I'm in favor of refactoring it if it improves the internal design.
Updated by Eric Davis at 2011-03-24 09:26 pm
Felix:
I had a bunch of problems getting the tests to pass. Looks like the configuration.yml is used in the test environment and it's easy to get it messed up if you don't update your local version. +1 on working on Configuration some more...
Merged to master.
- Assignee set to Eric Davis
- Status changed from Ready for review to Closed