From 025ace3c3282e8009e1262bc758352e78a8174b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 13 Jun 2024 11:17:26 +0300 Subject: [PATCH 1/4] feat: [AXM-644] Add authorization via cms worker for content generation view --- .../contentstore/signals/handlers.py | 5 +++-- cms/djangoapps/contentstore/utils.py | 17 +++++++++++++++++ cms/envs/common.py | 2 ++ cms/envs/production.py | 3 +++ lms/urls.py | 2 +- openedx/features/offline_mode/urls.py | 1 + openedx/features/offline_mode/views.py | 18 ++++++++++++++++-- 7 files changed, 43 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 08347dd9cd53..d04a28a1d71d 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -2,7 +2,6 @@ import logging -import requests from datetime import datetime, timezone from functools import wraps from typing import Optional @@ -23,6 +22,7 @@ CoursewareSearchIndexer, LibrarySearchIndexer, ) +from cms.djangoapps.contentstore.utils import get_cms_api_client from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type from common.djangoapps.util.block_utils import yield_dynamic_block_descendants from lms.djangoapps.grades.api import task_compute_all_grades_for_course @@ -160,7 +160,8 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) if is_offline_mode_enabled(course_key): - requests.post( + client = get_cms_api_client() + client.post( url=urljoin(settings.LMS_ROOT_URL, LMS_OFFLINE_HANDLER_URL), data={'course_id': str(course_key)}, ) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 9ed3f1ce4b36..82269214af69 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -5,6 +5,7 @@ import configparser import logging import re +import requests from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone @@ -12,10 +13,12 @@ from uuid import uuid4 from django.conf import settings +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.urls import reverse from django.utils import translation from django.utils.translation import gettext as _ +from edx_rest_api_client.auth import SuppliedJwtAuth from eventtracking import tracker from help_tokens.core import HelpUrlExpert from lti_consumer.models import CourseAllowPIISharingInLTIFlag @@ -67,6 +70,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.models.course_details import CourseDetails +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.lib.courses import course_image_url from openedx.core.lib.html_to_text import html_to_text from openedx.features.content_type_gating.models import ContentTypeGatingConfig @@ -107,6 +111,7 @@ IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip') log = logging.getLogger(__name__) +User = get_user_model() def add_instructor(course_key, requesting_user, new_instructor): @@ -2317,3 +2322,15 @@ def get_xblock_render_context(request, block): return str(exc) return "" + + +def get_cms_api_client(): + """ + Returns an API client which can be used to make Exams API requests. + """ + user = User.objects.get(username=settings.EDXAPP_CMS_SERVICE_USER_NAME) + jwt = create_jwt_for_user(user) + client = requests.Session() + client.auth = SuppliedJwtAuth(jwt) + + return client diff --git a/cms/envs/common.py b/cms/envs/common.py index 24a63fb7d99e..8f9187c15de2 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2525,6 +2525,8 @@ EXAMS_SERVICE_URL = 'http://localhost:18740/api/v1' EXAMS_SERVICE_USERNAME = 'edx_exams_worker' +CMS_SERVICE_USER_NAME = 'edxapp_cms_worker' + FINANCIAL_REPORTS = { 'STORAGE_TYPE': 'localfs', 'BUCKET': None, diff --git a/cms/envs/production.py b/cms/envs/production.py index cf2a7d2f3fad..351804fdd379 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -169,6 +169,9 @@ def get_env_setting(setting): AUTHORING_API_URL = ENV_TOKENS.get('AUTHORING_API_URL', '') # Note that FEATURES['PREVIEW_LMS_BASE'] gets read in from the environment file. +CMS_SERVICE_USER_NAME = ENV_TOKENS.get('CMS_SERVICE_USER_NAME', CMS_SERVICE_USER_NAME) + + OPENAI_API_KEY = ENV_TOKENS.get('OPENAI_API_KEY', '') LEARNER_ENGAGEMENT_PROMPT_FOR_ACTIVE_CONTRACT = ENV_TOKENS.get('LEARNER_ENGAGEMENT_PROMPT_FOR_ACTIVE_CONTRACT', '') LEARNER_ENGAGEMENT_PROMPT_FOR_NON_ACTIVE_CONTRACT = ENV_TOKENS.get( diff --git a/lms/urls.py b/lms/urls.py index 98e02398de38..f382df0184b2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1055,5 +1055,5 @@ ] urlpatterns += [ - path('offline_mode/', include('openedx.features.offline_mode.urls')), + path('offline_mode/', include('openedx.features.offline_mode.urls', namespace='offline_mode')), ] diff --git a/openedx/features/offline_mode/urls.py b/openedx/features/offline_mode/urls.py index f5178a424316..d44d0d738f8a 100644 --- a/openedx/features/offline_mode/urls.py +++ b/openedx/features/offline_mode/urls.py @@ -5,6 +5,7 @@ from .views import SudioCoursePublishedEventHandler +app_name = 'offline_mode' urlpatterns = [ path('handle_course_published', SudioCoursePublishedEventHandler.as_view(), name='handle_course_published'), ] diff --git a/openedx/features/offline_mode/views.py b/openedx/features/offline_mode/views.py index 111b69175770..8e6309b202cb 100644 --- a/openedx/features/offline_mode/views.py +++ b/openedx/features/offline_mode/views.py @@ -2,10 +2,15 @@ Views for the offline_mode app. """ from opaque_keys.edx.keys import CourseKey +from opaque_keys import InvalidKeyError +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import status +from rest_framework.authentication import SessionAuthentication +from rest_framework.permissions import IsAdminUser from rest_framework.response import Response from rest_framework.views import APIView +from openedx.core.lib.api.authentication import BearerAuthentication from .tasks import generate_offline_content_for_course from .toggles import is_offline_mode_enabled @@ -18,6 +23,9 @@ class SudioCoursePublishedEventHandler(APIView): and it triggers the generation of offline content. """ + authentication_classes = (JwtAuthentication, BearerAuthentication, SessionAuthentication) + permission_classes = (IsAdminUser,) + def post(self, request, *args, **kwargs): """ Trigger the generation of offline content task. @@ -30,14 +38,20 @@ def post(self, request, *args, **kwargs): Returns: Response: The response object. """ - course_id = request.data.get('course_id') if not course_id: return Response( data={'error': 'course_id is required'}, status=status.HTTP_400_BAD_REQUEST ) - course_key = CourseKey.from_string(course_id) + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError: + return Response( + data={'error': 'Invalid course_id'}, + status=status.HTTP_400_BAD_REQUEST + ) + if is_offline_mode_enabled(course_key): generate_offline_content_for_course.apply_async(args=[course_id]) return Response(status=status.HTTP_200_OK) From 939591053f7af40917c5c9ed820a763296d493f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 13 Jun 2024 11:17:54 +0300 Subject: [PATCH 2/4] feat: [AXM-644] add tests for content generation view --- .../features/offline_mode/tests/__init__.py | 0 .../features/offline_mode/tests/test_views.py | 80 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 openedx/features/offline_mode/tests/__init__.py create mode 100644 openedx/features/offline_mode/tests/test_views.py diff --git a/openedx/features/offline_mode/tests/__init__.py b/openedx/features/offline_mode/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/offline_mode/tests/test_views.py b/openedx/features/offline_mode/tests/test_views.py new file mode 100644 index 000000000000..87fea7c79268 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_views.py @@ -0,0 +1,80 @@ +from unittest.mock import patch + +from django.test import TestCase, RequestFactory +from django.test.client import Client +from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_flag + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.features.offline_mode.views import SudioCoursePublishedEventHandler + +from openedx.features.offline_mode.toggles import ENABLE_OFFLINE_MODE + + +class TestSudioCoursePublishedEventHandler(TestCase): + """ + Tests for the SudioCoursePublishedEventHandler view. + """ + + def setUp(self): + self.client = Client() + self.factory = RequestFactory() + self.view = SudioCoursePublishedEventHandler.as_view() + self.url = reverse('offline_mode:handle_course_published') + + self.user_password = 'Password1234' + self.user = UserFactory.create(password=self.user_password) + self.staff_user = UserFactory.create(is_staff=True, password=self.user_password) + + def staff_login(self): + self.client.login(username=self.staff_user.username, password=self.user_password) + + def test_unauthorized(self): + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 401) + self.assertEqual(response.data, {'detail': 'Authentication credentials were not provided.'}) + + def test_not_admin(self): + self.client.login(username=self.user.username, password=self.user_password) + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data, {'detail': 'You do not have permission to perform this action.'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + @patch('openedx.features.offline_mode.views.generate_offline_content_for_course.apply_async') + def test_admin_enabled_waffle_flag(self, mock_generate_offline_content_for_course_task): + self.staff_login() + course_id = 'course-v1:edX+DemoX+Demo_Course' + response = self.client.post(self.url, {'course_id': course_id}) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, None) + mock_generate_offline_content_for_course_task.assert_called_once_with(args=[course_id]) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=False) + def test_admin_disabled_waffle_flag(self): + self.staff_login() + response = self.client.post(self.url, {'course_id': 'course-v1:edX+DemoX+Demo_Course'}) + + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'Offline mode is not enabled for this course'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + def test_admin_enabled_waffle_flag_no_course_id(self): + self.staff_login() + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'course_id is required'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=False) + def test_admin_disabled_waffle_flag_no_course_id(self): + self.staff_login() + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'course_id is required'}) + + def test_invalid_course_id(self): + self.staff_login() + response = self.client.post(self.url, {'course_id': 'invalid_course_id'}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'Invalid course_id'}) From 522034f79fdbf61078d1899e0786d3942dd438af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 13 Jun 2024 11:18:44 +0300 Subject: [PATCH 3/4] fix: [AXM-644] add problem type to supported types --- cms/djangoapps/contentstore/utils.py | 2 +- openedx/features/offline_mode/constants.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 82269214af69..c86accc22fb6 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2326,7 +2326,7 @@ def get_xblock_render_context(request, block): def get_cms_api_client(): """ - Returns an API client which can be used to make Exams API requests. + Returns an API client which can be used to make requests from the CMS service. """ user = User.objects.get(username=settings.EDXAPP_CMS_SERVICE_USER_NAME) jwt = create_jwt_for_user(user) diff --git a/openedx/features/offline_mode/constants.py b/openedx/features/offline_mode/constants.py index a1337a7029c1..771b341505aa 100644 --- a/openedx/features/offline_mode/constants.py +++ b/openedx/features/offline_mode/constants.py @@ -9,7 +9,7 @@ MATHJAX_CDN_URL = f'https://cdn.jsdelivr.net/npm/mathjax@{MATHJAX_VERSION}/MathJax.js' MATHJAX_STATIC_PATH = os.path.join('assets', 'js', f'MathJax-{MATHJAX_VERSION}.js') -DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html'] +DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html', 'problem'] OFFLINE_SUPPORTED_XBLOCKS = getattr(settings, 'OFFLINE_SUPPORTED_XBLOCKS', DEFAULT_OFFLINE_SUPPORTED_XBLOCKS) RETRY_BACKOFF_INITIAL_TIMEOUT = 60 # one minute From c8ff00aa1d549986856f614c12f33b4b024d4443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Mon, 17 Jun 2024 10:37:34 +0300 Subject: [PATCH 4/4] style: [AXM-644] add missing docstrings --- openedx/features/offline_mode/assets_management.py | 2 +- openedx/features/offline_mode/tests/test_views.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index 3594d31c24c0..38462a8e3ccc 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -69,7 +69,7 @@ def save_asset_file(temp_dir, xblock, path, filename): def create_subdirectories_for_asset(file_path): """ - Creates subdirectories for asset. + Creates the subdirectories for the asset file path if they do not exist. """ out_dir_name = '/' for dir_name in file_path.split('/')[:-1]: diff --git a/openedx/features/offline_mode/tests/test_views.py b/openedx/features/offline_mode/tests/test_views.py index 87fea7c79268..a4478d8796ab 100644 --- a/openedx/features/offline_mode/tests/test_views.py +++ b/openedx/features/offline_mode/tests/test_views.py @@ -1,3 +1,6 @@ +""" +Tests for view that handles course published event. +""" from unittest.mock import patch from django.test import TestCase, RequestFactory