Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #13041 from edx/robrap/activate-fix

Browse Source 
activate fix
This commit is contained in:
Robert Raposa
2016-07-20 13:22:49 -04:00
committed by GitHub
parent 5ba8cf8324 95c0b50ebe
commit 6e04ddcc4d
6 changed files with 200 additions and 170 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
common/djangoapps/student/forms.py
View File

@@ -19,11 +19,7 @@ from django.template import loader
from django.conf import settings
from microsite_configuration import microsite
from student.models import CourseEnrollmentAllowed
from util.password_policy_validators import (
validate_password_length,
validate_password_complexity,
validate_password_dictionary,
)
from util.password_policy_validators import validate_password_strength
from openedx.core.djangoapps.theming import helpers as theming_helpers
@@ -223,9 +219,7 @@ class AccountCreationForm(forms.Form):
raise ValidationError(_("Username and password fields cannot match"))
if self.enforce_password_policy:
try:
validate_password_length(password)
validate_password_complexity(password)
validate_password_dictionary(password)
validate_password_strength(password)
except ValidationError, err:
raise ValidationError(_("Password: ") + "; ".join(err.messages))
return password

2
common/djangoapps/student/tests/test_password_policy.py
View File

@@ -59,7 +59,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
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})

67
common/djangoapps/student/tests/test_reset_password.py
View File

@@ -6,8 +6,8 @@ import re
import unittest
from django.core.cache import cache
from django.core.urlresolvers import reverse
from django.conf import settings
from django.test import TestCase
from django.test.client import RequestFactory
from django.contrib.auth.models import User
from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
@@ -29,6 +29,10 @@ from .test_microsite import fake_microsite_get_value
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
class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
""" Tests that clicking reset password sends email, and doesn't activate the user
@@ -50,10 +54,6 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
self.user_bad_passwd.password = UNUSABLE_PASSWORD_PREFIX
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))
def test_user_bad_password_reset(self):
"""Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX"""
@@ -219,29 +219,37 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
)
self.assertEqual(from_addr, "no-reply@fakeuniversity.com")
@patch('student.views.password_reset_confirm')
def test_reset_password_bad_token(self, reset_confirm):
@ddt.data(
('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"""
if uidb36 is None:
uidb36 = self.uidb36
if token is None:
token = self.token
bad_reset_req = self.request_factory.get('/password_reset_confirm/NO-OP/')
password_reset_confirm_wrapper(bad_reset_req, 'NO', 'OP')
confirm_kwargs = reset_confirm.call_args[1]
self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64('NO'))
self.assertEquals(confirm_kwargs['token'], 'OP')
bad_request = self.request_factory.get(
reverse(
"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.assertFalse(self.user.is_active)
@patch('student.views.password_reset_confirm')
def test_reset_password_good_token(self, reset_confirm):
def test_reset_password_good_token(self):
"""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))
url = reverse(
"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)
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.assertTrue(self.user.is_active)
@@ -249,20 +257,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
@patch("microsite_configuration.microsite.get_value", fake_microsite_get_value)
def test_reset_password_good_token_microsite(self, reset_confirm):
"""Tests password reset confirmation page for micro site"""
good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
url = reverse(
"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)
confirm_kwargs = reset_confirm.call_args[1]
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.assertTrue(self.user.is_active)

211
common/djangoapps/student/views.py
View File

@@ -99,11 +99,7 @@ from util.milestones_helpers import (
)
from microsite_configuration import microsite
from util.password_policy_validators import (
validate_password_length, validate_password_complexity,
validate_password_dictionary
)
from util.password_policy_validators import validate_password_strength
import third_party_auth
from third_party_auth import pipeline, provider
from student.helpers import (
@@ -2125,105 +2121,140 @@ def password_reset(request):
})
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.
def uidb36_to_uidb64(uidb36):
"""
# cribbed from django.contrib.auth.views.password_reset_confirm
Needed to support old password reset URLs that use base36-encoded user IDs
https://github.com/django/django/commit/1184d077893ff1bc947e45b00a4d565f3df81776#diff-c571286052438b2e3190f8db8331a92bR231
Args:
uidb36: base36-encoded user ID
Returns: base64-encoded user ID. Otherwise returns a dummy, invalid ID
"""
try:
uidb64 = force_text(urlsafe_base64_encode(force_bytes(base36_to_int(uidb36))))
except ValueError:
uidb64 = '1' # dummy invalid ID (incorrect padding for base64)
return uidb64
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.
Returns:
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
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try:
validate_password_strength(password)
except ValidationError as err:
err_msg = _('Password: ') + '; '.join(err.messages)
# also, check the password reuse policy
if not PasswordHistory.is_allowable_password_reuse(user, password):
if user.is_staff:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
else:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
# Because of how ngettext is, splitting the following into shorter lines would be ugly.
# pylint: disable=line-too-long
err_msg = ungettext(
"You are re-using a password that you have used recently. You must have {num} distinct password before reusing a previous password.",
"You are re-using a password that you have used recently. You must have {num} distinct passwords before reusing a previous password.",
num_distinct
).format(num=num_distinct)
# also, check to see if passwords are getting reset too frequent
if PasswordHistory.is_password_reset_too_soon(user):
num_days = settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
# Because of how ngettext is, splitting the following into shorter lines would be ugly.
# pylint: disable=line-too-long
err_msg = ungettext(
"You are resetting passwords too frequently. Due to security policies, {num} day must elapse between password resets.",
"You are resetting passwords too frequently. Due to security policies, {num} days must elapse between password resets.",
num_days
).format(num=num_days)
is_password_valid = err_msg is None
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)
user.is_active = True
user.save()
except (ValueError, User.DoesNotExist):
pass
# tie in password strength enforcement as an optional level of
# security protection
err_msg = None
# 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']
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try:
validate_password_length(password)
validate_password_complexity(password)
validate_password_dictionary(password)
except ValidationError, err:
err_msg = _('Password: ') + '; '.join(err.messages)
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 = {
'validlink': False,
'form': None,
'title': _('Password reset unsuccessful'),
'err_msg': password_err_msg,
}
context.update(platform_name)
return TemplateResponse(
request, 'registration/password_reset_confirm.html', context
)
# also, check the password reuse policy
if not PasswordHistory.is_allowable_password_reuse(user, password):
if user.is_staff:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
else:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
# Because of how ngettext is, splitting the following into shorter lines would be ugly.
# pylint: disable=line-too-long
err_msg = ungettext(
"You are re-using a password that you have used recently. You must have {num} distinct password before reusing a previous password.",
"You are re-using a password that you have used recently. You must have {num} distinct passwords before reusing a previous password.",
num_distinct
).format(num=num_distinct)
# remember what the old password hash is before we call down
old_password_hash = user.password
# also, check to see if passwords are getting reset too frequent
if PasswordHistory.is_password_reset_too_soon(user):
num_days = settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
# Because of how ngettext is, splitting the following into shorter lines would be ugly.
# pylint: disable=line-too-long
err_msg = ungettext(
"You are resetting passwords too frequently. Due to security policies, {num} day must elapse between password resets.",
"You are resetting passwords too frequently. Due to security policies, {num} days must elapse between password resets.",
num_days
).format(num=num_days)
response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=platform_name
)
# get the updated user
updated_user = User.objects.get(id=uid_int)
# did the password hash change, if so record it in the PasswordHistory
if updated_user.password != old_password_hash:
entry = PasswordHistory()
entry.create(updated_user)
if err_msg:
# We have an password reset attempt which violates some security policy, use the
# existing Django template to communicate this back to the user
context = {
'validlink': True,
'form': None,
'title': _('Password reset unsuccessful'),
'err_msg': err_msg,
'platform_name': microsite.get_value('platform_name', settings.PLATFORM_NAME),
}
return TemplateResponse(request, 'registration/password_reset_confirm.html', context)
else:
# we also want to pass settings.PLATFORM_NAME in as extra_context
extra_context = {"platform_name": microsite.get_value('platform_name', settings.PLATFORM_NAME)}
response = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=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)
response_was_successful = response.context_data.get('validlink')
if response_was_successful and not user.is_active:
user.is_active = True
user.save()
if request.method == 'POST':
# remember what the old password hash is before we call down
old_password_hash = user.password
result = password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=extra_context
)
# get the updated user
updated_user = User.objects.get(id=uid_int)
# did the password hash change, if so record it in the PasswordHistory
if updated_user.password != old_password_hash:
entry = PasswordHistory()
entry.create(updated_user)
return result
else:
return password_reset_confirm(
request, uidb64=uidb64, token=token, extra_context=extra_context
)
return response
def reactivation_email_for_user(user):

22
common/djangoapps/util/password_policy_validators.py
View File

@@ -15,6 +15,26 @@ from django.conf import settings
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):
"""
Validator that enforces minimum length of a password
@@ -28,7 +48,7 @@ def validate_password_length(value):
if min_length and len(value) < min_length:
raise ValidationError(message.format(_("must be {0} characters or more").format(min_length)), code=code)
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):

58
lms/djangoapps/courseware/tests/test_password_history.py
View File

@@ -71,6 +71,18 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
history = PasswordHistory()
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_STUDENT_ACCOUNTS_PASSWORD_RESETS': None})
def test_no_forced_password_change(self):
@@ -168,10 +180,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo'
}, follow=True)
self.assertIn(
err_msg,
resp.content
)
self.assertPasswordResetError(resp, err_msg)
# now retry with a different password
resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), {
@@ -179,10 +188,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'bar'
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2})
def test_staff_password_reset_reuse(self):
@@ -204,10 +210,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo',
}, follow=True)
self.assertIn(
err_msg,
resp.content
)
self.assertPasswordResetError(resp, err_msg)
# now use different one
user = User.objects.get(email=staff_email)
@@ -219,10 +222,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'bar',
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)
# now try again with the first one
user = User.objects.get(email=staff_email)
@@ -234,11 +234,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo',
}, follow=True)
# should be rejected
self.assertIn(
err_msg,
resp.content
)
self.assertPasswordResetError(resp, err_msg)
# now use different one
user = User.objects.get(email=staff_email)
@@ -250,10 +246,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'baz',
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)
# now we should be able to reuse the first one
user = User.objects.get(email=staff_email)
@@ -265,10 +258,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo',
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)
@patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1})
def test_password_reset_frequency_limit(self):
@@ -308,10 +298,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foo',
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)
@patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True})
@override_settings(PASSWORD_MIN_LENGTH=6)
@@ -350,7 +337,4 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
'new_password2': 'foofoo',
}, follow=True)
self.assertIn(
success_msg,
resp.content
)
self.assertIn(success_msg, resp.content)