diff --git a/common/djangoapps/student/management/commands/manage_user.py b/common/djangoapps/student/management/commands/manage_user.py index 5932caacaf..f7cc172ac1 100644 --- a/common/djangoapps/student/management/commands/manage_user.py +++ b/common/djangoapps/student/management/commands/manage_user.py @@ -10,6 +10,7 @@ from django.core.management.base import BaseCommand, CommandError from django.db import transaction from django.utils.translation import gettext as _ +from openedx.core.djangoapps.user_api.accounts.utils import generate_password from student.models import UserProfile @@ -92,7 +93,7 @@ class Command(BaseCommand): # Set the password to a random, unknown, but usable password # allowing self-service password resetting. Cases where unusable # passwords are required, should be explicit, and will be handled below. - user.set_password(BaseUserManager().make_random_password(25)) + user.set_password(generate_password(length=25)) self.stderr.write(_('Created new user: "{}"').format(user)) else: # NOTE, we will not update the email address of an existing user. diff --git a/common/djangoapps/student/management/tests/test_manage_user.py b/common/djangoapps/student/management/tests/test_manage_user.py index cdf103ca89..7daaad7249 100644 --- a/common/djangoapps/student/management/tests/test_manage_user.py +++ b/common/djangoapps/student/management/tests/test_manage_user.py @@ -9,6 +9,8 @@ from django.contrib.auth.models import Group, User from django.core.management import call_command, CommandError from django.test import TestCase +from openedx.core.djangoapps.user_api.accounts.utils import generate_password + TEST_EMAIL = 'test@example.com' TEST_USERNAME = 'test-user' @@ -55,7 +57,7 @@ class TestManageUserCommand(TestCase): """ user = User.objects.create(username=TEST_USERNAME, email=TEST_EMAIL) self.assertEqual([(TEST_USERNAME, TEST_EMAIL)], [(u.username, u.email) for u in User.objects.all()]) - user.set_password(User.objects.make_random_password()) + user.set_password(generate_password()) user.save() # Run once without passing --unusable-password and make sure the password is usable diff --git a/common/djangoapps/student/views/login.py b/common/djangoapps/student/views/login.py index 78fda29311..d676c3add6 100644 --- a/common/djangoapps/student/views/login.py +++ b/common/djangoapps/student/views/login.py @@ -45,6 +45,7 @@ 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.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.user_api.accounts.utils import generate_password 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 @@ -585,10 +586,11 @@ def auto_auth(request): # Generate a unique name to use if none provided generated_username = uuid.uuid4().hex[0:30] + generated_password = generate_password() # Use the params from the request, otherwise use these defaults username = request.GET.get('username', generated_username) - password = request.GET.get('password', username) + password = request.GET.get('password', generated_password) email = request.GET.get('email', username + "@example.com") full_name = request.GET.get('full_name', username) is_staff = str2bool(request.GET.get('staff', False)) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 75866b7dda..d91498b9d6 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -65,6 +65,7 @@ from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers from openedx.core.djangoapps.user_api import accounts as accounts_settings +from openedx.core.djangoapps.user_api.accounts.utils import generate_password from openedx.core.djangoapps.user_api.preferences import api as preferences_api from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle from openedx.core.djangolib.markup import HTML, Text @@ -610,7 +611,7 @@ def create_account_with_params(request, params): is_third_party_auth_enabled = third_party_auth.is_enabled() if is_third_party_auth_enabled and (pipeline.running(request) or third_party_auth_credentials_in_api): - params["password"] = pipeline.make_random_password() + params["password"] = generate_password() # in case user is registering via third party (Google, Facebook) and pipeline has expired, show appropriate # error message diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 02a578414b..db510c15e8 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -61,8 +61,6 @@ import base64 import hashlib import hmac import json -import random -import string import urllib from collections import OrderedDict from logging import getLogger @@ -148,8 +146,6 @@ _AUTH_ENTRY_CHOICES = frozenset([ AUTH_ENTRY_REGISTER_API, ] + AUTH_ENTRY_CUSTOM.keys()) -_DEFAULT_RANDOM_PASSWORD_LENGTH = 12 -_PASSWORD_CHARSET = string.letters + string.digits logger = getLogger(__name__) @@ -430,27 +426,6 @@ def get_provider_user_states(user): return states -def make_random_password(length=None, choice_fn=random.SystemRandom().choice): - """Makes a random password. - - When a user creates an account via a social provider, we need to create a - placeholder password for them to satisfy the ORM's consistency and - validation requirements. Users don't know (and hence cannot sign in with) - this password; that's OK because they can always use the reset password - flow to set it to a known value. - - Args: - choice_fn: function or method. Takes an iterable and returns a random - element. - length: int. Number of chars in the returned value. None to use default. - - Returns: - String. The resulting password. - """ - length = length if length is not None else _DEFAULT_RANDOM_PASSWORD_LENGTH - return ''.join(choice_fn(_PASSWORD_CHARSET) for _ in xrange(length)) - - def running(request): """Returns True iff request is running a third-party auth pipeline.""" return get(request) is not None # Avoid False for {}. diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline.py b/common/djangoapps/third_party_auth/tests/test_pipeline.py index be128e24ed..55ea8788e6 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline.py @@ -11,30 +11,6 @@ from third_party_auth.tests import testutil # pylint: disable=protected-access -class MakeRandomPasswordTest(testutil.TestCase): - """Tests formation of random placeholder passwords.""" - - def setUp(self): - super(MakeRandomPasswordTest, self).setUp() - self.seed = 1 - - def test_default_args(self): - self.assertEqual(pipeline._DEFAULT_RANDOM_PASSWORD_LENGTH, len(pipeline.make_random_password())) - - def test_probably_only_uses_charset(self): - # This is ultimately probablistic since we could randomly select a good character 100000 consecutive times. - for char in pipeline.make_random_password(length=100000): - self.assertIn(char, pipeline._PASSWORD_CHARSET) - - def test_pseudorandomly_picks_chars_from_charset(self): - random_instance = random.Random(self.seed) - expected = ''.join( - random_instance.choice(pipeline._PASSWORD_CHARSET) - for _ in xrange(pipeline._DEFAULT_RANDOM_PASSWORD_LENGTH)) - random_instance.seed(self.seed) - self.assertEqual(expected, pipeline.make_random_password(choice_fn=random_instance.choice)) - - @unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled') class ProviderUserStateTestCase(testutil.TestCase): """Tests ProviderUserState behavior.""" diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 2788b83029..aa820641d6 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -37,7 +37,7 @@ from dashboard.git_import import GitImportError from dashboard.models import CourseImportLog from edxmako.shortcuts import render_to_response from openedx.core.djangoapps.external_auth.models import ExternalAuthMap -from openedx.core.djangoapps.external_auth.views import generate_password +from openedx.core.djangoapps.user_api.accounts.utils import generate_password from student.models import CourseEnrollment, Registration, UserProfile from student.roles import CourseInstructorRole, CourseStaffRole from xmodule.modulestore.django import modulestore diff --git a/openedx/core/djangoapps/external_auth/views.py b/openedx/core/djangoapps/external_auth/views.py index 80fd430f02..470711fbe6 100644 --- a/openedx/core/djangoapps/external_auth/views.py +++ b/openedx/core/djangoapps/external_auth/views.py @@ -38,6 +38,7 @@ from edxmako.shortcuts import render_to_response, render_to_string from openedx.core.djangoapps.external_auth.djangostore import DjangoOpenIDStore from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.core.djangoapps.site_configuration.helpers import get_value +from openedx.core.djangoapps.user_api.accounts.utils import generate_password from student.helpers import get_next_url_for_login_page from student.models import UserProfile from util.db import outer_atomic @@ -78,12 +79,6 @@ def default_render_failure(request, # pylint: disable=unused-argument # ----------------------------------------------------------------------------- -def generate_password(length=12, chars=string.letters + string.digits): - """Generate internal password for externally authenticated user""" - choice = random.SystemRandom().choice - return ''.join([choice(chars) for _i in range(length)]) - - @transaction.non_atomic_requests @csrf_exempt def openid_login_complete(request, diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py index ec8cf1f1e9..c901c865c5 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -16,7 +16,7 @@ from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from ..utils import format_social_link, validate_social_link +from ..utils import format_social_link, validate_social_link, generate_password @ddt.ddt @@ -135,3 +135,29 @@ class CompletionUtilsTestCase(SharedModuleStoreTestCase, CompletionWaffleTestMix ) ) self.assertEqual(empty_block_url, None) + + +class GeneratePasswordTest(TestCase): + """Tests formation of randomly generated passwords.""" + + def test_default_args(self): + password = generate_password() + self.assertEqual(12, len(password)) + self.assertTrue(any(c.isdigit for c in password)) + self.assertTrue(any(c.isalpha for c in password)) + + def test_length(self): + length = 25 + self.assertEqual(length, len(generate_password(length=length))) + + def test_chars(self): + char = '!' + password = generate_password(length=12, chars=(char,)) + + self.assertTrue(any(c.isdigit for c in password)) + self.assertTrue(any(c.isalpha for c in password)) + self.assertEqual(char * 10, password[2:]) + + def test_min_length(self): + with self.assertRaises(ValueError): + generate_password(length=7) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 50eef5e248..2967863861 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -1,7 +1,9 @@ """ Utility methods for the account settings. """ +import random import re +import string from urlparse import urlparse from django.conf import settings @@ -176,3 +178,17 @@ def retrieve_last_sitewide_block_completed(username): course_key=text_type(item.location.course_key), location=text_type(item.location), ) + + +def generate_password(length=12, chars=string.letters + string.digits): + """Generate a valid random password""" + if length < 8: + raise ValueError("password must be at least 8 characters") + + choice = random.SystemRandom().choice + + password = '' + password += choice(string.digits) + password += choice(string.letters) + password += ''.join([choice(chars) for _i in xrange(length - 2)]) + return password