Skip to content

Latest commit

 

History

History
148 lines (113 loc) · 6.48 KB

0003-permissions-via-bridgekeeper.rst

File metadata and controls

148 lines (113 loc) · 6.48 KB
Raw

0003. Use bridgekeeper for Permissions and Tracks

Context

In 0002. Use django-rules for Permissions and Tracks, the case was laid out for using django-rules to manage permissions. In the intervening time, no active work has been done to complete the conversion. During that period, authorization checking was observed to be a significant performance in several list views of courses (the CourseListView from course_api and the student dashboard view). In both cases, access checking had to be performed after the CourseOverview had been loaded from the database, and resulted in significant numbers of additional queries.

bridgekeeper is a library with an interface that is very similar to django-rules, except that it also supports specifying the permissions as Django query filters. By inspection, it seems to support non-boolean return values for the Python-based permissions checks.

Decision

Rather than use django-rules in edx-platform, we will convert permissions checks to use bridgekeeper. The conversion will follow a similar outline to the plan in 0002.

We will update all uses of has_access, and all CourseMode membership checks, to use bridgekeeper and named permissions.

Plan of Action

  1. For each caller of has_access:
    1. Convert the caller to use user.has_perm instead.
    2. Implement the new permission created in 1. by referencing to the previous has_access call. These bridgekeeper rules would implement query to raise an exception when called, and implement check to pass through to has_access.
  2. Refactor contents of has_access out into their own predicates that can be used to implement specific permissions. Where possible, implement query, either manually by specifying a custom rule, or by using Attribute or Relation from bridgekeeper.
  3. For any listing views that are currently checking access as a filter after querying the database, improve their performance by using the bridgekeeper permission to filter the queryset instead. If those views need to display the existence (but not contents) of the object, two separate permissions can be used: the first to limit the query to only those objects to be displayed, and the second to check whether the user has permissions to view the contents of the object.
  4. For each place that checks if a user is enrolled in a specific track:
    1. Convert the that check to use user.has_perm for a named permission
    2. Implement that permission by checking

Details

Convert callers of has_access to use user.has_perm

Currently, the LMS uses has_access to check if a given user has a particular role on a particular object (usually a course or an xblock). From that, it assumes various permissions. The primary goal of this project is to convert those implicit permissions into explicit named permissions that are tied to roles by the use of various predicates.

To bootstrap this process, we can wrap has_access in named permissions by:

  1. Convert each caller of has_access to use user.has_perm instead.
  2. Implement the new permission created in 1. by referencing to the previous has_access call.

This work can be done incrementally, one call to has_access at a time, and can be parallelized. However, at present, there are ~150 calls to has_access in edx-platform, so this is not an insignificant amount of work.

Refactor contents of has_access

As implemented, has_access has many subclauses to handle the various roles and object types. With bridgekeeper, those clauses could be converted to smaller individual Rules, either divided by roles, object types, or both. These predicates would then be simpler to test and to use in determining future permissions.

Convert track membership tests to permissions

Future work in the same vein would be to convert current usage of track membership into user.has_perm checks. This would allow disaggregation of edx-platform features and would make it easier to add new tracks with variations of those features.

Offramps

The primary offramp would be suspending the project after converting all callers of has_access to use user.has_perm. If we have more time, then refactoring has_access would be a definite positive, but not required. If we are forced to cut scope, then only partially completing the conversion of has_access would be an improvement, perhaps with the addition of deprecation warnings for direct callers to has_access so that we can track the remaining work with INCR tickets.

Status

Proposed

Consequences

When the conversion of has_access has been completed, it will be easier to add additional conditions to various permissions checks on specific objects. It will also allow those conditions (predicates) to be written in a location that is central to the app they are responsible for, rather than requiring that they be added to access.py.

When the conversion of CourseMode membership checks has been completed, it will be easier to add new CourseMode types with similar permissions schema to the codebase. It will also open the way towards making CourseMode permissions be data-driven, rather than being code specific, which would allow configuration-time specification of CourseMode, rather than requiring the current combination of code and database entries.

Converting all of these checks to use bridgekeeper will allow list-view query performance to be optimized.