From 95c67e36df12c5524b161b46179fc52811137301 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Tue, 5 Feb 2019 09:40:07 -0500 Subject: [PATCH 1/2] Added permission check for proctoring reviews --- edx_proctoring/__init__.py | 2 +- edx_proctoring/rules.py | 16 +++++++++++++ edx_proctoring/tests/test_reviews.py | 34 ++++++++++++++++++++++++++++ edx_proctoring/views.py | 8 +++++-- package.json | 2 +- requirements/base.txt | 8 +++---- requirements/dev.txt | 27 +++++++++++----------- requirements/doc.txt | 16 ++++++------- requirements/quality.txt | 10 ++++---- requirements/test.txt | 26 ++++++++++----------- 10 files changed, 101 insertions(+), 48 deletions(-) create mode 100644 edx_proctoring/rules.py diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 07b72830cdd..4147b5e6516 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -5,6 +5,6 @@ from __future__ import absolute_import # Be sure to update the version number in edx_proctoring/package.json -__version__ = '1.5.8' +__version__ = '1.5.9' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/rules.py b/edx_proctoring/rules.py new file mode 100644 index 00000000000..9b00aa67cd1 --- /dev/null +++ b/edx_proctoring/rules.py @@ -0,0 +1,16 @@ +"Django Rules for edx_proctoring" +from __future__ import absolute_import + +import rules + + +@rules.predicate +def is_in_reviewer_group(user, attempt): + """ + Returns whether user is in a group allowed to review this attempt + """ + backend_group = '%s_review' % attempt['proctored_exam']['backend'] + return user.groups.filter(name=backend_group).exists() + + +rules.add_perm('edx_proctoring.can_review_attempt', is_in_reviewer_group | rules.is_staff) diff --git a/edx_proctoring/tests/test_reviews.py b/edx_proctoring/tests/test_reviews.py index 364bd5d7d58..dcb44b28fef 100644 --- a/edx_proctoring/tests/test_reviews.py +++ b/edx_proctoring/tests/test_reviews.py @@ -180,6 +180,8 @@ def test_bad_review_status(self): ) @ddt.unpack def test_post_review(self, external_id, status): + self.user.is_staff = True + self.user.save() review = self.get_review_payload() if not external_id: external_id = self.attempt['external_id'] @@ -191,6 +193,38 @@ def test_post_review(self, external_id, status): ) self.assertEqual(response.status_code, status) + def test_post_review_auth(self): + review = json.dumps(self.get_review_payload()) + external_id = self.attempt['external_id'] + url = reverse('edx_proctoring:proctored_exam.attempt.callback', + kwargs={'external_id': external_id}) + response = self.client.post( + url, + review, + content_type='application/json' + ) + assert response.status_code == 403 + # staff users can review + self.user.is_staff = True + self.user.save() + response = self.client.post( + url, + review, + content_type='application/json' + ) + assert response.status_code == 200 + # user in the review group + group_name = '%s_review' % (self.attempt['proctored_exam']['backend']) + self.user.groups.get_or_create(name=group_name) + self.user.is_staff = False + self.user.save() + response = self.client.post( + url, + review, + content_type='application/json' + ) + assert response.status_code == 200 + @patch('edx_proctoring.constants.REQUIRE_FAILURE_SECOND_REVIEWS', False) def test_review_on_archived_attempt(self): """ diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 8b9e4d6122e..6143425bb97 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -898,8 +898,12 @@ def post(self, request, external_id): attempt = get_exam_attempt_by_external_id(external_id) if attempt is None: raise StudentExamAttemptDoesNotExistsException('not found') - self.make_review(attempt, request.data) - return Response(data='OK') + if request.user.has_perm('edx_proctoring.can_review_attempt', attempt): + self.make_review(attempt, request.data) + resp = Response(data='OK') + else: + resp = Response(status=403) + return resp class IgnoreClientContentNegotiation(BaseContentNegotiation): diff --git a/package.json b/package.json index 888b5be77d5..05e8572a63c 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "@edx/edx-proctoring", "//": "Be sure to update the version number in edx_proctoring/__init__.py", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "1.5.8", + "version": "1.5.9", "main": "edx_proctoring/static/index.js", "repository": { "type": "git", diff --git a/requirements/base.txt b/requirements/base.txt index f2a1e0750ad..57daf65573c 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -9,7 +9,7 @@ chardet==3.0.4 # via requests django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 -django-waffle==0.15.0 +django-waffle==0.15.1 django-webpack-loader==0.6.0 django==1.11.18 djangorestframework-jwt==1.11.0 # via edx-drf-extensions @@ -22,10 +22,10 @@ event-tracking==0.2.7 future==0.17.1 # via pyjwkest idna==2.8 # via requests jsonfield==2.0.2 -newrelic==4.8.0.110 # via edx-django-utils -pbr==5.1.1 # via stevedore +newrelic==4.12.0.113 # via edx-django-utils +pbr==5.1.2 # via stevedore psutil==1.2.1 # via edx-django-utils, edx-drf-extensions -pycryptodomex==3.7.2 +pycryptodomex==3.7.3 pyjwkest==1.3.2 # via edx-drf-extensions pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking diff --git a/requirements/dev.txt b/requirements/dev.txt index 36d2f524aaf..869f588632c 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -7,13 +7,13 @@ argparse==1.4.0 # via caniusepython3 astroid==1.5.2 # via edx-lint, pylint, pylint-celery backports.functools-lru-cache==1.5 # via astroid, caniusepython3, pylint -bleach==3.0.2 # via readme-renderer +bleach==3.1.0 # via readme-renderer caniusepython3==7.0.0 certifi==2018.11.29 # via requests chardet==3.0.4 # via requests click-log==0.1.8 # via edx-lint click==7.0 # via click-log, edx-lint, pip-tools -configparser==3.5.0 # via importlib-metadata, pydocstyle, pylint +configparser==3.7.1 # via importlib-metadata, pydocstyle, pylint contextlib2==0.5.5 # via importlib-metadata diff-cover==1.0.6 distlib==0.2.8 # via caniusepython3 @@ -33,40 +33,39 @@ jinja2==2.10 # via diff-cover, jinja2-pluralize lazy-object-proxy==1.3.1 # via astroid markupsafe==1.1.0 # via jinja2 mccabe==0.6.1 # via pylint -packaging==18.0 # via caniusepython3 +packaging==19.0 # via caniusepython3 path.py==11.5.0 # via edx-i18n-tools pathlib2==2.3.3 # via importlib-metadata -pip-tools==3.2.0 -pkginfo==1.4.2 # via twine -pluggy==0.8.0 # via tox +pip-tools==3.3.2 +pkginfo==1.5.0.1 # via twine +pluggy==0.8.1 # via tox polib==1.1.0 # via edx-i18n-tools py==1.7.0 # via tox -pycodestyle==2.4.0 +pycodestyle==2.5.0 pydocstyle==3.0.0 pygments==2.3.1 # via diff-cover, readme-renderer pylint-celery==0.3 # via edx-lint pylint-django==0.7.2 # via edx-lint pylint-plugin-utils==0.4 # via pylint-celery, pylint-django pylint==1.7.1 # via edx-lint, pylint-celery, pylint-django, pylint-plugin-utils -pyparsing==2.3.0 # via packaging +pyparsing==2.3.1 # via packaging pytz==2018.9 # via django pyyaml==3.13 # via edx-i18n-tools readme-renderer==24.0 # via twine -requests-toolbelt==0.8.0 # via twine +requests-toolbelt==0.9.1 # via twine requests==2.21.0 # via caniusepython3, requests-toolbelt, twine -rules==2.0.1 scandir==1.9.0 # via pathlib2 singledispatch==3.4.0.3 # via astroid, pylint six==1.12.0 # via astroid, bleach, diff-cover, edx-i18n-tools, edx-lint, packaging, pathlib2, pip-tools, pydocstyle, pylint, readme-renderer, singledispatch, tox snowballstemmer==1.2.1 # via pydocstyle toml==0.10.0 # via tox tox-battery==0.5.1 -tox==3.6.1 -tqdm==4.29.0 # via twine +tox==3.7.0 +tqdm==4.30.0 # via twine twine==1.12.1 urllib3==1.24.1 # via requests -virtualenv==16.2.0 # via tox +virtualenv==16.3.0 # via tox webencodings==0.5.1 # via bleach wheel==0.32.3 -wrapt==1.10.11 # via astroid +wrapt==1.11.1 # via astroid zipp==0.3.3 # via importlib-metadata diff --git a/requirements/doc.txt b/requirements/doc.txt index 4e5f84f39ae..1d9cd4abe25 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -6,13 +6,13 @@ # alabaster==0.7.12 # via sphinx babel==2.6.0 # via sphinx -bleach==3.0.2 # via readme-renderer +bleach==3.1.0 # via readme-renderer certifi==2018.11.29 # via requests chardet==3.0.4 # via doc8, requests django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 -django-waffle==0.15.0 +django-waffle==0.15.1 django-webpack-loader==0.6.0 django==1.11.18 djangorestframework-jwt==1.11.0 # via edx-drf-extensions @@ -31,17 +31,17 @@ imagesize==1.1.0 # via sphinx jinja2==2.10 # via sphinx jsonfield==2.0.2 markupsafe==1.1.0 # via jinja2 -newrelic==4.8.0.110 # via edx-django-utils -packaging==18.0 # via sphinx -pbr==5.1.1 # via stevedore +newrelic==4.12.0.113 # via edx-django-utils +packaging==19.0 # via sphinx +pbr==5.1.2 # via stevedore pockets==0.7.2 # via sphinxcontrib-napoleon psutil==1.2.1 # via edx-django-utils, edx-drf-extensions -pycryptodomex==3.7.2 +pycryptodomex==3.7.3 pygments==2.3.1 # via readme-renderer, sphinx pyjwkest==1.3.2 # via edx-drf-extensions pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking -pyparsing==2.3.0 # via packaging +pyparsing==2.3.1 # via packaging python-dateutil==2.7.5 pytz==2018.9 readme-renderer==24.0 @@ -53,7 +53,7 @@ semantic-version==2.6.0 # via edx-drf-extensions six==1.12.0 # via bleach, doc8, edx-drf-extensions, edx-opaque-keys, edx-sphinx-theme, packaging, pockets, pyjwkest, python-dateutil, readme-renderer, sphinx, sphinxcontrib-napoleon, stevedore slumber==0.7.1 # via edx-rest-api-client snowballstemmer==1.2.1 # via sphinx -sphinx==1.8.3 +sphinx==1.8.4 sphinxcontrib-napoleon==0.7 sphinxcontrib-websupport==1.1.0 # via sphinx stevedore==1.30.0 # via doc8, edx-opaque-keys diff --git a/requirements/quality.txt b/requirements/quality.txt index 681cc9d2512..812daf5e1cf 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -12,7 +12,7 @@ certifi==2018.11.29 # via requests chardet==3.0.4 # via requests click-log==0.1.8 # via edx-lint click==7.0 # via click-log, edx-lint -configparser==3.5.0 # via pydocstyle, pylint +configparser==3.7.1 # via pydocstyle, pylint distlib==0.2.8 # via caniusepython3 django==1.11.18 edx_lint==0.5.5 @@ -22,18 +22,18 @@ idna==2.8 # via requests isort==4.3.4 lazy-object-proxy==1.3.1 # via astroid mccabe==0.6.1 # via pylint -packaging==18.0 # via caniusepython3 -pycodestyle==2.4.0 +packaging==19.0 # via caniusepython3 +pycodestyle==2.5.0 pydocstyle==3.0.0 pylint-celery==0.3 # via edx-lint pylint-django==0.7.2 # via edx-lint pylint-plugin-utils==0.4 # via pylint-celery, pylint-django pylint==1.7.1 # via edx-lint, pylint-celery, pylint-django, pylint-plugin-utils -pyparsing==2.3.0 # via packaging +pyparsing==2.3.1 # via packaging pytz==2018.9 # via django requests==2.21.0 # via caniusepython3 singledispatch==3.4.0.3 # via astroid, pylint six==1.12.0 # via astroid, edx-lint, packaging, pydocstyle, pylint, singledispatch snowballstemmer==1.2.1 # via pydocstyle urllib3==1.24.1 # via requests -wrapt==1.10.11 # via astroid +wrapt==1.11.1 # via astroid diff --git a/requirements/test.txt b/requirements/test.txt index 989f07524bb..4cfde8e4c80 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -5,7 +5,7 @@ # pip-compile --output-file requirements/test.txt requirements/test.in # apipkg==1.5 # via execnet -atomicwrites==1.2.1 # via pytest +atomicwrites==1.3.0 # via pytest attrs==18.2.0 # via pytest bok-choy==0.9.0 certifi==2018.11.29 # via requests @@ -16,7 +16,7 @@ ddt==1.2.0 django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 -django-waffle==0.15.0 +django-waffle==0.15.1 django-webpack-loader==0.6.0 djangorestframework-jwt==1.11.0 # via edx-drf-extensions djangorestframework==3.6.4 @@ -29,29 +29,29 @@ execnet==1.5.0 # via pytest-xdist freezegun==0.3.11 funcsigs==1.0.2 # via mock, pytest future==0.17.1 # via pyjwkest -httmock==1.2.6 +httmock==1.3.0 httpretty==0.9.6 idna==2.8 # via requests jsonfield==2.0.2 -lazy==1.3 # via bok-choy +lazy==1.4 # via bok-choy logilab-common==1.4.2 mock==2.0.0 more-itertools==5.0.0 # via pytest -newrelic==4.8.0.110 # via edx-django-utils +newrelic==4.12.0.113 # via edx-django-utils pathlib2==2.3.3 # via pytest, pytest-django -pbr==5.1.1 # via mock, stevedore -pluggy==0.8.0 # via pytest +pbr==5.1.2 # via mock, stevedore +pluggy==0.8.1 # via pytest psutil==1.2.1 # via edx-django-utils, edx-drf-extensions py==1.7.0 # via pytest -pycryptodomex==3.7.2 +pycryptodomex==3.7.3 pyjwkest==1.3.2 # via edx-drf-extensions pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking pytest-cov==2.6.1 -pytest-django==3.4.4 -pytest-forked==0.2 # via pytest-xdist -pytest-xdist==1.25.0 -pytest==4.1.0 # via pytest-cov, pytest-django, pytest-forked, pytest-xdist +pytest-django==3.4.7 +pytest-forked==1.0.1 # via pytest-xdist +pytest-xdist==1.26.1 +pytest==4.2.0 # via pytest-cov, pytest-django, pytest-forked, pytest-xdist python-dateutil==2.7.5 pytz==2018.9 requests==2.21.0 # via edx-drf-extensions, edx-rest-api-client, httmock, pyjwkest, responses, slumber @@ -65,5 +65,5 @@ six==1.12.0 # via bok-choy, edx-drf-extensions, edx-opaque-keys, f slumber==0.7.1 # via edx-rest-api-client stevedore==1.30.0 # via edx-opaque-keys sure==1.2.7 -testfixtures==6.4.1 +testfixtures==6.5.0 urllib3==1.24.1 # via requests, selenium From d30c89996c4137729eae00d5f03e53daebccc3e9 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Wed, 6 Feb 2019 07:51:15 -0500 Subject: [PATCH 2/2] Added note in docs about creating group for review permission --- docs/developing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developing.rst b/docs/developing.rst index c4ea83bc75a..60b3dfaed89 100644 --- a/docs/developing.rst +++ b/docs/developing.rst @@ -86,7 +86,7 @@ Then back in your host shell:: pip install -e .[server] python -m mockprock.server -The command will tell you you have to supply an client_id and client_secret. It'll open your browser to the django admin page where you should create or use an existing credential. Note the client_id and client_secret and restart the server:: +The command will tell you you have to supply an client_id and client_secret. It'll open your browser to the Django admin page where you should create or use an existing credential. You'll also need to add the user associated with the credential to the "mockprock_review" Django group. You can create the group at ``/admin/auth/group/``. Note the client_id and client_secret and restart the server:: python -m mockprock.server {client_id} {client_secret}