diff --git a/olx_importer/management/commands/load_components.py b/olx_importer/management/commands/load_components.py index 0f5ab66c..55cd268c 100644 --- a/olx_importer/management/commands/load_components.py +++ b/olx_importer/management/commands/load_components.py @@ -126,7 +126,6 @@ def create_content(self, static_local_path, now, component_version): component_version, content.id, key=key, - learner_downloadable=True, ) def import_block_type(self, block_type_name, now): # , publish_log_entry): @@ -177,7 +176,6 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry): component_version, text_content.pk, key="block.xml", - learner_downloadable=False ) # Cycle through static assets references and add those as well... diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 1521ec58..23b5c7c2 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -2,4 +2,4 @@ Open edX Learning ("Learning Core"). """ -__version__ = "0.16.3" +__version__ = "0.17.0" diff --git a/openedx_learning/apps/authoring/components/admin.py b/openedx_learning/apps/authoring/components/admin.py index edb83921..2fa501dd 100644 --- a/openedx_learning/apps/authoring/components/admin.py +++ b/openedx_learning/apps/authoring/components/admin.py @@ -71,7 +71,6 @@ def get_queryset(self, request): fields = [ "key", "format_size", - "learner_downloadable", "rendered_data", ] readonly_fields = [ diff --git a/openedx_learning/apps/authoring/components/api.py b/openedx_learning/apps/authoring/components/api.py index ace74fbb..48a45af4 100644 --- a/openedx_learning/apps/authoring/components/api.py +++ b/openedx_learning/apps/authoring/components/api.py @@ -215,7 +215,6 @@ def create_next_component_version( content_id=content_pk, component_version=component_version, key=key, - learner_downloadable=False, ) # Now copy any old associations that existed, as long as they aren't # in conflict with the new stuff or marked for deletion. @@ -227,7 +226,6 @@ def create_next_component_version( content_id=cvrc.content_id, component_version=component_version, key=cvrc.key, - learner_downloadable=cvrc.learner_downloadable, ) return component_version @@ -422,7 +420,6 @@ def create_component_version_content( content_id: int, /, key: str, - learner_downloadable: bool = False, ) -> ComponentVersionContent: """ Add a Content to the given ComponentVersion @@ -445,7 +442,6 @@ def create_component_version_content( component_version_id=component_version_id, content_id=content_id, key=key, - learner_downloadable=learner_downloadable, ) return cvrc @@ -453,7 +449,6 @@ def create_component_version_content( class AssetError(StrEnum): """Error codes related to fetching ComponentVersion assets.""" ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION = auto() - ASSET_NOT_LEARNER_DOWNLOADABLE = auto() ASSET_HAS_NO_DOWNLOAD_FILE = auto() @@ -484,7 +479,6 @@ def get_redirect_response_for_component_asset( component_version_uuid: UUID, asset_path: Path, public: bool = False, - learner_downloadable_only: bool = True, ) -> HttpResponse: """ ``HttpResponse`` for a reverse-proxy to serve a ``ComponentVersion`` asset. @@ -498,11 +492,6 @@ def get_redirect_response_for_component_asset( If ``True``, this will return an ``HttpResponse`` that can be cached in a CDN and shared across many clients. - :param learner_downloadable_only: Only return assets that are meant to be - downloadable by Learners, i.e. in the LMS experience. If this is - ``True``, then requests for assets that are not meant for student - download will return a ``404`` error response. - **Response Codes** If the asset exists for this ``ComponentVersion``, this function will return @@ -512,15 +501,10 @@ def get_redirect_response_for_component_asset( the ``ComponentVersion`` itself does not exist, the response code will be ``404``. - Other than checking the coarse-grained ``learner_downloadable_only`` flag, - *this function does not do auth checking of any sort*–it will never return + This function does not do auth checking of any sort. It will never return a ``401`` or ``403`` response code. That is by design. Figuring out who is making the request and whether they have permission to do so is the - responsiblity of whatever is calling this function. The - ``learner_downloadable_only`` flag is intended to be a filter for the entire - view. When it's True, not even staff can download component-internal assets. - This is intended to protect us from accidentally allowing sensitive grading - code to get leaked out. + responsiblity of whatever is calling this function. **Metadata Headers** @@ -596,24 +580,6 @@ def _error_header(error: AssetError) -> dict[str, str]: ) return HttpResponseNotFound(headers=info_headers) - # Check: If we're asking only for Learner Downloadable assets, and the asset - # in question is not supposed to be downloadable by learners, then we give a - # 404 error. Even staff members are not expected to be able to download - # these assets via the LMS endpoint that serves students. Studio would be - # expected to have an entirely different view to serve these assets in that - # context (along with different timeouts, auth, and cache settings). So in - # that sense, the asset doesn't exist for that particular endpoint. - if learner_downloadable_only and (not cv_content.learner_downloadable): - logger.error( - f"ComponentVersion {component_version_uuid} has asset {asset_path}, " - "but it is not meant to be downloadable by learners " - "(ComponentVersionContent.learner_downloadable=False)." - ) - info_headers.update( - _error_header(AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE) - ) - return HttpResponseNotFound(headers=info_headers) - # At this point, we know that there is valid Content that we want to send. # This adds Content-level headers, like the hash/etag and content type. info_headers.update(contents_api.get_content_info_headers(content)) diff --git a/openedx_learning/apps/authoring/components/migrations/0003_remove_componentversioncontent_learner_downloadable.py b/openedx_learning/apps/authoring/components/migrations/0003_remove_componentversioncontent_learner_downloadable.py new file mode 100644 index 00000000..4ae618b7 --- /dev/null +++ b/openedx_learning/apps/authoring/components/migrations/0003_remove_componentversioncontent_learner_downloadable.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.16 on 2024-11-06 17:14 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_components', '0002_alter_componentversioncontent_key'), + ] + + operations = [ + migrations.RemoveField( + model_name='componentversioncontent', + name='learner_downloadable', + ), + ] diff --git a/openedx_learning/apps/authoring/components/models.py b/openedx_learning/apps/authoring/components/models.py index df17a999..b0225fc9 100644 --- a/openedx_learning/apps/authoring/components/models.py +++ b/openedx_learning/apps/authoring/components/models.py @@ -254,43 +254,6 @@ class ComponentVersionContent(models.Model): # identifiers that don't map as cleanly to file paths at some point. key = key_field(db_column="_key") - # Long explanation for the ``learner_downloadable`` field: - # - # Is this Content downloadable during the learning experience? This is - # NOT about public vs. private permissions on course assets, as that will be - # a policy that can be changed independently of new versions of the content. - # For instance, a course team could decide to flip their course assets from - # private to public for CDN caching reasons, and that should not require - # new ComponentVersions to be created. - # - # What the ``learner_downloadable`` field refers to is whether this asset is - # supposed to *ever* be directly downloadable by browsers during the - # learning experience. This will be True for things like images, PDFs, and - # video transcript files. This field will be False for things like: - # - # * Problem Block OLX will contain the answers to the problem. The XBlock - # runtime and ProblemBlock will use this information to generate HTML and - # grade responses, but the the user's browser is never permitted to - # actually download the raw OLX itself. - # * Many courses include a python_lib.zip file holding custom Python code - # to be used by codejail to assess student answers. This code will also - # potentially reveal answers, and is never intended to be downloadable by - # the student's browser. - # * Some course teams will upload other file formats that their OLX is - # derived from (e.g. specially formatted LaTeX files). These files will - # likewise contain answers and should never be downloadable by the - # student. - # * Other custom metadata may be attached as files in the import, such as - # custom identifiers, author information, etc. - # - # Even if ``learner_downloadble`` is True, the LMS may decide that this - # particular student isn't allowed to see this particular piece of content - # yet–e.g. because they are not enrolled, or because the exam this Component - # is a part of hasn't started yet. That's a matter of LMS permissions and - # policy that is not intrinsic to the content itself, and exists at a layer - # above this. - learner_downloadable = models.BooleanField(default=False) - class Meta: constraints = [ # Uniqueness is only by ComponentVersion and key. If for some reason diff --git a/openedx_learning/contrib/media_server/views.py b/openedx_learning/contrib/media_server/views.py index 542f041a..d7088879 100644 --- a/openedx_learning/contrib/media_server/views.py +++ b/openedx_learning/contrib/media_server/views.py @@ -5,7 +5,7 @@ """ from pathlib import Path -from django.core.exceptions import ObjectDoesNotExist, PermissionDenied +from django.core.exceptions import ObjectDoesNotExist from django.http import FileResponse, Http404 from openedx_learning.apps.authoring.components.api import look_up_component_version_content @@ -34,11 +34,6 @@ def component_asset( except ObjectDoesNotExist: raise Http404("File not found") # pylint: disable=raise-missing-from - if not cvc.learner_downloadable and not ( - request.user and request.user.is_superuser - ): - raise PermissionDenied("This file is not publicly downloadable.") - response = FileResponse(cvc.raw_content.file, filename=Path(asset_path).name) response["Content-Type"] = cvc.raw_content.mime_type diff --git a/tests/openedx_learning/apps/authoring/components/test_api.py b/tests/openedx_learning/apps/authoring/components/test_api.py index 2a683b05..279a0335 100644 --- a/tests/openedx_learning/apps/authoring/components/test_api.py +++ b/tests/openedx_learning/apps/authoring/components/test_api.py @@ -388,7 +388,6 @@ def test_add(self): new_version.pk, new_content.pk, key="my/path/to/hello.txt", - learner_downloadable=False, ) # re-fetch from the database to check to see if we wrote it correctly new_version = components_api.get_component(self.problem.pk) \ @@ -405,7 +404,6 @@ def test_add(self): new_version.pk, new_content.pk, key="//nested/path/hello.txt", - learner_downloadable=False, ) new_version = components_api.get_component(self.problem.pk) \ .versions \ diff --git a/tests/openedx_learning/apps/authoring/components/test_assets.py b/tests/openedx_learning/apps/authoring/components/test_assets.py index 56bfda41..f9cbf064 100644 --- a/tests/openedx_learning/apps/authoring/components/test_assets.py +++ b/tests/openedx_learning/apps/authoring/components/test_assets.py @@ -75,7 +75,6 @@ def setUpTestData(cls) -> None: cls.component_version.pk, cls.problem_content.id, key="block.xml", - learner_downloadable=False, ) # Python source file, stored as a file. This is hypothetical, as we @@ -90,7 +89,6 @@ def setUpTestData(cls) -> None: cls.component_version.pk, cls.python_source_asset.id, key="src/grader.py", - learner_downloadable=False, ) # An HTML file that is student downloadable @@ -104,7 +102,6 @@ def setUpTestData(cls) -> None: cls.component_version.pk, cls.html_asset_content.id, key="static/hello.html", - learner_downloadable=True, ) def test_no_component_version(self): @@ -145,10 +142,6 @@ def test_404s_with_component_version_info(self): # This is testing that asset paths are case sensitive "static/HELLO.html": AssetError.ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION, - # Files that want to guarantee can never be downloaded (they're for - # backend usage only). - "src/grader.py": AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE, - # Text stored in the database directly instead of file storage. "block.xml": AssetError.ASSET_HAS_NO_DOWNLOAD_FILE, }