diff --git a/cms/envs/common.py b/cms/envs/common.py index 864cc33bed44b953c7b58162ba2bf38035510c9f..2a5c20cd8a07633a1810aeb637e9f691e45a4701 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1364,6 +1364,7 @@ ADVANCED_PROBLEM_TYPES = [ } ] +USERNAME_REPLACEMENT_WORKER = "REPLACE WITH VALID USERNAME" # Files and Uploads type filter values diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index aafad0df20a3de1268525d72c57f00f49e71ae99..6c0335c2b9510d54523dc9117a872a975863ab12 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -236,6 +236,100 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): pass +@ddt.ddt +@httpretty.activate +@mock.patch('django.conf.settings.USERNAME_REPLACEMENT_WORKER', 'test_replace_username_service_worker') +@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) +class ReplaceUsernamesViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): + """Tests for ReplaceUsernamesView""" + def setUp(self): + super(ReplaceUsernamesViewTest, self).setUp() + self.client_user = UserFactory() + self.client_user.username = "test_replace_username_service_worker" + self.new_username = "test_username_replacement" + self.url = reverse("replace_discussion_username") + + def assert_response_correct(self, response, expected_status, expected_content): + """ + Assert that the response has the given status code and content + """ + self.assertEqual(response.status_code, expected_status) + + if expected_content: + self.assertEqual(text_type(response.content), expected_content) + + def build_jwt_headers(self, user): + """ + Helper function for creating headers for the JWT authentication. + """ + token = create_jwt_for_user(user) + headers = {'HTTP_AUTHORIZATION': 'JWT ' + token} + return headers + + def call_api(self, user, data): + """ Helper function to call API with data """ + data = json.dumps(data) + headers = self.build_jwt_headers(user) + return self.client.post(self.url, data, content_type='application/json', **headers) + + @ddt.data( + [{}, {}], + {}, + [{"test_key": "test_value", "test_key_2": "test_value_2"}] + ) + def test_bad_schema(self, mapping_data): + """ Verify the endpoint rejects bad data schema """ + data = { + "username_mappings": mapping_data + } + response = self.call_api(self.client_user, data) + self.assertEqual(response.status_code, 400) + + def test_auth(self): + """ Verify the endpoint only works with the service worker """ + data = { + "username_mappings": [ + {"test_username_1": "test_new_username_1"}, + {"test_username_2": "test_new_username_2"} + ] + } + + # Test unauthenticated + response = self.client.post(self.url, data) + self.assertEqual(response.status_code, 401) + + # Test non-service worker + random_user = UserFactory() + response = self.call_api(random_user, data) + self.assertEqual(response.status_code, 403) + + # Test service worker + response = self.call_api(self.client_user, data) + self.assertEqual(response.status_code, 200) + + def test_basic(self): + """ Check successful replacement """ + data = { + "username_mappings": [ + {self.user.username: self.new_username}, + ] + } + expected_response = { + 'failed_replacements': [], + 'successful_replacements': data["username_mappings"] + } + self.register_get_username_replacement_response(self.user) + response = self.call_api(self.client_user, data) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, expected_response) + + def test_not_authenticated(self): + """ + Override the parent implementation of this, we JWT auth for this API + """ + pass + + @ddt.ddt @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/discussion_api/tests/utils.py b/lms/djangoapps/discussion_api/tests/utils.py index 39902d2669038668c33a28be224627d68f115112..4641ccc80721ee01b1d242da246a83952b0c2983 100644 --- a/lms/djangoapps/discussion_api/tests/utils.py +++ b/lms/djangoapps/discussion_api/tests/utils.py @@ -226,6 +226,15 @@ class CommentsServiceMockMixin(object): status=status ) + def register_get_username_replacement_response(self, user, status=200, body=""): + assert httpretty.is_enabled(), 'httpretty must be enabled to mock calls.' + httpretty.register_uri( + httpretty.POST, + "http://localhost:4567/api/v1/users/{id}/replace_username".format(id=user.id), + body=body, + status=status + ) + def register_subscribed_threads_response(self, user, threads, page, num_pages): """Register a mock response for GET on the CS user instance endpoint""" assert httpretty.is_enabled(), 'httpretty must be enabled to mock calls.' diff --git a/lms/djangoapps/discussion_api/urls.py b/lms/djangoapps/discussion_api/urls.py index f138bfcf41b868fa4bc53dbfa22deadcd949f959..76a6f156e37e2855b5af88d4aa2726ae68c26810 100644 --- a/lms/djangoapps/discussion_api/urls.py +++ b/lms/djangoapps/discussion_api/urls.py @@ -13,6 +13,7 @@ from discussion_api.views import ( CourseView, ThreadViewSet, RetireUserView, + ReplaceUsernamesView, ) ROUTER = SimpleRouter() @@ -40,6 +41,7 @@ urlpatterns = [ name="discussion_course" ), url(r"^v1/accounts/retire_forum", RetireUserView.as_view(), name="retire_discussion_user"), + url(r"^v1/accounts/replace_username", ReplaceUsernamesView.as_view(), name="replace_discussion_username"), url( r"^v1/course_topics/{}".format(settings.COURSE_ID_PATTERN), CourseTopicsView.as_view(), diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 872897ab205e6429c7bb1b4a96cbdad736388f61..0000bb156307acf4f0f6f8531370e244ae9648ce 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -1,13 +1,16 @@ """ Discussion API views """ +import logging + +from django.contrib.auth.models import User from django.core.exceptions import ValidationError from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from opaque_keys.edx.keys import CourseKey from rest_framework import permissions from rest_framework import status -from rest_framework.exceptions import UnsupportedMediaType +from rest_framework.exceptions import ParseError, UnsupportedMediaType from rest_framework.parsers import JSONParser from rest_framework.response import Response from rest_framework.views import APIView @@ -49,11 +52,13 @@ from discussion_api.serializers import ( from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes -from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser +from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser from openedx.core.djangoapps.user_api.models import UserRetirementStatus from util.json_request import JsonResponse from xmodule.modulestore.django import modulestore +log = logging.getLogger(__name__) + @view_auth_classes() class CourseView(DeveloperErrorViewMixin, APIView): @@ -588,6 +593,107 @@ class RetireUserView(APIView): return Response(status=status.HTTP_204_NO_CONTENT) +class ReplaceUsernamesView(APIView): + """ + WARNING: This API is only meant to be used as part of a larger job that + updates usernames across all services. DO NOT run this alone or users will + not match across the system and things will be broken. + + API will recieve a list of current usernames and their new username. + + POST /api/discussion/v1/accounts/replace_usernames/ + { + "username_mappings": [ + {"current_username_1": "desired_username_1"}, + {"current_username_2": "desired_username_2"} + ] + } + """ + + authentication_classes = (JwtAuthentication,) + permission_classes = (permissions.IsAuthenticated, CanReplaceUsername) + + def post(self, request): + """ + Implements the username replacement endpoint + """ + + username_mappings = request.data.get("username_mappings") + + if not self._has_valid_schema(username_mappings): + raise ParseError("Request data does not match schema") + + successful_replacements, failed_replacements = [], [] + + for username_pair in username_mappings: + current_username = list(username_pair.keys())[0] + new_username = list(username_pair.values())[0] + successfully_replaced = self._replace_username(current_username, new_username) + if successfully_replaced: + successful_replacements.append({current_username: new_username}) + else: + failed_replacements.append({current_username: new_username}) + + return Response( + status=status.HTTP_200_OK, + data={ + "successful_replacements": successful_replacements, + "failed_replacements": failed_replacements + } + ) + + def _replace_username(self, current_username, new_username): + """ + Replaces the current username with the new username in the forums service + """ + try: + # This API will be called after the regular LMS API, so the username in + # the DB will have already been updated to new_username + current_user = User.objects.get(username=new_username) + cc_user = comment_client.User.from_django_user(current_user) + cc_user.replace_username(new_username) + except User.DoesNotExist: + log.warning( + u"Unable to change username from %s to %s in forums because %s doesn't exist in LMS DB.", + current_username, + new_username, + new_username, + ) + return True + except comment_client.CommentClientRequestError as exc: + if exc.status_code == 404: + log.info( + u"Unable to change username from %s to %s in forums because user doesn't exist in forums", + current_username, + new_username, + ) + return True + else: + log.exception( + u"Unable to change username from %s to %s in forums because forums API call failed with: %s.", + current_username, + new_username, + exc, + ) + return False + + log.info( + u"Successfully changed username from %s to %s in forums.", + current_username, + new_username, + ) + return True + + def _has_valid_schema(self, post_data): + """ Verifies the data is a list of objects with a single key:value pair """ + if not isinstance(post_data, list): + return False + for obj in post_data: + if not (isinstance(obj, dict) and len(obj) == 1): + return False + return True + + class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView): """ **Use Cases** diff --git a/lms/envs/common.py b/lms/envs/common.py index 6d76244ae1b9b284fbbc5ca1a983e037d8ae27a5..46e2e47c5bd52f4ca72fc410ffc16007ba5669a3 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3437,6 +3437,8 @@ RETIREMENT_STATES = [ 'COMPLETE', ] +USERNAME_REPLACEMENT_WORKER = "REPLACE WITH VALID USERNAME" + ############## Settings for Microfrontends ######################### # If running a Gradebook container locally, # modify lms/envs/private.py to give it a non-null value diff --git a/lms/lib/comment_client/user.py b/lms/lib/comment_client/user.py index d358f09c4068a066a5683a8ca9ccf7434599ea23..a007bd1d7512707f70fc7bce3b724646fcd27a9b 100644 --- a/lms/lib/comment_client/user.py +++ b/lms/lib/comment_client/user.py @@ -180,6 +180,17 @@ class User(models.Model): metric_tags=self._metric_tags ) + def replace_username(self, new_username): + url = _url_for_username_replacement(self.id) + params = {"new_username": new_username} + + utils.perform_request( + 'post', + url, + params, + raw=True, + ) + def _url_for_vote_comment(comment_id): return "{prefix}/comments/{comment_id}/votes".format(prefix=settings.PREFIX, comment_id=comment_id) @@ -213,3 +224,10 @@ def _url_for_retire(user_id): Returns cs_comments_service url endpoint to retire a user (remove all post content, etc.) """ return "{prefix}/users/{user_id}/retire".format(prefix=settings.PREFIX, user_id=user_id) + + +def _url_for_username_replacement(user_id): + """ + Returns cs_comments_servuce url endpoint to replace the username of a user + """ + return "{prefix}/users/{user_id}/replace_username".format(prefix=settings.PREFIX, user_id=user_id) diff --git a/openedx/core/djangoapps/user_api/accounts/permissions.py b/openedx/core/djangoapps/user_api/accounts/permissions.py index fd7b197f9c901e4f5602183c02335ee520b75d6d..7b14ce00c6498b36625b1ca9e69232c7c43f534d 100644 --- a/openedx/core/djangoapps/user_api/accounts/permissions.py +++ b/openedx/core/djangoapps/user_api/accounts/permissions.py @@ -3,6 +3,7 @@ Permissions classes for User accounts API views. """ from __future__ import unicode_literals +from django.conf import settings from rest_framework import permissions @@ -23,3 +24,11 @@ class CanRetireUser(permissions.BasePermission): """ def has_permission(self, request, view): return request.user.has_perm('accounts.can_retire_user') + + +class CanReplaceUsername(permissions.BasePermission): + """ + Grants access to the Username Replacement API for the service user. + """ + def has_permission(self, request, view): + return request.user.username == getattr(settings, "USERNAME_REPLACEMENT_WORKER", False) 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 8a1fb22e83f51fdd4cb9f4116934033c7c429704..0f342000318ae7fdf24f9b969e86063334129d4b 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -17,6 +17,7 @@ import mock import pytz from rest_framework.test import APIClient, APITestCase +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user 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 @@ -933,3 +934,81 @@ class TestAccountAPITransactions(TransactionTestCase): data = response.data self.assertEqual(old_email, data["email"]) self.assertEqual(u"m", data["gender"]) + + +@ddt.ddt +@mock.patch('django.conf.settings.USERNAME_REPLACEMENT_WORKER', 'test_replace_username_service_worker') +class UsernameReplacementViewTests(APITestCase): + """ Tests UsernameReplacementView """ + SERVICE_USERNAME = 'test_replace_username_service_worker' + + def setUp(self): + super(UsernameReplacementViewTests, self).setUp() + self.service_user = UserFactory(username=self.SERVICE_USERNAME) + self.url = reverse("username_replacement") + + def build_jwt_headers(self, user): + """ + Helper function for creating headers for the JWT authentication. + """ + token = create_jwt_for_user(user) + headers = {'HTTP_AUTHORIZATION': u'JWT {}'.format(token)} + return headers + + def call_api(self, user, data): + """ Helper function to call API with data """ + data = json.dumps(data) + headers = self.build_jwt_headers(user) + return self.client.post(self.url, data, content_type='application/json', **headers) + + def test_auth(self): + """ Verify the endpoint only works with the service worker """ + data = { + "username_mappings": [ + {"test_username_1": "test_new_username_1"}, + {"test_username_2": "test_new_username_2"} + ] + } + + # Test unauthenticated + response = self.client.post(self.url) + self.assertEqual(response.status_code, 401) + + # Test non-service worker + random_user = UserFactory() + response = self.call_api(random_user, data) + self.assertEqual(response.status_code, 403) + + # Test service worker + response = self.call_api(self.service_user, data) + self.assertEqual(response.status_code, 200) + + @ddt.data( + [{}, {}], + {}, + [{"test_key": "test_value", "test_key_2": "test_value_2"}] + ) + def test_bad_schema(self, mapping_data): + """ Verify the endpoint rejects bad data schema """ + data = { + "username_mappings": mapping_data + } + response = self.call_api(self.service_user, data) + self.assertEqual(response.status_code, 400) + + def test_existing_and_non_existing_users(self): + """ Tests a mix of existing and non existing users """ + random_users = [UserFactory() for _ in range(5)] + fake_usernames = ["myname_" + str(x) for x in range(5)] + existing_users = [{user.username: user.username + '_new'} for user in random_users] + non_existing_users = [{username: username + '_new'} for username in fake_usernames] + data = { + "username_mappings": existing_users + non_existing_users + } + expected_response = { + 'failed_replacements': [], + 'successful_replacements': existing_users + non_existing_users + } + response = self.call_api(self.service_user, data) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, expected_response) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index d3a6a8457a9edfee9c09b7f2792dc513cce65036..4ee7fbaa2e951b67b0c8c2dfac7e1e8950b5f996 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -7,9 +7,11 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API import datetime import logging from functools import wraps +import uuid import pytz from consent.models import DataSharingConsent +from django.apps import apps from django.conf import settings from django.contrib.auth import authenticate, get_user_model, logout from django.contrib.sites.models import Site @@ -27,6 +29,7 @@ from rest_framework import permissions, status from rest_framework.authentication import SessionAuthentication from rest_framework.parsers import JSONParser from rest_framework.response import Response +from rest_framework.serializers import ValidationError from rest_framework.views import APIView from rest_framework.viewsets import ViewSet from six import iteritems, text_type @@ -71,7 +74,7 @@ from ..models import ( UserRetirementStatus ) from .api import get_account_settings, update_account_settings -from .permissions import CanDeactivateUser, CanRetireUser +from .permissions import CanDeactivateUser, CanReplaceUsername, CanRetireUser from .serializers import UserRetirementPartnerReportSerializer, UserRetirementStatusSerializer from .signals import ( USER_RETIRE_LMS_CRITICAL, @@ -1002,3 +1005,167 @@ class AccountRetirementView(ViewSet): """ for entitlement in CourseEntitlement.objects.filter(user_id=user.id): entitlement.courseentitlementsupportdetail_set.all().update(comments='') + + +class UsernameReplacementView(APIView): + """ + WARNING: This API is only meant to be used as part of a larger job that + updates usernames across all services. DO NOT run this alone or users will + not match across the system and things will be broken. + + API will recieve a list of current usernames and their requested new + username. If their new username is taken, it will randomly assign a new username. + + This API will be called first, before calling the APIs in other services as this + one handles the checks on the usernames provided. + """ + authentication_classes = (JwtAuthentication, ) + permission_classes = (permissions.IsAuthenticated, CanReplaceUsername) + + def post(self, request): + """ + POST /api/user/v1/accounts/replace_usernames/ + { + "username_mappings": [ + {"current_username_1": "desired_username_1"}, + {"current_username_2": "desired_username_2"} + ] + } + + **POST Parameters** + + A POST request must include the following parameter. + + * username_mappings: Required. A list of objects that map the current username (key) + to the desired username (value) + + **POST Response Values** + + As long as data validation passes, the request will return a 200 with a new mapping + of old usernames (key) to new username (value) + + { + "successful_replacements": [ + {"old_username_1": "new_username_1"} + ], + "failed_replacements": [ + {"old_username_2": "new_username_2"} + ] + } + """ + + # (model_name, column_name) + MODELS_WITH_USERNAME = ( + ('auth.user', 'username'), + ('consent.DataSharingConsent', 'username'), + ('consent.HistoricalDataSharingConsent', 'username'), + ('credit.CreditEligibility', 'username'), + ('credit.CreditRequest', 'username'), + ('credit.CreditRequirementStatus', 'username'), + ('user_api.UserRetirementPartnerReportingStatus', 'original_username'), + ('user_api.UserRetirementStatus', 'original_username') + ) + UNIQUE_SUFFIX_LENGTH = getattr(settings, 'SOCIAL_AUTH_UUID_LENGTH', 4) + + username_mappings = request.data.get("username_mappings") + replacement_locations = self._load_models(MODELS_WITH_USERNAME) + + if not self._has_valid_schema(username_mappings): + raise ValidationError("Request data does not match schema") + + successful_replacements, failed_replacements = [], [] + + for username_pair in username_mappings: + current_username = list(username_pair.keys())[0] + desired_username = list(username_pair.values())[0] + new_username = self._generate_unique_username(desired_username, suffix_length=UNIQUE_SUFFIX_LENGTH) + successfully_replaced = self._replace_username_for_all_models( + current_username, + new_username, + replacement_locations + ) + if successfully_replaced: + successful_replacements.append({current_username: new_username}) + else: + failed_replacements.append({current_username: new_username}) + return Response( + status=status.HTTP_200_OK, + data={ + "successful_replacements": successful_replacements, + "failed_replacements": failed_replacements + } + ) + + def _load_models(self, models_with_fields): + """ Takes tuples that contain a model path and returns the list with a loaded version of the model """ + try: + replacement_locations = [(apps.get_model(model), column) for (model, column) in models_with_fields] + except LookupError: + log.exception("Unable to load models for username replacement") + raise + return replacement_locations + + def _has_valid_schema(self, post_data): + """ Verifies the data is a list of objects with a single key:value pair """ + if not isinstance(post_data, list): + return False + for obj in post_data: + if not (isinstance(obj, dict) and len(obj) == 1): + return False + return True + + def _generate_unique_username(self, desired_username, suffix_length=4): + """ + Generates a unique username. + If the desired username is available, that will be returned. + Otherwise it will generate unique suffixs to the desired username until it is an available username. + """ + new_username = desired_username + # Keep checking usernames in case desired_username + random suffix is already taken + while True: + if User.objects.filter(username=new_username).exists(): + unique_suffix = uuid.uuid4().hex[:suffix_length] + new_username = desired_username + unique_suffix + else: + break + return new_username + + def _replace_username_for_all_models(self, current_username, new_username, replacement_locations): + """ + Replaces current_username with new_username for all (model, column) pairs in replacement locations. + Returns if it was successful or not. Will return successful even if no matching + + TODO: Determine if logs of username are a PII issue. + """ + try: + with transaction.atomic(): + num_rows_changed = 0 + for (model, column) in replacement_locations: + num_rows_changed += model.objects.filter( + **{column: current_username} + ).update( + **{column: new_username} + ) + except Exception as exc: # pylint: disable=broad-except + log.exception( + u"Unable to change username from %s to %s. Failed on table %s because %s", + current_username, + new_username, + model.__class__.__name__, # Retrieves the model name that it failed on + exc + ) + return False + if num_rows_changed == 0: + log.info( + u"Unable to change username from %s to %s because %s doesn't exist.", + current_username, + new_username, + current_username, + ) + else: + log.info( + u"Successfully changed username from %s to %s.", + current_username, + new_username, + ) + return True diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index bdea5668ca59df408ec45e879f2cb5dc33a16320..334ffc7e8c551dfd3c0b27d22838d956a262ac1f 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -13,7 +13,8 @@ from .accounts.views import ( AccountRetirementView, AccountViewSet, DeactivateLogoutView, - LMSAccountRetirementView + LMSAccountRetirementView, + UsernameReplacementView ) from .preferences.views import PreferencesDetailView, PreferencesView from .verification_api.views import IDVerificationStatusView @@ -150,6 +151,11 @@ urlpatterns = [ RETIREMENT_UPDATE, name='accounts_retirement_update' ), + url( + r'^v1/accounts/replace_usernames/$', + UsernameReplacementView.as_view(), + name='username_replacement' + ), url( r'^v1/validation/registration$', RegistrationValidationView.as_view(),