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

Just a quick note to say that (inspired by the edX ADR) we’ve been using bridgekeeper for the LabXchange project and are pretty happy with it. I am also working on using it for Blockstore-based content libraries in edx-platform and should be merging that in the next week or two. The only snag I’ve hit so far is that edx-platform seems to be using an out of date fork of Bridgekeeper that doesn’t quite match the docs (some things have been made easier/simpler in new versions); I think that’s only to support python 2 though so should be something that can be improved in the coming weeks after the switch to python 3.

Thanks @braden! Great to know that it’s working well for community installations.

I’d like to share some updates. I’ve now finished the first version of the Access Control Bakends. Because we are still using Hawthorn, this is the most important version to support i.e. without Bridgekeeper.

Later on, I’ll follow up with Bridgekeeper support till the whole platform is converted.

@cpennington I’m still working on this and experimenting with both intermediate solution (Hawthorn) that keeps in mind the long term direction towards Bridgekeeper.

I’ve noticed that most of the course access in the platform is guarded by has_access except for the notable exception of edx-search:course_discovery_search (link to GitHub).

Do you know of any plan to put access control mechanism in front of that view? If there’s no such plan, what do you think of using a similar mechanism to SearchResultProcessor that is used in do_search?

I’d like to share some progress regarding the pluggable access control backends. I’ve been able to get something working for Hawthorn:

As I have mentioned earlier I’ve noticed that edx-search does not check for access via the has_access function so I made a hack that I’d like to eventually refactor into something more solid to add such check:

There’s still a good amount of work left mostly to coup with the changes that’ll released in Juniper. The most notable change is the use of Bridgekeeper. So please reach out to me whether here or via email omar@appsembler.com if you’d like to collaborate on this project.

I don’t know of any plans to change how access control works for course_discovery_search. I think that as long as you can do a small number of queries to fetch all the attributes needed for the permissions checks for all the search results, then the performance won’t be terrible. Although pagination in the face of post-search access control still may lead to some funny results.

Thanks! So currently there’s no access control and my plan is to add that in a proper-ish way.

:+1: