[Tickets #10423] Re: Horde_Auth_Sql 1.0.4 expiration feature severly broken - if I am not completely wrong

[bugs at horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/10423
------------------------------------------------------------------------------
  Ticket             | 10423
  Updated By         | Ralf Lang (B1 Systems GmbH) <lang at b1-systems.de>
  Summary            | Horde_Auth_Sql 1.0.4 expiration feature severly broken
                     | - if I am not completely wrong
  Queue              | Horde Framework Packages
  Version            | Git master
  Type               | Bug
  State              | Assigned
  Priority           | 1. Low
  Milestone          |
  Patch              |
  Owners             | Horde Developers
------------------------------------------------------------------------------


Ralf Lang (B1 Systems GmbH) <lang at b1-systems.de> (2011-08-12 07:33) wrote:

>> Excuse me if I got something wrong and this is all bogus.
>>
>> The Horde_Auth_Sql driver provides account password expiration in a
>> soft (warn) and hard (lock) flavour.
>>
>> To me this feature looks totally broken in Horde 4.
>>
>> 1) The migration file creates the soft and hard timestamp fields as
>> signed int 11 rather than unsigned. (tested on mysql)
>
> Not sure why that's a problem? Unix timestamps are signed 32 bit integers.

Wasn't aware of that. I've always only known positive timestamps.

>> 2) The calculation routine produces a negative value for the soft and
>> hard expiration timestamp. This means users changing password
>> immediately expired accounts if it wasn't for 5)
>
> Not sure about this, but it might be due to the errors described in (4).

Moreover, time gives me a positive value, but that formular involving  
time and mktime returns a negative integer. If Timestamps are a number  
of seconds since 1970, this should be before 1970 but then the formula  
is broken. I'd rather ditch the formula in favor of  
http://www.php.net/manual/en/class.datetime.php as we're PHP 5.2+ now.

>> 3) The hard expiration timestamp is calculated via soft_expiration_window.
>
> Appears to be a bug.
>
>>
>> 4) The hard expiration timestamp is completely ignored if soft
>> expiration is not configured - is this intended? The calculation of
>> hard_expiration_date at least looks a little like this could be true
>> - but see 3)
>
> If you are talking about Horde_Auth_Sql#updateUser(), then I agree -  
> this doesn't look correct. Looks like it was included within the  
> else clause erroneously. Also, it appears it could be a copy/paste  
> error, as the hard expiration calc is using the $date variable that  
> was already incremented by soft exp.
>
>> 5) If hard expiration is configured, changing users is completely
>> broken because the generated SQL has more values than fields (or
>> other way around, the last value is treated as a key).
>
> Looks like it's missing the $query portion for hard expiration.

Yes. Adding it makes saving work but then horde expires my user as any  
negative timestamp is before $now.

>> 6) The addUser routine doesn't initialize these additional fields. As
>> a result, all accounts last forever until the user changes
>> credentials for the first time.
>
> Sounds like a bug.
>
>> I think I can fix this as I'm working on #10387 already.
>
> Go for it :)
>

I'll do.