Commit 67e15e22 by Robert Raposa Committed by GitHub

Merge pull request #13046 from edx/release

Release 2016-07-20 back to Master
parents d4f359d4 80c361f1
...@@ -19,11 +19,7 @@ from django.template import loader ...@@ -19,11 +19,7 @@ from django.template import loader
from django.conf import settings from django.conf import settings
from microsite_configuration import microsite from microsite_configuration import microsite
from student.models import CourseEnrollmentAllowed from student.models import CourseEnrollmentAllowed
from util.password_policy_validators import ( from util.password_policy_validators import validate_password_strength
validate_password_length,
validate_password_complexity,
validate_password_dictionary,
)
from openedx.core.djangoapps.theming import helpers as theming_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers
...@@ -223,9 +219,7 @@ class AccountCreationForm(forms.Form): ...@@ -223,9 +219,7 @@ class AccountCreationForm(forms.Form):
raise ValidationError(_("Username and password fields cannot match")) raise ValidationError(_("Username and password fields cannot match"))
if self.enforce_password_policy: if self.enforce_password_policy:
try: try:
validate_password_length(password) validate_password_strength(password)
validate_password_complexity(password)
validate_password_dictionary(password)
except ValidationError, err: except ValidationError, err:
raise ValidationError(_("Password: ") + "; ".join(err.messages)) raise ValidationError(_("Password: ") + "; ".join(err.messages))
return password return password
......
...@@ -59,7 +59,7 @@ class TestPasswordPolicy(TestCase): ...@@ -59,7 +59,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content) obj = json.loads(response.content)
self.assertEqual( self.assertEqual(
obj['value'], obj['value'],
"Password: Invalid Length (must be 12 characters or less)", "Password: Invalid Length (must be 12 characters or fewer)",
) )
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3}) @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3})
......
...@@ -6,8 +6,8 @@ import re ...@@ -6,8 +6,8 @@ import re
import unittest import unittest
from django.core.cache import cache from django.core.cache import cache
from django.core.urlresolvers import reverse
from django.conf import settings from django.conf import settings
from django.test import TestCase
from django.test.client import RequestFactory from django.test.client import RequestFactory
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
...@@ -29,6 +29,10 @@ from .test_microsite import fake_microsite_get_value ...@@ -29,6 +29,10 @@ from .test_microsite import fake_microsite_get_value
from openedx.core.djangoapps.theming import helpers as theming_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers
@unittest.skipUnless(
settings.ROOT_URLCONF == "lms.urls",
"reset password tests should only run in LMS"
)
@ddt.ddt @ddt.ddt
class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
""" Tests that clicking reset password sends email, and doesn't activate the user """ Tests that clicking reset password sends email, and doesn't activate the user
...@@ -50,10 +54,6 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): ...@@ -50,10 +54,6 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
self.user_bad_passwd.password = UNUSABLE_PASSWORD_PREFIX self.user_bad_passwd.password = UNUSABLE_PASSWORD_PREFIX
self.user_bad_passwd.save() self.user_bad_passwd.save()
def uidb36_to_uidb64(self, uidb36=None):
""" Converts uidb36 into uidb64 """
return force_text(urlsafe_base64_encode(force_bytes(base36_to_int(uidb36 or self.uidb36))))
@patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
def test_user_bad_password_reset(self): def test_user_bad_password_reset(self):
"""Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX""" """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX"""
...@@ -219,29 +219,37 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): ...@@ -219,29 +219,37 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
) )
self.assertEqual(from_addr, "no-reply@fakeuniversity.com") self.assertEqual(from_addr, "no-reply@fakeuniversity.com")
@patch('student.views.password_reset_confirm') @ddt.data(
def test_reset_password_bad_token(self, reset_confirm): ('invalidUid', 'invalid_token'),
(None, 'invalid_token'),
('invalidUid', None),
)
@ddt.unpack
def test_reset_password_bad_token(self, uidb36, token):
"""Tests bad token and uidb36 in password reset""" """Tests bad token and uidb36 in password reset"""
if uidb36 is None:
bad_reset_req = self.request_factory.get('/password_reset_confirm/NO-OP/') uidb36 = self.uidb36
password_reset_confirm_wrapper(bad_reset_req, 'NO', 'OP') if token is None:
confirm_kwargs = reset_confirm.call_args[1] token = self.token
self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64('NO')) bad_request = self.request_factory.get(
reverse(
self.assertEquals(confirm_kwargs['token'], 'OP') "password_reset_confirm",
kwargs={"uidb36": uidb36, "token": token}
)
)
password_reset_confirm_wrapper(bad_request, uidb36, token)
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
self.assertFalse(self.user.is_active) self.assertFalse(self.user.is_active)
@patch('student.views.password_reset_confirm') def test_reset_password_good_token(self):
def test_reset_password_good_token(self, reset_confirm):
"""Tests good token and uidb36 in password reset""" """Tests good token and uidb36 in password reset"""
url = reverse(
good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token)) "password_reset_confirm",
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token) password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
confirm_kwargs = reset_confirm.call_args[1]
self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64())
self.assertEquals(confirm_kwargs['token'], self.token)
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active) self.assertTrue(self.user.is_active)
...@@ -249,20 +257,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): ...@@ -249,20 +257,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
@patch("microsite_configuration.microsite.get_value", fake_microsite_get_value) @patch("microsite_configuration.microsite.get_value", fake_microsite_get_value)
def test_reset_password_good_token_microsite(self, reset_confirm): def test_reset_password_good_token_microsite(self, reset_confirm):
"""Tests password reset confirmation page for micro site""" """Tests password reset confirmation page for micro site"""
url = reverse(
good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token)) "password_reset_confirm",
kwargs={"uidb36": self.uidb36, "token": self.token}
)
good_reset_req = self.request_factory.get(url)
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token) password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
confirm_kwargs = reset_confirm.call_args[1] confirm_kwargs = reset_confirm.call_args[1]
self.assertEquals(confirm_kwargs['extra_context']['platform_name'], 'Fake University') self.assertEquals(confirm_kwargs['extra_context']['platform_name'], 'Fake University')
@patch('student.views.password_reset_confirm')
def test_reset_password_with_reused_password(self, reset_confirm):
"""Tests good token and uidb36 in password reset"""
good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
confirm_kwargs = reset_confirm.call_args[1]
self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64())
self.assertEquals(confirm_kwargs['token'], self.token)
self.user = User.objects.get(pk=self.user.pk) self.user = User.objects.get(pk=self.user.pk)
self.assertTrue(self.user.is_active) self.assertTrue(self.user.is_active)
...@@ -99,11 +99,7 @@ from util.milestones_helpers import ( ...@@ -99,11 +99,7 @@ from util.milestones_helpers import (
) )
from microsite_configuration import microsite from microsite_configuration import microsite
from util.password_policy_validators import ( from util.password_policy_validators import validate_password_strength
validate_password_length, validate_password_complexity,
validate_password_dictionary
)
import third_party_auth import third_party_auth
from third_party_auth import pipeline, provider from third_party_auth import pipeline, provider
from student.helpers import ( from student.helpers import (
...@@ -2125,35 +2121,43 @@ def password_reset(request): ...@@ -2125,35 +2121,43 @@ def password_reset(request):
}) })
def password_reset_confirm_wrapper( def uidb36_to_uidb64(uidb36):
request, """
uidb36=None, Needed to support old password reset URLs that use base36-encoded user IDs
token=None, https://github.com/django/django/commit/1184d077893ff1bc947e45b00a4d565f3df81776#diff-c571286052438b2e3190f8db8331a92bR231
): Args:
""" A wrapper around django.contrib.auth.views.password_reset_confirm. uidb36: base36-encoded user ID
Needed because we want to set the user as active at this step.
Returns: base64-encoded user ID. Otherwise returns a dummy, invalid ID
""" """
# cribbed from django.contrib.auth.views.password_reset_confirm
try: try:
uid_int = base36_to_int(uidb36) uidb64 = force_text(urlsafe_base64_encode(force_bytes(base36_to_int(uidb36))))
user = User.objects.get(id=uid_int) except ValueError:
user.is_active = True uidb64 = '1' # dummy invalid ID (incorrect padding for base64)
user.save() return uidb64
except (ValueError, User.DoesNotExist):
pass
def validate_password(user, password):
"""
Tie in password policy enforcement as an optional level of
security protection
Args:
user: the user object whose password we're checking.
password: the user's proposed new password.
# tie in password strength enforcement as an optional level of Returns:
# security protection is_valid_password: a boolean indicating if the new password
passes the validation.
err_msg: an error message if there's a violation of one of the password
checks. Otherwise, `None`.
"""
err_msg = None err_msg = None
if request.method == 'POST':
password = request.POST['new_password1']
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False): if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try: try:
validate_password_length(password) validate_password_strength(password)
validate_password_complexity(password) except ValidationError as err:
validate_password_dictionary(password)
except ValidationError, err:
err_msg = _('Password: ') + '; '.join(err.messages) err_msg = _('Password: ') + '; '.join(err.messages)
# also, check the password reuse policy # also, check the password reuse policy
...@@ -2181,34 +2185,55 @@ def password_reset_confirm_wrapper( ...@@ -2181,34 +2185,55 @@ def password_reset_confirm_wrapper(
num_days num_days
).format(num=num_days) ).format(num=num_days)
if err_msg: is_password_valid = err_msg is None
# We have an password reset attempt which violates some security policy, use the
# existing Django template to communicate this back to the user return is_password_valid, err_msg
def password_reset_confirm_wrapper(request, uidb36=None, token=None):
"""
A wrapper around django.contrib.auth.views.password_reset_confirm.
Needed because we want to set the user as active at this step.
We also optionally do some additional password policy checks.
"""
# convert old-style base36-encoded user id to base64
uidb64 = uidb36_to_uidb64(uidb36)
platform_name = {
"platform_name": microsite.get_value('platform_name', settings.PLATFORM_NAME)
}
try:
uid_int = base36_to_int(uidb36)
user = User.objects.get(id=uid_int)
except (ValueError, User.DoesNotExist):
# if there's any error getting a user, just let django's
# password_reset_confirm function handle it.
return password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=platform_name
)
if request.method == 'POST':
password = request.POST['new_password1']
is_password_valid, password_err_msg = validate_password(user, password)
if not is_password_valid:
# We have a password reset attempt which violates some security
# policy. Use the existing Django template to communicate that
# back to the user.
context = { context = {
'validlink': True, 'validlink': False,
'form': None, 'form': None,
'title': _('Password reset unsuccessful'), 'title': _('Password reset unsuccessful'),
'err_msg': err_msg, 'err_msg': password_err_msg,
'platform_name': microsite.get_value('platform_name', settings.PLATFORM_NAME),
} }
return TemplateResponse(request, 'registration/password_reset_confirm.html', context) context.update(platform_name)
else: return TemplateResponse(
# we also want to pass settings.PLATFORM_NAME in as extra_context request, 'registration/password_reset_confirm.html', context
extra_context = {"platform_name": microsite.get_value('platform_name', settings.PLATFORM_NAME)} )
# Support old password reset URLs that used base36 encoded user IDs.
# https://github.com/django/django/commit/1184d077893ff1bc947e45b00a4d565f3df81776#diff-c571286052438b2e3190f8db8331a92bR231
try:
uidb64 = force_text(urlsafe_base64_encode(force_bytes(base36_to_int(uidb36))))
except ValueError:
uidb64 = '1' # dummy invalid ID (incorrect padding for base64)
if request.method == 'POST':
# remember what the old password hash is before we call down # remember what the old password hash is before we call down
old_password_hash = user.password old_password_hash = user.password
result = password_reset_confirm( response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=extra_context request, uidb64=uidb64, token=token, extra_context=platform_name
) )
# get the updated user # get the updated user
...@@ -2219,12 +2244,18 @@ def password_reset_confirm_wrapper( ...@@ -2219,12 +2244,18 @@ def password_reset_confirm_wrapper(
entry = PasswordHistory() entry = PasswordHistory()
entry.create(updated_user) entry.create(updated_user)
return result
else: else:
return password_reset_confirm( response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=extra_context request, uidb64=uidb64, token=token, extra_context=platform_name
) )
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not user.is_active:
user.is_active = True
user.save()
return response
def reactivation_email_for_user(user): def reactivation_email_for_user(user):
try: try:
......
...@@ -15,6 +15,26 @@ from django.conf import settings ...@@ -15,6 +15,26 @@ from django.conf import settings
import nltk import nltk
def validate_password_strength(value):
"""
This function loops through each validator defined in this file
and applies it to a user's proposed password
Args:
value: a user's proposed password
Returns: None, but raises a ValidationError if the proposed password
fails any one of the validators in password_validators
"""
password_validators = [
validate_password_length,
validate_password_complexity,
validate_password_dictionary,
]
for validator in password_validators:
validator(value)
def validate_password_length(value): def validate_password_length(value):
""" """
Validator that enforces minimum length of a password Validator that enforces minimum length of a password
...@@ -28,7 +48,7 @@ def validate_password_length(value): ...@@ -28,7 +48,7 @@ def validate_password_length(value):
if min_length and len(value) < min_length: if min_length and len(value) < min_length:
raise ValidationError(message.format(_("must be {0} characters or more").format(min_length)), code=code) raise ValidationError(message.format(_("must be {0} characters or more").format(min_length)), code=code)
elif max_length and len(value) > max_length: elif max_length and len(value) > max_length:
raise ValidationError(message.format(_("must be {0} characters or less").format(max_length)), code=code) raise ValidationError(message.format(_("must be {0} characters or fewer").format(max_length)), code=code)
def validate_password_complexity(value): def validate_password_complexity(value):
......
...@@ -71,6 +71,18 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -71,6 +71,18 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
history = PasswordHistory() history = PasswordHistory()
history.create(user) history.create(user)
def assertPasswordResetError(self, response, error_message):
"""
This method is a custom assertion that verifies that a password reset
view returns an error response as expected.
Args:
response: response from calling a password reset endpoint
error_message: message we expect to see in the response
"""
self.assertFalse(response.context_data['validlink'])
self.assertIn(error_message, response.content)
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS': None}) @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS': None})
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': None}) @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': None})
def test_no_forced_password_change(self): def test_no_forced_password_change(self):
...@@ -168,10 +180,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -168,10 +180,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo' 'new_password2': 'foo'
}, follow=True) }, follow=True)
self.assertIn( self.assertPasswordResetError(resp, err_msg)
err_msg,
resp.content
)
# now retry with a different password # now retry with a different password
resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), {
...@@ -179,10 +188,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -179,10 +188,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'bar' 'new_password2': 'bar'
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2}) @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2})
def test_staff_password_reset_reuse(self): def test_staff_password_reset_reuse(self):
...@@ -204,10 +210,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -204,10 +210,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo', 'new_password2': 'foo',
}, follow=True) }, follow=True)
self.assertIn( self.assertPasswordResetError(resp, err_msg)
err_msg,
resp.content
)
# now use different one # now use different one
user = User.objects.get(email=staff_email) user = User.objects.get(email=staff_email)
...@@ -219,10 +222,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -219,10 +222,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'bar', 'new_password2': 'bar',
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
# now try again with the first one # now try again with the first one
user = User.objects.get(email=staff_email) user = User.objects.get(email=staff_email)
...@@ -234,11 +234,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -234,11 +234,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo', 'new_password2': 'foo',
}, follow=True) }, follow=True)
# should be rejected self.assertPasswordResetError(resp, err_msg)
self.assertIn(
err_msg,
resp.content
)
# now use different one # now use different one
user = User.objects.get(email=staff_email) user = User.objects.get(email=staff_email)
...@@ -250,10 +246,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -250,10 +246,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'baz', 'new_password2': 'baz',
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
# now we should be able to reuse the first one # now we should be able to reuse the first one
user = User.objects.get(email=staff_email) user = User.objects.get(email=staff_email)
...@@ -265,10 +258,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -265,10 +258,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo', 'new_password2': 'foo',
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1}) @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1})
def test_password_reset_frequency_limit(self): def test_password_reset_frequency_limit(self):
...@@ -308,10 +298,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -308,10 +298,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo', 'new_password2': 'foo',
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
@patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True}) @patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True})
@override_settings(PASSWORD_MIN_LENGTH=6) @override_settings(PASSWORD_MIN_LENGTH=6)
...@@ -350,7 +337,4 @@ class TestPasswordHistory(LoginEnrollmentTestCase): ...@@ -350,7 +337,4 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foofoo', 'new_password2': 'foofoo',
}, follow=True) }, follow=True)
self.assertIn( self.assertIn(success_msg, resp.content)
success_msg,
resp.content
)
"""
Django admin bindings for catalog support models.
"""
from django.contrib import admin
from config_models.admin import ConfigurationModelAdmin
from openedx.core.djangoapps.catalog.models import CatalogIntegration
admin.site.register(CatalogIntegration, ConfigurationModelAdmin)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment