Merge pull request #19853 from edx/tuchfarber/add_username_replacement_api

Adds username replacement API
This commit is contained in:
Matt Tuchfarber
2019-03-21 12:39:23 -04:00
committed by GitHub
11 changed files with 497 additions and 4 deletions

View File

@@ -1364,6 +1364,7 @@ ADVANCED_PROBLEM_TYPES = [
}
]
USERNAME_REPLACEMENT_WORKER = "REPLACE WITH VALID USERNAME"
# Files and Uploads type filter values

View File

@@ -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):

View File

@@ -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.'

View File

@@ -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(),

View File

@@ -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**

View File

@@ -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

View File

@@ -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)

View File

@@ -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)

View File

@@ -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)

View File

@@ -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

View File

@@ -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(),