On Tue, Jan 22, 2013 at 09:35:18AM +0100, Jakub Hrozek wrote:
> On Sat, Jan 19, 2013 at 07:19:38PM +0100, Jakub Hrozek wrote:
> > https://fedorahosted.org/sssd/ticket/1773
> > 
> > Commit bf8cce77a35cb0a3cdb0d21fb9c39b7b6372bc11 broke
> > pam_pwd_expiration_warning because it started treating its value as if
> > it was seconds, but in fact it's days.
> > 
> > I tested the path that hits parse_krb5_child_response() and the one that
> > goes through check_pwexpire_ldap() but I'm not entirely sure how to test
> > the LDAP authentication that uses check_pwexpire_kerberos(). Could
> > anyone educate me (or test the patch as part of review maybe?).
> > 
> > I also don't like that the Kerberos password expiration is checked in
> > the provider and the LDAP password expiration in the responder. I think
> > this kind of logic belongs to the responder only. I filed #1774 to get
> > it fixed, but since this patch is intended for the 1.9.4 release that is
> > going to happen after Wednesday, I think we can do the refactor
> > separately.
> 
> This patch was wrong and Sumit nacked it on IRC. It is enough to convert
> the values coming from the [pam] section to seconds.

ACK, I tested this patch with pwd_expiration_warning and
pam_pwd_expiration_warning and both are working as expected now. I used
check_pwexpire_kerberos() for testing. This method was introduced to
allow to use FreeIPA as a pure LDAP server with LDAP authentication.
Here is expiration time is stored krbPasswordExpiration attribute.

bye,
Sumit

> From eb3cbc91ab13c8e5928ea007f3049680087b3606 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhro...@redhat.com>
> Date: Tue, 22 Jan 2013 09:07:37 +0100
> Subject: [PATCH] Convert the value of pwd_exp_warning to seconds
> 
> When read from the domain section, the pwd_expiration_warning was
> properly converted to seconds from days, but not the
> pam_pwd_expiration_warning set in the [pam] section.
> 
> https://fedorahosted.org/sssd/ticket/1773
> ---
>  src/confdb/confdb.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
> index 
> 00643cd398ecf8bb69258c39a5b4deedb9feade9..8ae8d016b4e171683660934ef975d1a4ea7c567c
>  100644
> --- a/src/confdb/confdb.c
> +++ b/src/confdb/confdb.c
> @@ -1024,10 +1024,7 @@ static int confdb_get_domain_internal(struct 
> confdb_ctx *cdb,
>      val = ldb_msg_find_attr_as_int(res->msgs[0],
>                                     CONFDB_DOMAIN_PWD_EXPIRATION_WARNING,
>                                     -1);
> -    if (val > 0) {
> -        /* The value is in days, transform it to seconds */
> -        val *= 24 * 3600;
> -    } else {
> +    if (val == -1) {
>          ret = confdb_get_int(cdb, CONFDB_PAM_CONF_ENTRY,
>                               CONFDB_PAM_PWD_EXPIRATION_WARNING,
>                               -1, &val);
> @@ -1036,7 +1033,11 @@ static int confdb_get_domain_internal(struct 
> confdb_ctx *cdb,
>              val = -1;
>          }
>      }
> -    domain->pwd_expiration_warning = val;
> +
> +    if (val > 0) {
> +        /* The value is in days, transform it to seconds */
> +        domain->pwd_expiration_warning = val * 24 * 3600;
> +    }
>  
>      ret = get_entry_as_uint32(res->msgs[0], &domain->override_gid,
>                                CONFDB_DOMAIN_OVERRIDE_GID, 0);
> -- 
> 1.8.1
> 

> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to