Stephen Gallagher <sgallagh(a)redhat.com> wrote:
On Fri, 2012-02-03 at 15:45 +0100, Jan Zelený wrote:
> Please note that I haven't fully tested this yet, the LDAP server
> configuration needed for this is a little bit twisted ;-) I will perform
> more testing during the weekend. Consider this patch being preliminary
> and don't push it until it's tested.
Nack.
You need to update the IPA LDAP options as well:
Running suite(s): ipa_ldap_opt
50%: Checks: 2, Failures: 1, Errors: 0
/home/sgallagh/workspace/sssd/src/tests/ipa_ldap_opt-tests.c:79:F:ipa_ldap_
opt:test_check_num_opts:0: Failure 'IPA_OPTS_BASIC_TEST != SDAP_OPTS_BASIC'
occured
Just for aesthetic reasons, please don't use the prefix sdap_ldap_*.
"sdap" is already shorthand for SSSD LDAP. sdap_ldap_result() is a
special case, since it's specifically a handler for "ldap_result"
structures. In general, I'd prefer that you call it something like
sdap_modify_shadow_lastchange_send().
Why are you changing the user's password? That's not only out of scope,
but you're setting their password in plaintext on the server, if I'm
reading this correctly (you're modifying the entry with the raw text of
the new password). We ONLY support password modification via
password-change extended operation.
The scope of
https://fedorahosted.org/sssd/ticket/1019 is ONLY to update
the last change time. We absolutely don't want people using shadow
passwords; they're extremely insecure (highly vulnerable to offline
dictionary attack).
That's what I originally designed the patch to do, but I got totally lost,
because looking at the code, I don't get how modifying shadowLastChange
attribute will solve the issue described in the original bug, considering that
this attribute is tested only when shadow password policy is configured. I
will modify the patch as was my original intention, but I might need a little
help testing it.
Thanks
Jan