diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index e2bb848d2c..d8136aca0b 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -6,7 +6,7 @@ consist primarily of authentication, request validation, and serialization. from ipware.ip import get_ip from django.utils.decorators import method_decorator from opaque_keys import InvalidKeyError -from openedx.core.djangoapps.user_api import api as user_api +from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.permissions import ApiKeyHeaderPermission, ApiKeyHeaderPermissionIsAuthenticated from rest_framework import status from rest_framework.response import Response @@ -349,7 +349,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): email_opt_in = request.DATA.get('email_opt_in', None) if email_opt_in is not None: org = course_id.org - user_api.profile.update_email_opt_in(request.user, org, email_opt_in) + update_email_opt_in(request.user, org, email_opt_in) return Response(response) except CourseModeNotFoundError as error: return Response( diff --git a/common/djangoapps/lang_pref/middleware.py b/common/djangoapps/lang_pref/middleware.py index da3a64ddef..a7df9803c4 100644 --- a/common/djangoapps/lang_pref/middleware.py +++ b/common/djangoapps/lang_pref/middleware.py @@ -2,7 +2,7 @@ Middleware for Language Preferences """ -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from lang_pref import LANGUAGE_KEY @@ -20,6 +20,6 @@ class LanguagePreferenceMiddleware(object): no language set on the session (i.e. from dark language overrides), use the user's preference. """ if request.user.is_authenticated() and 'django_language' not in request.session: - user_pref = UserPreference.get_preference(request.user, LANGUAGE_KEY) + user_pref = get_user_preference(request.user, LANGUAGE_KEY) if user_pref: request.session['django_language'] = user_pref diff --git a/common/djangoapps/lang_pref/tests/test_middleware.py b/common/djangoapps/lang_pref/tests/test_middleware.py index 042e86a712..b2bd54cb74 100644 --- a/common/djangoapps/lang_pref/tests/test_middleware.py +++ b/common/djangoapps/lang_pref/tests/test_middleware.py @@ -3,7 +3,7 @@ from django.test.client import RequestFactory from django.contrib.sessions.middleware import SessionMiddleware from lang_pref.middleware import LanguagePreferenceMiddleware -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from lang_pref import LANGUAGE_KEY from student.tests.factories import UserFactory @@ -28,7 +28,7 @@ class TestUserPreferenceMiddleware(TestCase): def test_language_in_user_prefs(self): # language set in the user preferences and not the session - UserPreference.set_preference(self.user, LANGUAGE_KEY, 'eo') + set_user_preference(self.user, LANGUAGE_KEY, 'eo') self.middleware.process_request(self.request) self.assertEquals(self.request.session['django_language'], 'eo') @@ -36,7 +36,7 @@ class TestUserPreferenceMiddleware(TestCase): # language set in both the user preferences and session, # session should get precedence self.request.session['django_language'] = 'en' - UserPreference.set_preference(self.user, LANGUAGE_KEY, 'eo') + set_user_preference(self.user, LANGUAGE_KEY, 'eo') self.middleware.process_request(self.request) self.assertEquals(self.request.session['django_language'], 'en') diff --git a/common/djangoapps/lang_pref/tests/test_views.py b/common/djangoapps/lang_pref/tests/test_views.py index daa8e89127..39a164af23 100644 --- a/common/djangoapps/lang_pref/tests/test_views.py +++ b/common/djangoapps/lang_pref/tests/test_views.py @@ -4,7 +4,7 @@ Tests for the language setting view from django.core.urlresolvers import reverse from django.test import TestCase from student.tests.factories import UserFactory -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from lang_pref import LANGUAGE_KEY @@ -20,7 +20,7 @@ class TestLanguageSetting(TestCase): response = self.client.post(reverse('lang_pref_set_language'), {'language': lang}) self.assertEquals(response.status_code, 200) - user_pref = UserPreference.get_preference(user, LANGUAGE_KEY) + user_pref = get_user_preference(user, LANGUAGE_KEY) self.assertEqual(user_pref, lang) def test_set_preference_missing_lang(self): @@ -31,4 +31,4 @@ class TestLanguageSetting(TestCase): self.assertEquals(response.status_code, 400) - self.assertIsNone(UserPreference.get_preference(user, LANGUAGE_KEY)) + self.assertIsNone(get_user_preference(user, LANGUAGE_KEY)) diff --git a/common/djangoapps/lang_pref/views.py b/common/djangoapps/lang_pref/views.py index df75166392..02b94cc363 100644 --- a/common/djangoapps/lang_pref/views.py +++ b/common/djangoapps/lang_pref/views.py @@ -4,7 +4,7 @@ Views for accessing language preferences from django.contrib.auth.decorators import login_required from django.http import HttpResponse, HttpResponseBadRequest -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from lang_pref import LANGUAGE_KEY @@ -13,11 +13,10 @@ def set_language(request): """ This view is called when the user would like to set a language preference """ - user = request.user lang_pref = request.POST.get('language', None) if lang_pref: - UserPreference.set_preference(user, LANGUAGE_KEY, lang_pref) + set_user_preference(request.user, LANGUAGE_KEY, lang_pref) return HttpResponse('{"success": true}') return HttpResponseBadRequest('no language provided') diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index 23d3750202..f07dc57860 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -40,7 +40,7 @@ class UserProfileFactory(DjangoModelFactory): level_of_education = None gender = u'm' mailing_address = None - goals = u'World domination' + goals = u'Learn a lot' class CourseModeFactory(DjangoModelFactory): diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index 3a9a5b0180..88f574bd74 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -1,4 +1,4 @@ -"Tests for account creation" +"""Tests for account creation""" import json import ddt @@ -14,7 +14,7 @@ from django.test.utils import override_settings import mock -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from lang_pref import LANGUAGE_KEY from notification_prefs import NOTIFICATION_PREF_KEY @@ -42,7 +42,7 @@ TEST_CS_URL = 'https://comments.service.test:123/' } ) class TestCreateAccount(TestCase): - "Tests for account creation" + """Tests for account creation""" def setUp(self): self.username = "test_user" @@ -63,14 +63,14 @@ class TestCreateAccount(TestCase): response = self.client.post(self.url, self.params) self.assertEqual(response.status_code, 200) user = User.objects.get(username=self.username) - self.assertEqual(UserPreference.get_preference(user, LANGUAGE_KEY), lang) + self.assertEqual(get_user_preference(user, LANGUAGE_KEY), lang) @ddt.data("en", "eo") def test_header_lang_pref_saved(self, lang): response = self.client.post(self.url, self.params, HTTP_ACCEPT_LANGUAGE=lang) user = User.objects.get(username=self.username) self.assertEqual(response.status_code, 200) - self.assertEqual(UserPreference.get_preference(user, LANGUAGE_KEY), lang) + self.assertEqual(get_user_preference(user, LANGUAGE_KEY), lang) def create_account_and_fetch_profile(self): """ @@ -225,7 +225,7 @@ class TestCreateAccount(TestCase): response = self.client.post(self.url, self.params) self.assertEqual(response.status_code, 200) user = User.objects.get(username=self.username) - preference = UserPreference.get_preference(user, NOTIFICATION_PREF_KEY) + preference = get_user_preference(user, NOTIFICATION_PREF_KEY) if digest_enabled: self.assertIsNotNone(preference) else: diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 3c784446ab..6cf29170fc 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -103,7 +103,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase): self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) @patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True}) - @patch('openedx.core.djangoapps.user_api.api.profile.update_email_opt_in') + @patch('openedx.core.djangoapps.user_api.preferences.api.update_email_opt_in') @ddt.data( ([], 'true'), ([], 'false'), diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 4521d9fb00..ec244effce 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -85,7 +85,6 @@ from external_auth.login_and_register import ( from bulk_email.models import Optout, CourseAuthorization import shoppingcart from lang_pref import LANGUAGE_KEY -from notification_prefs.views import enable_notifications import track.views @@ -118,6 +117,12 @@ from embargo import api as embargo_api import analytics from eventtracking import tracker +# Note that this lives in LMS, so this dependency should be refactored. +from notification_prefs.views import enable_notifications + +# Note that this lives in openedx, so this dependency should be refactored. +from openedx.core.djangoapps.user_api.preferences import api as preferences_api + log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") @@ -632,20 +637,17 @@ def dashboard(request): # Re-alphabetize language options language_options.sort() - # TODO: remove circular dependency on openedx from common - from openedx.core.djangoapps.user_api.models import UserPreference - - # try to get the prefered language for the user - cur_pref_lang_code = UserPreference.get_preference(request.user, LANGUAGE_KEY) + # try to get the preferred language for the user + preferred_language_code = preferences_api.get_user_preference(request.user, LANGUAGE_KEY) # try and get the current language of the user - cur_lang_code = get_language() - if cur_pref_lang_code and cur_pref_lang_code in settings.LANGUAGE_DICT: + current_language_code = get_language() + if preferred_language_code and preferred_language_code in settings.LANGUAGE_DICT: # if the user has a preference, get the name from the code - current_language = settings.LANGUAGE_DICT[cur_pref_lang_code] - elif cur_lang_code in settings.LANGUAGE_DICT: + current_language = settings.LANGUAGE_DICT[preferred_language_code] + elif current_language_code in settings.LANGUAGE_DICT: # if the user's browser is showing a particular language, # use that as the current language - current_language = settings.LANGUAGE_DICT[cur_lang_code] + current_language = settings.LANGUAGE_DICT[current_language_code] else: # otherwise, use the default language current_language = settings.LANGUAGE_DICT[settings.LANGUAGE_CODE] @@ -680,7 +682,7 @@ def dashboard(request): 'billing_email': settings.PAYMENT_SUPPORT_EMAIL, 'language_options': language_options, 'current_language': current_language, - 'current_language_code': cur_lang_code, + 'current_language_code': current_language_code, 'user': user, 'duplicate_provider': None, 'logout_url': reverse(logout_user), @@ -800,13 +802,10 @@ def try_change_enrollment(request): def _update_email_opt_in(request, org): """Helper function used to hit the profile API if email opt-in is enabled.""" - # TODO: remove circular dependency on openedx from common - from openedx.core.djangoapps.user_api.api import profile as profile_api - email_opt_in = request.POST.get('email_opt_in') if email_opt_in is not None: email_opt_in_boolean = email_opt_in == 'true' - profile_api.update_email_opt_in(request.user, org, email_opt_in_boolean) + preferences_api.update_email_opt_in(request.user, org, email_opt_in_boolean) @require_POST @@ -1391,10 +1390,7 @@ def _do_create_account(form): log.exception("UserProfile creation failed for user {id}.".format(id=user.id)) raise - # TODO: remove circular dependency on openedx from common - from openedx.core.djangoapps.user_api.models import UserPreference - - UserPreference.set_preference(user, LANGUAGE_KEY, get_language()) + preferences_api.set_user_preference(user, LANGUAGE_KEY, get_language()) return (user, profile, registration) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 97c905fac0..368e4dba26 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -89,6 +89,9 @@ from logging import getLogger from . import provider +# Note that this lives in openedx, so this dependency should be refactored. +from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in + # These are the query string params you can pass # to the URL that starts the authentication process. @@ -669,10 +672,8 @@ def change_enrollment(strategy, user=None, is_dashboard=False, *args, **kwargs): # If the email opt in parameter is found, set the preference. email_opt_in = strategy.session_get(AUTH_EMAIL_OPT_IN_KEY) if email_opt_in: - # TODO: remove circular dependency on openedx from common - from openedx.core.djangoapps.user_api.api import profile opt_in = email_opt_in.lower() == 'true' - profile.update_email_opt_in(user, course_id.org, opt_in) + update_email_opt_in(user, course_id.org, opt_in) # Check whether we're blocked from enrolling by a # country access rule. diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 04d1cbd3df..f85d5bc139 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -79,7 +79,9 @@ def get_user_email_language(user): Return the language most appropriate for writing emails to user. Returns None if the preference has not been set, or if the user does not exist. """ - return UserPreference.get_preference(user, LANGUAGE_KEY) + # Calling UserPreference directly instead of get_user_preference because the user requesting the + # information is not "user" and also may not have is_staff access. + return UserPreference.get_value(user, LANGUAGE_KEY) def enroll_email(course_id, student_email, auto_enroll=False, email_students=False, email_params=None, language=None): diff --git a/lms/djangoapps/instructor/tests/test_api_email_localization.py b/lms/djangoapps/instructor/tests/test_api_email_localization.py index bf4db954a8..b78c71d3ce 100644 --- a/lms/djangoapps/instructor/tests/test_api_email_localization.py +++ b/lms/djangoapps/instructor/tests/test_api_email_localization.py @@ -10,7 +10,7 @@ from courseware.tests.factories import InstructorFactory from lang_pref import LANGUAGE_KEY from student.models import CourseEnrollment from student.tests.factories import UserFactory -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -29,11 +29,11 @@ class TestInstructorAPIEnrollmentEmailLocalization(ModuleStoreTestCase): # French. self.course = CourseFactory.create() self.instructor = InstructorFactory(course_key=self.course.id) - UserPreference.set_preference(self.instructor, LANGUAGE_KEY, 'zh-cn') + set_user_preference(self.instructor, LANGUAGE_KEY, 'zh-cn') self.client.login(username=self.instructor.username, password='test') self.student = UserFactory.create() - UserPreference.set_preference(self.student, LANGUAGE_KEY, 'fr') + set_user_preference(self.student, LANGUAGE_KEY, 'fr') def update_enrollement(self, action, student_email): """ diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ec3d2edf27..0badb2251d 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -79,7 +79,7 @@ import instructor_analytics.basic import instructor_analytics.distributions import instructor_analytics.csvs import csv -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, set_user_preference from instructor.views import INVOICE_KEY from submissions import api as sub_api # installed from the edx-submissions repository @@ -1238,7 +1238,7 @@ def generate_registration_codes(request, course_id): invoice_copy = True sale_price = unit_price * course_code_number - UserPreference.set_preference(request.user, INVOICE_KEY, invoice_copy) + set_user_preference(request.user, INVOICE_KEY, invoice_copy) sale_invoice = Invoice.objects.create( total_amount=sale_price, company_name=company_name, @@ -2187,8 +2187,9 @@ def get_user_invoice_preference(request, course_id): # pylint: disable=unused-a Gets invoice copy user's preferences. """ invoice_copy_preference = True - if UserPreference.get_preference(request.user, INVOICE_KEY) is not None: - invoice_copy_preference = UserPreference.get_preference(request.user, INVOICE_KEY) == 'True' + invoice_preference_value = get_user_preference(request.user, INVOICE_KEY) + if invoice_preference_value is not None: + invoice_copy_preference = invoice_preference_value == 'True' return JsonResponse({ 'invoice_copy': invoice_copy_preference diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index d2224e756b..4b1097426a 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -18,7 +18,7 @@ from xmodule.partitions.partitions import Group, UserPartition from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory -import openedx.core.djangoapps.user_api.api.course_tag as course_tag_api +import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme from instructor_task.models import ReportStore from instructor_task.tasks_helper import cohort_students_and_upload, upload_grades_csv, upload_students_csv diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 72ee689dbd..65b0f3eb87 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -8,7 +8,7 @@ import xblock.reference.plugins from django.core.urlresolvers import reverse from django.conf import settings from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig -from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api +from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api from xmodule.modulestore.django import modulestore from xmodule.services import SettingsService from xmodule.library_tools import LibraryToolsService diff --git a/lms/djangoapps/mobile_api/social_facebook/preferences/views.py b/lms/djangoapps/mobile_api/social_facebook/preferences/views.py index 5994f07875..c48901e67d 100644 --- a/lms/djangoapps/mobile_api/social_facebook/preferences/views.py +++ b/lms/djangoapps/mobile_api/social_facebook/preferences/views.py @@ -5,7 +5,7 @@ Views for users sharing preferences from rest_framework import generics, status from rest_framework.response import Response -from openedx.core.djangoapps.user_api.api.profile import preference_info, update_preferences +from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences, set_user_preference from ...utils import mobile_view from . import serializers @@ -42,11 +42,11 @@ class UserSharing(generics.ListCreateAPIView): serializer = self.get_serializer(data=request.DATA, files=request.FILES) if serializer.is_valid(): value = serializer.object['share_with_facebook_friends'] - update_preferences(request.user.username, share_with_facebook_friends=value) + set_user_preference(request.user, "share_with_facebook_friends", value) return self.get(request, *args, **kwargs) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def get(self, request, *args, **kwargs): - preferences = preference_info(request.user.username) + preferences = get_user_preferences(request.user) response = {'share_with_facebook_friends': preferences.get('share_with_facebook_friends', 'False')} return Response(response) diff --git a/lms/djangoapps/mobile_api/social_facebook/test_utils.py b/lms/djangoapps/mobile_api/social_facebook/test_utils.py index 462d33910b..5bb3130ff4 100644 --- a/lms/djangoapps/mobile_api/social_facebook/test_utils.py +++ b/lms/djangoapps/mobile_api/social_facebook/test_utils.py @@ -12,7 +12,7 @@ from social.apps.django_app.default.models import UserSocialAuth from student.models import CourseEnrollment from student.views import login_oauth_token -from openedx.core.djangoapps.user_api.api.profile import preference_info, update_preferences +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, set_user_preference from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from courseware.tests.factories import UserFactory @@ -132,8 +132,9 @@ class SocialFacebookTestCase(ModuleStoreTestCase, APITestCase): """ Sets self.user's share settings to boolean_value """ - update_preferences(user.username, share_with_facebook_friends=boolean_value) - self.assertEqual(preference_info(user.username)['share_with_facebook_friends'], unicode(boolean_value)) + # Note that setting the value to boolean will result in the conversion to the unicode form of the boolean. + set_user_preference(user, 'share_with_facebook_friends', boolean_value) + self.assertEqual(get_user_preference(user, 'share_with_facebook_friends'), unicode(boolean_value)) def _change_enrollment(self, action, course_id=None, email_opt_in=None): """ diff --git a/lms/djangoapps/mobile_api/social_facebook/utils.py b/lms/djangoapps/mobile_api/social_facebook/utils.py index d65f281f61..daa379edc6 100644 --- a/lms/djangoapps/mobile_api/social_facebook/utils.py +++ b/lms/djangoapps/mobile_api/social_facebook/utils.py @@ -5,10 +5,12 @@ import json import urllib2 import facebook from django.conf import settings +from django.core.exceptions import ObjectDoesNotExist from rest_framework import status from rest_framework.response import Response from social.apps.django_app.default.models import UserSocialAuth -from openedx.core.djangoapps.user_api.api.profile import preference_info +from openedx.core.djangoapps.user_api.models import UserPreference +from student.models import User # TODO @@ -64,5 +66,11 @@ def share_with_facebook_friends(friend): """ Return true if the user's share_with_facebook_friends preference is set to true. """ - share_fb_friends_settings = preference_info(friend['edX_username']) - return share_fb_friends_settings.get('share_with_facebook_friends', None) == 'True' + + # Calling UserPreference directly because the requesting user may be different (and not is_staff). + try: + existing_user = User.objects.get(username=friend['edX_username']) + except ObjectDoesNotExist: + return False + + return UserPreference.get_value(existing_user, 'share_with_facebook_friends') == 'True' diff --git a/lms/djangoapps/notification_prefs/features/unsubscribe.py b/lms/djangoapps/notification_prefs/features/unsubscribe.py index 987211b0cd..00221732ce 100644 --- a/lms/djangoapps/notification_prefs/features/unsubscribe.py +++ b/lms/djangoapps/notification_prefs/features/unsubscribe.py @@ -1,7 +1,7 @@ from django.contrib.auth.models import User from lettuce import step, world from notification_prefs import NOTIFICATION_PREF_KEY -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference, get_user_preference USERNAME = "robot" @@ -11,7 +11,7 @@ UNSUB_TOKEN = "av9E-14sAP1bVBRCPbrTHQ==" @step(u"I have notifications enabled") def enable_notifications(step_): user = User.objects.get(username=USERNAME) - UserPreference.objects.create(user=user, key=NOTIFICATION_PREF_KEY, value=UNSUB_TOKEN) + set_user_preference(user, NOTIFICATION_PREF_KEY, UNSUB_TOKEN) @step(u"I access my unsubscribe url") @@ -22,4 +22,4 @@ def access_unsubscribe_url(step_): @step(u"my notifications should be disabled") def notifications_should_be_disabled(step_): user = User.objects.get(username=USERNAME) - assert not UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).exists() + assert not get_user_preference(user, NOTIFICATION_PREF_KEY) diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py index af9e62c5d5..c60d702e21 100644 --- a/lms/djangoapps/notification_prefs/views.py +++ b/lms/djangoapps/notification_prefs/views.py @@ -13,6 +13,7 @@ from django.views.decorators.http import require_GET, require_POST from edxmako.shortcuts import render_to_response from notification_prefs import NOTIFICATION_PREF_KEY from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import delete_user_preference class UsernameDecryptionException(Exception): @@ -95,6 +96,8 @@ def enable_notifications(user): Enable notifications for a user. Currently only used for daily forum digests. """ + # Calling UserPreference directly because this method is called from a couple of places, + # and it is not clear that user is always the user initiating the request. UserPreference.objects.get_or_create( user=user, key=NOTIFICATION_PREF_KEY, @@ -104,17 +107,6 @@ def enable_notifications(user): ) -def disable_notifications(user): - """ - Disable notifications for a user. - Currently only used for daily forum digests. - """ - UserPreference.objects.filter( - user=user, - key=NOTIFICATION_PREF_KEY - ).delete() - - @require_POST def ajax_enable(request): """ @@ -123,7 +115,7 @@ def ajax_enable(request): This view should be invoked by an AJAX POST call. It returns status 204 (no content) or an error. If notifications were already enabled for this user, this has no effect. Otherwise, a preference is created with the - unsubscribe token (an ecnryption of the username) as the value.unsernam + unsubscribe token (an encryption of the username) as the value.username """ if not request.user.is_authenticated(): raise PermissionDenied @@ -144,7 +136,7 @@ def ajax_disable(request): if not request.user.is_authenticated(): raise PermissionDenied - disable_notifications(request.user) + delete_user_preference(request.user, NOTIFICATION_PREF_KEY) return HttpResponse(status=204) @@ -192,6 +184,8 @@ def set_subscription(request, token, subscribe): # pylint: disable=unused-argum except User.DoesNotExist: raise Http404("username") + # Calling UserPreference directly because the fact that the user is passed in the token implies + # that it may not match request.user. if subscribe: UserPreference.objects.get_or_create(user=user, key=NOTIFICATION_PREF_KEY, diff --git a/lms/djangoapps/oauth2_handler/handlers.py b/lms/djangoapps/oauth2_handler/handlers.py index be784cd166..5989dfb2fe 100644 --- a/lms/djangoapps/oauth2_handler/handlers.py +++ b/lms/djangoapps/oauth2_handler/handlers.py @@ -66,10 +66,10 @@ class ProfileHandler(object): """ Return the locale for the users based on their preferences. Does not return a value if the users have not set their locale preferences. - """ - language = UserPreference.get_preference(data['user'], LANGUAGE_KEY) + # Calling UserPreference directly because it is not clear which user made the request. + language = UserPreference.get_value(data['user'], LANGUAGE_KEY) # If the user has no language specified, return the default one. if not language: diff --git a/lms/djangoapps/oauth2_handler/tests.py b/lms/djangoapps/oauth2_handler/tests.py index 703e2b49d1..d34d24072c 100644 --- a/lms/djangoapps/oauth2_handler/tests.py +++ b/lms/djangoapps/oauth2_handler/tests.py @@ -9,7 +9,7 @@ from student.models import anonymous_id_for_user from student.models import UserProfile from student.roles import CourseStaffRole, CourseInstructorRole from student.tests.factories import UserFactory, UserProfileFactory -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # Will also run default tests for IDTokens and UserInfo @@ -68,7 +68,7 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): def test_user_with_locale_claim(self): language = 'en' - UserPreference.set_preference(self.user, LANGUAGE_KEY, language) + set_user_preference(self.user, LANGUAGE_KEY, language) scopes, claims = self.get_id_token_values('openid profile') self.assertIn('profile', scopes) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index a3897e806f..63202e7c10 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -18,8 +18,8 @@ from django.test.utils import override_settings from util.testing import UrlResetMixin from third_party_auth.tests.testutil import simulate_running_pipeline from embargo.test_utils import restrict_course -from openedx.core.djangoapps.user_api.api import account as account_api -from openedx.core.djangoapps.user_api.api import profile as profile_api +from openedx.core.djangoapps.user_api.accounts.api import activate_account, create_account +from openedx.core.djangoapps.user_api.accounts import EMAIL_MAX_LENGTH from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) @@ -53,7 +53,7 @@ class StudentAccountUpdateTest(UrlResetMixin, TestCase): # Long email -- subtract the length of the @domain # except for one character (so we exceed the max length limit) u"{user}@example.com".format( - user=(u'e' * (account_api.EMAIL_MAX_LENGTH - 11)) + user=(u'e' * (EMAIL_MAX_LENGTH - 11)) ) ] @@ -63,8 +63,8 @@ class StudentAccountUpdateTest(UrlResetMixin, TestCase): super(StudentAccountUpdateTest, self).setUp("student_account.urls") # Create/activate a new account - activation_key = account_api.create_account(self.USERNAME, self.OLD_PASSWORD, self.OLD_EMAIL) - account_api.activate_account(activation_key) + activation_key = create_account(self.USERNAME, self.OLD_PASSWORD, self.OLD_EMAIL) + activate_account(activation_key) # Login result = self.client.login(username=self.USERNAME, password=self.OLD_PASSWORD) @@ -148,7 +148,7 @@ class StudentAccountUpdateTest(UrlResetMixin, TestCase): self.client.logout() # Create a second user, but do not activate it - account_api.create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL) + create_account(self.ALTERNATE_USERNAME, self.OLD_PASSWORD, self.NEW_EMAIL) # Send the view the email address tied to the inactive user response = self._change_password(email=self.NEW_EMAIL) @@ -226,8 +226,8 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) @ddt.data("account_login", "account_register") def test_login_and_registration_form_already_authenticated(self, url_name): # Create/activate a new account and log in - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.activate_account(activation_key) + activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + activate_account(activation_key) result = self.client.login(username=self.USERNAME, password=self.PASSWORD) self.assertTrue(result) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 042f893422..2b110e2b62 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -11,15 +11,13 @@ from django.http import ( from django.shortcuts import redirect from django.http import HttpRequest from django.core.urlresolvers import reverse, resolve -from django.core.mail import send_mail from django.utils.translation import ugettext as _ from django_future.csrf import ensure_csrf_cookie -from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError -from edxmako.shortcuts import render_to_response, render_to_string +from edxmako.shortcuts import render_to_response from microsite_configuration import microsite from embargo import api as embargo_api import third_party_auth @@ -32,8 +30,8 @@ from student.views import ( register_user as old_register_view ) -from openedx.core.djangoapps.user_api.api import account as account_api -from openedx.core.djangoapps.user_api.api import profile as profile_api +from openedx.core.djangoapps.user_api.accounts.api import request_password_change +from openedx.core.djangoapps.user_api.errors import UserNotFound from util.bad_request_rate_limiter import BadRequestRateLimiter from student_account.helpers import auth_pipeline_urls @@ -136,8 +134,8 @@ def password_change_request_handler(request): if email: try: - account_api.request_password_change(email, request.get_host(), request.is_secure()) - except account_api.AccountUserNotFound: + request_password_change(email, request.get_host(), request.is_secure()) + except UserNotFound: AUDIT_LOG.info("Invalid password reset attempt") # Increment the rate limit counter limiter.tick_bad_request_counter(request) diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 1ec6931b89..58bee7efe7 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -29,7 +29,7 @@ from django.core.mail import send_mail from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH -from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountValidationError +from openedx.core.djangoapps.user_api.errors import UserNotFound, AccountValidationError from course_modes.models import CourseMode from student.models import CourseEnrollment @@ -734,7 +734,7 @@ def submit_photos_for_verification(request): if request.POST.get('full_name'): try: update_account_settings(request.user, {"name": request.POST.get('full_name')}) - except AccountUserNotFound: + except UserNotFound: return HttpResponseBadRequest(_("No profile found for user")) except AccountValidationError: msg = _( diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 4db70bee44..02acd3f7b3 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -2,8 +2,21 @@ Account constants """ -# The minimum acceptable length for the name account field +# The minimum and maximum length for the name ("full name") account field NAME_MIN_LENGTH = 2 +NAME_MAX_LENGTH = 255 + +# The minimum and maximum length for the username account field +USERNAME_MIN_LENGTH = 2 +USERNAME_MAX_LENGTH = 30 + +# The minimum and maximum length for the email account field +EMAIL_MIN_LENGTH = 3 +EMAIL_MAX_LENGTH = 254 + +# The minimum and maximum length for the password account field +PASSWORD_MIN_LENGTH = 2 +PASSWORD_MAX_LENGTH = 75 ACCOUNT_VISIBILITY_PREF_KEY = 'account_privacy' diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 4789241a5b..d625d7817a 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -1,20 +1,32 @@ -from django.contrib.auth.models import User from django.utils.translation import ugettext as _ +from django.db import transaction, IntegrityError import datetime from pytz import UTC from django.core.exceptions import ObjectDoesNotExist from django.conf import settings +from django.core.validators import validate_email, validate_slug, ValidationError -from openedx.core.djangoapps.user_api.api.account import ( - AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError +from student.models import User, UserProfile, Registration +from student.views import validate_new_email, do_email_change_request + +from ..errors import ( + AccountUpdateError, AccountValidationError, AccountUsernameInvalid, AccountPasswordInvalid, + AccountEmailInvalid, AccountUserAlreadyExists, + UserAPIInternalError, UserAPIRequestError, UserNotFound, UserNotAuthorized +) +from ..forms import PasswordResetFormNoActive +from ..helpers import intercept_errors +from ..models import UserPreference + +from . import ( + ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY, + EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, + USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH ) from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer -from student.models import UserProfile -from student.views import validate_new_email, do_email_change_request -from ..models import UserPreference -from . import ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) def get_account_settings(requesting_user, username=None, configuration=None, view=None): """Returns account information for a user serialized as JSON. @@ -39,8 +51,9 @@ def get_account_settings(requesting_user, username=None, configuration=None, vie A dict containing account fields. Raises: - AccountUserNotFound: no user with username `username` exists (or `requesting_user.username` if + UserNotFound: no user with username `username` exists (or `requesting_user.username` if `username` is not specified) + UserAPIInternalError: the operation failed due to an unexpected error. """ if username is None: username = requesting_user.username @@ -63,7 +76,9 @@ def get_account_settings(requesting_user, username=None, configuration=None, vie visible_settings = {} - profile_privacy = UserPreference.get_preference(existing_user, ACCOUNT_VISIBILITY_PREF_KEY) + # Calling UserPreference directly because the requesting user may be different from existing_user + # (and does not have to be is_staff). + profile_privacy = UserPreference.get_value(existing_user, ACCOUNT_VISIBILITY_PREF_KEY) privacy_setting = profile_privacy if profile_privacy else configuration.get('default_visibility') if privacy_setting == ALL_USERS_VISIBILITY: @@ -77,6 +92,8 @@ def get_account_settings(requesting_user, username=None, configuration=None, vie return visible_settings +@transaction.commit_on_success +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) def update_account_settings(requesting_user, update, username=None): """Update user account information. @@ -92,9 +109,9 @@ def update_account_settings(requesting_user, update, username=None): `requesting_user.username` is assumed. Raises: - AccountUserNotFound: no user with username `username` exists (or `requesting_user.username` if + UserNotFound: no user with username `username` exists (or `requesting_user.username` if `username` is not specified) - AccountNotAuthorized: the requesting_user does not have access to change the account + UserNotAuthorized: the requesting_user does not have access to change the account associated with `username` AccountValidationError: the update was not attempted because validation errors were found with the supplied update @@ -102,7 +119,7 @@ def update_account_settings(requesting_user, update, username=None): time, some parts of the update may have been successful, even if an AccountUpdateError is returned; in particular, the user account (not including e-mail address) may have successfully been updated, but then the e-mail change request, which is processed last, may throw an error. - + UserAPIInternalError: the operation failed due to an unexpected error. """ if username is None: username = requesting_user.username @@ -110,7 +127,7 @@ def update_account_settings(requesting_user, update, username=None): existing_user, existing_user_profile = _get_user_and_profile(username) if requesting_user.username != username: - raise AccountNotAuthorized() + raise UserNotAuthorized() # If user has requested to change email, we must call the multi-step process to handle this. # It is not handled by the serializer (which considers email to be read-only). @@ -138,7 +155,7 @@ def update_account_settings(requesting_user, update, username=None): for read_only_field in read_only_fields: field_errors[read_only_field] = { "developer_message": "This field is not editable via this API", - "user_message": _("Field '{field_name}' cannot be edited.".format(field_name=read_only_field)) + "user_message": _(u"Field '{field_name}' cannot be edited.").format(field_name=read_only_field) } del update[read_only_field] @@ -154,7 +171,7 @@ def update_account_settings(requesting_user, update, username=None): validate_new_email(existing_user, new_email) except ValueError as err: field_errors["email"] = { - "developer_message": "Error thrown from validate_new_email: '{}'".format(err.message), + "developer_message": u"Error thrown from validate_new_email: '{}'".format(err.message), "user_message": err.message } @@ -175,7 +192,7 @@ def update_account_settings(requesting_user, update, username=None): meta['old_names'] = [] meta['old_names'].append([ old_name, - "Name change requested through account API by {0}".format(requesting_user.username), + u"Name change requested through account API by {0}".format(requesting_user.username), datetime.datetime.now(UTC).isoformat() ]) existing_user_profile.set_meta(meta) @@ -183,7 +200,7 @@ def update_account_settings(requesting_user, update, username=None): except Exception as err: raise AccountUpdateError( - "Error thrown when saving account updates: '{}'".format(err.message) + u"Error thrown when saving account updates: '{}'".format(err.message) ) # And try to send the email change request if necessary. @@ -192,7 +209,7 @@ def update_account_settings(requesting_user, update, username=None): do_email_change_request(existing_user, new_email) except ValueError as err: raise AccountUpdateError( - "Error thrown from do_email_change_request: '{}'".format(err.message), + u"Error thrown from do_email_change_request: '{}'".format(err.message), user_message=err.message ) @@ -205,7 +222,7 @@ def _get_user_and_profile(username): existing_user = User.objects.get(username=username) existing_user_profile = UserProfile.objects.get(user=existing_user) except ObjectDoesNotExist: - raise AccountUserNotFound() + raise UserNotFound() return existing_user, existing_user_profile @@ -217,16 +234,286 @@ def _add_serializer_errors(update, serializer, field_errors): """ if not serializer.is_valid(): errors = serializer.errors - for key, value in errors.iteritems(): - if isinstance(value, list) and len(value) > 0: - developer_message = value[0] - else: - developer_message = "Invalid value: {field_value}'".format(field_value=update[key]) + for key, error in errors.iteritems(): + field_value = update[key] field_errors[key] = { - "developer_message": developer_message, - "user_message": _("Value '{field_value}' is not valid for field '{field_name}'.".format( - field_value=update[key], field_name=key) - ) + "developer_message": u"Value '{field_value}' is not valid for field '{field_name}': {error}".format( + field_value=field_value, field_name=key, error=error + ), + "user_message": _(u"Value '{field_value}' is not valid for field '{field_name}'.").format( + field_value=field_value, field_name=key + ), } return field_errors + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +@transaction.commit_on_success +def create_account(username, password, email): + """Create a new user account. + + This will implicitly create an empty profile for the user. + + WARNING: This function does NOT yet implement all the features + in `student/views.py`. Until it does, please use this method + ONLY for tests of the account API, not in production code. + In particular, these are currently missing: + + * 3rd party auth + * External auth (shibboleth) + * Complex password policies (ENFORCE_PASSWORD_POLICY) + + In addition, we assume that some functionality is handled + at higher layers: + + * Analytics events + * Activation email + * Terms of service / honor code checking + * Recording demographic info (use profile API) + * Auto-enrollment in courses (if invited via instructor dash) + + Args: + username (unicode): The username for the new account. + password (unicode): The user's password. + email (unicode): The email address associated with the account. + + Returns: + unicode: an activation key for the account. + + Raises: + AccountUserAlreadyExists + AccountUsernameInvalid + AccountEmailInvalid + AccountPasswordInvalid + UserAPIInternalError: the operation failed due to an unexpected error. + """ + # Validate the username, password, and email + # This will raise an exception if any of these are not in a valid format. + _validate_username(username) + _validate_password(password, username) + _validate_email(email) + + # Create the user account, setting them to "inactive" until they activate their account. + user = User(username=username, email=email, is_active=False) + user.set_password(password) + + try: + user.save() + except IntegrityError: + raise AccountUserAlreadyExists + + # Create a registration to track the activation process + # This implicitly saves the registration. + registration = Registration() + registration.register(user) + + # Create an empty user profile with default values + UserProfile(user=user).save() + + # Return the activation key, which the caller should send to the user + return registration.activation_key + + +def check_account_exists(username=None, email=None): + """Check whether an account with a particular username or email already exists. + + Keyword Arguments: + username (unicode) + email (unicode) + + Returns: + list of conflicting fields + + Example Usage: + >>> account_api.check_account_exists(username="bob") + [] + >>> account_api.check_account_exists(username="ted", email="ted@example.com") + ["email", "username"] + + """ + conflicts = [] + + if email is not None and User.objects.filter(email=email).exists(): + conflicts.append("email") + + if username is not None and User.objects.filter(username=username).exists(): + conflicts.append("username") + + return conflicts + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +def activate_account(activation_key): + """Activate a user's account. + + Args: + activation_key (unicode): The activation key the user received via email. + + Returns: + None + + Raises: + UserNotAuthorized + UserAPIInternalError: the operation failed due to an unexpected error. + """ + try: + registration = Registration.objects.get(activation_key=activation_key) + except Registration.DoesNotExist: + raise UserNotAuthorized + else: + # This implicitly saves the registration + registration.activate() + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +def request_password_change(email, orig_host, is_secure): + """Email a single-use link for performing a password reset. + + Users must confirm the password change before we update their information. + + Args: + email (string): An email address + orig_host (string): An originating host, extracted from a request with get_host + is_secure (Boolean): Whether the request was made with HTTPS + + Returns: + None + + Raises: + UserNotFound + AccountRequestError + UserAPIInternalError: the operation failed due to an unexpected error. + """ + # Binding data to a form requires that the data be passed as a dictionary + # to the Form class constructor. + form = PasswordResetFormNoActive({'email': email}) + + # Validate that a user exists with the given email address. + if form.is_valid(): + # Generate a single-use link for performing a password reset + # and email it to the user. + form.save( + from_email=settings.DEFAULT_FROM_EMAIL, + domain_override=orig_host, + use_https=is_secure + ) + else: + # No user with the provided email address exists. + raise UserNotFound + + +def _validate_username(username): + """Validate the username. + + Arguments: + username (unicode): The proposed username. + + Returns: + None + + Raises: + AccountUsernameInvalid + + """ + if not isinstance(username, basestring): + raise AccountUsernameInvalid(u"Username must be a string") + + if len(username) < USERNAME_MIN_LENGTH: + raise AccountUsernameInvalid( + u"Username '{username}' must be at least {min} characters long".format( + username=username, + min=USERNAME_MIN_LENGTH + ) + ) + if len(username) > USERNAME_MAX_LENGTH: + raise AccountUsernameInvalid( + u"Username '{username}' must be at most {max} characters long".format( + username=username, + max=USERNAME_MAX_LENGTH + ) + ) + try: + validate_slug(username) + except ValidationError: + raise AccountUsernameInvalid( + u"Username '{username}' must contain only A-Z, a-z, 0-9, -, or _ characters" + ) + + +def _validate_password(password, username): + """Validate the format of the user's password. + + Passwords cannot be the same as the username of the account, + so we take `username` as an argument. + + Arguments: + password (unicode): The proposed password. + username (unicode): The username associated with the user's account. + + Returns: + None + + Raises: + AccountPasswordInvalid + + """ + if not isinstance(password, basestring): + raise AccountPasswordInvalid(u"Password must be a string") + + if len(password) < PASSWORD_MIN_LENGTH: + raise AccountPasswordInvalid( + u"Password must be at least {min} characters long".format( + min=PASSWORD_MIN_LENGTH + ) + ) + + if len(password) > PASSWORD_MAX_LENGTH: + raise AccountPasswordInvalid( + u"Password must be at most {max} characters long".format( + max=PASSWORD_MAX_LENGTH + ) + ) + + if password == username: + raise AccountPasswordInvalid(u"Password cannot be the same as the username") + + +def _validate_email(email): + """Validate the format of the email address. + + Arguments: + email (unicode): The proposed email. + + Returns: + None + + Raises: + AccountEmailInvalid + + """ + if not isinstance(email, basestring): + raise AccountEmailInvalid(u"Email must be a string") + + if len(email) < EMAIL_MIN_LENGTH: + raise AccountEmailInvalid( + u"Email '{email}' must be at least {min} characters long".format( + email=email, + min=EMAIL_MIN_LENGTH + ) + ) + + if len(email) > EMAIL_MAX_LENGTH: + raise AccountEmailInvalid( + u"Email '{email}' must be at most {max} characters long".format( + email=email, + max=EMAIL_MAX_LENGTH + ) + ) + + try: + validate_email(email) + except ValidationError: + raise AccountEmailInvalid( + u"Email '{email}' format is not valid".format(email=email) + ) diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index a44f4864c7..8cef22a90a 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -10,8 +10,8 @@ class AccountUserSerializer(serializers.HyperlinkedModelSerializer): """ class Meta: model = User - fields = ("username", "email", "date_joined") - read_only_fields = ("username", "email", "date_joined") + fields = ("username", "email", "date_joined", "is_active") + read_only_fields = ("username", "email", "date_joined", "is_active") class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index fff9493cdf..0fb92da9e9 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -3,18 +3,27 @@ Unit tests for behavior that is specific to the api methods (vs. the view methods). Most of the functionality is covered in test_views.py. """ +import re +import ddt +from dateutil.parser import parse as parse_datetime from mock import Mock, patch from django.test import TestCase +from nose.tools import raises import unittest from student.tests.factories import UserFactory from django.conf import settings +from django.contrib.auth.models import User +from django.core import mail from student.models import PendingEmailChange -from openedx.core.djangoapps.user_api.api.account import ( - AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError +from ...errors import ( + UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError, + AccountUserAlreadyExists, AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, AccountRequestError ) -from ..api import get_account_settings, update_account_settings -from ..serializers import AccountUserSerializer +from ..api import ( + get_account_settings, update_account_settings, create_account, activate_account, request_password_change +) +from .. import USERNAME_MAX_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MAX_LENGTH def mock_render_to_string(template_name, context): @@ -70,12 +79,12 @@ class TestAccountApi(TestCase): self.assertEqual(self.different_user.email, account_settings["email"]) def test_get_user_not_found(self): - """Test that AccountUserNotFound is thrown if there is no user with username.""" - with self.assertRaises(AccountUserNotFound): + """Test that UserNotFound is thrown if there is no user with username.""" + with self.assertRaises(UserNotFound): get_account_settings(self.user, username="does_not_exist") self.user.username = "does_not_exist" - with self.assertRaises(AccountUserNotFound): + with self.assertRaises(UserNotFound): get_account_settings(self.user) def test_update_username_provided(self): @@ -88,16 +97,16 @@ class TestAccountApi(TestCase): account_settings = get_account_settings(self.user) self.assertEqual("Donald Duck", account_settings["name"]) - with self.assertRaises(AccountNotAuthorized): + with self.assertRaises(UserNotAuthorized): update_account_settings(self.different_user, {"name": "Pluto"}, username=self.user.username) def test_update_user_not_found(self): - """Test that AccountUserNotFound is thrown if there is no user with username.""" - with self.assertRaises(AccountUserNotFound): + """Test that UserNotFound is thrown if there is no user with username.""" + with self.assertRaises(UserNotFound): update_account_settings(self.user, {}, username="does_not_exist") self.user.username = "does_not_exist" - with self.assertRaises(AccountUserNotFound): + with self.assertRaises(UserNotFound): update_account_settings(self.user, {}) def test_update_error_validating(self): @@ -117,18 +126,13 @@ class TestAccountApi(TestCase): "email": "not an email address" } - error_thrown = False - try: + with self.assertRaises(AccountValidationError) as context_manager: update_account_settings(self.user, naughty_update) - except AccountValidationError as response: - error_thrown = True - field_errors = response.field_errors - self.assertEqual(3, len(field_errors)) - self.assertEqual("This field is not editable via this API", field_errors["username"]["developer_message"]) - self.assertIn("Select a valid choice", field_errors["gender"]["developer_message"]) - self.assertIn("Valid e-mail address required.", field_errors["email"]["developer_message"]) - - self.assertTrue(error_thrown, "No AccountValidationError was thrown") + field_errors = context_manager.exception.field_errors + self.assertEqual(3, len(field_errors)) + self.assertEqual("This field is not editable via this API", field_errors["username"]["developer_message"]) + self.assertIn("Select a valid choice", field_errors["gender"]["developer_message"]) + self.assertIn("Valid e-mail address required.", field_errors["email"]["developer_message"]) @patch('django.core.mail.send_mail') @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) @@ -139,14 +143,9 @@ class TestAccountApi(TestCase): "name": "Mickey Mouse", "email": "seems_ok@sample.com" } - error_thrown = False - try: + with self.assertRaises(AccountUpdateError) as context_manager: update_account_settings(self.user, less_naughty_update) - except AccountUpdateError as response: - error_thrown = True - self.assertIn("Error thrown from do_email_change_request", response.developer_message) - - self.assertTrue(error_thrown, "No AccountUpdateError was thrown") + self.assertIn("Error thrown from do_email_change_request", context_manager.exception.developer_message) # Verify that the name change happened, even though the attempt to send the email failed. account_settings = get_account_settings(self.user) @@ -163,15 +162,193 @@ class TestAccountApi(TestCase): "name": "Mickey Mouse", "email": "ok@sample.com" } - error_thrown = False - try: - update_account_settings(self.user, update_will_fail) - except AccountUpdateError as response: - error_thrown = True - self.assertIn("Error thrown when saving account updates", response.developer_message) - self.assertTrue(error_thrown, "No AccountUpdateError was thrown") + with self.assertRaises(AccountUpdateError) as context_manager: + update_account_settings(self.user, update_will_fail) + self.assertIn("Error thrown when saving account updates", context_manager.exception.developer_message) # Verify that no email change request was initiated. pending_change = PendingEmailChange.objects.filter(user=self.user) self.assertEqual(0, len(pending_change)) + + +class AccountSettingsOnCreationTest(TestCase): + + USERNAME = u'frank-underwood' + PASSWORD = u'ṕáśśẃőŕd' + EMAIL = u'frank+underwood@example.com' + + def test_create_account(self): + # Create a new account, which should have empty account settings by default. + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + + # Retrieve the account settings + user = User.objects.get(username=self.USERNAME) + account_settings = get_account_settings(user) + + # Expect a date joined field but remove it to simplify the following comparison + self.assertIsNotNone(account_settings['date_joined']) + del account_settings['date_joined'] + + # Expect all the values to be defaulted + self.assertEqual(account_settings, { + 'username': self.USERNAME, + 'email': self.EMAIL, + 'name': u'', + 'gender': None, + 'language': u'', + 'goals': None, + 'is_active': False, + 'level_of_education': None, + 'mailing_address': None, + 'year_of_birth': None, + 'country': None, + }) + + +@ddt.ddt +class AccountCreationActivationAndPasswordChangeTest(TestCase): + + USERNAME = u'frank-underwood' + PASSWORD = u'ṕáśśẃőŕd' + EMAIL = u'frank+underwood@example.com' + + ORIG_HOST = 'example.com' + IS_SECURE = False + + INVALID_USERNAMES = [ + None, + u'', + u'a', + u'a' * (USERNAME_MAX_LENGTH + 1), + u'invalid_symbol_@', + u'invalid-unicode_fŕáńḱ', + ] + + INVALID_EMAILS = [ + None, + u'', + u'a', + 'no_domain', + 'no+domain', + '@', + '@domain.com', + 'test@no_extension', + u'fŕáńḱ@example.com', + u'frank@éxáḿṕĺé.ćőḿ', + + # Long email -- subtract the length of the @domain + # except for one character (so we exceed the max length limit) + u'{user}@example.com'.format( + user=(u'e' * (EMAIL_MAX_LENGTH - 11)) + ) + ] + + INVALID_PASSWORDS = [ + None, + u'', + u'a', + u'a' * (PASSWORD_MAX_LENGTH + 1) + ] + + def test_activate_account(self): + # Create the account, which is initially inactive + activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + user = User.objects.get(username=self.USERNAME) + account = get_account_settings(user) + self.assertEqual(self.USERNAME, account["username"]) + self.assertEqual(self.EMAIL, account["email"]) + self.assertFalse(account["is_active"]) + + # Activate the account and verify that it is now active + activate_account(activation_key) + account = get_account_settings(user) + self.assertTrue(account['is_active']) + + def test_create_account_duplicate_username(self): + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + with self.assertRaises(AccountUserAlreadyExists): + create_account(self.USERNAME, self.PASSWORD, 'different+email@example.com') + + # Email uniqueness constraints were introduced in a database migration, + # which we disable in the unit tests to improve the speed of the test suite. + @unittest.skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required") + def test_create_account_duplicate_email(self): + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + with self.assertRaises(AccountUserAlreadyExists): + create_account('different_user', self.PASSWORD, self.EMAIL) + + def test_username_too_long(self): + long_username = 'e' * (USERNAME_MAX_LENGTH + 1) + with self.assertRaises(AccountUsernameInvalid): + create_account(long_username, self.PASSWORD, self.EMAIL) + + @raises(AccountEmailInvalid) + @ddt.data(*INVALID_EMAILS) + def test_create_account_invalid_email(self, invalid_email): + create_account(self.USERNAME, self.PASSWORD, invalid_email) + + @raises(AccountPasswordInvalid) + @ddt.data(*INVALID_PASSWORDS) + def test_create_account_invalid_password(self, invalid_password): + create_account(self.USERNAME, invalid_password, self.EMAIL) + + @raises(AccountPasswordInvalid) + def test_create_account_username_password_equal(self): + # Username and password cannot be the same + create_account(self.USERNAME, self.USERNAME, self.EMAIL) + + @raises(AccountRequestError) + @ddt.data(*INVALID_USERNAMES) + def test_create_account_invalid_username(self, invalid_username): + create_account(invalid_username, self.PASSWORD, self.EMAIL) + + @raises(UserNotAuthorized) + def test_activate_account_invalid_key(self): + activate_account(u'invalid') + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') + def test_request_password_change(self): + # Create and activate an account + activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + activate_account(activation_key) + + # Request a password change + request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) + + # Verify that one email message has been sent + self.assertEqual(len(mail.outbox), 1) + + # Verify that the body of the message contains something that looks + # like an activation link + email_body = mail.outbox[0].body + result = re.search('(?Phttps?://[^\s]+)', email_body) + self.assertIsNot(result, None) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') + def test_request_password_change_invalid_user(self): + with self.assertRaises(UserNotFound): + request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) + + # Verify that no email messages have been sent + self.assertEqual(len(mail.outbox), 0) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') + def test_request_password_change_inactive_user(self): + # Create an account, but do not activate it + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + + request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) + + # Verify that the activation email was still sent + self.assertEqual(len(mail.outbox), 1) + + def _assert_is_datetime(self, timestamp): + if not timestamp: + return False + try: + parse_datetime(timestamp) + except ValueError: + return False + else: + return True diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index de518b8b49..e516fc4199 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -11,31 +11,31 @@ from rest_framework.test import APITestCase, APIClient from student.tests.factories import UserFactory from student.models import UserProfile, PendingEmailChange from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY -TEST_PASSWORD = "test" - class UserAPITestCase(APITestCase): """ The base class for all tests of the User API """ + test_password = "test" + def setUp(self): super(UserAPITestCase, self).setUp() self.anonymous_client = APIClient() - self.different_user = UserFactory.create(password=TEST_PASSWORD) + self.different_user = UserFactory.create(password=self.test_password) self.different_client = APIClient() - self.staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD) + self.staff_user = UserFactory(is_staff=True, password=self.test_password) self.staff_client = APIClient() - self.user = UserFactory.create(password=TEST_PASSWORD) + self.user = UserFactory.create(password=self.test_password) # will be assigned to self.client by default def login_client(self, api_client, user): """Helper method for getting the client and user and logging in. Returns client. """ client = getattr(self, api_client) user = getattr(self, user) - client.login(username=user.username, password=TEST_PASSWORD) + client.login(username=user.username, password=self.test_password) return client def send_patch(self, client, json_data, content_type="application/merge-patch+json", expected_status=204): @@ -57,6 +57,22 @@ class UserAPITestCase(APITestCase): self.assertEqual(expected_status, response.status_code) return response + def send_put(self, client, json_data, content_type="application/json", expected_status=204): + """ + Helper method for sending a PUT to the server. Verifies the expected status and returns the response. + """ + response = client.put(self.url, data=json.dumps(json_data), content_type=content_type) + self.assertEqual(expected_status, response.status_code) + return response + + def send_delete(self, client, expected_status=204): + """ + Helper method for sending a DELETE to the server. Verifies the expected status and returns the response. + """ + response = client.delete(self.url) + self.assertEqual(expected_status, response.status_code) + return response + def create_mock_profile(self, user): """ Helper method that creates a mock profile for the specified user @@ -109,7 +125,7 @@ class TestAccountAPI(UserAPITestCase): Verify that all account fields are returned (even those that are not shareable). """ data = response.data - self.assertEqual(11, len(data)) + self.assertEqual(12, len(data)) self.assertEqual(self.user.username, data["username"]) self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) self.assertEqual("US", data["country"]) @@ -120,6 +136,7 @@ class TestAccountAPI(UserAPITestCase): self.assertEqual("world peace", data["goals"]) self.assertEqual("Park Ave", data['mailing_address']) self.assertEqual(self.user.email, data["email"]) + self.assertTrue(data["is_active"]) self.assertIsNotNone(data["date_joined"]) def test_anonymous_access(self): @@ -133,7 +150,7 @@ class TestAccountAPI(UserAPITestCase): """ Test that DELETE, POST, and PUT are not supported. """ - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) self.assertEqual(405, self.client.put(self.url).status_code) self.assertEqual(405, self.client.post(self.url).status_code) self.assertEqual(405, self.client.delete(self.url).status_code) @@ -160,7 +177,7 @@ class TestAccountAPI(UserAPITestCase): Test that a client (logged in) can only get the shareable fields for a different user. This is the case when default_visibility is set to "all_users". """ - self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) + self.different_client.login(username=self.different_user.username, password=self.test_password) self.create_mock_profile(self.user) response = self.send_get(self.different_client) self._verify_full_shareable_account_response(response) @@ -174,7 +191,7 @@ class TestAccountAPI(UserAPITestCase): Test that a client (logged in) can only get the shareable fields for a different user. This is the case when default_visibility is set to "private". """ - self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) + self.different_client.login(username=self.different_user.username, password=self.test_password) self.create_mock_profile(self.user) response = self.send_get(self.different_client) self._verify_private_account_response(response) @@ -201,7 +218,7 @@ class TestAccountAPI(UserAPITestCase): client = self.login_client(api_client, requesting_username) # Update user account visibility setting. - UserPreference.set_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, preference_visibility) + set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, preference_visibility) self.create_mock_profile(self.user) response = self.send_get(client) @@ -219,21 +236,30 @@ class TestAccountAPI(UserAPITestCase): Test that a client (logged in) can get her own account information (using default legacy profile information, as created by the test UserFactory). """ - self.client.login(username=self.user.username, password=TEST_PASSWORD) - response = self.send_get(self.client) - data = response.data - self.assertEqual(11, len(data)) - self.assertEqual(self.user.username, data["username"]) - self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) - for empty_field in ("year_of_birth", "level_of_education", "mailing_address"): - self.assertIsNone(data[empty_field]) - self.assertIsNone(data["country"]) - # TODO: what should the format of this be? - self.assertEqual("", data["language"]) - self.assertEqual("m", data["gender"]) - self.assertEqual("World domination", data["goals"]) - self.assertEqual(self.user.email, data["email"]) - self.assertIsNotNone(data["date_joined"]) + def verify_get_own_information(): + response = self.send_get(self.client) + data = response.data + self.assertEqual(12, len(data)) + self.assertEqual(self.user.username, data["username"]) + self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) + for empty_field in ("year_of_birth", "level_of_education", "mailing_address"): + self.assertIsNone(data[empty_field]) + self.assertIsNone(data["country"]) + # TODO: what should the format of this be? + self.assertEqual("", data["language"]) + self.assertEqual("m", data["gender"]) + self.assertEqual("Learn a lot", data["goals"]) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["date_joined"]) + self.assertEqual(self.user.is_active, data["is_active"]) + + self.client.login(username=self.user.username, password=self.test_password) + verify_get_own_information() + + # Now make sure that the user can get the same information, even if not active + self.user.is_active = False + self.user.save() + verify_get_own_information() def test_get_account_empty_string(self): """ @@ -245,7 +271,7 @@ class TestAccountAPI(UserAPITestCase): legacy_profile.gender = "" legacy_profile.save() - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country"): self.assertIsNone(response.data[empty_field]) @@ -280,12 +306,12 @@ class TestAccountAPI(UserAPITestCase): self.assertEqual(404, response.status_code) @ddt.data( - ("gender", "f", "not a gender", "Select a valid choice. not a gender is not one of the available choices."), - ("level_of_education", "none", "x", "Select a valid choice. x is not one of the available choices."), - ("country", "GB", "XY", "Select a valid choice. XY is not one of the available choices."), - ("year_of_birth", 2009, "not_an_int", "Enter a whole number."), - ("name", "bob", "z" * 256, "Ensure this value has at most 255 characters (it has 256)."), - ("name", u"ȻħȺɍłɇs", "z ", "The name field must be at least 2 characters long."), + ("gender", "f", "not a gender", u"Select a valid choice. not a gender is not one of the available choices."), + ("level_of_education", "none", u"ȻħȺɍłɇs", u"Select a valid choice. ȻħȺɍłɇs is not one of the available choices."), + ("country", "GB", "XY", u"Select a valid choice. XY is not one of the available choices."), + ("year_of_birth", 2009, "not_an_int", u"Enter a whole number."), + ("name", "bob", "z" * 256, u"Ensure this value has at most 255 characters (it has 256)."), + ("name", u"ȻħȺɍłɇs", "z ", u"The name field must be at least 2 characters long."), ("language", "Creole"), ("goals", "Smell the roses"), ("mailing_address", "Sesame Street"), @@ -305,11 +331,13 @@ class TestAccountAPI(UserAPITestCase): if fails_validation_value: error_response = self.send_patch(client, {field: fails_validation_value}, expected_status=400) self.assertEqual( - "Value '{0}' is not valid for field '{1}'.".format(fails_validation_value, field), + u"Value '{0}' is not valid for field '{1}'.".format(fails_validation_value, field), error_response.data["field_errors"][field]["user_message"] ) self.assertEqual( - developer_validation_message, + u"Value '{value}' is not valid for field '{field}': {messages}".format( + value=fails_validation_value, field=field, messages=[developer_validation_message] + ), error_response.data["field_errors"][field]["developer_message"] ) else: @@ -319,6 +347,15 @@ class TestAccountAPI(UserAPITestCase): get_response = self.send_get(client) self.assertEqual("", get_response.data[field]) + def test_patch_inactive_user(self): + """ Verify that a user can patch her own account, even if inactive. """ + self.client.login(username=self.user.username, password=self.test_password) + self.user.is_active = False + self.user.save() + self.send_patch(self.client, {"goals": "to not activate account"}) + get_response = self.send_get(self.client) + self.assertEqual("to not activate account", get_response.data["goals"]) + @ddt.unpack def test_patch_account_noneditable(self): """ @@ -334,7 +371,7 @@ class TestAccountAPI(UserAPITestCase): "Field '{0}' cannot be edited.".format(field_name), data["field_errors"][field_name]["user_message"] ) - for field_name in ["username", "date_joined"]: + for field_name in ["username", "date_joined", "is_active"]: response = self.send_patch(client, {field_name: "will_error", "gender": "f"}, expected_status=400) verify_error_response(field_name, response.data) @@ -352,7 +389,7 @@ class TestAccountAPI(UserAPITestCase): """ Test the behavior of patch when an incorrect content_type is specified. """ - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) self.send_patch(self.client, {}, content_type="application/json", expected_status=415) self.send_patch(self.client, {}, content_type="application/xml", expected_status=415) @@ -361,7 +398,7 @@ class TestAccountAPI(UserAPITestCase): Tests the behavior of patch when attempting to set fields with a select list of options to the empty string. Also verifies the behaviour when setting to None. """ - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) for field_name in ["gender", "level_of_education", "country"]: self.send_patch(self.client, {field_name: ""}) response = self.send_get(self.client) @@ -393,7 +430,7 @@ class TestAccountAPI(UserAPITestCase): get_response = self.send_get(self.client) self.assertEqual(new_name, get_response.data["name"]) - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) legacy_profile = UserProfile.objects.get(id=self.user.id) self.assertEqual({}, legacy_profile.get_meta()) old_name = legacy_profile.name @@ -465,7 +502,7 @@ class TestAccountAPI(UserAPITestCase): Test that AccountUpdateErrors are passed through to the response. """ serializer_save.side_effect = [Exception("bummer"), None] - self.client.login(username=self.user.username, password=TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.test_password) error_response = self.send_patch(self.client, {"goals": "save an account field"}, expected_status=400) self.assertEqual( "Error thrown when saving account updates: 'bummer'", diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 0b150ceac7..f749cfad49 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -7,12 +7,10 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API from rest_framework.views import APIView from rest_framework.response import Response from rest_framework import status -from rest_framework.authentication import OAuth2Authentication, SessionAuthentication +from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser from rest_framework import permissions -from openedx.core.djangoapps.user_api.api.account import ( - AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError -) +from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError from openedx.core.lib.api.parsers import MergePatchParser from .api import get_account_settings, update_account_settings @@ -93,9 +91,9 @@ class AccountView(APIView): If the update could not be completed due to failure at the time of update, this method returns a 400 with specific errors in the returned JSON. - If the updated is successful, a 204 status is returned with no additional content. + If the update is successful, a 204 status is returned with no additional content. """ - authentication_classes = (OAuth2Authentication, SessionAuthentication) + authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser) permission_classes = (permissions.IsAuthenticated,) parser_classes = (MergePatchParser,) @@ -105,7 +103,7 @@ class AccountView(APIView): """ try: account_settings = get_account_settings(request.user, username, view=request.QUERY_PARAMS.get('view')) - except AccountUserNotFound: + except UserNotFound: return Response(status=status.HTTP_404_NOT_FOUND) return Response(account_settings) @@ -120,7 +118,7 @@ class AccountView(APIView): """ try: update_account_settings(request.user, request.DATA, username=username) - except (AccountUserNotFound, AccountNotAuthorized): + except (UserNotFound, UserNotAuthorized): return Response(status=status.HTTP_404_NOT_FOUND) except AccountValidationError as err: return Response({"field_errors": err.field_errors}, status=status.HTTP_400_BAD_REQUEST) diff --git a/openedx/core/djangoapps/user_api/api/account.py b/openedx/core/djangoapps/user_api/api/account.py deleted file mode 100644 index 2e797c8160..0000000000 --- a/openedx/core/djangoapps/user_api/api/account.py +++ /dev/null @@ -1,381 +0,0 @@ -"""Python API for user accounts. - - -Account information includes a student's username, password, and email -address, but does NOT include user profile information (i.e., demographic -information and preferences). - -""" -from django.conf import settings -from django.db import transaction, IntegrityError -from django.core.validators import validate_email, validate_slug, ValidationError - -from ..forms import PasswordResetFormNoActive -from ..models import User, UserProfile, Registration, PendingEmailChange -from ..helpers import intercept_errors - - -USERNAME_MIN_LENGTH = 2 -USERNAME_MAX_LENGTH = 30 - -EMAIL_MIN_LENGTH = 3 -EMAIL_MAX_LENGTH = 254 - -PASSWORD_MIN_LENGTH = 2 -PASSWORD_MAX_LENGTH = 75 - - -class AccountRequestError(Exception): - """There was a problem with the request to the account API. """ - pass - - -class AccountInternalError(Exception): - """An internal error occurred in the account API. """ - pass - - -class AccountUserAlreadyExists(AccountRequestError): - """User with the same username and/or email already exists. """ - pass - - -class AccountUsernameInvalid(AccountRequestError): - """The requested username is not in a valid format. """ - pass - - -class AccountEmailInvalid(AccountRequestError): - """The requested email is not in a valid format. """ - pass - - -class AccountPasswordInvalid(AccountRequestError): - """The requested password is not in a valid format. """ - pass - - -class AccountUserNotFound(AccountRequestError): - """The requested user does not exist. """ - pass - - -class AccountNotAuthorized(AccountRequestError): - """The user is not authorized to perform the requested action. """ - pass - - -class AccountUpdateError(AccountRequestError): - """ - An update to the account failed. More detailed information is present in developer_message, - and depending on the type of error encountered, there may also be a non-null user_message field. - """ - def __init__(self, developer_message, user_message=None): - self.developer_message = developer_message - self.user_message = user_message - - -class AccountValidationError(AccountRequestError): - """ - Validation issues were found with the supplied data. More detailed information is present in field_errors, - a dict with specific information about each field that failed validation. For each field, - there will be at least a developer_message describing the validation issue, and possibly - also a user_message. - """ - def __init__(self, field_errors): - self.field_errors = field_errors - - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -@transaction.commit_on_success -def create_account(username, password, email): - """Create a new user account. - - This will implicitly create an empty profile for the user. - - WARNING: This function does NOT yet implement all the features - in `student/views.py`. Until it does, please use this method - ONLY for tests of the account API, not in production code. - In particular, these are currently missing: - - * 3rd party auth - * External auth (shibboleth) - * Complex password policies (ENFORCE_PASSWORD_POLICY) - - In addition, we assume that some functionality is handled - at higher layers: - - * Analytics events - * Activation email - * Terms of service / honor code checking - * Recording demographic info (use profile API) - * Auto-enrollment in courses (if invited via instructor dash) - - Args: - username (unicode): The username for the new account. - password (unicode): The user's password. - email (unicode): The email address associated with the account. - - Returns: - unicode: an activation key for the account. - - Raises: - AccountUserAlreadyExists - AccountUsernameInvalid - AccountEmailInvalid - AccountPasswordInvalid - - """ - # Validate the username, password, and email - # This will raise an exception if any of these are not in a valid format. - _validate_username(username) - _validate_password(password, username) - _validate_email(email) - - # Create the user account, setting them to "inactive" until they activate their account. - user = User(username=username, email=email, is_active=False) - user.set_password(password) - - try: - user.save() - except IntegrityError: - raise AccountUserAlreadyExists - - # Create a registration to track the activation process - # This implicitly saves the registration. - registration = Registration() - registration.register(user) - - # Create an empty user profile with default values - UserProfile(user=user).save() - - # Return the activation key, which the caller should send to the user - return registration.activation_key - - -def check_account_exists(username=None, email=None): - """Check whether an account with a particular username or email already exists. - - Keyword Arguments: - username (unicode) - email (unicode) - - Returns: - list of conflicting fields - - Example Usage: - >>> account_api.check_account_exists(username="bob") - [] - >>> account_api.check_account_exists(username="ted", email="ted@example.com") - ["email", "username"] - - """ - conflicts = [] - - if email is not None and User.objects.filter(email=email).exists(): - conflicts.append("email") - - if username is not None and User.objects.filter(username=username).exists(): - conflicts.append("username") - - return conflicts - - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -def account_info(username): - """Retrieve information about a user's account. - - Arguments: - username (unicode): The username associated with the account. - - Returns: - dict: User's account information, if the user was found. - None: The user does not exist. - - """ - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - return None - else: - return { - u'username': username, - u'email': user.email, - u'is_active': user.is_active, - } - - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -def activate_account(activation_key): - """Activate a user's account. - - Args: - activation_key (unicode): The activation key the user received via email. - - Returns: - None - - Raises: - AccountNotAuthorized - - """ - try: - registration = Registration.objects.get(activation_key=activation_key) - except Registration.DoesNotExist: - raise AccountNotAuthorized - else: - # This implicitly saves the registration - registration.activate() - - -@intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError]) -def request_password_change(email, orig_host, is_secure): - """Email a single-use link for performing a password reset. - - Users must confirm the password change before we update their information. - - Args: - email (string): An email address - orig_host (string): An originating host, extracted from a request with get_host - is_secure (Boolean): Whether the request was made with HTTPS - - Returns: - None - - Raises: - AccountUserNotFound - AccountRequestError - - """ - # Binding data to a form requires that the data be passed as a dictionary - # to the Form class constructor. - form = PasswordResetFormNoActive({'email': email}) - - # Validate that a user exists with the given email address. - if form.is_valid(): - # Generate a single-use link for performing a password reset - # and email it to the user. - form.save( - from_email=settings.DEFAULT_FROM_EMAIL, - domain_override=orig_host, - use_https=is_secure - ) - else: - # No user with the provided email address exists. - raise AccountUserNotFound - - -def _validate_username(username): - """Validate the username. - - Arguments: - username (unicode): The proposed username. - - Returns: - None - - Raises: - AccountUsernameInvalid - - """ - if not isinstance(username, basestring): - raise AccountUsernameInvalid(u"Username must be a string") - - if len(username) < USERNAME_MIN_LENGTH: - raise AccountUsernameInvalid( - u"Username '{username}' must be at least {min} characters long".format( - username=username, - min=USERNAME_MIN_LENGTH - ) - ) - if len(username) > USERNAME_MAX_LENGTH: - raise AccountUsernameInvalid( - u"Username '{username}' must be at most {max} characters long".format( - username=username, - max=USERNAME_MAX_LENGTH - ) - ) - try: - validate_slug(username) - except ValidationError: - raise AccountUsernameInvalid( - u"Username '{username}' must contain only A-Z, a-z, 0-9, -, or _ characters" - ) - - -def _validate_password(password, username): - """Validate the format of the user's password. - - Passwords cannot be the same as the username of the account, - so we take `username` as an argument. - - Arguments: - password (unicode): The proposed password. - username (unicode): The username associated with the user's account. - - Returns: - None - - Raises: - AccountPasswordInvalid - - """ - if not isinstance(password, basestring): - raise AccountPasswordInvalid(u"Password must be a string") - - if len(password) < PASSWORD_MIN_LENGTH: - raise AccountPasswordInvalid( - u"Password must be at least {min} characters long".format( - min=PASSWORD_MIN_LENGTH - ) - ) - - if len(password) > PASSWORD_MAX_LENGTH: - raise AccountPasswordInvalid( - u"Password must be at most {max} characters long".format( - max=PASSWORD_MAX_LENGTH - ) - ) - - if password == username: - raise AccountPasswordInvalid(u"Password cannot be the same as the username") - - -def _validate_email(email): - """Validate the format of the email address. - - Arguments: - email (unicode): The proposed email. - - Returns: - None - - Raises: - AccountEmailInvalid - - """ - if not isinstance(email, basestring): - raise AccountEmailInvalid(u"Email must be a string") - - if len(email) < EMAIL_MIN_LENGTH: - raise AccountEmailInvalid( - u"Email '{email}' must be at least {min} characters long".format( - email=email, - min=EMAIL_MIN_LENGTH - ) - ) - - if len(email) > EMAIL_MAX_LENGTH: - raise AccountEmailInvalid( - u"Email '{email}' must be at most {max} characters long".format( - email=email, - max=EMAIL_MAX_LENGTH - ) - ) - - try: - validate_email(email) - except ValidationError: - raise AccountEmailInvalid( - u"Email '{email}' format is not valid".format(email=email) - ) diff --git a/openedx/core/djangoapps/user_api/api/profile.py b/openedx/core/djangoapps/user_api/api/profile.py deleted file mode 100644 index 9e19914243..0000000000 --- a/openedx/core/djangoapps/user_api/api/profile.py +++ /dev/null @@ -1,158 +0,0 @@ -"""Python API for user profiles. - -Profile information includes a student's demographic information and preferences, -but does NOT include basic account information such as username, password, and -email address. - -""" -import datetime -import logging - -from django.conf import settings -from django.db import IntegrityError -from pytz import UTC -import analytics - -from eventtracking import tracker -from ..accounts import NAME_MIN_LENGTH -from ..accounts.api import get_account_settings -from ..models import User, UserPreference, UserOrgTag -from ..helpers import intercept_errors - -log = logging.getLogger(__name__) - - -class ProfileRequestError(Exception): - """ The request to the API was not valid. """ - pass - - -class ProfileUserNotFound(ProfileRequestError): - """ The requested user does not exist. """ - pass - - -class ProfileInternalError(Exception): - """ An error occurred in an API call. """ - pass - - -FULL_NAME_MAX_LENGTH = 255 -FULL_NAME_MIN_LENGTH = NAME_MIN_LENGTH - - -@intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) -def preference_info(username): - """Retrieve information about a user's preferences. - - Arguments: - username (unicode): The username of the account to retrieve. - - Returns: - dict: Empty if there is no user - - """ - preferences = UserPreference.objects.filter(user__username=username) - - preferences_dict = {} - for preference in preferences: - preferences_dict[preference.key] = preference.value - - return preferences_dict - - -@intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) -def update_preferences(username, **kwargs): - """Update a user's preferences. - - Sets the provided preferences for the given user. - - Arguments: - username (unicode): The username of the account to retrieve. - - Keyword Arguments: - **kwargs (unicode): Arbitrary key-value preference pairs - - Returns: - None - - Raises: - ProfileUserNotFound - - """ - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - raise ProfileUserNotFound - else: - for key, value in kwargs.iteritems(): - UserPreference.set_preference(user, key, value) - - -@intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) -def update_email_opt_in(user, org, optin): - """Updates a user's preference for receiving org-wide emails. - - Sets a User Org Tag defining the choice to opt in or opt out of organization-wide - emails. - - Arguments: - user (User): The user to set a preference for. - org (str): The org is used to determine the organization this setting is related to. - optin (Boolean): True if the user is choosing to receive emails for this organization. If the user is not - the correct age to receive emails, email-optin is set to False regardless. - - Returns: - None - - """ - account_settings = get_account_settings(user) - year_of_birth = account_settings['year_of_birth'] - of_age = ( - year_of_birth is None or # If year of birth is not set, we assume user is of age. - datetime.datetime.now(UTC).year - year_of_birth > # pylint: disable=maybe-no-member - getattr(settings, 'EMAIL_OPTIN_MINIMUM_AGE', 13) - ) - - try: - preference, _ = UserOrgTag.objects.get_or_create( - user=user, org=org, key='email-optin' - ) - preference.value = str(optin and of_age) - preference.save() - - if settings.FEATURES.get('SEGMENT_IO_LMS') and settings.SEGMENT_IO_LMS_KEY: - _track_update_email_opt_in(user.id, org, optin) - - except IntegrityError as err: - log.warn(u"Could not update organization wide preference due to IntegrityError: {}".format(err.message)) - - -def _track_update_email_opt_in(user_id, organization, opt_in): - """Track an email opt-in preference change. - - Arguments: - user_id (str): The ID of the user making the preference change. - organization (str): The organization whose emails are being opted into or out of by the user. - opt_in (Boolean): Whether the user has chosen to opt-in to emails from the organization. - - Returns: - None - - """ - event_name = 'edx.bi.user.org_email.opted_in' if opt_in else 'edx.bi.user.org_email.opted_out' - tracking_context = tracker.get_tracker().resolve_context() - - analytics.track( - user_id, - event_name, - { - 'category': 'communication', - 'label': organization - }, - context={ - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - ) diff --git a/openedx/core/djangoapps/user_api/api/__init__.py b/openedx/core/djangoapps/user_api/course_tag/__init__.py similarity index 100% rename from openedx/core/djangoapps/user_api/api/__init__.py rename to openedx/core/djangoapps/user_api/course_tag/__init__.py diff --git a/openedx/core/djangoapps/user_api/api/course_tag.py b/openedx/core/djangoapps/user_api/course_tag/api.py similarity index 100% rename from openedx/core/djangoapps/user_api/api/course_tag.py rename to openedx/core/djangoapps/user_api/course_tag/api.py diff --git a/openedx/core/djangoapps/user_api/tests/test_course_tag_api.py b/openedx/core/djangoapps/user_api/course_tag/tests/test_api.py similarity index 95% rename from openedx/core/djangoapps/user_api/tests/test_course_tag_api.py rename to openedx/core/djangoapps/user_api/course_tag/tests/test_api.py index a329df2a3a..ac0aeef409 100644 --- a/openedx/core/djangoapps/user_api/tests/test_course_tag_api.py +++ b/openedx/core/djangoapps/user_api/course_tag/tests/test_api.py @@ -4,7 +4,7 @@ Test the user course tag API. from django.test import TestCase from student.tests.factories import UserFactory -from openedx.core.djangoapps.user_api.api import course_tag as course_tag_api +from openedx.core.djangoapps.user_api.course_tag import api as course_tag_api from opaque_keys.edx.locations import SlashSeparatedCourseKey diff --git a/openedx/core/djangoapps/user_api/errors.py b/openedx/core/djangoapps/user_api/errors.py new file mode 100644 index 0000000000..ea104f01e6 --- /dev/null +++ b/openedx/core/djangoapps/user_api/errors.py @@ -0,0 +1,95 @@ +""" +Errors thrown by the various user APIs. +""" + + +class UserAPIRequestError(Exception): + """There was a problem with the request to the User API. """ + pass + + +class UserAPIInternalError(Exception): + """An internal error occurred in the User API. """ + pass + + +class UserNotFound(UserAPIRequestError): + """The requested user does not exist. """ + pass + + +class UserNotAuthorized(UserAPIRequestError): + """The user is not authorized to perform the requested action. """ + pass + + +class AccountRequestError(UserAPIRequestError): + """There was a problem with the request to the account API. """ + pass + + +class AccountUserAlreadyExists(AccountRequestError): + """User with the same username and/or email already exists. """ + pass + + +class AccountUsernameInvalid(AccountRequestError): + """The requested username is not in a valid format. """ + pass + + +class AccountEmailInvalid(AccountRequestError): + """The requested email is not in a valid format. """ + pass + + +class AccountPasswordInvalid(AccountRequestError): + """The requested password is not in a valid format. """ + pass + + +class AccountUpdateError(AccountRequestError): + """ + An update to the account failed. More detailed information is present in developer_message, + and depending on the type of error encountered, there may also be a non-null user_message field. + """ + def __init__(self, developer_message, user_message=None): + self.developer_message = developer_message + self.user_message = user_message + + +class AccountValidationError(AccountRequestError): + """ + Validation issues were found with the supplied data. More detailed information is present in field_errors, + a dict with specific information about each field that failed validation. For each field, + there will be at least a developer_message describing the validation issue, and possibly + also a user_message. + """ + def __init__(self, field_errors): + self.field_errors = field_errors + + +class PreferenceRequestError(UserAPIRequestError): + """There was a problem with a preference request.""" + pass + + +class PreferenceValidationError(PreferenceRequestError): + """ + Validation issues were found with the supplied data. More detailed information is present + in preference_errors, a dict with specific information about each preference that failed + validation. For each preference, there will be at least a developer_message describing + the validation issue, and possibly also a user_message. + """ + def __init__(self, preference_errors): + self.preference_errors = preference_errors + + +class PreferenceUpdateError(PreferenceRequestError): + """ + An update to a user preference failed. More detailed information is present in developer_message, + and depending on the type of error encountered, there may also be a non-null user_message field. + """ + def __init__(self, developer_message, user_message=None): + self.developer_message = developer_message + self.user_message = user_message diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index a99c53767c..b939a41b77 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -45,9 +45,20 @@ def intercept_errors(api_error, ignore_errors=None): try: return func(*args, **kwargs) except Exception as ex: - # Raise the original exception if it's in our list of "ignored" errors + # Raise and log the original exception if it's in our list of "ignored" errors for ignored in ignore_errors or []: if isinstance(ex, ignored): + msg = ( + u"A handled error occurred when calling '{func_name}' " + u"with arguments '{args}' and keyword arguments '{kwargs}': " + u"{exception}" + ).format( + func_name=func.func_name, + args=args, + kwargs=kwargs, + exception=repr(ex) + ) + LOGGER.warning(msg) raise # Otherwise, log the error and raise the API-specific error diff --git a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py index f7c43fe863..a776687292 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py @@ -19,7 +19,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.models import CourseEnrollment -import openedx.core.djangoapps.user_api.api.profile as profile_api +from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.djangoapps.user_api.models import UserOrgTag from openedx.core.djangoapps.user_api.management.commands import email_opt_in_list @@ -297,7 +297,7 @@ class EmailOptInListTest(ModuleStoreTestCase): None """ - profile_api.update_email_opt_in(user, org, is_opted_in) + update_email_opt_in(user, org, is_opted_in) def _latest_pref_set_datetime(self, user): """Retrieve the latest opt-in preference for the user, diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index e71437214b..f8f94648c0 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -25,27 +25,26 @@ class UserPreference(models.Model): unique_together = ("user", "key") @classmethod - def set_preference(cls, user, preference_key, preference_value): - """ - Sets the user preference for a given key - """ - user_pref, _ = cls.objects.get_or_create(user=user, key=preference_key) - user_pref.value = preference_value - user_pref.save() + def get_value(cls, user, preference_key): + """Gets the user preference value for a given key. - @classmethod - def get_preference(cls, user, preference_key, default=None): - """ - Gets the user preference value for a given key + Note: + This method provides no authorization of access to the user preference. + Consider using user_api.preferences.api.get_user_preference instead if + this is part of a REST API request. - Returns the given default if there isn't a preference for the given key - """ + Arguments: + user (User): The user whose preference should be set. + preference_key (string): The key for the user preference. + Returns: + The user preference value, or None if one is not set. + """ try: - user_pref = cls.objects.get(user=user, key=preference_key) - return user_pref.value + user_preference = cls.objects.get(user=user, key=preference_key) + return user_preference.value except cls.DoesNotExist: - return default + return None class UserCourseTag(models.Model): diff --git a/openedx/core/djangoapps/user_api/partition_schemes.py b/openedx/core/djangoapps/user_api/partition_schemes.py index 53c9a8046b..6f10af7f31 100644 --- a/openedx/core/djangoapps/user_api/partition_schemes.py +++ b/openedx/core/djangoapps/user_api/partition_schemes.py @@ -3,7 +3,7 @@ Provides partition support to the user service. """ import logging import random -import api.course_tag as course_tag_api +import course_tag.api as course_tag_api from xmodule.partitions.partitions import UserPartitionError, NoSuchUserPartitionGroupError diff --git a/openedx/core/djangoapps/user_api/preferences/__init__.py b/openedx/core/djangoapps/user_api/preferences/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/preferences/api.py b/openedx/core/djangoapps/user_api/preferences/api.py new file mode 100644 index 0000000000..c3a2b3805d --- /dev/null +++ b/openedx/core/djangoapps/user_api/preferences/api.py @@ -0,0 +1,391 @@ +""" +API for managing user preferences. +""" +import datetime +import logging +import string +import analytics +from eventtracking import tracker +from pytz import UTC + +from django.conf import settings +from django.contrib.auth.models import User +from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction, IntegrityError +from django.utils.translation import ugettext as _ + +from student.models import UserProfile + +from ..errors import ( + UserAPIInternalError, UserAPIRequestError, UserNotFound, UserNotAuthorized, + PreferenceValidationError, PreferenceUpdateError +) +from ..helpers import intercept_errors +from ..models import UserOrgTag, UserPreference +from ..serializers import UserSerializer, RawUserPreferenceSerializer + +log = logging.getLogger(__name__) + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +def get_user_preference(requesting_user, preference_key, username=None): + """Returns the value of the user preference with the specified key. + + Args: + requesting_user (User): The user requesting the user preferences. Only the user with username + `username` or users with "is_staff" privileges can access the preferences. + preference_key (string): The key for the user preference. + username (str): Optional username for which to look up the preferences. If not specified, + `requesting_user.username` is assumed. + + Returns: + The value for the user preference which is always a string, or None if a preference + has not been specified. + + Raises: + UserNotFound: no user with username `username` exists (or `requesting_user.username` if + `username` is not specified) + UserNotAuthorized: the requesting_user does not have access to the user preference. + UserAPIInternalError: the operation failed due to an unexpected error. + """ + existing_user = _get_user(requesting_user, username, allow_staff=True) + return UserPreference.get_value(existing_user, preference_key) + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +def get_user_preferences(requesting_user, username=None): + """Returns all user preferences as a JSON response. + + Args: + requesting_user (User): The user requesting the user preferences. Only the user with username + `username` or users with "is_staff" privileges can access the preferences. + username (str): Optional username for which to look up the preferences. If not specified, + `requesting_user.username` is assumed. + + Returns: + A dict containing account fields. + + Raises: + UserNotFound: no user with username `username` exists (or `requesting_user.username` if + `username` is not specified) + UserNotAuthorized: the requesting_user does not have access to the user preference. + UserAPIInternalError: the operation failed due to an unexpected error. + """ + existing_user = _get_user(requesting_user, username, allow_staff=True) + user_serializer = UserSerializer(existing_user) + return user_serializer.data["preferences"] + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +@transaction.commit_on_success +def update_user_preferences(requesting_user, update, username=None): + """Update the user preferences for the given username. + + Note: + It is up to the caller of this method to enforce the contract that this method is only called + with the user who made the request. + + Arguments: + requesting_user (User): The user requesting to modify account information. Only the user with username + 'username' has permissions to modify account information. + update (dict): The updated account field values. + Some notes: + Values are expected to be strings. Non-string values will be converted to strings. + Null values for a preference will be treated as a request to delete the key in question. + username (string): Optional username specifying which account should be updated. If not specified, + `requesting_user.username` is assumed. + + Raises: + UserNotFound: no user with username `username` exists (or `requesting_user.username` if + `username` is not specified) + UserNotAuthorized: the requesting_user does not have access to change the account + associated with `username` + PreferenceValidationError: the update was not attempted because validation errors were found + PreferenceUpdateError: the operation failed when performing the update. + UserAPIInternalError: the operation failed due to an unexpected error. + """ + existing_user = _get_user(requesting_user, username) + + # First validate each preference setting + errors = {} + serializers = {} + for preference_key in update.keys(): + preference_value = update[preference_key] + if preference_value is not None: + try: + serializer = create_user_preference_serializer(existing_user, preference_key, preference_value) + validate_user_preference_serializer(serializer, preference_key, preference_value) + serializers[preference_key] = serializer + except PreferenceValidationError as error: + preference_error = error.preference_errors[preference_key] + errors[preference_key] = { + "developer_message": preference_error["developer_message"], + "user_message": preference_error["user_message"], + } + if errors: + raise PreferenceValidationError(errors) + # Then perform the patch + for preference_key in update.keys(): + preference_value = update[preference_key] + if preference_value is not None: + try: + serializer = serializers[preference_key] + serializer.save() + except Exception as error: + raise _create_preference_update_error(preference_key, preference_value, error) + else: + delete_user_preference(requesting_user, preference_key) + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +@transaction.commit_on_success +def set_user_preference(requesting_user, preference_key, preference_value, username=None): + """Update a user preference for the given username. + + Note: + It is up to the caller of this method to enforce the contract that this method is only called + with the user who made the request. + + Arguments: + requesting_user (User): The user requesting to modify account information. Only the user with username + 'username' has permissions to modify account information. + preference_key (string): The key for the user preference. + preference_value (string): The value to be stored. Non-string values will be converted to strings. + username (string): Optional username specifying which account should be updated. If not specified, + `requesting_user.username` is assumed. + + Raises: + UserNotFound: no user with username `username` exists (or `requesting_user.username` if + `username` is not specified) + UserNotAuthorized: the requesting_user does not have access to change the account + associated with `username` + PreferenceValidationError: the update was not attempted because validation errors were found + PreferenceUpdateError: the operation failed when performing the update. + UserAPIInternalError: the operation failed due to an unexpected error. + """ + existing_user = _get_user(requesting_user, username) + serializer = create_user_preference_serializer(existing_user, preference_key, preference_value) + validate_user_preference_serializer(serializer, preference_key, preference_value) + try: + serializer.save() + except Exception as error: + raise _create_preference_update_error(preference_key, preference_value, error) + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +@transaction.commit_on_success +def delete_user_preference(requesting_user, preference_key, username=None): + """Deletes a user preference on behalf of a requesting user. + + Note: + It is up to the caller of this method to enforce the contract that this method is only called + with the user who made the request. + + Arguments: + requesting_user (User): The user requesting to delete the preference. Only the user with username + 'username' has permissions to delete their own preference. + preference_key (string): The key for the user preference. + username (string): Optional username specifying which account should be updated. If not specified, + `requesting_user.username` is assumed. + + Returns: + True if the preference was deleted, False if the user did not have a preference with the supplied key. + + Raises: + UserNotFound: no user with username `username` exists (or `requesting_user.username` if + `username` is not specified) + UserNotAuthorized: the requesting_user does not have access to change the account + associated with `username` + PreferenceUpdateError: the operation failed when performing the update. + UserAPIInternalError: the operation failed due to an unexpected error. + """ + existing_user = _get_user(requesting_user, username) + try: + user_preference = UserPreference.objects.get(user=existing_user, key=preference_key) + except ObjectDoesNotExist: + return False + + try: + user_preference.delete() + except Exception as error: + raise PreferenceUpdateError( + developer_message=u"Delete failed for user preference '{preference_key}': {error}".format( + preference_key=preference_key, error=error + ), + user_message=_(u"Delete failed for user preference '{preference_key}'.").format( + preference_key=preference_key + ), + ) + return True + + +@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) +def update_email_opt_in(user, org, optin): + """Updates a user's preference for receiving org-wide emails. + + Sets a User Org Tag defining the choice to opt in or opt out of organization-wide + emails. + + Arguments: + user (User): The user to set a preference for. + org (str): The org is used to determine the organization this setting is related to. + optin (Boolean): True if the user is choosing to receive emails for this organization. If the user is not + the correct age to receive emails, email-optin is set to False regardless. + + Returns: + None + + """ + # Avoid calling get_account_settings because it introduces circularity for many callers who need both + # preferences and account information. + try: + user_profile = UserProfile.objects.get(user=user) + except ObjectDoesNotExist: + raise UserNotFound() + + year_of_birth = user_profile.year_of_birth + of_age = ( + year_of_birth is None or # If year of birth is not set, we assume user is of age. + datetime.datetime.now(UTC).year - year_of_birth > # pylint: disable=maybe-no-member + getattr(settings, 'EMAIL_OPTIN_MINIMUM_AGE', 13) + ) + + try: + preference, _ = UserOrgTag.objects.get_or_create( + user=user, org=org, key='email-optin' + ) + preference.value = str(optin and of_age) + preference.save() + + if settings.FEATURES.get('SEGMENT_IO_LMS') and settings.SEGMENT_IO_LMS_KEY: + _track_update_email_opt_in(user.id, org, optin) + + except IntegrityError as err: + log.warn(u"Could not update organization wide preference due to IntegrityError: {}".format(err.message)) + + +def _track_update_email_opt_in(user_id, organization, opt_in): + """Track an email opt-in preference change. + + Arguments: + user_id (str): The ID of the user making the preference change. + organization (str): The organization whose emails are being opted into or out of by the user. + opt_in (Boolean): Whether the user has chosen to opt-in to emails from the organization. + + Returns: + None + + """ + event_name = 'edx.bi.user.org_email.opted_in' if opt_in else 'edx.bi.user.org_email.opted_out' + tracking_context = tracker.get_tracker().resolve_context() + + analytics.track( + user_id, + event_name, + { + 'category': 'communication', + 'label': organization + }, + context={ + 'Google Analytics': { + 'clientId': tracking_context.get('client_id') + } + } + ) + + +def _get_user(requesting_user, username=None, allow_staff=False): + """ + Helper method to return the user for a given username. + If username is not provided, requesting_user.username is assumed. + """ + if username is None: + username = requesting_user.username + + try: + existing_user = User.objects.get(username=username) + except ObjectDoesNotExist: + raise UserNotFound() + + if requesting_user.username != username: + if not requesting_user.is_staff or not allow_staff: + raise UserNotAuthorized() + + return existing_user + + +def create_user_preference_serializer(user, preference_key, preference_value): + """Creates a serializer for the specified user preference. + + Arguments: + user (User): The user whose preference is being serialized. + preference_key (string): The key for the user preference. + preference_value (string): The value to be stored. Non-string values will be converted to strings. + + Returns: + A serializer that can be used to save the user preference. + """ + try: + existing_user_preference = UserPreference.objects.get(user=user, key=preference_key) + except ObjectDoesNotExist: + existing_user_preference = None + new_data = { + "user": user.id, + "key": preference_key, + "value": preference_value, + } + if existing_user_preference: + serializer = RawUserPreferenceSerializer(existing_user_preference, data=new_data) + else: + serializer = RawUserPreferenceSerializer(data=new_data) + return serializer + + +def validate_user_preference_serializer(serializer, preference_key, preference_value): + """Validates a user preference serializer. + + Arguments: + serializer (UserPreferenceSerializer): The serializer to be validated. + preference_key (string): The key for the user preference. + preference_value (string): The value to be stored. Non-string values will be converted to strings. + + Raises: + PreferenceValidationError: the supplied key and/or value for a user preference are invalid. + """ + if preference_value is None or unicode(preference_value).strip() == '': + message = _(u"Preference '{preference_key}' cannot be set to an empty value.").format( + preference_key=preference_key + ) + raise PreferenceValidationError({ + preference_key: {"developer_message": message, "user_message": message} + }) + if not serializer.is_valid(): + developer_message = u"Value '{preference_value}' not valid for preference '{preference_key}': {error}".format( + preference_key=preference_key, preference_value=preference_value, error=serializer.errors + ) + if serializer.errors["key"]: + user_message = _(u"Invalid user preference key '{preference_key}'.").format( + preference_key=preference_key + ) + else: + user_message = _(u"Value '{preference_value}' is not valid for user preference '{preference_key}'.").format( + preference_key=preference_key, preference_value=preference_value + ) + raise PreferenceValidationError({ + preference_key: { + "developer_message": developer_message, + "user_message": user_message, + } + }) + + +def _create_preference_update_error(preference_key, preference_value, error): + """ Creates a PreferenceUpdateError with developer_message and user_message. """ + return PreferenceUpdateError( + developer_message=u"Save failed for user preference '{key}' with value '{value}': {error}".format( + key=preference_key, value=preference_value, error=error + ), + user_message=_(u"Save failed for user preference '{key}' with value '{value}'.").format( + key=preference_key, value=preference_value + ), + ) diff --git a/openedx/core/djangoapps/user_api/preferences/tests/__init__.py b/openedx/core/djangoapps/user_api/preferences/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/preferences/tests/test_api.py b/openedx/core/djangoapps/user_api/preferences/tests/test_api.py new file mode 100644 index 0000000000..a2949a206e --- /dev/null +++ b/openedx/core/djangoapps/user_api/preferences/tests/test_api.py @@ -0,0 +1,415 @@ +# -*- coding: utf-8 -*- +""" +Unit tests for preference APIs. +""" +import datetime +import ddt +import unittest +from mock import patch +from pytz import UTC + +from django.conf import settings +from django.contrib.auth.models import User +from django.test import TestCase +from django.test.utils import override_settings +from dateutil.parser import parse as parse_datetime + +from student.tests.factories import UserFactory + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from ...accounts.api import create_account +from ...errors import UserNotFound, UserNotAuthorized, PreferenceValidationError, PreferenceUpdateError +from ...models import UserProfile, UserOrgTag +from ...preferences.api import ( + get_user_preference, get_user_preferences, set_user_preference, update_user_preferences, delete_user_preference, + update_email_opt_in +) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestPreferenceAPI(TestCase): + """ + These tests specifically cover the parts of the API methods that are not covered by test_views.py. + This includes the specific types of error raised, and default behavior when optional arguments + are not specified. + """ + password = "test" + + def setUp(self): + super(TestPreferenceAPI, self).setUp() + self.user = UserFactory.create(password=self.password) + self.different_user = UserFactory.create(password=self.password) + self.staff_user = UserFactory(is_staff=True, password=self.password) + self.no_such_user = UserFactory.create(password=self.password) + self.no_such_user.username = "no_such_user" + self.test_preference_key = "test_key" + self.test_preference_value = "test_value" + set_user_preference(self.user, self.test_preference_key, self.test_preference_value) + + def test_get_user_preference(self): + """ + Verifies the basic behavior of get_user_preference. + """ + self.assertEqual( + get_user_preference(self.user, self.test_preference_key), + self.test_preference_value + ) + self.assertEqual( + get_user_preference(self.staff_user, self.test_preference_key, username=self.user.username), + self.test_preference_value + ) + + def test_get_user_preference_errors(self): + """ + Verifies that get_user_preference returns appropriate errors. + """ + with self.assertRaises(UserNotFound): + get_user_preference(self.user, self.test_preference_key, username="no_such_user") + + with self.assertRaises(UserNotFound): + get_user_preference(self.no_such_user, self.test_preference_key) + + with self.assertRaises(UserNotAuthorized): + get_user_preference(self.different_user, self.test_preference_key, username=self.user.username) + + def test_get_user_preferences(self): + """ + Verifies the basic behavior of get_user_preferences. + """ + expected_user_preferences = { + self.test_preference_key: self.test_preference_value, + } + self.assertEqual(get_user_preferences(self.user), expected_user_preferences) + self.assertEqual(get_user_preferences(self.staff_user, username=self.user.username), expected_user_preferences) + + def test_get_user_preferences_errors(self): + """ + Verifies that get_user_preferences returns appropriate errors. + """ + with self.assertRaises(UserNotFound): + get_user_preferences(self.user, username="no_such_user") + + with self.assertRaises(UserNotFound): + get_user_preferences(self.no_such_user) + + with self.assertRaises(UserNotAuthorized): + get_user_preferences(self.different_user, username=self.user.username) + + def test_set_user_preference(self): + """ + Verifies the basic behavior of set_user_preference. + """ + test_key = u'ⓟⓡⓔⓕⓔⓡⓔⓝⓒⓔ_ⓚⓔⓨ' + test_value = u'ǝnןɐʌ_ǝɔuǝɹǝɟǝɹd' + set_user_preference(self.user, test_key, test_value) + self.assertEqual(get_user_preference(self.user, test_key), test_value) + set_user_preference(self.user, test_key, "new_value", username=self.user.username) + self.assertEqual(get_user_preference(self.user, test_key), "new_value") + + @patch('openedx.core.djangoapps.user_api.models.UserPreference.save') + def test_set_user_preference_errors(self, user_preference_save): + """ + Verifies that set_user_preference returns appropriate errors. + """ + with self.assertRaises(UserNotFound): + set_user_preference(self.user, self.test_preference_key, "new_value", username="no_such_user") + + with self.assertRaises(UserNotFound): + set_user_preference(self.no_such_user, self.test_preference_key, "new_value") + + with self.assertRaises(UserNotAuthorized): + set_user_preference(self.staff_user, self.test_preference_key, "new_value", username=self.user.username) + + with self.assertRaises(UserNotAuthorized): + set_user_preference(self.different_user, self.test_preference_key, "new_value", username=self.user.username) + + too_long_key = "x" * 256 + with self.assertRaises(PreferenceValidationError) as context_manager: + set_user_preference(self.user, too_long_key, "new_value") + errors = context_manager.exception.preference_errors + self.assertEqual(len(errors.keys()), 1) + self.assertEqual( + errors[too_long_key], + { + "developer_message": get_expected_validation_developer_message(too_long_key, "new_value"), + "user_message": get_expected_key_error_user_message(too_long_key, "new_value"), + } + ) + + for empty_value in (None, "", " "): + with self.assertRaises(PreferenceValidationError) as context_manager: + set_user_preference(self.user, self.test_preference_key, empty_value) + errors = context_manager.exception.preference_errors + self.assertEqual(len(errors.keys()), 1) + self.assertEqual( + errors[self.test_preference_key], + { + "developer_message": get_empty_preference_message(self.test_preference_key), + "user_message": get_empty_preference_message(self.test_preference_key), + } + ) + + user_preference_save.side_effect = [Exception, None] + with self.assertRaises(PreferenceUpdateError) as context_manager: + set_user_preference(self.user, u"new_key_ȻħȺɍłɇs", u"new_value_ȻħȺɍłɇs") + self.assertEqual( + context_manager.exception.developer_message, + u"Save failed for user preference 'new_key_ȻħȺɍłɇs' with value 'new_value_ȻħȺɍłɇs': " + ) + self.assertEqual( + context_manager.exception.user_message, + u"Save failed for user preference 'new_key_ȻħȺɍłɇs' with value 'new_value_ȻħȺɍłɇs'." + ) + + def test_update_user_preferences(self): + """ + Verifies the basic behavior of update_user_preferences. + """ + expected_user_preferences = { + self.test_preference_key: "new_value", + } + set_user_preference(self.user, self.test_preference_key, "new_value") + self.assertEqual( + get_user_preference(self.user, self.test_preference_key), + "new_value" + ) + set_user_preference(self.user, self.test_preference_key, "new_value", username=self.user.username) + self.assertEqual( + get_user_preference(self.user, self.test_preference_key), + "new_value" + ) + + @patch('openedx.core.djangoapps.user_api.models.UserPreference.delete') + @patch('openedx.core.djangoapps.user_api.models.UserPreference.save') + def test_update_user_preferences_errors(self, user_preference_save, user_preference_delete): + """ + Verifies that set_user_preferences returns appropriate errors. + """ + update_data = { + self.test_preference_key: "new_value" + } + with self.assertRaises(UserNotFound): + update_user_preferences(self.user, update_data, username="no_such_user") + + with self.assertRaises(UserNotFound): + update_user_preferences(self.no_such_user, update_data) + + with self.assertRaises(UserNotAuthorized): + update_user_preferences(self.staff_user, update_data, username=self.user.username) + + with self.assertRaises(UserNotAuthorized): + update_user_preferences(self.different_user, update_data, username=self.user.username) + + too_long_key = "x" * 256 + with self.assertRaises(PreferenceValidationError) as context_manager: + update_user_preferences(self.user, { too_long_key: "new_value"}) + errors = context_manager.exception.preference_errors + self.assertEqual(len(errors.keys()), 1) + self.assertEqual( + errors[too_long_key], + { + "developer_message": get_expected_validation_developer_message(too_long_key, "new_value"), + "user_message": get_expected_key_error_user_message(too_long_key, "new_value"), + } + ) + + for empty_value in ("", " "): + with self.assertRaises(PreferenceValidationError) as context_manager: + update_user_preferences(self.user, { self.test_preference_key: empty_value}) + errors = context_manager.exception.preference_errors + self.assertEqual(len(errors.keys()), 1) + self.assertEqual( + errors[self.test_preference_key], + { + "developer_message": get_empty_preference_message(self.test_preference_key), + "user_message": get_empty_preference_message(self.test_preference_key), + } + ) + + user_preference_save.side_effect = [Exception, None] + with self.assertRaises(PreferenceUpdateError) as context_manager: + update_user_preferences(self.user, { self.test_preference_key: "new_value"}) + self.assertEqual( + context_manager.exception.developer_message, + u"Save failed for user preference 'test_key' with value 'new_value': " + ) + self.assertEqual( + context_manager.exception.user_message, + u"Save failed for user preference 'test_key' with value 'new_value'." + ) + + user_preference_delete.side_effect = [Exception, None] + with self.assertRaises(PreferenceUpdateError) as context_manager: + update_user_preferences(self.user, {self.test_preference_key: None}) + self.assertEqual( + context_manager.exception.developer_message, + u"Delete failed for user preference 'test_key': " + ) + self.assertEqual( + context_manager.exception.user_message, + u"Delete failed for user preference 'test_key'." + ) + + def test_delete_user_preference(self): + """ + Verifies the basic behavior of delete_user_preference. + """ + self.assertTrue(delete_user_preference(self.user, self.test_preference_key)) + set_user_preference(self.user, self.test_preference_key, self.test_preference_value) + self.assertTrue(delete_user_preference(self.user, self.test_preference_key, username=self.user.username)) + self.assertFalse(delete_user_preference(self.user, "no_such_key")) + + @patch('openedx.core.djangoapps.user_api.models.UserPreference.delete') + def test_delete_user_preference_errors(self, user_preference_delete): + """ + Verifies that delete_user_preference returns appropriate errors. + """ + with self.assertRaises(UserNotFound): + delete_user_preference(self.user, self.test_preference_key, username="no_such_user") + + with self.assertRaises(UserNotFound): + delete_user_preference(self.no_such_user, self.test_preference_key) + + with self.assertRaises(UserNotAuthorized): + delete_user_preference(self.staff_user, self.test_preference_key, username=self.user.username) + + with self.assertRaises(UserNotAuthorized): + delete_user_preference(self.different_user, self.test_preference_key, username=self.user.username) + + user_preference_delete.side_effect = [Exception, None] + with self.assertRaises(PreferenceUpdateError) as context_manager: + delete_user_preference(self.user, self.test_preference_key) + self.assertEqual( + context_manager.exception.developer_message, + u"Delete failed for user preference 'test_key': " + ) + self.assertEqual( + context_manager.exception.user_message, + u"Delete failed for user preference 'test_key'." + ) + + +@ddt.ddt +class UpdateEmailOptInTests(ModuleStoreTestCase): + + USERNAME = u'frank-underwood' + PASSWORD = u'ṕáśśẃőŕd' + EMAIL = u'frank+underwood@example.com' + + @ddt.data( + # Check that a 27 year old can opt-in + (27, True, u"True"), + + # Check that a 32-year old can opt-out + (32, False, u"False"), + + # Check that someone 14 years old can opt-in + (14, True, u"True"), + + # Check that someone 13 years old cannot opt-in (must have turned 13 before this year) + (13, True, u"False"), + + # Check that someone 12 years old cannot opt-in + (12, True, u"False") + ) + @ddt.unpack + @override_settings(EMAIL_OPTIN_MINIMUM_AGE=13) + def test_update_email_optin(self, age, option, expected_result): + # Create the course and account. + course = CourseFactory.create() + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + + # Set year of birth + user = User.objects.get(username=self.USERNAME) + profile = UserProfile.objects.get(user=user) + year_of_birth = datetime.datetime.now().year - age # pylint: disable=maybe-no-member + profile.year_of_birth = year_of_birth + profile.save() + + update_email_opt_in(user, course.id.org, option) + result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') + self.assertEqual(result_obj.value, expected_result) + + def test_update_email_optin_no_age_set(self): + # Test that the API still works if no age is specified. + # Create the course and account. + course = CourseFactory.create() + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + + user = User.objects.get(username=self.USERNAME) + + update_email_opt_in(user, course.id.org, True) + result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') + self.assertEqual(result_obj.value, u"True") + + @ddt.data( + # Check that a 27 year old can opt-in, then out. + (27, True, False, u"False"), + + # Check that a 32-year old can opt-out, then in. + (32, False, True, u"True"), + + # Check that someone 13 years old can opt-in, then out. + (13, True, False, u"False"), + + # Check that someone 12 years old cannot opt-in, then explicitly out. + (12, True, False, u"False") + ) + @ddt.unpack + @override_settings(EMAIL_OPTIN_MINIMUM_AGE=13) + def test_change_email_optin(self, age, option, second_option, expected_result): + # Create the course and account. + course = CourseFactory.create() + create_account(self.USERNAME, self.PASSWORD, self.EMAIL) + + # Set year of birth + user = User.objects.get(username=self.USERNAME) + profile = UserProfile.objects.get(user=user) + year_of_birth = datetime.datetime.now(UTC).year - age # pylint: disable=maybe-no-member + profile.year_of_birth = year_of_birth + profile.save() + + update_email_opt_in(user, course.id.org, option) + update_email_opt_in(user, course.id.org, second_option) + + result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') + self.assertEqual(result_obj.value, expected_result) + + def _assert_is_datetime(self, timestamp): + if not timestamp: + return False + try: + parse_datetime(timestamp) + except ValueError: + return False + else: + return True + + +def get_expected_validation_developer_message(preference_key, preference_value): + """ + Returns the expected dict of validation messages for the specified key. + """ + return u"Value '{preference_value}' not valid for preference '{preference_key}': {error}".format( + preference_key=preference_key, + preference_value=preference_value, + error={ + "key": [u"Ensure this value has at most 255 characters (it has 256)."] + } + ) + + +def get_expected_key_error_user_message(preference_key, preference_value): + """ + Returns the expected user message for an invalid key. + """ + return u"Invalid user preference key '{preference_key}'.".format(preference_key=preference_key) + + +def get_empty_preference_message(preference_key): + """ + Returns the validation message shown for an empty preference. + """ + return "Preference '{preference_key}' cannot be set to an empty value.".format(preference_key=preference_key) diff --git a/openedx/core/djangoapps/user_api/preferences/tests/test_views.py b/openedx/core/djangoapps/user_api/preferences/tests/test_views.py new file mode 100644 index 0000000000..3a9fac1423 --- /dev/null +++ b/openedx/core/djangoapps/user_api/preferences/tests/test_views.py @@ -0,0 +1,531 @@ +# -*- coding: utf-8 -*- +""" +Unit tests for preference APIs. +""" + +import unittest +import ddt +import json + +from django.core.urlresolvers import reverse +from django.conf import settings + +from ...accounts.tests.test_views import UserAPITestCase +from ..api import set_user_preference +from .test_api import get_expected_validation_developer_message, get_expected_key_error_user_message + +TOO_LONG_PREFERENCE_KEY = u"x" * 256 + + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class TestPreferencesAPI(UserAPITestCase): + """ + Unit tests /api/user/v0/accounts/{username}/ + """ + def setUp(self): + super(TestPreferencesAPI, self).setUp() + self.url_endpoint_name = "preferences_api" + self.url = reverse(self.url_endpoint_name, kwargs={'username': self.user.username}) + + def test_anonymous_access(self): + """ + Test that an anonymous client (not logged in) cannot call GET or PATCH. + """ + self.send_get(self.anonymous_client, expected_status=401) + self.send_patch(self.anonymous_client, {}, expected_status=401) + + def test_unsupported_methods(self): + """ + Test that DELETE, POST, and PUT are not supported. + """ + self.client.login(username=self.user.username, password=self.test_password) + self.assertEqual(405, self.client.put(self.url).status_code) + self.assertEqual(405, self.client.post(self.url).status_code) + self.assertEqual(405, self.client.delete(self.url).status_code) + + def test_get_different_user(self): + """ + Test that a client (logged in) cannot get the preferences information for a different client. + """ + self.different_client.login(username=self.different_user.username, password=self.test_password) + self.send_get(self.different_client, expected_status=404) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_get_unknown_user(self, api_client, username): + """ + Test that requesting a user who does not exist returns a 404. + """ + client = self.login_client(api_client, username) + response = client.get(reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist"})) + self.assertEqual(404, response.status_code) + + def test_get_preferences_default(self): + """ + Test that a client (logged in) can get her own preferences information (verifying the default + state before any preferences are stored). + """ + self.client.login(username=self.user.username, password=self.test_password) + response = self.send_get(self.client) + self.assertEqual({}, response.data) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_get_preferences(self, api_client, user): + """ + Test that a client (logged in) can get her own preferences information. Also verifies that a "is_staff" + user can get the preferences information for other users. + """ + # Create some test preferences values. + set_user_preference(self.user, "dict_pref", {"int_key": 10}) + set_user_preference(self.user, "string_pref", "value") + + # Log in the client and do the GET. + client = self.login_client(api_client, user) + response = self.send_get(client) + self.assertEqual({"dict_pref": "{'int_key': 10}", "string_pref": "value"}, response.data) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_patch_unknown_user(self, api_client, user): + """ + Test that trying to update preferences for a user who does not exist returns a 404. + """ + client = self.login_client(api_client, user) + response = client.patch( + reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist"}), + data=json.dumps({"string_pref": "value"}), content_type="application/merge-patch+json" + ) + self.assertEqual(404, response.status_code) + + def test_patch_bad_content_type(self): + """ + Test the behavior of patch when an incorrect content_type is specified. + """ + self.client.login(username=self.user.username, password=self.test_password) + self.send_patch(self.client, {}, content_type="application/json", expected_status=415) + self.send_patch(self.client, {}, content_type="application/xml", expected_status=415) + + def test_create_preferences(self): + """ + Test that a client (logged in) can create her own preferences information. + """ + self._do_create_preferences_test(True) + + def test_create_preferences_inactive(self): + """ + Test that a client (logged in but not active) can create her own preferences information. + """ + self._do_create_preferences_test(False) + + def _do_create_preferences_test(self, is_active): + self.client.login(username=self.user.username, password=self.test_password) + if not is_active: + self.user.is_active = False + self.user.save() + self.send_patch(self.client, { + "dict_pref": {"int_key": 10}, + "string_pref": "value", + }) + response = self.send_get(self.client) + self.assertEqual({u"dict_pref": u"{u'int_key': 10}", u"string_pref": u"value"}, response.data) + + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_create_preferences_other_user(self, api_client, user): + """ + Test that a client (logged in) cannot create preferences for another user. + """ + client = self.login_client(api_client, user) + self.send_patch( + client, + { + "dict_pref": {"int_key": 10}, + "string_pref": "value", + }, + expected_status=404, + ) + + def test_update_preferences(self): + """ + Test that a client (logged in) can update her own preferences information. + """ + # Create some test preferences values. + set_user_preference(self.user, "dict_pref", {"int_key": 10}) + set_user_preference(self.user, "string_pref", "value") + set_user_preference(self.user, "extra_pref", "extra_value") + + # Send the patch request + self.client.login(username=self.user.username, password=self.test_password) + self.send_patch(self.client, { + "string_pref": "updated_value", + "new_pref": "new_value", + "extra_pref": None, + }) + + # Verify that GET returns the updated preferences + response = self.send_get(self.client) + expected_preferences = { + "dict_pref": "{'int_key': 10}", + "string_pref": "updated_value", + "new_pref": "new_value", + } + self.assertEqual(expected_preferences, response.data) + + def test_update_preferences_bad_data(self): + """ + Test that a client (logged in) receives appropriate errors for a bad update. + """ + # Create some test preferences values. + set_user_preference(self.user, "dict_pref", {"int_key": 10}) + set_user_preference(self.user, "string_pref", "value") + set_user_preference(self.user, "extra_pref", "extra_value") + + # Send the patch request + self.client.login(username=self.user.username, password=self.test_password) + response = self.send_patch( + self.client, + { + "string_pref": "updated_value", + TOO_LONG_PREFERENCE_KEY: "new_value", + "new_pref": "new_value", + u"empty_pref_ȻħȺɍłɇs": "", + }, + expected_status=400 + ) + self.assertTrue(response.data.get("field_errors", None)) + field_errors = response.data["field_errors"] + self.assertEquals( + field_errors, + { + TOO_LONG_PREFERENCE_KEY: { + "developer_message": get_expected_validation_developer_message( + TOO_LONG_PREFERENCE_KEY, "new_value" + ), + "user_message": get_expected_key_error_user_message( + TOO_LONG_PREFERENCE_KEY, "new_value" + ), + }, + u"empty_pref_ȻħȺɍłɇs": { + "developer_message": u"Preference 'empty_pref_ȻħȺɍłɇs' cannot be set to an empty value.", + "user_message": u"Preference 'empty_pref_ȻħȺɍłɇs' cannot be set to an empty value.", + }, + } + ) + + # Verify that GET returns the original preferences + response = self.send_get(self.client) + expected_preferences = { + u"dict_pref": u"{'int_key': 10}", + u"string_pref": u"value", + u"extra_pref": u"extra_value", + } + self.assertEqual(expected_preferences, response.data) + + def test_update_preferences_bad_request(self): + """ + Test that a client (logged in) receives appropriate errors for a bad request. + """ + self.client.login(username=self.user.username, password=self.test_password) + + # Verify a non-dict request + response = self.send_patch(self.client, "non_dict_request", expected_status=400) + self.assertEqual( + response.data, + { + "developer_message": u"No data provided for user preference update", + "user_message": u"No data provided for user preference update" + } + ) + + # Verify an empty dict request + response = self.send_patch(self.client, {}, expected_status=400) + self.assertEqual( + response.data, + { + "developer_message": u"No data provided for user preference update", + "user_message": u"No data provided for user preference update" + } + ) + + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_update_preferences_other_user(self, api_client, user): + """ + Test that a client (logged in) cannot update preferences for another user. + """ + # Create some test preferences values. + set_user_preference(self.user, "dict_pref", {"int_key": 10}) + set_user_preference(self.user, "string_pref", "value") + set_user_preference(self.user, "extra_pref", "extra_value") + + # Send the patch request + client = self.login_client(api_client, user) + self.send_patch( + client, + { + "string_pref": "updated_value", + "new_pref": "new_value", + "extra_pref": None, + }, + expected_status=404 + ) + + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class TestPreferencesDetailAPI(UserAPITestCase): + """ + Unit tests /api/user/v0/accounts/{username}/{preference_key} + """ + def setUp(self): + super(TestPreferencesDetailAPI, self).setUp() + self.test_pref_key = "test_key" + self.test_pref_value = "test_value" + set_user_preference(self.user, self.test_pref_key, self.test_pref_value) + self.url_endpoint_name = "preferences_detail_api" + self._set_url(self.test_pref_key) + + def _set_url(self, preference_key): + self.url = reverse( + self.url_endpoint_name, + kwargs={'username': self.user.username, 'preference_key': preference_key} + ) + + def test_anonymous_user_access(self): + """ + Test that an anonymous client (logged in) cannot manipulate preferences. + """ + self.send_get(self.anonymous_client, expected_status=401) + self.send_put(self.anonymous_client, "new_value", expected_status=401) + self.send_delete(self.anonymous_client, expected_status=401) + + def test_unsupported_methods(self): + """ + Test that POST and PATCH are not supported. + """ + self.client.login(username=self.user.username, password=self.test_password) + self.assertEqual(405, self.client.post(self.url).status_code) + self.assertEqual(405, self.client.patch(self.url).status_code) + + def test_different_user_access(self): + """ + Test that a client (logged in) cannot manipulate a preference for a different client. + """ + self.different_client.login(username=self.different_user.username, password=self.test_password) + self.send_get(self.different_client, expected_status=404) + self.send_put(self.different_client, "new_value", expected_status=404) + self.send_delete(self.different_client, expected_status=404) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_get_unknown_user(self, api_client, username): + """ + Test that requesting a user who does not exist returns a 404. + """ + client = self.login_client(api_client, username) + response = client.get( + reverse(self.url_endpoint_name, kwargs={'username': "does_not_exist", 'preference_key': self.test_pref_key}) + ) + self.assertEqual(404, response.status_code) + + def test_get_preference_does_not_exist(self): + """ + Test that a 404 is returned if the user does not have a preference with the given preference_key. + """ + self._set_url("does_not_exist") + self.client.login(username=self.user.username, password=self.test_password) + response = self.send_get(self.client, expected_status=404) + self.assertIsNone(response.data) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_get_preference(self, api_client, user): + """ + Test that a client (logged in) can get her own preferences information. Also verifies that a "is_staff" + user can get the preferences information for other users. + """ + client = self.login_client(api_client, user) + response = self.send_get(client) + self.assertEqual(self.test_pref_value, response.data) + + # Test a different value. + set_user_preference(self.user, "dict_pref", {"int_key": 10}) + self._set_url("dict_pref") + response = self.send_get(client) + self.assertEqual("{'int_key': 10}", response.data) + + def test_create_preference(self): + """ + Test that a client (logged in) can create a preference. + """ + self._do_create_preference_test(True) + + def test_create_preference_inactive(self): + """ + Test that a client (logged in but not active) can create a preference. + """ + self._do_create_preference_test(False) + + def _do_create_preference_test(self, is_active): + self.client.login(username=self.user.username, password=self.test_password) + if not is_active: + self.user.is_active = False + self.user.save() + self._set_url("new_key") + new_value = "new value" + self.send_put(self.client, new_value) + response = self.send_get(self.client) + self.assertEqual(new_value, response.data) + + @ddt.data( + (None,), + ("",), + (" ",), + ) + @ddt.unpack + def test_create_empty_preference(self, preference_value): + """ + Test that a client (logged in) cannot create an empty preference. + """ + self._set_url("new_key") + self.client.login(username=self.user.username, password=self.test_password) + response = self.send_put(self.client, preference_value, expected_status=400) + self.assertEqual( + response.data, + { + "developer_message": u"Preference 'new_key' cannot be set to an empty value.", + "user_message": u"Preference 'new_key' cannot be set to an empty value." + } + ) + self.send_get(self.client, expected_status=404) + + def test_create_preference_too_long_key(self): + """ + Test that a client cannot create preferences with bad keys + """ + self.client.login(username=self.user.username, password=self.test_password) + + too_long_preference_key = "x" * 256 + new_value = "new value" + self._set_url(too_long_preference_key) + response = self.send_put(self.client, new_value, expected_status=400) + self.assertEquals( + response.data, + { + "developer_message": get_expected_validation_developer_message(too_long_preference_key, new_value), + "user_message": get_expected_key_error_user_message(too_long_preference_key, new_value), + } + ) + + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_create_preference_other_user(self, api_client, user): + """ + Test that a client (logged in) cannot create a preference for a different user. + """ + # Verify that a new preference cannot be created + self._set_url("new_key") + client = self.login_client(api_client, user) + new_value = "new value" + self.send_put(client, new_value, expected_status=404) + + @ddt.data( + (u"new value",), + (10,), + ({u"int_key": 10},) + ) + @ddt.unpack + def test_update_preference(self, preference_value): + """ + Test that a client (logged in) can update a preference. + """ + self.client.login(username=self.user.username, password=self.test_password) + self.send_put(self.client, preference_value) + response = self.send_get(self.client) + self.assertEqual(unicode(preference_value), response.data) + + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_update_preference_other_user(self, api_client, user): + """ + Test that a client (logged in) cannot update a preference for another user. + """ + client = self.login_client(api_client, user) + new_value = "new value" + self.send_put(client, new_value, expected_status=404) + + @ddt.data( + (None,), + ("",), + (" ",), + ) + @ddt.unpack + def test_update_preference_to_empty(self, preference_value): + """ + Test that a client (logged in) cannot update a preference to null. + """ + self.client.login(username=self.user.username, password=self.test_password) + response = self.send_put(self.client, preference_value, expected_status=400) + self.assertEqual( + response.data, + { + "developer_message": u"Preference 'test_key' cannot be set to an empty value.", + "user_message": u"Preference 'test_key' cannot be set to an empty value." + } + ) + response = self.send_get(self.client) + self.assertEqual(self.test_pref_value, response.data) + + def test_delete_preference(self): + """ + Test that a client (logged in) can delete her own preference. + """ + self.client.login(username=self.user.username, password=self.test_password) + + # Verify that a preference can be deleted + self.send_delete(self.client) + self.send_get(self.client, expected_status=404) + + # Verify that deleting a non-existent preference throws a 404 + self.send_delete(self.client, expected_status=404) + + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_delete_preference_other_user(self, api_client, user): + """ + Test that a client (logged in) cannot delete a preference for another user. + """ + client = self.login_client(api_client, user) + self.send_delete(client, expected_status=404) diff --git a/openedx/core/djangoapps/user_api/preferences/views.py b/openedx/core/djangoapps/user_api/preferences/views.py new file mode 100644 index 0000000000..b9f7544874 --- /dev/null +++ b/openedx/core/djangoapps/user_api/preferences/views.py @@ -0,0 +1,208 @@ +""" +NOTE: this API is WIP and has not yet been approved. Do not use this API without talking to Christina or Andy. + +For more information, see: +https://openedx.atlassian.net/wiki/display/TNL/User+API +""" +from rest_framework.views import APIView +from rest_framework.response import Response +from rest_framework import status +from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser +from rest_framework import permissions + +from django.utils.translation import ugettext as _ + +from openedx.core.lib.api.parsers import MergePatchParser +from ..errors import UserNotFound, UserNotAuthorized, PreferenceValidationError, PreferenceUpdateError +from .api import ( + get_user_preference, get_user_preferences, set_user_preference, update_user_preferences, delete_user_preference +) + + +class PreferencesView(APIView): + """ + **Use Cases** + + Get or update the user's preference information. Updates are only supported through merge patch. + Preference values of null in a patch request are treated as requests to remove the preference. + + **Example Requests**: + + GET /api/user/v0/preferences/{username}/ + + PATCH /api/user/v0/preferences/{username}/ with content_type "application/merge-patch+json" + + **Response Value for GET** + + A JSON dictionary will be returned with key/value pairs (all of type String). + + If a user without "is_staff" access has requested preferences for a different user, + this method returns a 404. + + If the specified username does not exist, this method returns a 404. + + **Response for PATCH** + + Users can only modify their own preferences. If the requesting user does not have username + "username", this method will return with a status of 404. + + This method will also return a 404 if no user exists with username "username". + + If "application/merge-patch+json" is not the specified content_type, this method returns a 415 status. + + If the update could not be completed due to validation errors, this method returns a 400 with all + preference-specific error messages in the "field_errors" field of the returned JSON. + + If the update could not be completed due to failure at the time of update, this method returns a 400 with + specific errors in the returned JSON. + + If the update is successful, a 204 status is returned with no additional content. + + """ + authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser) + permission_classes = (permissions.IsAuthenticated,) + parser_classes = (MergePatchParser,) + + def get(self, request, username): + """ + GET /api/user/v0/preferences/{username}/ + """ + try: + user_preferences = get_user_preferences(request.user, username=username) + except (UserNotFound, UserNotAuthorized): + return Response(status=status.HTTP_404_NOT_FOUND) + + return Response(user_preferences) + + def patch(self, request, username): + """ + PATCH /api/user/v0/preferences/{username}/ + """ + if not request.DATA or not getattr(request.DATA, "keys", None): + error_message = _("No data provided for user preference update") + return Response( + { + "developer_message": error_message, + "user_message": error_message + }, + status=status.HTTP_400_BAD_REQUEST + ) + try: + update_user_preferences(request.user, request.DATA, username=username) + except (UserNotFound, UserNotAuthorized): + return Response(status=status.HTTP_404_NOT_FOUND) + except PreferenceValidationError as error: + return Response( + {"field_errors": error.preference_errors}, + status=status.HTTP_400_BAD_REQUEST + ) + except PreferenceUpdateError as error: + return Response( + { + "developer_message": error.developer_message, + "user_message": error.user_message + }, + status=status.HTTP_400_BAD_REQUEST + ) + return Response(status=status.HTTP_204_NO_CONTENT) + + +class PreferencesDetailView(APIView): + """ + **Use Cases** + + Get, create, update, or delete a specific user preference. + + **Example Requests**: + + GET /api/user/v0/preferences/{username}/{preference_key} + + PUT /api/user/v0/preferences/{username}/{preference_key} + + DELETE /api/user/v0/preferences/{username}/{preference_key} + + **Response Values for GET** + + The preference value will be returned as a JSON string. + + If a user without "is_staff" access has requested preferences for a different user, + this method returns a 404. + + If the specified username or preference does not exist, this method returns a 404. + + **Response Values for PUT** + + A successful put returns a 204 and no content. + + If the specified username or preference does not exist, this method returns a 404. + + **Response for DELETE** + + A successful delete returns a 204 and no content. + + If the specified username or preference does not exist, this method returns a 404. + + """ + authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser) + permission_classes = (permissions.IsAuthenticated,) + + def get(self, request, username, preference_key): + """ + GET /api/user/v0/preferences/{username}/{preference_key} + """ + try: + value = get_user_preference(request.user, preference_key, username=username) + # There was no preference with that key, raise a 404. + if value is None: + return Response(status=status.HTTP_404_NOT_FOUND) + except (UserNotFound, UserNotAuthorized): + return Response(status=status.HTTP_404_NOT_FOUND) + return Response(value) + + def put(self, request, username, preference_key): + """ + PUT /api/user/v0/preferences/{username}/{preference_key} + """ + try: + set_user_preference(request.user, preference_key, request.DATA, username=username) + except (UserNotFound, UserNotAuthorized): + return Response(status=status.HTTP_404_NOT_FOUND) + except PreferenceValidationError as error: + return Response( + { + "developer_message": error.preference_errors[preference_key]["developer_message"], + "user_message": error.preference_errors[preference_key]["user_message"] + }, + status=status.HTTP_400_BAD_REQUEST + ) + except PreferenceUpdateError as error: + return Response( + { + "developer_message": error.developer_message, + "user_message": error.user_message + }, + status=status.HTTP_400_BAD_REQUEST + ) + return Response(status=status.HTTP_204_NO_CONTENT) + + def delete(self, request, username, preference_key): + """ + DELETE /api/user/v0/preferences/{username}/{preference_key} + """ + try: + preference_existed = delete_user_preference(request.user, preference_key, username=username) + except (UserNotFound, UserNotAuthorized): + return Response(status=status.HTTP_404_NOT_FOUND) + except PreferenceUpdateError as error: + return Response( + { + "developer_message": error.developer_message, + "user_message": error.user_message + }, + status=status.HTTP_400_BAD_REQUEST + ) + + if not preference_existed: + return Response(status=status.HTTP_404_NOT_FOUND) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index 7c7227fff6..12ba2c1d92 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/openedx/core/djangoapps/user_api/serializers.py @@ -29,3 +29,13 @@ class UserPreferenceSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = UserPreference depth = 1 + + +class RawUserPreferenceSerializer(serializers.ModelSerializer): + """Serializer that generates a raw representation of a user preference. + """ + user = serializers.PrimaryKeyRelatedField() + + class Meta: + model = UserPreference + depth = 1 diff --git a/openedx/core/djangoapps/user_api/tests/test_account_api.py b/openedx/core/djangoapps/user_api/tests/test_account_api.py deleted file mode 100644 index 3040708dfc..0000000000 --- a/openedx/core/djangoapps/user_api/tests/test_account_api.py +++ /dev/null @@ -1,168 +0,0 @@ -# -*- coding: utf-8 -*- -""" Tests for the account API. """ - -import re -from unittest import skipUnless - -from nose.tools import raises -from mock import patch -import ddt -from dateutil.parser import parse as parse_datetime -from django.core import mail -from django.test import TestCase -from django.conf import settings - -from ..api import account as account_api -from ..models import UserProfile - - -@ddt.ddt -class AccountApiTest(TestCase): - - USERNAME = u'frank-underwood' - PASSWORD = u'ṕáśśẃőŕd' - EMAIL = u'frank+underwood@example.com' - - ORIG_HOST = 'example.com' - IS_SECURE = False - - INVALID_USERNAMES = [ - None, - u'', - u'a', - u'a' * (account_api.USERNAME_MAX_LENGTH + 1), - u'invalid_symbol_@', - u'invalid-unicode_fŕáńḱ', - ] - - INVALID_EMAILS = [ - None, - u'', - u'a', - 'no_domain', - 'no+domain', - '@', - '@domain.com', - 'test@no_extension', - u'fŕáńḱ@example.com', - u'frank@éxáḿṕĺé.ćőḿ', - - # Long email -- subtract the length of the @domain - # except for one character (so we exceed the max length limit) - u'{user}@example.com'.format( - user=(u'e' * (account_api.EMAIL_MAX_LENGTH - 11)) - ) - ] - - INVALID_PASSWORDS = [ - None, - u'', - u'a', - u'a' * (account_api.PASSWORD_MAX_LENGTH + 1) - ] - - def test_activate_account(self): - # Create the account, which is initially inactive - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account = account_api.account_info(self.USERNAME) - self.assertEqual(account, { - 'username': self.USERNAME, - 'email': self.EMAIL, - 'is_active': False - }) - - # Activate the account and verify that it is now active - account_api.activate_account(activation_key) - account = account_api.account_info(self.USERNAME) - self.assertTrue(account['is_active']) - - def test_create_account_duplicate_username(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - with self.assertRaises(account_api.AccountUserAlreadyExists): - account_api.create_account(self.USERNAME, self.PASSWORD, 'different+email@example.com') - - # Email uniqueness constraints were introduced in a database migration, - # which we disable in the unit tests to improve the speed of the test suite. - @skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required") - def test_create_account_duplicate_email(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - with self.assertRaises(account_api.AccountUserAlreadyExists): - account_api.create_account('different_user', self.PASSWORD, self.EMAIL) - - def test_username_too_long(self): - long_username = 'e' * (account_api.USERNAME_MAX_LENGTH + 1) - with self.assertRaises(account_api.AccountUsernameInvalid): - account_api.create_account(long_username, self.PASSWORD, self.EMAIL) - - def test_account_info_no_user(self): - self.assertIs(account_api.account_info('does_not_exist'), None) - - @raises(account_api.AccountEmailInvalid) - @ddt.data(*INVALID_EMAILS) - def test_create_account_invalid_email(self, invalid_email): - account_api.create_account(self.USERNAME, self.PASSWORD, invalid_email) - - @raises(account_api.AccountPasswordInvalid) - @ddt.data(*INVALID_PASSWORDS) - def test_create_account_invalid_password(self, invalid_password): - account_api.create_account(self.USERNAME, invalid_password, self.EMAIL) - - @raises(account_api.AccountPasswordInvalid) - def test_create_account_username_password_equal(self): - # Username and password cannot be the same - account_api.create_account(self.USERNAME, self.USERNAME, self.EMAIL) - - @raises(account_api.AccountRequestError) - @ddt.data(*INVALID_USERNAMES) - def test_create_account_invalid_username(self, invalid_username): - account_api.create_account(invalid_username, self.PASSWORD, self.EMAIL) - - @raises(account_api.AccountNotAuthorized) - def test_activate_account_invalid_key(self): - account_api.activate_account(u'invalid') - - @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') - def test_request_password_change(self): - # Create and activate an account - activation_key = account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - account_api.activate_account(activation_key) - - # Request a password change - account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) - - # Verify that one email message has been sent - self.assertEqual(len(mail.outbox), 1) - - # Verify that the body of the message contains something that looks - # like an activation link - email_body = mail.outbox[0].body - result = re.search('(?Phttps?://[^\s]+)', email_body) - self.assertIsNot(result, None) - - @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') - def test_request_password_change_invalid_user(self): - with self.assertRaises(account_api.AccountUserNotFound): - account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) - - # Verify that no email messages have been sent - self.assertEqual(len(mail.outbox), 0) - - @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') - def test_request_password_change_inactive_user(self): - # Create an account, but do not activate it - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) - - # Verify that the activation email was still sent - self.assertEqual(len(mail.outbox), 1) - - def _assert_is_datetime(self, timestamp): - if not timestamp: - return False - try: - parse_datetime(timestamp) - except ValueError: - return False - else: - return True diff --git a/openedx/core/djangoapps/user_api/tests/test_models.py b/openedx/core/djangoapps/user_api/tests/test_models.py index b3324cd1cf..aae66e10d6 100644 --- a/openedx/core/djangoapps/user_api/tests/test_models.py +++ b/openedx/core/djangoapps/user_api/tests/test_models.py @@ -5,6 +5,7 @@ from student.tests.factories import UserFactory from ..tests.factories import UserPreferenceFactory, UserCourseTagFactory, UserOrgTagFactory from ..models import UserPreference +from ..preferences.api import set_user_preference class UserPreferenceModelTest(ModuleStoreTestCase): @@ -67,20 +68,18 @@ class UserPreferenceModelTest(ModuleStoreTestCase): self.assertEquals(tag.value, "barfoo") self.assertNotEqual(original_modified, tag.modified) - def test_get_set_preference(self): - # Checks that you can set a preference and get that preference later - # Also, tests that no preference is returned for keys that are not set + def test_get_value(self): + """Verifies the behavior of get_value.""" user = UserFactory.create() key = 'testkey' value = 'testvalue' # does a round trip - UserPreference.set_preference(user, key, value) - pref = UserPreference.get_preference(user, key) - + set_user_preference(user, key, value) + pref = UserPreference.get_value(user, key) self.assertEqual(pref, value) # get preference for key that doesn't exist for user - pref = UserPreference.get_preference(user, 'testkey_none') + pref = UserPreference.get_value(user, 'testkey_none') self.assertIsNone(pref) diff --git a/openedx/core/djangoapps/user_api/tests/test_profile_api.py b/openedx/core/djangoapps/user_api/tests/test_profile_api.py deleted file mode 100644 index 530fadd90c..0000000000 --- a/openedx/core/djangoapps/user_api/tests/test_profile_api.py +++ /dev/null @@ -1,163 +0,0 @@ -# -*- coding: utf-8 -*- -""" Tests for the profile API. """ -from django.contrib.auth.models import User - -import ddt -from django.test.utils import override_settings -from nose.tools import raises -from dateutil.parser import parse as parse_datetime -from pytz import UTC -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -import datetime - -from ..accounts.api import get_account_settings -from ..api import account as account_api -from ..api import profile as profile_api -from ..models import UserProfile, UserOrgTag - - -@ddt.ddt -class ProfileApiTest(ModuleStoreTestCase): - - USERNAME = u'frank-underwood' - PASSWORD = u'ṕáśśẃőŕd' - EMAIL = u'frank+underwood@example.com' - - def test_create_profile(self): - # Create a new account, which should have an empty profile by default. - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Retrieve the account settings - user = User.objects.get(username=self.USERNAME) - account_settings = get_account_settings(user) - - # Expect a date joined field but remove it to simplify the following comparison - self.assertIsNotNone(account_settings['date_joined']) - del account_settings['date_joined'] - - # Expect all the values to be defaulted - self.assertEqual(account_settings, { - 'username': self.USERNAME, - 'email': self.EMAIL, - 'name': u'', - 'gender': None, - 'language': u'', - 'goals': None, - 'level_of_education': None, - 'mailing_address': None, - 'year_of_birth': None, - 'country': None, - }) - - def test_update_and_retrieve_preference_info(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - profile_api.update_preferences(self.USERNAME, preference_key='preference_value') - - preferences = profile_api.preference_info(self.USERNAME) - self.assertEqual(preferences['preference_key'], 'preference_value') - - @ddt.data( - # Check that a 27 year old can opt-in - (27, True, u"True"), - - # Check that a 32-year old can opt-out - (32, False, u"False"), - - # Check that someone 14 years old can opt-in - (14, True, u"True"), - - # Check that someone 13 years old cannot opt-in (must have turned 13 before this year) - (13, True, u"False"), - - # Check that someone 12 years old cannot opt-in - (12, True, u"False") - ) - @ddt.unpack - @override_settings(EMAIL_OPTIN_MINIMUM_AGE=13) - def test_update_email_optin(self, age, option, expected_result): - # Create the course and account. - course = CourseFactory.create() - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Set year of birth - user = User.objects.get(username=self.USERNAME) - profile = UserProfile.objects.get(user=user) - year_of_birth = datetime.datetime.now().year - age # pylint: disable=maybe-no-member - profile.year_of_birth = year_of_birth - profile.save() - - profile_api.update_email_opt_in(user, course.id.org, option) - result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') - self.assertEqual(result_obj.value, expected_result) - - def test_update_email_optin_no_age_set(self): - # Test that the API still works if no age is specified. - # Create the course and account. - course = CourseFactory.create() - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - user = User.objects.get(username=self.USERNAME) - - profile_api.update_email_opt_in(user, course.id.org, True) - result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') - self.assertEqual(result_obj.value, u"True") - - @ddt.data( - # Check that a 27 year old can opt-in, then out. - (27, True, False, u"False"), - - # Check that a 32-year old can opt-out, then in. - (32, False, True, u"True"), - - # Check that someone 13 years old can opt-in, then out. - (13, True, False, u"False"), - - # Check that someone 12 years old cannot opt-in, then explicitly out. - (12, True, False, u"False") - ) - @ddt.unpack - @override_settings(EMAIL_OPTIN_MINIMUM_AGE=13) - def test_change_email_optin(self, age, option, second_option, expected_result): - # Create the course and account. - course = CourseFactory.create() - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - # Set year of birth - user = User.objects.get(username=self.USERNAME) - profile = UserProfile.objects.get(user=user) - year_of_birth = datetime.datetime.now(UTC).year - age # pylint: disable=maybe-no-member - profile.year_of_birth = year_of_birth - profile.save() - - profile_api.update_email_opt_in(user, course.id.org, option) - profile_api.update_email_opt_in(user, course.id.org, second_option) - - result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin') - self.assertEqual(result_obj.value, expected_result) - - @raises(profile_api.ProfileUserNotFound) - def test_retrieve_and_update_preference_info_no_user(self): - preferences = profile_api.preference_info(self.USERNAME) - self.assertEqual(preferences, {}) - - profile_api.update_preferences(self.USERNAME, preference_key='preference_value') - - def test_update_and_retrieve_preference_info_unicode(self): - account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - - profile_api.update_preferences(self.USERNAME, **{u'ⓟⓡⓔⓕⓔⓡⓔⓝⓒⓔ_ⓚⓔⓨ': u'ǝnןɐʌ_ǝɔuǝɹǝɟǝɹd'}) - - preferences = profile_api.preference_info(self.USERNAME) - self.assertEqual(preferences[u'ⓟⓡⓔⓕⓔⓡⓔⓝⓒⓔ_ⓚⓔⓨ'], u'ǝnןɐʌ_ǝɔuǝɹǝɟǝɹd') - - def _assert_is_datetime(self, timestamp): - if not timestamp: - return False - try: - parse_datetime(timestamp) - except ValueError: - return False - else: - return True diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index e4f2bbe69d..4ff87c9f84 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -25,7 +25,10 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from third_party_auth.tests.testutil import simulate_running_pipeline from ..accounts.api import get_account_settings -from ..api import account as account_api, profile as profile_api +from ..accounts import ( + NAME_MAX_LENGTH, EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, + USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH +) from ..models import UserOrgTag from ..tests.factories import UserPreferenceFactory from ..tests.test_constants import SORTED_COUNTRIES @@ -618,8 +621,8 @@ class LoginSessionViewTest(ApiTestCase): platform_name=settings.PLATFORM_NAME ), "restrictions": { - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH }, "errorMessages": {}, }, @@ -632,8 +635,8 @@ class LoginSessionViewTest(ApiTestCase): "placeholder": "", "instructions": "", "restrictions": { - "min_length": account_api.PASSWORD_MIN_LENGTH, - "max_length": account_api.PASSWORD_MAX_LENGTH + "min_length": PASSWORD_MIN_LENGTH, + "max_length": PASSWORD_MAX_LENGTH }, "errorMessages": {}, } @@ -769,8 +772,8 @@ class PasswordResetViewTest(ApiTestCase): platform_name=settings.PLATFORM_NAME ), "restrictions": { - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH }, "errorMessages": {}, } @@ -827,8 +830,8 @@ class RegistrationViewTest(ApiTestCase): u"label": u"Email", u"placeholder": u"username@domain.com", u"restrictions": { - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH }, } ) @@ -842,7 +845,7 @@ class RegistrationViewTest(ApiTestCase): u"label": u"Full name", u"instructions": u"The name that will appear on your certificates", u"restrictions": { - "max_length": profile_api.FULL_NAME_MAX_LENGTH, + "max_length": NAME_MAX_LENGTH, }, } ) @@ -856,8 +859,8 @@ class RegistrationViewTest(ApiTestCase): u"label": u"Public username", u"instructions": u"The name that will identify you in your courses", u"restrictions": { - "min_length": account_api.USERNAME_MIN_LENGTH, - "max_length": account_api.USERNAME_MAX_LENGTH + "min_length": USERNAME_MIN_LENGTH, + "max_length": USERNAME_MAX_LENGTH }, } ) @@ -870,8 +873,8 @@ class RegistrationViewTest(ApiTestCase): u"required": True, u"label": u"Password", u"restrictions": { - "min_length": account_api.PASSWORD_MIN_LENGTH, - "max_length": account_api.PASSWORD_MAX_LENGTH + "min_length": PASSWORD_MIN_LENGTH, + "max_length": PASSWORD_MAX_LENGTH }, } ) @@ -905,8 +908,8 @@ class RegistrationViewTest(ApiTestCase): u"label": u"Email", u"placeholder": u"username@domain.com", u"restrictions": { - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH }, } ) @@ -922,7 +925,7 @@ class RegistrationViewTest(ApiTestCase): u"label": u"Full name", u"instructions": u"The name that will appear on your certificates", u"restrictions": { - "max_length": profile_api.FULL_NAME_MAX_LENGTH, + "max_length": NAME_MAX_LENGTH, } } ) @@ -939,8 +942,8 @@ class RegistrationViewTest(ApiTestCase): u"placeholder": u"", u"instructions": u"The name that will identify you in your courses", u"restrictions": { - "min_length": account_api.USERNAME_MIN_LENGTH, - "max_length": account_api.USERNAME_MAX_LENGTH + "min_length": USERNAME_MIN_LENGTH, + "max_length": USERNAME_MAX_LENGTH } } ) @@ -1237,20 +1240,13 @@ class RegistrationViewTest(ApiTestCase): self.assertHttpOK(response) self.assertIn(settings.EDXMKTG_COOKIE_NAME, self.client.cookies) - # Verify that the user exists - self.assertEqual( - account_api.account_info(self.USERNAME), - { - "username": self.USERNAME, - "email": self.EMAIL, - "is_active": False - } - ) - - # Verify that the user's full name is set user = User.objects.get(username=self.USERNAME) account_settings = get_account_settings(user) - self.assertEqual(account_settings["name"], self.NAME) + + self.assertEqual(self.USERNAME, account_settings["username"]) + self.assertEqual(self.EMAIL, account_settings["email"]) + self.assertFalse(account_settings["is_active"]) + self.assertEqual(self.NAME, account_settings["name"]) # Verify that we've been logged in # by trying to access a page that requires authentication diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 4d4ed03731..688634bb57 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -3,6 +3,7 @@ Defines the URL routes for this app. """ from .accounts.views import AccountView +from .preferences.views import PreferencesView, PreferencesDetailView from django.conf.urls import patterns, url @@ -15,4 +16,14 @@ urlpatterns = patterns( AccountView.as_view(), name="accounts_api" ), + url( + r'^v0/preferences/' + USERNAME_PATTERN + '$', + PreferencesView.as_view(), + name="preferences_api" + ), + url( + r'^v0/preferences/' + USERNAME_PATTERN + '/(?P[a-zA-Z0-9_]+)$', + PreferencesDetailView.as_view(), + name="preferences_detail_api" + ), ) diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 929c7133d1..b9b236df80 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -28,9 +28,14 @@ from edxmako.shortcuts import marketing_link from student.views import create_account_with_params, set_marketing_cookie from util.authentication import SessionAuthenticationAllowInactiveUser from util.json_request import JsonResponse -from .api import account as account_api, profile as profile_api +from .preferences.api import update_email_opt_in from .helpers import FormDescription, shim_student_view, require_post_params from .models import UserPreference, UserProfile +from .accounts import ( + NAME_MAX_LENGTH, EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH, + USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH +) +from .accounts.api import check_account_exists from .serializers import UserSerializer, UserPreferenceSerializer @@ -79,8 +84,8 @@ class LoginSessionView(APIView): placeholder=email_placeholder, instructions=email_instructions, restrictions={ - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH, + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH, } ) @@ -93,8 +98,8 @@ class LoginSessionView(APIView): label=password_label, field_type="password", restrictions={ - "min_length": account_api.PASSWORD_MIN_LENGTH, - "max_length": account_api.PASSWORD_MAX_LENGTH, + "min_length": PASSWORD_MIN_LENGTH, + "max_length": PASSWORD_MAX_LENGTH, } ) @@ -251,7 +256,7 @@ class RegistrationView(APIView): username = data.get('username') # Handle duplicate email/username - conflicts = account_api.check_account_exists(email=email, username=username) + conflicts = check_account_exists(email=email, username=username) if conflicts: conflict_messages = { # Translators: This message is shown to users who attempt to create a new @@ -321,8 +326,8 @@ class RegistrationView(APIView): label=email_label, placeholder=email_placeholder, restrictions={ - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH, + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH, }, required=required ) @@ -350,7 +355,7 @@ class RegistrationView(APIView): label=name_label, instructions=name_instructions, restrictions={ - "max_length": profile_api.FULL_NAME_MAX_LENGTH, + "max_length": NAME_MAX_LENGTH, }, required=required ) @@ -380,8 +385,8 @@ class RegistrationView(APIView): label=username_label, instructions=username_instructions, restrictions={ - "min_length": account_api.USERNAME_MIN_LENGTH, - "max_length": account_api.USERNAME_MAX_LENGTH, + "min_length": USERNAME_MIN_LENGTH, + "max_length": USERNAME_MAX_LENGTH, }, required=required ) @@ -405,8 +410,8 @@ class RegistrationView(APIView): label=password_label, field_type="password", restrictions={ - "min_length": account_api.PASSWORD_MIN_LENGTH, - "max_length": account_api.PASSWORD_MAX_LENGTH, + "min_length": PASSWORD_MIN_LENGTH, + "max_length": PASSWORD_MAX_LENGTH, }, required=required ) @@ -775,8 +780,8 @@ class PasswordResetView(APIView): placeholder=email_placeholder, instructions=email_instructions, restrictions={ - "min_length": account_api.EMAIL_MIN_LENGTH, - "max_length": account_api.EMAIL_MAX_LENGTH, + "min_length": EMAIL_MIN_LENGTH, + "max_length": EMAIL_MAX_LENGTH, } ) @@ -870,5 +875,5 @@ class UpdateEmailOptInPreference(APIView): ) # Only check for true. All other values are False. email_opt_in = request.DATA['email_opt_in'].lower() == 'true' - profile_api.update_email_opt_in(request.user, org, email_opt_in) + update_email_opt_in(request.user, org, email_opt_in) return HttpResponse(status=status.HTTP_200_OK) diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index 129ae7dee2..e02b190a5c 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -54,17 +54,7 @@ class IsUserInUrl(permissions.BasePermission): def has_permission(self, request, view): # Return a 404 instead of a 403 (Unauthorized). If one user is looking up # other users, do not let them deduce the existence of an account. - if request.user.username != request.parser_context.get('kwargs', {}).get('username', None): + url_username = request.parser_context.get('kwargs', {}).get('username', '') + if request.user.username.lower() != url_username.lower(): raise Http404() return True - - -class IsUserInUrlOrStaff(IsUserInUrl): - """ - Permission that checks to see if the request user matches the user in the URL or has is_staff access. - """ - def has_permission(self, request, view): - if request.user.is_staff: - return True - - return super(IsUserInUrlOrStaff, self).has_permission(request, view)