#49082 Fix password expiration related shadow attributes
Closed: wontfix Opened by gordonmessmer.

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.


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?

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.

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

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.

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!

Looks good to me.

Thanks Gordon for clarifying our misunderstanding of this attribute :)

Reviewed by Gordon and William (Thanks!!)

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

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

Metadata Update from @gordonmessmer:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.3.5.14

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2141

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

Metadata