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(a)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
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel