Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #29519 from edx/aakbar/PROD-2590

Browse Source 
feat: allow get account info on lms user id
This commit is contained in:
Ali Akbar
2021-12-10 12:50:13 +05:00
committed by GitHub
parent 9f9e0acc55 c07c5bc0dd
commit 7fe43a3cbf
3 changed files with 82 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
openedx/core/djangoapps/user_api/accounts/permissions.py
View File

@@ -2,7 +2,6 @@
Permissions classes for User accounts API views.
"""
from django.conf import settings
from rest_framework import permissions
@@ -12,6 +11,7 @@ class CanDeactivateUser(permissions.BasePermission):
Grants access to AccountDeactivationView if the requesting user is a superuser
or has the explicit permission to deactivate a User account.
"""
def has_permission(self, request, view):
return request.user.has_perm('student.can_deactivate_users')
@@ -22,6 +22,7 @@ class CanRetireUser(permissions.BasePermission):
a superuser, the RETIREMENT_SERVICE_USERNAME, or has the explicit permission to
retire a User account.
"""
def has_permission(self, request, view):
return request.user.has_perm('accounts.can_retire_user')
@@ -30,5 +31,18 @@ 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)
class CanGetAccountInfoUsingId(permissions.BasePermission):
"""
Grants access to AccountViewSet if the requesting user is a superuser/staff
and reqesting to get account info based on LMS User id
"""
def has_permission(self, request, view):
return request.GET.get('lms_user_id') is None or (
request.GET.get('lms_user_id') and (request.user.is_staff or request.user.is_superuser)
)

65
openedx/core/djangoapps/user_api/accounts/tests/test_views.py
View File

@@ -2,7 +2,6 @@
Test cases to cover Accounts-related behaviors of the User API application
"""
import datetime
import hashlib
import json
@@ -15,15 +14,16 @@ from django.conf import settings
from django.test.testcases import TransactionTestCase
from django.test.utils import override_settings
from django.urls import reverse
from rest_framework import status
from rest_framework.test import APIClient, APITestCase
from common.djangoapps.student.models import PendingEmailChange, UserProfile
from common.djangoapps.student.tests.factories import TEST_PASSWORD, RegistrationFactory, UserFactory
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
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from common.djangoapps.student.models import PendingEmailChange, UserProfile
from common.djangoapps.student.tests.factories import TEST_PASSWORD, UserFactory, RegistrationFactory
from .. import ALL_USERS_VISIBILITY, CUSTOM_VISIBILITY, PRIVATE_VISIBILITY
@@ -94,7 +94,7 @@ class UserAPITestCase(APITestCase):
"""
Helper method for sending a GET to the server. Verifies the expected status and returns the response.
"""
url = self.url + '?' + query_parameters if query_parameters else self.url # pylint: disable=no-member
url = self.url + '?' + query_parameters if query_parameters else self.url # pylint: disable=no-member
response = client.get(url)
assert expected_status == response.status_code
return response
@@ -386,6 +386,34 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
response = self.send_get(client, query_parameters=f'email={self.user.email}')
self._verify_full_account_response(response)
def test_successful_get_account_by_user_id(self):
"""
Test that request using lms user id by a staff user successfully retrieves Account Info.
"""
api_client = "staff_client"
user = "staff_user"
client = self.login_client(api_client, user)
self.create_mock_profile(self.user)
set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY)
response = self.send_get(client, query_parameters=f'lms_user_id={self.user.id}')
self._verify_full_account_response(response)
def test_unsuccessful_get_account_by_user_id(self):
"""
Test that requesting using lms user id by a normal user fails to retrieve Account Info.
"""
api_client = "client"
user = "user"
client = self.login_client(api_client, user)
self.create_mock_profile(self.user)
set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY)
response = self.send_get(
client, query_parameters=f'lms_user_id={self.user.id}', expected_status=status.HTTP_403_FORBIDDEN
)
assert response.data.get('detail') == 'You do not have permission to perform this action.'
def test_search_emails(self):
client = self.login_client('staff_client', 'staff_user')
json_data = {'emails': [self.user.email]}
@@ -460,6 +488,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
Test the return from GET based on user visibility setting.
"""
def verify_fields_visible_to_all_users(response):
"""
Confirms that private fields are private, and public/shareable fields are public/shareable
@@ -688,11 +717,11 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
assert expected_user_message == error_response.data['field_errors'][field]['user_message']
assert "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']
assert "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']
elif field != "account_privacy":
# If there are no values that would fail validation, then empty string should be supported;
@@ -720,8 +749,9 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
Internal helper to check the error messages returned
"""
assert 'This field is not editable via this API' == data['field_errors'][field_name]['developer_message']
assert "The '{}' field cannot be edited."\
.format(field_name) == data['field_errors'][field_name]['user_message']
assert "The '{}' field cannot be edited.".format(
field_name
) == data['field_errors'][field_name]['user_message']
for field_name in ["username", "date_joined", "is_active", "profile_image", "requires_parental_consent"]:
response = self.send_patch(client, {field_name: "will_error", "gender": "o"}, expected_status=400)
@@ -765,6 +795,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
Test the metadata stored when changing the name field.
"""
def get_name_change_info(expected_entries):
"""
Internal method to encapsulate the retrieval of old names used
@@ -799,7 +830,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
# 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, self.user.username, "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")
@mock.patch.dict(
@@ -850,7 +881,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
# Try changing to an invalid email to make sure error messages are appropriately returned.
error_response = self.send_patch(client, {"email": bad_email}, expected_status=400)
field_errors = error_response.data["field_errors"]
assert "Error thrown from validate_new_email: 'Valid e-mail address required.'" ==\
assert "Error thrown from validate_new_email: 'Valid e-mail address required.'" == \
field_errors['email']['developer_message']
assert 'Valid e-mail address required.' == field_errors['email']['user_message']
@@ -919,7 +950,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
client = self.login_client("client", "user")
response = self.send_patch(client, {"language_proficiencies": patch_value}, expected_status=400)
assert response.data['field_errors']['language_proficiencies']['developer_message'] == \
f"Value '{patch_value}' is not valid for field 'language_proficiencies': {expected_error_message}"
f"Value '{patch_value}' is not valid for field 'language_proficiencies': {expected_error_message}"
@mock.patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save')
def test_patch_serializer_save_fails(self, serializer_save):
@@ -941,9 +972,9 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.client.login(username=self.user.username, password=TEST_PASSWORD)
response = self.send_get(self.client)
assert response.data['profile_image'] ==\
{'has_image': False,
'image_url_full': 'http://testserver/static/default_50.png',
assert response.data['profile_image'] == \
{'has_image': False,
'image_url_full': 'http://testserver/static/default_50.png',
'image_url_small': 'http://testserver/static/default_10.png'}
@ddt.data(

31
openedx/core/djangoapps/user_api/accounts/views.py
View File

@@ -5,15 +5,12 @@ For additional information and historical context, see:
https://openedx.atlassian.net/wiki/display/TNL/User+API
"""
import datetime
import logging
import uuid
from functools import wraps
import pytz
from rest_framework.exceptions import UnsupportedMediaType
from consent.models import DataSharingConsent
from django.apps import apps
from django.conf import settings
@@ -31,6 +28,7 @@ from integrated_channels.degreed.models import DegreedLearnerDataTransmissionAud
from integrated_channels.sap_success_factors.models import SapSuccessFactorsLearnerDataTransmissionAudit
from rest_framework import permissions, status
from rest_framework.authentication import SessionAuthentication
from rest_framework.exceptions import UnsupportedMediaType
from rest_framework.parsers import JSONParser
from rest_framework.response import Response
from rest_framework.serializers import ValidationError
@@ -41,13 +39,11 @@ from wiki.models.pluginbase import RevisionPluginRevision
from common.djangoapps.entitlements.models import CourseEntitlement
from common.djangoapps.student.models import ( # lint-amnesty, pylint: disable=unused-import
AccountRecovery,
CourseEnrollmentAllowed,
LoginFailures,
ManualEnrollmentAudit,
PendingEmailChange,
PendingNameChange,
Registration,
User,
UserProfile,
get_potentially_retired_user_by_username,
@@ -78,7 +74,7 @@ from ..models import (
UserRetirementStatus
)
from .api import get_account_settings, update_account_settings
from .permissions import CanDeactivateUser, CanReplaceUsername, CanRetireUser
from .permissions import CanDeactivateUser, CanGetAccountInfoUsingId, CanReplaceUsername, CanRetireUser
from .serializers import (
PendingNameChangeSerializer,
UserRetirementPartnerReportSerializer,
@@ -114,6 +110,7 @@ def request_requires_username(function):
Requires that a ``username`` key containing a truthy value exists in
the ``request.data`` attribute of the decorated function.
"""
@wraps(function)
def wrapper(self, request): # pylint: disable=missing-docstring
username = request.data.get('username', None)
@@ -123,6 +120,7 @@ def request_requires_username(function):
data={'message': 'The user was not specified.'}
)
return function(self, request)
return wrapper
@@ -293,7 +291,7 @@ class AccountViewSet(ViewSet):
authentication_classes = (
JwtAuthentication, BearerAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser
)
permission_classes = (permissions.IsAuthenticated,)
permission_classes = (permissions.IsAuthenticated, CanGetAccountInfoUsingId)
parser_classes = (JSONParser, MergePatchParser,)
def get(self, request):
@@ -306,9 +304,11 @@ class AccountViewSet(ViewSet):
"""
GET /api/user/v1/accounts?username={username1,username2}
GET /api/user/v1/accounts?email={user_email}
GET /api/user/v1/accounts?lms_user_id={lms_user_id} (Staff Only)
"""
usernames = request.GET.get('username')
user_email = request.GET.get('email')
lms_user_id = request.GET.get('lms_user_id')
search_usernames = []
if usernames:
@@ -320,9 +320,16 @@ class AccountViewSet(ViewSet):
except (UserNotFound, User.DoesNotExist):
return Response(status=status.HTTP_404_NOT_FOUND)
search_usernames = [user.username]
elif lms_user_id:
try:
user = User.objects.get(id=lms_user_id)
except (UserNotFound, User.DoesNotExist):
return Response(status=status.HTTP_404_NOT_FOUND)
search_usernames = [user.username]
try:
account_settings = get_account_settings(
request, search_usernames, view=request.query_params.get('view'))
request, search_usernames, view=request.query_params.get('view')
)
except UserNotFound:
return Response(status=status.HTTP_404_NOT_FOUND)
@@ -459,7 +466,7 @@ class AccountDeactivationView(APIView):
Account deactivation viewset. Currently only supports POST requests.
Only admins can deactivate accounts.
"""
authentication_classes = (JwtAuthentication, )
authentication_classes = (JwtAuthentication,)
permission_classes = (permissions.IsAuthenticated, CanDeactivateUser)
def post(self, request, username):
@@ -505,8 +512,8 @@ class DeactivateLogoutView(APIView):
- Log the user out
- Create a row in the retirement table for that user
"""
authentication_classes = (JwtAuthentication, SessionAuthentication, )
permission_classes = (permissions.IsAuthenticated, )
authentication_classes = (JwtAuthentication, SessionAuthentication,)
permission_classes = (permissions.IsAuthenticated,)
def post(self, request):
"""
@@ -1208,7 +1215,7 @@ class UsernameReplacementView(APIView):
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, )
authentication_classes = (JwtAuthentication,)
permission_classes = (permissions.IsAuthenticated, CanReplaceUsername)
def post(self, request):