[sssd PR#979][opened] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Author: pbrezina
 Title: #979: sdap: provide error message when password change fail in ldap_modify mode
Action: opened

PR body:
"""
Steps to reproduce:
1. Configure LDAP server to enable password constraints
2. Set ldap_pwmodify_mode = ldap_modify in [domain]
3. Run SSSD and authenticate as a user
4. Run passwd to change password, use password that does not meet requirements

It will print "password change successful" without this patch and server
error message with this patch applied.

Resolves:
https://pagure.io/SSSD/sssd/issue/4148
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/979/head:pr979
git checkout pr979

[sssd PR#979][+branch: sssd-1-16] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +branch: sssd-1-16

[sssd PR#979][comment] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

pbrezina commented:
"""
Downstream tests passed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/979#issuecomment-581385411

[sssd PR#979][+Waiting for review] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +Waiting for review

[sssd PR#979][synchronized] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Author: pbrezina
 Title: #979: sdap: provide error message when password change fail in ldap_modify mode
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/979/head:pr979
git checkout pr979

[sssd PR#979][+Changes requested] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +Changes requested

[sssd PR#979][-Waiting for review] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: -Waiting for review

[sssd PR#979][comment] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

pbrezina commented:
"""
> TLDR please initialize ldap_msg to NULL here:
>  909 static void sdap_modify_passwd_done(struct tevent_req *subreq)
>  910 {
>  911     struct tevent_req *req;
>  912     struct sdap_modify_passwd_state *state;
>  913     int ldap_result;
>  914     char *ldap_msg;
>                ^^^^^^^^ init this to NULL
>  915     errno_t ret;
>
> Explanation:
>
> If in sdap_modify_recv the _ldap_msg equals NULL here:
>  842     if (_ldap_msg != NULL) {
>              ^^^^^^^^^^^^^^^^^ If this is false and we do not get here
>  843         *_ldap_msg = talloc_steal(mem_ctx, state->ldap_msg);
>  844     }
>  845 
>  846     return EOK;
>  847 }

Sorry, I can't see how we can get `sdap_modify_recv(state, subreq, &ldap_result, NULL)` from `sdap_modify_recv(state, subreq, &ldap_result, &ldap_msg)`.
> then in sdap_modify_passwd_done we enter the function sdap_chpass_result
> with uninitialized value of ldap_msg here:
>  929     ret = sdap_chpass_result(state, ldap_result, ldap_msg, &state->user_msg);
>                                                       ^^^^^^^^ This is not initialized
>
> and then all conditions in sdap_chpass_result with ldap_msg may have some garbage value in it.
>
> Other than that the code LGTM but I still have to test it.

"""

See the full comment at https://github.com/SSSD/sssd/pull/979#issuecomment-593327431

[sssd PR#979][synchronized] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Author: pbrezina
 Title: #979: sdap: provide error message when password change fail in ldap_modify mode
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/979/head:pr979
git checkout pr979

[sssd PR#979][comment] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

mzidek-gh commented:
"""
> Sorry, I can't see how we can get sdap_modify_recv(state, subreq, &ldap_result, NULL) from sdap_modify_recv(state, subreq, &ldap_result, &ldap_msg).

Well, of course you can not :) My bad. Removing changes requested.
"""

See the full comment at https://github.com/SSSD/sssd/pull/979#issuecomment-593893493

[sssd PR#979][-Changes requested] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: -Changes requested

[sssd PR#979][comment] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

mzidek-gh commented:
"""
When changing passwd to "foo" (does not meet requirements).

Fedora 30 with git master:
```
Changing password for user user-1.
Current Password: 
New password: 
Retype new password: 
passwd: all authentication tokens updated successfully.

```

master + this patch:
```
Current Password: 
New password: 
Retype new password: 
Password change failed. Server message: invalid password syntax - password must be at least 8 characters long
passwd: Authentication token is no longer valid; new one required

```

ACK.
"""

See the full comment at https://github.com/SSSD/sssd/pull/979#issuecomment-594527645

[sssd PR#979][+Accepted] sdap: provide error message when password change fail in ldap_modify mode

[mzidek-gh]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +Accepted

[sssd PR#979][+Ready to push] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +Ready to push

[sssd PR#979][comment] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

pbrezina commented:
"""
* `master`
    * e4c6ebf6754dca194487f02b616018a860e5dbdf - sdap: provide error message when password change fail in ldap_modify mode
* `sssd-1-16`
    * ddf0a59a610570111fadc391f104c29442e01ac8 - sdap: provide error message when password change fail in ldap_modify mode

"""

See the full comment at https://github.com/SSSD/sssd/pull/979#issuecomment-595123519

[sssd PR#979][+Pushed] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: +Pushed

[sssd PR#979][-Ready to push] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: -Ready to push

[sssd PR#979][-Accepted] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Title: #979: sdap: provide error message when password change fail in ldap_modify mode

Label: -Accepted

[sssd PR#979][closed] sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/979
Author: pbrezina
 Title: #979: sdap: provide error message when password change fail in ldap_modify mode
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/979/head:pr979
git checkout pr979