Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ Unreleased

*


0.20.1 - 2026-02-05
********************

Added
=====

* Add PoF role and permissions for the advanced course settings section

0.20.0 - 2025-11-27
********************

Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

import os

__version__ = "0.20.0"
__version__ = "0.20.1"

ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
105 changes: 105 additions & 0 deletions openedx_authz/api/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@

from attrs import define
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocatorV2

from openedx_authz.tests.stubs.models import CourseOverview

try:
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
except ImportError:
Expand Down Expand Up @@ -462,6 +465,108 @@ def __repr__(self):
return self.namespaced_key


@define
class CourseOverviewData(ScopeData):
"""A course scope for authorization in the Open edX platform.

Courses uses the CourseKey format for identification.

Attributes:
NAMESPACE: 'course' for course scopes.
external_key: The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1').
Must be a valid CourseKey format.
namespaced_key: The course identifier with namespace (e.g., 'course^course-v1:TestOrg+TestCourse+2024_T1').
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ScopeMeta.get_subclass_by_external_key assumes what is before the first ':' is the namespace, so in our case, if we do something like ScopeData(external_key='course-v1:TestOrg+TestCourse+2024_T1'), ScopeMeta will assume that course-v1 is the namespace, but we set the namespace to be 'course', so it will fail to find this subclass. We will need to analyze how to handle this difference between course keys and library keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could explore using the course-v1 namespace. That would mean creating a new scope for each new version, which actually sounds reasonable to me.
Otherwise, we’d need to handle versioning within our Scope model.
I updated the tests to use course-v1, and if that approach looks good to you, we can keep it that way.
What do you think? @rodmgwgu @wgu-taylor-payne @bmtcril

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that makes sense to me, and I think is what we were planning on. Given the way different opaque keys work it makes scoping by the different types easier. I think you'll need to change it at line 483 here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. However, on second thought, there could be side effects depending on what we want. I’d like to ask:

What should the expected namespaced_key be?

A) course^course-v1:TestOrg+TestCourse+2024_T1

Implications:

  • Update get_subclass_by_external_key to retrieve it based on the ScopeModel namespace instead of the - ScopeData namespace (somehow).
  • The policy remains as is:
    p, role^course_staff, act^courses.manage_advanced_settings, course^*, allow

B) course-v1^course-v1:TestOrg+TestCourse+2024_T1
Implications:

  • Keep get_subclass_by_external_key as is.
  • Update the policy to:
    p, role^course_staff, act^courses.manage_advanced_settings, course-v1^*, allow

What do you think is the better option?

Copy link
Contributor

@rodmgwgu rodmgwgu Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both should match course-v1 so it should be like course-v1^course-v1:TestOrg+TestCourse+2024_T1.

If we are all aligned with this, I'll also change this on my frontend PR, where I'm referring to these permissions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree with @rodmgwgu

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes applied. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: With option B, I had to update the policy to p, role^course_staff, act^courses.manage_advanced_settings, course-v1^*, allow

course_id: Property alias for external_key.

Examples:
>>> course = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1')
>>> course.namespaced_key
'course^course-v1:TestOrg+TestCourse+2024_T1'
>>> course.course_id
'course-v1:TestOrg+TestCourse+2024_T1'

"""

NAMESPACE: ClassVar[str] = "course"

@property
def course_id(self) -> str:
"""The course identifier as used in Open edX (e.g., 'course-v1:TestOrg+TestCourse+2024_T1').

This is an alias for external_key that represents the course ID without the namespace prefix.

Returns:
str: The course identifier without namespace.
"""
return self.external_key

@property
def course_key(self) -> CourseKey:
"""The CourseKey object for the course.

Returns:
CourseKey: The course key object.
"""
return CourseKey.from_string(self.course_id)

@classmethod
def validate_external_key(cls, external_key: str) -> bool:
"""Validate the external_key format for CourseOverviewData.

Args:
external_key: The external key to validate.

Returns:
bool: True if valid, False otherwise.
"""
try:
CourseKey.from_string(external_key)
return True
except InvalidKeyError:
return False

def get_object(self) -> CourseOverview | None:
"""Retrieve the CourseOverview instance associated with this scope.

This method converts the course_id to a CourseKey and queries the
database to fetch the corresponding CourseOverview object.

Returns:
CourseOverview | None: The CourseOverview instance if found in the database,
or None if the course does not exist or has an invalid key format.

Examples:
>>> course_scope = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1')
>>> course_obj = course_scope.get_object() # CourseOverview object
"""
try:
course_obj = CourseOverview.get_from_id(self.course_key)
# Validate canonical key: get_by_key is case-insensitive, but we require exact match
# This ensures authorization uses canonical course IDs consistently
if course_obj.id != self.course_key:
raise CourseOverview.DoesNotExist
except (InvalidKeyError, CourseOverview.DoesNotExist):
return None

return course_obj

def exists(self) -> bool:
"""Check if the course overview exists.

Returns:
bool: True if the course overview exists, False otherwise.
"""
return self.get_object() is not None

def __str__(self):
"""Human readable string representation of the course overview."""
return self.course_id

def __repr__(self):
"""Developer friendly string representation of the course overview."""
return self.namespaced_key


class SubjectMeta(type):
"""Metaclass for SubjectData to handle dynamic subclass instantiation based on namespace."""

Expand Down
9 changes: 9 additions & 0 deletions openedx_authz/constants/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,12 @@
action=ActionData(external_key=f"{CONTENT_LIBRARIES_NAMESPACE}.delete_library_collection"),
effect="allow",
)

# Course Permissions

COURSES_NAMESPACE = "courses"

MANAGE_ADVANCED_SETTINGS = PermissionData(
action=ActionData(external_key=f"{COURSES_NAMESPACE}.manage_advanced_settings"),
effect="allow",
)
10 changes: 10 additions & 0 deletions openedx_authz/constants/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,13 @@
LIBRARY_AUTHOR = RoleData(external_key="library_author", permissions=LIBRARY_AUTHOR_PERMISSIONS)
LIBRARY_CONTRIBUTOR = RoleData(external_key="library_contributor", permissions=LIBRARY_CONTRIBUTOR_PERMISSIONS)
LIBRARY_USER = RoleData(external_key="library_user", permissions=LIBRARY_USER_PERMISSIONS)


# Course Roles and Permissions


COURSE_STAFF_PERMISSIONS = [
permissions.MANAGE_ADVANCED_SETTINGS,
]

COURSE_STAFF = RoleData(external_key="course_staff", permissions=COURSE_STAFF_PERMISSIONS)
6 changes: 6 additions & 0 deletions openedx_authz/engine/config/authz.policy
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,9 @@ g2, act^content_libraries.manage_library_team, act^content_libraries.view_librar
g2, act^content_libraries.delete_library_collection, act^content_libraries.edit_library_collection
g2, act^content_libraries.create_library_collection, act^content_libraries.edit_library_collection
g2, act^content_libraries.edit_library_collection, act^content_libraries.view_library


# Course Policies

# Course Staff Permissions
p, role^course_staff, act^courses.manage_advanced_settings, course^*, allow
14 changes: 10 additions & 4 deletions openedx_authz/engine/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,24 @@
from django.contrib.auth import get_user_model
from edx_django_utils.cache import RequestCache

from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData
from openedx_authz.api.data import ContentLibraryData, CourseOverviewData, ScopeData, UserData
from openedx_authz.rest_api.utils import get_user_by_username_or_email

User = get_user_model()


SCOPES_WITH_ADMIN_OR_SUPERUSER_CHECK = {
(ContentLibraryData.NAMESPACE, ContentLibraryData),
(CourseOverviewData.NAMESPACE, CourseOverviewData),
}


def is_admin_or_superuser_check(request_user: str, request_action: str, request_scope: str) -> bool: # pylint: disable=unused-argument
"""
Evaluates custom, non-role-based conditions for authorization checks.

Checks attribute-based conditions that don't rely on role assignments.
Currently handles ContentLibraryData scopes by granting access to staff
Currently handles ContentLibraryData and CourseOverviewData scopes by granting access to staff
and superusers.

Args:
Expand All @@ -24,7 +30,7 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_

Returns:
bool: True if the condition is satisfied (user is staff/superuser for
ContentLibraryData scopes), False otherwise (including when user
ContentLibraryData and CourseOverviewData scopes), False otherwise (including when user
doesn't exist or scope type is not supported)
"""

Expand All @@ -34,7 +40,7 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_

# TODO: This special case for superuser and staff users is currently only for
# content libraries. See: https://github.com/openedx/openedx-authz/issues/87
if not isinstance(scope, ContentLibraryData):
if (scope.NAMESPACE, type(scope)) not in SCOPES_WITH_ADMIN_OR_SUPERUSER_CHECK:
return False

cached_response = request_cache.get_cached_response(username)
Expand Down
45 changes: 45 additions & 0 deletions openedx_authz/migrations/0007_coursescope.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Generated by Django 4.2.24 on 2026-02-06 17:19

import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("stubs", "__first__"),
("openedx_authz", "0006_migrate_legacy_permissions"),
]

operations = [
migrations.CreateModel(
name="CourseScope",
fields=[
(
"scope_ptr",
models.OneToOneField(
auto_created=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
primary_key=True,
serialize=False,
to="openedx_authz.scope",
),
),
(
"course_overview",
models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.CASCADE,
related_name="authz_scopes",
to=settings.OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL,
),
),
],
options={
"abstract": False,
},
bases=("openedx_authz.scope",),
),
]
65 changes: 64 additions & 1 deletion openedx_authz/models/scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.apps import apps
from django.conf import settings
from django.db import models
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocatorV2

from openedx_authz.models.core import Scope
Expand All @@ -31,7 +32,26 @@ def get_content_library_model():
return None


def get_course_overview_model():
"""Return the CourseOverview model class specified by settings.

The setting `OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL` should be an
app_label.ModelName string (e.g. 'content.CourseOverview').
"""
COURSE_OVERVIEW_MODEL = getattr(
settings,
"OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL",
"content.CourseOverview",
)
try:
app_label, model_name = COURSE_OVERVIEW_MODEL.split(".")
return apps.get_model(app_label, model_name, require_ready=False)
except LookupError:
return None


ContentLibrary = get_content_library_model()
CourseOverview = get_course_overview_model()


class ContentLibraryScope(Scope):
Expand All @@ -42,7 +62,7 @@ class ContentLibraryScope(Scope):

NAMESPACE = "lib"

# Link to the actual course or content library, if applicable. In other cases, this could be null.
# Link to the actual content library, if applicable. In other cases, this could be null.
# Piggybacking on the existing ContentLibrary model to keep the ExtendedCasbinRule up to date
# by deleting the Scope, and thus the ExtendedCasbinRule, when the ContentLibrary is deleted.
#
Expand Down Expand Up @@ -75,3 +95,46 @@ def get_or_create_for_external_key(cls, scope):
content_library = ContentLibrary.objects.get_by_key(library_key)
scope, _ = cls.objects.get_or_create(content_library=content_library)
return scope


class CourseScope(Scope):
"""Scope representing a course in the authorization system.

.. no_pii:
"""

NAMESPACE = "course"

# Link to the actual course, if applicable. In other cases, this could be null.
# Piggybacking on the existing CourseOverview model to keep the ExtendedCasbinRule up to date
# by deleting the Scope, and thus the ExtendedCasbinRule, when the CourseOverview is deleted.
#
# When content IS available, the on_delete=CASCADE will still work at the
# application level through Django's signal handlers.
# Use a string reference to the external app's model so Django won't try
# to import it at model import time. The migration already records the
# dependency on `content` when the app is present.
course_overview = models.ForeignKey(
settings.OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL,
on_delete=models.CASCADE,
null=True,
blank=True,
related_name="authz_scopes",
swappable=True,
)

@classmethod
def get_or_create_for_external_key(cls, scope):
"""Get or create a CourseScope for the given external key.

Args:
scope: ScopeData object with an external_key attribute containing
a CourseKey string.

Returns:
CourseScope: The Scope instance for the given CourseOverview
"""
course_key = CourseKey.from_string(scope.external_key)
course_overview = CourseOverview.get_from_id(course_key)
scope, _ = cls.objects.get_or_create(course_overview=course_overview)
return scope
4 changes: 4 additions & 0 deletions openedx_authz/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ def plugin_settings(settings):
if not hasattr(settings, "OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL"):
settings.OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL = "content_libraries.ContentLibrary"

# Set default CourseOverview model for swappable dependency
if not hasattr(settings, "OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL"):
settings.OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "content.CourseOverview"

# Set default CASBIN_LOG_LEVEL if not already set.
# This setting defines the logging level for the Casbin enforcer.
if not hasattr(settings, "CASBIN_LOG_LEVEL"):
Expand Down
1 change: 1 addition & 0 deletions openedx_authz/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,4 @@ def plugin_settings(settings): # pylint: disable=unused-argument

# Use stub model for testing instead of the real content_libraries app
OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL = "stubs.ContentLibrary"
OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "stubs.CourseOverview"
Loading