Pluggable access control both viewing and enrolling in a course

Summary

I’m proposing a method to customize the access control in Open edX via plugins. This proposal is concerned about augmenting the course load and enroll permissions by providing django app and registering it using the Django settings.

Access Control in Open edX

Enrollment access control is pretty much hard-coded in Open edX. The course view and enrollment are not an exception.

Often times, when the access control logic need to be changed, the modifications are also hardcoded
which adds to the tech-debt that fork maintainers needs to carry on.

One example is the ability to make a Course Access Group feature which is something that
a couple of Open edX providers have expressed interest in and also being requested by our customers.

Pluggable Access Control

I’m trying to collect some architecture feedback before going and making such change in Open edX,
but mostly I’m interested in stealing from the LTI Consumer XBlock extensions architecture.

You can also check original pull request and how an LTI Plugin looks like.

If you’re really into it check other related work in Open edX:

Proposal

There’s a couple of ways to design the extension, I’m going for the most simple one that can work focusing on the _has_access_course function.

Hypothetically, if we’d like to make the enrollment in the course-v1:Uni+Demo+2020 course
to be exclusive for users with@example.edu email we’d use the following configuration:

# my-configs/server-vars.yml
ACCESS_CONTROL_BACKENDS:
    course_enroll:
      NAME: 'my_access_plugins.backends.domain_based_course_enrollment'
      OPTIONS:
        enforce_domain_name_check: true
    course_load:
      NAME: 'my_access_plugins.backends.domain_based_course_enrollment'

And the backend would look like this:

# my_access_plugins/backends.py
def domain_based_course_enrollment(user, courselike, default_access, options):
    if courselike.id == 'course-v1:Uni+Demo+2020' and not user.email.endswith('@example.edu'):
        return False

    if options.get('enforce_domain_name_check', False):
       # Check the DNS records for such email.
       pass

    return default_access

Some Implementation Example

While the actual implementation will be different, here’s an approximate idea of what would it look like:

The settings would follow the same patten like other Open edX settings.

# lms/envs/aws.py
ACCESS_CONTROL_BACKENDS = ENV_TOKENS.get('ACCESS_CONTROL_BACKENDS', {})

We also need a helper to handle the common logic for invoking the backends:

# somewhere/helpers.py
def use_acl_backend(backend_name, user, resource, default_access):
    # Not doing too much error handling at the moment
    backend_path = ACCESS_CONTROL_BACKENDS.get(backend_name).get('NAME')
    backend_options = ACCESS_CONTROL_BACKENDS.get(backend_name).get('OPTIONS', {})

    if backend_path:
        backend = import_module(module_path)
        return backend(user, resource, default_access=default_access, options=backend_options)

    return default_access

This would be the actual use of the backends, it needs to put it in other places in the platform.

# lms/djangoapps/courseware/access.py
def can_enroll(user, course):
    # Over-simplified function, since the actual `access.py` is pretty complex.
    # ...
    can_enroll = through_some_other_logic()
    # ...
    #
    return use_acl_backend('course_enroll', user, resource=course, default_access=can_enroll)

Let me know if you’d like me to expand with more details.

Questions

  • Tell me what do you think?
  • How can we get this into Open edX?
  • Do you see any issues with the proposal above?
  • Any better alternative do you have in mind?
  • Is this change has enough impact to require an OEP?

@omar Would you please provide example code that uses this configuration?

I understand your point, but can you please describe instead how the setting would affect the envs/*.py files instead of server-vars.yml? I feel like we are closer to the bone when talking about *.py files, while the server-vars.yml file needs to be interepreted by multiple layers of ansible playbooks.

What’s the point of the action argument? If I understand correctly, in this case action == "enroll", right?

This goes in the right direction. It’s funny, as I am currently working on Django authentication and what you are proposing seems to be similar to the AUTHENTICATION_BACKENDS setting.
Maybe you should plan ahead and expect that access control backends will require optional settings, just like the authentication backends? In that case, you would have to write:

ACCESS_CONTROL_BACKENDS = {
    "enroll": [
        {"NAME": "my_access_plugins.backends.course_enroll", "OPTIONS": {...}}
    ]
}

You know the answer to that one, right? :wink:

No

I certainly hope not.

For performance reasons, I’ve been trying to push access checks over into Bridgekeeper. The rationale for that is described in an ADR.

Warning
Bridgekeeper (and these docs!) are a work in progress.

:thinking:

@john Thank you! Examples have been added, let me know if I should expand more to clarify the idea.

@regis Thank you, great notes. I’ve updated the post to the following:

  • More code examples.
  • I added the OPTIONS as you suggested. But kept it to a single backend per “action”, instead of having multiple ones. I’d like to keep it simple unless there’s a compelling use case to do otherwise.
  • The action parameter is removed.

Thanks for sharing the decision @cpennington . That’s a major change to the existing access control. I have a couple of questions so I can decided how to steer this proposal:

  • Is this decision final, and the edX architects have committed to do?
  • Even if Bridgekeeper is an unmaintained tool that’s still officially “Work in Progress” (i.e. Regis concern)?
  • What’s a reasonable timeline for such change to be completed? Any kind of estimation or target date would be good.
  • Would this change be backward incompatible, or we’d like to keep the old method and allow switching between?
  • Assuming that it would be done sometime soon, how do you think extensibility can be built into Bridgekeeper? A starting point for me to dig in this subject would be awesome.

It hasn’t come up as an OEP, so right now it’s a direction that I’d like to move edx-platform in, and I see benefits to adoption in other services. It’s certainly possible that the library we use could shift, but I think the key piece of allowing composable database filters to implement the all (or the majority) of access control is important, and is something that we should commit to.

The library itself is relatively small and self-contained, so I’m not super worried about either needing to do work on it ourselves (I’ve already PRed a small fix during my initial prototyping), or replacing with a more widely supported library if one comes along.

That’s an excellent question. Right now, we don’t have the whole project (converting all existing access control in edx-platform and/or other services) on a roadmap over here at edX. There are a some INCR tickets that I created for naming/migrating existing permissions (see INCR-560 for details).

There are actually two things that need to change. First, we need to convert the places in edx-platform where we are implicitly checking a permission (by checking what group a user is in, or whether they are staff on a course) to use the standard django user.has_perm syntax. Those named permissions could then be implemented by wrapping the existing logic into a form compatible w/ either Bridgekeeper or django-rules (django-rules, of course, not having the capability for query-filtering). This step is both backwards compatible, and makes future work of changing how individual permissions are implemented easier.

The second step is to convert the existing has_access implementations to use a composable permissions framework (Bridgekeeper, for instance), so that we can take advantages of the performance improvements from those. As that work is completed, existing code that calls has_access won’t function the same (it wouldn’t be able to check the converted permissions.

I think it would be pretty easy to set up your proposed extensibility points to work w/ Bridgekeeper. The standard setup for that is to add your Rule objects to a dictionary of perms (like in https://github.com/edx/edx-platform/blob/master/lms/djangoapps/ccx/permissions.py). I could imagine your extension point either directly adding Rules to perms, or being set up to augment whatever existing Rule is assigned to the permission by the core platform.

Hope that all helps!

1 Like

Thanks @cpennington for the detailed explanation. It really helps. I’ll do some homework and see how can I make something that works for now (Hawthorn/Ironwood) while is has some prep for the future Bridgekeeper changes.

1 Like