From 921dadac9999f0b2d5e26fa985d131c9e33a7769 Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Tue, 5 Apr 2022 11:18:52 +0500 Subject: [PATCH] feat: add password compliance check for login (#30149) Add nudge and block checks for HIBP API on login view VAN-667 VAN-668 --- cms/envs/common.py | 24 +++++++++++ lms/envs/common.py | 24 +++++++++++ .../djangoapps/user_api/accounts/__init__.py | 9 ++++ .../core/djangoapps/user_authn/exceptions.py | 20 +++++++++ openedx/core/djangoapps/user_authn/tasks.py | 3 +- .../core/djangoapps/user_authn/views/login.py | 43 +++++++++++++++---- .../user_authn/views/tests/test_login.py | 31 +++++++++++++ 7 files changed, 145 insertions(+), 9 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 759f474d68..3d7683ae84 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -542,6 +542,30 @@ ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY = False ENABLE_AUTHN_REGISTER_HIBP_POLICY = False HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD = 3 +# .. toggle_name: ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When enabled, this toggle activates the use of the password validation +# on Authn MFE's login. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2022-03-29 +# .. toggle_target_removal_date: None +# .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-668 +ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY = False +HIBP_LOGIN_NUDGE_PASSWORD_FREQUENCY_THRESHOLD = 3 + +# .. toggle_name: ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When enabled, this toggle activates the use of the password validation +# on Authn MFE's login. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2022-03-29 +# .. toggle_target_removal_date: None +# .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-667 +ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY = False +HIBP_LOGIN_BLOCK_PASSWORD_FREQUENCY_THRESHOLD = 5 + ############################# SOCIAL MEDIA SHARING ############################# SOCIAL_SHARING_SETTINGS = { # Note: Ensure 'CUSTOM_COURSE_URLS' has a matching value in lms/envs/common.py diff --git a/lms/envs/common.py b/lms/envs/common.py index bf81a9769e..91f0e58d20 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4829,6 +4829,30 @@ ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY = False ENABLE_AUTHN_REGISTER_HIBP_POLICY = False HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD = 3 +# .. toggle_name: ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When enabled, this toggle activates the use of the password validation +# on Authn MFE's login. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2022-03-29 +# .. toggle_target_removal_date: None +# .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-668 +ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY = False +HIBP_LOGIN_NUDGE_PASSWORD_FREQUENCY_THRESHOLD = 3 + +# .. toggle_name: ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: When enabled, this toggle activates the use of the password validation +# on Authn MFE's login. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2022-03-29 +# .. toggle_target_removal_date: None +# .. toggle_tickets: https://openedx.atlassian.net/browse/VAN-667 +ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY = False +HIBP_LOGIN_BLOCK_PASSWORD_FREQUENCY_THRESHOLD = 5 + ############### Settings for the ace_common plugin ################# ACE_ENABLED_CHANNELS = ['django_email'] ACE_ENABLED_POLICIES = ['bulk_email_optout'] diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 2fe11ba4c0..6eca7b3532 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -107,3 +107,12 @@ REQUIRED_FIELD_LEVEL_OF_EDUCATION_MSG = _("Select the highest level of education REQUIRED_FIELD_YEAR_OF_BIRTH_MSG = _("Select your year of birth") REQUIRED_FIELD_GENDER_MSG = _("Select your gender") REQUIRED_FIELD_MAILING_ADDRESS_MSG = _("Enter your mailing address.") + +# HIBP Strings +AUTHN_LOGIN_BLOCK_HIBP_POLICY_MSG = _( + 'Our system detected that your password is vulnerable. Change your password so that your account stays secure.' +) +AUTHN_LOGIN_NUDGE_HIBP_POLICY_MSG = _( + 'Our system detected that your password is vulnerable. ' + 'We recommend you change it so that your account stays secure.' +) diff --git a/openedx/core/djangoapps/user_authn/exceptions.py b/openedx/core/djangoapps/user_authn/exceptions.py index 6ce80aa1cb..54f208730f 100644 --- a/openedx/core/djangoapps/user_authn/exceptions.py +++ b/openedx/core/djangoapps/user_authn/exceptions.py @@ -27,3 +27,23 @@ class AuthFailedError(Exception): resp[attr] = self.__getattribute__(attr) return resp + + +class VulnerablePasswordError(Exception): + """ + This is a helper for the login view, allowing the view to error out if password + is vulnerable. + """ + def __init__(self, value, error_code, redirect_url=None): + super().__init__() + self.value = value + self.error_code = error_code + self.redirect_url = redirect_url + + def get_response(self): + return { + 'value': self.value, + 'error_code': self.error_code, + 'redirect_url': self.redirect_url, + 'success': False + } diff --git a/openedx/core/djangoapps/user_authn/tasks.py b/openedx/core/djangoapps/user_authn/tasks.py index 4c39085bfd..0104cd2409 100644 --- a/openedx/core/djangoapps/user_authn/tasks.py +++ b/openedx/core/djangoapps/user_authn/tasks.py @@ -34,12 +34,13 @@ def check_pwned_password_and_send_track_event(user_id, password, internal_user=F pwned_properties['internal_user'] = internal_user pwned_properties['new_user'] = is_new_user segment.track(user_id, 'edx.bi.user.pwned.password.status', pwned_properties) + return pwned_properties except Exception: # pylint: disable=W0703 log.exception( 'Unable to get response from pwned password api for user_id: "%s"', user_id, ) - return None # lint-amnesty, pylint: disable=raise-missing-from + return {} # lint-amnesty, pylint: disable=raise-missing-from @shared_task(bind=True) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 537ab32ec3..22b24dce4d 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -42,9 +42,10 @@ from common.djangoapps.util.password_policy_validators import normalize_password from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.user_api import accounts from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY from openedx.core.djangoapps.user_authn.cookies import get_response_with_refreshed_jwt_cookies, set_logged_in_cookies -from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError +from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError, VulnerablePasswordError from openedx.core.djangoapps.user_authn.toggles import ( is_require_third_party_auth_enabled, should_redirect_to_authn_microfrontend @@ -576,13 +577,26 @@ def login_user(request, api_version='v1'): if possibly_authenticated_user and password_policy_compliance.should_enforce_compliance_on_login(): # Important: This call must be made AFTER the user was successfully authenticated. _enforce_password_policy_compliance(request, possibly_authenticated_user) - check_pwned_password_and_send_track_event.delay(user.id, request.POST.get('password'), user.is_staff) if possibly_authenticated_user is None or not ( possibly_authenticated_user.is_active or settings.MARKETING_EMAILS_OPT_IN ): _handle_failed_authentication(user, possibly_authenticated_user) + pwned_properties = check_pwned_password_and_send_track_event( + user.id, request.POST.get('password'), user.is_staff + ) if not is_user_third_party_authenticated else {} + # Set default for third party login + password_frequency = pwned_properties.get('frequency', -1) + if ( + settings.ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY and + password_frequency >= settings.HIBP_LOGIN_BLOCK_PASSWORD_FREQUENCY_THRESHOLD + ): + raise VulnerablePasswordError( + accounts.AUTHN_LOGIN_BLOCK_HIBP_POLICY_MSG, + 'require-password-change' + ) + _handle_successful_authentication_and_login(possibly_authenticated_user, request) # The AJAX method calling should know the default destination upon success @@ -601,6 +615,16 @@ def login_user(request, api_version='v1'): enterprise_selection_page(request, possibly_authenticated_user, finish_auth_url or next_url) ) + if ( + settings.ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY and + 0 <= password_frequency <= settings.HIBP_LOGIN_NUDGE_PASSWORD_FREQUENCY_THRESHOLD + ): + raise VulnerablePasswordError( + accounts.AUTHN_LOGIN_NUDGE_HIBP_POLICY_MSG, + 'nudge-password-change', + redirect_url + ) + response = JsonResponse({ 'success': True, 'redirect_url': redirect_url, @@ -623,13 +647,16 @@ def login_user(request, api_version='v1'): set_custom_attribute('login_error_code', error_code) email_or_username_key = 'email' if api_version == API_V1 else 'email_or_username' email_or_username = request.POST.get(email_or_username_key, None) - email_or_username = possibly_authenticated_user.email \ - if possibly_authenticated_user else email_or_username + email_or_username = possibly_authenticated_user.email if possibly_authenticated_user else email_or_username response_content['email'] = email_or_username - response = JsonResponse(response_content, status=400) - set_custom_attribute('login_user_auth_failed_error', True) - set_custom_attribute('login_user_response_status', response.status_code) - return response + except VulnerablePasswordError as error: + response_content = error.get_response() + log.exception(response_content) + + response = JsonResponse(response_content, status=400) + set_custom_attribute('login_user_auth_failed_error', True) + set_custom_attribute('login_user_response_status', response.status_code) + return response # CSRF protection is not needed here because the only side effect diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 48452fdd36..e85611e996 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -27,7 +27,9 @@ from openedx.core.djangoapps.password_policy.compliance import ( NonCompliantPasswordException, NonCompliantPasswordWarning ) +from openedx.core.djangoapps.password_policy.hibp import PwnedPasswordsAPI from openedx.core.djangoapps.user_api.accounts import EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH +from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_PWNED_PASSWORD_API from openedx.core.djangoapps.user_authn.cookies import jwt_cookies from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client from openedx.core.djangoapps.user_authn.views.login import ( @@ -375,6 +377,35 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin): ) self._assert_not_in_audit_log(mock_audit_log, 'warning', [self.user_email]) + @override_settings(ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY=True) + @override_waffle_switch(ENABLE_PWNED_PASSWORD_API, True) + def test_password_compliance_block_error(self): + """ + Test that if HIBP Block flag is set to True and user's password lies + within block threshold, then login fails and user is not authenticated. + """ + password = hashlib.sha1(self.password.encode('utf-8')).hexdigest().upper() + api_response = {password[5:]: 1000000} + with patch.object(PwnedPasswordsAPI, 'range', return_value=api_response): + response, _ = self._login_response(self.user_email, self.password) + + self._assert_response(response, success=False, error_code='require-password-change') + + @override_settings(ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY=True) + @override_waffle_switch(ENABLE_PWNED_PASSWORD_API, True) + def test_password_compliance_nudge_error(self): + """ + Test that if HIBP Nudge flag is set to True and user's password lies + within nudge threshold, then user is authenticated and response contains + proper error code. + """ + password = hashlib.sha1(self.password.encode('utf-8')).hexdigest().upper() + api_response = {password[5:]: 10} + with patch.object(PwnedPasswordsAPI, 'range', return_value=api_response): + response, _ = self._login_response(self.user_email, self.password) + + self._assert_response(response, success=False, error_code='nudge-password-change') + def test_login_not_activated_no_pii(self): # De-activate the user self.user.is_active = False