<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> Hi David, I did a functional review, and everything works well, so functional-ACK. But I did not do the code review. <div class="moz-cite-prefix">On 07/01/2016 10:26 AM, thierry bordaz wrote: </div> <blockquote cite="mid:5776291B.30301@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> Hi David, The patch looks good but being not familiar with that code, my comments may be absolutely wrong In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'. If for some reason 'mod_time==0', it has now a specific meaning 'not expiring' . Does it match the comment '* not 'self', so reset */' In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just before it adds in the mods krbPasswordExpiration=0 or krbPasswordExpiration=entry->pw_expiration Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ? In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above. Something that I am not sure is what is the expected relation between passwordexpirationtime and krbPasswordExpiration thanks thierry <div class="moz-cite-prefix">On 06/30/2016 09:34 PM, David Kupka wrote: </div> <blockquote cite="mid:1777358e-8acf-fd18-6714-f705a4a48de8@redhat.com" type="cite">On 04/05/16 17:22, Pavel Vomacka wrote: <blockquote type="cite"> On 05/04/2016 04:36 PM, Simo Sorce wrote: <blockquote type="cite">On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote: <blockquote type="cite">On 05/02/2016 02:28 PM, David Kupka wrote: <blockquote type="cite"><a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/2795">https://fedorahosted.org/freeipa/ticket/2795</a> </blockquote> That patch looks suspiciously short given the struggles I saw in <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html">http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html</a> :-) Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid filling "krbPasswordExpiration" attribute at all, i.e. have password *without* expiration? Or is krbPasswordExpiration mandatory? </blockquote> So I looked at the MIT code, and it seem like they are coping just fine with a missing (ie value = 0 internally) pw_expiration attribute. So if we make our code cope with omitting any expiration if the attribute is missing then yes, we can mark no expiration with simply removing (or not setting) the krbPasswordExpiration attribute. The attribute itself is optional and can be omitted. I think this is a good idea, and is definitely better than inventing a a magic value. Simo. </blockquote> Just a note: I tested David's patch and it actually doesn't work when the new password policy for ipausers group is created (priority = 0, which should be the highest priority). The maxlife and minlife values are empty. Even if I set the new password policy maxlife and minlife to 0 the result was that password will expire in 90 days. The patch worked correctly when I changed value of maxlife and minlife to 0 in 'global_policy'. Then the password expiration was set to 2038-01-01. </blockquote> Hello! I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop plugins to tickle in order to have password that don't expire. Updated patch attached. <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/2795">https://fedorahosted.org/freeipa/ticket/2795</a> <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> </blockquote> <pre class="moz-signature" cols="72">-- Pavel^3 Vomacka</pre> </body> </html>