From 2d1eb9923fa89f397914f85327fb256fab332cd9 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 29 Jul 2019 13:56:29 -0400 Subject: [PATCH] Convert a has_access check into a bridgekeeper rule This reverts commit 5e8f90caa1b751208182b84a80568f759f1467d9. --- cms/envs/common.py | 2 ++ lms/djangoapps/courseware/permissions.py | 9 +++++++++ lms/djangoapps/courseware/rules.py | 24 +++++++++++++++++++++++- lms/djangoapps/courseware/views/views.py | 3 ++- lms/envs/common.py | 4 +++- openedx/core/tests/__init__.py | 0 openedx/core/tests/test_admin_view.py | 24 ++++++++++++++++++++++++ requirements/edx/base.txt | 1 + requirements/edx/development.txt | 1 + requirements/edx/github.in | 5 +++++ requirements/edx/testing.txt | 1 + 11 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 lms/djangoapps/courseware/permissions.py create mode 100644 openedx/core/tests/__init__.py create mode 100644 openedx/core/tests/test_admin_view.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 69f08d9bf3..8d496c2b27 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -450,6 +450,7 @@ LOGIN_URL = reverse_lazy('login_redirect_to_lms') AUTHENTICATION_BACKENDS = [ 'rules.permissions.ObjectPermissionBackend', 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.backends.EdxRateLimitedAllowAllUsersModelBackend', + 'bridgekeeper.backends.RulePermissionBackend', ] STATIC_ROOT_BASE = '/edx/var/edxapp/staticfiles' @@ -1312,6 +1313,7 @@ INSTALLED_APPS = [ # rule-based authorization 'rules.apps.AutodiscoverRulesConfig', + 'bridgekeeper', # management of user-triggered async tasks (course import/export, etc.) 'user_tasks', diff --git a/lms/djangoapps/courseware/permissions.py b/lms/djangoapps/courseware/permissions.py new file mode 100644 index 0000000000..94ee2a0ee6 --- /dev/null +++ b/lms/djangoapps/courseware/permissions.py @@ -0,0 +1,9 @@ +""" +Permission definitions for the courseware djangoapp +""" + +from bridgekeeper import perms +from .rules import HasAccessRule + +VIEW_COURSE_HOME = 'courseware.view_course_home' +perms[VIEW_COURSE_HOME] = HasAccessRule('load') diff --git a/lms/djangoapps/courseware/rules.py b/lms/djangoapps/courseware/rules.py index 2dcb72c049..9d92abd664 100644 --- a/lms/djangoapps/courseware/rules.py +++ b/lms/djangoapps/courseware/rules.py @@ -1,12 +1,16 @@ """ -django-rules for courseware related features +django-rules and Bridgekeeper rules for courseware related features """ from __future__ import absolute_import +from bridgekeeper.rules import Rule from course_modes.models import CourseMode +from django.db.models import Q from opaque_keys.edx.keys import CourseKey from student.models import CourseEnrollment +from .access import has_access + import rules @@ -24,3 +28,21 @@ def is_track_ok_for_exam(user, exam): # proctored experience can_take_proctored_exam = is_track_ok_for_exam rules.set_perm('edx_proctoring.can_take_proctored_exam', is_track_ok_for_exam) + + +class HasAccessRule(Rule): + """ + A rule that calls `has_access` to determine whether it passes + """ + def __init__(self, action): + self.action = action + + def check(self, user, instance=None): + return has_access(user, self.action, instance) + + def query(self, user): + # Return an always-empty queryset filter so that this always + # fails permissions, but still passes the is_possible_for check + # that is used to determine if the rule should allow a user + # into django admin + return Q(pk__in=[]) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 23675ba5e5..5a0a649ffa 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -57,6 +57,7 @@ from courseware.courses import ( from courseware.masquerade import setup_masquerade from courseware.model_data import FieldDataCache from courseware.models import BaseStudentModuleHistory, StudentModule +from courseware.permissions import VIEW_COURSE_HOME from courseware.url_helpers import get_redirect_url from courseware.user_state_client import DjangoXBlockUserStateClient from edxmako.shortcuts import marketing_link, render_to_response, render_to_string @@ -810,7 +811,7 @@ def course_about(request, course_id): staff_access = bool(has_access(request.user, 'staff', course)) studio_url = get_studio_url(course, 'settings/details') - if has_access(request.user, 'load', course): + if request.user.has_perm(VIEW_COURSE_HOME, course): course_target = reverse(course_home_url_name(course.id), args=[text_type(course.id)]) else: course_target = reverse('about_course', args=[text_type(course.id)]) diff --git a/lms/envs/common.py b/lms/envs/common.py index f4ad12ca8b..98f3b0f8a9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -749,7 +749,8 @@ derived_collection_entry('DEFAULT_TEMPLATE_ENGINE', 'DIRS') AUTHENTICATION_BACKENDS = [ 'rules.permissions.ObjectPermissionBackend', - 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.backends.EdxRateLimitedAllowAllUsersModelBackend' + 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.backends.EdxRateLimitedAllowAllUsersModelBackend', + 'bridgekeeper.backends.RulePermissionBackend', ] STUDENT_FILEUPLOAD_MAX_SIZE = 4 * 1000 * 1000 # 4 MB @@ -2480,6 +2481,7 @@ INSTALLED_APPS = [ # rule-based authorization 'rules.apps.AutodiscoverRulesConfig', + 'bridgekeeper', # Customized celery tasks, including persisting failed tasks so they can # be retried diff --git a/openedx/core/tests/__init__.py b/openedx/core/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/tests/test_admin_view.py b/openedx/core/tests/test_admin_view.py new file mode 100644 index 0000000000..403c2163d1 --- /dev/null +++ b/openedx/core/tests/test_admin_view.py @@ -0,0 +1,24 @@ +""" +Tests that verify that the admin view loads. + +This is not inside a django app because it is a global property of the system. +""" + +from django.test import TestCase, Client +from django.urls import reverse +from student.tests.factories import UserFactory, TEST_PASSWORD + + +class TestAdminView(TestCase): + """ + Tests of the admin view + """ + def setUp(self): + super(TestAdminView, self).setUp() + self.client = Client() + self.staff_user = UserFactory.create(is_staff=True) + self.client.login(username=self.staff_user.username, password=TEST_PASSWORD) + + def test_admin_view_loads(self): + response = self.client.get(reverse('admin:index')) + assert response.status_code == 200 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 86d193fb57..4f3e9f7508 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -35,6 +35,7 @@ bleach==2.1.4 boto3==1.4.8 boto==2.39.0 botocore==1.8.17 +git+https://github.com/edx/bridgekeeper.git@4e34894e4ac5d0467ed1901811a81fd87ee01937#egg=bridgekeeper==0.0 git+https://github.com/edx/openedx-calc.git@e9b698c85ad1152002bc0868f475f153dce88952#egg=calc==0.4 celery==3.1.25 certifi==2019.6.16 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 22ad41be3b..d808476462 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -43,6 +43,7 @@ bok-choy==1.0.0 boto3==1.4.8 boto==2.39.0 botocore==1.8.17 +git+https://github.com/edx/bridgekeeper.git@4e34894e4ac5d0467ed1901811a81fd87ee01937#egg=bridgekeeper==0.0 git+https://github.com/edx/openedx-calc.git@e9b698c85ad1152002bc0868f475f153dce88952#egg=calc==0.4 caniusepython3==7.1.0 celery==3.1.25 diff --git a/requirements/edx/github.in b/requirements/edx/github.in index 7f20755908..fa39772337 100644 --- a/requirements/edx/github.in +++ b/requirements/edx/github.in @@ -74,6 +74,11 @@ git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4b # This dependency will be removed by the Celery 4 upgrade: https://openedx.atlassian.net/browse/PLAT-1684 git+https://github.com/edx/django-celery.git@756cb57aad765cb2b0d37372c1855b8f5f37e6b0#egg=django-celery==3.2.1+edx.2 +# Forked to add support for python2.7 and to fix predicate inversion +# Once https://github.com/excitedleigh/bridgekeeper/pull/10 is merged, and we get to python3 and django 2, we can +# remove this fork +git+https://github.com/edx/bridgekeeper.git@4e34894e4ac5d0467ed1901811a81fd87ee01937#egg=bridgekeeper==0.0 + # Our libraries: -e git+https://github.com/edx/codejail.git@a320d43ce6b9c93b17636b2491f724d9e433be47#egg=codejail -e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index f1407036cf..7cbc29fb37 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -41,6 +41,7 @@ bok-choy==1.0.0 boto3==1.4.8 boto==2.39.0 botocore==1.8.17 +git+https://github.com/edx/bridgekeeper.git@4e34894e4ac5d0467ed1901811a81fd87ee01937#egg=bridgekeeper==0.0 git+https://github.com/edx/openedx-calc.git@e9b698c85ad1152002bc0868f475f153dce88952#egg=calc==0.4 caniusepython3==7.1.0 celery==3.1.25