From c0cb493a4161dabe1fc80d481bcd6058cd782a00 Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Mon, 4 Mar 2019 15:24:25 -0500 Subject: [PATCH] Filter certificate list API based on user profile visibility preference. --- .../certificates/apis/v0/tests/test_views.py | 54 +++++++++++++++++-- lms/djangoapps/certificates/apis/v0/views.py | 20 ++++++- lms/envs/common.py | 1 + .../user_api/accounts/tests/test_api.py | 1 + .../user_api/accounts/tests/test_views.py | 8 +-- 5 files changed, 76 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index 534c7d3f77..03f77fb891 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -16,6 +16,7 @@ from lms.djangoapps.certificates.apis.v0.views import CertificatesDetailView, Ce from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from openedx.core.djangoapps.oauth_dispatch.toggles import ENFORCE_JWT_SCOPES +from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory from openedx.core.djangoapps.user_authn.tests.utils import AuthType, AuthAndScopesTestMixin, JWT_AUTH_TYPES from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -175,6 +176,53 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(len(resp.data), 0) + @ddt.data(*product(list(AuthType), (True, False))) + @ddt.unpack + def test_another_user_with_certs_shared_public(self, auth_type, scopes_enforced): + """ + Returns 200 with cert list for OAuth, Session, and JWT auth. + Returns 200 for jwt_restricted and user:me filter unset. + """ + self.student.profile.year_of_birth = 1977 + self.student.profile.save() + UserPreferenceFactory.build( + user=self.student, + key='account_privacy', + value='all_users', + ).save() + + with ENFORCE_JWT_SCOPES.override(active=scopes_enforced): + resp = self.get_response(auth_type, requesting_user=self.other_student) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(len(resp.data), 1) + + @ddt.data(*product(list(AuthType), (True, False))) + @ddt.unpack + def test_another_user_with_certs_shared_custom(self, auth_type, scopes_enforced): + """ + Returns 200 with cert list for OAuth, Session, and JWT auth. + Returns 200 for jwt_restricted and user:me filter unset. + """ + self.student.profile.year_of_birth = 1977 + self.student.profile.save() + UserPreferenceFactory.build( + user=self.student, + key='account_privacy', + value='custom', + ).save() + UserPreferenceFactory.build( + user=self.student, + key='visibility.course_certificates', + value='all_users', + ).save() + + with ENFORCE_JWT_SCOPES.override(active=scopes_enforced): + resp = self.get_response(auth_type, requesting_user=self.other_student) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(len(resp.data), 1) + @patch('edx_rest_framework_extensions.permissions.log') @ddt.data(*product(JWT_AUTH_TYPES, (True, False))) @ddt.unpack @@ -210,7 +258,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC def test_query_counts(self): # Test student with no certificates student_no_cert = UserFactory.create(password=self.user_password) - with self.assertNumQueries(21): + with self.assertNumQueries(22): resp = self.get_response( AuthType.jwt, requesting_user=student_no_cert, @@ -220,7 +268,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC self.assertEqual(len(resp.data), 0) # Test student with 1 certificate - with self.assertNumQueries(29): + with self.assertNumQueries(30): resp = self.get_response( AuthType.jwt, requesting_user=self.student, @@ -253,7 +301,7 @@ class CertificatesListRestApiTest(AuthAndScopesTestMixin, SharedModuleStoreTestC download_url='www.google.com', grade="0.88", ) - with self.assertNumQueries(29): + with self.assertNumQueries(30): resp = self.get_response( AuthType.jwt, requesting_user=student_2_certs, diff --git a/lms/djangoapps/certificates/apis/v0/views.py b/lms/djangoapps/certificates/apis/v0/views.py index 98cfb971a6..93f472fdfe 100644 --- a/lms/djangoapps/certificates/apis/v0/views.py +++ b/lms/djangoapps/certificates/apis/v0/views.py @@ -1,6 +1,7 @@ """ API v0 views. """ import logging +from django.contrib.auth import get_user_model from rest_condition import C from rest_framework.generics import GenericAPIView from rest_framework.permissions import IsAuthenticated @@ -14,10 +15,12 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.certificates.api import certificates_viewable_for_course from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.user_api.accounts.api import visible_fields from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser log = logging.getLogger(__name__) +User = get_user_model() class CertificatesDetailView(GenericAPIView): @@ -215,7 +218,7 @@ class CertificatesListView(GenericAPIView): A JSON serialized representation of the list of certificates. """ user_certs = [] - if request.user.username == username or request.user.is_staff: + if self._viewable_by_requestor(request, username): for user_cert in self._get_certificates_for_user(username): user_certs.append({ 'username': user_cert.get('username'), @@ -232,6 +235,21 @@ class CertificatesListView(GenericAPIView): return Response(user_certs) + def _viewable_by_requestor(self, request, username): + """ + Returns whether or not the requesting user is allowed to view the given user's certificates. + """ + try: + user = User.objects.select_related('profile').get(username=username) + except User.DoesNotExist: + return False + + is_owner = request.user.username == username + is_staff = request.user.is_staff + certificates_viewable = 'course_certificates' in visible_fields(user.profile, user) + + return is_owner or is_staff or certificates_viewable + def _get_certificates_for_user(self, username): """ Returns a user's viewable certificates sorted by course name. diff --git a/lms/envs/common.py b/lms/envs/common.py index 7de94110cf..20d60d39ad 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2946,6 +2946,7 @@ ACCOUNT_VISIBILITY_CONFIGURATION = { ACCOUNT_VISIBILITY_CONFIGURATION["shareable_fields"] = ( ACCOUNT_VISIBILITY_CONFIGURATION["public_fields"] + [ 'bio', + 'course_certificates', 'country', 'date_joined', 'language_proficiencies', diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 88364afb57..95f7635b6e 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -424,6 +424,7 @@ class AccountSettingsOnCreationTest(TestCase): 'extended_profile': [], 'secondary_email': None, 'time_zone': None, + 'course_certificates': None, }) def test_normalize_password(self): 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 bad7effa5a..7cd79c3006 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -230,7 +230,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): Verify that the shareable fields from the account are returned """ data = response.data - self.assertEqual(11, len(data)) + self.assertEqual(12, len(data)) # public fields (3) self.assertEqual(account_privacy, data["account_privacy"]) @@ -262,7 +262,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): Verify that all account fields are returned (even those that are not shareable). """ data = response.data - self.assertEqual(21, len(data)) + self.assertEqual(22, len(data)) # public fields (3) expected_account_privacy = ( @@ -470,7 +470,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): with self.assertNumQueries(queries): response = self.send_get(self.client) data = response.data - self.assertEqual(21, len(data)) + self.assertEqual(22, len(data)) self.assertEqual(self.user.username, data["username"]) self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"): @@ -877,7 +877,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): response = self.send_get(client) if has_full_access: data = response.data - self.assertEqual(21, len(data)) + self.assertEqual(22, len(data)) self.assertEqual(self.user.username, data["username"]) self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) self.assertEqual(self.user.email, data["email"])