From 07609e7d7554540810e8ed0f44bd7a4d513d83c0 Mon Sep 17 00:00:00 2001 From: Anthony Mangano Date: Thu, 19 Apr 2018 10:42:32 -0400 Subject: [PATCH] Add password_policy checks to login views LEARNER-4869 --- cms/urls.py | 6 + common/djangoapps/student/tests/test_login.py | 49 ++++ common/djangoapps/student/views/login.py | 16 ++ .../util/password_policy_validators.py | 6 +- lms/urls.py | 12 +- .../djangoapps/password_policy/__init__.py | 0 .../core/djangoapps/password_policy/apps.py | 60 +++++ .../djangoapps/password_policy/compliance.py | 144 ++++++++++++ .../core/djangoapps/password_policy/forms.py | 32 +++ .../password_policy/settings/__init__.py | 0 .../password_policy/settings/aws.py | 12 + .../password_policy/settings/common.py | 37 +++ .../password_policy/settings/devstack.py | 10 + .../password_policy/tests/__init__.py | 0 .../password_policy/tests/test_apps.py | 37 +++ .../password_policy/tests/test_compliance.py | 212 ++++++++++++++++++ .../password_policy/tests/test_forms.py | 78 +++++++ setup.py | 4 +- 18 files changed, 709 insertions(+), 6 deletions(-) create mode 100644 openedx/core/djangoapps/password_policy/__init__.py create mode 100644 openedx/core/djangoapps/password_policy/apps.py create mode 100644 openedx/core/djangoapps/password_policy/compliance.py create mode 100644 openedx/core/djangoapps/password_policy/forms.py create mode 100644 openedx/core/djangoapps/password_policy/settings/__init__.py create mode 100644 openedx/core/djangoapps/password_policy/settings/aws.py create mode 100644 openedx/core/djangoapps/password_policy/settings/common.py create mode 100644 openedx/core/djangoapps/password_policy/settings/devstack.py create mode 100644 openedx/core/djangoapps/password_policy/tests/__init__.py create mode 100644 openedx/core/djangoapps/password_policy/tests/test_apps.py create mode 100644 openedx/core/djangoapps/password_policy/tests/test_compliance.py create mode 100644 openedx/core/djangoapps/password_policy/tests/test_forms.py diff --git a/cms/urls.py b/cms/urls.py index ac3de948f5..d8f855b08e 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -10,6 +10,8 @@ import openedx.core.djangoapps.common_views.xblock import openedx.core.djangoapps.debug.views import openedx.core.djangoapps.external_auth.views import openedx.core.djangoapps.lang_pref.views +from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance +from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm from ratelimitbackend import admin @@ -17,6 +19,10 @@ django_autodiscover() admin.site.site_header = _('Studio Administration') admin.site.site_title = admin.site.site_header +if password_policy_compliance.should_enforce_compliance_on_login(): + admin.site.login_form = PasswordPolicyAwareAdminAuthForm + + # Pattern to match a course key or a library key COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format( r'[^/]+/[^/]+/[^/]+', r'[^/:]+:[^/+]+\+[^/+]+(\+[^/]+)?' diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index fd53f94d2a..9da1e8cbd9 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -19,6 +19,10 @@ from social_django.models import UserSocialAuth from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, waffle +from openedx.core.djangoapps.password_policy.compliance import ( + NonCompliantPasswordException, + NonCompliantPasswordWarning +) from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.tests.util import expected_redirect_url from student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory @@ -432,6 +436,51 @@ class LoginTest(CacheIsolationTestCase): self.assertIsNone(response_content["redirect_url"]) self._assert_response(response, success=True) + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_check_password_policy_compliance(self): + """ + Tests _enforce_password_policy_compliance succeeds when no exception is thrown + """ + with patch('student.views.login.password_policy_compliance.enforce_compliance_on_login') as mock_check_password_policy_compliance: + mock_check_password_policy_compliance.return_value = HttpResponse() + response, _ = self._login_response( + 'test@edx.org', + 'test_password', + ) + response_content = json.loads(response.content) + self.assertTrue(response_content.get('success')) + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_check_password_policy_compliance_exception(self): + """ + Tests _enforce_password_policy_compliance fails with an exception thrown + """ + with patch('student.views.login.password_policy_compliance.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' + ) + response_content = json.loads(response.content) + self.assertFalse(response_content.get('success')) + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_check_password_policy_compliance_warning(self): + """ + Tests _enforce_password_policy_compliance succeeds with a warning thrown + """ + with patch('student.views.login.password_policy_compliance.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_content = json.loads(response.content) + self.assertIn('Test warning', self.client.session['_messages']) + self.assertTrue(response_content.get('success')) + def _login_response(self, email, password, patched_audit_log='student.views.AUDIT_LOG', extra_post_params=None): """ Post the login info diff --git a/common/djangoapps/student/views/login.py b/common/djangoapps/student/views/login.py index cf07c3c475..0113902c96 100644 --- a/common/djangoapps/student/views/login.py +++ b/common/djangoapps/student/views/login.py @@ -44,8 +44,10 @@ from edxmako.shortcuts import render_to_response, render_to_string from eventtracking import tracker from openedx.core.djangoapps.external_auth.login_and_register import login as external_auth_login from openedx.core.djangoapps.external_auth.models import ExternalAuthMap +from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api.accounts.utils import generate_password +from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.features.course_experience import course_home_url_name from student.cookies import delete_logged_in_cookies, set_logged_in_cookies from student.forms import AccountCreationForm @@ -192,6 +194,17 @@ def _check_forced_password_reset(user): '"Forgot Password" link on this page to reset your password before logging in again.')) +def _enforce_password_policy_compliance(request, user): + try: + password_policy_compliance.enforce_compliance_on_login(user, request.POST.get('password')) + except password_policy_compliance.NonCompliantPasswordWarning as e: + # Allow login, but warn the user that they will be required to reset their password soon. + PageLevelMessages.register_warning_message(request, e.message) + except password_policy_compliance.NonCompliantPasswordException as e: + # Prevent the login attempt. + raise AuthFailedError(e.message) + + def _generate_not_activated_message(user): """ Generates the message displayed on the sign-in screen when a learner attempts to access the @@ -448,6 +461,9 @@ def login_user(request): if not was_authenticated_third_party: possibly_authenticated_user = _authenticate_first_party(request, email_user) + 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) if possibly_authenticated_user is None or not possibly_authenticated_user.is_active: _handle_failed_authentication(email_user) diff --git a/common/djangoapps/util/password_policy_validators.py b/common/djangoapps/util/password_policy_validators.py index 0363bfbcc0..d7c0138fb3 100644 --- a/common/djangoapps/util/password_policy_validators.py +++ b/common/djangoapps/util/password_policy_validators.py @@ -149,7 +149,7 @@ def password_instructions(): min_length).format(num=min_length, requirements=' & '.join(reqs)) -def validate_password(password, user=None, username=None): +def validate_password(password, user=None, username=None, password_reset=True): """ Checks user-provided password against our current site policy. @@ -159,6 +159,8 @@ def validate_password(password, user=None, username=None): password: The user-provided password as a string user: A User model object, if available. Required to check against security policy. username: The user-provided username, if available. Taken from 'user' if not provided. + password_reset: Whether to run validators that only make sense in a password reset + context (like PasswordHistory). """ if not isinstance(password, text_type): try: @@ -168,7 +170,7 @@ def validate_password(password, user=None, username=None): username = username or (user and user.username) - if user: + if user and password_reset: _validate_password_security(password, user) _validate_password_dictionary(password) diff --git a/lms/urls.py b/lms/urls.py index 7a72051748..f1aedc8198 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -10,7 +10,6 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic.base import RedirectView from branding import views as branding_views -from lms.djangoapps.certificates import views as certificates_views from config_models.views import ConfigurationModelCurrentAPIView from courseware.masquerade import handle_ajax as courseware_masquerade_handle_ajax from courseware.module_render import handle_xblock_callback, handle_xblock_callback_noauth, xblock_view, xqueue_callback @@ -20,6 +19,7 @@ from courseware.views.views import CourseTabView, EnrollStaffView, StaticCourseT from debug import views as debug_views from django_comment_common.models import ForumsConfig from django_openid_auth import views as django_openid_auth_views +from lms.djangoapps.certificates import views as certificates_views from lms.djangoapps.discussion import views as discussion_views from lms.djangoapps.instructor.views import coupons as instructor_coupons_views from lms.djangoapps.instructor.views import instructor_dashboard as instructor_dashboard_views @@ -30,17 +30,20 @@ from notes import views as notes_views from notification_prefs import views as notification_prefs_views from openedx.core.djangoapps.auth_exchange.views import LoginWithAccessTokenView from openedx.core.djangoapps.catalog.models import CatalogIntegration +from openedx.core.djangoapps.common_views.xblock import xblock_resource from openedx.core.djangoapps.cors_csrf import views as cors_csrf_views from openedx.core.djangoapps.course_groups import views as course_groups_views from openedx.core.djangoapps.debug import views as openedx_debug_views from openedx.core.djangoapps.external_auth import views as external_auth_views from openedx.core.djangoapps.lang_pref import views as lang_pref_views -from openedx.core.djangoapps.plugins import constants as plugin_constants, plugin_urls +from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance +from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm +from openedx.core.djangoapps.plugins import constants as plugin_constants +from openedx.core.djangoapps.plugins import plugin_urls from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.verified_track_content import views as verified_track_content_views -from openedx.core.djangoapps.common_views.xblock import xblock_resource from openedx.features.enterprise_support.api import enterprise_enabled from ratelimitbackend import admin from static_template_view import views as static_template_view_views @@ -55,6 +58,9 @@ if settings.DEBUG or settings.FEATURES.get('ENABLE_DJANGO_ADMIN_SITE'): admin.site.site_header = _('LMS Administration') admin.site.site_title = admin.site.site_header + if password_policy_compliance.should_enforce_compliance_on_login(): + admin.site.login_form = PasswordPolicyAwareAdminAuthForm + urlpatterns = [ url(r'^$', branding_views.index, name='root'), # Main marketing page, or redirect to courseware diff --git a/openedx/core/djangoapps/password_policy/__init__.py b/openedx/core/djangoapps/password_policy/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/password_policy/apps.py b/openedx/core/djangoapps/password_policy/apps.py new file mode 100644 index 0000000000..025c04ddb7 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/apps.py @@ -0,0 +1,60 @@ +""" +Configuration for password_policy Django app +""" +import logging +import six +from dateutil.parser import parse as parse_date +from django.apps import AppConfig +from django.conf import settings +from django.utils.translation import ugettext_lazy as _ + +from openedx.core.djangoapps.plugins.constants import PluginSettings, ProjectType, SettingsType + +log = logging.getLogger(__name__) + + +class PasswordPolicyConfig(AppConfig): + """ + Configuration class for password_policy Django app + """ + name = 'openedx.core.djangoapps.password_policy' + verbose_name = _("Password Policy") + + plugin_app = { + PluginSettings.CONFIG: { + ProjectType.LMS: { + SettingsType.AWS: {PluginSettings.RELATIVE_PATH: u'settings.aws'}, + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'}, + SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.devstack'}, + }, + ProjectType.CMS: { + SettingsType.AWS: {PluginSettings.RELATIVE_PATH: u'settings.aws'}, + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'}, + SettingsType.DEVSTACK: {PluginSettings.RELATIVE_PATH: u'settings.devstack'}, + } + } + } + + def ready(self): + # Convert settings from strings to datetime objects, logging any problems + self._parse_dates_safely(settings.PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG) + + def _parse_dates_safely(self, config): + """ + Convert the string dates in a config file to datetime.datetime versions, logging any issues. + """ + self._update_date_safely(config, 'STAFF_USER_COMPLIANCE_DEADLINE') + self._update_date_safely(config, 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE') + self._update_date_safely(config, 'GENERAL_USER_COMPLIANCE_DEADLINE') + + def _update_date_safely(self, config, setting): + """ + Updates a parsed datetime.datetime object for a given config setting name. + """ + deadline = config.get(setting) + try: + if isinstance(deadline, six.string_types): + config[setting] = parse_date(deadline) + except (ValueError, OverflowError): + log.exception("Could not parse %s password policy rollout value of '%s'.", setting, deadline) + config[setting] = None diff --git a/openedx/core/djangoapps/password_policy/compliance.py b/openedx/core/djangoapps/password_policy/compliance.py new file mode 100644 index 0000000000..1be5495945 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/compliance.py @@ -0,0 +1,144 @@ +""" +Utilities for enforcing and tracking compliance with password policy rules. +""" +from datetime import datetime + +import pytz +from django.conf import settings +from django.utils.translation import ugettext as _ + +from util.date_utils import DEFAULT_SHORT_DATE_FORMAT, strftime_localized +from util.password_policy_validators import validate_password + + +class NonCompliantPasswordException(Exception): + """ + Exception that should be raised when a user who is required to be compliant with password policy requirements + is found to have a non-compliant password. + """ + pass + + +class NonCompliantPasswordWarning(Exception): + """ + Exception that should be raised when a user who will soon be required to be compliant with password policy + requirements is found to have a non-compliant password. + """ + pass + + +def should_enforce_compliance_on_login(): + """ + Returns a boolean indicating whether or not password policy compliance should be enforced on login. + """ + config = _rollout_config() + return config.get('ENFORCE_COMPLIANCE_ON_LOGIN', False) + + +def _capitalize_first(s): + """ + Capitalize only the first letter and leave the rest alone. Note that normal Python capitalize() will + lowercase all other letters. This does not. + """ + return s[0].upper() + s[1:] + + +def enforce_compliance_on_login(user, password): + """ + Verify that the user's password is compliant with password policy rules and determine what should be done + if it is not. + + Raises NonCompliantPasswordException when the password is found to be non-compliant and the compliance deadline + for the user has been reached. In this case, login should be prevented. + + Raises NonCompliantPasswordWarning when the password is found to be non-compliant and the compliance deadline for + the user is in the future. + + Returns None when the password is found to be compliant, or when no deadline for compliance has been set for the + user. + + Important: This method should only be called AFTER the user has been authenticated. + """ + is_compliant = _check_user_compliance(user, password) + if is_compliant: + return + + deadline = _get_compliance_deadline_for_user(user) + if deadline is None: + return + + now = datetime.now(pytz.UTC) + if now >= deadline: + raise NonCompliantPasswordException( + _capitalize_first(_( + '{platform_name} now requires more complex passwords. Your current password does not meet the new ' + 'requirements. Change your password now to continue using the site. Thank you for helping us keep ' + 'your data safe.' + ).format( + platform_name=settings.PLATFORM_NAME + )) + ) + else: + raise NonCompliantPasswordWarning( + _capitalize_first(_( + '{platform_name} now requires more complex passwords. Your current password does not meet the new ' + 'requirements. You must change your password by {deadline} to be able to continue using the site. ' + 'Thank you for helping us keep your data safe.' + ).format( + platform_name=settings.PLATFORM_NAME, + deadline=strftime_localized(deadline, DEFAULT_SHORT_DATE_FORMAT) + )) + ) + + +def _rollout_config(): + """ + Return a dictionary with configuration settings for managing the rollout of password policy compliance + enforcement. + """ + return getattr(settings, 'PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG', {}) + + +def _check_user_compliance(user, password): + """ + Returns a boolean indicating whether or not the user is compliant with password policy rules. + """ + try: + validate_password(password, user=user, password_reset=False) + return True + except Exception: # pylint: disable=broad-except + # If anything goes wrong, we should assume the password is not compliant but we don't necessarily + # need to prevent login. + return False + + +def _get_compliance_deadline_for_user(user): + """ + Returns the date that the user will be required to comply with password policy rules, or None if no such date + applies to this user. If a deadline is not set, it will fall back to a more general deadline that is set. + """ + config = _rollout_config() + + # Implied hierarchy of general->staff in terms of scope, so we'll use each as a fallback to the other for any + # blank fields. + general_deadline = config.get('GENERAL_USER_COMPLIANCE_DEADLINE') + privilege_deadline = config.get('ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE', general_deadline) + staff_deadline = config.get('STAFF_USER_COMPLIANCE_DEADLINE', privilege_deadline) + + # Now only keep the deadlines that apply to this user + privilege_deadline = privilege_deadline if privilege_deadline and _user_has_course_access_role(user) else None + staff_deadline = staff_deadline if staff_deadline and user.is_staff else None + + # Take minimum remaining deadline + filtered_deadlines = filter(None, (staff_deadline, privilege_deadline, general_deadline,)) + return min(filtered_deadlines) if filtered_deadlines else None + + +def _user_has_course_access_role(user): + """ + Returns a boolean indicating whether or not the user is known to have at least one course access role. + """ + try: + return user.courseaccessrole_set.exists() + except Exception: # pylint: disable=broad-except + return False diff --git a/openedx/core/djangoapps/password_policy/forms.py b/openedx/core/djangoapps/password_policy/forms.py new file mode 100644 index 0000000000..379479dc05 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/forms.py @@ -0,0 +1,32 @@ +""" +Forms for the password policy app. +""" +from django.contrib import messages +from django.contrib.admin.forms import AdminAuthenticationForm +from django.forms import ValidationError + +from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance + + +class PasswordPolicyAwareAdminAuthForm(AdminAuthenticationForm): + """ + Custom AdminAuthenticationForm that can enforce password policy rules on login. + """ + + def clean(self): + """ + Overrides the clean method to allow for the enforcement of password policy requirements. + """ + cleaned_data = super(PasswordPolicyAwareAdminAuthForm, self).clean() + + if password_policy_compliance.should_enforce_compliance_on_login(): + try: + password_policy_compliance.enforce_compliance_on_login(self.user_cache, cleaned_data['password']) + except password_policy_compliance.NonCompliantPasswordWarning as e: + # Allow login, but warn the user that they will be required to reset their password soon. + messages.warning(self.request, e.message) + except password_policy_compliance.NonCompliantPasswordException as e: + # Prevent the login attempt. + raise ValidationError(e.message) + + return cleaned_data diff --git a/openedx/core/djangoapps/password_policy/settings/__init__.py b/openedx/core/djangoapps/password_policy/settings/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/password_policy/settings/aws.py b/openedx/core/djangoapps/password_policy/settings/aws.py new file mode 100644 index 0000000000..b502f11c4b --- /dev/null +++ b/openedx/core/djangoapps/password_policy/settings/aws.py @@ -0,0 +1,12 @@ +""" +Production settings for the password_policy app. +""" + + +def plugin_settings(settings): + """ + Override the default password_policy app settings with production settings. + """ + config = dict(settings.PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG) + config.update(settings.ENV_TOKENS.get('PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG', {})) + settings.PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG = config diff --git a/openedx/core/djangoapps/password_policy/settings/common.py b/openedx/core/djangoapps/password_policy/settings/common.py new file mode 100644 index 0000000000..9518aa4ca4 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/settings/common.py @@ -0,0 +1,37 @@ +""" +Default settings for the password_policy app. +""" + + +def plugin_settings(settings): + """ + Adds default settings for the password_policy app. + """ + # Settings for managing the rollout of password policy compliance enforcement. + settings.PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG = { + # Global switch to enable/disable password policy compliance enforcement on login. + 'ENFORCE_COMPLIANCE_ON_LOGIN': False, + + # The date that staff users (users with is_staff permissions) will be required to be compliant with + # current password policy requirements. After this date, non-compliant users will be forced to reset their + # password before logging in. + # + # This should be a timezone-aware date string parsable by dateutils.parser.parse + # Ex: 2018-04-19 00:00:00+00:00 + 'STAFF_USER_COMPLIANCE_DEADLINE': None, + + # The date that users with elevated privileges (users with entries in the course_access_roles table) will be + # required to be compliant with current password policy requirements. After this date, non-compliant users will + # be forced to reset their password before logging in. + # + # This should be a timezone-aware date string parsable by dateutils.parser.parse + # Ex: 2018-04-19 00:00:00+00:00 + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': None, + + # The date that all users will be required to be compliant with current password policy requirements. After + # this date, non-compliant users will be forced to reset their password before logging in. + # + # This should be a timezone-aware date string parsable by dateutils.parser.parse + # Ex: 2018-04-19 00:00:00+00:00 + 'GENERAL_USER_COMPLIANCE_DEADLINE': None, + } diff --git a/openedx/core/djangoapps/password_policy/settings/devstack.py b/openedx/core/djangoapps/password_policy/settings/devstack.py new file mode 100644 index 0000000000..d7917c79e8 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/settings/devstack.py @@ -0,0 +1,10 @@ +""" +Development settings for the password_policy app. +""" + + +def plugin_settings(settings): # pylint: disable=unused-argument + """ + Override the default password_policy app settings with development settings. + """ + pass diff --git a/openedx/core/djangoapps/password_policy/tests/__init__.py b/openedx/core/djangoapps/password_policy/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/password_policy/tests/test_apps.py b/openedx/core/djangoapps/password_policy/tests/test_apps.py new file mode 100644 index 0000000000..4fdf8252b0 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/tests/test_apps.py @@ -0,0 +1,37 @@ +""" +Test password policy settings +""" + +import datetime +from dateutil.parser import parse as parse_date +from django.conf import settings +from django.test import TestCase, override_settings +from mock import patch + +import openedx.core.djangoapps.password_policy as password_policy +from openedx.core.djangoapps.password_policy.apps import PasswordPolicyConfig + + +class TestApps(TestCase): + """ + Tests plugin config + """ + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={ + 'GENERAL_USER_COMPLIANCE_DEADLINE': '2018-01-01 00:00:00+00:00', + 'STAFF_USER_COMPLIANCE_DEADLINE': 'foo', + }) + @patch('openedx.core.djangoapps.password_policy.apps.log') + def test_settings_misconfiguration(self, mock_log): + """ + Test that we gracefully handle misconfigurations + """ + app = PasswordPolicyConfig('openedx.core.djangoapps.password_policy', password_policy) + app.ready() + config = settings.PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG + + self.assertEqual(mock_log.exception.call_count, 1) + self.assertIsNone(config['STAFF_USER_COMPLIANCE_DEADLINE']) + + self.assertIsInstance(config['GENERAL_USER_COMPLIANCE_DEADLINE'], datetime.datetime) + self.assertEqual(config['GENERAL_USER_COMPLIANCE_DEADLINE'], parse_date('2018-01-01 00:00:00+00:00')) diff --git a/openedx/core/djangoapps/password_policy/tests/test_compliance.py b/openedx/core/djangoapps/password_policy/tests/test_compliance.py new file mode 100644 index 0000000000..3a68c7c185 --- /dev/null +++ b/openedx/core/djangoapps/password_policy/tests/test_compliance.py @@ -0,0 +1,212 @@ +""" +Test password policy utilities +""" +from datetime import datetime, timedelta + +import pytz +from dateutil.parser import parse as parse_date +from django.test import TestCase, override_settings +from mock import patch + +from openedx.core.djangoapps.password_policy.compliance import (NonCompliantPasswordException, + NonCompliantPasswordWarning, + _check_user_compliance, + _get_compliance_deadline_for_user, + enforce_compliance_on_login, + should_enforce_compliance_on_login) +from student.tests.factories import (CourseAccessRoleFactory, + UserFactory) +from util.password_policy_validators import SecurityPolicyError, ValidationError, validate_password + + +date1 = parse_date('2018-01-01 00:00:00+00:00') +date2 = parse_date('2018-02-02 00:00:00+00:00') +date3 = parse_date('2018-03-03 00:00:00+00:00') + + +class TestCompliance(TestCase): + """ + Tests compliance methods for password policy + """ + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_should_enforce_compliance_on_login(self): + """ + Test that if the config is disabled or nonexistent nothing is returned + """ + # Parameters don't matter for this method as it only tests the config + self.assertTrue(should_enforce_compliance_on_login()) + + def test_enforce_compliance_on_login(self): + """ + Verify that compliance does not need to be enforced if: + * Password is compliant + * There is no compliance deadline + + Verify that compliance does need to be enforced if: + * Deadline has passed and the password is not compliant + + Verify that a warning is thrown if: + * Deadline is in the future + """ + user = UserFactory() + password = 'S0m3p@ssw0rd' # Don't actually need a password or user as methods will be mocked + + # Test password is compliant + with patch('openedx.core.djangoapps.password_policy.compliance._check_user_compliance') as \ + mock_check_user_compliance: + mock_check_user_compliance.return_value = True + self.assertIsNone(enforce_compliance_on_login(user, password)) + + # Test no deadline is set + with patch('openedx.core.djangoapps.password_policy.compliance._check_user_compliance') as \ + mock_check_user_compliance: + mock_check_user_compliance.return_value = False + with patch('openedx.core.djangoapps.password_policy.compliance._get_compliance_deadline_for_user') as \ + mock_get_compliance_deadline_for_user: + mock_get_compliance_deadline_for_user.return_value = None + self.assertIsNone(enforce_compliance_on_login(user, password)) + + # Test deadline is in the past + with patch('openedx.core.djangoapps.password_policy.compliance._check_user_compliance') as \ + mock_check_user_compliance: + mock_check_user_compliance.return_value = False + with patch('openedx.core.djangoapps.password_policy.compliance._get_compliance_deadline_for_user') as \ + mock_get_compliance_deadline_for_user: + mock_get_compliance_deadline_for_user.return_value = datetime.now(pytz.UTC) - timedelta(1) + self.assertRaises(NonCompliantPasswordException, enforce_compliance_on_login, user, password) + + # Test deadline is in the future + with patch('openedx.core.djangoapps.password_policy.compliance._check_user_compliance') as \ + mock_check_user_compliance: + mock_check_user_compliance.return_value = False + with patch('openedx.core.djangoapps.password_policy.compliance._get_compliance_deadline_for_user') as \ + mock_get_compliance_deadline_for_user: + mock_get_compliance_deadline_for_user.return_value = datetime.now(pytz.UTC) + timedelta(1) + self.assertRaises(NonCompliantPasswordWarning, enforce_compliance_on_login, user, password) + + def test_check_user_compliance(self): + """ + Test that if the config is enabled: + * Returns True if the user has a compliant password + * Returns False if the user does not have a compliant password + """ + + # Test that a user that passes validate_password returns True + with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as mock_validate_password: + user = UserFactory() + # Mock validate_password to return True without checking the password + mock_validate_password.return_value = True + self.assertTrue(_check_user_compliance(user, None)) # Don't need a password here + + # Test that a user that does not pass validate_password returns False + with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as mock_validate_password: + user = UserFactory() + # Mock validate_password to throw a ValidationError without checking the password + mock_validate_password.side_effect = ValidationError('Some validation error') + self.assertFalse(_check_user_compliance(user, None)) # Don't need a password here + + @patch('student.models.PasswordHistory.is_allowable_password_reuse') + @override_settings(ADVANCED_SECURITY_CONFIG={'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE': 1}) + def test_ignore_reset_checks(self, mock_reuse): + """ + Test that we don't annoy user about compliance failures that only affect password resets + """ + user = UserFactory() + password = 'nope1234' + mock_reuse.return_value = False + + # Sanity check that normal validation would trip us up + with self.assertRaises(SecurityPolicyError): + validate_password(password, user=user) + + # Confirm that we don't trip on it + self.assertTrue(_check_user_compliance(user, password)) + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={ + 'STAFF_USER_COMPLIANCE_DEADLINE': date1, + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date2, + 'GENERAL_USER_COMPLIANCE_DEADLINE': date3, + }) + def test_get_compliance_deadline_for_user(self): + """ + Test that the proper deadlines get returned for each user scenario + * Staff deadline returns STAFF_USER_COMPLIANCE_DEADLINE + * CourseAccessRole Users return ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE + * Everyone else gets GENERAL_USER_COMPLIANCE_DEADLINE + """ + # Staff user returned the STAFF_USER_COMPLIANCE_DEADLINE + user = UserFactory(is_staff=True) + self.assertEqual(date1, _get_compliance_deadline_for_user(user)) + + # User with CourseAccessRole returns the ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE + user = UserFactory() + CourseAccessRoleFactory.create(user=user) + self.assertEqual(date2, _get_compliance_deadline_for_user(user)) + + user = UserFactory() + self.assertEqual(date3, _get_compliance_deadline_for_user(user)) + + def test_get_compliance_deadline_for_user_fallbacks(self): + """ + Test that when some deadlines aren't specified, we cascade from general to specific. + """ + staff = UserFactory(is_staff=True) + privileged = UserFactory() + CourseAccessRoleFactory.create(user=privileged) + both = UserFactory(is_staff=True) + CourseAccessRoleFactory.create(user=both) + user = UserFactory() + + only_general = { + 'GENERAL_USER_COMPLIANCE_DEADLINE': date3 + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=only_general): + self.assertEqual(date3, _get_compliance_deadline_for_user(staff)) + self.assertEqual(date3, _get_compliance_deadline_for_user(privileged)) + self.assertEqual(date3, _get_compliance_deadline_for_user(both)) + + no_staff = { + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date2, + 'GENERAL_USER_COMPLIANCE_DEADLINE': date3 + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=no_staff): + self.assertEqual(date2, _get_compliance_deadline_for_user(both)) + self.assertEqual(date2, _get_compliance_deadline_for_user(staff)) + + no_privileged = { + 'STAFF_USER_COMPLIANCE_DEADLINE': date1, + 'GENERAL_USER_COMPLIANCE_DEADLINE': date3 + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=no_privileged): + self.assertEqual(date1, _get_compliance_deadline_for_user(both)) + self.assertEqual(date3, _get_compliance_deadline_for_user(privileged)) + + only_privileged = { + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date2, + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=only_privileged): + self.assertEqual(date2, _get_compliance_deadline_for_user(both)) + self.assertEqual(date2, _get_compliance_deadline_for_user(staff)) + self.assertIsNone(_get_compliance_deadline_for_user(user)) + + early_elevated = { + 'STAFF_USER_COMPLIANCE_DEADLINE': date2, + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date1, + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=early_elevated): + self.assertEqual(date1, _get_compliance_deadline_for_user(both)) + self.assertEqual(date2, _get_compliance_deadline_for_user(staff)) + self.assertEqual(date1, _get_compliance_deadline_for_user(privileged)) + self.assertIsNone(_get_compliance_deadline_for_user(user)) + + early_general = { + 'STAFF_USER_COMPLIANCE_DEADLINE': date3, + 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date2, + 'GENERAL_USER_COMPLIANCE_DEADLINE': date1, + } + with self.settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG=early_general): + self.assertEqual(date1, _get_compliance_deadline_for_user(both)) + self.assertEqual(date1, _get_compliance_deadline_for_user(staff)) + self.assertEqual(date1, _get_compliance_deadline_for_user(privileged)) + self.assertEqual(date1, _get_compliance_deadline_for_user(user)) diff --git a/openedx/core/djangoapps/password_policy/tests/test_forms.py b/openedx/core/djangoapps/password_policy/tests/test_forms.py new file mode 100644 index 0000000000..be9c3fd98e --- /dev/null +++ b/openedx/core/djangoapps/password_policy/tests/test_forms.py @@ -0,0 +1,78 @@ +""" +Test password policy forms +""" +import mock + +from django.forms import ValidationError +from django.test import TestCase +from django.test.utils import override_settings + +from openedx.core.djangoapps.password_policy.compliance import ( + NonCompliantPasswordException, NonCompliantPasswordWarning +) +from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm +from student.tests.factories import UserFactory + + +class PasswordPolicyAwareAdminAuthFormTests(TestCase): + """ + Tests the custom form for enforcing password policy rules + """ + def setUp(self): + super(PasswordPolicyAwareAdminAuthFormTests, self).setUp() + self.auth_form = PasswordPolicyAwareAdminAuthForm() + self.user = UserFactory.create(username='test_user', password='test_password', is_staff=True) + self.auth_form.cleaned_data = { + 'username': 'test_user', + 'password': 'test_password' + } + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': False}) + def test_auth_form_policy_disabled(self): + """ + Verify that the username and password are returned when compliance is disabled + """ + cleaned_data = self.auth_form.clean() + self.assertEqual(cleaned_data.get('username'), 'test_user') + self.assertTrue(cleaned_data.get('password'), 'test_password') + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_auth_form_policy_enabled(self): + """ + Verify that the username and password are returned when compliance is enabled + """ + with mock.patch( + 'openedx.core.djangoapps.password_policy.forms.password_policy_compliance.enforce_compliance_on_login' + ) as mock_enforce_compliance_on_login: + mock_enforce_compliance_on_login.return_value = True + cleaned_data = self.auth_form.clean() + self.assertEqual(cleaned_data.get('username'), self.user.username) + self.assertTrue(cleaned_data.get('password'), self.user.password) + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_auth_form_policy_enabled_with_warning(self): + """ + Verify that the username and password are returned when compliance is + enabled despite a NonCompliantPasswordWarning being thrown + """ + # Need to mock messages here as it will fail due to a lack of requests on this unit test + with mock.patch('openedx.core.djangoapps.password_policy.forms.messages') as mock_messages: + mock_messages.return_value = True + with mock.patch( + 'openedx.core.djangoapps.password_policy.forms.password_policy_compliance.enforce_compliance_on_login' + ) as mock_enforce_compliance_on_login: + mock_enforce_compliance_on_login.side_effect = NonCompliantPasswordWarning('Test warning') + cleaned_data = self.auth_form.clean() + self.assertEqual(cleaned_data.get('username'), self.user.username) + self.assertTrue(cleaned_data.get('password')) + + @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={'ENFORCE_COMPLIANCE_ON_LOGIN': True}) + def test_auth_form_policy_enabled_with_exception(self): + """ + Verify that an exception is raised when enforce_compliance_on_login throws a NonCompliantPasswordException + """ + with mock.patch( + 'openedx.core.djangoapps.password_policy.forms.password_policy_compliance.enforce_compliance_on_login' + ) as mock_enforce_compliance_on_login: + mock_enforce_compliance_on_login.side_effect = NonCompliantPasswordException('Test exception') + self.assertRaises(ValidationError, self.auth_form.clean) diff --git a/setup.py b/setup.py index d741230b98..3728fa5d83 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ from setuptools import setup setup( name="Open edX", - version="0.9", + version="0.10", install_requires=["setuptools"], requires=[], # NOTE: These are not the names we should be installing. This tree should @@ -75,6 +75,7 @@ setup( "bookmarks = openedx.core.djangoapps.bookmarks.apps:BookmarksConfig", "zendesk_proxy = openedx.core.djangoapps.zendesk_proxy.apps:ZendeskProxyConfig", "instructor = lms.djangoapps.instructor.apps:InstructorConfig", + "password_policy = openedx.core.djangoapps.password_policy.apps:PasswordPolicyConfig", ], "cms.djangoapp": [ "ace_common = openedx.core.djangoapps.ace_common.apps:AceCommonConfig", @@ -83,6 +84,7 @@ setup( "theming = openedx.core.djangoapps.theming.apps:ThemingConfig", "bookmarks = openedx.core.djangoapps.bookmarks.apps:BookmarksConfig", "zendesk_proxy = openedx.core.djangoapps.zendesk_proxy.apps:ZendeskProxyConfig", + "password_policy = openedx.core.djangoapps.password_policy.apps:PasswordPolicyConfig", ], } )