Skip to content

Commit

Permalink
feat: adds django-rules based permissions for tagging app
Browse files Browse the repository at this point in the history
Also:
* Adds rules requirement and app settings to enable it
* Adds mock to test requirements, so we can test system taxonomy rules
* ADR: Clarifies that rules will be enforced in the views, not the model or APIs
  • Loading branch information
pomegranited committed Jun 26, 2023
1 parent bb1790d commit ac388bb
Show file tree
Hide file tree
Showing 12 changed files with 340 additions and 0 deletions.
3 changes: 3 additions & 0 deletions docs/decisions/0009-tagging-administrators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ But because permissions #2 + #3 require access to the edx-platform CMS model `Co

Per `OEP-9`_, ``openedx_tagging`` will allow applications to use the standard Django API to query permissions, for example: ``user.has_perm('openedx_tagging.edit_taxonomy', taxonomy)``, and the appropriate permissions will be applied in that application's context.

These rules will be enforced in the tagging `views`_, not the API or models, so that external code using this library need not have a logged-in user in order to call the API. So please use with care.

Rejected Alternatives
---------------------

Expand All @@ -37,3 +39,4 @@ This is a standard way to grant access in Django apps, but it is not used in Ope
.. _get_organizations: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/contentstore/views/course.py#L1958
.. _CourseCreator: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/course_creators/models.py#L27
.. _OEP-9: https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0009-bp-permissions.html
.. _views: https://github.com/dfunckt/django-rules#permissions-in-views
74 changes: 74 additions & 0 deletions openedx_tagging/core/tagging/rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
"""Django rules-based permissions for tagging"""

import rules

# Global staff are taxonomy admins.
# (Superusers can already do anything)
is_taxonomy_admin = rules.is_staff


@rules.predicate
def can_view_taxonomy(user, taxonomy=None):
"""
Anyone can view an enabled taxonomy,
but only taxonomy admins can view a disabled taxonomy.
"""
return (taxonomy and taxonomy.enabled) or is_taxonomy_admin(user)


@rules.predicate
def can_change_taxonomy(user, taxonomy=None):
"""
Even taxonomy admins cannot change system taxonomies.
"""
return is_taxonomy_admin(user) and (
not taxonomy or not taxonomy or (taxonomy and not taxonomy.system_defined)
)


@rules.predicate
def can_change_taxonomy_tag(user, tag=None):
"""
Even taxonomy admins cannot add tags to system taxonomies (their tags are system-defined), or free-text taxonomies
(these don't have predefined tags).
"""
return is_taxonomy_admin(user) and (
not tag
or not tag.taxonomy
or (
tag.taxonomy
and not tag.taxonomy.allow_free_text
and not tag.taxonomy.system_defined
)
)


@rules.predicate
def can_change_object_tag(user, object_tag=None):
"""
Taxonomy admins can create or modify object tags on enabled taxonomies.
"""
return is_taxonomy_admin(user) and (
not object_tag
or not object_tag.taxonomy
or (object_tag.taxonomy and object_tag.taxonomy.enabled)
)


# Taxonomy
rules.add_perm("oel_tagging.add_taxonomy", can_change_taxonomy)
rules.add_perm("oel_tagging.change_taxonomy", can_change_taxonomy)
rules.add_perm("oel_tagging.delete_taxonomy", can_change_taxonomy)
rules.add_perm("oel_tagging.view_taxonomy", can_view_taxonomy)

# Tag
rules.add_perm("oel_tagging.add_tag", can_change_taxonomy_tag)
rules.add_perm("oel_tagging.change_tag", can_change_taxonomy_tag)
rules.add_perm("oel_tagging.delete_tag", is_taxonomy_admin)
rules.add_perm("oel_tagging.view_tag", rules.always_allow)

# ObjectTag
rules.add_perm("oel_tagging.add_object_tag", can_change_object_tag)
rules.add_perm("oel_tagging.change_object_tag", can_change_object_tag)
rules.add_perm("oel_tagging.delete_object_tag", is_taxonomy_admin)
rules.add_perm("oel_tagging.view_object_tag", rules.always_allow)
6 changes: 6 additions & 0 deletions projects/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,19 @@
# REST API
"rest_framework",
"openedx_learning.rest_api.apps.RESTAPIConfig",
# django-rules based authorization
'rules.apps.AutodiscoverRulesConfig',
# Tagging Core Apps
"openedx_tagging.core.tagging.apps.TaggingConfig",

# Debugging
"debug_toolbar",
)

AUTHENTICATION_BACKENDS = [
'rules.permissions.ObjectPermissionBackend',
]

MIDDLEWARE = [
"debug_toolbar.middleware.DebugToolbarMiddleware",
"django.middleware.security.SecurityMiddleware",
Expand Down
2 changes: 2 additions & 0 deletions requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
Django<5.0 # Web application framework

djangorestframework<4.0 # REST API

rules<4.0 # Django extension for rules-based authorization checks
2 changes: 2 additions & 0 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pytz==2023.3
# via
# django
# djangorestframework
rules==3.3
# via -r requirements/base.in
sqlparse==0.4.4
# via django
typing-extensions==4.6.3
Expand Down
4 changes: 4 additions & 0 deletions requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ mdurl==0.1.2
# via
# -r requirements/quality.txt
# markdown-it-py
mock==5.0.2
# via -r requirements/quality.txt
more-itertools==9.1.0
# via
# -r requirements/quality.txt
Expand Down Expand Up @@ -296,6 +298,8 @@ rich==13.4.2
# via
# -r requirements/quality.txt
# twine
rules==3.3
# via -r requirements/quality.txt
secretstorage==3.3.3
# via
# -r requirements/quality.txt
Expand Down
4 changes: 4 additions & 0 deletions requirements/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ markupsafe==2.1.3
# via
# -r requirements/test.txt
# jinja2
mock==5.0.2
# via -r requirements/test.txt
mysqlclient==2.1.1
# via -r requirements/test.txt
packaging==23.1
Expand Down Expand Up @@ -139,6 +141,8 @@ requests==2.31.0
# via sphinx
restructuredtext-lint==1.4.0
# via doc8
rules==3.3
# via -r requirements/test.txt
six==1.16.0
# via bleach
snowballstemmer==2.2.0
Expand Down
4 changes: 4 additions & 0 deletions requirements/quality.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ mccabe==0.7.0
# via pylint
mdurl==0.1.2
# via markdown-it-py
mock==5.0.2
# via -r requirements/test.txt
more-itertools==9.1.0
# via jaraco-classes
mysqlclient==2.1.1
Expand Down Expand Up @@ -182,6 +184,8 @@ rfc3986==2.0.0
# via twine
rich==13.4.2
# via twine
rules==3.3
# via -r requirements/test.txt
secretstorage==3.3.3
# via keyring
six==1.16.0
Expand Down
1 change: 1 addition & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ pytest-cov # pytest extension for code coverage statistics
pytest-django # pytest extension for better Django support
code-annotations # provides commands used by the pii_check make target.
ddt # supports data driven tests
mock # supports overriding classes and methods in tests
4 changes: 4 additions & 0 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ jinja2==3.1.2
# via code-annotations
markupsafe==2.1.3
# via jinja2
mock==5.0.2
# via -r requirements/test.in
mysqlclient==2.1.1
# via -r requirements/test.in
packaging==23.1
Expand All @@ -64,6 +66,8 @@ pytz==2023.3
# djangorestframework
pyyaml==6.0
# via code-annotations
rules==3.3
# via -r requirements/base.txt
sqlparse==0.4.4
# via
# -r requirements/base.txt
Expand Down
6 changes: 6 additions & 0 deletions test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,19 @@ def root(*args):
# Admin
# 'django.contrib.admin',
# 'django.contrib.admindocs',
# django-rules based authorization
'rules.apps.AutodiscoverRulesConfig',
# Our own apps
"openedx_learning.core.components.apps.ComponentsConfig",
"openedx_learning.core.contents.apps.ContentsConfig",
"openedx_learning.core.publishing.apps.PublishingConfig",
"openedx_tagging.core.tagging.apps.TaggingConfig",
]

AUTHENTICATION_BACKENDS = [
'rules.permissions.ObjectPermissionBackend',
]

LOCALE_PATHS = [
root("openedx_learning", "conf", "locale"),
]
Expand Down
Loading

0 comments on commit ac388bb

Please sign in to comment.