[SSSD] [PATCH] warn user if password is about to expire

Pavel Březina pbrezina at redhat.com
Wed Nov 28 15:54:45 UTC 2012


On 11/28/2012 11:15 AM, Sumit Bose wrote:
> On Tue, Nov 20, 2012 at 11:18:22AM +0100, Pavel Březina wrote:
>> On 11/19/2012 04:59 PM, Pavel Březina wrote:
>>> https://fedorahosted.org/sssd/ticket/1638
>>
>> Self nack.
>>
>>> -            if (pwd_exp_warning == 0 ||
>>> -                difftime(now + pwd_exp_warning, ppolicy->expire) > 0.0) {
>>> +            if (pwd_exp_warning != 0 &&
>>> +                difftime(pwd_exp_warning, ppolicy->expire) > 0.0) {
>>>                  goto done;
>>>              }
>>
>> We don't need to test if pwd_exp_warning != 0, because if it is 0 then
>> difftime returns value <= 0 and the condition fails.
>>
>> New patch is attached.
>>
>
>>  From 3bb970535dd1d12016ed8ba431039b843be9da7c Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>> Date: Mon, 19 Nov 2012 16:52:36 +0100
>> Subject: [PATCH] warn user if password is about to expire
>>
>> https://fedorahosted.org/sssd/ticket/1638
>>
>> If pwd_exp_warning == 0, expiry warning should be printed if it is
>> returned by server.
>>
>> If pwd_exp_warning > 0, expiry warning should be printed only if
>> the password will expire in time <= pwd_exp_warning.
>>
>> ppolicy->expiry contains period in seconds after which the password
>> expires. Not the exact timestamp. Thus we should not add 'now' to
>> pwd_exp_warning.
>> ---
>>   src/providers/ldap/ldap_auth.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
>> index 32a2e04ea959a3cc81b88f5b2b19575c813e8adf..04fd24ffa4708dce53fb937e92efbbc102bef97c 100644
>> --- a/src/providers/ldap/ldap_auth.c
>> +++ b/src/providers/ldap/ldap_auth.c
>> @@ -212,7 +212,6 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd,
>>       if (ppolicy->grace > 0 || ppolicy->expire > 0) {
>>           uint32_t *data;
>>           uint32_t *ptr;
>> -        time_t now = time(NULL);
>>           int ret;
>>
>>           if (pwd_exp_warning < 0) {
>> @@ -231,8 +230,7 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd,
>>               ptr++;
>>               *ptr = ppolicy->grace;
>>           } else if (ppolicy->expire > 0) {
>> -            if (pwd_exp_warning == 0 ||
>> -                difftime(now + pwd_exp_warning, ppolicy->expire) > 0.0) {
>> +            if (difftime(pwd_exp_warning, ppolicy->expire) > 0.0) {
>>                   goto done;
>>               }
>>               *ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
>
> I haven't tested the patch yet, but I would recommend to drop difftime()
> because pwd_exp_warning and ppolicy->expire are integers and just using
> '<' or '>' would be easier and better to read.
>
> That said I think the current logic is flawed. As you said of
> pwd_exp_warning is 0 difftime returns value <= 0 and the expire time is
> added to the response. But if pwd_exp_warning is positive
> ppolicy->expire must be larger than pwd_exp_warning to be added to the
> response. This means the warning is only printed if it is still longer
> than the time given in pwd_expiration_warning.
>
> bye,
> Sumit


You are correct sir. Thank you. Looks like I forgot to test the option.

I'm sending a new patch. I don't have time to test it myself right now,
but I don't want to delay 1.9.3 if with any luck it is going to be
released today.

I'll try it tomorrow if it's still pending, though I'm quite confident 
it will work.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-warn-user-if-password-is-about-to-expire.patch
Type: text/x-patch
Size: 1848 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121128/ae061005/attachment.bin>


More information about the sssd-devel mailing list