Fix password expiration related shadow attributes

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49082

• Created at 2017-01-06 22:30:00 by gordonmessmer
• Closed as Fixed
• Assigned to nhosoi (@nhosoi)
• Associated bugzillas
  • https://bugzilla.redhat.com/show_bug.cgi?id=1411944

---

Shadow attributes (in /etc/shadow and in LDAP) are typically unset when no policy is in place. 389-ds will incorrectly return values (possibly set to 0) when there is no policy.

Only auto-fill shadow attributes when a password policy is available. These are empty when no policy is in place.

Don't auto-fill expiration related shadow attributes if passwords never expire.

[389-ds-bot]

Comment from gordonmessmer at 2017-01-06 22:30:18

attachment
389-shadow-expiration.patch

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-01-08 08:22:18

Hi, Sorry for misunderstanding your issue on the mailing list.

I know we use "long long" in the code here, but try to use PRUint64 or PRInt64. We should avoid this old and confusing syntax, and new code should use the modern types.

Can you give us the example steps you took to test and assert the new patches functionality so that we can create a python test case to prevent regressions?

[389-ds-bot]

Comment from gordonmessmer at 2017-01-08 09:15:02

No testing has been done as of yet. These patches are merely suggestions. In this case, that shadowMin, shadowMax, and shadowWarning should only be auto-filled if the equivalent settings have a value on the directory server, and that shadowMax and shadowWarning should only be set if password expiration is enabled.

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-01-12 05:17:08

git patch file (master)
0002-Ticket-49082-Fix-password-expiration-related-shadow-.patch

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-01-12 05:17:53

git patch file (master) -- CI test; adjusting the test case
0003-Ticket-49082-Adjusted-the-CI-test-case-to-the-fix.patch

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-01-12 05:23:05

Hello, Gordon.

Could you please review the attached patch 0002-Ticket-49082-Fix-password-expiration-related-shadow-.patch​?

I slightly modified your original patch not to update unless "shadowval" was retrieved. Thanks.

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-01-12 05:28:20

Hi William,
Replying to [comment:1 Firstyear]:

I know we use "long long" in the code here, but try to use PRUint64 or PRInt64. We should avoid this old and confusing syntax, and new code should use the modern types.

I did not change "long long shadowval", which is supposed to have the same type as pwpolicy->pw_minage, pwpolicy->pw_maxage, etc. (unless we cast or change the type of pwpolicy->pw_...). If we want to do that, I think we'd better do that in a new ticket.

Can you give us the example steps you took to test and assert the new patches functionality so that we can create a python test case to prevent regressions?

I updated ticket548_test.py to adjust the new behaviour.

Thanks!

[389-ds-bot]

Comment from gordonmessmer at 2017-01-12 05:48:53

Looks good to me.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2017-01-12 05:50:02

Thanks Gordon for clarifying our misunderstanding of this attribute :)

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-01-12 06:03:42

Reviewed by Gordon and William (Thanks!!)

Pushed to master:
9835e2b..5a6a5a1 master -> master
commit 5bcd966
commit 5a6a5a1

Pushed to 389-ds-base-1.3.5:
238d3c7..b9e565d 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit faae0fa
commit b9e565d

[389-ds-bot]

Comment from gordonmessmer at 2017-02-11 23:03:01

Metadata Update from @gordonmessmer:

• Issue assigned to nhosoi
• Issue set to the milestone: 1.3.5.14