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
>From c60cd566c7b84ebcd82730d4dfce93d610e4aca9 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Wed, 6 Jun 2012 14:38:08 +0200
Subject: [PATCH] Password change capability for form-based auth

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 by LDAP
password change command.

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, error}
  (optional) X-IPA-Pwchange-Policy-Error: $policy_error_text

https://fedorahosted.org/freeipa/ticket/2276
---
 install/conf/ipa.conf                 |    8 ++-
 ipaserver/plugins/xmlserver.py        |    3 +-
 ipaserver/rpcserver.py                |  108 ++++++++++++++++++++++++++++++++-
 tests/test_ipaserver/httptest.py      |   52 ++++++++++++++++
 tests/test_ipaserver/test_changepw.py |  109 +++++++++++++++++++++++++++++++++
 5 files changed, 277 insertions(+), 3 deletions(-)
 create mode 100644 tests/test_ipaserver/httptest.py
 create mode 100644 tests/test_ipaserver/test_changepw.py

diff --git a/install/conf/ipa.conf b/install/conf/ipa.conf
index 89c9849ca6656ae3da585a72392d5d2463f4d892..b52d9d2ff722c77c37619cc4a4c0fb7cebd5354f 100644
--- a/install/conf/ipa.conf
+++ b/install/conf/ipa.conf
@@ -1,5 +1,5 @@
 #
-# VERSION 4 - DO NOT REMOVE THIS LINE
+# VERSION 5 - DO NOT REMOVE THIS LINE
 #
 # LoadModule auth_kerb_module modules/mod_auth_kerb.so
 
@@ -72,6 +72,12 @@ KrbConstrainedDelegationLock ipa
   Allow from all
 </Location>
 
+<Location "/ipa/session/change_password">
+  Satisfy Any
+  Order Deny,Allow
+  Allow from all
+</Location>
+
 # This is where we redirect on failed auth
 Alias /ipa/errors "/usr/share/ipa/html"
 
diff --git a/ipaserver/plugins/xmlserver.py b/ipaserver/plugins/xmlserver.py
index 4ae914950b3dc4f593f03853dc1450d1dfea78d5..bd9eb1fdf72a1b8f5cab727d63070de377df07c9 100644
--- a/ipaserver/plugins/xmlserver.py
+++ b/ipaserver/plugins/xmlserver.py
@@ -25,10 +25,11 @@ Loads WSGI server plugins.
 from ipalib import api
 
 if 'in_server' in api.env and api.env.in_server is True:
-    from ipaserver.rpcserver import wsgi_dispatch, xmlserver, jsonserver_kerb, jsonserver_session, login_kerberos, login_password
+    from ipaserver.rpcserver import wsgi_dispatch, xmlserver, jsonserver_kerb, jsonserver_session, login_kerberos, login_password, change_password
     api.register(wsgi_dispatch)
     api.register(xmlserver)
     api.register(jsonserver_kerb)
     api.register(jsonserver_session)
     api.register(login_kerberos)
     api.register(login_password)
+    api.register(change_password)
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index f9a549f4e8e0135e68c3a2ae8a9e876cac0d88df..5abbaf1a693dbcdfee53a83cf87fb70bc2e0eadc 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -28,7 +28,7 @@ from xml.sax.saxutils import escape
 from xmlrpclib import Fault
 from ipalib import plugable
 from ipalib.backend import Executioner
-from ipalib.errors import PublicError, InternalError, CommandError, JSONError, ConversionError, CCacheError, RefererError, InvalidSessionPassword
+from ipalib.errors import PublicError, InternalError, CommandError, JSONError, ConversionError, CCacheError, RefererError, InvalidSessionPassword, NotFound, ACIError, ExecutionError
 from ipalib.request import context, Connection, destroy_context
 from ipalib.rpc import xml_dumps, xml_loads
 from ipalib.util import parse_time_duration
@@ -100,6 +100,18 @@ _unauthorized_template = """<html>
 </body>
 </html>"""
 
+_pwchange_template = """<html>
+<head>
+<title>200 Success</title>
+</head>
+<body>
+<h1>%(title)s</h1>
+<p>
+<strong>%(message)s</strong>
+</p>
+</body>
+</html>"""
+
 class HTTP_Status(plugable.Plugin):
     def not_found(self, environ, start_response, url, message):
         """
@@ -992,3 +1004,97 @@ class login_password(Backend, KerberosSession, HTTP_Status):
         if returncode != 0:
             raise InvalidSessionPassword(principal=principal, message=unicode(stderr))
 
+class change_password(Backend, HTTP_Status):
+
+    content_type = 'text/plain'
+    key = '/session/change_password'
+
+    def __init__(self):
+        super(change_password, self).__init__()
+
+    def _on_finalize(self):
+        super(change_password, self)._on_finalize()
+        self.api.Backend.wsgi_dispatch.mount(self, self.key)
+
+    def __call__(self, environ, start_response):
+        self.info('WSGI change_password.__call__:')
+
+        # Get the user and password parameters from the request
+        content_type = environ.get('CONTENT_TYPE', '').lower()
+        if not content_type.startswith('application/x-www-form-urlencoded'):
+            return self.bad_request(environ, start_response, "Content-Type must be application/x-www-form-urlencoded")
+
+        method = environ.get('REQUEST_METHOD', '').upper()
+        if method == 'POST':
+            query_string = read_input(environ)
+        else:
+            return self.bad_request(environ, start_response, "HTTP request method must be POST")
+
+        try:
+            query_dict = urlparse.parse_qs(query_string)
+        except Exception, e:
+            return self.bad_request(environ, start_response, "cannot parse query data")
+
+        data = {}
+        for field in ('user', 'old_password', 'new_password'):
+            value = query_dict.get(field, None)
+            if value is not None:
+                if len(value) == 1:
+                    data[field] = value[0]
+                else:
+                    return self.bad_request(environ, start_response, "more than one %s parameter"
+                                            % field)
+            else:
+                return self.bad_request(environ, start_response, "no %s specified" % field)
+
+        # start building the response
+        self.info("WSGI change_password: start password change of user '%s'", data['user'])
+        status = HTTP_STATUS_SUCCESS
+        response_headers = [('Content-Type', 'text/html; charset=utf-8')]
+        title = 'Password change rejected'
+        result = 'error'
+        policy_error = None
+
+        bind_dn = str(DN((self.api.Object.user.primary_key.name, data['user']),
+                      self.api.env.container_user, self.api.env.basedn))
+
+        try:
+            conn = ldap2(shared_instance=False,
+                         ldap_uri=self.api.env.ldap_uri)
+            conn.connect(bind_dn=bind_dn, bind_pw=data['old_password'])
+        except (NotFound, ACIError):
+            result = 'invalid-password'
+            message = 'The old password or username is not correct.'
+        except Exception, e:
+            message = "Could not connect to LDAP server."
+            self.error("change_password: cannot authenticate '%s' to LDAP server: %s",
+                    data['user'], str(e))
+        else:
+            try:
+                conn.modify_password(bind_dn, data['new_password'], data['old_password'])
+            except ExecutionError, e:
+                result = 'policy-error'
+                policy_error = escape(str(e))
+                message = "Password change was rejected: %s" % escape(str(e))
+            except Exception, e:
+                message = "Could not change the password"
+                self.error("change_password: cannot change password of '%s': %s",
+                        data['user'], str(e))
+            else:
+                result = 'ok'
+                title = "Password change successful"
+                message = "Password was changed."
+            finally:
+                if conn.isconnected():
+                    conn.destroy_connection()
+
+        self.info('%s: %s', status, message)
+
+        response_headers.append(('X-IPA-Pwchange-Result', result))
+        if policy_error:
+            response_headers.append(('X-IPA-Pwchange-Policy-Error', policy_error))
+
+        start_response(status, response_headers)
+        output = _pwchange_template % dict(title=str(title),
+                                           message=str(message))
+        return [output]
diff --git a/tests/test_ipaserver/httptest.py b/tests/test_ipaserver/httptest.py
new file mode 100644
index 0000000000000000000000000000000000000000..7f1b5b136f3b0c0d62a0ea406be142fe3c1da98b
--- /dev/null
+++ b/tests/test_ipaserver/httptest.py
@@ -0,0 +1,52 @@
+# Authors:
+#   Martin Kosek <mko...@redhat.com>
+#
+# Copyright (C) 2012  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""
+Base class for HTTP request tests
+"""
+
+import urllib
+import httplib
+
+from ipalib import api
+
+class Unauthorized_HTTP_test(object):
+    """
+    Base class for simple HTTP request tests executed against URI
+    with no required authorization
+    """
+    app_uri = ''
+    host = api.env.host
+    content_type = 'application/x-www-form-urlencoded'
+
+    def send_request(self, method='POST', params=None):
+        """
+        Send a request to HTTP server
+
+        :param key When not None, overrides default app_uri
+        """
+        if params is not None:
+            params = urllib.urlencode(params, True)
+        url = 'https://' + self.host + self.app_uri
+
+        headers = {'Content-Type' : self.content_type,
+                   'Referer' : url}
+
+        conn = httplib.HTTPSConnection(self.host)
+        conn.request(method, self.app_uri, params, headers)
+        return conn.getresponse()
diff --git a/tests/test_ipaserver/test_changepw.py b/tests/test_ipaserver/test_changepw.py
new file mode 100644
index 0000000000000000000000000000000000000000..035766855abc384aed52a926b719c3e414cbe3c9
--- /dev/null
+++ b/tests/test_ipaserver/test_changepw.py
@@ -0,0 +1,109 @@
+# Authors:
+#   Martin Kosek <mko...@redhat.com>
+#
+# Copyright (C) 2012  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import nose
+
+from httptest import Unauthorized_HTTP_test
+from tests.test_xmlrpc.xmlrpc_test import XMLRPC_test
+from tests.util import assert_equal, assert_not_equal
+from ipalib import api, errors
+from ipalib.dn import DN
+import ldap
+
+testuser = u'tuser'
+old_password = u'old_password'
+new_password = u'new_password'
+
+class test_changepw(XMLRPC_test, Unauthorized_HTTP_test):
+    app_uri = '/ipa/session/change_password'
+
+    def setUp(self):
+        super(test_changepw, self).setUp()
+        try:
+            api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User')
+            api.Command['passwd'](testuser, password=u'old_password')
+        except errors.ExecutionError, e:
+            raise nose.SkipTest(
+                'Cannot set up test user: %s' % e
+            )
+
+    def tearDown(self):
+        try:
+            api.Command['user_del']([testuser])
+        except errors.NotFound:
+            pass
+        super(test_changepw, self).tearDown()
+
+    def _changepw(self, user, old_password, new_password):
+        return self.send_request(params={'user': str(user),
+                                  'old_password' : str(old_password),
+                                  'new_password' : str(new_password)},
+                                 )
+
+    def _checkpw(self, user, password):
+        dn = str(DN(('uid', user),
+                    api.env.container_user,
+                    api.env.basedn))
+        conn = ldap.initialize(api.env.ldap_uri)
+        try:
+            conn.simple_bind_s(dn, password)
+        finally:
+            conn.unbind_s()
+
+    def test_bad_options(self):
+        for params in (None,                    # no params
+                      {'user': 'foo'},          # missing options
+                      {'user': 'foo',
+                       'old_password' : 'old'}, # missing option
+                      {'user': 'foo',
+                       'old_password' : 'old',
+                       'new_password' : ''},    # empty option
+                      ):
+            response = self.send_request(params=params)
+            assert_equal(response.status, 400)
+            assert_equal(response.reason, 'Bad Request')
+
+    def test_invalid_auth(self):
+        response = self._changepw(testuser, 'wrongpassword', 'new_password')
+
+        assert_equal(response.status, 200)
+        assert_equal(response.getheader('X-IPA-Pwchange-Result'), 'invalid-password')
+
+        # make sure that password is NOT changed
+        self._checkpw(testuser, old_password)
+
+    def test_pwpolicy_error(self):
+        response = self._changepw(testuser, old_password, '1')
+
+        assert_equal(response.status, 200)
+        assert_equal(response.getheader('X-IPA-Pwchange-Result'), 'policy-error')
+        assert_equal(response.getheader('X-IPA-Pwchange-Policy-Error'),
+                     'Constraint violation: Password is too short')
+
+        # make sure that password is NOT changed
+        self._checkpw(testuser, old_password)
+
+    def test_pwpolicy_success(self):
+        response = self._changepw(testuser, old_password, new_password)
+
+        assert_equal(response.status, 200)
+        assert_equal(response.getheader('X-IPA-Pwchange-Result'), 'ok')
+
+        # make sure that password IS changed
+        self._checkpw(testuser, new_password)
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to