diff --git a/lms/envs/common.py b/lms/envs/common.py index e4bbaa9ff5a2..e8f4d54ac0b5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2157,22 +2157,21 @@ add_optional_apps(OPTIONAL_APPS, INSTALLED_APPS) # noqa: F405 -###################### drf-spectacular (LMS enrollment schema) ###################### +###################### drf-spectacular (LMS API schema) ###################### SPECTACULAR_SETTINGS = { - 'TITLE': 'LMS Enrollment API', + 'TITLE': 'LMS API', 'VERSION': '0.1.0', 'SERVE_INCLUDE_SCHEMA': False, 'PREPROCESSING_HOOKS': ['lms.lib.spectacular.lms_api_filter'], - # Mark legacy slashless enrollment addresses deprecated. The enum hook is - # drf-spectacular's default, restated because setting this key replaces - # the default list. + # Mark superseded addresses deprecated. The enum hook is drf-spectacular's + # default, restated because setting this key replaces the default list. 'POSTPROCESSING_HOOKS': [ 'drf_spectacular.hooks.postprocess_schema_enums', 'lms.lib.spectacular.lms_mark_legacy_paths_deprecated', ], # Stripped from each path to derive its tags and operationId, so changing # it renames every operationId. Paths themselves are emitted in full. - 'SCHEMA_PATH_PREFIX': '/api/enrollment', + 'SCHEMA_PATH_PREFIX': r'/api/(enrollment|user)', # Serve the Swagger UI and ReDoc assets from drf-spectacular-sidecar rather # than drf-spectacular's default unpinned jsdelivr CDN URLs, keeping them # self-hosted and version-pinned as the drf-yasg bundles were. diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 789945a32c40..e74a18aaf8ce 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -583,7 +583,7 @@ def should_show_debug_toolbar(request): # pylint: disable=missing-function-docs 'COMPLETE', ] -###################### drf-spectacular (LMS enrollment schema) ###################### +###################### drf-spectacular (LMS API schema) ###################### SPECTACULAR_SETTINGS['SERVERS'] = [ # noqa: F405 {'url': LMS_ROOT_URL, 'description': 'Local'}, # noqa: F405 ] diff --git a/lms/envs/production.py b/lms/envs/production.py index 24350824c391..1defebe32992 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -398,7 +398,7 @@ def get_env_setting(setting): MIDDLEWARE.extend(_YAML_TOKENS.get('EXTRA_MIDDLEWARE_CLASSES', [])) # noqa: F405 -###################### drf-spectacular (LMS enrollment schema) ###################### +###################### drf-spectacular (LMS API schema) ###################### SPECTACULAR_SETTINGS['SERVERS'] = [ # noqa: F405 {'url': LMS_ROOT_URL, 'description': 'Local'}, # noqa: F405 ] diff --git a/lms/lib/spectacular.py b/lms/lib/spectacular.py index 942199deac69..cfd310de404a 100644 --- a/lms/lib/spectacular.py +++ b/lms/lib/spectacular.py @@ -11,21 +11,70 @@ # addresses already have distinct ids and keep them. COLLIDING_LEGACY_PATH = "/api/enrollment/v2/enrollments" +# Addresses listed here are superseded by a conforming address this schema also +# publishes. Each entry is spelled out rather than matched by prefix, so an +# address whose replacement has not been written yet is never marked. +SUPERSEDED_USER_PATHS = ( + "/api/user/v1/users/", + "/api/user/v1/users/{id}/", + "/api/user/v1/user_prefs/", + "/api/user/v1/user_prefs/{id}/", + "/api/user/v1/preferences/{pref_key}/users/", + "/api/user/v1/forum_roles/{name}/users/", + "/api/user/v1/preferences/time_zones/", +) + +PUBLISHED_PATH_PATTERN = re.compile(r"^/api/enrollment/v\d+/") + +# The user addresses this schema documents are the ones the version below +# serves, and beside each of those the address it supersedes. Apps older than +# that version are mounted under the same ``/api/user/v2/`` prefix, so matching +# the prefix alone would publish endpoints described nowhere here; the package +# a view comes from decides instead. +PUBLISHED_USER_PREFIX = "/api/user/v2/" +PUBLISHED_USER_VIEW_MODULE = "openedx.core.djangoapps.user_api.v2." + +# Every user address, whatever its version, belongs to one tag group. A tag is +# the first path segment left once the configured prefix is removed, which in a +# document that trims ``/api/user`` is the version, so user operations would be +# grouped with the enrollment operations that carry the same version. A document +# that trims nothing already derives a tag naming an area, and regrouping its +# operations would move them in every client generated from it. +USER_PATH_PREFIX = "/api/user/" +USER_TAG = "user" + +_PATH_PARAMETER = re.compile(r"\{([\w-]+)\}") +_VERSION_TAG = re.compile(r"v\d+") + +# The superseded addresses with their parameter names removed. The generator +# renames a ``{pk}`` parameter to ``{id}`` only after this filter has run, so +# the comparison cannot depend on how a parameter is spelled. +_SUPERSEDED_USER_SHAPES = frozenset(_PATH_PARAMETER.sub("{}", path) for path in SUPERSEDED_USER_PATHS) + def _is_legacy_path(path): """Conforming routes always end in a slash, so a slashless v2 path is a legacy address.""" return path.startswith(LEGACY_V2_PREFIX) and not path.endswith("/") +def _is_published_user_path(path, callback): + """A user address is documented when this version serves it, or when this version replaces it.""" + if _PATH_PARAMETER.sub("{}", path) in _SUPERSEDED_USER_SHAPES: + return True + if not path.startswith(PUBLISHED_USER_PREFIX): + return False + view = getattr(callback, "cls", None) + return getattr(view, "__module__", "").startswith(PUBLISHED_USER_VIEW_MODULE) + + def lms_api_filter(endpoints): """ - Pre-processing hook: keep only enrollment v2 endpoints tagged for the SDK. + Pre-processing hook: keep only the endpoints this schema documents. """ filtered = [] - ENROLLMENT_PATH_PATTERN = re.compile(r"^/api/enrollment/v\d+/") for path, path_regex, method, callback in endpoints: - if ENROLLMENT_PATH_PATTERN.match(path): + if PUBLISHED_PATH_PATTERN.match(path) or _is_published_user_path(path, callback): filtered.append((path, path_regex, method, callback)) return filtered @@ -33,26 +82,37 @@ def lms_api_filter(endpoints): class LmsAutoSchema(AutoSchema): """ - Give the colliding legacy Enrollment v2 address a distinct operationId. - - It and its conforming twin tokenize to the same operationId, so - drf-spectacular would otherwise break the tie with a numeral suffix in - registration order. The conforming address keeps the clean id; the - deprecated one is suffixed ``_legacy``. + Keep the one colliding address distinct, and group each operation under its own area. + + The slashless Enrollment v2 address tokenizes to the id of its conforming + twin, so the two would be told apart only by a numeral suffix handed out + in registration order, which says nothing about the operation and moves + when the routes move; it is suffixed ``_legacy`` instead, leaving the + conforming address the clean id. Every other address keeps the id and the + tag the generator derives for it, because a client generated from a + document takes a module from each operationId and groups it by tag, so + renaming either drops what a consumer already imports. """ def get_operation_id(self): - """Suffix the colliding legacy address so the pair never shares an operationId.""" + """Suffix the one colliding address so the pair never shares an operationId.""" operation_id = super().get_operation_id() if self.path == COLLIDING_LEGACY_PATH: return f"{operation_id}_legacy" return operation_id + def get_tags(self): + """Name the area whenever the tag derived for a user address would name a version.""" + tags = super().get_tags() + if self.path.startswith(USER_PATH_PREFIX) and tags and _VERSION_TAG.fullmatch(tags[0]): + return [USER_TAG] + return tags + def lms_mark_legacy_paths_deprecated(result, generator, request, public): # pylint: disable=unused-argument - """Mark the legacy slashless Enrollment v2 addresses ``deprecated: true``.""" + """Mark every superseded address ``deprecated: true``.""" for path, path_item in result.get("paths", {}).items(): - if not _is_legacy_path(path): + if not (_is_legacy_path(path) or path in SUPERSEDED_USER_PATHS): continue for operation in path_item.values(): if isinstance(operation, dict): diff --git a/lms/lib/tests/test_spectacular.py b/lms/lib/tests/test_spectacular.py index b611c8ad7776..66c193901f33 100644 --- a/lms/lib/tests/test_spectacular.py +++ b/lms/lib/tests/test_spectacular.py @@ -1,20 +1,33 @@ """Tests for the LMS drf-spectacular hooks and schema class.""" +import re +from collections import defaultdict +from types import SimpleNamespace from unittest import mock -from django.test import SimpleTestCase +from django.test import SimpleTestCase, TestCase from django.urls import path from drf_spectacular.generators import SchemaGenerator -from drf_spectacular.settings import spectacular_settings +from drf_spectacular.settings import patched_settings, spectacular_settings from rest_framework import serializers, viewsets +from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.settings import api_settings from lms.lib.spectacular import LmsAutoSchema, lms_api_filter, lms_mark_legacy_paths_deprecated +from openedx.core.apidocs import get_api_docs_settings +ENROLLMENT_VIEWS = "openedx.core.djangoapps.enrollments.views" +USER_API_V1_VIEWS = "openedx.core.djangoapps.user_api.views" +USER_API_V2_VIEWS = "openedx.core.djangoapps.user_api.v2.views" +USER_AUTHN_VIEWS = "openedx.core.djangoapps.user_authn.views.register" +COURSE_VIEWS = "lms.djangoapps.course_api.views" -def _endpoint(route): - return (route, route, "GET", None) + +def _endpoint(route, view_module=COURSE_VIEWS): + """An endpoint tuple as the generator hands it to the hook, served from ``view_module``.""" + callback = SimpleNamespace(cls=type("_View", (), {"__module__": view_module})) + return (route, route, "GET", callback) def _schema(*routes): @@ -22,19 +35,45 @@ def _schema(*routes): class LmsApiFilterTest(SimpleTestCase): - """The pre-processing hook keeps only enrollment endpoints.""" + """The hook keeps the enrollment mounts, this version of the user mounts, and what they supersede.""" - def test_keeps_enrollment_drops_the_rest(self): + def test_keeps_enrollment_this_version_and_the_addresses_it_supersedes(self): kept = lms_api_filter([ - _endpoint("/api/enrollment/v2/enrollments/"), - _endpoint("/api/enrollment/v1/enrollment"), - _endpoint("/api/user/v1/accounts/"), + _endpoint("/api/enrollment/v2/enrollments/", ENROLLMENT_VIEWS), + _endpoint("/api/enrollment/v1/enrollment", ENROLLMENT_VIEWS), + _endpoint("/api/user/v2/users/", USER_API_V2_VIEWS), + _endpoint("/api/user/v2/user_preferences/{username},{key}/", USER_API_V2_VIEWS), + # Superseded, and spelled with the parameter name the generator has + # not renamed to ``{id}`` yet. + _endpoint("/api/user/v1/users/{pk}/", USER_API_V1_VIEWS), + _endpoint("/api/user/v1/preferences/time_zones/", USER_API_V1_VIEWS), + # Neither served nor superseded by this version. + _endpoint("/api/user/v2/account/registration/", USER_AUTHN_VIEWS), + _endpoint("/api/user/v1/accounts", USER_API_V1_VIEWS), + _endpoint("/api/user/v1/me", USER_API_V1_VIEWS), + _endpoint("/api/user/v1/preferences/{username}", USER_API_V1_VIEWS), + _endpoint("/api/courses/v1/courses/", COURSE_VIEWS), + _endpoint("/api/user_tours/v1/tours/"), + _endpoint("/user_api/v1/users/", USER_API_V1_VIEWS), ]) assert [path for path, *_ in kept] == [ "/api/enrollment/v2/enrollments/", "/api/enrollment/v1/enrollment", + "/api/user/v2/users/", + "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v1/users/{pk}/", + "/api/user/v1/preferences/time_zones/", ] + def test_under_the_version_prefix_the_view_package_decides(self): + """Other apps are mounted under the same prefix, so the path alone cannot admit an address.""" + route = "/api/user/v2/account/registration/" + admitted = { + view_module: [path for path, *_ in lms_api_filter([_endpoint(route, view_module)])] + for view_module in (USER_AUTHN_VIEWS, USER_API_V2_VIEWS) + } + assert admitted == {USER_AUTHN_VIEWS: [], USER_API_V2_VIEWS: [route]} + class LmsMarkLegacyPathsDeprecatedTest(SimpleTestCase): """Slashless v2 paths are legacy and deprecated; slashed ones are conforming.""" @@ -54,6 +93,25 @@ def test_only_slashless_v2_paths_are_deprecated(self): assert "deprecated" not in paths["/api/enrollment/v2/courses/{course_key}/"]["get"] assert "deprecated" not in paths["/api/enrollment/v1/enrollment"]["get"] + def test_only_the_listed_user_addresses_are_deprecated(self): + result = lms_mark_legacy_paths_deprecated(_schema( + "/api/user/v1/users/", + "/api/user/v1/users/{id}/", + "/api/user/v1/preferences/time_zones/", + "/api/user/v1/accounts", + "/api/user/v1/preferences/{username}", + "/api/user/v2/users/", + "/user_api/v1/users/", + ), None, None, False) + paths = result["paths"] + assert paths["/api/user/v1/users/"]["get"]["deprecated"] is True + assert paths["/api/user/v1/users/{id}/"]["get"]["deprecated"] is True + assert paths["/api/user/v1/preferences/time_zones/"]["get"]["deprecated"] is True + assert "deprecated" not in paths["/api/user/v1/accounts"]["get"] + assert "deprecated" not in paths["/api/user/v1/preferences/{username}"]["get"] + assert "deprecated" not in paths["/api/user/v2/users/"]["get"] + assert "deprecated" not in paths["/user_api/v1/users/"]["get"] + def test_paths_are_left_as_full_urls(self): """Paths are not trimmed, so every key resolves against LMS_ROOT_URL.""" result = lms_mark_legacy_paths_deprecated(_schema("/api/enrollment/v2/enrollments/"), None, None, False) @@ -99,3 +157,120 @@ def test_only_the_colliding_legacy_address_is_suffixed(self): assert ids["/api/enrollment/v2/courses/{id}/"] == "v2_courses_retrieve" # Already distinct from every conforming id, so it keeps its name. assert ids["/api/enrollment/v2/enrollment/{id}"] == "v2_enrollment_retrieve" + + +class _UserApiViewSet(viewsets.GenericViewSet): + """Stand-in for a user view read both as a collection and as a member.""" + + serializer_class = _EmptySerializer + permission_classes = (AllowAny,) + + def list(self, request): + return Response([]) + + def retrieve(self, request, **kwargs): + return Response({}) + + +def _user_routes(schema_class): + """User, enrollment and non-``/api/`` addresses, inspected by ``schema_class``.""" + view = type("_InspectedViewSet", (_UserApiViewSet,), {"schema": schema_class()}) + return [ + path("api/user/v1/accounts", view.as_view({"get": "retrieve"})), + path("api/user/v1/accounts/", view.as_view({"get": "retrieve"})), + path("api/user/v1/preferences/", view.as_view({"get": "retrieve"})), + path( + "api/user/v1/preferences//", + view.as_view({"get": "retrieve"}), + ), + path("api/user/v1/users/", view.as_view({"get": "list"})), + path("api/user/v2/users/", view.as_view({"get": "list"})), + path("api/enrollment/v1/enrollments/", view.as_view({"get": "list"})), + # Outside ``/api/``, so a document that derives its prefix from the + # routes it covers derives the one the service-wide document has. + path("user_api/v1/preferences/", view.as_view({"get": "retrieve"})), + ] + + +def _named_operations(schema_class, patches): + """The ``{address: (operationId, tags)}`` ``schema_class`` produces under ``patches``.""" + with patched_settings({"PREPROCESSING_HOOKS": [], **patches}): + schema = SchemaGenerator(patterns=_user_routes(schema_class)).get_schema(request=None, public=True) + return { + route: (operation["operationId"], tuple(operation["tags"])) + for route, path_item in schema["paths"].items() + for operation in path_item.values() + } + + +class UserOperationIdTest(SimpleTestCase): + """User addresses keep the names the generator derives; only their tag group is chosen here.""" + + def test_the_service_wide_document_keeps_every_name_the_generator_derives(self): + """Each of these is the name that document already published, and a client imports it by name.""" + assert _named_operations(LmsAutoSchema, get_api_docs_settings()) == { + "/api/user/v1/accounts": ("api_user_v1_accounts_retrieve", ("api",)), + "/api/user/v1/accounts/{username}": ("api_user_v1_accounts_retrieve_2", ("api",)), + "/api/user/v1/preferences/{username}": ("api_user_v1_preferences_retrieve", ("api",)), + "/api/user/v1/preferences/{username}/{preference_key}": + ("api_user_v1_preferences_retrieve_2", ("api",)), + "/api/user/v1/users/": ("api_user_v1_users_list", ("api",)), + "/api/user/v2/users/": ("api_user_v2_users_list", ("api",)), + "/api/enrollment/v1/enrollments/": ("api_enrollment_v1_enrollments_list", ("api",)), + "/user_api/v1/preferences/{username}": ("user_api_v1_preferences_retrieve", ("user_api",)), + } + + def test_a_user_address_is_grouped_by_area_where_the_derived_tag_would_name_a_version(self): + """In the document that trims the area off, the derived tag is the version, and ids are untouched.""" + assert _named_operations(LmsAutoSchema, {}) == { + "/api/user/v1/accounts": ("v1_accounts_retrieve", ("user",)), + "/api/user/v1/accounts/{username}": ("v1_accounts_retrieve_2", ("user",)), + "/api/user/v1/preferences/{username}": ("v1_preferences_retrieve", ("user",)), + "/api/user/v1/preferences/{username}/{preference_key}": ("v1_preferences_retrieve_2", ("user",)), + "/api/user/v1/users/": ("v1_users_list", ("user",)), + "/api/user/v2/users/": ("v2_users_list", ("user",)), + "/api/enrollment/v1/enrollments/": ("v1_enrollments_list", ("v1",)), + "/user_api/v1/preferences/{username}": ("user_api_v1_preferences_retrieve", ("user_api",)), + } + + +class PublishedDocumentTest(TestCase): + """What the service actually publishes: distinct ids, and one area per tag group.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.schema = SchemaGenerator().get_schema(request=None, public=True) + + @classmethod + def operations(cls): + """Yield every (path, operation) the published document carries.""" + for route, path_item in cls.schema["paths"].items(): + for operation in path_item.values(): + if isinstance(operation, dict) and "operationId" in operation: + yield route, operation + + def test_every_operation_id_is_unique(self): + ids = [operation["operationId"] for _route, operation in self.operations()] + assert sorted(ids) == sorted(set(ids)) + + def test_only_the_one_enrollment_pair_is_told_apart_by_a_numeral_suffix(self): + """A numeral suffix is assigned in route order, so it is not a stable name.""" + suffixed = { + route: operation["operationId"] + for route, operation in self.operations() + if re.fullmatch(r".+_\d+", operation["operationId"]) + } + # Renaming this one would change an id the document already published, so it stays. + assert suffixed == {"/api/enrollment/v1/enrollment/{course_id}": "v1_enrollment_retrieve_2"} + + def test_no_tag_group_holds_two_areas(self): + areas = defaultdict(set) + for route, operation in self.operations(): + for tag in operation.get("tags", []): + areas[tag].add(route.split("/")[2]) + assert {tag: sorted(members) for tag, members in areas.items()} == { + "v1": ["enrollment"], + "openedx-platform-sdk": ["enrollment"], + "user": ["user"], + } diff --git a/lms/urls.py b/lms/urls.py index 35a1b6cdabcd..82526e9a83ef 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -142,6 +142,7 @@ path('api/courses/', include('lms.djangoapps.course_api.urls')), # User API endpoints + path('api/user/v2/', include('openedx.core.djangoapps.user_api.v2.urls')), path('api/user/', include('openedx.core.djangoapps.user_api.urls')), # Note: these are older versions of the User API that will eventually be # subsumed by api/user listed above. diff --git a/openedx/core/apidocs.py b/openedx/core/apidocs.py index 4c96c8cce109..acf4de94ce16 100644 --- a/openedx/core/apidocs.py +++ b/openedx/core/apidocs.py @@ -21,7 +21,7 @@ # These are passed as ``custom_settings`` to SpectacularAPIView rather than # living in ``SPECTACULAR_SETTINGS``, because that global is already claimed by # a deliberately narrow schema in each service -- the Authoring API -# (``/authoring-api/``) in CMS and the Enrollment API (``/lms-api/``) in LMS. +# (``/authoring-api/``) in CMS and the LMS API (``/lms-api/``) in LMS. # Both filter the surface down via ``PREPROCESSING_HOOKS`` and trim a path # prefix, so ``/api-docs`` must switch that filtering off explicitly to cover # the whole service. diff --git a/openedx/core/djangoapps/user_api/v2/__init__.py b/openedx/core/djangoapps/user_api/v2/__init__.py new file mode 100644 index 000000000000..333c1a588afe --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/__init__.py @@ -0,0 +1 @@ +"""Version 2 of the User API.""" diff --git a/openedx/core/djangoapps/user_api/v2/errors.py b/openedx/core/djangoapps/user_api/v2/errors.py new file mode 100644 index 000000000000..223f7d59e69a --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/errors.py @@ -0,0 +1,36 @@ +"""Error types this version of the User API classifies for itself.""" + +from edx_rest_framework_extensions.errors import register_error_type +from rest_framework.exceptions import MethodNotAllowed, NotAcceptable + + +class RefusedMethod(MethodNotAllowed): + """A request method this API does not serve at the address it was sent to.""" + + +class UnsatisfiableAccept(NotAcceptable): + """An ``Accept`` header naming no representation this API can produce.""" + + +register_error_type(RefusedMethod, "method-not-allowed", "Method Not Allowed") +register_error_type(UnsatisfiableAccept, "not-acceptable", "Not Acceptable") + + +class CatalogedRequestErrorsMixin: + """ + Names the error type of a refused method and of an unanswerable ``Accept`` header. + + The shared catalog classifies neither, so both would reach the caller + described as a server fault at a 4xx status. Raising this API's own + subclasses of them names each error without changing how the same failure + is classified for any other API in the process; once the shared catalog + covers them, the subclasses can give way to it. + """ + + def handle_exception(self, exc): + """Classify the two request failures the shared catalog leaves unnamed.""" + if isinstance(exc, MethodNotAllowed) and not isinstance(exc, RefusedMethod): + exc = RefusedMethod(self.request.method, detail=exc.detail) + elif isinstance(exc, NotAcceptable) and not isinstance(exc, UnsatisfiableAccept): + exc = UnsatisfiableAccept(detail=exc.detail) + return super().handle_exception(exc) diff --git a/openedx/core/djangoapps/user_api/v2/filters.py b/openedx/core/djangoapps/user_api/v2/filters.py new file mode 100644 index 000000000000..741aaee5fc12 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/filters.py @@ -0,0 +1,178 @@ +"""Filter sets for version 2 of the User API.""" + +import logging + +from django import forms +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from django.core.exceptions import ValidationError as FormValidationError +from django_filters import rest_framework as filters +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from rest_framework.exceptions import ValidationError +from rest_framework.filters import OrderingFilter + +from openedx.core.djangoapps.user_api.models import UserPreference + +log = logging.getLogger(__name__) + + +class StrictOrderingFilter(OrderingFilter): + """ + Refuses a sort field the view does not offer, and completes the sort it accepts. + + The default backend drops an unusable field silently and answers 200 in + the default order, which a caller reads as a page sorted the way it asked. + It also replaces the view's own order rather than adding to it, so a sort + on a column several rows share leaves their relative order to the database + and the pages of one listing need not agree on it; the fields named by the + view's ``ordering_tiebreak`` are appended to settle those rows. + """ + + def get_ordering(self, request, queryset, view): + """Return the requested sort, followed by the fields that make it total.""" + ordering = super().get_ordering(request, queryset, view) + if not ordering: + return ordering + chosen = {field.lstrip("-") for field in ordering} + tiebreak = [field for field in getattr(view, "ordering_tiebreak", ()) if field not in chosen] + return [*ordering, *tiebreak] + + def remove_invalid_fields(self, queryset, fields, view, request): + """Return the requested sort fields, refusing the request when one is not offered.""" + allowed = super().remove_invalid_fields(queryset, fields, view, request) + rejected = [field for field in fields if field not in allowed] + if rejected: + log.info("Sort refused on %s: %r", type(view).__name__, rejected) + offered = sorted( + name for name, _label in self.get_valid_fields(queryset, view, {"request": request}) + ) + raise ValidationError({ + "ordering": [ + f"Sort by any of: {', '.join(offered)}, each optionally prefixed with '-'.", + ], + }) + return allowed + + +class _BooleanQueryField(forms.NullBooleanField): + """Reads a boolean out of the query string, refusing a value that names neither side.""" + + def to_python(self, value): + """Return the boolean this value names, or refuse the request.""" + if value in self.empty_values: + return None + parsed = super().to_python(value) + if parsed is None: + raise FormValidationError("Enter true or false.", code="invalid") + return parsed + + +class StrictBooleanFilter(filters.BooleanFilter): + """ + Refuses a value that names neither true nor false instead of ignoring it. + + The default field reads an unrecognised value as "no preference" and + answers 200 with the filter dropped, handing the caller the rows it asked + to leave out, which is the opposite of what it asked for. + """ + + field_class = _BooleanQueryField + + def __init__(self, *args, **kwargs): + """Pass the raw value through to the field, which is what judges it.""" + # The default widget maps anything it does not recognise to "no + # preference" before the field ever sees it; a text widget hands the + # value over untouched. + kwargs.setdefault("widget", forms.TextInput) + super().__init__(*args, **kwargs) + + +class UserFilterSet(filters.FilterSet): + """ + Narrows the user directory. + + ``forum_role`` and ``course_key`` name a role within one course and are + only meaningful together, so either on its own is rejected. A role that no + course has defined matches no users and is not created. + """ + + username = filters.CharFilter(help_text="Return the user with exactly this username.") + email = filters.CharFilter(help_text="Return the user with exactly this email address.") + is_active = StrictBooleanFilter( + help_text="Return only users whose account is active, or only inactive ones. true or false.", + ) + preference_key = filters.CharFilter( + method="filter_preference_key", + help_text="Return only users who have stored a preference with this key.", + ) + forum_role = filters.CharFilter( + method="filter_forum_role", + help_text="Return only users holding this course forum role. Requires course_key.", + ) + course_key = filters.CharFilter( + method="filter_course_key", + help_text=( + "Course the forum role belongs to, percent-encoded, for example " + "course-v1:edX%2BDemoX%2BDemo. A plus sign that is not encoded arrives as a " + "space and the key is refused. Requires forum_role." + ), + ) + + class Meta: + model = User + fields = ("username", "email", "is_active", "preference_key", "forum_role", "course_key") + + def filter_queryset(self, queryset): + """Reject a forum role without its course, or a course without its role, before filtering.""" + cleaned = self.form.cleaned_data + forum_role = cleaned.get("forum_role") + course_key = cleaned.get("course_key") + if forum_role and not course_key: + raise ValidationError({"course_key": ["This parameter is required when forum_role is supplied."]}) + if course_key and not forum_role: + raise ValidationError({"forum_role": ["This parameter is required when course_key is supplied."]}) + return super().filter_queryset(queryset) + + def filter_preference_key(self, queryset, name, value): # pylint: disable=unused-argument + """Keep users who have stored a preference under ``value``.""" + return queryset.filter(preferences__key=value) + + def filter_forum_role(self, queryset, name, value): # pylint: disable=unused-argument + """ + Keep users holding the role ``value`` in the course named by ``course_key``. + + Nothing stops a course from carrying the same role under several rows, + and a user may hold any number of them, so the join is collapsed to one + entry per user. + """ + raw_course_key = self.form.cleaned_data.get("course_key") + try: + course_key = CourseKey.from_string(raw_course_key) + except InvalidKeyError as exc: + log.info("User directory filtered with an unparsable course key: %r", raw_course_key) + raise ValidationError({"course_key": ["Enter a valid course key."]}) from exc + return queryset.filter(roles__name=value, roles__course_id=course_key).distinct() + + def filter_course_key(self, queryset, name, value): # pylint: disable=unused-argument + """Narrowing happens in ``filter_forum_role``, which needs both values at once.""" + return queryset + + +class UserPreferenceFilterSet(filters.FilterSet): + """ + Narrows the stored-preference list by preference name or by owner. + + An owner is named by username: the list is keyed the way the addresses of + its members are, and publishes no database identifier for a caller to + filter on. + """ + + key = filters.CharFilter(help_text="Return only preferences stored under exactly this key.") + username = filters.CharFilter( + field_name="user__username", + help_text="Return only preferences belonging to this user.", + ) + + class Meta: + model = UserPreference + fields = ("key", "username") diff --git a/openedx/core/djangoapps/user_api/v2/paginators.py b/openedx/core/djangoapps/user_api/v2/paginators.py new file mode 100644 index 000000000000..5e796180d3cb --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/paginators.py @@ -0,0 +1,57 @@ +"""Pagination for version 2 of the User API.""" + +from edx_rest_framework_extensions.paginators import DefaultPagination + + +class UserApiPagination(DefaultPagination): + """ + Page-number pagination whose published schema matches the body it returns. + + ``DefaultPagination`` answers with ``count``, ``num_pages``, + ``current_page``, ``start``, ``next``, ``previous`` and ``results`` but + inherits the four-member schema of DRF's ``PageNumberPagination``, so a + generated client would not know about the three page-position members. + This subclass only restates the response schema; the body is unchanged. + """ + + def get_paginated_response_schema(self, schema): + """Describe the full envelope the paginator returns.""" + return { + "type": "object", + "required": ["count", "num_pages", "current_page", "start", "results"], + "properties": { + "count": { + "type": "integer", + "example": 123, + "description": "Total number of items across every page.", + }, + "num_pages": { + "type": "integer", + "example": 13, + "description": "Total number of pages.", + }, + "current_page": { + "type": "integer", + "example": 2, + "description": "Number of the page in this response, counting from one.", + }, + "start": { + "type": "integer", + "example": 10, + "description": "Index of the first item of this page within the full result set.", + }, + "next": { + "type": "string", + "nullable": True, + "format": "uri", + "description": "URL of the next page, or null on the last page.", + }, + "previous": { + "type": "string", + "nullable": True, + "format": "uri", + "description": "URL of the previous page, or null on the first page.", + }, + "results": schema, + }, + } diff --git a/openedx/core/djangoapps/user_api/v2/permissions.py b/openedx/core/djangoapps/user_api/v2/permissions.py new file mode 100644 index 000000000000..80e78b269541 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/permissions.py @@ -0,0 +1,20 @@ +"""Permission classes for version 2 of the User API.""" + +from rest_framework.permissions import BasePermission + + +class IsActiveUser(BasePermission): + """ + Admits only a caller whose account is still active. + + The authenticated and administrator checks read ``is_authenticated`` and + ``is_staff``, and neither reads ``is_active``. A token stays valid until it + expires and is accepted without the account being re-read for it, so an + account deactivated after a token was issued keeps every access that token + carries unless the account's state is checked here. edx-drf-extensions + ships no equivalent class, so this one stands in for it. + """ + + def has_permission(self, request, view): + """Return whether the caller's account is still active.""" + return bool(request.user and request.user.is_active) diff --git a/openedx/core/djangoapps/user_api/v2/schema.py b/openedx/core/djangoapps/user_api/v2/schema.py new file mode 100644 index 000000000000..1db961213b16 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/schema.py @@ -0,0 +1,31 @@ +"""Schema helpers for version 2 of the User API.""" + +from drf_spectacular.serializers import PolymorphicProxySerializerExtension +from drf_spectacular.utils import PolymorphicProxySerializer + + +class ResponseShapes(PolymorphicProxySerializer): + """ + The set of bodies one operation returns for the same status code. + + An endpoint whose caller can reduce the response answers with the default + object, with a preset, or with a selection of properties. A reduced body + satisfies more than one of those descriptions, so they are published as + alternatives a body may match in any combination. + """ + + def __init__(self, component_name, serializers, **kwargs): + # The generator re-instantiates a declaration from the arguments it + # retained, so the discriminator is only supplied when it is missing. + kwargs.setdefault("resource_type_field_name", None) + super().__init__(component_name=component_name, serializers=serializers, **kwargs) + + +class ResponseShapesExtension(PolymorphicProxySerializerExtension): + """Publishes the alternatives of a ``ResponseShapes`` declaration.""" + + target_class = ResponseShapes + + def map_serializer(self, auto_schema, direction): + """Return the alternatives as a set a body may match more than one of.""" + return {"anyOf": super().map_serializer(auto_schema, direction)["oneOf"]} diff --git a/openedx/core/djangoapps/user_api/v2/serializers.py b/openedx/core/djangoapps/user_api/v2/serializers.py new file mode 100644 index 000000000000..8c418252640d --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/serializers.py @@ -0,0 +1,206 @@ +"""Serializers for version 2 of the User API.""" + +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from django.urls import reverse +from drf_spectacular.utils import extend_schema_field +from edx_rest_framework_extensions.errors import ErrorResponseSerializer +from rest_framework import serializers + +from openedx.core.djangoapps.user_api.models import UserPreference + +#: Value of the ``view`` query parameter that embeds the owning user in each preference. +FULL_VIEW = "full" + +#: Value of the ``view`` query parameter that reduces a user to its identifying fields. +MINIMAL_VIEW = "minimal" + +#: Properties the ``minimal`` preset keeps of a directory entry. +USER_MINIMAL_FIELDS = ("id", "username", "email") + +#: Properties the ``minimal`` preset keeps of a stored preference. +PREFERENCE_MINIMAL_FIELDS = ("username", "key", "value") + + +class ErrorResponse(ErrorResponseSerializer): # pylint: disable=abstract-method + """ + The body of every unsuccessful response. + + ``type`` is a URI naming the kind of failure, ``title`` names it in one + line, ``status`` repeats the HTTP status code and ``detail`` explains what + happened; ``errors`` lists the per-field messages of a rejected parameter. + """ + + +class _KeptFieldsMixin: + """ + Describes an object reduced to the properties named in ``kept_fields``. + + The response presets reduce an already serialized object, so a preset's + shape has a describing serializer of its own rather than a serializing one. + """ + + #: Names of the properties the preset returns. + kept_fields = () + + def get_fields(self): + """Return the declared fields narrowed to the ones the preset returns.""" + fields = super().get_fields() + return {name: field for name, field in fields.items() if name in self.kept_fields} + + +class _OptionalFieldsMixin: + """ + Describes an object whose every property is optional. + + A caller that selects properties with ``fields`` decides which ones come + back, so none of them is guaranteed to be present. + """ + + def get_fields(self): + """Return the declared fields, none of them mandatory.""" + fields = super().get_fields() + for field in fields.values(): + field.required = False + field.read_only = False + return fields + + +def full_view_requested(request): + """Return True when the caller asked for the embedded-user response preset.""" + if request is None: + return False + params = getattr(request, "query_params", None) + if params is None: + params = getattr(request, "GET", {}) + return params.get("view") == FULL_VIEW + + +class UserDirectorySerializer(serializers.ModelSerializer): + """A user of the platform directory, addressed by username.""" + + id = serializers.IntegerField( + read_only=True, + help_text="Database identifier of the user. Addresses in this API use the username instead.", + ) + url = serializers.HyperlinkedIdentityField( + view_name="user_v2:user_detail", + lookup_field="username", + help_text="Absolute URL of this user's directory entry.", + ) + email = serializers.EmailField(read_only=True, help_text="Registered email address of the user.") + username = serializers.CharField(read_only=True, help_text="Unique name the user is addressed by.") + name = serializers.SerializerMethodField( + help_text="Full name recorded on the user's profile, or null when the user has no profile.", + ) + preferences = serializers.SerializerMethodField( + help_text="The user's stored preferences, as a mapping of preference key to value.", + ) + + class Meta: + model = User + fields = ("id", "url", "email", "name", "username", "preferences") + read_only_fields = fields + + @extend_schema_field(serializers.CharField(allow_null=True)) + def get_name(self, user): + """Return the profile name, or None for a user who has no profile row.""" + profile = getattr(user, "profile", None) + return profile.name if profile is not None else None + + @extend_schema_field(serializers.DictField(child=serializers.CharField())) + def get_preferences(self, user): + """Return the user's preferences as a key-to-value mapping.""" + return UserPreference.get_all_preferences(user) + + +class UserDirectoryMinimalSerializer(_KeptFieldsMixin, UserDirectorySerializer): + """A directory entry reduced to the properties that identify the user.""" + + kept_fields = USER_MINIMAL_FIELDS + + +class UserDirectoryProjectionSerializer(_OptionalFieldsMixin, UserDirectorySerializer): + """A directory entry reduced to the properties the caller selected.""" + + +class UserPreferenceRecordSerializer(serializers.ModelSerializer): + """ + A stored preference, identified by the user that owns it and its key. + + ``user`` carries the owner's whole directory entry and is included only + when the caller asks for the ``full`` response preset; by default the + owner appears as the ``username`` string alone. + """ + + url = serializers.SerializerMethodField(help_text="Absolute URL of this preference.") + username = serializers.CharField( + source="user.username", + read_only=True, + help_text="Username of the user this preference belongs to.", + ) + key = serializers.CharField(read_only=True, help_text="Name of the preference.") + value = serializers.CharField(read_only=True, help_text="Stored value of the preference.") + user = serializers.SerializerMethodField( + help_text="Directory entry of the owning user; null unless the full response preset is asked for.", + ) + + class Meta: + model = UserPreference + fields = ("username", "key", "value", "url", "user") + read_only_fields = fields + + @extend_schema_field(serializers.URLField()) + def get_url(self, preference): + """Return the member address of this preference.""" + path = reverse( + "user_v2:user_preference_detail", + kwargs={"username": preference.user.username, "key": preference.key}, + ) + request = self.context.get("request") + return request.build_absolute_uri(path) if request is not None else path + + @extend_schema_field(UserDirectorySerializer(allow_null=True)) + def get_user(self, preference): + """Embed the owner only when the caller asked for the full preset.""" + if not full_view_requested(self.context.get("request")): + return None + return UserDirectorySerializer(preference.user, context=self.context).data + + +class UserPreferenceRecordMinimalSerializer(_KeptFieldsMixin, UserPreferenceRecordSerializer): + """A stored preference reduced to its owner, its name and its value.""" + + kept_fields = PREFERENCE_MINIMAL_FIELDS + + +class UserPreferenceRecordProjectionSerializer(_OptionalFieldsMixin, UserPreferenceRecordSerializer): + """A stored preference reduced to the properties the caller selected.""" + + +class TimeZoneSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A time zone and the way it is displayed to a learner.""" + + time_zone = serializers.CharField(help_text="Name of the time zone, for example ``US/Pacific``.") + description = serializers.CharField( + help_text="Display form of the time zone, for example ``US/Pacific (PST, UTC-0800)``.", + ) + + +class TimeZoneQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters accepted by the time-zone list.""" + + country_code = serializers.RegexField( + r"^[A-Za-z]{2}$", + required=False, + # An empty value narrows nothing, the way an empty value of any other + # parameter does, and is accepted rather than held to the pattern. + allow_blank=True, + error_messages={ + "invalid": "Enter a two-letter ISO 3166-1 alpha-2 country code, for example FR.", + }, + help_text=( + "Two-letter ISO 3166-1 alpha-2 country code, case-insensitive. " + "Every time zone is returned when it is omitted or left empty, and when " + "the code names a country with no time zones of its own." + ), + ) diff --git a/openedx/core/djangoapps/user_api/v2/tests/__init__.py b/openedx/core/djangoapps/user_api/v2/tests/__init__.py new file mode 100644 index 000000000000..61ab32b09c15 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/__init__.py @@ -0,0 +1 @@ +"""Tests for version 2 of the User API.""" diff --git a/openedx/core/djangoapps/user_api/v2/tests/test_parity.py b/openedx/core/djangoapps/user_api/v2/tests/test_parity.py new file mode 100644 index 000000000000..38a75fe6bd13 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/test_parity.py @@ -0,0 +1,374 @@ +"""Behaviour parity between the version 1 and version 2 User API addresses.""" + +import json + +import pytest +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from django.core.exceptions import ObjectDoesNotExist +from django.test.utils import override_settings +from django.urls import reverse +from edx_rest_framework_extensions.testing import assert_error_envelope +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from rest_framework import status +from rest_framework.test import APITestCase + +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from openedx.core.djangoapps.django_comment_common.models import Role +from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + +PASSWORD = "Password1234" +API_KEY = "parity-test-api-key" +COURSE_KEY = CourseKey.from_string("course-v1:edX+DemoX+Demo") + +#: Fields that legitimately differ per request and are ignored in the diff. +VOLATILE_FIELDS = {"instance", "next", "previous"} + +#: Differences the plan declares intentional, per case. Each entry is a +#: (json path, reason) pair and the diff must contain all of them and nothing else. +INTENTIONAL_DIFFERENCES = { + "user_list": [ + ("results[0].url", "the member address is keyed by username instead of database id"), + ("results[1].url", "the member address is keyed by username instead of database id"), + ], + "user_detail": [ + ("url", "the member address is keyed by username instead of database id"), + ], + "preference_list": [ + ( + "results[0].user", + "the embedded owner is a member of every row and is null unless the full preset is asked for", + ), + ("results[0].username", "the owner's username is carried in place of the embedded user object"), + ("results[0].url", "the member address is keyed by username and preference key"), + ( + "results[1].user", + "the embedded owner is a member of every row and is null unless the full preset is asked for", + ), + ("results[1].username", "the owner's username is carried in place of the embedded user object"), + ("results[1].url", "the member address is keyed by username and preference key"), + ], + "preference_detail": [ + ("user", "the embedded owner is a member of every row and is null unless the full preset is asked for"), + ("username", "the owner's username is carried in place of the embedded user object"), + ("url", "the member address is keyed by username and preference key"), + ], + "preference_key_filter": [ + ("results[0].url", "the member address is keyed by username instead of database id"), + ], + "forum_role_filter": [ + ("results[0].url", "the member address is keyed by username instead of database id"), + ], +} + + +def normalize(obj, path=""): + """Sort keys, drop volatile fields, and collect a {json path: value} map for diffing.""" + flat = {} + if isinstance(obj, dict): + for key in sorted(obj): + if key in VOLATILE_FIELDS: + continue + flat.update(normalize(obj[key], f"{path}.{key}" if path else key)) + elif isinstance(obj, list): + for index, item in enumerate(obj): + flat.update(normalize(item, f"{path}[{index}]")) + else: + flat[path] = obj + return flat + + +def diff(legacy, new): + """Return (every changed path, the changed paths no declared difference covers).""" + left, right = normalize(legacy), normalize(new) + return {key for key in set(left) | set(right) if left.get(key, "") != right.get(key, "")} + + +def unexpected(changed, intentional): + """Return the changed paths that no declared intentional difference covers.""" + declared = {path for path, _reason in intentional} + return { + key for key in changed + if not any(key == path or key.startswith(path + ".") or key.startswith(path + "[") for path in declared) + } + + +@skip_unless_lms +@override_settings(EDX_API_KEY=API_KEY) +class ParityTests(APITestCase): + """The same fixtures through the legacy and the conforming address.""" + + def setUp(self): + super().setUp() + self.admin = AdminFactory.create(username="parity-admin", password=PASSWORD) + self.user = UserFactory.create( + username="parity-user", email="parity-user@example.com", password=PASSWORD, + ) + self.preference = UserPreferenceFactory.create(user=self.user, key="pref-lang", value="en") + role = Role.objects.create(course_id=COURSE_KEY, name="Moderator") + role.users.add(self.user) + + def legacy(self, address, params=None): + """Call a version 1 address with the API key it is gated by.""" + return self.client.get(address, params or {}, HTTP_X_EDX_API_KEY=API_KEY) + + def conforming(self, address, params=None): + """Call a version 2 address as the administrator it is gated by.""" + assert self.client.login(username=self.admin.username, password=PASSWORD) + response = self.client.get(address, params or {}) + self.client.logout() + return response + + def assert_parity(self, legacy, new, case, expected_status=status.HTTP_200_OK): + """ + Assert the two bodies differ only where the case declares they do. + + Rows are compared where each version put them, so a page whose rows + move is a difference like any other and has to be declared. + """ + assert legacy.status_code == expected_status, legacy.content + assert new.status_code == expected_status, new.content + legacy_body = json.loads(legacy.content) + new_body = json.loads(new.content) + changed = diff(legacy_body, new_body) + intentional = INTENTIONAL_DIFFERENCES[case] + assert not unexpected(changed, intentional), ( + f"undeclared differences: {sorted(unexpected(changed, intentional))}\n" + f"legacy={legacy_body}\nnew={new_body}" + ) + for path, reason in intentional: + assert any( + key == path or key.startswith(path + ".") or key.startswith(path + "[") + for key in changed + ), f"declared difference did not occur: {path} ({reason})" + + def test_user_list_parity(self): + legacy = self.legacy("/api/user/v1/users/", {"page_size": 100}) + new = self.conforming(reverse("user_v2:user_list"), {"page_size": 100}) + self.assert_parity(legacy, new, "user_list") + + def test_the_conforming_page_is_ordered_by_username_whatever_the_creation_order(self): + """Version 1 applies no ordering, so only the conforming side can be pinned to a sequence.""" + for username in ("parity-zulu", "parity-alpha"): + UserFactory.create(username=username, email=f"{username}@example.com", password=PASSWORD) + + new = self.conforming(reverse("user_v2:user_list"), {"page_size": 100}) + usernames = [row["username"] for row in json.loads(new.content)["results"]] + assert usernames == ["parity-admin", "parity-alpha", "parity-user", "parity-zulu"] + + legacy = self.legacy("/api/user/v1/users/", {"page_size": 100}) + legacy_usernames = [row["username"] for row in json.loads(legacy.content)["results"]] + assert sorted(legacy_usernames) == usernames + + def test_the_api_key_admits_at_the_legacy_addresses_only(self): + """The header the version 1 addresses are gated by is not a credential at version 2.""" + legacy = self.legacy("/api/user/v1/users/", {"page_size": 100}) + assert legacy.status_code == status.HTTP_200_OK, legacy.content + assert sorted(row["username"] for row in json.loads(legacy.content)["results"]) == [ + "parity-admin", "parity-user", + ] + + for address in ( + reverse("user_v2:user_list"), + reverse("user_v2:user_detail", kwargs={"username": "parity-user"}), + reverse("user_v2:user_preference_list"), + reverse( + "user_v2:user_preference_detail", + kwargs={"username": "parity-user", "key": "pref-lang"}, + ), + ): + refused = self.client.get(address, HTTP_X_EDX_API_KEY=API_KEY) + envelope = assert_error_envelope( + refused, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + assert envelope["detail"] == "Authentication credentials were not provided.", address + + def test_user_detail_parity(self): + legacy = self.legacy(f"/api/user/v1/users/{self.user.id}/") + new = self.conforming(reverse("user_v2:user_detail", kwargs={"username": "parity-user"})) + self.assert_parity(legacy, new, "user_detail") + + def test_preference_list_parity(self): + """More than one row, so the comparison pairs rows rather than reading a single one.""" + UserPreferenceFactory.create(user=self.user, key="pref-time", value="UTC") + legacy = self.legacy("/api/user/v1/user_prefs/", {"page_size": 100}) + new = self.conforming(reverse("user_v2:user_preference_list"), {"page_size": 100}) + assert len(json.loads(new.content)["results"]) == 2 + self.assert_parity(legacy, new, "preference_list") + + def test_the_conforming_preference_page_is_ordered_by_owner_and_key(self): + """Version 1 applies no ordering, so only the conforming side can be pinned to a sequence.""" + UserPreferenceFactory.create(user=self.user, key="pref-time", value="UTC") + UserPreferenceFactory.create(user=self.admin, key="pref-zone", value="UTC") + + new = self.conforming(reverse("user_v2:user_preference_list"), {"page_size": 100}) + rows = [(row["username"], row["key"]) for row in json.loads(new.content)["results"]] + assert rows == [ + ("parity-admin", "pref-zone"), + ("parity-user", "pref-lang"), + ("parity-user", "pref-time"), + ] + + legacy = self.legacy("/api/user/v1/user_prefs/", {"page_size": 100}) + legacy_rows = [ + (row["user"]["username"], row["key"]) for row in json.loads(legacy.content)["results"] + ] + assert legacy_rows == [ + ("parity-user", "pref-lang"), + ("parity-user", "pref-time"), + ("parity-admin", "pref-zone"), + ] + assert sorted(legacy_rows) == rows + + def test_preference_detail_parity(self): + legacy = self.legacy(f"/api/user/v1/user_prefs/{self.preference.id}/") + new = self.conforming( + reverse( + "user_v2:user_preference_detail", + kwargs={"username": "parity-user", "key": "pref-lang"}, + ), + ) + self.assert_parity(legacy, new, "preference_detail") + + def test_preference_key_filter_parity(self): + legacy = self.legacy("/api/user/v1/preferences/pref-lang/users/", {"page_size": 100}) + new = self.conforming( + reverse("user_v2:user_list"), {"preference_key": "pref-lang", "page_size": 100}, + ) + self.assert_parity(legacy, new, "preference_key_filter") + + def test_forum_role_filter_parity(self): + legacy = self.legacy( + "/api/user/v1/forum_roles/Moderator/users/", + {"course_id": str(COURSE_KEY), "page_size": 100}, + ) + new = self.conforming( + reverse("user_v2:user_list"), + {"forum_role": "Moderator", "course_key": str(COURSE_KEY), "page_size": 100}, + ) + self.assert_parity(legacy, new, "forum_role_filter") + + def test_full_preset_reproduces_the_legacy_embedded_user(self): + legacy = self.legacy(f"/api/user/v1/user_prefs/{self.preference.id}/") + new = self.conforming( + reverse( + "user_v2:user_preference_detail", + kwargs={"username": "parity-user", "key": "pref-lang"}, + ), + {"view": "full"}, + ) + legacy_user = json.loads(legacy.content)["user"] + new_user = json.loads(new.content)["user"] + assert set(legacy_user) == set(new_user) + for field in ("id", "email", "name", "username", "preferences"): + assert legacy_user[field] == new_user[field], field + assert legacy_user["url"].endswith(f"/user_api/v1/users/{self.user.id}/") + assert new_user["url"].endswith("/api/user/v2/users/parity-user/") + + def test_a_user_without_a_profile_is_a_row_here_and_a_failure_at_version_one(self): + """Declared difference: the version 1 entry cannot render a user who has no profile row.""" + profileless = User.objects.create_user("parity-profileless", "pp@example.com", PASSWORD) + with pytest.raises(ObjectDoesNotExist): + self.legacy(f"/api/user/v1/users/{profileless.id}/") + + new = self.conforming( + reverse("user_v2:user_detail", kwargs={"username": "parity-profileless"}), + ) + assert new.status_code == status.HTTP_200_OK + body = json.loads(new.content) + assert body["name"] is None + assert body["username"] == "parity-profileless" + + def test_time_zone_rows_are_unchanged_inside_the_new_envelope(self): + legacy = json.loads(self.legacy("/api/user/v1/preferences/time_zones/").content) + new = json.loads(self.conforming(reverse("user_v2:time_zone_list"), {"page_size": 100}).content) + assert isinstance(legacy, list) + assert new["count"] == len(legacy) + assert new["results"] == legacy[:100] + + def test_country_filtered_time_zones_are_unchanged(self): + legacy = json.loads( + self.legacy("/api/user/v1/preferences/time_zones/", {"country_code": "FR"}).content, + ) + new = json.loads( + self.conforming(reverse("user_v2:time_zone_list"), {"country_code": "FR"}).content, + ) + assert new["results"] == legacy + + def test_unauthenticated_bodies_differ_as_declared(self): + legacy = self.client.get("/api/user/v1/users/") + assert legacy.status_code == status.HTTP_401_UNAUTHORIZED + assert json.loads(legacy.content) == { + "detail": "Authentication credentials were not provided.", + } + + new = self.client.get(reverse("user_v2:user_list")) + envelope = assert_error_envelope( + new, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + assert envelope["title"] == "Authentication Required" + assert envelope["detail"] == "Authentication credentials were not provided." + assert envelope["instance"] == "/api/user/v2/users/" + + def test_forbidden_bodies_differ_as_declared(self): + assert self.client.login(username=self.user.username, password=PASSWORD) + legacy = self.client.get("/api/user/v1/users/") + assert legacy.status_code == status.HTTP_403_FORBIDDEN + assert json.loads(legacy.content) == { + "detail": "You do not have permission to perform this action.", + } + + new = self.client.get(reverse("user_v2:user_list")) + envelope = assert_error_envelope( + new, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + assert envelope["title"] == "Permission Denied" + assert envelope["detail"] == "You do not have permission to perform this action." + + def test_a_missing_course_parameter_keeps_its_status_and_changes_its_body(self): + """The legacy address already refused this caller; only the shape of the refusal moves.""" + legacy = self.legacy("/api/user/v1/forum_roles/Moderator/users/") + assert legacy.status_code == status.HTTP_400_BAD_REQUEST + assert json.loads(legacy.content) == {"detail": "course_id must be specified"} + + new = self.conforming(reverse("user_v2:user_list"), {"forum_role": "Moderator"}) + envelope = assert_error_envelope( + new, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "course_key": ["This parameter is required when forum_role is supplied."], + } + + def test_an_unparsable_course_parameter_is_answered_rather_than_raised(self): + """Declared difference: the version 1 address lets the key error escape the handler.""" + with pytest.raises(InvalidKeyError): + self.legacy("/api/user/v1/forum_roles/Moderator/users/", {"course_id": "not a course"}) + + new = self.conforming( + reverse("user_v2:user_list"), {"forum_role": "Moderator", "course_key": "not a course"}, + ) + envelope = assert_error_envelope( + new, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == {"course_key": ["Enter a valid course key."]} + + def test_a_database_identifier_is_refused_instead_of_narrowing_nothing(self): + """The directory is keyed by username, and says so rather than answering an unfiltered page.""" + refused = self.conforming(reverse("user_v2:user_list"), {"id": self.user.id, "page_size": 100}) + envelope = assert_error_envelope( + refused, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert sorted(envelope["errors"]) == ["id"] + + def test_an_identifier_held_by_a_caller_resolves_to_the_username_the_entry_is_keyed_by(self): + """The step an integration holding identifiers takes before addressing an entry.""" + assert self.client.login(username=self.admin.username, password=PASSWORD) + resolved = self.client.get("/api/user/v1/accounts", {"lms_user_id": self.user.id}) + self.client.logout() + assert resolved.status_code == status.HTTP_200_OK + assert [account["username"] for account in json.loads(resolved.content)] == ["parity-user"] + + entry = self.conforming(reverse("user_v2:user_detail", kwargs={"username": "parity-user"})) + assert entry.status_code == status.HTTP_200_OK + assert json.loads(entry.content)["id"] == self.user.id diff --git a/openedx/core/djangoapps/user_api/v2/tests/test_query_counts.py b/openedx/core/djangoapps/user_api/v2/tests/test_query_counts.py new file mode 100644 index 000000000000..e8cc522fa8f1 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/test_query_counts.py @@ -0,0 +1,178 @@ +"""Query-count guards for the version 2 User API list and member addresses.""" + +import pytest +from django.test.utils import override_settings +from django.urls import reverse + +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from openedx.core.djangoapps.django_comment_common.models import Role +from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + +PASSWORD = "Password1234" +COURSE_KEY = "course-v1:edX+DemoX+Demo" + +#: Row counts a per-row query would show up at. +ROW_COUNTS = [1, 5, 25] + +#: Page sizes a per-item query would show up at, for the list whose rows are +#: not database rows and whose size is therefore set by the page alone. +PAGE_SIZES = [1, 10, 100] + +USER_LIST_QUERIES = 16 +USER_DETAIL_QUERIES = 15 +PREFERENCE_LIST_QUERIES = 15 +PREFERENCE_LIST_FULL_VIEW_QUERIES = 16 +PREFERENCE_DETAIL_QUERIES = 14 +TIME_ZONE_LIST_QUERIES = 13 + +#: Address of the stored-preference list this version replaces, and the key it +#: is gated by, so the cost of the two can be compared in one measurement. +EARLIER_PREFERENCE_LIST = "/api/user/v1/user_prefs/" +EARLIER_API_KEY = "query-count-api-key" + +#: What that list costs for a page holding two rows per user, measured the same +#: way as the counts above: a constant part plus five queries for every user on +#: the page, because each row fetches its owner, the owner's profile and the +#: owner's own preferences again. +EARLIER_PREFERENCE_LIST_QUERIES = {1: 15, 5: 35, 25: 135} + + +@pytest.fixture(name="directory") +@skip_unless_lms +def directory_fixture(request, client, django_user_model): # pylint: disable=unused-argument + """Sign an administrator in and populate the directory with ``request.param`` users.""" + admin = AdminFactory.create(username="count-admin", password=PASSWORD) + users = [UserFactory.create(username=f"counted{index}") for index in range(request.param)] + for user in users: + UserPreferenceFactory.create(user=user, key="pref-lang", value="en") + UserPreferenceFactory.create(user=user, key="time_zone", value="UTC") + role = Role.objects.create(course_id=COURSE_KEY, name="Moderator") + role.users.add(*users) + assert client.login(username=admin.username, password=PASSWORD) + # The first authenticated request of a session populates caches the rest + # of the session reuses; the counts below are the steady-state ones. + client.get(reverse("user_v2:user_list")) + return users + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_user_list_query_count_does_not_grow_with_rows(client, directory, django_assert_num_queries): + url = reverse("user_v2:user_list") + with django_assert_num_queries(USER_LIST_QUERIES): + response = client.get(url, {"page_size": 100}) + assert len(response.json()["results"]) == len(directory) + 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_preference_key_filter_query_count_does_not_grow_with_rows( + client, directory, django_assert_num_queries, +): + url = reverse("user_v2:user_list") + with django_assert_num_queries(USER_LIST_QUERIES): + response = client.get(url, {"preference_key": "pref-lang", "page_size": 100}) + assert len(response.json()["results"]) == len(directory) + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_forum_role_filter_query_count_does_not_grow_with_rows( + client, directory, django_assert_num_queries, +): + url = reverse("user_v2:user_list") + with django_assert_num_queries(USER_LIST_QUERIES): + response = client.get( + url, {"forum_role": "Moderator", "course_key": COURSE_KEY, "page_size": 100}, + ) + assert len(response.json()["results"]) == len(directory) + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_preference_list_query_count_does_not_grow_with_rows( + client, directory, django_assert_num_queries, +): + url = reverse("user_v2:user_preference_list") + with django_assert_num_queries(PREFERENCE_LIST_QUERIES): + response = client.get(url, {"page_size": 100}) + assert len(response.json()["results"]) == 2 * len(directory) + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_preference_list_full_view_query_count_does_not_grow_with_rows( + client, directory, django_assert_num_queries, +): + url = reverse("user_v2:user_preference_list") + with django_assert_num_queries(PREFERENCE_LIST_FULL_VIEW_QUERIES): + response = client.get(url, {"page_size": 100, "view": "full"}) + rows = response.json()["results"] + assert len(rows) == 2 * len(directory) + assert rows[0]["user"]["preferences"] == {"pref-lang": "en", "time_zone": "UTC"} + + +@pytest.mark.django_db +@pytest.mark.parametrize("page_size", PAGE_SIZES) +@pytest.mark.parametrize("directory", [1], indirect=True) +@skip_unless_lms +def test_time_zone_list_query_count_does_not_grow_with_page_size( + client, directory, django_assert_num_queries, page_size, +): # pylint: disable=unused-argument + """The rows come from the time-zone database, so only the page size varies them.""" + url = reverse("user_v2:time_zone_list") + with django_assert_num_queries(TIME_ZONE_LIST_QUERIES): + response = client.get(url, {"page_size": page_size}) + assert len(response.json()["results"]) == page_size + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", [5], indirect=True) +@skip_unless_lms +def test_member_query_counts(client, directory, django_assert_num_queries): + with django_assert_num_queries(USER_DETAIL_QUERIES): + response = client.get(reverse("user_v2:user_detail", kwargs={"username": "counted0"})) + assert response.json()["username"] == "counted0" + + with django_assert_num_queries(PREFERENCE_DETAIL_QUERIES): + response = client.get( + reverse( + "user_v2:user_preference_detail", + kwargs={"username": "counted0", "key": "pref-lang"}, + ), + ) + assert response.json()["value"] == "en" + + +@pytest.mark.django_db +@pytest.mark.parametrize("directory", ROW_COUNTS, indirect=True) +@skip_unless_lms +def test_the_earlier_preference_list_pays_per_row_where_this_one_does_not( + client, directory, django_assert_num_queries, +): + """ + Both lists are measured over the same rows, so the saving cannot drift apart. + + The earlier row embeds its owner without a join and re-reads that owner's + own preferences, so a page of it costs five queries per user; this one + reads the same page at a count that does not move with it. + """ + with django_assert_num_queries(PREFERENCE_LIST_QUERIES): + current = client.get(reverse("user_v2:user_preference_list"), {"page_size": 100}) + assert len(current.json()["results"]) == 2 * len(directory) + + client.logout() + with override_settings(EDX_API_KEY=EARLIER_API_KEY): + with django_assert_num_queries(EARLIER_PREFERENCE_LIST_QUERIES[len(directory)]): + earlier = client.get( + EARLIER_PREFERENCE_LIST, + {"page_size": 100}, + HTTP_X_EDX_API_KEY=EARLIER_API_KEY, + ) + assert len(earlier.json()["results"]) == 2 * len(directory) diff --git a/openedx/core/djangoapps/user_api/v2/tests/test_schema.py b/openedx/core/djangoapps/user_api/v2/tests/test_schema.py new file mode 100644 index 000000000000..de29bdae413e --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/test_schema.py @@ -0,0 +1,531 @@ +"""Tests for what version 2 of the User API publishes in the OpenAPI schema.""" + +import json +from importlib import import_module + +import jsonschema +import pytest +from django.conf import settings +from django.urls import reverse +from drf_spectacular.generators import SchemaGenerator +from drf_spectacular.settings import patched_settings + +from common.djangoapps.student.tests.factories import AdminFactory +from lms.lib.spectacular import SUPERSEDED_USER_PATHS, lms_mark_legacy_paths_deprecated +from openedx.core.apidocs import get_api_docs_settings +from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory +from openedx.core.djangoapps.user_api.v2.serializers import TimeZoneQuerySerializer +from openedx.core.djangoapps.user_api.v2.views import PREFERENCE_KEY_PATTERN_PARTIAL +from openedx.core.djangolib.testing.utils import skip_unless_lms + +PASSWORD = "Password1234" + +USER_MEMBER_PATH = "/api/user/v2/users/{username}/" +PREFERENCE_MEMBER_PATH = "/api/user/v2/user_preferences/{username},{key}/" +TIME_ZONE_LIST_PATH = "/api/user/v2/time_zones/" + +#: The addresses that answer with a page of rows, and the ones that answer with +#: a single record. +LIST_PATHS = { + "/api/user/v2/users/", + "/api/user/v2/user_preferences/", + TIME_ZONE_LIST_PATH, +} +MEMBER_PATHS = {USER_MEMBER_PATH, PREFERENCE_MEMBER_PATH} + +VERSION_TWO_PATHS = LIST_PATHS | MEMBER_PATHS + +PAGINATION_MEMBERS = {"count", "num_pages", "current_page", "start", "next", "previous", "results"} + +#: Where this version is mounted in the service's URL configuration. +VERSION_TWO_MOUNT = "api/user/v2/" + +#: The components this version contributes to the document. +VERSION_TWO_COMPONENTS = ( + "ErrorResponse", + "PaginatedTimeZoneList", + "PaginatedUserDirectoryResponseList", + "PaginatedUserPreferenceRecordResponseList", + "TimeZone", + "UserDirectory", + "UserDirectoryMinimal", + "UserDirectoryProjection", + "UserDirectoryResponse", + "UserPreferenceRecord", + "UserPreferenceRecordMinimal", + "UserPreferenceRecordProjection", + "UserPreferenceRecordResponse", +) + +#: Every body these endpoints return: the address and parameters that produce +#: it, the published path it is documented under, and the rows it carries. +SHAPE_CASES = ( + ("/api/user/v2/users/", "/api/user/v2/users/", {}), + ("/api/user/v2/users/", "/api/user/v2/users/", {"view": "minimal"}), + ("/api/user/v2/users/", "/api/user/v2/users/", {"fields": "username,email"}), + ("/api/user/v2/users/", "/api/user/v2/users/", {"view": "minimal", "fields": "username"}), + ("/api/user/v2/users/{username}/", "/api/user/v2/users/shape-admin/", {}), + ("/api/user/v2/users/{username}/", "/api/user/v2/users/shape-admin/", {"view": "minimal"}), + ("/api/user/v2/users/{username}/", "/api/user/v2/users/shape-admin/", {"fields": "id,url"}), + ("/api/user/v2/user_preferences/", "/api/user/v2/user_preferences/", {}), + ("/api/user/v2/user_preferences/", "/api/user/v2/user_preferences/", {"view": "full"}), + ("/api/user/v2/user_preferences/", "/api/user/v2/user_preferences/", {"view": "minimal"}), + ("/api/user/v2/user_preferences/", "/api/user/v2/user_preferences/", {"fields": "key,value"}), + ( + "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v2/user_preferences/shape-admin,pref-lang/", + {}, + ), + ( + "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v2/user_preferences/shape-admin,pref-lang/", + {"view": "full"}, + ), + ( + "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v2/user_preferences/shape-admin,pref-lang/", + {"view": "minimal"}, + ), + ( + "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v2/user_preferences/shape-admin,pref-lang/", + {"fields": "value"}, + ), + ("/api/user/v2/time_zones/", "/api/user/v2/time_zones/", {"country_code": "FR"}), +) + + +def as_json_schema(node): + """ + Return a published schema node as a JSON Schema document. + + The document describes an absent value with ``nullable``, which JSON + Schema spells as an alternative type, so a validator reading the published + node verbatim would reject a member the API is documented to omit. + """ + if isinstance(node, list): + return [as_json_schema(item) for item in node] + if not isinstance(node, dict): + return node + converted = {key: as_json_schema(value) for key, value in node.items() if key != "nullable"} + if node.get("nullable"): + return {"anyOf": [converted, {"type": "null"}]} + return converted + + +def published_body_schema(schema, path): + """Return what a successful body of ``path`` must satisfy, components included.""" + body = schema["paths"][path]["get"]["responses"]["200"]["content"]["application/json"]["schema"] + return as_json_schema({**body, "components": schema["components"]}) + +#: The version 1 addresses this change supersedes, written out independently of +#: the constant the hook reads so that changing either one shows up here. +SUPERSEDED_USER_ADDRESSES = { + "/api/user/v1/users/": "/api/user/v2/users/", + "/api/user/v1/users/{id}/": "/api/user/v2/users/{username}/", + "/api/user/v1/user_prefs/": "/api/user/v2/user_preferences/", + "/api/user/v1/user_prefs/{id}/": "/api/user/v2/user_preferences/{username},{key}/", + "/api/user/v1/preferences/{pref_key}/users/": "/api/user/v2/users/", + "/api/user/v1/forum_roles/{name}/users/": "/api/user/v2/users/", + "/api/user/v1/preferences/time_zones/": "/api/user/v2/time_zones/", +} + +#: Legacy enrollment addresses this schema already marked before the user API joined it. +SUPERSEDED_ENROLLMENT_PATHS = { + "/api/enrollment/v2/course/{course_id}", + "/api/enrollment/v2/enrollment/{course_id}", + "/api/enrollment/v2/enrollment/{username},{course_id}", + "/api/enrollment/v2/enrollments", +} + +#: Enrollment operations whose ids must survive the widened path prefix. +ENROLLMENT_OPERATION_IDS = { + "v1_enrollments_list", + "v1_enrollment_create", + "v2_enrollments_list", + "v2_enrollments_list_legacy", + "v2_enrollment_,_retrieve", + "v2_roles_retrieve", +} + + +@pytest.fixture(name="schema") +@skip_unless_lms +def schema_fixture(db): # pylint: disable=invalid-name, unused-argument + """The schema published under the service's own drf-spectacular settings.""" + return SchemaGenerator().get_schema(request=None, public=True) + + +def service_wide_schema(patterns=None): + """Return the document the service publishes for browsing its whole surface.""" + with patched_settings(get_api_docs_settings()): + return SchemaGenerator(patterns=patterns).get_schema(request=None, public=True) + + +def patterns_without_this_version(): + """Return the service's URL configuration with this version's mount taken out.""" + configured = import_module(settings.ROOT_URLCONF).urlpatterns + remaining = [entry for entry in configured if str(entry.pattern) != VERSION_TWO_MOUNT] + assert len(remaining) == len(configured) - 1, "this version is not mounted where it is looked for" + return remaining + + +def published_names(schema): + """Return every component name and operation id a document carries.""" + return set(schema["components"]["schemas"]) | { + operation["operationId"] for _path, _method, operation in operations(schema) + } + + +def operations(schema): + """Yield every (path, method, operation) the schema documents.""" + for path, path_item in schema["paths"].items(): + for method, operation in path_item.items(): + if isinstance(operation, dict) and method in {"get", "post", "put", "patch", "delete"}: + yield path, method, operation + + +@skip_unless_lms +def test_version_two_addresses_are_published(schema): + assert VERSION_TWO_PATHS <= set(schema["paths"]) + + +@skip_unless_lms +def test_version_two_operations_carry_the_area_tag(schema): + tags = { + operation["operationId"]: operation.get("tags") + for path, _method, operation in operations(schema) + if path in VERSION_TWO_PATHS + } + assert tags == { + "v2_users_list": ["user"], + "v2_users_retrieve": ["user"], + "v2_user_preferences_list": ["user"], + "v2_user_preferences_retrieve": ["user"], + "v2_time_zones_list": ["user"], + } + + +@skip_unless_lms +def test_enrollment_operation_ids_survive_the_widened_prefix(schema): + published = {operation["operationId"] for _path, _method, operation in operations(schema)} + assert ENROLLMENT_OPERATION_IDS <= published + + +@skip_unless_lms +def test_only_superseded_addresses_are_marked_deprecated(schema): + deprecated = { + path for path, _method, operation in operations(schema) if operation.get("deprecated") + } + assert deprecated == SUPERSEDED_ENROLLMENT_PATHS | set(SUPERSEDED_USER_ADDRESSES) + + +@skip_unless_lms +def test_only_this_version_and_the_addresses_it_supersedes_are_published(schema): + """A user address this change neither serves nor replaces stays out of the document.""" + published = {path for path in schema["paths"] if path.startswith("/api/user/")} + assert published == VERSION_TWO_PATHS | set(SUPERSEDED_USER_ADDRESSES) + + +@skip_unless_lms +def test_addresses_owned_by_other_apps_are_not_published(schema): + """Neighbours of the mounts this change adds, including one under its own prefix.""" + for path in ( + "/api/user/v1/accounts", + "/api/user/v1/me", + "/api/user/v1/preferences/{username}", + "/api/user/v1/account/registration/", + "/api/user/v2/account/registration/", + ): + assert path not in schema["paths"], path + + +@skip_unless_lms +def test_pagination_envelope_is_published_in_full(schema): + for component in ( + "PaginatedUserDirectoryResponseList", + "PaginatedUserPreferenceRecordResponseList", + "PaginatedTimeZoneList", + ): + published = schema["components"]["schemas"][component] + assert set(published["properties"]) == PAGINATION_MEMBERS, component + + +@pytest.mark.django_db +@skip_unless_lms +def test_published_pagination_envelope_matches_the_response_body(client, schema): + admin = AdminFactory.create(username="schema-admin", password=PASSWORD) + UserPreferenceFactory.create(user=admin, key="pref-lang", value="en") + assert client.login(username=admin.username, password=PASSWORD) + body = client.get(reverse("user_v2:user_preference_list")).json() + published = schema["components"]["schemas"]["PaginatedUserPreferenceRecordResponseList"] + assert set(body) == set(published["properties"]) + + +@skip_unless_lms +def test_error_responses_reference_the_shared_envelope(schema): + for path, method, operation in operations(schema): + if path not in VERSION_TWO_PATHS: + continue + for code, response in operation["responses"].items(): + if not code.startswith("4"): + continue + schema_ref = response["content"]["application/json"]["schema"] + assert schema_ref == {"$ref": "#/components/schemas/ErrorResponse"}, (path, method, code) + + +@skip_unless_lms +def test_an_operation_that_takes_a_response_preset_publishes_the_refusal(schema): + """A preset the operation does not answer to is a 400, so each of them documents one.""" + takes_a_preset = [ + (path, method) + for path, method, operation in operations(schema) + if path in VERSION_TWO_PATHS + and any(parameter["name"] == "view" for parameter in operation.get("parameters", [])) + and "400" in operation["responses"] + ] + assert sorted(takes_a_preset) == [ + ("/api/user/v2/user_preferences/", "get"), + ("/api/user/v2/user_preferences/{username},{key}/", "get"), + ("/api/user/v2/users/", "get"), + ("/api/user/v2/users/{username}/", "get"), + ] + + +@skip_unless_lms +def test_user_list_publishes_its_filters(schema): + parameters = { + parameter["name"]: parameter + for parameter in schema["paths"]["/api/user/v2/users/"]["get"]["parameters"] + } + assert set(parameters) == { + "course_key", "email", "fields", "forum_role", "is_active", "ordering", + "page", "page_size", "preference_key", "username", "view", + } + # The filter that refuses an unparsable value is still published as a boolean. + assert parameters["is_active"]["schema"] == {"type": "boolean"} + + +@skip_unless_lms +def test_preference_list_publishes_its_filters(schema): + parameters = { + parameter["name"]: parameter + for parameter in schema["paths"]["/api/user/v2/user_preferences/"]["get"]["parameters"] + } + assert set(parameters) == {"fields", "key", "ordering", "page", "page_size", "username", "view"} + # Every published parameter is one the list reads, and one it keeps. + assert [name for name, parameter in parameters.items() if parameter.get("deprecated")] == [] + + +@skip_unless_lms +def test_enum_post_processing_runs_before_the_deprecation_hook(): + hooks = settings.SPECTACULAR_SETTINGS["POSTPROCESSING_HOOKS"] + assert hooks[0] == "drf_spectacular.hooks.postprocess_schema_enums" + assert "lms.lib.spectacular.lms_mark_legacy_paths_deprecated" in hooks[1:] + + +@skip_unless_lms +def test_hook_leaves_other_resources_alone(): + """The hook marks only the addresses it lists, even among their neighbours.""" + untouched = [ + "/api/user/v1/accounts", + "/api/user/v1/me", + "/api/user/v1/preferences/email_opt_in/", + "/api/user/v2/account/registration/", + "/user_api/v1/users/", + "/user_api/v1/preferences/time_zones/", + "/api/enrollment/v2/enrollments/", + "/api/courses/v1/courses/", + ] + marked = ["/api/user/v1/users/", "/api/enrollment/v2/enrollments"] + result = {"paths": {path: {"get": {"operationId": path}} for path in untouched + marked}} + + lms_mark_legacy_paths_deprecated(result, generator=None, request=None, public=True) + + for path in untouched: + assert "deprecated" not in result["paths"][path]["get"], path + for path in marked: + assert result["paths"][path]["get"]["deprecated"] is True, path + + +@skip_unless_lms +def test_every_superseded_address_has_a_published_replacement(schema): + assert set(SUPERSEDED_USER_ADDRESSES) == set(SUPERSEDED_USER_PATHS) + for superseded, replacement in SUPERSEDED_USER_ADDRESSES.items(): + assert superseded in schema["paths"] + assert replacement in schema["paths"] + + +@pytest.mark.django_db +@skip_unless_lms +def test_this_version_only_adds_to_the_service_wide_document(): + """ + Nothing published for browsing the whole service is lost when this version joins it. + + Enum post-processing runs over every operation the service publishes and + renames both sides of a name collision, so a component this version never + declared can be renamed out of the document, and a client generated from it + loses the matching model. The document is therefore read with this version + mounted and with its mount taken out, and the second has to be contained in + the first. + """ + without_this_version = published_names(service_wide_schema(patterns_without_this_version())) + whole = service_wide_schema() + missing = without_this_version - published_names(whole) + assert missing == set(), sorted(missing) + + components = whole["components"]["schemas"] + assert "user_api.User" in components + assert {"UserDirectory", "UserPreferenceRecord", "TimeZone"} <= set(components) + assert "CountryTimeZone" in components + + +@skip_unless_lms +def test_every_response_shape_is_published(schema): + """The reduced bodies the presets return are described, not only the full one.""" + components = schema["components"]["schemas"] + assert components["UserDirectoryResponse"]["anyOf"] == [ + {"$ref": "#/components/schemas/UserDirectory"}, + {"$ref": "#/components/schemas/UserDirectoryMinimal"}, + {"$ref": "#/components/schemas/UserDirectoryProjection"}, + ] + assert components["UserPreferenceRecordResponse"]["anyOf"] == [ + {"$ref": "#/components/schemas/UserPreferenceRecord"}, + {"$ref": "#/components/schemas/UserPreferenceRecordMinimal"}, + {"$ref": "#/components/schemas/UserPreferenceRecordProjection"}, + ] + assert sorted(components["UserDirectory"]["required"]) == [ + "email", "id", "name", "preferences", "url", "username", + ] + assert sorted(components["UserDirectoryMinimal"]["required"]) == ["email", "id", "username"] + assert sorted(components["UserPreferenceRecordMinimal"]["required"]) == ["key", "username", "value"] + for component in ("UserDirectoryProjection", "UserPreferenceRecordProjection"): + published = components[component] + assert "required" not in published, component + assert set(published["properties"]) == set( + components[component.replace("Projection", "")]["properties"] + ), component + + +@pytest.mark.django_db +@skip_unless_lms +def test_published_shapes_accept_every_body_the_endpoints_return(client, schema): + """Every preset and selection the parameters offer returns a body the document describes.""" + admin = AdminFactory.create(username="shape-admin", password=PASSWORD) + UserPreferenceFactory.create(user=admin, key="pref-lang", value="en") + assert client.login(username=admin.username, password=PASSWORD) + for published_path, address, params in SHAPE_CASES: + case = (address, params) + response = client.get(address, params) + assert response.status_code == 200, case + body = response.json() + if "results" in body: + assert body["count"] == 1, case + validator = jsonschema.Draft202012Validator(published_body_schema(schema, published_path)) + assert [error.message for error in validator.iter_errors(body)] == [], case + + +@skip_unless_lms +def test_sort_parameter_publishes_the_fields_it_accepts(schema): + """The sort allow-list and the order used when none is asked for are published.""" + for path, accepted, default in ( + ("/api/user/v2/users/", ["-date_joined", "-username", "date_joined", "username"], ["username"]), + ("/api/user/v2/user_preferences/", ["-key", "-username", "key", "username"], ["username", "key"]), + ): + parameter = next( + published for published in schema["paths"][path]["get"]["parameters"] + if published["name"] == "ordering" + ) + assert parameter["schema"]["items"]["enum"] == accepted, path + assert parameter["schema"]["default"] == default, path + + +@skip_unless_lms +def test_every_list_publishes_the_page_that_does_not_exist(schema): + """Asking for a page past the end of a list is answered 404, so each list documents it.""" + assert LIST_PATHS | MEMBER_PATHS == set(VERSION_TWO_PATHS) + documented = { + path: sorted(operation["responses"]) + for path, _method, operation in operations(schema) + if path in LIST_PATHS + } + assert documented == { + "/api/user/v2/users/": ["200", "400", "401", "403", "404"], + "/api/user/v2/user_preferences/": ["200", "400", "401", "403", "404"], + TIME_ZONE_LIST_PATH: ["200", "400", "404"], + } + + +@skip_unless_lms +def test_member_addresses_describe_the_parts_they_are_built_from(schema): + """Each part of a member address is published with its meaning and what its route matches.""" + username_schema = {"type": "string", "pattern": rf"^{settings.USERNAME_REGEX_PARTIAL}$"} + published = { + (path, parameter["name"]): parameter + for path, _method, operation in operations(schema) + if path in MEMBER_PATHS + for parameter in operation["parameters"] + if parameter["in"] == "path" + } + assert published == { + (USER_MEMBER_PATH, "username"): { + "in": "path", + "name": "username", + "required": True, + "schema": username_schema, + "description": "Unique name of the user whose directory entry is returned.", + }, + (PREFERENCE_MEMBER_PATH, "username"): { + "in": "path", + "name": "username", + "required": True, + "schema": username_schema, + "description": ( + "Username of the user the preference belongs to: the part of the address " + "before the comma." + ), + }, + (PREFERENCE_MEMBER_PATH, "key"): { + "in": "path", + "name": "key", + "required": True, + "schema": {"type": "string", "pattern": rf"^{PREFERENCE_KEY_PATTERN_PARTIAL}$"}, + "description": ( + "Name the preference is stored under: the part of the address after the comma." + ), + }, + } + + +@skip_unless_lms +def test_the_country_parameter_publishes_the_constraint_it_is_held_to(schema): + """The parameter comes from the serializer that validates it, so the two cannot disagree.""" + field = TimeZoneQuerySerializer().fields["country_code"] + enforced = next( + validator.regex.pattern for validator in field.validators if hasattr(validator, "regex") + ) + parameter = next( + published for published in schema["paths"][TIME_ZONE_LIST_PATH]["get"]["parameters"] + if published["name"] == "country_code" + ) + assert parameter["description"] == field.help_text + assert parameter["schema"] == { + "oneOf": [{"type": "string", "pattern": enforced}, {"type": "string", "maxLength": 0}], + } + assert "required" not in parameter + + +@skip_unless_lms +def test_published_text_is_written_for_the_clients_that_read_it(schema): + """Descriptions reach every consumer, so they carry no governance references.""" + published = json.dumps({ + "paths": {path: schema["paths"][path] for path in VERSION_TWO_PATHS}, + "components": { + name: schema["components"]["schemas"][name] for name in VERSION_TWO_COMPONENTS + }, + }) + for reference in ("ADR", "OEP-", "drf-spectacular", "extend_schema"): + assert reference not in published, reference + assert schema["components"]["schemas"]["ErrorResponse"]["description"].startswith( + "The body of every unsuccessful response." + ) diff --git a/openedx/core/djangoapps/user_api/v2/tests/test_url_mount.py b/openedx/core/djangoapps/user_api/v2/tests/test_url_mount.py new file mode 100644 index 000000000000..019d3c535434 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/test_url_mount.py @@ -0,0 +1,68 @@ +"""Addresses of other apps that share the prefix version 2 of the User API is mounted on.""" + +from django.test import SimpleTestCase +from django.urls import resolve, reverse + +from openedx.core.djangolib.testing.utils import skip_unless_lms + +AUTHN_LOGIN_VIEWS = "openedx.core.djangoapps.user_authn.views.login" +AUTHN_REGISTER_VIEWS = "openedx.core.djangoapps.user_authn.views.register" + + +@skip_unless_lms +class SharedPrefixTests(SimpleTestCase): + """ + The authentication app keeps the two addresses it serves under this prefix. + + The mount for this version is matched before the include that carries the + rest of the user surface and before the authentication app's own patterns, + so it is first in line for every address that begins with + ``/api/user/v2/``. Those two addresses reach the authentication app only + because no pattern here matches them first. + """ + + def assert_resolves_to(self, address, module, class_name, url_name, kwargs=None): + """Assert an address reaches the named view through the named route.""" + match = resolve(address) + view_class = match.func.view_class + assert (view_class.__module__, view_class.__name__) == (module, class_name), address + assert match.url_name == url_name, address + assert match.kwargs == (kwargs or {}), address + + def test_registration_still_reaches_the_authentication_app(self): + self.assert_resolves_to( + "/api/user/v2/account/registration/", + AUTHN_REGISTER_VIEWS, + "RegistrationView", + "user_api_registration_v2", + ) + + def test_login_session_still_reaches_the_authentication_app(self): + self.assert_resolves_to( + "/api/user/v2/account/login_session/", + AUTHN_LOGIN_VIEWS, + "LoginSessionView", + "user_api_login_session", + {"api_version": "v2"}, + ) + + def test_the_version_one_addresses_beside_them_are_unaffected(self): + self.assert_resolves_to( + "/api/user/v1/account/registration/", + AUTHN_REGISTER_VIEWS, + "RegistrationView", + "user_api_registration", + ) + self.assert_resolves_to( + "/api/user/v1/account/login_session/", + AUTHN_LOGIN_VIEWS, + "LoginSessionView", + "user_api_login_session", + {"api_version": "v1"}, + ) + + def test_both_shared_prefix_addresses_still_reverse_to_themselves(self): + assert reverse("user_api_registration_v2") == "/api/user/v2/account/registration/" + assert reverse( + "user_api_login_session", kwargs={"api_version": "v2"}, + ) == "/api/user/v2/account/login_session/" diff --git a/openedx/core/djangoapps/user_api/v2/tests/test_urls.py b/openedx/core/djangoapps/user_api/v2/tests/test_urls.py new file mode 100644 index 000000000000..c51ceab78a13 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/tests/test_urls.py @@ -0,0 +1,296 @@ +"""Tests for the version 2 User API URL surface.""" + +import re + +import pytest +from django.core.exceptions import ImproperlyConfigured +from django.urls import get_resolver, resolve, reverse +from edx_rest_framework_extensions.testing import assert_error_envelope +from rest_framework import status +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory, APITestCase + +from common.djangoapps.student.tests.factories import AdminFactory +from openedx.core.djangoapps.user_api.v2 import urls as v2_urls +from openedx.core.djangoapps.user_api.v2.views import UserPreferenceViewSet, UserViewSet +from openedx.core.djangolib.testing.utils import skip_unless_lms + +PASSWORD = "Password1234" + +V2_VIEW_MODULE = "openedx.core.djangoapps.user_api.v2.views" +V1_VIEW_MODULE = "openedx.core.djangoapps.user_api.views" +V2_NAMESPACE = "user_v2" + +#: A route name is lower case, starts with a letter and separates words with +#: underscores; nothing else is accepted. +SNAKE_CASE_NAME = re.compile(r"[a-z][a-z0-9_]*\Z") +VERSION_IN_NAME = re.compile(r"v\d") + +EXPECTED_ADDRESSES = { + "user_v2:user_list": ({}, "/api/user/v2/users/"), + "user_v2:user_detail": ({"username": "alice"}, "/api/user/v2/users/alice/"), + "user_v2:user_preference_list": ({}, "/api/user/v2/user_preferences/"), + "user_v2:user_preference_detail": ( + {"username": "alice", "key": "time_zone"}, + "/api/user/v2/user_preferences/alice,time_zone/", + ), + "user_v2:time_zone_list": ({}, "/api/user/v2/time_zones/"), +} + + +def registered_route_names(): + """ + Return the route names the mounted URLconf registers under this namespace. + + These are the names ``reverse()`` accepts; they are read from the resolver + rather than from this module so that a route added or renamed in the + URLconf is seen here. + """ + _prefix, namespace_resolver = get_resolver().namespace_dict[V2_NAMESPACE] + return {key for key in namespace_resolver.reverse_dict if isinstance(key, str)} + + +def resolved_route_names(): + """Return the route names ``resolve()`` reports for the published addresses.""" + return {resolve(address).url_name for _kwargs, address in EXPECTED_ADDRESSES.values()} + + +def routed_actions(view_func): + """ + Return the method-to-action map a viewset mount was registered with. + + DRF adds a ``head`` entry to a mount's ``actions`` the first time it serves + a GET, so the raw attribute depends on what the test suite requested first. + """ + return {method: action for method, action in view_func.actions.items() if method != "head"} + + +def view_identity(view_func): + """ + Return the module and class name of the view a route is mounted on. + + A viewset mount carries the class name on the wrapper itself; Django's + plain class-based views record the class separately instead. + """ + view_class = getattr(view_func, "view_class", None) + name = view_class.__name__ if view_class is not None else view_func.__name__ + return (view_func.__module__, name) + + +@skip_unless_lms +class UrlLiteralTests(APITestCase): + """Every name reverses to the address the API publishes.""" + + def test_reverse_literals(self): + for name, (kwargs, expected) in EXPECTED_ADDRESSES.items(): + assert reverse(name, kwargs=kwargs) == expected, name + + def test_names_are_snake_case_and_carry_no_version(self): + registered = registered_route_names() + assert registered == {name.split(":")[1] for name in EXPECTED_ADDRESSES} + for name in registered: + assert SNAKE_CASE_NAME.match(name), name + assert not VERSION_IN_NAME.search(name), name + + def test_resolved_names_are_the_names_that_reverse(self): + resolved = resolved_route_names() + assert resolved == registered_route_names() + for address, name in ( + ("/api/user/v2/users/", "user_list"), + ("/api/user/v2/users/alice/", "user_detail"), + ("/api/user/v2/user_preferences/", "user_preference_list"), + ("/api/user/v2/user_preferences/alice,time_zone/", "user_preference_detail"), + ("/api/user/v2/time_zones/", "time_zone_list"), + ): + match = resolve(address) + assert match.url_name == name, address + assert match.view_name == f"{V2_NAMESPACE}:{name}", address + + def test_viewsets_build_action_addresses_under_this_mount(self): + assert UserViewSet(basename="user").reverse_action("list") == "/api/user/v2/users/" + assert UserViewSet(basename="user").reverse_action( + "detail", kwargs={"username": "alice"}, + ) == "/api/user/v2/users/alice/" + preferences = UserPreferenceViewSet(basename="user_preference") + assert preferences.reverse_action("list") == "/api/user/v2/user_preferences/" + assert preferences.reverse_action( + "detail", kwargs={"username": "alice", "key": "time_zone"}, + ) == "/api/user/v2/user_preferences/alice,time_zone/" + + def test_action_addresses_follow_the_namespace_the_request_arrived_on(self): + request = Request(APIRequestFactory().get("/api/user/v2/users/")) + request.resolver_match = resolve("/api/user/v2/users/") + view = UserViewSet(basename="user", request=request) + assert view.reverse_action("list") == "http://testserver/api/user/v2/users/" + + def test_addresses_resolve_to_the_version_two_viewsets(self): + expected = { + "/api/user/v2/users/": ("UserViewSet", {"get": "list"}), + "/api/user/v2/users/alice/": ("UserViewSet", {"get": "retrieve"}), + "/api/user/v2/user_preferences/": ("UserPreferenceViewSet", {"get": "list"}), + "/api/user/v2/user_preferences/alice,time_zone/": ( + "UserPreferenceViewSet", {"get": "retrieve"}, + ), + "/api/user/v2/time_zones/": ("CountryTimeZoneViewSet", {"get": "list"}), + } + for address, (class_name, actions) in expected.items(): + match = resolve(address) + assert view_identity(match.func) == (V2_VIEW_MODULE, class_name), address + assert routed_actions(match.func) == actions, address + + def test_username_with_a_dot_resolves_to_the_member_route(self): + match = resolve("/api/user/v2/users/ada.lovelace/") + assert view_identity(match.func) == (V2_VIEW_MODULE, "UserViewSet") + assert match.kwargs == {"username": "ada.lovelace"} + + def test_preference_member_address_splits_into_username_and_key(self): + match = resolve("/api/user/v2/user_preferences/ada.lovelace,pref-lang/") + assert match.kwargs == {"username": "ada.lovelace", "key": "pref-lang"} + + def test_a_key_containing_a_comma_belongs_to_the_key(self): + """A preference name is free-form text, and the username half cannot contain a comma.""" + match = resolve("/api/user/v2/user_preferences/ada.lovelace,a,b/") + assert match.kwargs == {"username": "ada.lovelace", "key": "a,b"} + assert reverse( + "user_v2:user_preference_detail", kwargs={"username": "ada.lovelace", "key": "a,b"}, + ) == "/api/user/v2/user_preferences/ada.lovelace,a,b/" + + def test_a_username_outside_the_allowed_pattern_does_not_reach_the_viewset(self): + match = resolve("/api/user/v2/user_preferences/al!ce,pref-lang/") + assert view_identity(match.func) == (V2_VIEW_MODULE, "UnknownAddressView") + + def test_no_database_id_member_address_is_served(self): + match = resolve("/api/user/v2/user_preferences/42/") + assert view_identity(match.func) == (V2_VIEW_MODULE, "UnknownAddressView") + + +@skip_unless_lms +class RouteShapeTests(APITestCase): + """ + Every address this version serves is named and ends in a slash. + + The mount also carries one pattern that is neither: the fallback matched + after all of them, which answers an address no route serves. It addresses + no resource and reverses to nothing, which is why it has no name, and + pinning it here keeps a second wildcard from being added quietly beside + it. + """ + + def test_every_address_is_named_and_ends_in_a_slash(self): + addresses = [pattern for pattern in v2_urls.urlpatterns if pattern.name] + assert sorted(pattern.name for pattern in addresses) == [ + "time_zone_list", + "user_detail", + "user_list", + "user_preference_detail", + "user_preference_list", + ] + for pattern in addresses: + assert SNAKE_CASE_NAME.match(pattern.name), pattern.name + assert str(pattern.pattern).endswith("/$"), pattern.name + + def test_the_router_route_that_is_replaced_has_to_exist(self): + """Dropping a route the router does not generate is refused instead of passing silently.""" + routes = v2_urls.router.urls + assert v2_urls.REPLACED_ROUTER_ROUTE in {route.name for route in routes} + kept = v2_urls.without_route(routes, v2_urls.REPLACED_ROUTER_ROUTE) + assert [route.name for route in kept] == [ + "user_list", + "user_detail", + "user_preference_list", + "time_zone_list", + ] + with pytest.raises(ImproperlyConfigured): + v2_urls.without_route(routes, "user_preference_detail_renamed") + + def test_the_one_unnamed_pattern_is_the_fallback_and_it_is_matched_last(self): + unnamed = [pattern for pattern in v2_urls.urlpatterns if not pattern.name] + assert len(unnamed) == 1 + assert unnamed[0] is v2_urls.urlpatterns[-1] + assert view_identity(unnamed[0].callback) == (V2_VIEW_MODULE, "UnknownAddressView") + + +@skip_unless_lms +class LegacyAddressTests(APITestCase): + """Mounting version 2 leaves the version 1 surface exactly where it was.""" + + def test_version_one_addresses_still_reach_the_version_one_views(self): + expected = { + "/api/user/v1/users/": "UserViewSet", + "/api/user/v1/users/42/": "UserViewSet", + "/api/user/v1/user_prefs/": "UserPreferenceViewSet", + "/api/user/v1/user_prefs/42/": "UserPreferenceViewSet", + "/api/user/v1/preferences/pref-lang/users/": "PreferenceUsersListView", + "/api/user/v1/forum_roles/Moderator/users/": "ForumRoleUsersListView", + "/api/user/v1/preferences/time_zones/": "CountryTimeZoneListView", + } + for address, class_name in expected.items(): + match = resolve(address) + assert view_identity(match.func) == (V1_VIEW_MODULE, class_name), address + + def test_version_one_url_names_still_reverse(self): + assert reverse("accounts_api", kwargs={"username": "alice"}) == "/api/user/v1/accounts/alice" + assert reverse("own_username_api") == "/api/user/v1/me" + assert reverse("preferences_api", kwargs={"username": "alice"}) == "/api/user/v1/preferences/alice" + + def test_unversioned_twins_still_resolve(self): + match = resolve("/user_api/v1/users/") + assert view_identity(match.func) == (V1_VIEW_MODULE, "UserViewSet") + + +@skip_unless_lms +class MalformedAddressTests(APITestCase): + """An address that matches no route is refused in the body shape this API answers with.""" + + def setUp(self): + super().setUp() + self.admin = AdminFactory.create(username="url-admin", password=PASSWORD) + assert self.client.login(username=self.admin.username, password=PASSWORD) + + def assert_refused_in_json(self, response): + """Assert the refusal is the standard error body and not a page meant for a browser.""" + assert response["Content-Type"].startswith("application/json"), response["Content-Type"] + assert "\w+)", + re.IGNORECASE, +) + +#: The platform's ``SafeSessionMiddleware`` puts the signed-in caller's e-mail +#: into the session the first time it finds it missing, which rewrites that one +#: row once per session; the deployed configuration keeps sessions in the cache +#: and only the test settings put them in a table at all. It is framework +#: bookkeeping rather than a change to the data this API serves, and it is the +#: only table left out of the captures below. +SESSION_TABLE = "django_session" + + +def write_target(sql): + """Return the table a statement writes to, or ``None`` if it cannot be read.""" + match = WRITE_TARGET.match(sql) + return match.group("table") if match else None + + +def write_queries(captured): + """ + Return the captured statements that would change the data this API serves. + + Only the statement's own target table decides: a write to a domain table is + reported however it mentions the session table elsewhere, and a write whose + target cannot be read is reported rather than assumed harmless. + """ + return [ + query["sql"] + for query in captured + if WRITE_VERB.match(query["sql"]) and write_target(query["sql"]) != SESSION_TABLE + ] + + +class WriteCaptureFilterTests(SimpleTestCase): + """The capture filter reports writes by their target table and nothing else.""" + + PREFERENCE_INSERT = ( + 'INSERT INTO "user_api_userpreference" ("user_id", "key", "value") ' + "VALUES (1, 'pref-lang', 'django_session')" + ) + PREFERENCE_DELETE = ( + 'DELETE FROM "user_api_userpreference" WHERE "user_id" IN ' + '(SELECT "user_id" FROM "django_session" WHERE "session_key" = \'abc\')' + ) + ROLE_INSERT = 'INSERT INTO "django_comment_common_role" ("name") VALUES (\'Moderator\')' + SESSION_UPDATE = 'UPDATE "django_session" SET "session_data" = \'x\' WHERE "session_key" = \'abc\'' + SESSION_INSERT = 'INSERT INTO "django_session" ("session_key", "session_data") VALUES (\'abc\', \'x\')' + SESSION_DELETE = 'DELETE FROM "django_session" WHERE "session_key" = \'abc\'' + USER_SELECT = 'SELECT "auth_user"."id" FROM "auth_user" ORDER BY "auth_user"."username" ASC' + UNREADABLE_WRITE = 'UPDATE /* hint */ "user_api_userpreference" SET "value" = \'en\'' + + def captured(self, *statements): + """Return the statements in the shape ``CaptureQueriesContext`` records them.""" + return [{"sql": statement, "time": "0.000"} for statement in statements] + + def test_domain_writes_are_reported_even_when_they_name_the_session_table(self): + reported = write_queries( + self.captured(self.PREFERENCE_INSERT, self.PREFERENCE_DELETE, self.ROLE_INSERT), + ) + assert reported == [self.PREFERENCE_INSERT, self.PREFERENCE_DELETE, self.ROLE_INSERT] + + def test_session_bookkeeping_is_the_only_statement_dropped(self): + reported = write_queries( + self.captured( + self.SESSION_INSERT, + self.SESSION_UPDATE, + self.SESSION_DELETE, + self.USER_SELECT, + self.ROLE_INSERT, + ), + ) + assert reported == [self.ROLE_INSERT] + + def test_reads_are_not_writes(self): + assert write_queries(self.captured(self.USER_SELECT)) == [] + + def test_a_write_whose_target_cannot_be_read_is_still_reported(self): + assert write_target(self.UNREADABLE_WRITE) is None + assert write_queries(self.captured(self.UNREADABLE_WRITE)) == [self.UNREADABLE_WRITE] + + def test_the_target_table_is_read_through_any_backend_quoting(self): + assert write_target(self.ROLE_INSERT) == "django_comment_common_role" + assert write_target("UPDATE `django_session` SET `session_data` = 'x'") == SESSION_TABLE + assert write_target(" insert into user_api_userpreference (value) values ('x')") == ( + "user_api_userpreference" + ) + assert write_target("INSERT OR IGNORE INTO `user_api_userpreference` (`value`) VALUES ('en')") == ( + "user_api_userpreference" + ) + assert write_target(self.USER_SELECT) is None + + +class UserApiV2TestCase(APITestCase): + """Fixtures shared by the version 2 User API tests.""" + + def setUp(self): + super().setUp() + self.admin = AdminFactory.create(username="admin-caller", password=PASSWORD) + self.learner = UserFactory.create(username="learner-caller", password=PASSWORD) + self.alice = UserFactory.create(username="alice", email="alice@example.com", password=PASSWORD) + self.bob = UserFactory.create(username="bob", email="bob@example.com", password=PASSWORD) + UserPreferenceFactory.create(user=self.alice, key="pref-lang", value="en") + UserPreferenceFactory.create(user=self.bob, key="time_zone", value="UTC") + + def login(self, user): + """Sign the given user in through the session the platform default accepts.""" + assert self.client.login(username=user.username, password=PASSWORD) + + def usernames_in(self, response): + """Return the usernames carried by a paginated directory page.""" + return [row["username"] for row in response.data["results"]] + + +@skip_unless_lms +class UserListViewTests(UserApiV2TestCase): + """GET /api/user/v2/users/""" + + def setUp(self): + super().setUp() + self.url = reverse("user_v2:user_list") + + def test_anonymous_caller_is_unauthenticated(self): + response = self.client.get(self.url) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_non_admin_caller_is_forbidden(self): + self.login(self.learner) + response = self.client.get(self.url) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_non_admin_cannot_list_another_users_preference_holders(self): + self.login(self.learner) + response = self.client.get(self.url, {"preference_key": "pref-lang"}) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_admin_caller_receives_the_directory(self): + self.login(self.admin) + response = self.client.get(self.url, {"page_size": 100}) + assert response.status_code == status.HTTP_200_OK + assert set(response.data) == PAGINATION_KEYS + assert self.usernames_in(response) == ["admin-caller", "alice", "bob", "learner-caller"] + assert response.data["count"] == 4 + assert response.data["num_pages"] == 1 + assert response.data["current_page"] == 1 + assert response.data["start"] == 0 + + def test_row_shape_matches_the_declared_fields(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice"}) + row = response.data["results"][0] + assert set(row) == {"id", "url", "email", "name", "username", "preferences"} + assert row["id"] == self.alice.id + assert row["email"] == "alice@example.com" + assert row["name"] == self.alice.profile.name + assert row["preferences"] == {"pref-lang": "en"} + assert row["url"].endswith("/api/user/v2/users/alice/") + + def test_second_page_reports_its_position(self): + self.login(self.admin) + response = self.client.get(self.url, {"page_size": 2, "page": 2}) + assert response.status_code == status.HTTP_200_OK + assert self.usernames_in(response) == ["bob", "learner-caller"] + assert response.data["current_page"] == 2 + assert response.data["num_pages"] == 2 + assert response.data["start"] == 2 + assert response.data["previous"] is not None + assert response.data["next"] is None + + def test_a_page_beyond_the_last_one_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url, {"page": 9999}) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_a_page_number_that_is_not_a_number_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url, {"page": "abc"}) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_ordering_can_be_reversed(self): + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "-username", "page_size": 100}) + assert self.usernames_in(response) == ["learner-caller", "bob", "alice", "admin-caller"] + + def test_ordering_by_join_date(self): + self.alice.date_joined = datetime(2020, 1, 1, tzinfo=UTC) + self.alice.save() + self.bob.date_joined = datetime(2019, 1, 1, tzinfo=UTC) + self.bob.save() + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "date_joined", "page_size": 100}) + assert self.usernames_in(response)[-2:] == ["bob", "alice"] + + def test_unknown_ordering_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "password", "page_size": 100}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "ordering": ["Sort by any of: date_joined, username, each optionally prefixed with '-'."], + } + + def test_one_unknown_field_refuses_the_whole_sort(self): + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "username,password", "page_size": 100}) + assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + + def test_filter_by_email(self): + self.login(self.admin) + response = self.client.get(self.url, {"email": "bob@example.com"}) + assert self.usernames_in(response) == ["bob"] + + def test_filter_by_is_active(self): + self.alice.is_active = False + self.alice.save() + self.login(self.admin) + response = self.client.get(self.url, {"is_active": "false", "page_size": 100}) + assert self.usernames_in(response) == ["alice"] + + def test_filter_by_is_active_keeps_only_the_active_accounts(self): + self.alice.is_active = False + self.alice.save() + self.login(self.admin) + response = self.client.get(self.url, {"is_active": "true", "page_size": 100}) + assert self.usernames_in(response) == ["admin-caller", "bob", "learner-caller"] + + def test_an_is_active_value_that_names_no_side_is_refused(self): + """A mistyped value must be refused, not read as "no preference" and answered unfiltered.""" + self.alice.is_active = False + self.alice.save() + self.login(self.admin) + response = self.client.get(self.url, {"is_active": "maybe", "page_size": 100}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == {"is_active": ["Enter true or false."]} + + def test_an_empty_is_active_value_narrows_nothing(self): + self.alice.is_active = False + self.alice.save() + self.login(self.admin) + response = self.client.get(self.url, {"is_active": "", "page_size": 100}) + assert response.status_code == status.HTTP_200_OK + assert self.usernames_in(response) == ["admin-caller", "alice", "bob", "learner-caller"] + + def test_filter_by_preference_key(self): + self.login(self.admin) + response = self.client.get(self.url, {"preference_key": "pref-lang", "page_size": 100}) + assert self.usernames_in(response) == ["alice"] + + def test_filter_by_unknown_preference_key_returns_no_rows(self): + self.login(self.admin) + response = self.client.get(self.url, {"preference_key": "nope", "page_size": 100}) + assert response.data["results"] == [] + assert response.data["count"] == 0 + + def give_alice_the_moderator_role(self): + """Make alice a moderator of the course the forum-role tests narrow by.""" + role = Role.objects.create(course_id=COURSE_KEY, name="Moderator") + role.users.add(self.alice) + return role + + def test_filter_by_forum_role_and_course(self): + self.give_alice_the_moderator_role() + other_role = Role.objects.create(course_id=OTHER_COURSE_KEY, name="Moderator") + other_role.users.add(self.bob) + self.login(self.admin) + response = self.client.get( + self.url, {"forum_role": "Moderator", "course_key": str(COURSE_KEY), "page_size": 100}, + ) + assert self.usernames_in(response) == ["alice"] + + def test_anonymous_caller_cannot_list_a_courses_forum_role_holders(self): + self.give_alice_the_moderator_role() + response = self.client.get( + self.url, {"forum_role": "Moderator", "course_key": str(COURSE_KEY)}, + ) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_non_admin_cannot_list_a_courses_forum_role_holders(self): + self.give_alice_the_moderator_role() + self.login(self.learner) + response = self.client.get( + self.url, {"forum_role": "Moderator", "course_key": str(COURSE_KEY)}, + ) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_holding_the_role_does_not_let_a_learner_list_its_holders(self): + self.give_alice_the_moderator_role() + self.login(self.alice) + response = self.client.get( + self.url, {"forum_role": "Moderator", "course_key": str(COURSE_KEY)}, + ) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_the_published_course_key_example_can_be_pasted_into_a_query_string(self): + """A caller that copies the documented key into a query string reaches the role holders.""" + example = re.search( + r"for example (course-v1:[^\s.]+)", + UserFilterSet.base_filters["course_key"].extra["help_text"], + ).group(1) + role = Role.objects.create(course_id=COURSE_KEY, name="Moderator") + role.users.add(self.alice) + self.login(self.admin) + response = self.client.get(f"{self.url}?forum_role=Moderator&course_key={example}") + assert response.status_code == status.HTTP_200_OK + assert self.usernames_in(response) == ["alice"] + + def test_a_course_key_whose_plus_signs_are_not_encoded_is_refused(self): + """An unencoded plus sign arrives as a space, which no course key may contain.""" + self.login(self.admin) + response = self.client.get( + f"{self.url}?forum_role=Moderator&course_key=course-v1:edX+DemoX+Demo", + ) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == {"course_key": ["Enter a valid course key."]} + + def test_a_user_holding_the_role_through_several_rows_is_listed_once(self): + """Nothing stops a course from carrying one role name under several rows.""" + for _ in range(2): + role = Role.objects.create(course_id=COURSE_KEY, name="Moderator") + role.users.add(self.alice) + self.login(self.admin) + response = self.client.get( + self.url, {"forum_role": "Moderator", "course_key": str(COURSE_KEY), "page_size": 100}, + ) + assert self.usernames_in(response) == ["alice"] + assert response.data["count"] == 1 + + def test_forum_role_without_course_key_is_rejected(self): + self.login(self.admin) + response = self.client.get(self.url, {"forum_role": "Moderator"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "course_key": ["This parameter is required when forum_role is supplied."], + } + + def test_course_key_without_forum_role_is_rejected(self): + self.login(self.admin) + response = self.client.get(self.url, {"course_key": str(COURSE_KEY)}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "forum_role": ["This parameter is required when course_key is supplied."], + } + + def test_malformed_course_key_is_rejected(self): + self.login(self.admin) + response = self.client.get(self.url, {"forum_role": "Moderator", "course_key": "not a course"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == {"course_key": ["Enter a valid course key."]} + + def test_unknown_forum_role_returns_no_rows_and_creates_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + response = self.client.get( + self.url, {"forum_role": "Inquisitor", "course_key": str(COURSE_KEY)}, + ) + assert response.status_code == status.HTTP_200_OK + assert response.data["results"] == [] + assert Role.objects.filter(name="Inquisitor").count() == 0 + assert write_queries(captured.captured_queries) == [] + + def test_minimal_view_returns_only_identifying_fields(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "view": "minimal"}) + row = response.data["results"][0] + assert set(row) == {"id", "username", "email"} + assert row["username"] == "alice" + + def test_fields_parameter_projects_the_row(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "fields": "username,email"}) + assert response.data["results"] == [{"username": "alice", "email": "alice@example.com"}] + + def test_fields_parameter_narrows_the_minimal_view(self): + self.login(self.admin) + response = self.client.get( + self.url, {"username": "alice", "view": "minimal", "fields": "username"}, + ) + assert response.data["results"] == [{"username": "alice"}] + + def test_a_field_name_the_row_does_not_carry_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"fields": "user_name"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "fields": [ + "No such field: user_name. " + "Select any of: email, id, name, preferences, url, username.", + ], + } + + def test_a_field_the_minimal_preset_drops_is_refused(self): + """The preset decides what the row carries, so the selection is judged against it.""" + self.login(self.admin) + response = self.client.get(self.url, {"view": "minimal", "fields": "name"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "fields": ["No such field: name. Select any of: email, id, username."], + } + + def test_every_unknown_field_name_is_named_at_once(self): + self.login(self.admin) + response = self.client.get(self.url, {"fields": "username,e_mail,joined"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"]["fields"][0].startswith("No such field: e_mail, joined.") + + def test_a_field_name_with_surrounding_space_is_accepted(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "fields": " username , email "}) + assert response.data["results"] == [{"username": "alice", "email": "alice@example.com"}] + + def test_an_unknown_field_does_not_answer_an_anonymous_caller(self): + response = self.client.get(self.url, {"fields": "user_name"}) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_an_unknown_response_preset_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"view": "nonsense"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "view": ["Ask for any of: minimal, or omit this parameter for the default response."], + } + + def test_the_preset_that_belongs_to_the_preference_list_is_refused_here(self): + self.login(self.admin) + response = self.client.get(self.url, {"view": "full"}) + assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + + def test_an_empty_response_preset_returns_the_default_shape(self): + """A query string built from an empty variable narrows nothing rather than failing.""" + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "view": ""}) + assert response.status_code == status.HTTP_200_OK + assert set(response.data["results"][0]) == { + "id", "url", "email", "name", "username", "preferences", + } + + def test_an_unknown_preset_does_not_answer_an_anonymous_caller(self): + response = self.client.get(self.url, {"view": "nonsense"}) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_listing_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url, {"page_size": 100}) + assert write_queries(captured.captured_queries) == [] + + def test_head_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + response = self.client.head(self.url) + assert response.status_code == status.HTTP_200_OK + assert write_queries(captured.captured_queries) == [] + + +@skip_unless_lms +class UserDetailViewTests(UserApiV2TestCase): + """GET /api/user/v2/users/{username}/""" + + def url_for(self, username): + """Return the member address of the named user.""" + return reverse("user_v2:user_detail", kwargs={"username": username}) + + def test_anonymous_caller_is_unauthenticated(self): + response = self.client.get(self.url_for("alice")) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_non_admin_caller_is_forbidden(self): + self.login(self.learner) + response = self.client.get(self.url_for("alice")) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_owner_who_is_not_an_admin_is_forbidden(self): + self.login(self.alice) + response = self.client.get(self.url_for("alice")) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_admin_caller_receives_the_entry(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice")) + assert response.status_code == status.HTTP_200_OK + assert response.data["username"] == "alice" + assert response.data["email"] == "alice@example.com" + assert response.data["preferences"] == {"pref-lang": "en"} + + def test_unknown_username_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url_for("nobody")) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_username_containing_a_dot_resolves(self): + UserFactory.create(username="ada.lovelace", email="ada@example.com", password=PASSWORD) + self.login(self.admin) + response = self.client.get(self.url_for("ada.lovelace")) + assert response.status_code == status.HTTP_200_OK + assert response.data["username"] == "ada.lovelace" + + def test_minimal_view_returns_only_identifying_fields(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice"), {"view": "minimal"}) + assert set(response.data) == {"id", "username", "email"} + + def test_a_field_name_the_entry_does_not_carry_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice"), {"fields": "user_name"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "fields": [ + "No such field: user_name. " + "Select any of: email, id, name, preferences, url, username.", + ], + } + + def test_fields_parameter_projects_the_entry(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice"), {"fields": "username"}) + assert response.data == {"username": "alice"} + + def test_an_unknown_response_preset_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice"), {"view": "nonsense"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "view": ["Ask for any of: minimal, or omit this parameter for the default response."], + } + + def test_retrieval_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url_for("alice")) + assert write_queries(captured.captured_queries) == [] + + def test_head_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + response = self.client.head(self.url_for("alice")) + assert response.status_code == status.HTTP_200_OK + assert write_queries(captured.captured_queries) == [] + + +@skip_unless_lms +class ProfilelessUserTests(UserApiV2TestCase): + """A user with no profile row is listed with a null name rather than failing.""" + + def test_user_without_a_profile_has_a_null_name(self): + User.objects.create_user("profileless", "profileless@example.com", PASSWORD) + self.login(self.admin) + response = self.client.get( + reverse("user_v2:user_detail", kwargs={"username": "profileless"}), + ) + assert response.status_code == status.HTTP_200_OK + assert response.data["name"] is None + + +@skip_unless_lms +class UserPreferenceListViewTests(UserApiV2TestCase): + """GET /api/user/v2/user_preferences/""" + + def setUp(self): + super().setUp() + self.url = reverse("user_v2:user_preference_list") + + def keys_in(self, response): + """Return the (username, key) pairs carried by a page of preferences.""" + return [(row["username"], row["key"]) for row in response.data["results"]] + + def test_anonymous_caller_is_unauthenticated(self): + response = self.client.get(self.url) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_non_admin_caller_is_forbidden(self): + self.login(self.learner) + response = self.client.get(self.url) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_owner_who_is_not_an_admin_is_forbidden(self): + self.login(self.alice) + response = self.client.get(self.url, {"username": "alice"}) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_admin_caller_receives_every_preference(self): + self.login(self.admin) + response = self.client.get(self.url, {"page_size": 100}) + assert response.status_code == status.HTTP_200_OK + assert set(response.data) == PAGINATION_KEYS + assert self.keys_in(response) == [("alice", "pref-lang"), ("bob", "time_zone")] + + def test_row_shape_is_flat_by_default(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice"}) + row = response.data["results"][0] + assert set(row) == {"username", "key", "value", "url", "user"} + assert row["username"] == "alice" + assert row["key"] == "pref-lang" + assert row["value"] == "en" + assert row["user"] is None + assert row["url"].endswith("/api/user/v2/user_preferences/alice,pref-lang/") + + def test_full_view_embeds_the_owning_user(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "view": "full"}) + row = response.data["results"][0] + assert set(row["user"]) == {"id", "url", "email", "name", "username", "preferences"} + assert row["user"]["username"] == "alice" + assert row["user"]["preferences"] == {"pref-lang": "en"} + + def test_minimal_view_drops_the_address_and_owner(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "view": "minimal"}) + assert response.data["results"] == [{"username": "alice", "key": "pref-lang", "value": "en"}] + + def test_a_field_name_the_row_does_not_carry_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"fields": "preference_key"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "fields": [ + "No such field: preference_key. " + "Select any of: key, url, user, username, value.", + ], + } + + def test_a_page_beyond_the_last_one_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url, {"page": 9999}) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_fields_parameter_projects_the_row(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice", "fields": "key,value"}) + assert response.data["results"] == [{"key": "pref-lang", "value": "en"}] + + def test_filter_by_key(self): + UserPreferenceFactory.create(user=self.bob, key="pref-lang", value="fr") + self.login(self.admin) + response = self.client.get(self.url, {"key": "pref-lang", "page_size": 100}) + assert self.keys_in(response) == [("alice", "pref-lang"), ("bob", "pref-lang")] + + def test_filter_by_username(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "bob", "page_size": 100}) + assert self.keys_in(response) == [("bob", "time_zone")] + + def test_the_owner_id_the_earlier_list_took_is_refused_rather_than_ignored(self): + """A caller that ported by changing the version segment is told, not quietly unfiltered.""" + self.login(self.admin) + response = self.client.get(self.url, {"user": self.alice.id, "page_size": 100}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "user": [f"No such query parameter. Send any of: {PREFERENCE_QUERY_PARAMS}."], + } + + def test_an_answered_page_announces_no_deprecated_parameter(self): + self.login(self.admin) + response = self.client.get(self.url, {"username": "alice"}) + assert response.status_code == status.HTTP_200_OK + assert "Deprecation" not in response + + def test_ordering_by_key_descending(self): + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "-key", "page_size": 100}) + assert self.keys_in(response) == [("bob", "time_zone"), ("alice", "pref-lang")] + + def test_ordering_takes_the_fields_in_the_order_they_are_named(self): + UserPreferenceFactory.create(user=self.bob, key="pref-lang", value="fr") + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "key,-username", "page_size": 100}) + assert self.keys_in(response) == [ + ("bob", "pref-lang"), ("alice", "pref-lang"), ("bob", "time_zone"), + ] + + def test_unknown_ordering_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"ordering": "value", "page_size": 100}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "ordering": ["Sort by any of: key, username, each optionally prefixed with '-'."], + } + + def test_listing_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url, {"page_size": 100}) + assert write_queries(captured.captured_queries) == [] + + def test_full_view_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url, {"page_size": 100, "view": "full"}) + assert write_queries(captured.captured_queries) == [] + + def test_head_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + response = self.client.head(self.url) + assert response.status_code == status.HTTP_200_OK + assert write_queries(captured.captured_queries) == [] + + def test_an_unknown_response_preset_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url, {"view": "nonsense"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "view": [ + "Ask for any of: full, minimal, or omit this parameter for the default response.", + ], + } + + +@skip_unless_lms +class UserPreferenceDetailViewTests(UserApiV2TestCase): + """GET /api/user/v2/user_preferences/{username},{key}/""" + + def url_for(self, username, key): + """Return the member address of one stored preference.""" + return reverse("user_v2:user_preference_detail", kwargs={"username": username, "key": key}) + + def test_anonymous_caller_is_unauthenticated(self): + response = self.client.get(self.url_for("alice", "pref-lang")) + assert_error_envelope(response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn") + + def test_non_admin_caller_is_forbidden(self): + self.login(self.learner) + response = self.client.get(self.url_for("alice", "pref-lang")) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_owner_who_is_not_an_admin_is_forbidden(self): + self.login(self.alice) + response = self.client.get(self.url_for("alice", "pref-lang")) + assert_error_envelope(response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz") + + def test_admin_caller_receives_the_preference(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "pref-lang")) + assert response.status_code == status.HTTP_200_OK + assert set(response.data) == {"username", "key", "value", "url", "user"} + assert response.data["username"] == "alice" + assert response.data["key"] == "pref-lang" + assert response.data["value"] == "en" + assert response.data["user"] is None + + def test_full_view_embeds_the_owning_user(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "pref-lang"), {"view": "full"}) + assert response.data["user"]["username"] == "alice" + + def test_a_field_name_the_preference_does_not_carry_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "pref-lang"), {"fields": "preference_key"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "fields": [ + "No such field: preference_key. " + "Select any of: key, url, user, username, value.", + ], + } + + def test_unknown_key_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "nope")) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_key_belonging_to_another_user_is_not_found(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "time_zone")) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_an_unknown_response_preset_is_refused(self): + self.login(self.admin) + response = self.client.get(self.url_for("alice", "pref-lang"), {"view": "nonsense"}) + assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + + def test_retrieval_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url_for("alice", "pref-lang")) + assert write_queries(captured.captured_queries) == [] + + def test_head_writes_nothing(self): + self.login(self.admin) + with CaptureQueriesContext(connection) as captured: + response = self.client.head(self.url_for("alice", "pref-lang")) + assert response.status_code == status.HTTP_200_OK + assert write_queries(captured.captured_queries) == [] + + +@skip_unless_lms +class TimeZoneListViewTests(UserApiV2TestCase): + """GET /api/user/v2/time_zones/""" + + def setUp(self): + super().setUp() + self.url = reverse("user_v2:time_zone_list") + + def test_anonymous_caller_receives_the_list(self): + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + assert set(response.data) == PAGINATION_KEYS + assert len(response.data["results"]) == 10 + assert set(response.data["results"][0]) == {"time_zone", "description"} + + def test_authenticated_caller_receives_the_same_count(self): + anonymous_count = self.client.get(self.url).data["count"] + self.login(self.learner) + response = self.client.get(self.url) + assert response.data["count"] == anonymous_count + + def test_country_code_narrows_the_list(self): + response = self.client.get(self.url, {"country_code": "FR", "page_size": 100}) + assert [row["time_zone"] for row in response.data["results"]] == ["Europe/Paris"] + + def test_country_code_is_case_insensitive(self): + upper = self.client.get(self.url, {"country_code": "FR", "page_size": 100}).data["results"] + lower = self.client.get(self.url, {"country_code": "fr", "page_size": 100}).data["results"] + assert upper == lower + + def test_country_without_time_zones_falls_back_to_every_zone(self): + every = self.client.get(self.url).data["count"] + response = self.client.get(self.url, {"country_code": "BV"}) + assert response.data["count"] == every + + def test_an_empty_country_code_returns_every_zone(self): + """A query string built from an empty variable narrows nothing rather than failing.""" + every = self.client.get(self.url).data["count"] + response = self.client.get(self.url, {"country_code": ""}) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == every + + def test_a_page_beyond_the_last_one_is_not_found(self): + """The list is public, so its most common failure is answered to anyone.""" + response = self.client.get(self.url, {"page": 9999}) + assert_error_envelope(response, expected_status=status.HTTP_404_NOT_FOUND, expected_type_slug="not-found") + + def test_malformed_country_code_is_rejected(self): + response = self.client.get(self.url, {"country_code": "FRA"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == { + "country_code": ["Enter a two-letter ISO 3166-1 alpha-2 country code, for example FR."], + } + + def test_listing_writes_nothing(self): + with CaptureQueriesContext(connection) as captured: + self.client.get(self.url, {"page_size": 100}) + assert write_queries(captured.captured_queries) == [] + + def test_head_writes_nothing(self): + with CaptureQueriesContext(connection) as captured: + response = self.client.head(self.url) + assert response.status_code == status.HTTP_200_OK + assert write_queries(captured.captured_queries) == [] + + +@skip_unless_lms +class PageSizeCapTests(UserApiV2TestCase): + """ + No caller can ask any of the lists for more rows than one maximum page. + + The time-zone list is the one a caller reaches without signing in, and it + is long enough to be worth asking for whole, so the cap is asserted against + live rows on every list rather than read off the paginator. + """ + + MAX_PAGE_SIZE = 100 + + def test_the_directory_page_is_capped(self): + User.objects.bulk_create( + User(username=f"extra-{index:03d}", email=f"extra-{index:03d}@example.com") + for index in range(self.MAX_PAGE_SIZE) + ) + self.login(self.admin) + response = self.client.get(reverse("user_v2:user_list"), {"page_size": 1000}) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == self.MAX_PAGE_SIZE + 4 + assert len(response.data["results"]) == self.MAX_PAGE_SIZE + assert response.data["num_pages"] == 2 + assert response.data["next"] is not None + + def test_the_stored_preference_page_is_capped(self): + UserPreference.objects.bulk_create( + UserPreference(user=self.alice, key=f"extra-{index:03d}", value="x") + for index in range(self.MAX_PAGE_SIZE) + ) + self.login(self.admin) + response = self.client.get(reverse("user_v2:user_preference_list"), {"page_size": 1000}) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == self.MAX_PAGE_SIZE + 2 + assert len(response.data["results"]) == self.MAX_PAGE_SIZE + assert response.data["num_pages"] == 2 + + def test_the_time_zone_page_is_capped_for_an_anonymous_caller(self): + every_zone = len(get_country_time_zones()) + assert every_zone > self.MAX_PAGE_SIZE, every_zone + response = self.client.get(reverse("user_v2:time_zone_list"), {"page_size": 1000}) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == every_zone + assert len(response.data["results"]) == self.MAX_PAGE_SIZE + assert response.data["num_pages"] == ceil(every_zone / self.MAX_PAGE_SIZE) + + +@skip_unless_lms +class RefusedRequestTests(UserApiV2TestCase): + """A method this API does not serve, and an Accept header it cannot answer.""" + + #: Public, standardized, and served by another API: what its refused method + #: is called must not change because this one names its own. + OTHER_API_URL = "/api/enrollment/v2/courses/course-v1:edX+DemoX+Demo/" + + def setUp(self): + super().setUp() + self.public_url = reverse("user_v2:time_zone_list") + + def test_a_refused_method_is_named_for_an_anonymous_caller(self): + response = self.client.post(self.public_url, {}, format="json") + envelope = assert_error_envelope( + response, + expected_status=status.HTTP_405_METHOD_NOT_ALLOWED, + expected_type_slug="method-not-allowed", + ) + assert envelope["title"] == "Method Not Allowed" + assert envelope["detail"] == 'Method "POST" not allowed.' + assert response["Content-Type"] == "application/json" + + def test_an_accept_header_this_api_cannot_answer_is_named(self): + response = self.client.get(self.public_url, HTTP_ACCEPT="application/xml") + envelope = assert_error_envelope( + response, + expected_status=status.HTTP_406_NOT_ACCEPTABLE, + expected_type_slug="not-acceptable", + ) + assert envelope["title"] == "Not Acceptable" + assert envelope["detail"] == "Could not satisfy the request Accept header." + assert response["Content-Type"] == "application/json" + + def test_a_refused_method_is_named_on_the_gated_collections(self): + self.login(self.admin) + for url_name in ("user_v2:user_list", "user_v2:user_preference_list"): + response = self.client.delete(reverse(url_name)) + envelope = assert_error_envelope( + response, + expected_status=status.HTTP_405_METHOD_NOT_ALLOWED, + expected_type_slug="method-not-allowed", + ) + assert envelope["detail"] == 'Method "DELETE" not allowed.' + + def test_a_refused_method_is_named_on_the_gated_members(self): + self.login(self.admin) + response = self.client.put(reverse("user_v2:user_detail", args=[self.alice.username])) + envelope = assert_error_envelope( + response, + expected_status=status.HTTP_405_METHOD_NOT_ALLOWED, + expected_type_slug="method-not-allowed", + ) + assert envelope["detail"] == 'Method "PUT" not allowed.' + + def test_another_api_is_classified_exactly_as_the_shared_catalog_classifies_it(self): + """The catalog names no refused method, and nothing here changes that for anyone else.""" + response = self.client.post(self.OTHER_API_URL, {}, format="json") + envelope = assert_error_envelope( + response, + expected_status=status.HTTP_405_METHOD_NOT_ALLOWED, + expected_type_slug="internal", + ) + assert envelope["title"] == "Internal Server Error" + + +@skip_unless_lms +class UnknownQueryParameterTests(UserApiV2TestCase): + """Each endpoint refuses a query parameter it reads nothing from, and takes the rest.""" + + def setUp(self): + super().setUp() + self.user_list = reverse("user_v2:user_list") + self.user_detail = reverse("user_v2:user_detail", kwargs={"username": "alice"}) + self.preference_list = reverse("user_v2:user_preference_list") + self.preference_detail = reverse( + "user_v2:user_preference_detail", kwargs={"username": "alice", "key": "pref-lang"}, + ) + self.time_zone_list = reverse("user_v2:time_zone_list") + + def assert_refused(self, response, name, offered): + """Assert the response named the parameter it would not take, and the ones it takes.""" + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert envelope["errors"] == {name: [f"No such query parameter. Send any of: {offered}."]} + + def assert_each_is_answered(self, address, parameter_sets): + """Assert the endpoint answers each of these query strings.""" + for params in parameter_sets: + with self.subTest(params=params): + response = self.client.get(address, params) + assert response.status_code == status.HTTP_200_OK, response.content + + def test_the_directory_refuses_a_parameter_it_does_not_read(self): + self.login(self.admin) + response = self.client.get(self.user_list, {"lms_user_id": self.alice.id, "page_size": 100}) + self.assert_refused(response, "lms_user_id", USER_QUERY_PARAMS) + + def test_a_mistyped_filter_refuses_the_page_instead_of_returning_every_row(self): + self.login(self.admin) + response = self.client.get(self.user_list, {"usernme": "alice", "page_size": 100}) + self.assert_refused(response, "usernme", USER_QUERY_PARAMS) + + def test_every_unrecognised_parameter_is_named_at_once(self): + self.login(self.admin) + response = self.client.get(self.user_list, {"sort": "username", "limit": "10"}) + envelope = assert_error_envelope( + response, expected_status=status.HTTP_400_BAD_REQUEST, expected_type_slug="validation", + ) + assert sorted(envelope["errors"]) == ["limit", "sort"] + + def test_a_directory_entry_refuses_a_parameter_it_does_not_read(self): + self.login(self.admin) + response = self.client.get(self.user_detail, {"expand": "profile"}) + self.assert_refused(response, "expand", USER_QUERY_PARAMS) + + def test_the_preference_list_refuses_a_parameter_it_does_not_read(self): + self.login(self.admin) + response = self.client.get(self.preference_list, {"value": "en", "page_size": 100}) + self.assert_refused(response, "value", PREFERENCE_QUERY_PARAMS) + + def test_a_stored_preference_refuses_a_parameter_it_does_not_read(self): + self.login(self.admin) + response = self.client.get(self.preference_detail, {"value": "en"}) + self.assert_refused(response, "value", PREFERENCE_QUERY_PARAMS) + + def test_the_time_zone_list_refuses_a_parameter_it_does_not_read(self): + response = self.client.get(self.time_zone_list, {"country": "FR"}) + self.assert_refused(response, "country", TIME_ZONE_QUERY_PARAMS) + + def test_the_time_zone_list_refuses_a_sort_it_does_not_offer(self): + """This list is ordered by its display form and reads no sort parameter.""" + response = self.client.get(self.time_zone_list, {"ordering": "country_code"}) + self.assert_refused(response, "ordering", TIME_ZONE_QUERY_PARAMS) + + def test_an_unknown_parameter_does_not_answer_an_anonymous_caller(self): + response = self.client.get(self.user_list, {"lms_user_id": self.alice.id}) + assert_error_envelope( + response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + + def test_an_unknown_parameter_does_not_answer_a_non_admin_caller(self): + self.login(self.learner) + response = self.client.get(self.preference_list, {"lms_user_id": self.alice.id}) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + def test_the_directory_answers_every_parameter_it_reads(self): + self.login(self.admin) + self.assert_each_is_answered(self.user_list, [ + {"page": 1}, + {"page_size": 2}, + {"ordering": "-username"}, + {"view": "minimal"}, + {"fields": "username"}, + {"format": "json"}, + {"username": "alice"}, + {"email": "alice@example.com"}, + {"is_active": "true"}, + {"preference_key": "pref-lang"}, + {"forum_role": "Moderator", "course_key": str(COURSE_KEY)}, + ]) + + def test_a_directory_entry_answers_every_parameter_it_reads(self): + self.login(self.admin) + self.assert_each_is_answered(self.user_detail, [ + {"view": "minimal"}, {"fields": "username"}, {"format": "json"}, {"ordering": "username"}, + ]) + + def test_the_preference_list_answers_every_parameter_it_reads(self): + self.login(self.admin) + self.assert_each_is_answered(self.preference_list, [ + {"page": 1}, + {"page_size": 2}, + {"ordering": "-key"}, + {"view": "full"}, + {"fields": "key"}, + {"format": "json"}, + {"key": "pref-lang"}, + {"username": "alice"}, + ]) + + def test_a_stored_preference_answers_every_parameter_it_reads(self): + self.login(self.admin) + self.assert_each_is_answered(self.preference_detail, [ + {"view": "full"}, {"fields": "key"}, {"format": "json"}, + ]) + + def test_the_time_zone_list_answers_every_parameter_it_reads(self): + self.assert_each_is_answered(self.time_zone_list, [ + {"country_code": "FR"}, {"page": 1}, {"page_size": 5}, {"format": "json"}, + ]) + + def test_a_page_of_the_directory_is_still_narrowed_by_the_filters_it_takes(self): + """The refusal guards the parameters, not the filtering they ask for.""" + self.login(self.admin) + response = self.client.get(self.user_list, {"username": "alice", "page_size": 100}) + assert self.usernames_in(response) == ["alice"] + + +@skip_unless_lms +class TokenCallerTests(UserApiV2TestCase): + """The staff-gated endpoints answer a bearer of a staff token and refuse everyone else.""" + + def setUp(self): + super().setUp() + self.addresses = { + "user_list": reverse("user_v2:user_list"), + "user_detail": reverse("user_v2:user_detail", kwargs={"username": "alice"}), + "preference_list": reverse("user_v2:user_preference_list"), + "preference_detail": reverse( + "user_v2:user_preference_detail", kwargs={"username": "alice", "key": "pref-lang"}, + ), + } + + def token_headers(self, user): + """Return the request headers that carry a token identifying the given user.""" + return {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(user)}"} + + def test_a_staff_token_reaches_the_directory(self): + response = self.client.get( + self.addresses["user_list"], {"page_size": 100}, **self.token_headers(self.admin), + ) + assert response.status_code == status.HTTP_200_OK + assert self.usernames_in(response) == ["admin-caller", "alice", "bob", "learner-caller"] + assert response.data["count"] == 4 + + def test_a_staff_token_reaches_every_staff_gated_address(self): + headers = self.token_headers(self.admin) + detail = self.client.get(self.addresses["user_detail"], **headers) + assert detail.status_code == status.HTTP_200_OK + assert detail.data["username"] == "alice" + + preferences = self.client.get( + self.addresses["preference_list"], {"username": "alice"}, **headers, + ) + assert preferences.status_code == status.HTTP_200_OK + assert [(row["username"], row["key"]) for row in preferences.data["results"]] == [ + ("alice", "pref-lang"), + ] + + preference = self.client.get(self.addresses["preference_detail"], **headers) + assert preference.status_code == status.HTTP_200_OK + assert preference.data["value"] == "en" + + def test_the_token_and_not_a_session_identifies_the_caller(self): + token = create_jwt_for_user(self.admin) + response = self.client.get( + self.addresses["user_list"], {"username": "alice"}, HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == status.HTTP_200_OK + assert response.wsgi_request.user == self.admin + assert response.wsgi_request.auth == token + assert "sessionid" not in response.wsgi_request.COOKIES + + def test_a_learners_token_is_forbidden_on_every_staff_gated_address(self): + headers = self.token_headers(self.learner) + for name, address in self.addresses.items(): + with self.subTest(address=name): + response = self.client.get(address, **headers) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + def test_the_owner_of_the_record_is_still_not_staff(self): + response = self.client.get( + self.addresses["preference_detail"], **self.token_headers(self.alice), + ) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + def test_a_token_that_does_not_decode_is_unauthenticated(self): + response = self.client.get( + self.addresses["user_list"], HTTP_AUTHORIZATION="JWT not.a.token", + ) + assert_error_envelope( + response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + + +@skip_unless_lms +class SharedSortValueTests(UserApiV2TestCase): + """Users that compare equal on the sorted column are ordered the same way on every page.""" + + def setUp(self): + super().setUp() + self.url = reverse("user_v2:user_list") + joined = datetime(2024, 3, 1, tzinfo=UTC) + for username in ("zoe-shared", "mia-shared", "ada-shared"): + UserFactory.create(username=username, date_joined=joined) + self.login(self.admin) + + def test_users_sharing_a_join_date_are_ordered_by_username(self): + response = self.client.get(self.url, {"ordering": "date_joined", "page_size": 100}) + shared = [name for name in self.usernames_in(response) if name.endswith("-shared")] + assert shared == ["ada-shared", "mia-shared", "zoe-shared"] + + def test_the_sort_the_caller_asked_for_still_decides_the_order(self): + """The tie-break settles rows that compare equal; the requested sort places them.""" + response = self.client.get(self.url, {"ordering": "-date_joined", "page_size": 100}) + assert self.usernames_in(response)[:3] == ["ada-shared", "mia-shared", "zoe-shared"] + + def test_paging_through_a_shared_join_date_lists_each_user_once(self): + listed = [] + for page in (1, 2, 3): + response = self.client.get( + self.url, {"ordering": "date_joined", "page_size": 3, "page": page}, + ) + listed.extend(self.usernames_in(response)) + assert sorted(listed) == [ + "ada-shared", "admin-caller", "alice", "bob", "learner-caller", "mia-shared", "zoe-shared", + ] + + +@skip_unless_lms +class DeactivatedStaffCallerTests(UserApiV2TestCase): + """ + A staff account whose user record has been deactivated is refused by both halves of + the default authentication pair: the session half stops accepting it, and the active + account these views require refuses a token issued before the account was deactivated. + """ + + def setUp(self): + super().setUp() + self.url = reverse("user_v2:user_list") + self.staff = AdminFactory.create(username="deactivated-admin", password=PASSWORD) + self.staff_gated = { + "user_list": self.url, + "user_detail": reverse("user_v2:user_detail", kwargs={"username": "alice"}), + "preference_list": reverse("user_v2:user_preference_list"), + "preference_detail": reverse( + "user_v2:user_preference_detail", kwargs={"username": "alice", "key": "pref-lang"}, + ), + } + + def deactivate(self, user): + """Clear an account's active flag the way an operator does, leaving its staff flag.""" + user.is_active = False + user.save() + + def test_a_session_held_by_a_deactivated_staff_account_is_refused(self): + self.login(self.staff) + assert self.client.get(self.url).status_code == status.HTTP_200_OK + self.deactivate(self.staff) + response = self.client.get(self.url) + assert_error_envelope( + response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + + def test_a_token_held_by_a_deactivated_staff_account_is_refused(self): + headers = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(self.staff)}"} + assert self.client.get(self.url, **headers).status_code == status.HTTP_200_OK + self.deactivate(self.staff) + response = self.client.get(self.url, {"page_size": 100}, **headers) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + def test_a_deactivated_staff_token_is_refused_at_every_staff_gated_address(self): + self.deactivate(self.staff) + headers = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(self.staff)}"} + for name, address in self.staff_gated.items(): + with self.subTest(address=name): + response = self.client.get(address, **headers) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + def test_a_deactivated_staff_session_is_refused_at_every_staff_gated_address(self): + self.login(self.staff) + self.deactivate(self.staff) + for name, address in self.staff_gated.items(): + with self.subTest(address=name): + response = self.client.get(address) + assert_error_envelope( + response, expected_status=status.HTTP_401_UNAUTHORIZED, expected_type_slug="authn", + ) + + def test_the_directory_does_not_list_the_account_that_was_refused(self): + """The refusal is the whole response: no row of the page leaks past it.""" + self.deactivate(self.staff) + response = self.client.get( + self.url, {"page_size": 100}, HTTP_AUTHORIZATION=f"JWT {create_jwt_for_user(self.staff)}", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert "results" not in response.data + + def test_an_active_staff_token_still_reaches_the_directory(self): + response = self.client.get( + self.url, {"page_size": 100}, HTTP_AUTHORIZATION=f"JWT {create_jwt_for_user(self.staff)}", + ) + assert response.status_code == status.HTTP_200_OK + assert "deactivated-admin" in self.usernames_in(response) + + def test_a_token_held_by_a_deactivated_learner_is_still_forbidden(self): + self.deactivate(self.learner) + response = self.client.get( + self.url, HTTP_AUTHORIZATION=f"JWT {create_jwt_for_user(self.learner)}", + ) + assert_error_envelope( + response, expected_status=status.HTTP_403_FORBIDDEN, expected_type_slug="authz", + ) + + +@skip_unless_lms +class PreferenceSerializerTests(UserApiV2TestCase): + """The preference address is built with or without a request to make it absolute.""" + + def test_url_is_relative_without_a_request(self): + preference = self.alice.preferences.get(key="pref-lang") + data = UserPreferenceRecordSerializer(preference).data + assert data["url"] == "/api/user/v2/user_preferences/alice,pref-lang/" + assert data["user"] is None diff --git a/openedx/core/djangoapps/user_api/v2/urls.py b/openedx/core/djangoapps/user_api/v2/urls.py new file mode 100644 index 000000000000..911fb321af69 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/urls.py @@ -0,0 +1,71 @@ +"""URLs for version 2 of the User API.""" + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured +from django.urls import re_path +from rest_framework.routers import SimpleRouter + +from .views import ( + PREFERENCE_KEY_PATTERN_PARTIAL, + CountryTimeZoneViewSet, + UnknownAddressView, + UserPreferenceViewSet, + UserViewSet, +) + +app_name = "user_v2" + +#: A preference is identified by its owner and its key, so the router's +#: database-id member route is dropped and replaced by the address declared +#: below under the same name. +REPLACED_ROUTER_ROUTE = "user_preference_detail" + + +def without_route(routes, name): + """ + Return ``routes`` without the one registered under ``name``. + + A name that matches nothing means the router stopped generating that route + under that name: the address it serves would then be published alongside + the one declared here instead of being replaced by it, so the mismatch is + refused here rather than discovered at the address. + """ + kept = [route for route in routes if route.name != name] + if len(kept) == len(routes): + raise ImproperlyConfigured(f"The router registers no route named {name!r} to replace.") + return kept + + +class SnakeCaseRouter(SimpleRouter): + """ + A router that names its routes ``_``. + + The names a route is registered under are the ones ``resolve()`` reports + and the ones ``reverse()`` accepts, so they are generated in the published + form rather than rewritten afterwards. + """ + + routes = [route._replace(name=route.name.replace("-", "_")) for route in SimpleRouter.routes] + + +router = SnakeCaseRouter() +router.register(r"users", UserViewSet, basename="user") +router.register(r"user_preferences", UserPreferenceViewSet, basename="user_preference") +router.register(r"time_zones", CountryTimeZoneViewSet, basename="time_zone") + + +urlpatterns = [ + *without_route(router.urls, REPLACED_ROUTER_ROUTE), + # The username half carries the pattern a username is allowed to match and + # the key half takes the rest of the segment. + re_path( + rf"^user_preferences/(?P{settings.USERNAME_REGEX_PARTIAL})" + rf",(?P{PREFERENCE_KEY_PATTERN_PARTIAL})/$", + UserPreferenceViewSet.as_view({"get": "retrieve"}), + name="user_preference_detail", + ), + # The authentication app serves two addresses under this prefix, which this + # mount is matched before; everything else under it belongs to this version. + # The route answers rather than reverses, so it carries no name. + re_path(r"^(?!account/).*$", UnknownAddressView.as_view()), +] diff --git a/openedx/core/djangoapps/user_api/v2/views.py b/openedx/core/djangoapps/user_api/v2/views.py new file mode 100644 index 000000000000..6022c5dcec40 --- /dev/null +++ b/openedx/core/djangoapps/user_api/v2/views.py @@ -0,0 +1,593 @@ +"""Views for version 2 of the User API.""" + +import logging + +from django.conf import settings +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from django.db.models import F +from django.http import Http404 +from django_filters.rest_framework import DjangoFilterBackend +from drf_spectacular.utils import OpenApiParameter, OpenApiResponse, extend_schema, extend_schema_view +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from edx_rest_framework_extensions.paginators import IterablePaginationMixin +from edx_rest_framework_extensions.scoping import FullScopePolicy, ScopedQuerysetMixin +from edx_rest_framework_extensions.shaping import MinimalViewMixin, project +from rest_framework import permissions, viewsets +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.reverse import reverse +from rest_framework.settings import api_settings +from rest_framework.views import APIView + +from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.preferences.api import get_country_time_zones + +from .errors import CatalogedRequestErrorsMixin +from .filters import StrictOrderingFilter, UserFilterSet, UserPreferenceFilterSet +from .paginators import UserApiPagination +from .permissions import IsActiveUser +from .schema import ResponseShapes +from .serializers import ( + FULL_VIEW, + MINIMAL_VIEW, + PREFERENCE_MINIMAL_FIELDS, + USER_MINIMAL_FIELDS, + ErrorResponse, + TimeZoneQuerySerializer, + TimeZoneSerializer, + UserDirectoryMinimalSerializer, + UserDirectoryProjectionSerializer, + UserDirectorySerializer, + UserPreferenceRecordMinimalSerializer, + UserPreferenceRecordProjectionSerializer, + UserPreferenceRecordSerializer, + full_view_requested, +) + +log = logging.getLogger(__name__) + +USER_TAG = "user" + +#: Fields the user directory sorts on, and the order it uses when asked for none. +USER_SORT_FIELDS = ("username", "date_joined") +USER_DEFAULT_SORT = ("username",) + +#: Fields the stored-preference list sorts on, and the order it uses when asked for none. +PREFERENCE_SORT_FIELDS = ("key", "username") +PREFERENCE_DEFAULT_SORT = ("username", "key") + +#: The part of a preference's address that carries its key. The key is +#: free-form text that may itself contain a comma, so it takes the rest of the +#: segment; the route and the published address are built from this one pattern. +PREFERENCE_KEY_PATTERN_PARTIAL = "[^/]+" + + +def _query_param(name, description, **kwargs): + """Build an optional query parameter for the published schema.""" + return OpenApiParameter( + name=name, + description=description, + required=False, + location=OpenApiParameter.QUERY, + **kwargs, + ) + + +def _path_param(name, description, pattern): + """Build a path parameter for the published schema, carrying what its route matches.""" + return OpenApiParameter( + name=name, + description=description, + required=True, + location=OpenApiParameter.PATH, + type=str, + pattern=pattern, + ) + + +_USERNAME_PATTERN = rf"^{settings.USERNAME_REGEX_PARTIAL}$" + +_USER_PATH_PARAM = _path_param( + "username", + "Unique name of the user whose directory entry is returned.", + _USERNAME_PATTERN, +) +_PREFERENCE_USERNAME_PATH_PARAM = _path_param( + "username", + "Username of the user the preference belongs to: the part of the address before the comma.", + _USERNAME_PATTERN, +) +_PREFERENCE_KEY_PATH_PARAM = _path_param( + "key", + "Name the preference is stored under: the part of the address after the comma.", + rf"^{PREFERENCE_KEY_PATTERN_PARTIAL}$", +) + +#: Parameters that shape a response rather than select the rows in it. +VIEW_PARAM = "view" +FIELDS_PARAM = "fields" +RESPONSE_SHAPING_PARAMS = (VIEW_PARAM, FIELDS_PARAM) + +#: The framework's own representation selector. This deployment reads nothing +#: from it, since ``URL_FORMAT_OVERRIDE`` is unset and JSON is the only +#: renderer, and a caller that sends it anyway is answered rather than refused. +FORMAT_PARAM = "format" + +_FIELDS_PARAM = _query_param( + FIELDS_PARAM, + "Comma-separated list of top-level field names to keep in each returned object. " + "A name the response cannot carry is refused.", +) +#: The presets each list answers to, published as the parameter's enum and +#: accepted by the view from the same declaration. +USER_PRESETS = (MINIMAL_VIEW,) +PREFERENCE_PRESETS = (FULL_VIEW, MINIMAL_VIEW) + +_MINIMAL_VIEW_PARAM = _query_param( + VIEW_PARAM, + "Response preset. `minimal` reduces each user to its identifying fields " + "(`id`, `username`, `email`). Omit it to receive the full entry.", + enum=list(USER_PRESETS), +) +_PREFERENCE_VIEW_PARAM = _query_param( + VIEW_PARAM, + "Response preset. `full` embeds the owning user's whole directory entry in each " + "preference; `minimal` returns only `username`, `key` and `value`. Omit it to " + "receive the owner's username alongside the preference.", + enum=list(PREFERENCE_PRESETS), +) + + +def _ordering_param(sort_fields, default): + """Build the sort parameter of a list, naming the fields it accepts.""" + return _query_param( + "ordering", + "Fields to sort by, in order of precedence, each optionally prefixed with `-` to sort " + "descending. A field that is not listed here is refused.", + many=True, + enum=[*sort_fields, *(f"-{field}" for field in sort_fields)], + default=list(default), + style="form", + explode=False, + ) + + +_USER_ORDERING_PARAM = _ordering_param(USER_SORT_FIELDS, USER_DEFAULT_SORT) +_PREFERENCE_ORDERING_PARAM = _ordering_param(PREFERENCE_SORT_FIELDS, PREFERENCE_DEFAULT_SORT) + +_RESP_UNAUTHENTICATED = OpenApiResponse(ErrorResponse, description="The requester is not authenticated.") +_RESP_FORBIDDEN = OpenApiResponse(ErrorResponse, description="The requester may not use this endpoint.") +_RESP_NOT_FOUND = OpenApiResponse(ErrorResponse, description="No such resource.") +_RESP_NO_SUCH_PAGE = OpenApiResponse(ErrorResponse, description="There is no such page of results.") +_RESP_BAD_REQUEST = OpenApiResponse( + ErrorResponse, description="A query parameter is missing, invalid, or not one this endpoint reads.", +) + +#: A directory entry comes back whole, as the identifying fields the ``minimal`` +#: preset keeps, or as the properties ``fields`` selected. +_USER_SHAPES = ResponseShapes( + component_name="UserDirectoryResponse", + serializers=[ + UserDirectorySerializer, + UserDirectoryMinimalSerializer, + UserDirectoryProjectionSerializer, + ], +) + +#: A stored preference comes back whole, as the three fields the ``minimal`` +#: preset keeps, or as the properties ``fields`` selected. +_PREFERENCE_SHAPES = ResponseShapes( + component_name="UserPreferenceRecordResponse", + serializers=[ + UserPreferenceRecordSerializer, + UserPreferenceRecordMinimalSerializer, + UserPreferenceRecordProjectionSerializer, + ], +) + + +class _MemberLookupMixin: + """Report a missing member through the standard error envelope.""" + + def get_object(self): + try: + return super().get_object() + except Http404 as exc: + raise NotFound from exc + + +class _ActionAddressMixin: + """ + Builds the address of an action from the snake_case names these routes carry. + + A router names an action ``-`` by default, and that name + belongs to an earlier mount of the same resource, so the default lookup + would hand out the older address instead of this one. + """ + + #: The namespace these routes are mounted under, read when an address is + #: asked for outside a request and there is no resolver match to take it from. + url_namespace = "user_v2" + + def reverse_action(self, url_name, *args, **kwargs): + """Return the address of one of this viewset's actions.""" + request = getattr(self, "request", None) + namespace = self.url_namespace + if request is not None and request.resolver_match is not None: + namespace = request.resolver_match.namespace or namespace + kwargs.setdefault("request", request) + return reverse(f"{namespace}:{self.basename}_{url_name}", *args, **kwargs) + + +def _requested_field_names(raw): + """Return the top-level property names a ``fields`` value asks for.""" + if not raw: + return [] + return [name.strip().split(".", 1)[0] for name in raw.split(",") if name.strip()] + + +class _StrictQueryParametersMixin: + """ + Refuses a query parameter the view reads nothing from. + + An unknown parameter is otherwise dropped in silence: a caller that + mistyped a filter, or kept one an earlier version of the endpoint + answered, receives the whole collection and reads it as the narrowed page + it asked for. The names a view answers to are assembled from what it + declares (its paginator, its sort backend, its filter set, and the + serializer that validates its query string), so a parameter that is added + or renamed cannot fall out of step with this check. + """ + + #: Names the view reads that none of the declarations below account for. + extra_query_params = () + + def known_query_params(self): + """Return the names of the query parameters this view reads.""" + names = {FORMAT_PARAM, api_settings.URL_FORMAT_OVERRIDE, *self.extra_query_params} + pagination_class = getattr(self, "pagination_class", None) + if pagination_class is not None: + names.update({pagination_class.page_query_param, pagination_class.page_size_query_param}) + for backend in getattr(self, "filter_backends", ()): + names.add(getattr(backend, "ordering_param", None)) + filterset_class = getattr(self, "filterset_class", None) + if filterset_class is not None: + names.update(filterset_class.base_filters) + query_serializer_class = getattr(self, "query_serializer_class", None) + if query_serializer_class is not None: + names.update(query_serializer_class().fields) + return names - {None} + + def initial(self, request, *args, **kwargs): + """Check the query string once the caller has been admitted.""" + super().initial(request, *args, **kwargs) + known = self.known_query_params() + unknown = sorted(set(request.query_params) - known) + if unknown: + log.info("Query refused on %s: %r", type(self).__name__, unknown) + offered = ", ".join(sorted(known)) + raise ValidationError({ + name: [f"No such query parameter. Send any of: {offered}."] + for name in unknown + }) + + +class _ResponsePresetMixin(MinimalViewMixin): + """ + Applies the ``view`` and ``fields`` presets to an already serialized payload. + + A view lists the presets it answers to, and a ``view`` value outside that + list is refused: a caller that mistypes a preset asked for a smaller + payload and would otherwise be handed the full one without being told. + The same holds for a field name the response cannot carry, which would + otherwise select nothing and leave the caller with empty objects. + """ + + #: Presets this view answers to. An empty ``view`` narrows nothing, the way + #: an empty value of any other parameter does. + response_presets = () + + def initial(self, request, *args, **kwargs): + """Check the requested preset and fields once the caller has been admitted.""" + super().initial(request, *args, **kwargs) + preset = request.query_params.get(VIEW_PARAM) + if preset and preset not in self.response_presets: + raise ValidationError({ + VIEW_PARAM: [ + f"Ask for any of: {', '.join(self.response_presets)}, " + "or omit this parameter for the default response.", + ], + }) + self.check_requested_fields(request) + + def shapeable_fields(self): + """Return the names of the properties a response of this view can carry.""" + if self.is_minimal_view_requested(): + return set(self.minimal_fields) + return set(self.get_serializer_class()().fields) + + def check_requested_fields(self, request): + """Refuse a selected field the response cannot carry, naming the ones it can.""" + available = self.shapeable_fields() + unknown = sorted(set(_requested_field_names(request.query_params.get(FIELDS_PARAM))) - available) + if unknown: + raise ValidationError({ + FIELDS_PARAM: [ + f"No such field: {', '.join(unknown)}. " + f"Select any of: {', '.join(sorted(available))}.", + ], + }) + + def shape(self, data, request): + """Return ``data`` reduced to the preset and the fields the caller asked for.""" + data = self.shape_minimal(data, request) + requested_fields = request.query_params.get(FIELDS_PARAM) + if not requested_fields: + return data + if isinstance(data, list): + return [project(item, requested_fields) for item in data] + return project(data, requested_fields) + + def shape_page(self, response, request): + """Shape the items of a paginated response in place.""" + response.data["results"] = self.shape(response.data["results"], request) + return response + + +@extend_schema(tags=[USER_TAG]) +@extend_schema_view( + list=extend_schema( + summary="List users", + description=( + "Returns a page of the user directory. Supply `preference_key` to keep only the " + "users who have stored a preference under that key, or `forum_role` together with " + "`course_key` to keep only the holders of a course forum role." + ), + parameters=[_MINIMAL_VIEW_PARAM, _FIELDS_PARAM, _USER_ORDERING_PARAM], + responses={ + 200: _USER_SHAPES, + 400: _RESP_BAD_REQUEST, + 401: _RESP_UNAUTHENTICATED, + 403: _RESP_FORBIDDEN, + 404: _RESP_NO_SUCH_PAGE, + }, + ), + retrieve=extend_schema( + summary="Get one user", + description="Returns the directory entry of the user with this username.", + parameters=[_USER_PATH_PARAM, _MINIMAL_VIEW_PARAM, _FIELDS_PARAM], + responses={ + 200: _USER_SHAPES, + 400: _RESP_BAD_REQUEST, + 401: _RESP_UNAUTHENTICATED, + 403: _RESP_FORBIDDEN, + 404: _RESP_NOT_FOUND, + }, + ), +) +class UserViewSet( + StandardizedErrorMixin, + CatalogedRequestErrorsMixin, + _ActionAddressMixin, + _MemberLookupMixin, + _ResponsePresetMixin, + _StrictQueryParametersMixin, + ScopedQuerysetMixin, + viewsets.ReadOnlyModelViewSet, +): + """ + Read-only directory of platform users, addressed by username. + + Access is layered: ``permission_classes`` admits only platform + administrators whose account is still active, the scoping policy decides + which rows such an administrator may see, and the filter set narrows those + rows further by the caller's query parameters. + """ + + permission_classes = (permissions.IsAuthenticated, IsActiveUser, permissions.IsAdminUser) + serializer_class = UserDirectorySerializer + pagination_class = UserApiPagination + queryset = User.objects.all().prefetch_related("preferences").select_related("profile") + # Platform administrators may read every entry, so the policy keeps the + # queryset whole; a narrower policy can replace it without touching the + # filtering below. + scoping_policy = FullScopePolicy() + lookup_field = "username" + # The router's default lookup pattern excludes dots, which usernames allow. + lookup_value_regex = settings.USERNAME_REGEX_PARTIAL + filter_backends = (DjangoFilterBackend, StrictOrderingFilter) + filterset_class = UserFilterSet + ordering_fields = USER_SORT_FIELDS + ordering = USER_DEFAULT_SORT + # A sort the caller asks for is completed with these so that it is total: + # rows that compare equal would otherwise be ordered differently from one + # page to the next, and a client reading the pages in turn would see some + # of them twice and miss others. + ordering_tiebreak = ("username",) + minimal_fields = USER_MINIMAL_FIELDS + response_presets = USER_PRESETS + extra_query_params = RESPONSE_SHAPING_PARAMS + + def list(self, request, *args, **kwargs): + """Return a page of directory entries, shaped as the caller asked.""" + return self.shape_page(super().list(request, *args, **kwargs), request) + + def retrieve(self, request, *args, **kwargs): + """Return one directory entry, shaped as the caller asked.""" + response = super().retrieve(request, *args, **kwargs) + response.data = self.shape(response.data, request) + return response + + +@extend_schema(tags=[USER_TAG]) +@extend_schema_view( + list=extend_schema( + summary="List stored preferences", + description=( + "Returns a page of stored user preferences. Supply `key` to keep only the " + "preferences stored under that name, or `username` to keep only one user's." + ), + parameters=[_PREFERENCE_VIEW_PARAM, _FIELDS_PARAM, _PREFERENCE_ORDERING_PARAM], + responses={ + 200: _PREFERENCE_SHAPES, + 400: _RESP_BAD_REQUEST, + 401: _RESP_UNAUTHENTICATED, + 403: _RESP_FORBIDDEN, + 404: _RESP_NO_SUCH_PAGE, + }, + ), + retrieve=extend_schema( + summary="Get one stored preference", + description=( + "Returns the preference stored under this key for this user. A preference is " + "identified by the pair, which the address carries as the username, a comma and " + "the key, for example `alice,time_zone`." + ), + operation_id="v2_user_preferences_retrieve", + parameters=[ + _PREFERENCE_USERNAME_PATH_PARAM, + _PREFERENCE_KEY_PATH_PARAM, + _PREFERENCE_VIEW_PARAM, + _FIELDS_PARAM, + ], + responses={ + 200: _PREFERENCE_SHAPES, + 400: _RESP_BAD_REQUEST, + 401: _RESP_UNAUTHENTICATED, + 403: _RESP_FORBIDDEN, + 404: _RESP_NOT_FOUND, + }, + ), +) +class UserPreferenceViewSet( + StandardizedErrorMixin, + CatalogedRequestErrorsMixin, + _ActionAddressMixin, + _ResponsePresetMixin, + _StrictQueryParametersMixin, + ScopedQuerysetMixin, + viewsets.ReadOnlyModelViewSet, +): + """ + Read-only list of stored user preferences, each addressed by its owner and key. + + A preference is identified by the pair, so the member address is + ``{username},{key}`` rather than a database id. Access is layered the same + way as the user directory: permission classes, then the scoping policy, + then the filter set. + """ + + permission_classes = (permissions.IsAuthenticated, IsActiveUser, permissions.IsAdminUser) + serializer_class = UserPreferenceRecordSerializer + pagination_class = UserApiPagination + queryset = ( + UserPreference.objects.all() + .select_related("user", "user__profile") + .annotate(username=F("user__username")) + ) + scoping_policy = FullScopePolicy() + filter_backends = (DjangoFilterBackend, StrictOrderingFilter) + filterset_class = UserPreferenceFilterSet + ordering_fields = PREFERENCE_SORT_FIELDS + ordering = PREFERENCE_DEFAULT_SORT + # The owner and the key identify a row, so they complete any sort the + # caller asks for and keep the order of a page stable. + ordering_tiebreak = ("username", "key") + minimal_fields = PREFERENCE_MINIMAL_FIELDS + response_presets = PREFERENCE_PRESETS + extra_query_params = RESPONSE_SHAPING_PARAMS + + def filter_queryset(self, queryset): + """Fetch each owner's own preferences too when they are about to be embedded.""" + queryset = super().filter_queryset(queryset) + if full_view_requested(getattr(self, "request", None)): + queryset = queryset.prefetch_related("user__preferences") + return queryset + + def get_object(self): + """Look the preference up by the username and key that identify it.""" + try: + preference = self.get_queryset().get( + user__username=self.kwargs["username"], key=self.kwargs["key"], + ) + except UserPreference.DoesNotExist as exc: + raise NotFound from exc + self.check_object_permissions(self.request, preference) + return preference + + def list(self, request, *args, **kwargs): + """Return a page of stored preferences, shaped as the caller asked.""" + return self.shape_page(super().list(request, *args, **kwargs), request) + + def retrieve(self, request, *args, **kwargs): + """Return one stored preference, shaped as the caller asked.""" + response = super().retrieve(request, *args, **kwargs) + response.data = self.shape(response.data, request) + return response + + +@extend_schema(tags=[USER_TAG]) +class CountryTimeZoneViewSet( + StandardizedErrorMixin, + CatalogedRequestErrorsMixin, + _ActionAddressMixin, + _StrictQueryParametersMixin, + IterablePaginationMixin, + viewsets.ViewSet, +): + """ + Time zones commonly used in a country, or every time zone when no country is named. + + The rows come from the time-zone database rather than from a queryset, so + this is a plain ViewSet whose one query parameter is validated by a + serializer. The reference data it returns is the same for every caller and + is public on purpose. + """ + + permission_classes = (permissions.AllowAny,) + serializer_class = TimeZoneSerializer + #: Validates the query string, and names the parameters this view reads. + query_serializer_class = TimeZoneQuerySerializer + pagination_class = UserApiPagination + + def get_serializer(self, *args, **kwargs): + """Build the output serializer; a plain ViewSet has no generic machinery to do it.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List time zones", + description=( + "Returns a page of time zones, ordered by their display form. Supply " + "`country_code` to receive the time zones commonly used in one country." + ), + # The parameters are published from the serializer that validates them, + # so the constraint a caller is held to is the one it reads. + parameters=[TimeZoneQuerySerializer], + responses={ + 200: TimeZoneSerializer, + 400: _RESP_BAD_REQUEST, + 404: _RESP_NO_SUCH_PAGE, + }, + ) + def list(self, request): + """Return a page of time zones, for one country or for the world.""" + query = self.query_serializer_class(data=request.query_params) + query.is_valid(raise_exception=True) + time_zones = get_country_time_zones(query.validated_data.get("country_code")) + return self.paginate_iterable(request, time_zones) + + +@extend_schema(exclude=True) +class UnknownAddressView(StandardizedErrorMixin, APIView): + """ + Refuses every address under this version of the API that no route serves. + + It serves no resource: it exists so that a mistyped address is answered + with the same JSON body as every other failure instead of the service's + HTML error page, which a client that asked for JSON cannot read. + """ + + permission_classes = (permissions.AllowAny,) + + def initial(self, request, *args, **kwargs): + """Refuse the address before a handler is chosen, whatever the method.""" + raise NotFound