On 08/11/2013 10:49 PM, Jakub Hrozek wrote:
On Sun, Aug 11, 2013 at 10:47:13PM +0200, Jakub Hrozek wrote:
> On Wed, Jul 17, 2013 at 01:18:27PM +0200, Jakub Hrozek wrote:
>> On Tue, Jul 16, 2013 at 08:03:04PM +0200, Jakub Hrozek wrote:
>>> On Tue, Jul 16, 2013 at 10:25:34AM +0200, Pavel Březina wrote:
>>>> On 07/10/2013 04:32 PM, Jakub Hrozek wrote:
>>>>> On Fri, Jun 21, 2013 at 02:49:51PM +0200, Pavel Březina wrote:
>>>>>> Now the output looks like:
>>>>>>
>>>>>> $ su test-user
>>>>>> Password:
>>>>>> Password expired. Change your password now.
>>>>>> Current Password:
>>>>>> New password:
>>>>>> Retype new password:
>>>>>> Password change failed. Please make sure the password meets the
complexity
>>>>>> constraints.
>>>>>> su: incorrect password
>>>>>
>>>>> This works but I think the change is too broad, the hint would now
be
>>>>> printed for any password change failure without a specific message
and
>>>>> that might be misleading.
>>>>>
>>>>> The reason why msg is zero-sized is that the user_error_message
variable
>>>>> in changepw_child() is overwritten with result_string which is
"". So
>>>>> one change might be to not overwrite user_error_message with empty
>>>>> string.
>>>>
>>>> For some reason Kerberos returns empty string but result_string.length =
30.
>>>>
>>>>> The other change would be in changepw_child() -- we could
>>>>> special-case when krb5_change_password() returns result_code 4
>>>>> (Password change rejected) and then instead of sending
>>>>> SSS_PAM_USER_INFO_CHPASS_ERROR send a new code
>>>>> (SSS_PAM_USER_INFO_CHPASS_REJECTED?) that would instruct the user to
>>>>> check password complexity settings.
>>>>
>>>> How about this? I avoided new error code and chose to fill server message
in
>>>> provider. The output now looks like:
>>>>
>>>> Password change failed. Server message: Please make sure the password
meets
>>>> the complexity constraints.
>>>
>>> This works for me, I tested a couple of usual scenarios to make sure we
>>> don't print this message too frequently, but seems like everything is
>>> working fine.
>>>
>>> Ack
>>
>> Pushed to master and sssd-1-10
>
> FYI, attached is a backport of the patch on the 1.9 branch. I think the
> backport was trivial enough so that I pushed it, but retroactive review
> would be welcome, too.
>
> Also, the patch was already verified by a customer.
Let's try it again, now with the patch attached.
Just tested on 1.9. The error message is printed properly. The code
looks good to me too.
Additional ACK from me.
Michal