Skip to content

Commit

Permalink
feat: generalize proxy assets for use with every storage
Browse files Browse the repository at this point in the history
We do this so that custom storage does not need to be defined for Scorm
The default storage configured for Open edX should work out of the box with scorm now
This also fixes an issue of X-Frame-Options on the LMS when running in development mode
closes overhangio#87
  • Loading branch information
Danyal-Faheem committed Nov 6, 2024
1 parent c42ae34 commit 858a1f7
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 40 deletions.
29 changes: 14 additions & 15 deletions openedxscorm/scormxblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,11 @@ def assets_proxy(self, request, suffix):
Response object containing the content of the requested file with the appropriate content type.
"""
file_name = os.path.basename(suffix)
signed_url = self.storage.url(suffix)
if request.query_string:
signed_url = "&".join([signed_url, request.query_string])
file_path = self.find_file_path(file_name)
file_type, _ = mimetypes.guess_type(file_name)
with urllib.request.urlopen(signed_url) as response:
file_content = response.read()
with self.storage.open(file_path) as response:
file_content = response.read()


return Response(file_content, content_type=file_type)

Expand Down Expand Up @@ -420,16 +419,20 @@ def extract_package(self, package_file):
def index_page_url(self):
if not self.package_meta or not self.index_page_path:
return ""
folder = self.extract_folder_path
if self.storage.exists(
os.path.join(self.extract_folder_base_path, self.clean_path(self.index_page_path))
):
# For backward-compatibility, we must handle the case when the xblock data
# is stored in the base folder.
folder = self.extract_folder_base_path
logger.warning("Serving SCORM content from old-style path: %s", folder)
logger.warning("Serving SCORM content from old-style path: %s", self.extract_folder_base_path)

return f"{self.proxy_base_url}/{self.index_page_path}"

return self.storage.url(os.path.join(folder, self.index_page_path))
@property
def proxy_base_url(self):
return self.runtime.handler_url(
self, "assets_proxy"
).rstrip("?/")

@property
def extract_folder_path(self):
Expand Down Expand Up @@ -692,11 +695,7 @@ def find_titles_recursively(self, item, prefix, root):
f"{prefix}resources/{prefix}resource[@identifier='{item_identifier}']"
)
# Attach the storage path with the file path
resource_link = urllib.parse.unquote(
self.storage.url(
os.path.join(self.extract_folder_path, resource.get("href"))
)
)
resource_link = f"{self.proxy_base_url}/{resource.get('href')}"
if not children:
return [(sanitized_title, resource_link)]
child_titles = []
Expand Down Expand Up @@ -768,7 +767,7 @@ def find_file_path(self, filename):
Search recursively in the extracted folder for a given file. Path of the first
found file will be returned. Raise a ScormError if file cannot be found.
"""
path = self.get_file_path(filename, self.extract_folder_path)
path = self.get_file_path(filename, self.extract_folder_path) or self.get_file_path(filename, self.extract_folder_base_path)
if path is None:
raise ScormError(f"Invalid package: could not find '{filename}' file")
return path
Expand Down
25 changes: 0 additions & 25 deletions openedxscorm/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,6 @@ def __init__(
querystring_expire=querystring_expire,
)

def url(self, name, parameters=None, expire=None):
"""
Override url method of S3Boto3Storage
"""
if not self.querystring_auth:
# No need to use assets proxy when authentication is disabled
return super().url(name, parameters=parameters, expire=expire)

if name.startswith(self.xblock.extract_folder_path):
# Proxy assets serving through the `assets_proxy` view. This case should
# only ever happen when we attempt to serve the index page from the
# index_page_url method.
proxy_base_url = self.xblock.runtime.handler_url(
self.xblock, "assets_proxy"
).rstrip("?/")
# Note that we serve the index page here.
return f"{proxy_base_url}/{self.xblock.index_page_path}"

# This branch is executed when the `url` method is called from `assets_proxy`
return super().url(
os.path.join(self.xblock.extract_folder_path, name),
parameters=parameters,
expire=expire,
)


def s3(xblock):
"""
Expand Down

0 comments on commit 858a1f7

Please sign in to comment.