From 952ab26dd2bc5d29ca0f8bc094dcef09d7c5e52f Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Tue, 19 Oct 2021 12:37:09 +0200 Subject: [PATCH 2/2] ldap: inform user when password change has failed (#57733) --- src/authentic2/backends/ldap_backend.py | 4 +-- src/authentic2/utils/misc.py | 5 ++++ src/authentic2/views.py | 6 +++- tests/test_ldap.py | 38 +++++++++++++++++++++---- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/authentic2/backends/ldap_backend.py b/src/authentic2/backends/ldap_backend.py index 70f45382..e07ef0c4 100644 --- a/src/authentic2/backends/ldap_backend.py +++ b/src/authentic2/backends/ldap_backend.py @@ -58,7 +58,7 @@ from authentic2.ldap_utils import FilterFormatter from authentic2.middleware import StoreRequestMiddleware from authentic2.models import UserExternalId from authentic2.user_login_failure import user_login_failure, user_login_success -from authentic2.utils.misc import to_list +from authentic2.utils.misc import PasswordChangeError, to_list from django_rbac.utils import get_ou_model # code originaly copied from by now merely inspired by @@ -478,7 +478,7 @@ class LDAPUser(User): self.ldap_backend.modify_password(conn, self.block, self.dn, _current_password, new_password) except ldap.LDAPError as e: log.warning('ldap: set_password failed (%s)', type(e).__name__) - return + raise PasswordChangeError(_('LDAP directory refused the password change.')) self._current_password = new_password self.keep_password_in_session(new_password) if self.block['keep_password']: diff --git a/src/authentic2/utils/misc.py b/src/authentic2/utils/misc.py index f716d939..c6865de1 100644 --- a/src/authentic2/utils/misc.py +++ b/src/authentic2/utils/misc.py @@ -1310,3 +1310,8 @@ def prepend_remember_cookie(request, response, name, value, count=5): path=request.path, httponly=True, ) + + +class PasswordChangeError(Exception): + def __init__(self, message): + self.message = message diff --git a/src/authentic2/views.py b/src/authentic2/views.py index 5063a1ad..1513d211 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -1414,7 +1414,11 @@ class PasswordChangeView(DjPasswordChangeView): hooks.call_hooks('event', name='change-password', user=self.request.user, request=self.request) messages.info(self.request, _('Password changed')) models.PasswordReset.objects.filter(user=self.request.user).delete() - response = super().form_valid(form) + try: + response = super().form_valid(form) + except utils_misc.PasswordChangeError as e: + messages.error(self.request, e.message) + return utils_misc.redirect(self.request, self.post_change_redirect) self.request.journal.record('user.password.change', session=self.request.session) return response diff --git a/tests/test_ldap.py b/tests/test_ldap.py index f6c576ca..a1b87678 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -37,7 +37,7 @@ from authentic2.a2_rbac.utils import get_default_ou from authentic2.backends import ldap_backend from authentic2.models import Service from authentic2.utils import switch_user -from authentic2.utils.misc import authenticate +from authentic2.utils.misc import PasswordChangeError, authenticate from django_rbac.utils import get_ou_model from . import utils @@ -1056,6 +1056,32 @@ def test_user_cannot_change_password(slapd, settings, app, db): assert response['Location'].endswith('/accounts/') +def test_user_change_password_denied(slapd, settings, app, db): + settings.LDAP_AUTH_SETTINGS = [ + { + 'url': [slapd.ldap_url], + 'basedn': 'o=ôrga', + 'use_tls': False, + } + ] + assert User.objects.count() == 0 + # first login + response = app.get('/login/') + response.form['username'] = USERNAME + response.form['password'] = PASS + response = response.form.submit('login-password-submit').follow() + + response = app.get('/accounts/password/change/') + response.form['old_password'] = PASS + response.form['new_password1'] = 'hopAbcde1' + response.form['new_password2'] = 'hopAbcde1' + with mock.patch( + 'authentic2.backends.ldap_backend.LDAPBackend.modify_password', side_effect=ldap.UNWILLING_TO_PERFORM + ): + response = response.form.submit().follow() + assert 'LDAP directory refused the password change' in response.text + + def test_tls(db, tls_slapd, settings, client): conn = tls_slapd.get_connection_admin() conn.modify_s( @@ -1266,8 +1292,9 @@ def test_set_password(slapd, settings, db, caplog): with mock.patch( 'authentic2.backends.ldap_backend.LDAPBackend.modify_password', side_effect=ldap.UNWILLING_TO_PERFORM ): - user.set_password('passé') - assert 'set_password failed (UNWILLING_TO_PERFORM)' in caplog.text + with pytest.raises(PasswordChangeError): + user.set_password('passé') + assert 'set_password failed (UNWILLING_TO_PERFORM)' in caplog.text def test_login_ppolicy_pwdMaxFailure(slapd_ppolicy, settings, db, app): @@ -1664,8 +1691,9 @@ pwdSafeModify: FALSE ) user = authenticate(username=USERNAME, password=UPASS) - assert user.set_password('ogutOmyetew4') is None - assert 'STRONG_AUTH_REQUIRED' in caplog.text + with pytest.raises(PasswordChangeError): + user.set_password('ogutOmyetew4') + assert 'STRONG_AUTH_REQUIRED' in caplog.text def test_ou_selector(slapd, settings, app, ou1): -- 2.30.2