[FC-0118] docs: extend OEP-66 with a queryset-scoping pattern for list endpoints - #802
Conversation
Adds a Django REST Framework subsection to OEP-66 covering the separation of three authorization concerns in list endpoints: endpoint access (permission_classes), record visibility (an RLS policy in get_queryset), and user-driven filtering (django-filter). Record visibility delegates to openedx-authz via get_scopes_for_subject_and_permission rather than per-row enforcement, and documents the list vs. single-object access paths and the modulestore fallback. Supersedes the standalone ADR proposed in openedx/openedx-platform#38354, moving the architectural pattern into OEP-66 per review feedback.
Abdul-Muqadim-Arbisoft
changed the title
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
I think this way of structuring things makes a lot of sense, and the explanation here is clear.
I just have some concerns about the term "RLS" and am wondering if we can update the example to use the newer DB table.
|
|
||
| Keeping these separate makes access behavior consistent across endpoints, | ||
| auditable in one place per model, and testable in isolation. The record | ||
| visibility layer is the row-level security (RLS) concern; the rest of this |
There was a problem hiding this comment.
This is a bit of a nit, but I don't think this is really "row-level security (RLS)"?
As I understand, RLS is when you configure policies within the database system like MySQL or PostgreSQL, so that it enforces detailed row-level permissions automatically.
tables can have row security policies that restrict, on a per-user basis, which rows can be returned by normal queries or inserted, updated, or deleted by data modification commands. This feature is also known as Row-Level Security - Postgres docs
(Supabase also has great docs on RLS since they encourage use of it in Supabase projects.)
Applying a "Record visibility" filter to get_queryset() is affecting rows in the query, but it's not what people normally think of as RLS, because it's only at the application layer and even there it can be easily bypassed (e.g., by querying CourseOverview.objects.all() directly instead of using the get_queryset queryset).
I would call this an "Authorization-filtered queryset" or "queryset scoping" but not RLS. RLS is a much higher level of security, but not something we are able to implement at the moment given that we don't configure such policies nor use different user accounts at the database level.
There was a problem hiding this comment.
You're right, thanks for the precise definition and references. This is application-level queryset scoping, not database-enforced RLS and as the Postgres/Supabase docs make clear.by the way I've renamed it throughout to "queryset scoping" / "authorization-filtered queryset" (ScopingPolicy, ScopedQuerysetMixin, scoping_policy) and added a note explicitly drawing that distinction, and ofcourse with a link to the PostgreSQL RLS docs so the difference is unambiguous.
There was a problem hiding this comment.
Updated pr title and description as well
| permission_classes = [IsAuthenticated] # endpoint access | ||
| rls_policy = CourseRLSPolicy() # record visibility | ||
| filterset_class = CourseFilterSet # user-driven filtering | ||
| queryset = CourseOverview.objects.all() |
There was a problem hiding this comment.
This is very optional, but:
We're trying to encourage use of the new CourseRun table instead of CourseOverview. CourseRun uses integer primary keys and loads much less data on each query (it has way fewer columns), so it should be way more efficient for most use cases. It would be awesome if you could update the examples here to use CourseRun instead of CourseOverview to help raise awareness of CourseRun being the better option moving forward.
There was a problem hiding this comment.
Completely Agree, thank you for bringing this up ,i just switched the examples to CourseRun.
…amples Addresses review on PR openedx#802: - Rename "row-level security (RLS)" to "queryset scoping" throughout. The pattern is application-layer queryset filtering, not database-enforced RLS, and is bypassable via direct ORM access. Rename the ScopingPolicy/ScopedQuerysetMixin/scoping_policy abstractions, add a note contrasting the two, and link the PostgreSQL RLS docs. - Switch examples from CourseOverview to CourseRun. CourseRun has an internal integer pk, so the single-course scope matches on course_key, and the org scope joins through catalog_course__org__short_name since CourseRun has no org column.
Abdul-Muqadim-Arbisoft
changed the title
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks for those tweaks. LGTM.
3 participants
Adds a Django REST Framework subsection to OEP-66 covering the separation of three authorization concerns in list endpoints: endpoint access(permission_classes), record visibility (a scoping policy applied in get_queryset), and user-driven filtering (django-filter).
Record visibility delegates to openedx-authz via get_scopes_for_subject_and_permission — a single bulk scope lookup translated into a queryset filter, rather than a per-row enforce — and the section documents the list vs. single-object access paths and the modulestore fallback.
Supersedes the standalone ADR proposed in openedx/openedx-platform#38354, moving the architectural pattern into OEP-66 per review feedback.