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)


Added by Ted P at 2011-03-13 02:48 am. Updated at 2011-03-24 09:26 pm.


Status:Closed Start date:2011-03-13
Priority:Normal Due date:
Assignee:Eric Davis % Done:

0%

Category:User accounts
Target version:1.2.0
Remote issue URL: Affected version:

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.


cookie_name.patch (2.2 kB) Ted P, 2011-03-13 02:48 am


Associated revisions

Revision 10dffcf3
Added by Felix Schäfer at 2011-03-20 10:18 am

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…).

Anyway, all this ranting for this:
  • 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?
And on a more philosophical/semantic matter:
  • 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

Also available in: Atom PDF