From 023e6ec8cdbca25e1d3dec662302be2e5c1b8a72 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 20 Feb 2015 13:35:29 -0500 Subject: [PATCH] GET method for user account API. --- lms/djangoapps/mobile_api/testutils.py | 4 +- lms/djangoapps/mobile_api/utils.py | 10 +- lms/urls.py | 2 + .../djangoapps/user_api/accounts/__init__.py | 0 .../user_api/accounts/serializers.py | 26 ++++ .../user_api/accounts/tests/test_views.py | 111 ++++++++++++++++++ .../core/djangoapps/user_api/accounts/urls.py | 14 +++ .../djangoapps/user_api/accounts/views.py | 101 ++++++++++++++++ openedx/core/lib/api/permissions.py | 24 ++++ 9 files changed, 282 insertions(+), 10 deletions(-) create mode 100644 openedx/core/djangoapps/user_api/accounts/__init__.py create mode 100644 openedx/core/djangoapps/user_api/accounts/serializers.py create mode 100644 openedx/core/djangoapps/user_api/accounts/tests/test_views.py create mode 100644 openedx/core/djangoapps/user_api/accounts/urls.py create mode 100644 openedx/core/djangoapps/user_api/accounts/views.py diff --git a/lms/djangoapps/mobile_api/testutils.py b/lms/djangoapps/mobile_api/testutils.py index c5bc3f2872..09e2cab339 100644 --- a/lms/djangoapps/mobile_api/testutils.py +++ b/lms/djangoapps/mobile_api/testutils.py @@ -105,7 +105,7 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin): """ def test_invalid_user(self): self.login_and_enroll() - self.api_response(expected_response_code=403, username='no_user') + self.api_response(expected_response_code=404, username='no_user') def test_other_user(self): # login and enroll as the test user @@ -120,7 +120,7 @@ class MobileAuthUserTestMixin(MobileAuthTestMixin): # now login and call the API as the test user self.login() - self.api_response(expected_response_code=403, username=other.username) + self.api_response(expected_response_code=404, username=other.username) @ddt.ddt diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index cd7720f8a3..0b15facf48 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -11,6 +11,7 @@ from rest_framework.authentication import OAuth2Authentication, SessionAuthentic from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.django import modulestore from courseware.courses import get_course_with_access +from openedx.core.lib.api.permissions import IsUserInUrl def mobile_course_access(depth=0, verify_enrolled=True): @@ -43,13 +44,6 @@ def mobile_view(is_user=False): """ Function and class decorator that abstracts the authentication and permission checks for mobile api views. """ - class IsUser(permissions.BasePermission): - """ - Permission that checks to see if the request user matches the user in the URL. - """ - def has_permission(self, request, view): - return request.user.username == request.parser_context.get('kwargs', {}).get('username', None) - def _decorator(func_or_class): """ Requires either OAuth2 or Session-based authentication. @@ -58,6 +52,6 @@ def mobile_view(is_user=False): func_or_class.authentication_classes = (OAuth2Authentication, SessionAuthentication) func_or_class.permission_classes = (permissions.IsAuthenticated,) if is_user: - func_or_class.permission_classes += (IsUser,) + func_or_class.permission_classes += (IsUserInUrl,) return func_or_class return _decorator diff --git a/lms/urls.py b/lms/urls.py index a9ac601513..af4a6071b8 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -61,6 +61,8 @@ urlpatterns = ('', # nopep8 url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')), + url(r'^api/user/', include('openedx.core.djangoapps.user_api.accounts.urls')), + url(r'^notifier_api/', include('notifier_api.urls')), url(r'^lang_pref/', include('lang_pref.urls')), diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py new file mode 100644 index 0000000000..710450af67 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -0,0 +1,26 @@ +from rest_framework import serializers +from django.contrib.auth.models import User +from student.models import UserProfile + + +class AccountUserSerializer(serializers.HyperlinkedModelSerializer): + """ + Class that serializes the portion of User model needed for account information. + """ + class Meta: + model = User + fields = ("username", "email", "date_joined") + read_only_fields = ("username", "email", "date_joined") + + +class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer): + """ + Class that serializes the portion of UserProfile model needed for account information. + """ + class Meta: + model = UserProfile + fields = ( + "name", "gender", "goals", "year_of_birth", "level_of_education", "language", "city", "country", + "mailing_address" + ) + read_only_fields = ("name",) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py new file mode 100644 index 0000000000..ebb9e85d22 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -0,0 +1,111 @@ +import unittest +import ddt + +from django.test import TestCase +from django.core.urlresolvers import reverse +from django.conf import settings +from rest_framework.test import APITestCase, APIClient + +from student.tests.factories import UserFactory +from student.models import UserProfile + +TEST_PASSWORD = "test" + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class TestAccountAPI(APITestCase): + USERNAME = "Christina" + EMAIL = "christina@example.com" + PASSWORD = TEST_PASSWORD + + BAD_USERNAME = "Bad" + BAD_EMAIL = "bad@example.com" + BAD_PASSWORD = TEST_PASSWORD + + STAFF_USERNAME = "Staff" + STAFF_EMAIL = "staff@example.com" + STAFF_PASSWORD = TEST_PASSWORD + + def setUp(self): + super(TestAccountAPI, self).setUp() + + self.anonymous_client = APIClient() + + self.bad_user = UserFactory.create(password=TEST_PASSWORD) + self.bad_client = APIClient() + + self.staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD) + self.staff_client = APIClient() + + self.user = UserFactory.create(password=TEST_PASSWORD) + # Create some test profile values. + legacy_profile = UserProfile.objects.get(id=self.user.id) + legacy_profile.city = "Indi" + legacy_profile.country = "US" + legacy_profile.year_of_birth = 1900 + legacy_profile.level_of_education = "m" + legacy_profile.goals = "world peace" + legacy_profile.mailing_address = "North Pole" + legacy_profile.save() + + self.accounts_base_uri = reverse("accounts_api", kwargs={'username': self.user.username}) + + def test_get_account_anonymous_user(self): + response = self.anonymous_client.get(self.accounts_base_uri) + self.assert_status_code(401, response) + + def test_get_account_bad_user(self): + self.bad_client.login(username=self.bad_user.username, password=TEST_PASSWORD) + response = self.bad_client.get(self.accounts_base_uri) + self.assert_status_code(404, response) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_get_account(self, api_client, user): + client = self.login_client(api_client, user) + + response = client.get(self.accounts_base_uri) + self.assert_status_code(200, response) + data = response.data + self.assertEqual(12, len(data)) + self.assertEqual(self.user.username, data["username"]) + # TODO: should we rename this "full_name"? + self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) + self.assertEqual("Indi", data["city"]) + self.assertEqual("US", data["country"]) + # TODO: what should the format of this be? + 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("North Pole", data['mailing_address']) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["date_joined"]) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ) + @ddt.unpack + def test_patch_account(self, api_client, user): + client = self.login_client(api_client, user) + response = client.patch(self.accounts_base_uri, data={"usernamae": "willbeignored", "gender": "f"}) + self.assert_status_code(200, response) + data = response.data + # Note that username is read-only, so passing it in patch is ignored. We want to change this behavior so it throws an exception. + self.assertEqual(self.user.username, data["username"]) + self.assertEqual("f", data["gender"]) + + def assert_status_code(self, expected_status_code, response): + """Assert that the given response has the expected status code""" + self.assertEqual(expected_status_code, response.status_code) + + def login_client(self, api_client, user): + client = getattr(self, api_client) + user = getattr(self, user) + client.login(username=user.username, password=TEST_PASSWORD) + return client diff --git a/openedx/core/djangoapps/user_api/accounts/urls.py b/openedx/core/djangoapps/user_api/accounts/urls.py new file mode 100644 index 0000000000..05d94ec7a2 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/urls.py @@ -0,0 +1,14 @@ +from .views import AccountView + +from django.conf.urls import include, patterns, url + +USERNAME_PATTERN = r'(?P[\w.+-]+)' + +urlpatterns = patterns( + '', + url( + r'^v0/accounts/' + USERNAME_PATTERN + '$', + AccountView.as_view(), + name="accounts_api" + ) +) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py new file mode 100644 index 0000000000..b3f2564041 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -0,0 +1,101 @@ +from rest_framework.views import APIView +from django.core.exceptions import ObjectDoesNotExist +from django.contrib.auth.models import User +from rest_framework.response import Response +from rest_framework import status + +from student.models import UserProfile +from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer, AccountUserSerializer +from openedx.core.lib.api.permissions import IsUserInUrlOrStaff + +from rest_framework.authentication import OAuth2Authentication, SessionAuthentication +from rest_framework import permissions + + +class AccountView(APIView): + """ + **Use Cases** + + Get the user's account information. + + **Example Requests**: + + GET /api/user/v0/accounts/{username}/ + + **Response Values** + + * username: The username associated with the account (not editable). + + * name: The full name of the user (not editable through this API). + + * email: The email for the user (not editable through this API). + + * date_joined: The date this account was created (not editable). + + * gender: null, "m", "f", or "o": + + * year_of_birth: null or integer year: + + * level_of_education: null or one of the following choices: + + * "p" signifying "Doctorate" + * "m" signifying "Master's or professional degree" + * "b" signifying "Bachelor's degree" + * "a" signifying "Associate's degree" + * "hs" signifying "Secondary/high school" + * "jhs" signifying "Junior secondary/junior high/middle school" + * "el" signifying "Elementary/primary school" + * "none" signifying "None" + * "o" signifying "Other" + + * language: null or name of preferred language + + * city: null or name of city + + * country: null or a Country corresponding to one of the ISO 3166-1 countries + + * mailing_address: null or textual representation of mailing address + + * goals: null or textual representation of goals + + """ + authentication_classes = (OAuth2Authentication, SessionAuthentication) + permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff) + + def get(self, request, username): + """ + GET /api/user/v0/accounts/{username}/ + """ + existing_user, existing_user_profile = self._get_user_and_profile(username) + user_serializer = AccountUserSerializer(existing_user) + legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile) + + return Response(dict(user_serializer.data, **legacy_profile_serializer.data)) + + def patch(self, request, username): + """ + PATCH /api/user/v0/accounts/{username}/ + """ + existing_user, existing_user_profile = self._get_user_and_profile(username) + + user_serializer = AccountUserSerializer(existing_user, data=request.DATA) + user_serializer.is_valid() + user_serializer.save() + + legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=request.DATA) + legacy_profile_serializer.is_valid() + legacy_profile_serializer.save() + + return Response(dict(user_serializer.data, **legacy_profile_serializer.data)) + + def _get_user_and_profile(self, username): + """ + Helper method to return the legacy user and profile objects based on username. + """ + try: + existing_user = User.objects.get(username=username) + except ObjectDoesNotExist: + return Response({}, status=status.HTTP_404_NOT_FOUND) + existing_user_profile = UserProfile.objects.get(id=existing_user.id) + + return existing_user, existing_user_profile diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index dca589d88b..2b7c122665 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -1,6 +1,7 @@ from django.conf import settings from rest_framework import permissions from rest_framework.exceptions import PermissionDenied +from django.http import Http404 class ApiKeyHeaderPermission(permissions.BasePermission): @@ -31,3 +32,26 @@ class IsAuthenticatedOrDebug(permissions.BasePermission): user = getattr(request, 'user', None) return user and user.is_authenticated() + + +class IsUserInUrl(permissions.BasePermission): + """ + Permission that checks to see if the request user matches the user in the URL. + """ + def has_permission(self, request, view): + # Return a 404 instead of a 403 (Unauthorized). If one user is looking up + # other users, do not let them deduce the existence of an account. + if request.user.username != request.parser_context.get('kwargs', {}).get('username', None): + raise Http404() + return True + + +class IsUserInUrlOrStaff(IsUserInUrl): + """ + Permission that checks to see if the request user matches the user in the URL or has is_staff access. + """ + def has_permission(self, request, view): + if request.user.is_staff: + return True + + return super(IsUserInUrlOrStaff, self).has_permission(request, view)