[Freeipa-devel] [PATCH] 1006 detect expired passwords in forms login

[Rob Crittenden]

Martin Kosek wrote:
> On Tue, 2012-04-17 at 10:13 -0400, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Mon, 2012-04-16 at 09:34 -0400, Rob Crittenden wrote:
>>>> Rob Crittenden wrote:
>>>>> Petr Vobornik wrote:
>>>>>> On 04/13/2012 09:28 PM, Rob Crittenden wrote:
>>>>>>> When doing a forms-based login there is no notification that a password
>>>>>>> needs to be reset. We don't currently provide a facility for that but we
>>>>>>> should at least tell users what is going on.
>>>>>>>
>>>>>>> This patch adds an LDAP bind to test the password to see if it is
>>>>>>> expired and returns the string "Password Expired" along with the 401 if
>>>>>>> it is. I'm told this is all the UI will need to be able to identify this
>>>>>>> condition.
>>>>>>>
>>>>>>> rob
>>>>>>>
>>>>>>
>>>>>> UI can work with it. I have a patch ready. I'll send it when this will
>>>>>> be ACKed.
>>>>>>
>>>>>> Some notes:
>>>>>>
>>>>>> 1) The error templates and the 'Password Expired' message are hardcoded
>>>>>> to be English. It's fine at the moment. Will we internationalize them
>>>>>> sometime in future? If so, we will run into the same problem again.
>>>>>
>>>>> No plans to. I can update the patch with a comment specifically to not
>>>>> internationalize it if you'd like.
>>>>>
>>>>>> 2) conn.destroy_connection() won't be called if an exception occurs. Not
>>>>>> sure if it is a problem, GC and __del__ should take care of it.
>>>>>
>>>>> Hmm, this is due to a late stage change I made. I originally had this
>>>>> broken out into two blocks where the only thing done in the first
>>>>> try/except block was the connection, so the only exception that could
>>>>> happen was a failed connection.
>>>>>
>>>>> That isn't true any more. I'll update the patch.
>>>>
>>>> And here you go.
>>>>
>>>> rob
>>>
>>> I still think that deciding based on error message string is not right,
>>> we may want to have it internationalized and thus hit the same error.
>>> What about returning a custom HTTP header specifying the reason?
>>>
>>> Something like that:
>>>
>>> X-rejection-reason: password-expired
>>> OR
>>> X-rejection-reason: password-invalid
>>> OR
>>> X-rejection-reason: account-locked
>>>
>>> Web UI could customize next actions based on this header instead of
>>> parsing the error message.
>>>
>>> If you decide to rather stick with current solution and file my proposal
>>> as an enhancement ticket (or discard it entirely), then ACK.
>>>
>>> Martin
>>>
>>
>> I think this is a better solution. I've updated my patch.
>>
>> rob
>
> ACK. Worked for me nice, header was there. Now, the ball is on WebUI
> side.
>
> I would just rename the header from "X-rejection-reason" to
> "X-Rejection-Reason" (camel case used) in order to be consistent with
> other headers.
>
> Honza also suggested to add "IPA" prefix so that people know where this
> headers comes from. So the final header name would be:
> X-IPA-Rejection-Reason
>
> Martin
>

Good suggestions. Applied both and pushed to ipa-2-2 and master

rob