Repository navigation
feat: standardize the user directory, preference and time-zone APIs as /api/user/v2/ - #39237
Abdul-Muqadim-Arbisoft wants to merge 2 commits into
Conversation
The user directory, the stored-preference listings and the time-zone
reference list were the least conforming endpoints of the User API: the
only gate was the deprecated X-Edx-Api-Key header, which ignores
request.user entirely, so any holder of the key could read every user's
email address and full preference mapping. The listings were addressed by
database id, two of them put the parent key in the path instead of the
query string, the time-zone list returned an unbounded array, the stored
preference row embedded its owner's entire record (one extra query per
row), and the forum-role listing created the role it was asked about on
every GET.
Each of those is a contract change, so they land on a new version and
/api/user/v1/ is untouched and keeps serving them.
What callers get instead:
GET /api/user/v2/users/ (also ?preference_key=,
?forum_role=&course_key=)
GET /api/user/v2/users/{username}/
GET /api/user/v2/user_preferences/
GET /api/user/v2/user_preferences/{username},{key}/
GET /api/user/v2/time_zones/
Authenticated staff callers rather than an API key, the standard error
envelope, the standard pagination envelope on all three lists, filter
sets and a deterministic ordering, and the ?view= and ?fields= response
presets. The forum-role filter no longer creates a role, and an
unparsable course key is a validation error rather than a 500.
Being refused is part of the contract, so the refusals are explicit. A
deactivated staff account is turned away on both paths rather than only
one: the session authenticator already rejected it, but IsAdminUser reads
only is_staff and the JWT authenticator does not read is_active, so such
an account could read the whole directory until its token expired. These
addresses require an active user.
An unrecognised query parameter is a 400 rather than silence. A client
porting from a superseded address by changing the version segment alone
would otherwise pass a filter this version does not define, have it
ignored, and receive every row believing the page was filtered. The
allowlist is derived from the paginator, the ordering backend, the filter
set and the query serializer, so it cannot drift away from the filters it
describes.
Shared building blocks are used rather than rewritten:
StandardizedErrorMixin, DefaultPagination, IterablePaginationMixin,
ScopedQuerysetMixin with FullScopePolicy, MinimalViewMixin and project.
The two local classes each supply one predicate the library does not,
and each docstring says which.
The tests pin behaviour rather than structure: every allowed and every
refused caller with the error type asserted, value-level parity with each
superseded address, query counts that stay constant across 1, 5 and 25
rows where the superseded listing pays per row, SQL captures proving no
GET writes, and the URL literals each name reverses to.
The LMS drf-spectacular pre-processing hook is an allowlist and admitted only /api/enrollment/vN/, so everything under /api/user/ generated, routed and tested while being invisible in the published document. The new v2 addresses would have shipped undocumented and the schema gate would have reported the prefix missing with no clue as to why. The filter now admits exactly what this change owns: the v2 addresses, matched by their view package rather than by path prefix, plus the seven legacy paths they supersede, so each superseded address appears beside its replacement. It is deliberately not a /api/user/vN/ prefix. Admitting the whole prefix pulled thirty-eight further v1 operations into the document -- undocumented request bodies and internal docstrings published as client-facing text -- none of which this change owns or documents. SCHEMA_PATH_PREFIX becomes a two-branch pattern, stripped only to derive tags and operationIds. Every existing enrollment operationId was compared before and after and is unchanged, and so is every operationId, path and component of the service-wide /api-docs document: a generated client derives a module per operationId and a model per component, so a rename there deletes a module as surely as a removal does. The title follows the contents, and the three comments that named the old title follow it. Deprecation is marked where it belongs, outside the frozen modules: the existing post-processing hook gains a tuple of the seven exact legacy paths this change supersedes. Exact paths, not a prefix, so an address whose replacement nobody has written yet can never be marked -- the hook test includes neighbouring user addresses, the unversioned twins and another area's paths and asserts they are left alone. POSTPROCESSING_HOOKS is unchanged: setting it replaces drf-spectacular's defaults, so the enum hook stays restated and stays first.
Reference detail for this pull requestMoved out of the description: GitHub caps a PR body at 65,536 characters. Nothing is omitted. What else a caller or a client generator seesEvery caller-visible change this branch makes, including the ones no row above carries. Nothing
Backward compatibilityNothing on
Follow-ups
|
Summary
This is phase A of five for the
[API] Userfunctional area(#39058), under the API standardization
umbrella #38137. The area is split A → E
on that issue's plan; this PR carries A only — seven read-only operations — and B, C, D and E
are separate PRs. B (accounts and profile images), C (preferences and email opt-in) and D (the
retirement pipeline and username replacement) follow this one; E (registration, login session,
validation and password reset) is not approved to build and is listed here only so the phase count
is not mistaken for four.
Phase A is deliberately the smallest blast radius in the area — the user directory, the
stored-preference listings and the time-zone reference list — and it carries the shared wiring
(the mount, the schema filter, the deprecation hook, the pagination and error building blocks) that
the later phases reuse.
Decisions applied here come from
0025,
0026,
0027,
0028,
0029,
0030,
0031,
0032,
0033,
0034,
0036,
0037,
0038, OEP-66 and OEP-69.
Why a new version rather than an in-place fix. Every one of these seven operations changes
contract: the error body moves from
{developer_message, field_errors}to the RFC 9457 envelope,the time-zone list grows a pagination envelope where it returned a bare unbounded array, the stored
preference row stops embedding the owner's entire record, the member addresses stop using database
ids, and the gate changes from the deprecated
X-Edx-Api-Keyheader to an authenticated staffcaller. A client parsing any of those would break.
/api/user/v1/is untouched and keeps servingevery one of them for its deprecation window.
Who has to do something. Only operators calling the four API-key-gated directory endpoints
server to server:
/api/user/v2/does not acceptX-Edx-Api-Keyand expects a JWT or session for astaff user instead. v1 continues to accept the key. No front end and no mobile app calls any of
these seven addresses (checked by grepping the master tarballs of
frontend-platform,frontend-app-account,frontend-app-profile,frontend-app-authnandopenedx-app-android;openedx-app-iosandopenedx-platform-sdkwere not readable and are named as unchecked).Two of the six gates are red and are reconciled rather than argued away — see Gate report.
Neither is described anywhere in this body as passing.
Reference detail
Three reference sections live in the first comment below, because the complete body exceeds GitHub's 65,536-character limit for a pull request description:
Nothing was cut; the split is purely a length limit.
Change table
feat: add /api/user/v2/ users, user_preferences and time_zonesGET /api/user/v2/users/←GET /api/user/v1/users/id,url,email,name,username,preferences), same pagination envelopeusername/email/is_activefilters;orderingonusernameanddate_joinedwith a deterministic default;?view=minimaland?fields=; RFC 9457 errors; staff authGET /api/user/v2/users/{username}/←GET /api/user/v1/users/{id}/GET /api/user/v2/users/?preference_key=←GET /api/user/v1/preferences/{pref_key}/users/userscollectionGET /api/user/v2/users/?forum_role=&course_key=←GET /api/user/v1/forum_roles/{name}/users/course_keyis a 400 envelope where v1 letInvalidKeyErrorescape and answered 500; a missing one keeps the 400 it already had at v1, with the body moving from{"detail": "course_id must be specified"}to the envelope'serrors.course_keyGET /api/user/v2/user_preferences/←GET /api/user/v1/user_prefs/key/value, same pagination envelopeusernameinstead of the embedded user object, and ausermember that is null unless?view=fullrestores the embedding);key/usernamefilters, and the legacy?user=<id>filter refused with a 400 rather than ignored; constant query countGET /api/user/v2/user_preferences/{username},{key}/←GET /api/user/v1/user_prefs/{id}/key/valueGET /api/user/v2/time_zones/←GET /api/user/v1/preferences/time_zones/country_codecountry_codeis a 400 envelopefeat: publish the user API in the LMS schemaSCHEMA_PATH_PREFIXcovers both areas; title becomes "LMS API"; the seven superseded addresses are markeddeprecated: true; user operations are grouped under ausertag wherever the generator would otherwise group them by version number, so the two areas never share a tag group. Every operationId and every tag the service-wide/api-docsdocument already published is unchanged, measured document against documentADRs applied
serializer_classon all three viewsets,help_texton every field, method fields typed, query parameters validated by a serializer where the data path is not a querysetpermission_classeson every view;(IsAuthenticated, IsAdminUser)on the directory,(AllowAny,)declared deliberately on the public reference list; the deprecatedApiKeyHeaderPermissionis not carried into v2@extend_schemaper operation with summary, description, parameters and a response per status code; the schema filter extended so the mount publishesReadOnlyModelViewSetfor the two ORM resources, plainViewSetfor the service-backed one, all three registered on a DRF router, with prefetching matched to what the serializers read. One of the six generated routes is replaced by a hand-declared one, because a router emits a single lookup parameter and the preference member is identified by the{username},{key}pair; the matrix row below carries the detailStandardizedErrorMixinfirst in the MRO, DRF exceptions only, no internal text in any responseget_or_createon the forum-role listing is droppeduserscollection with filtersFilterSet+OrderingFilteron the ORM lists, serializer-validated parameters on the non-ORM list, OEP-68 parameter namesauthentication_classesline anywhere in the diff?view=minimalon the directory,?view=fullon the preference rows,?fields=on both; the time-zone row has two scalar fields and needs neitherv2/package, no superseded file is in the diff, and the superseded addresses are flagged in the schema — but they carry no docstring marker, which decision 4 also asks for. The reason (FC-0118 review, OEP-21 timing) and the follow-up form are in the matrix row and in Deprecationsurlsgate is red on the version-level JSON-404 fallback, which is not an addressScopedQuerysetMixinwithFullScopePolicy()on both ORM lists, with permission, scoping and filtering as three separate layerstags=["openedx-platform-sdk"], and SDK tagging starts at phase B. The learner-facing account and preference endpoints are the SDK candidates; an admin-only directory is not, and tagging it would put it in a generated client for no callerCompliance matrix
UserDirectorySerializer,UserPreferenceRecordSerializer,TimeZoneSerializerdeclared on their views;help_texton every field;SerializerMethodFields annotated withextend_schema_fieldso the published types are real;TimeZoneQuerySerializervalidatescountry_code. Component names checked against the base schema —UserPreferenceandCountryTimeZonewere already taken by the v1 serializers, so the new ones areUserPreferenceRecordandTimeZonev2/tests/test_schema.py::test_user_list_publishes_its_filters,::test_pagination_envelope_is_published_in_full;v2/tests/test_views.py::test_row_shape_matches_the_declared_fields(IsAuthenticated, IsActiveUser, IsAdminUser)onUserViewSetandUserPreferenceViewSet;(AllowAny,)declared explicitly onCountryTimeZoneViewSetbecause the platform sets no default permission and the list is public on purpose.IsActiveUseris a four-line class inv2/permissions.py, written because neither DRF's permissions nor edx-drf-extensions 10.9.0 ship one that readsis_active, and a token is accepted until it expires without the account being re-read: without it a staff account deactivated after its token was issued would keep reading the directory. No inline permission logic in any handlerDeactivatedStaffCallerTests(7 tests: both halves of the default authentication pair, every staff-gated address, and the still-active account admitted)summary,description, its parameters and one response per status code;tags=["user"]; the LMS pre-processing filter widened so/api/user/v2/and the seven addresses it supersedes reach the published document.LmsAutoSchemaputs every user operation in theusertag group of the document that trims the area off the path, where the derived tag would otherwise be the version number; it renames nothing. The service-wide/api-docsdocument, which trims nothing, keeps the operationId and the tag it derived for each of these addresses before this changev2/tests/test_schema.py(23 tests);lms/lib/tests/test_spectacular.py(12 tests, includingPublishedDocumentTestover the generated document andUserOperationIdTest);schema_diff.py compare→PASS (0 BREAKING, 18 WARN, 12 INFO), read with the warning note under Gate reportReadOnlyModelViewSet,get_country_time_zones()→ plainViewSet. All three are registered onSnakeCaseRouter, a four-lineSimpleRoutersubclass that generates<basename>_<action>names so the published names are the onesreverse()andresolve()use. One deviation, deliberate: the preference member address is declared by hand withre_pathand the router's own member route for that viewset is dropped by name (without_route(), which raisesImproperlyConfiguredat import when the router generates no route under that name, so a renamedbasenamecannot quietly restore the database-id address), because a DRF router generates exactly one lookup parameter per registered viewset and this resource is identified by two —{username},{key}— with the key free-form enough to contain a comma itself. The hand-declared route serves the same viewset'sretrieveaction, so the resource is one viewset with one set of permissions, and no address keyed by a database id is served at all.prefetch_related("preferences").select_related("profile")kept on the directory;select_related("user", "user__profile")added to the preference list, withprefetch_related("user__preferences")only when the owner is embeddedv2/tests/test_urls.py::test_addresses_resolve_to_the_version_two_viewsets(each address against the viewset and action it routes to),::test_preference_member_address_splits_into_username_and_key,::test_a_key_containing_a_comma_belongs_to_the_keyand::test_no_database_id_member_address_is_servedfor the hand-declared route and the dropped one,::test_the_router_route_that_is_replaced_has_to_existfor the replacement being refused when it matches nothing;v2/tests/test_query_counts.py(22 tests: the five database-backed lists and the two member addresses over 1/5/25 rows, and the time-zone list over page sizes 1/10/100, since its rows are not database rows), of which::test_the_earlier_preference_list_pays_per_row_where_this_one_does_notis the before-and-after: the same rows read through both versions in one test, 15 / 35 / 135 queries at version 1 for 1 / 5 / 25 owners against 15, constant, hereStandardizedErrorMixinfirst in the MRO on all three viewsets; validation errors raised with constant messages;Http404from the generic member lookup translated toNotFoundso the envelope is typednot-foundand notinternal; a refused method and an unanswerableAcceptheader named through two subclasses this version registers for itself, leaving every other API's classification untouched; no exception text or upstream string reaches a responseassert_error_envelope(...)call inv2/tests/test_views.py, includingRefusedRequestTests(5 tests: the 405 and 406 envelopes, and another API's 405 asserted unchanged);v2/tests/test_parity.py::test_unauthenticated_bodies_differ_as_declaredand::test_forbidden_bodies_differ_as_declaredrecord the legacy body beside the new oneRole.objects.get_or_createon every GET; the v2 filter does not, so an unknown role returns an empty page instead of creating oneCaptureQueriesContextcaptures — GET and HEAD on each of the five read addresses, plus the forum-role filter and the?view=fullembedding — asserting no INSERT/UPDATE/DELETE, plus::test_unknown_forum_role_returns_no_rows_and_creates_nothing, which also asserts theRolerow count. Hygiene H8 is a grep overget/list/retrievebodies and cannot see a write in a service module, so a green hygiene gate is not evidence for this row — the SQL captures areusers,preferences/{key}/usersandforum_roles/{name}/usersare one resource addressed three ways, so they merge onto/api/user/v2/users/with filters. No other merge was taken: the partner-report and retirement groups are different resources and belong to later phasesv2/tests/test_parity.py::test_preference_key_filter_parity,::test_forum_role_filter_parityUserApiPagination, aDefaultPaginationsubclass that overrides onlyget_paginated_response_schema, because the library publishes four of the seven members it returns. The time-zone list paginates throughIterablePaginationMixinwhere it was previously unbounded (433 rows)v2/tests/test_schema.py::test_pagination_envelope_is_published_in_fulland::test_published_pagination_envelope_matches_the_response_body, which compares the published component against a real response body;v2/tests/test_views.pyasserts the seven members and the page positions;::PageSizeCapTestsasserts that?page_size=1000returns one maximum page of 100 on all three lists, measured against 104, 102 and 433 live rows, so a latermax_page_sizeoverride cannot unbound the anonymous time-zone list unnoticedfilter_backends = (DjangoFilterBackend, OrderingFilter)plus aFilterSeton both ORM lists,ordering_fieldsand a deterministic defaultordering(the legacy lists have noORDER BYat all). The time-zone list is not a queryset, so its one parameter is serializer-validated.course_keyis the OEP-68 spelling and owes no alias: the legacy listing carried the role and the course in the path, so no query parameter was renamed. The preference list is the one place a legacy query parameter is dropped —/api/user/v1/user_prefs/?user=<id>— and it is dropped rather than aliased: this list names owners by username, and?user=is refused with a 400 naming the parameters it does read, so the client that ported by changing the version segment is told rather than handed every user's rows. No parameter of this version is deprecated. Each view's accepted names are assembled from its own declarations (filter set, sort backend, paginator, query serializer), so a filter that is added or renamed cannot fall out of step with the check. The publishedcourse_keyexample is percent-encoded, because an unencoded+arrives as a space and is refusedv2/tests/test_views.pyfilter and ordering tests (14, including the three that pinis_active: each side narrows the page, an empty value narrows nothing, and?is_active=maybeis a 400 envelope rather than a silently unfiltered 200);v2/tests/test_schema.py::test_user_list_publishes_its_filters, which also assertsis_activeis still published as a boolean, and::test_preference_list_publishes_its_filters, which pins the published parameter set and that none of them is deprecated;v2/tests/test_views.py::UnknownQueryParameterTests(16 tests: each of the five addresses refuses a name it does not read, names every unknown one at once, and answers every name it does read),::test_the_owner_id_the_earlier_list_took_is_refused_rather_than_ignored,::test_an_answered_page_announces_no_deprecated_parameter,::test_the_published_course_key_example_can_be_pasted_into_a_query_stringand::test_a_course_key_whose_plus_signs_are_not_encoded_is_refusedauthentication_classes; all use the platform default JWT + session pair. The legacy views'ApiKeyHeaderPermissionis a permission class, not an authenticator, and is not carried forward. The two halves of that pair disagree about a deactivated account: the session half refuses one, and a token issued for one is still accepted, because only the session half reads the flag. That disagreement is closed above the authenticators rather than inside them, by theIsActiveUserpermission both staff-gated viewsets declare, so such an account is refused on both paths — 401authnthrough its session, 403authzthrough its token. The operations also publish nosecurityrequirement and the document carries nosecuritySchemes, because the generator has no authentication extension for the platform's default classes; the same is true of all 25 enrollment operations in this document, and giving them one is a platform change rather than a change to this packagev2/tests/test_views.pyexercise both default authenticators;::DeactivatedStaffCallerTests(7 tests) pins each half's answer to a deactivated staff account, at every staff-gated address?view=minimalreduces it to(id, username, email); the legacy preference row embeds the owner's entire record including that owner's preferences, so v2 returnsusernameby default and?view=fullrestores the embedding;?fields=projects bothv2/tests/test_views.py::test_minimal_view_returns_only_identifying_fields(×2),::test_full_view_embeds_the_owning_user(×2),::test_fields_parameter_*(×3);v2/tests/test_parity.py::test_full_preset_reproduces_the_legacy_embedded_user;::test_an_unknown_response_preset_is_refusedon all four shaped addresses andUserListViewTests::test_the_preset_that_belongs_to_the_preference_list_is_refused_here, with::test_an_empty_response_preset_returns_the_default_shapepinning the empty valueopenedx/core/djangoapps/user_api/v2/. No superseded module is in the diff — in particular none carries a deprecation notice, awarnings.warnor a comment. ADR 0037's decision 4 asks for the superseded version to be marked "in the OpenAPI schema and in the endpoint's docstring"; the schema flag is set and the docstring marker is not, because the FC-0118 reviewer asked for the depr notes to be dropped from the replacement PRs (#39100, #39102) and OEP-21's own marker is a one-line issue link added once the DEPR ticket reaches Plan Accepted, not a notice. If a reviewer wants a marker, the answer is that one line, in a follow-upcheck_old_versions_untouched.shwith 14--protectpaths → PASS, with the superseded modules absent from the diff — read with theallowed: 0caveat under Backward compatibility;v2/tests/test_schema.py::test_only_superseded_addresses_are_marked_deprecatedpins the schema flag on exactly the seven addressespath('api/user/v2/', ...)with the full prefix; trailing slash on every address; namesuser_list,user_detail,user_preference_list,user_preference_detail,time_zone_list— snake_case, version-free, unique; member addresses use the username, and the preference member uses the{username},{key}pair rather than a row id. The namespace isuser_v2— the domain and the version, never the Django app that implements it — so phases B to E extend one namespace for/api/user/. Two collection names were weighed rather than inherited:user_preferences/repeats the api_name, which the stutter clause would otherwise rule out, and is kept because phase C publishes the caller's own preference document ataccounts/{username}/preferences/and a barepreferences/here would read as that resource rather than as the cross-user index of stored rows;time_zones/stays under this api_name because the list exists to populate thetime_zonepreference, and an area that needs the same reference data links to this canonical address instead of publishing a second one. Both namings are open questions rather than settled ones — Open decisions, item 2v2/tests/test_urls.py(24 tests: reverse literals, resolution withrouted_actions(), v1 untouched, no database-id address, JSON refusals, andRouteShapeTests, which pins that every pattern in the URLconf but the trailing fallback is named and slash-terminated);v2/tests/test_url_mount.py(4 tests: the two authn addresses under the same prefix still reachuser_authn);url_snapshot.py check→ FAIL (2 FAIL, 1 WARN), every finding on the fallback pattern and none on an address, reconciled under Gate reportScopedQuerysetMixinwithFullScopePolicy(): a documented pass-through, because the permission classes already restrict the endpoint to platform administrators, who may see every row. The three layers stay separate — permission classes, then the policy, thenfilter_querysetv2/tests/test_urls.pyIn-place edits
Seven pre-existing files are in the diff. Plan §8 originally proposed three (
lms/urls.py,lms/lib/spectacular.py,lms/envs/common.py); a repair pass added the fourth after ruling on it,and the last three were found during implementation, not predicted by the plan. Each is named here
with its own justification and its sanctioned ADR 0037 category, because the PR — not the versions
gate, which never consults
--allowfor any of them — is what has to justify them.lms/urls.pypath('api/user/v2/', include('openedx.core.djangoapps.user_api.v2.urls'))), placed before theapi/user/include so resolution does not depend onuser_api/urls.pynever growing av2/pattern. Evidence:url_snapshot.py checkreports no address-preservation finding on any base route;test_urls.py::LegacyAddressTestsproves the v1 surface is unaffected;test_url_mount.py(4 tests) proves the two authn addresses under the same prefix still reachuser_authn, and goes red when a catch-all is inserted into the v2 URLconflms/lib/spectacular.pydeprecated: trueflag and the published names must live outside the frozen view modules; this hook is where the LMS schema already does that for enrollment. The pre-processing filter now admits, besides enrollmentvN, (a)/api/user/v2/addresses whose view comes from theuser_api.v2package — not the bare prefix, since older apps are mounted under the same prefix — and (b) the seven exact superseded paths, listed one at a time in a newSUPERSEDED_USER_PATHStuple, never a/api/user/v1/prefix, so an address whose replacement nobody has written cannot be marked.LmsAutoSchemagains a tag rule for/api/user/paths, applied only where the tag the generator derives would be a version number, which leaves the service-wide document's tags exactly as they were; it gains no operationId rule, so no published name changes.COLLIDING_LEGACY_PATHandLEGACY_V2_PREFIX— #39078's enrollment-only_legacymachinery — keep their contents and no parallel hook was added. Evidence: the twelve tests inlms/lib/tests/test_spectacular.py(the filter, the deprecation hook with foreign paths in the fixture, the enrollment_legacysuffix, the user ids and tags, and three over the generated document)lms/envs/common.pyTITLE→LMS API(the document now carries two areas),SCHEMA_PATH_PREFIX→r'/api/(enrollment|user)', which strips the area from both mounts so no enrollment operationId moves.POSTPROCESSING_HOOKSis unchanged in content andpostprocess_schema_enumsis still first, pinned bytest_schema.py::test_enum_post_processing_runs_before_the_deprecation_hook. Evidence: every enrollment operationId, tag and component compared base-to-head and byte-identicallms/lib/tests/test_spectacular.py/api/courses/,/api/user_tours/and/user_api/stay out, and a new test asserts that only the seven listed user addresses are deprecated while/api/user/v1/accounts,/api/user/v1/preferences/{username},/api/user/v2/users/and/user_api/v1/users/are not. Five tests are added for the names the document publishes: two on the schema class (the service-wide document keeps every id and tag the generator derives, spelled out address by address; the area tag replaces a derived version tag and nothing else) and three over the generated document (every operationId unique; the only numeral-suffixed id is the enrollment pair that predates this change; no tag group holds two areas)lms/envs/devstack.pySERVERSfor this document still read drf-spectacular (LMS enrollment schema) after the document was retitled LMS API and grew a second area. It now reads drf-spectacular (LMS API schema), matchinglms/envs/common.py. One line; no setting, no value and no behaviour changed. Consequent on the retitle in the row above, not an independent editlms/envs/production.pySERVERSblock, corrected the same way. One line; nothing executable changedopenedx/core/apidocs.py/api-docsdocument carries its own settings described the narrow document that claimsSPECTACULAR_SETTINGSin LMS as the Enrollment API; after the retitle that sentence is wrong. It now names it the LMS API. One comment line; no code changedNone of these seven files is inside a
vN/package or under one of the fourteen frozen legacyroots, which is why the versions gate counts all seven as
otherand its--allowlist is neverconsulted (see Backward compatibility).
No superseded view module is in the diff. They carry no deprecation notice, no comment and no
warnings.warn, which is why they do not appear here.Error-format decision
Versioned, not changed in place.
frontend-app-accountparses the legacy body directly —src/account-settings/data/utils/serviceUtils.jsreadserror.response.data.field_errorsand eachentry's
user_message. Changing the shape under the live v1 addresses would break every field-levelvalidation message in account settings, so the RFC 9457 envelope ships only on
/api/user/v2/.The two bodies are recorded side by side in
v2/tests/test_parity.py::test_unauthenticated_bodies_differ_as_declaredand::test_forbidden_bodies_differ_as_declared.Error types registered by this PR: two, and both are its own. Every refusal these seven
read-only operations raise themselves is already in the shared catalog (
authn,authz,not-found,validation). Two failures the framework raises are not:MethodNotAllowedandNotAcceptableare absent fromedx-drf-extensions10.9.0's catalog, so a refused method or anAcceptheader the API cannot answer is enveloped astype: …/errors/internal,title: "Internal Server Error"at status 405 or 406 — the caller is told its own bad request was aserver fault, and on the public
/api/user/v2/time_zones/that is reachable anonymously.The catalog is process-global, so this PR does not register DRF's own classes, which would
reclassify those failures for every standardized API in the process. It registers two subclasses of
its own (
v2/errors.py:RefusedMethod,UnsatisfiableAccept) and raises them from its own viewsthrough
CatalogedRequestErrorsMixin, so the two errors are namedmethod-not-allowedandnot-acceptablehere and classified exactly as before everywhere else. That isolation isasserted, not argued:
RefusedRequestTests::test_another_api_is_classified_exactly_as_the_shared_catalog_classifies_itsends a refused method to another standardized API (
POST /api/enrollment/v2/courses/{course_id}/)and asserts it still answers
internal/ "Internal Server Error". The other four tests in that classassert the 405 envelope for an anonymous caller and for the two staff-gated collections and members,
and the 406 envelope for
Accept: application/xml, each with its title, its detail andContent-Type: application/json.The durable fix belongs in the library: the four uncataloged DRF exceptions —
MethodNotAllowed,NotAcceptable,UnsupportedMediaTypeandParseError— belong in_BUILTIN_ERROR_TYPES, wherethey would be named for every standardized API at once. It is recorded under Follow-ups; when it
ships, these two subclasses and the mixin give way to it.
Deprecations
Exactly seven legacy paths are marked by this PR — the seven Phase-A addresses below, and no
others:
GET /api/user/v1/users/GET /api/user/v2/users/GET /api/user/v1/users/{id}/GET /api/user/v2/users/{username}/GET /api/user/v1/user_prefs/GET /api/user/v2/user_preferences/GET /api/user/v1/user_prefs/{id}/GET /api/user/v2/user_preferences/{username},{key}/GET /api/user/v1/preferences/{pref_key}/users/GET /api/user/v2/users/?preference_key={pref_key}GET /api/user/v1/forum_roles/{name}/users/GET /api/user/v2/users/?forum_role={name}&course_key={course_key}GET /api/user/v1/preferences/time_zones/GET /api/user/v2/time_zones/They are marked only by
deprecated: truein the published schema, set by the LMSpost-processing hook (
lms/lib/spectacular.py) from a tuple of those seven exact paths. There isno response header and no deprecation note in any superseded code, docstring or comment — the
superseded modules are not in the diff at all. That is a deliberate departure from ADR 0037's
decision 4, which marks the superseded version in the schema and in the endpoint's docstring: the
FC-0118 reviewer asked for the depr notes to be dropped from the replacement PRs (#39100, #39102),
and OEP-21's marker for a REST API is a single
Deprecated <issue link>line added after the DEPRticket reaches Plan Accepted, not a notice. If a reviewer wants the marker, that one line in a
follow-up is the form it should take.
The
/user_api/v1/…twins of these addresses are not deprecated here: they are called byfrontend-app-account,frontend-app-profileand the Android app, and this PR mounts no successorat those addresses.
No query parameter is deprecated alongside them.
GET /api/user/v1/user_prefs/accepts?user=<id>; its successor names owners by username and publishes no identifier filter, soGET /api/user/v2/user_preferences/?user=<id>is a 400validationenvelope naming the parametersthat list does read. Nothing in this version is published with
deprecated: trueand no responsecarries a
Deprecationheader. The removal note for the seven addresses carries the replacementfilter name, so a caller reading the ticket has the whole move in one place.
Tracked on the FC-0118 umbrella DEPR ticket,
#39128, which removes them one named
release after the release this ships in. This area's entry is posted on #39128 once the PR is
open.
Five behaviour notes the removal must carry:
GET /forum_roles/{name}/users/runsRole.objects.get_or_createon every call today. The v2filter does not create the role; an unknown role returns an empty page. No receiver is attached
to
Role, so nothing else fans out.Where that write still lives, for an operator who relied on reading the list to bring a role
into existence:
seed_permissions_roles(course_key)(
openedx/core/djangoapps/django_comment_common/utils.py:49) creates the five standard roleswith their permissions, and already runs when a course is created or imported in Studio
(
cms/djangoapps/contentstore/utils.py:126,contentstore/management/commands/import.py:82) andwhen a CCX course is set up (
lms/djangoapps/ccx/utils.py:517); the LMS management commandseed_permissions_roles <course_id>runs it for a course that predates that; andassign_role(course_id, user, rolename)(django_comment_common/models.py:62) creates a rolewhen granting it to a user. A role created by the legacy GET carried no permissions and no
members, so none of these is a like-for-like replacement for it — they are what should have
created the role in the first place.
What has to exist before the v1 address can be retired: every course whose roles were
brought into existence by calling it must have them seeded, because the instructor endpoint that
grants a role (
POST …/instructor/api/update_forum_role_membership) reads the role withRole.objects.getand raisesRole.DoesNotExistwhen it is missing.are_permissions_roles_seeded(course_id)reports whether a course is in that state and themanagement command puts it right; the check belongs in the removal ticket, not in this PR, since
the legacy address keeps its behaviour until then.
GET /api/user/v2/time_zones/validatescountry_codeas two letters. A well-formed but unknowncode still falls back to every time zone, exactly as before; a malformed one (
FRA) is now a 400envelope where v1 returned the full list. An empty value (
?country_code=) narrows nothing andreturns every zone, as it does at v1; that branch has a test on both counts.
/api/user/v2/does not acceptX-Edx-Api-Key. Server-to-server callers of the v1 directoryendpoints must issue a staff JWT before moving.
GET /api/user/v1/users/{id}/raise; the v2 entry returnsname: null. A caller moving to v2 gets a row where it used to get a server fault.username, so an integration holding identifiers has a step to take before it can address an
entry:
GET /api/user/v1/accounts?lms_user_id={id}— staff-gated, untouched by this PR anditself standardized in phase B — answers with the account whose
usernamekeys the v2 entry.The v2 directory publishes no identifier filter, and this version refuses a parameter it does
not read, so
GET /api/user/v2/users/?id={id}is a 400validationenvelope namingidratherthan a page that looks narrowed, and the resolution step is not optional.
v2/tests/test_parity.py::test_a_database_identifier_is_refused_instead_of_narrowing_nothingrecords the refusal and
::test_an_identifier_held_by_a_caller_resolves_to_the_username_the_entry_is_keyed_bywalks thetwo calls end to end. This step belongs in this area's entry on the DEPR ticket. Whether a v2
idfilter should exist instead is Open decisions, item 3.Authorization
GET /users/,GET /users/{username}/ApiKeyHeaderPermission— ignoresrequest.userentirely; a validX-Edx-Api-Keyreads every user's email address and full preference mapping(IsAuthenticated, IsActiveUser, IsAdminUser)+FullScopePolicy()GET /user_preferences/,GET /user_preferences/{username},{key}/ApiKeyHeaderPermission(IsAuthenticated, IsActiveUser, IsAdminUser)+FullScopePolicy()GET /users/?preference_key=,?forum_role=&course_key=ApiKeyHeaderPermissionGET /time_zones/(AllowAny,), declared on purposeBoth staff-gated viewsets require the caller's account to be active, not only authenticated and
staff. The platform's two default authenticators disagree about a deactivated account: the session
half stops accepting one, while a token issued before the account was deactivated is still accepted,
because only the session half reads the flag. The requirement is therefore declared where it applies
to both, as the
IsActiveUserpermission class inv2/permissions.py, and such an account isrefused on either path. edx-drf-extensions 10.9.0 ships no equivalent permission; if it gains one,
this class gives way to it.
Deny-path tests, one per refusal:
/users/authnUserListViewTests::test_anonymous_caller_is_unauthenticated/users/authz::test_non_admin_caller_is_forbidden/users/?preference_key=authz::test_non_admin_cannot_list_another_users_preference_holders/users/{username}/UserDetailViewTests(three tests)/users/{username}/not-foundUserDetailViewTests::test_unknown_username_is_not_found/user_preferences/UserPreferenceListViewTests(three tests)/user_preferences/{username},{key}/UserPreferenceDetailViewTests(three tests)/user_preferences/{username},{key}/not-found(×2)::test_unknown_key_is_not_found,::test_key_belonging_to_another_user_is_not_found/time_zones/TimeZoneListViewTests::test_anonymous_caller_receives_the_listcountry_code=FRA/time_zones/validation::test_malformed_country_code_is_rejected/users/,/user_preferences/,/time_zones/not-found::test_a_page_beyond_the_last_one_is_not_found(×3),UserListViewTests::test_a_page_number_that_is_not_a_number_is_not_foundfields=naming a property the response cannot carryvalidation, exact body asserted::test_a_field_name_the_row_does_not_carry_is_refusedand its three siblings,UserListViewTests::test_a_field_the_minimal_preset_drops_is_refused,::test_every_unknown_field_name_is_named_at_onceauthn— the session authenticator stops accepting itDeactivatedStaffCallerTests::test_a_session_held_by_a_deactivated_staff_account_is_refused,::test_a_deactivated_staff_session_is_refused_at_every_staff_gated_addressauthz— the token is still valid and the active-account permission refuses it::test_a_token_held_by_a_deactivated_staff_account_is_refused,::test_a_deactivated_staff_token_is_refused_at_every_staff_gated_address,::test_the_directory_does_not_list_the_account_that_was_refused(no page body reaches the caller)/users/::test_an_active_staff_token_still_reaches_the_directory/users/authz— the permission layer still applies on that pathDeactivatedStaffCallerTests::test_a_token_held_by_a_deactivated_learner_is_still_forbiddenforum_rolewithoutcourse_keyand the reverse/users/validation, exact body assertedcourse_key/users/validation::test_malformed_course_key_is_rejectedcourse_keywhose+is not encoded/users/validation, exact body asserted::test_a_course_key_whose_plus_signs_are_not_encoded_is_refused?user=<id>(the filter the v1 list took)/user_preferences/validation, exact body assertedUserPreferenceListViewTests::test_the_owner_id_the_earlier_list_took_is_refused_rather_than_ignored?lms_user_id=,?expand=, a mistyped filter,?sort=and?limit=together)validation, exact body asserted, every unknown name reported at onceUnknownQueryParameterTests(8 refusal tests)?country=or?ordering=/time_zones/validation— this list readscountry_codeand the page parameters onlyUnknownQueryParameterTests::test_the_time_zone_list_refuses_a_parameter_it_does_not_read,::test_the_time_zone_list_refuses_a_sort_it_does_not_offer/users/,/user_preferences/UnknownQueryParameterTests::test_an_unknown_parameter_does_not_answer_an_anonymous_caller,::test_an_unknown_parameter_does_not_answer_a_non_admin_caller/users/?forum_role=&course_key=UserListViewTests::test_anonymous_caller_cannot_list_a_courses_forum_role_holders,::test_non_admin_cannot_list_a_courses_forum_role_holders,::test_holding_the_role_does_not_let_a_learner_list_its_holders?view=naming a preset the address does not answer to/users/,/users/{username}/,/user_preferences/,/user_preferences/{username},{key}/validation, exact body asserted on the two collections::test_an_unknown_response_preset_is_refused(×4),UserListViewTests::test_the_preset_that_belongs_to_the_preference_list_is_refused_here?view=nonsense/users/authn— the caller is refused before the parameter is read::test_an_unknown_preset_does_not_answer_an_anonymous_callerauthzTokenCallerTests::test_a_learners_token_is_forbidden_on_every_staff_gated_address/user_preferences/…authz— owning the row is not being staffTokenCallerTests::test_the_owner_of_the_record_is_still_not_staffGate report
run_gates.sh, this branch againstorigin/master@ce074d1155, final run, after ten repair passes and the twodecisions taken at review. Six gates ran; none was skipped; two are red.
files in new version dirs: 16 other files: 7 added in old dirs: 0 allowed: 0— all fourteen protected legacy paths unchanged and nothing added inside a pre-existing version directoryWARN H3openedx/core/djangoapps/user_api/v2/paginators.py:6— the localDefaultPaginationsubclass, kept because it adds behaviour (the full envelope schema the library does not publish) and the class docstring says so. The twoWARN H11findings of the earlier run were on theDeprecationwording of the?user=alias and are gone with it. No superseded route carries any such text/api-docsdocument onmaster). The 18 WARN are all new generation warnings: twelve on the legacy v1 user views and serializers the document now admits, six on this PR's own three viewsets — in both casescould not resolve authenticatorfor the platform's default classes, plus two type-hint warnings on the v1 serializer. None is an error, and the effect is named belowopenedx/core/djangoapps/user_api/v2/tests/, 12 inlms/lib/tests/test_spectacular.py, 53 inopenedx/core/djangoapps/enrollments/v2/tests/(the regression test for the schema settings change). This is the new suite only. The legacyopenedx/core/djangoapps/user_api/tests/suite is unchanged against its baseline on this branch: 7 failed, 100 passed, 16 errors, the same counts before and after these changesWhat the four green gates do not prove. Each is green for what it measures, and no more:
pre-existing files in the diff: the
--allowarguments are mechanically inert (allowed: 0), andthe evidence for those seven is the per-file review in In-place edits.
get/list/retrievebodiesand cannot see a write in a service module. The twelve SQL captures are the evidence.
generation warnings are harmless; their concrete effect is the missing
securityblock, namedbelow and under Follow-ups.
19 legacy tests that cannot run in this environment.
The
securityblock, since the schema gate cannot flag it. Six of the 18 warnings are on thisPR's own views: could not resolve authenticator
DefaultJwtAuthentication/DefaultSessionAuthentication; there was noOpenApiAuthenticationExtensionregistered for thatclass. They are the platform's default pair, which ADR 0034 requires and which no view here
overrides; the same warning is emitted for the enrollment views on the base ref. The visible effect
is that the four staff-gated operations publish no
securityrequirement at all(
security: [{}]on the public time-zone list, absent on the other four), so a generated clientcannot see that a JWT or a session is required. Registering the extension is a shared-library
change, recorded under Follow-ups, not something to work around per view.
Why
urlsis red, and the rule that cannot see the difference. The gate rule-checks every routeadded under
api/. Six were added: the five Phase-A addresses and one fallback,re_path(r"^(?!account/).*$", UnknownAddressView.as_view()), which answers every address under/api/user/v2/that no route serves with the same JSON error envelope as the rest of the APIinstead of the service's HTML 404 page. The gate normalises that pattern to the address
api/user/v2/{_}.and reports it three times:FAIL R6missing trailing slashapi/user/v2/{_}.FAIL R8identifier directly under the versionapi/user/v2/{_}.collection/{id}, not{id}WARN R11route has no URL nameapi/user/v2/{_}.reverse()must work for an addressNone of the three applies to the subject: a fallback is not a resource, has no identifier, and is
deliberately unnamed because nothing should ever reverse to it. Six candidate shapes were run
through the checker itself rather than guessed at:
re_path(r"^(?!account/).*$", …)(shipped, nameless)api/user/v2/{_}.R6,R8,R11api/user/v2/{_}.R6,R8— and a name that must never be usedpath("<path:unknown_address>", …)api/user/v2/{unknown_address}R6,R8path("<path:unknown_address>/", …)api/user/v2/{unknown_address}/R8, and slashless addresses lose the JSON 404re_path(r"^.*/$", …)api/user/v2/./R4path("users/<path:unknown_address>/", …)per collectionapi/user/v2/users/{unknown_address}//api/user/v2/not_a_collection/, which is half the behaviourA version-level fallback cannot be spelled as a conforming address, because the thing that makes it
a fallback — matching anything — is exactly what the normaliser reads as an identifier in the
collection slot.
url_snapshot.py checkdoes take an--allowlist, matched against the normalisedpath and not, as is sometimes assumed, the route name (
url_snapshot.py:253), so the route'snamelessness was never the obstacle; with the single entry
api/user/v2/{_}.the run reportsnew api routes checked: 5,PASS (0 FAIL, 0 WARN)(saved asartifacts/urls-fallback-allowlist.txt). That figure is quoted only for the five real addressesand never as the gate result, for two reasons:
run_gates.sh:134passes no allowlist to thechecker, so the composite gate could not go green that way without editing the gate harness; and a
green line produced by exempting the subject of the finding would read as "the rules were met" when
the honest statement is "the rules do not apply here". So the gate stays red, this is reported as a
checker limitation and not a checker changed to suit the diff, and the fallback's shape is pinned in
the suite instead — by
v2/tests/test_urls.py::RouteShapeTests(every other pattern in the URLconfis named and slash-terminated; the fallback is the single unnamed one and is matched last) together
with
MalformedAddressTests, which asserts the JSON 404 — content type, no<html, and the errorenvelope — for an unknown collection, an unknown member, a malformed username and a write, anonymous
and authenticated.
Why
old-testsis red, and what that does and does not show. The gate runsopenedx/core/djangoapps/user_api/tests/and reports7 failed, 100 passed, 16 errors(381.78s).The same command on
origin/master@ce074d1155, in a clean worktree with the same image, the sameinterpreter and the same settings, reports
7 failed, 100 passed, 16 errors(381.02s). The twoFAILED/ERROR sets were compared, not just the counts: 23 lines over 19 distinct tests on each side
(four tests report both a FAILED and an ERROR line, the ERROR coming from teardown), and the
difference between the sets is empty. Every failure and error is a pymongo
ServerSelectionTimeoutError— the test container has no modulestore, soUserPreferenceModelTestand
UpdateEmailOptInTestCasecannot run in it at all.masterfails on this branch.masteras much as on this branch. CI, which has Mongo, is the only place that can answer it.100 passing legacy tests are a floor, not legacy coverage; Backward compatibility lists the
nine suites (~420 tests) that CI will be the first to run on this branch.
Open decisions — not settled by this PR
Four things on this branch are decisions rather than findings. Each is implemented one way, tested
and reversible, and each is cheap to change now and expensive later. None should be read as closed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>, and the repair-pass work still to becommitted will carry whatever is decided. The convention is that the trailer is the author's
call, never a default: keeping it needs no action, dropping it is a rebase of two commits. This
PR body carries no attribution line either way.
user_preferences/repeats the api_name (the stutter clause), kept because phase C will publish the caller's own
preference document at
accounts/{username}/preferences/and a barepreferences/here wouldread as that resource.
time_zones/stays under theuserapi_name because the list exists topopulate the
time_zonepreference and one canonical address beats two. Both reasons arerecorded in the 0038 matrix row. This is the last moment either can change for free: once the
addresses are published, a rename is a second address.
holding database ids resolves them with
GET /api/user/v1/accounts?lms_user_id={id}(staff-gated,standardized in phase B) and then addresses
users/{username}/; the v2 directory publishes noidfilter, and since this version refuses a parameter it does not read,?id=is a 400 namingthe parameter rather than a page that looks narrowed. Two tests pin both halves, and Deprecations
note 5 carries the step. The alternative — an
idfilter on the v2 directory — is about fourlines plus a published parameter plus tests, and it would reintroduce a database identifier into
the v2 contract.
urlsgate reconciliation reads honestly enough to ship. The gate is red onthree findings, all on the JSON-404 fallback, and the argument above is that the rules do not
apply to a route that addresses nothing. The alternatives were measured and each costs behaviour
or honesty: making it conforming drops the "unknown collection" case; allowlisting it needs a
change to
run_gates.shand produces a green line that would misrepresent what was checked. If areviewer would rather have a green gate than this paragraph, the honest green requires teaching
url_snapshot.pythe difference between an address and a fallback — a change to the checker, inits own PR.
Three decisions taken at review and now settled in the body above, recorded here so a reader of an
earlier draft can find them: the deactivated-account gap is closed on the v2 endpoints by the
IsActiveUserpermission (Authorization, and the 0026 and 0034 matrix rows); the?user=<id>alias and its
Deprecationheader are removed, and an unrecognised query parameter is now a400 instead (What else a caller or a client generator sees, and the 0033 matrix row).