diff --git a/cms/envs/common.py b/cms/envs/common.py index 8070c0a931d415a787688a2d0d6f7d5118b06601..36d156b6af120ebea2f30882b6fa678afde38085 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -436,7 +436,7 @@ LOGIN_URL = reverse_lazy('login_redirect_to_lms') # use the ratelimit backend to prevent brute force attacks AUTHENTICATION_BACKENDS = [ 'rules.permissions.ObjectPermissionBackend', - 'ratelimitbackend.backends.RateLimitModelBackend', + 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.backends.EdxRateLimitedAllowAllUsersModelBackend', ] LMS_BASE = None diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 8cc5f89c5b7cea8a8119cbb4acc0bdf3e7d7bf85..56d828e6affa6e1e7764a126fc82327e9b56eaa1 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -191,7 +191,7 @@ def _authenticate_first_party(request, unauthenticated_user): raise AuthFailedError(_('Too many failed login attempts. Try again later.')) -def _handle_failed_authentication(user): +def _handle_failed_authentication(user, has_authentication): """ Handles updating the failed login count, inactive user notifications, and logging failed authentications. """ @@ -199,7 +199,7 @@ def _handle_failed_authentication(user): if LoginFailures.is_feature_enabled(): LoginFailures.increment_lockout_counter(user) - if not user.is_active: + if has_authentication and not user.is_active: _log_and_raise_inactive_user_auth_error(user) # if we didn't find this username earlier, the account for this email @@ -335,7 +335,7 @@ def login_user(request): _enforce_password_policy_compliance(request, possibly_authenticated_user) if possibly_authenticated_user is None or not possibly_authenticated_user.is_active: - _handle_failed_authentication(email_user) + _handle_failed_authentication(email_user, possibly_authenticated_user) _handle_successful_authentication_and_login(possibly_authenticated_user, request) 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 00d6152f2d4fa6511f8bd84dfbe9e38a785486aa..00618e8daee3dd417b602267c3b4bba8e8449afe 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -1,4 +1,4 @@ -#coding:utf-8 +# coding:utf-8 """ Tests for student activation and login """ @@ -9,7 +9,6 @@ import unicodedata import ddt import six -from six.moves import range from django.conf import settings from django.contrib.auth.models import User from django.core import mail @@ -19,7 +18,6 @@ from django.test.client import Client from django.test.utils import override_settings from django.urls import NoReverseMatch, reverse from mock import patch - from openedx.core.djangoapps.password_policy.compliance import ( NonCompliantPasswordException, NonCompliantPasswordWarning @@ -28,6 +26,7 @@ from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRI 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.djangolib.testing.utils import CacheIsolationTestCase +from six.moves import range from student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory @@ -38,51 +37,54 @@ class LoginTest(CacheIsolationTestCase): """ ENABLED_CACHES = ['default'] + LOGIN_FAILED_WARNING = 'Email or password is incorrect' + ACTIVATE_ACCOUNT_WARNING = 'In order to sign in, you need to activate your account' + username = 'test' + user_email = 'test@edx.org' + password = 'test_password' def setUp(self): + """Setup a test user along with its registration and profile""" super(LoginTest, self).setUp() - # Create one user and save it to the database - self.user = UserFactory.build(username='test', email='test@edx.org') - self.user.set_password('test_password') + self.user = UserFactory.build(username=self.username, email=self.user_email) + self.user.set_password(self.password) self.user.save() - # Create a registration for the user RegistrationFactory(user=self.user) - - # Create a profile for the user UserProfileFactory(user=self.user) - # Create the test client self.client = Client() cache.clear() - # Store the login url try: self.url = reverse('login_post') except NoReverseMatch: self.url = reverse('login') def test_login_success(self): - response, mock_audit_log = self._login_response('test@edx.org', 'test_password', - patched_audit_log='student.models.AUDIT_LOG') + response, mock_audit_log = self._login_response( + self.user_email, self.password, patched_audit_log='student.models.AUDIT_LOG' + ) self._assert_response(response, success=True) - self._assert_audit_log(mock_audit_log, 'info', [u'Login success', u'test@edx.org']) + self._assert_audit_log(mock_audit_log, 'info', [u'Login success', self.user_email]) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_login_success_no_pii(self): - response, mock_audit_log = self._login_response('test@edx.org', 'test_password', - patched_audit_log='student.models.AUDIT_LOG') + response, mock_audit_log = self._login_response( + self.user_email, self.password, patched_audit_log='student.models.AUDIT_LOG' + ) self._assert_response(response, success=True) self._assert_audit_log(mock_audit_log, 'info', [u'Login success']) - self._assert_not_in_audit_log(mock_audit_log, 'info', [u'test@edx.org']) + self._assert_not_in_audit_log(mock_audit_log, 'info', [self.user_email]) def test_login_success_unicode_email(self): unicode_email = u'test' + six.unichr(40960) + u'@edx.org' self.user.email = unicode_email self.user.save() - response, mock_audit_log = self._login_response(unicode_email, 'test_password', - patched_audit_log='student.models.AUDIT_LOG') + response, mock_audit_log = self._login_response( + unicode_email, self.password, patched_audit_log='student.models.AUDIT_LOG' + ) self._assert_response(response, success=True) self._assert_audit_log(mock_audit_log, 'info', [u'Login success', unicode_email]) @@ -103,20 +105,9 @@ class LoginTest(CacheIsolationTestCase): nonexistent_email = u'not_a_user@edx.org' response, mock_audit_log = self._login_response( nonexistent_email, - 'test_password', - ) - self._assert_response(response, success=False, - value='Email or password is incorrect') - self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) - - def test_login_fail_incorrect_email(self): - nonexistent_email = u'not_a_user@edx.org' - response, mock_audit_log = self._login_response( - nonexistent_email, - 'test_password', + self.password, ) - self._assert_response(response, success=False, - value='Email or password is incorrect') + self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) @@ -124,85 +115,95 @@ class LoginTest(CacheIsolationTestCase): nonexistent_email = u'not_a_user@edx.org' response, mock_audit_log = self._login_response( nonexistent_email, - 'test_password', + self.password, ) - self._assert_response(response, success=False, - value='Email or password is incorrect') + self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email']) self._assert_not_in_audit_log(mock_audit_log, 'warning', [nonexistent_email]) def test_login_fail_wrong_password(self): response, mock_audit_log = self._login_response( - 'test@edx.org', + self.user_email, 'wrong_password', ) - self._assert_response(response, success=False, - value='Email or password is incorrect') + self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) self._assert_audit_log(mock_audit_log, 'warning', - [u'Login failed', u'password for', u'test@edx.org', u'invalid']) + [u'Login failed', u'password for', self.user_email, u'invalid']) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_login_fail_wrong_password_no_pii(self): - response, mock_audit_log = self._login_response( - 'test@edx.org', - 'wrong_password', - ) - self._assert_response(response, success=False, - value='Email or password is incorrect') + response, mock_audit_log = self._login_response(self.user_email, 'wrong_password') + self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'password for', u'invalid']) - self._assert_not_in_audit_log(mock_audit_log, 'warning', [u'test@edx.org']) + self._assert_not_in_audit_log(mock_audit_log, 'warning', [self.user_email]) - def test_login_not_activated(self): + @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) + def test_login_not_activated_no_pii(self): # De-activate the user self.user.is_active = False self.user.save() # Should now be unable to login response, mock_audit_log = self._login_response( - 'test@edx.org', - 'test_password', + self.user_email, + self.password ) self._assert_response(response, success=False, value="In order to sign in, you need to activate your account.") self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user']) + self._assert_not_in_audit_log(mock_audit_log, 'warning', [u'test']) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) - def test_login_not_activated_no_pii(self): - # De-activate the user + def test_login_not_activated_with_correct_credentials(self): + """ + Tests that when user login with the correct credentials but with an inactive + account, the system, send account activation email notification to the user. + """ self.user.is_active = False self.user.save() - # Should now be unable to login response, mock_audit_log = self._login_response( - 'test@edx.org', - 'test_password', + self.user_email, + self.password, ) - self._assert_response(response, success=False, - value="In order to sign in, you need to activate your account.") + self._assert_response(response, success=False, value=self.ACTIVATE_ACCOUNT_WARNING) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Account not active for user']) - self._assert_not_in_audit_log(mock_audit_log, 'warning', [u'test']) + + @patch('openedx.core.djangoapps.user_authn.views.login._log_and_raise_inactive_user_auth_error') + def test_login_inactivated_user_with_incorrect_credentials(self, mock_inactive_user_email_and_error): + """ + Tests that when user login with incorrect credentials and an inactive account, + the system does *not* send account activation email notification to the user. + """ + nonexistent_email = 'incorrect@email.com' + self.user.is_active = False + self.user.save() + response, mock_audit_log = self._login_response(nonexistent_email, 'incorrect_password') + + self.assertFalse(mock_inactive_user_email_and_error.called) + self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) + self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) def test_login_unicode_email(self): - unicode_email = u'test@edx.org' + six.unichr(40960) + unicode_email = self.user_email + six.unichr(40960) response, mock_audit_log = self._login_response( unicode_email, - 'test_password', + self.password, ) self._assert_response(response, success=False) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', unicode_email]) def test_login_unicode_password(self): - unicode_password = u'test_password' + six.unichr(1972) + unicode_password = self.password + six.unichr(1972) response, mock_audit_log = self._login_response( - 'test@edx.org', + self.user_email, unicode_password, ) self._assert_response(response, success=False) self._assert_audit_log(mock_audit_log, 'warning', - [u'Login failed', u'password for', u'test@edx.org', u'invalid']) + [u'Login failed', u'password for', self.user_email, u'invalid']) def test_logout_logging(self): - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) logout_url = reverse('logout') with patch('student.models.AUDIT_LOG') as mock_audit_log: @@ -211,7 +212,7 @@ class LoginTest(CacheIsolationTestCase): self._assert_audit_log(mock_audit_log, 'info', [u'Logout', u'test']) def test_login_user_info_cookie(self): - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) # Verify the format of the "user info" cookie set on login @@ -226,7 +227,7 @@ class LoginTest(CacheIsolationTestCase): self.assertIn("http://testserver/", url) def test_logout_deletes_mktg_cookies(self): - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) # Check that the marketing site cookies have been set @@ -251,7 +252,7 @@ class LoginTest(CacheIsolationTestCase): # When logged in cookie names are loaded from JSON files, they may # have type `unicode` instead of `str`, which can cause errors # when calling Django cookie manipulation functions. - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) response = self.client.post(reverse('logout')) @@ -262,7 +263,7 @@ class LoginTest(CacheIsolationTestCase): @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_logout_logging_no_pii(self): - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) logout_url = reverse('logout') with patch('student.models.AUDIT_LOG') as mock_audit_log: @@ -276,10 +277,10 @@ class LoginTest(CacheIsolationTestCase): # and verify that you can still successfully log in afterwards. for i in range(20): password = u'test_password{0}'.format(i) - response, _audit_log = self._login_response('test@edx.org', password) + response, _audit_log = self._login_response(self.user_email, password) self._assert_response(response, success=False) # now try logging in with a valid password - response, _audit_log = self._login_response('test@edx.org', 'test_password') + response, _audit_log = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) def test_login_ratelimited(self): @@ -287,9 +288,9 @@ class LoginTest(CacheIsolationTestCase): # login attempts in one 5 minute period before the rate gets limited for i in range(30): password = u'test_password{0}'.format(i) - self._login_response('test@edx.org', password) + self._login_response(self.user_email, password) # check to see if this response indicates that this was ratelimited - response, _audit_log = self._login_response('test@edx.org', 'wrong_password') + response, _audit_log = self._login_response(self.user_email, 'wrong_password') self._assert_response(response, success=False, value='Too many failed login attempts') @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) @@ -299,7 +300,7 @@ class LoginTest(CacheIsolationTestCase): self.assertIn(jwt_cookies.jwt_cookie_header_payload_name(), self.client.cookies) setup_login_oauth_client() - response, _ = self._login_response('test@edx.org', 'test_password') + response, _ = self._login_response(self.user_email, self.password) _assert_jwt_cookie_present(response) response = self.client.post(reverse('login_refresh')) @@ -313,7 +314,7 @@ class LoginTest(CacheIsolationTestCase): @patch.dict("django.conf.settings.FEATURES", {'PREVENT_CONCURRENT_LOGINS': True}) def test_single_session(self): - creds = {'email': 'test@edx.org', 'password': 'test_password'} + creds = {'email': self.user_email, 'password': self.password} client1 = Client() client2 = Client() @@ -348,13 +349,13 @@ class LoginTest(CacheIsolationTestCase): cms """ user = UserFactory.build(username='tester', email='tester@edx.org') - user.set_password('test_password') + user.set_password(self.password) user.save() # Assert that no profile is created. self.assertFalse(hasattr(user, 'profile')) - creds = {'email': 'tester@edx.org', 'password': 'test_password'} + creds = {'email': 'tester@edx.org', 'password': self.password} client1 = Client() client2 = Client() @@ -387,7 +388,7 @@ class LoginTest(CacheIsolationTestCase): # accessing logout url as it does not have login-required decorator it will avoid redirect # and go inside the enforce_single_login - creds = {'email': 'test@edx.org', 'password': 'test_password'} + creds = {'email': self.user_email, 'password': self.password} client1 = Client() client2 = Client() @@ -418,9 +419,7 @@ class LoginTest(CacheIsolationTestCase): with patch('student.views.change_enrollment') as mock_change_enrollment: mock_change_enrollment.return_value = HttpResponseBadRequest("I am a 400") response, _ = self._login_response( - 'test@edx.org', - 'test_password', - extra_post_params=extra_post_params, + self.user_email, self.password, extra_post_params=extra_post_params, ) response_content = json.loads(response.content) self.assertIsNone(response_content["redirect_url"]) @@ -436,9 +435,7 @@ class LoginTest(CacheIsolationTestCase): with patch('student.views.change_enrollment') as mock_change_enrollment: mock_change_enrollment.return_value = HttpResponse() response, _ = self._login_response( - 'test@edx.org', - 'test_password', - extra_post_params=extra_post_params, + self.user_email, self.password, extra_post_params=extra_post_params, ) response_content = json.loads(response.content) self.assertIsNone(response_content["redirect_url"]) @@ -452,10 +449,7 @@ class LoginTest(CacheIsolationTestCase): enforce_compliance_path = 'openedx.core.djangoapps.password_policy.compliance.enforce_compliance_on_login' with patch(enforce_compliance_path) as mock_check_password_policy_compliance: mock_check_password_policy_compliance.return_value = HttpResponse() - response, _ = self._login_response( - 'test@edx.org', - 'test_password', - ) + response, _ = self._login_response(self.user_email, self.password) response_content = json.loads(response.content) self.assertTrue(response_content.get('success')) @@ -464,12 +458,12 @@ class LoginTest(CacheIsolationTestCase): """ Tests _enforce_password_policy_compliance fails with an exception thrown """ - with patch('openedx.core.djangoapps.password_policy.compliance.enforce_compliance_on_login') as \ - mock_enforce_compliance_on_login: + enforce_compliance_on_login = 'openedx.core.djangoapps.password_policy.compliance.enforce_compliance_on_login' + with patch(enforce_compliance_on_login) as mock_enforce_compliance_on_login: mock_enforce_compliance_on_login.side_effect = NonCompliantPasswordException() response, _ = self._login_response( - 'test@edx.org', - 'test_password' + self.user_email, + self.password ) response_content = json.loads(response.content) self.assertFalse(response_content.get('success')) @@ -481,13 +475,10 @@ class LoginTest(CacheIsolationTestCase): """ Tests _enforce_password_policy_compliance succeeds with a warning thrown """ - with patch('openedx.core.djangoapps.password_policy.compliance.enforce_compliance_on_login') as \ - mock_enforce_compliance_on_login: + enforce_compliance_on_login = 'openedx.core.djangoapps.password_policy.compliance.enforce_compliance_on_login' + with patch(enforce_compliance_on_login) as mock_enforce_compliance_on_login: mock_enforce_compliance_on_login.side_effect = NonCompliantPasswordWarning('Test warning') - response, _ = self._login_response( - 'test@edx.org', - 'test_password' - ) + response, _ = self._login_response(self.user_email, self.password) response_content = json.loads(response.content) self.assertIn('Test warning', self.client.session['_messages']) self.assertTrue(response_content.get('success'))