diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index ecfab72331..6bd38ed934 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -130,7 +130,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase): # Verify that the profile API has been called as expected if email_opt_in is not None: opt_in = email_opt_in == 'true' - mock_update_email_opt_in.assert_called_once_with(self.USERNAME, self.course.org, opt_in) + mock_update_email_opt_in.assert_called_once_with(self.user, self.course.org, opt_in) else: self.assertFalse(mock_update_email_opt_in.called) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 88778e0806..f4dd626e02 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -797,7 +797,7 @@ def try_change_enrollment(request): log.exception(u"Exception automatically enrolling after login: %s", exc) -def _update_email_opt_in(request, username, org): +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 @@ -806,7 +806,7 @@ def _update_email_opt_in(request, username, org): 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(username, org, email_opt_in_boolean) + profile_api.update_email_opt_in(request.user, org, email_opt_in_boolean) @require_POST @@ -878,7 +878,7 @@ def change_enrollment(request, check_access=True): # Record the user's email opt-in preference if settings.FEATURES.get('ENABLE_MKTG_EMAIL_OPT_IN'): - _update_email_opt_in(request, user.username, course_id.org) + _update_email_opt_in(request, course_id.org) available_modes = CourseMode.modes_for_course_dict(course_id) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 28081c1b8b..97c905fac0 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -672,7 +672,7 @@ def change_enrollment(strategy, user=None, is_dashboard=False, *args, **kwargs): # 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.username, course_id.org, opt_in) + profile.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/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 4ceb00cb89..077e35e275 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -1149,7 +1149,7 @@ class TestSubmitPhotosForVerification(TestCase): AssertionError """ - account_settings = AccountView.get_serialized_account(self.user.username) + account_settings = AccountView.get_serialized_account(self.user) self.assertEqual(account_settings['name'], full_name) diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 7842da64e2..18d56e3e82 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -714,13 +714,11 @@ def submit_photos_for_verification(request): if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user): return HttpResponseBadRequest(_("You already have a valid or pending verification.")) - username = request.user.username - # If the user wants to change his/her full name, # then try to do that before creating the attempt. if request.POST.get('full_name'): try: - AccountView.update_account(username, {"name": request.POST.get('full_name')}) + AccountView.update_account(request.user, {"name": request.POST.get('full_name')}) except AccountUserNotFound: return HttpResponseBadRequest(_("No profile found for user")) except AccountUpdateError: @@ -743,7 +741,7 @@ def submit_photos_for_verification(request): attempt.mark_ready() attempt.submit() - account_settings = AccountView.get_serialized_account(username) + account_settings = AccountView.get_serialized_account(request.user) # Send a confirmation email to the user context = { diff --git a/lms/envs/common.py b/lms/envs/common.py index 38c7f3ea1a..312e52f242 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2047,14 +2047,14 @@ SEARCH_ENGINE = None # Use the LMS specific result processor SEARCH_RESULT_PROCESSOR = "lms.lib.courseware_search.lms_result_processor.LmsSearchResultProcessor" -# The configuration for learner profiles -PROFILE_CONFIGURATION = { +# The configuration visibility of account fields. +ACCOUNT_VISIBILITY_CONFIGURATION = { # Default visibility level for accounts without a specified value # The value is one of: 'all_users', 'private' "default_visibility": "private", - # The list of all fields that can be shown on a learner's profile - "all_fields": [ + # The list of all fields that can be shared with other users + "shareable_fields": [ 'username', 'profile_image', 'country', @@ -2063,7 +2063,7 @@ PROFILE_CONFIGURATION = { 'bio', ], - # The list of fields that are always public on a learner's profile + # The list of account fields that are always public "public_fields": [ 'username', 'profile_image', diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index 1a655b2835..4db70bee44 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -1,2 +1,14 @@ +""" +Account constants +""" + # The minimum acceptable length for the name account field NAME_MIN_LENGTH = 2 + +ACCOUNT_VISIBILITY_PREF_KEY = 'account_privacy' + +# Indicates the user's preference that all users can view the shareable fields in their account information. +ALL_USERS_VISIBILITY = 'all_users' + +# Indicates the user's preference that all their account information be private. +PRIVATE_VISIBILITY = 'private' 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 e20eebbede..1db5f894fc 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -2,6 +2,7 @@ import unittest import ddt import json +from mock import patch from django.conf import settings from django.core.urlresolvers import reverse @@ -9,6 +10,9 @@ 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 .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY TEST_PASSWORD = "test" @@ -73,24 +77,66 @@ class TestAccountAPI(UserAPITestCase): """ Unit tests for the Account API. """ - def setUp(self): super(TestAccountAPI, self).setUp() self.url = reverse("accounts_api", kwargs={'username': self.user.username}) - def test_get_account_anonymous_user(self): + def _verify_full_shareable_account_response(self, response): """ - Test that an anonymous client (not logged in) cannot call get. + Verify that the shareable fields from the account are returned + """ + data = response.data + self.assertEqual(6, len(data)) + self.assertEqual(self.user.username, data["username"]) + self.assertEqual("US", data["country"]) + self.assertIsNone(data["profile_image"]) + self.assertIsNone(data["time_zone"]) + self.assertIsNone(data["languages"]) + self.assertIsNone(data["bio"]) + + def _verify_private_account_response(self, response): + """ + Verify that only the public fields are returned if a user does not want to share account fields + """ + data = response.data + self.assertEqual(2, len(data)) + self.assertEqual(self.user.username, data["username"]) + self.assertIsNone(data["profile_image"]) + + def _verify_full_account_response(self, response): + """ + Verify that all account fields are returned (even those that are not shareable). + """ + 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"]) + self.assertEqual("US", data["country"]) + self.assertEqual("", data["language"]) + self.assertEqual("m", data["gender"]) + self.assertEqual(1900, data["year_of_birth"]) + self.assertEqual("m", data["level_of_education"]) + self.assertEqual("world peace", data["goals"]) + self.assertEqual("Park Ave", data['mailing_address']) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["date_joined"]) + + 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_get_account_different_user(self): + def test_unsupported_methods(self): """ - Test that a client (logged in) cannot get the account information for a different client. + Test that DELETE, POST, and PUT are not supported. """ - self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) - self.send_get(self.different_client, expected_status=404) + self.client.login(username=self.user.username, password=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) @ddt.data( ("client", "user"), @@ -105,6 +151,69 @@ class TestAccountAPI(UserAPITestCase): response = client.get(reverse("accounts_api", kwargs={'username': "does_not_exist"})) self.assertEqual(404, response.status_code) + # Note: using getattr so that the patching works even if there is no configuration. + # This is needed when testing CMS as the patching is still executed even though the + # suite is skipped. + @patch.dict(getattr(settings, "ACCOUNT_VISIBILITY_CONFIGURATION", {}), {"default_visibility": "all_users"}) + def test_get_account_different_user_visible(self): + """ + 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.create_mock_profile(self.user) + response = self.send_get(self.different_client) + self._verify_full_shareable_account_response(response) + + # Note: using getattr so that the patching works even if there is no configuration. + # This is needed when testing CMS as the patching is still executed even though the + # suite is skipped. + @patch.dict(getattr(settings, "ACCOUNT_VISIBILITY_CONFIGURATION", {}), {"default_visibility": "private"}) + def test_get_account_different_user_private(self): + """ + 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.create_mock_profile(self.user) + response = self.send_get(self.different_client) + self._verify_private_account_response(response) + + @ddt.data( + ("client", "user", PRIVATE_VISIBILITY), + ("different_client", "different_user", PRIVATE_VISIBILITY), + ("staff_client", "staff_user", PRIVATE_VISIBILITY), + ("client", "user", ALL_USERS_VISIBILITY), + ("different_client", "different_user", ALL_USERS_VISIBILITY), + ("staff_client", "staff_user", ALL_USERS_VISIBILITY), + ) + @ddt.unpack + def test_get_account_private_visibility(self, api_client, requesting_username, preference_visibility): + """ + Test the return from GET based on user visibility setting. + """ + def verify_fields_visible_to_all_users(response): + if preference_visibility == PRIVATE_VISIBILITY: + self._verify_private_account_response(response) + else: + self._verify_full_shareable_account_response(response) + + client = self.login_client(api_client, requesting_username) + + # Update user account visibility setting. + UserPreference.set_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, preference_visibility) + self.create_mock_profile(self.user) + response = self.send_get(client) + + if requesting_username == "different_user": + verify_fields_visible_to_all_users(response) + else: + self._verify_full_account_response(response) + + # Verify how the view parameter changes the fields that are returned. + response = self.send_get(client, query_parameters='view=shared') + verify_fields_visible_to_all_users(response) + def test_get_account_default(self): """ Test that a client (logged in) can get her own account information (using default legacy profile information, @@ -126,33 +235,6 @@ class TestAccountAPI(UserAPITestCase): self.assertEqual(self.user.email, data["email"]) self.assertIsNotNone(data["date_joined"]) - @ddt.data( - ("client", "user"), - ("staff_client", "staff_user"), - ) - @ddt.unpack - def test_get_account(self, api_client, user): - """ - Test that a client (logged in) can get her own account information. Also verifies that a "is_staff" - user can get the account information for other users. - """ - self.create_mock_profile(self.user) - client = self.login_client(api_client, user) - response = self.send_get(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"]) - self.assertEqual("US", data["country"]) - self.assertEqual("", data["language"]) - self.assertEqual("m", data["gender"]) - self.assertEqual(1900, data["year_of_birth"]) - self.assertEqual("m", data["level_of_education"]) - self.assertEqual("world peace", data["goals"]) - self.assertEqual("Park Ave", data['mailing_address']) - self.assertEqual(self.user.email, data["email"]) - self.assertIsNotNone(data["date_joined"]) - def test_get_account_empty_string(self): """ Test the conversion of empty strings to None for certain fields. @@ -168,25 +250,18 @@ class TestAccountAPI(UserAPITestCase): for empty_field in ("level_of_education", "gender", "country"): self.assertIsNone(response.data[empty_field]) - def test_patch_account_anonymous_user(self): + @ddt.data( + ("different_client", "different_user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_patch_account_disallowed_user(self, api_client, user): """ - Test that an anonymous client (not logged in) cannot call patch. + Test that a client cannot call PATCH on a different client's user account (even with + is_staff access). """ - self.send_patch(self.anonymous_client, {}, expected_status=401) - - def test_patch_account_different_user(self): - """ - Test that a client (logged in) cannot update the account information for a different client. - """ - self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) - self.send_patch(self.different_client, {}, expected_status=404) - - def test_patch_account_is_staff(self): - """ - Test that a client (logged in) with is_staff privileges cannot account settings for other users. - """ - self.staff_client.login(username=self.staff_user.username, password=TEST_PASSWORD) - self.send_patch(self.staff_client, {}, expected_status=404) + client = self.login_client(api_client, user) + self.send_patch(client, {}, expected_status=404) @ddt.data( ("client", "user"), @@ -217,9 +292,7 @@ class TestAccountAPI(UserAPITestCase): # Note that email is tested below, as it is not immediately updated. ) @ddt.unpack - def test_patch_account( - self, field, value, fails_validation_value=None, developer_validation_message=None - ): + def test_patch_account(self, field, value, fails_validation_value=None, developer_validation_message=None): """ Test the behavior of patch, when using the correct content_type. """ @@ -311,10 +384,10 @@ class TestAccountAPI(UserAPITestCase): self.assertEqual(expected_entries, len(name_change_info)) return name_change_info - def verify_change_info(change_info, old_name, new_name): + def verify_change_info(change_info, old_name, requester, new_name): self.assertEqual(3, len(change_info)) self.assertEqual(old_name, change_info[0]) - self.assertEqual("Name change requested through account API", change_info[1]) + self.assertEqual("Name change requested through account API by {}".format(requester), change_info[1]) self.assertIsNotNone(change_info[2]) # Verify the new name was also stored. get_response = self.send_get(self.client) @@ -328,13 +401,13 @@ class TestAccountAPI(UserAPITestCase): # First change the name as the user and verify meta information. self.send_patch(self.client, {"name": "Mickey Mouse"}) name_change_info = get_name_change_info(1) - verify_change_info(name_change_info[0], old_name, "Mickey Mouse") + verify_change_info(name_change_info[0], old_name, self.user.username, "Mickey Mouse") # Now change the name again and verify meta information. self.send_patch(self.client, {"name": "Donald Duck"}) name_change_info = get_name_change_info(2) - verify_change_info(name_change_info[0], old_name, "Donald Duck",) - verify_change_info(name_change_info[1], "Mickey Mouse", "Donald Duck") + verify_change_info(name_change_info[0], old_name, self.user.username, "Donald Duck",) + verify_change_info(name_change_info[1], "Mickey Mouse", self.user.username, "Donald Duck") def test_patch_email(self): """ diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index f5fb2846a3..b3b6ec8fed 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -7,6 +7,7 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API from django.core.exceptions import ObjectDoesNotExist from django.contrib.auth.models import User from django.utils.translation import ugettext as _ +from django.conf import settings import datetime from pytz import UTC @@ -17,11 +18,14 @@ from rest_framework.authentication import OAuth2Authentication, SessionAuthentic from rest_framework import permissions from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer, AccountUserSerializer -from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError +from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError, AccountNotAuthorized from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.permissions import IsUserInUrlOrStaff from student.models import UserProfile from student.views import do_email_change_request +from ..models import UserPreference + +from . import ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY class AccountView(APIView): @@ -79,7 +83,7 @@ class AccountView(APIView): """ authentication_classes = (OAuth2Authentication, SessionAuthentication) - permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff) + permission_classes = (permissions.IsAuthenticated,) parser_classes = (MergePatchParser,) def get(self, request, username): @@ -87,34 +91,75 @@ class AccountView(APIView): GET /api/user/v0/accounts/{username}/ """ try: - account_settings = AccountView.get_serialized_account(username) + account_settings = AccountView.get_serialized_account( + request.user, username, + view=request.QUERY_PARAMS.get('view') + ) except AccountUserNotFound: return Response(status=status.HTTP_404_NOT_FOUND) return Response(account_settings) @staticmethod - def get_serialized_account(username): - """Returns the user's account information serialized as JSON. + def get_serialized_account(requesting_user, username=None, configuration=None, view=None): + """Returns account information for a user serialized as JSON. Note: - This method does not perform authentication so it is up to the caller - to ensure that only the user themselves or staff can access the account. + If `requesting_user.username` != `username`, this method will return differing amounts of information + based on who `requesting_user` is and the privacy settings of the user associated with `username`. Args: - username (str): The username for the desired account. + requesting_user (User): The user requesting the account information. Only the user with username + `username` or users with "is_staff" privileges can get full account information. + Other users will get the account fields that the user has elected to share. + username (str): Optional username for the desired account information. If not specified, + `requesting_user.username` is assumed. + configuration (dict): an optional configuration specifying which fields in the account + can be shared, and the default visibility settings. If not present, the setting value with + key ACCOUNT_VISIBILITY_CONFIGURATION is used. + view (str): An optional string allowing "is_staff" users and users requesting their own + account information to get just the fields that are shared with everyone. If view is + "shared", only shared account information will be returned, regardless of `requesting_user`. Returns: - A dict containing each of the account's fields. + A dict containing account fields. Raises: - AccountUserNotFound: raised if there is no account for the specified username. + AccountUserNotFound: `username` was specified, but no user exists with that username. """ + if username is None: + username = requesting_user.username + + has_full_access = requesting_user.username == username or requesting_user.is_staff + return_all_fields = has_full_access and view != 'shared' + existing_user, existing_user_profile = AccountView._get_user_and_profile(username) + user_serializer = AccountUserSerializer(existing_user) legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile) - return dict(user_serializer.data, **legacy_profile_serializer.data) + account_settings = dict(user_serializer.data, **legacy_profile_serializer.data) + + if return_all_fields: + return account_settings + + if not configuration: + configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION + + visible_settings = {} + + profile_privacy = UserPreference.get_preference(existing_user, ACCOUNT_VISIBILITY_PREF_KEY) + privacy_setting = profile_privacy if profile_privacy else configuration.get('default_visibility') + + if privacy_setting == ALL_USERS_VISIBILITY: + field_names = configuration.get('shareable_fields') + else: + field_names = configuration.get('public_fields') + + for field_name in field_names: + visible_settings[field_name] = account_settings.get(field_name, None) + + return visible_settings def patch(self, request, username): """ @@ -124,13 +169,9 @@ class AccountView(APIView): https://tools.ietf.org/html/rfc7396. The content_type must be "application/merge-patch+json" or else an error response with status code 415 will be returned. """ - # Disallow users with is_staff access from calling patch on any account. - if request.user.username != username: - return Response(status=status.HTTP_404_NOT_FOUND) - try: - AccountView.update_account(username, request.DATA) - except AccountUserNotFound: + AccountView.update_account(request.user, request.DATA, username=username) + except (AccountUserNotFound, AccountNotAuthorized): return Response(status=status.HTTP_404_NOT_FOUND) except AccountUpdateError as err: return Response(err.error_info, status=status.HTTP_400_BAD_REQUEST) @@ -138,23 +179,33 @@ class AccountView(APIView): return Response(status=status.HTTP_204_NO_CONTENT) @staticmethod - def update_account(username, update): - """Update the account for the given username. + def update_account(requesting_user, update, username=None): + """Update user account information. Note: - No authorization or permissions checks are done in this method. It is up to the caller - of this method to enforce the contract that this method is only called - by the user with the specified username. + 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: - username (string): the username associated with the account to change - update (dict): the updated account field values + 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. + username (string): Optional username specifying which account should be updated. If not specified, + `requesting_user.username` is assumed. Raises: - AccountUserNotFound: no user exists with the specified username + AccountUserNotFound: `username` was specified, but no user exists with that username. AccountUpdateError: the update could not be completed, usually due to validation errors (for example, read-only fields were specified or field values are not legal) + AccountNotAuthorized: the requesting_user does not have access to change the account + associated with `username`. """ + if username is None: + username = requesting_user.username + + if requesting_user.username != username: + raise AccountNotAuthorized() + existing_user, existing_user_profile = AccountView._get_user_and_profile(username) # If user has requested to change email, we must call the multi-step process to handle this. @@ -203,14 +254,15 @@ class AccountView(APIView): raise AccountUpdateError(validation_errors) serializer.save() - # If the name was changed, store information about the change operation. + # If the name was changed, store information about the change operation. This is outside of the + # serializer so that we can store who requested the change. if old_name: meta = existing_user_profile.get_meta() if 'old_names' not in meta: meta['old_names'] = [] meta['old_names'].append([ old_name, - "Name change requested through account API", + "Name change requested through account API by {0}".format(requesting_user.username), datetime.datetime.now(UTC).isoformat() ]) existing_user_profile.set_meta(meta) diff --git a/openedx/core/djangoapps/user_api/api/profile.py b/openedx/core/djangoapps/user_api/api/profile.py index 371e4a4186..04b8b2184d 100644 --- a/openedx/core/djangoapps/user_api/api/profile.py +++ b/openedx/core/djangoapps/user_api/api/profile.py @@ -90,14 +90,14 @@ def update_preferences(username, **kwargs): @intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) -def update_email_opt_in(username, org, optin): +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: - username (str): The user to set a preference for. + 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. @@ -105,11 +105,8 @@ def update_email_opt_in(username, org, optin): Returns: None - Raises: - AccountUserNotFound: Raised when the username specified is not associated with a user. - """ - account_settings = AccountView.get_serialized_account(username) + account_settings = AccountView.get_serialized_account(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. @@ -118,7 +115,6 @@ def update_email_opt_in(username, org, optin): ) try: - user = User.objects.get(username=username) preference, _ = UserOrgTag.objects.get_or_create( user=user, org=org, key='email-optin' ) 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 8bf2b295ff..f7c43fe863 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 @@ -297,7 +297,7 @@ class EmailOptInListTest(ModuleStoreTestCase): None """ - profile_api.update_email_opt_in(user.username, org, is_opted_in) + profile_api.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/profiles/__init__.py b/openedx/core/djangoapps/user_api/profiles/__init__.py deleted file mode 100644 index b3d5698513..0000000000 --- a/openedx/core/djangoapps/user_api/profiles/__init__.py +++ /dev/null @@ -1,11 +0,0 @@ -""" -Profile constants -""" - -PROFILE_VISIBILITY_PREF_KEY = 'profile_privacy' - -# Indicates the user's preference that all users can view their profile. -ALL_USERS_VISIBILITY = 'all_users' - -# Indicates the user's preference that their profile be private. -PRIVATE_VISIBILITY = 'private' diff --git a/openedx/core/djangoapps/user_api/profiles/tests/__init__.py b/openedx/core/djangoapps/user_api/profiles/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/user_api/profiles/tests/test_views.py b/openedx/core/djangoapps/user_api/profiles/tests/test_views.py deleted file mode 100644 index 28e4377861..0000000000 --- a/openedx/core/djangoapps/user_api/profiles/tests/test_views.py +++ /dev/null @@ -1,137 +0,0 @@ -""" -Unit tests for profile APIs. -""" - -import ddt -import unittest - -from django.conf import settings -from django.core.urlresolvers import reverse -from mock import patch - -from openedx.core.djangoapps.user_api.accounts.tests.test_views import UserAPITestCase -from openedx.core.djangoapps.user_api.models import UserPreference -from openedx.core.djangoapps.user_api.profiles import PROFILE_VISIBILITY_PREF_KEY -from .. import PRIVATE_VISIBILITY - - -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile APIs are only supported in LMS') -class TestProfileAPI(UserAPITestCase): - """ - Unit tests for the profile API. - """ - - def setUp(self): - super(TestProfileAPI, self).setUp() - self.url = reverse("profiles_api", kwargs={'username': self.user.username}) - - def test_get_profile_anonymous_user(self): - """ - Test that an anonymous client (not logged in) cannot call get. - """ - self.send_get(self.anonymous_client, expected_status=401) - - def _verify_full_profile_response(self, response): - """ - Verify that all of the profile's fields are returned - """ - data = response.data - self.assertEqual(6, len(data)) - self.assertEqual(self.user.username, data["username"]) - self.assertEqual("US", data["country"]) - self.assertIsNone(data["profile_image"]) - self.assertIsNone(data["time_zone"]) - self.assertIsNone(data["languages"]) - self.assertIsNone(data["bio"]) - - def _verify_private_profile_response(self, response): - """ - Verify that only the public fields are returned for a private user's profile - """ - data = response.data - self.assertEqual(2, len(data)) - self.assertEqual(self.user.username, data["username"]) - self.assertIsNone(data["profile_image"]) - - @ddt.data( - ("client", "user"), - ("different_client", "different_user"), - ("staff_client", "staff_user"), - ) - @ddt.unpack - # Note: using getattr so that the patching works even if there is no configuration. - # This is needed when testing CMS as the patching is still executed even though the - # suite is skipped. - @patch.dict(getattr(settings, "PROFILE_CONFIGURATION", {}), {"default_visibility": "all_users"}) - def test_get_default_profile(self, api_client, username): - """ - Test that any logged in user can get the main test user's public profile information. - """ - client = self.login_client(api_client, username) - self.create_mock_profile(self.user) - response = self.send_get(client) - self._verify_full_profile_response(response) - - @ddt.data( - ("client", "user"), - ("different_client", "different_user"), - ("staff_client", "staff_user"), - ) - @ddt.unpack - # Note: using getattr so that the patching works even if there is no configuration. - # This is needed when testing CMS as the patching is still executed even though the - # suite is skipped. - @patch.dict(getattr(settings, "PROFILE_CONFIGURATION", {}), {"default_visibility": "private"}) - def test_get_default_private_profile(self, api_client, username): - """ - Test that any logged in user gets only the public fields for a profile - if the default visibility is 'private'. - """ - client = self.login_client(api_client, username) - self.create_mock_profile(self.user) - response = self.send_get(client) - self._verify_private_profile_response(response) - - @ddt.data( - ("client", "user"), - ("different_client", "different_user"), - ("staff_client", "staff_user"), - ) - @ddt.unpack - def test_get_private_profile(self, api_client, requesting_username): - """ - Test that private profile information is only available to the test user themselves. - """ - client = self.login_client(api_client, requesting_username) - - # Verify that a user with a private profile only returns the public fields - UserPreference.set_preference(self.user, PROFILE_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY) - self.create_mock_profile(self.user) - response = self.send_get(client) - self._verify_private_profile_response(response) - - # Verify that only the public fields are returned if 'include_all' parameter is specified as false - response = self.send_get(client, query_parameters='include_all=false') - self._verify_private_profile_response(response) - - # Verify that all fields are returned for the user themselves if - # the 'include_all' parameter is specified as true. - response = self.send_get(client, query_parameters='include_all=true') - if requesting_username == "user": - self._verify_full_profile_response(response) - else: - self._verify_private_profile_response(response) - - @ddt.data( - ("client", "user"), - ("staff_client", "staff_user"), - ) - @ddt.unpack - def test_get_profile_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("profiles_api", kwargs={'username': "does_not_exist"})) - self.assertEqual(404, response.status_code) diff --git a/openedx/core/djangoapps/user_api/profiles/views.py b/openedx/core/djangoapps/user_api/profiles/views.py deleted file mode 100644 index 2d4038e276..0000000000 --- a/openedx/core/djangoapps/user_api/profiles/views.py +++ /dev/null @@ -1,123 +0,0 @@ -""" -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 django.conf import settings -from django.contrib.auth.models import User - -from rest_framework import status -from rest_framework.views import APIView -from rest_framework.response import Response -from rest_framework.authentication import OAuth2Authentication, SessionAuthentication -from rest_framework import permissions - -from ..accounts.views import AccountView -from ..api.account import AccountUserNotFound -from ..models import UserPreference - -from . import PROFILE_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY - - -class ProfileView(APIView): - """ - **Use Cases** - - Get the user's public profile information. - - **Example Requests**: - - GET /api/user/v0/profiles/{username}/[?include_all={true | false}] - - **Response Values for GET** - - Returns the same responses as for the AccountView API for the - subset of fields that have been configured to be in a profile. - The fields are additionally filtered based upon the user's - specified privacy permissions. - - If the parameter 'include_all' is passed as 'true' then a user - can receive all fields for their own account, ignoring - any field visibility preferences. If the parameter is not - specified or if the user is requesting information for a - different account then the privacy filtering will be applied. - - """ - authentication_classes = (OAuth2Authentication, SessionAuthentication) - permission_classes = (permissions.IsAuthenticated,) - - def get(self, request, username): - """ - GET /api/user/v0/profiles/{username}/[?include_all={true | false}] - - Note: - The include_all query parameter will only be honored if the user is making - the request for their own username. It defaults to false, but if true - then all the profile fields will be returned even for a user with - a private profile. - """ - if request.user.username == username: - include_all_fields = self.request.QUERY_PARAMS.get('include_all') == 'true' - else: - include_all_fields = False - try: - profile_settings = ProfileView.get_serialized_profile( - username, - settings.PROFILE_CONFIGURATION, - include_all_fields=include_all_fields, - ) - except AccountUserNotFound: - return Response(status=status.HTTP_404_NOT_FOUND) - return Response(profile_settings) - - @staticmethod - def get_serialized_profile(username, configuration=None, include_all_fields=False): - """Returns the user's public profile settings serialized as JSON. - - The fields returned are by default governed by the user's privacy preference. - If the user has a private profile, then only the fields that are always - public are returned. If the user is sharing their profile with all users - then all profile fields are returned. - - Note: - This method does not perform authentication so it is up to the caller - to ensure that only edX users can access the profile. In addition, only - the user themselves should be able to access all fields of a private - profile through 'include_all_fields' being true. - - Args: - username (str): The username for the desired account. - configuration (dict): A dictionary specifying three profile configuration settings: - default_visibility: the default visibility level for user's with no preference - all_fields: the list of all fields that can be shown on a profile - public_fields: the list of profile fields that are public - include_all_fields (bool): If true, ignores the user's privacy setting. - - Returns: - A dict containing each of the user's profile fields. - - Raises: - AccountUserNotFound: raised if there is no account for the specified username. - """ - if not configuration: - configuration = settings.PROFILE_CONFIGURATION - account_settings = AccountView.get_serialized_account(username) - profile = {} - privacy_setting = ProfileView._get_user_profile_privacy(username, configuration) - if include_all_fields or privacy_setting == ALL_USERS_VISIBILITY: - field_names = configuration.get('all_fields') - else: - field_names = configuration.get('public_fields') - for field_name in field_names: - profile[field_name] = account_settings.get(field_name, None) - return profile - - @staticmethod - def _get_user_profile_privacy(username, configuration): - """ - Returns the profile privacy preference for the specified user. - """ - user = User.objects.get(username=username) - profile_privacy = UserPreference.get_preference(user, PROFILE_VISIBILITY_PREF_KEY) - return profile_privacy if profile_privacy else configuration.get('default_visibility') diff --git a/openedx/core/djangoapps/user_api/tests/test_profile_api.py b/openedx/core/djangoapps/user_api/tests/test_profile_api.py index c5cab80744..926a4ae403 100644 --- a/openedx/core/djangoapps/user_api/tests/test_profile_api.py +++ b/openedx/core/djangoapps/user_api/tests/test_profile_api.py @@ -29,7 +29,8 @@ class ProfileApiTest(ModuleStoreTestCase): account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) # Retrieve the account settings - account_settings = AccountView.get_serialized_account(self.USERNAME) + user = User.objects.get(username=self.USERNAME) + account_settings = AccountView.get_serialized_account(user) # Expect a date joined field but remove it to simplify the following comparison self.assertIsNotNone(account_settings['date_joined']) @@ -87,7 +88,7 @@ class ProfileApiTest(ModuleStoreTestCase): profile.year_of_birth = year_of_birth profile.save() - profile_api.update_email_opt_in(self.USERNAME, course.id.org, option) + 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) @@ -99,7 +100,7 @@ class ProfileApiTest(ModuleStoreTestCase): user = User.objects.get(username=self.USERNAME) - profile_api.update_email_opt_in(self.USERNAME, course.id.org, True) + 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") @@ -130,8 +131,8 @@ class ProfileApiTest(ModuleStoreTestCase): profile.year_of_birth = year_of_birth profile.save() - profile_api.update_email_opt_in(self.USERNAME, course.id.org, option) - profile_api.update_email_opt_in(self.USERNAME, course.id.org, second_option) + 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) diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 3d11542e38..62bad76989 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -8,6 +8,7 @@ import re from django.conf import settings from django.core.urlresolvers import reverse from django.core import mail +from django.contrib.auth.models import User from django.test import TestCase from django.test.utils import override_settings from unittest import skipUnless @@ -1247,7 +1248,8 @@ class RegistrationViewTest(ApiTestCase): ) # Verify that the user's full name is set - account_settings = AccountView.get_serialized_account(self.USERNAME) + user = User.objects.get(username=self.USERNAME) + account_settings = AccountView.get_serialized_account(user) self.assertEqual(account_settings["name"], self.NAME) # Verify that we've been logged in @@ -1280,7 +1282,8 @@ class RegistrationViewTest(ApiTestCase): self.assertHttpOK(response) # Verify the user's account - account_settings = AccountView.get_serialized_account(self.USERNAME) + user = User.objects.get(username=self.USERNAME) + account_settings = AccountView.get_serialized_account(user) self.assertEqual(account_settings["level_of_education"], self.EDUCATION) self.assertEqual(account_settings["mailing_address"], self.ADDRESS) self.assertEqual(account_settings["year_of_birth"], int(self.YEAR_OF_BIRTH)) diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 795deffb73..4d4ed03731 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -3,7 +3,6 @@ Defines the URL routes for this app. """ from .accounts.views import AccountView -from .profiles.views import ProfileView from django.conf.urls import patterns, url @@ -16,9 +15,4 @@ urlpatterns = patterns( AccountView.as_view(), name="accounts_api" ), - url( - r'^v0/profiles/' + USERNAME_PATTERN + '$', - ProfileView.as_view(), - name="profiles_api" - ), )