[Freeipa-devel] [PATCH] 274 Password change capability for form-based auth

[Rob Crittenden]

Martin Kosek wrote:
> On Mon, 2012-06-11 at 13:52 -0400, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Mon, 2012-06-11 at 10:36 +0200, Martin Kosek wrote:
>>>> On Thu, 2012-06-07 at 23:07 -0400, Simo Sorce wrote:
>>>>> On Thu, 2012-06-07 at 22:28 -0400, Rob Crittenden wrote:
>>>>>> Martin Kosek wrote:
>>>>>>> You can use the attached script (changepw.py) to test the PW change
>>>>>>> interface from command line (on IPA server).
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> IPA server web form-based authentication allows logins for users
>>>>>>> which for some reason cannot use Kerberos authentication. However,
>>>>>>> when a password for such users expires, they are unable change the
>>>>>>> password via web interface.
>>>>>>>
>>>>>>> This patch adds a new WSGI script attached to URL
>>>>>>> /ipa/session/change_password which can be accessed without
>>>>>>> authentication and which provides password change capability
>>>>>>> for web services.
>>>>>>>
>>>>>>> The actual password change in the script is processed with kpasswd
>>>>>>> to be consistent with /ipa/session/login_password.
>>>>>>>
>>>>>>> Password result is passed both in the resulting HTML page, but
>>>>>>> also in HTTP headers for easier parsing in web services:
>>>>>>>      X-IPA-Pwchange-Result: {ok, invalid-password, policy-error}
>>>>>>>      (optional) X-IPA-Pwchange-Policy-Error: $policy_error_text
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/2276
>>>>>>
>>>>>> It is probably more efficient to change the password using ldap. Simo,
>>>>>> do you know of an advantage of using one over the other? Better password
>>>>>> policy reporting may be reason enough.
>>>>>
>>>>> Yes you'll get better error reporting, plus forking out kpasswd is quite
>>>>> ugly, the python ldap code should be able to use the ldap passwd extend
>>>>> op quite easily.
>>>>>
>>>>> Simo.
>>>>>
>>>>
>>>> Ok, sending a second version of the patch based on password change via
>>>> LDAP. The error reporting is indeed easier and with no hard-coded
>>>> parsing.
>>>>
>>>> Martin
>>>
>>> This patch will only work with SELinux disabled, it seems there is a
>>> regression in SELinux policy which does not allow httpd to connect to
>>> dirsrv socket. I logged a Bug:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=830764
>>>
>>> This issue also disables other pages using dirsrv socket, like the
>>> migration page or password-expiration detection in form-based auth.
>>>
>>> Martin
>>
>> For '200 Success' you can use rpcserver.HTTP_STATUS_SUCCESS.
>
> Fixed.
>
>>
>> This works ok and does successfully change passwords but I don't like
>> the logging very much.
>
> Actually it does, it just did it in DEBUG level.
>
> I adapted the logging style from /ipa/session/login_password WSGI
> script, but I see that since this is a special page, it should have a
> bit different logging.
>
> Under normal conditions, it now prints a line when
> - the WSGI script is started on INFO level, i.e. in httpd error_log by
> default
> - parameters are validated and we start password change for user (user
> is now printed in log too - this will be useful)
> - when the WSGI script finishes - with either success or error status
>
>>   It should say that this is the password request
>> URI somewhere at a minimum. Having the HTTP response is a bit strange
>> too, and I don't know if a 400 should be logged as info.
>
> I used bad_request method of HTTP_Status class. It uses info log level
> for 400 statuses. I can change that, but it will be changed for all WSGI
> scripts using HTTP_Status. So far, judging from what I saw in
> rpcserver.py we use error log level when there is a problem on our side
> and not in a user request...
>
>>
>> I think this test program could be made into a test suite too,
>> particularly to check the more esoteric parts like checking for missing
>> options, too many options, etc.
>>
>> rob
>
> I added a test suite exercising this WSGI script. It is based on
> built-in httplib instead of original pyCurl - it has much better output
> parsing and is easier to handle.
>
> The new unit test tests bad options, authentication errors and of course
> successful password change, including a verification that that the
> password was actually changed.
>
> Martin

ACK, pushed to master. I like the tests very much.

rob