On 01/06/2012 08:04 PM, Matt Wagner wrote:
On Fri, Jan 06, 2012 at 12:10:34PM -0500, Matt Wagner wrote:
> On Thu, Jan 05, 2012 at 10:29:56AM +0100, jprovazn(a)redhat.com wrote:
>> From: Jan Provaznik<jprovazn(a)redhat.com>
>>
>>
https://bugzilla.redhat.com/show_bug.cgi?id=758644
>>
>> This rake task is called from aeolus-configure.
>
> As we discussed on IRC, this should be changed a little, as this will
> reset the counter any time aeolus-configure is run.
>
> The fix we discussed is to have a decrement_login_counter task, which
> feels a little weird but avoids the issue of wiping out data.
This sounded like a great idea at the time, although we acknowledged
that a rake task to decrement the login counter felt a little weird.
In practice, it seems like login_count isn't actually incremented on
each run of aeolus-configure. I have to confess that I don't yet
understand why this is the case. I implemented a patch to decrement the
login count, but it means that the total login count will go down by one
on each subsequent run since I decrement it at logout but it didn't get
incremented at login. This is even more broken than having it start>0
for fresh installs.
You are right - login_count is sometimes not incremented, but this
shouldn't happen. Actually it's a bug in aeolus-configure web_request
method. Web requests use cookies stored in /tmp/aeolus-$name. This file
is created (if it doesn't exist yet) when login is called and deleted on
logout. Problem is that sometimes logout is not executed and this cookie
file is kept. Then if aeolus-configure is executed again and login is
called, it finds this file with valid cookies and uses it when sending
login request.
We have before filter for login action which checks if user is not
already logged in, if he is he is redirected to his account page (and is
kept logged in) -> warden authentication is not used in this case,
login_counter is not increased.
I can think of a few options:
1.) Just accept your patch to reset the login count to zero on each run.
I don't love this, but it certainly avoids a headache.
2.) Do something hack-ish and conditionalize your patch to only run if
login_count<= 3 or so. This kind of avoids the issue but is a horrible
hack.
3.) Have aeolus-configure pass in something like a :do_not_increment =>
true param when it logs in. We had discussed this earlier and rejected
it as kind of undermining the point of a login tracker if you could just
request that we skip it. But it occurs to me that we could discard this
parameter if request.ip wasn't 127.0.0.1, which should hopefully
mitigate the potential for abuse. (I'm also not convinced that
login_count and last_login_ip are bulletproof security features anyway.)
4) Alternatively we could fix web_request method in aeolus-configure to
reset cookie file on login request and call the decrement_counter rake
task after login is done (instead of after logout).
This bug has probably worst "importance/fix-time" ratio.
Does anyone want to weigh in here? Is there a better option I'm
not
aware of? #3 seems like the least-bad solution to me.
-- Matt
Jan