Add support for rendering images

airlock/api.py

@@ -52,6 +52,9 @@
     def get_url(self, path: UrlPath = ROOT_PATH) -> str:
         """Get the url for the container object with path"""
 
+    def get_contents_url(self, path: UrlPath = ROOT_PATH) -> str:
+        """Get the url for the container object with path"""
+
 
 @dataclass
 class Workspace:
@@ -82,6 +85,12 @@
             kwargs={"workspace_name": self.name, "path": relpath},
         )
 
+    def get_contents_url(self, relpath):
+        return reverse(
+            "workspace_contents",
+            kwargs={"workspace_name": self.name, "path": relpath},
+        )
+
     def abspath(self, relpath):
         """Get absolute path for file
 
@@ -90,10 +99,10 @@
         path = root / relpath
 
         # protect against traversal
         path.resolve().relative_to(root)
 
         # validate path exists
         if not path.exists():
             raise ProviderAPI.FileNotFound(path)
 
         return path
@@ -159,6 +168,12 @@
             },
         )
 
+    def get_contents_url(self, relpath):
+        return reverse(
+            "request_contents",
+            kwargs={"request_id": self.id, "path": relpath},
+        )
+
     def abspath(self, relpath):
         """Returns abspath to the file on disk.
 
@@ -176,10 +191,10 @@
         path = root / filepath
 
         # protect against traversal
         path.resolve().relative_to(root)
 
         # validate path exists
         if not path.exists():
             raise ProviderAPI.FileNotFound(path)
 
         return path

airlock/file_browser_api.py

@@ -47,6 +47,18 @@ class PathNotFound(Exception):
     # but this allow it to be overridden.
     display_text: str = None
 
+    DISPLAY_TYPES = {
+        "html": "iframe",
+        "jpeg": "image",
+        "jpg": "image",
+        "png": "image",
+        "svg": "image",
+        "csv": "table",
+        "tsv": "table",
+        "txt": "preformatted",
+        "log": "preformatted",
+    }
+
     def __post_init__(self):
         # ensure is UrlPath
         self.relpath = UrlPath(self.relpath)
@@ -74,6 +86,11 @@ def url(self):
         suffix = "/" if self.is_directory() else ""
         return self.container.get_url(f"{self.relpath}{suffix}")
 
+    def contents_url(self):
+        if self.type != PathType.FILE:
+            raise Exception(f"contents_url called on non-file path {self.relpath}")
+        return self.container.get_contents_url(f"{self.relpath}")
+
     def siblings(self):
         if not self.relpath.parents:
             return []
@@ -94,6 +111,9 @@ def suffix(self):
     def file_type(self):
         return self.suffix().lstrip(".")
 
+    def display_type(self):
+        return self.DISPLAY_TYPES.get(self.file_type(), "preformatted")
+
     def breadcrumbs(self):
         item = self
         crumbs = [item]

airlock/settings.py

@@ -225,6 +225,8 @@ def run_connection_init_queries(*, connection, **kwargs):
 # Changing from the default allows us to share localhost port in developement
 SESSION_COOKIE_NAME = "airlock-sessionid"
 
+# Enable frames
+X_FRAME_OPTIONS = "SAMEORIGIN"
 
 # Serve files from static dirs directly. This removes the need to run collectstatic
 # https://whitenoise.readthedocs.io/en/latest/django.html#WHITENOISE_USE_FINDERS

airlock/templates/file_browser/index.html

@@ -55,13 +55,17 @@
     .content {
       width:100%;
       max-width: 100%;
+    }
+
+    .content-scroller {
       height: 75vh;
       overflow: scroll;
     }
 
     .datatable thead {
       position: sticky;
       top: 0;
+      text-align: left;
     }
 
 
@@ -178,19 +182,27 @@
           {% #card title=path_item.name container=True custom_button=add_button %}
 
             <div class="content">
-              {% if path_item.suffix == '.html' %}
+              {% if path_item.display_type == "iframe" %}
                 <iframe
-                  srcdoc="{{ path_item.contents }}"
+                  src="{{ path_item.contents_url }}"
                   title="{{ path_item.relpath }}"
                   frameborder=0
                   height=1000
                   style="width: 100%;"
                 >
                 </iframe>
-              {% elif path_item.suffix == '.csv' %}
-                {% include "file_browser/csv.html" with contents=path_item.contents %}
+              {% elif path_item.display_type == "image" %}
+                <div class="content-scroller">
+                  <img src="{{ path_item.contents_url }}">
+                </div>
+              {% elif path_item.display_type == "table" %}
+                <div class="content-scroller">
+                  {% include "file_browser/csv.html" with contents=path_item.contents %}
+                </div>
               {% else %}
-            <pre>{{ path_item.contents }}</pre>
+                <div class="content-scroller">
+                <pre>{{ path_item.contents }}</pre>
+                </div>
               {% endif %}
             </div>
           {% /card %}
@@ -208,19 +220,19 @@
   {% django_htmx_script %}
 
   <script type="text/javascript">
-      // keep the selected class up to date in the tree on the client side
+    // keep the selected class up to date in the tree on the client side
     function setTreeSelection(target, event) {
-          // target here is the hx-get link that has been clicked on
+      // target here is the hx-get link that has been clicked on
 
-          // remove class from currently selected node
+      // remove class from currently selected node
       document.querySelector('#tree .selected')?.classList.remove('selected');
 
-          // set current selected
+      // set current selected
       target.classList.add('selected');
-          // ensure parent details container is open, which means clicking on a directory will open containers.
+      // ensure parent details container is open, which means clicking on a directory will open containers.
       target.closest("details").open = true;
 
-          // if target link is a filegroup, ensure all child <details> are opened, to match server-side rendering of tree
+      // if target link is a filegroup, ensure all child <details> are opened, to match server-side rendering of tree
       if (target.classList.contains("filegroup")) {
         target.closest("li.tree").querySelectorAll("details").forEach((e) => e.open = true)
       }

airlock/urls.py

@@ -43,6 +43,11 @@
         airlock.views.workspace_view,
         name="workspace_view",
     ),
+    path(
+        "workspaces/content/<str:workspace_name>/<path:path>",
+        airlock.views.workspace_contents,
+        name="workspace_contents",
+    ),
     path(
         "workspaces/add-file-to-request/<str:workspace_name>",
         airlock.views.workspace_add_file_to_request,
@@ -65,6 +70,11 @@
         airlock.views.request_view,
         name="request_view",
     ),
+    path(
+        "requests/content/<str:request_id>/<path:path>",
+        airlock.views.request_contents,
+        name="request_contents",
+    ),
     path(
         "requests/release/<str:request_id>",
         airlock.views.request_release_files,

airlock/views/__init__.py

@@ -1,6 +1,7 @@
 from .auth import login, logout
 from .index import index
 from .request import (
+    request_contents,
     request_index,
     request_reject,
     request_release_files,
@@ -9,6 +10,7 @@
 )
 from .workspace import (
     workspace_add_file_to_request,
+    workspace_contents,
     workspace_index,
     workspace_view,
 )

airlock/views/helpers.py

@@ -1,5 +1,7 @@
+from email.utils import formatdate
+
 from django.core.exceptions import PermissionDenied
-from django.http import Http404
+from django.http import FileResponse, Http404
 
 from local_db.api import LocalDBProvider
 
@@ -36,3 +38,13 @@
     validate_workspace(user, release_request.workspace)
 
     return release_request
+
+
+def serve_file(abspath):
+    stat = abspath.stat()
+    # use same ETag format as whitenoise
+    headers = {
+        "Last-Modified": formatdate(stat.st_mtime, usegmt=True),
+        "ETag": f'"{int(stat.st_mtime):x}-{stat.st_size:x}"',
+    }
+    return FileResponse(abspath.open("rb"), headers=headers)

airlock/views/request.py

@@ -2,7 +2,7 @@
 from django.conf import settings
 from django.contrib import messages
 from django.core.exceptions import PermissionDenied
-from django.http import Http404
+from django.http import Http404, HttpResponseBadRequest
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -12,7 +12,7 @@
 from airlock.file_browser_api import get_request_tree
 from local_db.api import LocalDBProvider
 
-from .helpers import validate_release_request
+from .helpers import serve_file, validate_release_request
 
 
 api = LocalDBProvider()
@@ -84,6 +84,21 @@
     return TemplateResponse(request, "file_browser/index.html", context)
 
 
+@require_http_methods(["GET"])
+def request_contents(request, request_id: str, path: str):
+    release_request = validate_release_request(request.user, request_id)
+
+    try:
+        abspath = release_request.abspath(path)
+    except api.FileNotFound:
+        raise Http404()
+
+    if not abspath.is_file():
+        return HttpResponseBadRequest()
+
+    return serve_file(abspath)
+
+
 @require_http_methods(["POST"])
 def request_submit(request, request_id):
     release_request = validate_release_request(request.user, request_id)

airlock/views/workspace.py

@@ -1,5 +1,5 @@
 from django.contrib import messages
-from django.http import Http404
+from django.http import Http404, HttpResponseBadRequest
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -10,7 +10,7 @@
 from airlock.forms import AddFileForm
 from local_db.api import LocalDBProvider
 
-from .helpers import validate_workspace
+from .helpers import serve_file, validate_workspace
 
 
 api = LocalDBProvider()
@@ -67,6 +67,21 @@
     )
 
 
+@require_http_methods(["GET"])
+def workspace_contents(request, workspace_name: str, path: str):
+    workspace = validate_workspace(request.user, workspace_name)
+
+    try:
+        abspath = workspace.abspath(path)
+    except api.FileNotFound:
+        raise Http404()
+
+    if not abspath.is_file():
+        return HttpResponseBadRequest()
+
+    return serve_file(abspath)
+
+
 @require_http_methods(["POST"])
 def workspace_add_file_to_request(request, workspace_name):
     workspace = validate_workspace(request.user, workspace_name)

justfile

@@ -185,6 +185,8 @@ load-example-data: devenv
     curl -s https://jobs.opensafely.org/opensafely-internal/tpp-database-schema/outputs/85/download/ --output "$tmp"
     unzip -u "$tmp" -d "$workspace"
 
+    cp example-data/bennett.svg $workspace/output/sample.svg
+
     request_dir="${AIRLOCK_WORK_DIR%/}/${AIRLOCK_REQUEST_DIR%/}/example-workspace/test-request"
     mkdir -p $request_dir
     cp -a $workspace/output $request_dir

tests/integration/views/test_helpers.py

@@ -0,0 +1,16 @@
+import os
+
+from airlock.views import helpers
+
+
+def test_serve_file(tmp_path):
+    path = tmp_path / "test.txt"
+    path.write_text("hi")
+    time = 1709652904  # date this test was written
+    os.utime(path, (time, time))
+
+    response = helpers.serve_file(path)
+    assert response.headers["Last-Modified"] == "Tue, 05 Mar 2024 15:35:04 GMT"
+    assert response.headers["Etag"] == '"65e73ba8-2"'
+    assert response.headers["Content-Type"] == "text/plain"
+    assert response.headers["Content-Length"] == "2"

tests/integration/views/test_request.py

@@ -123,6 +123,34 @@ def test_request_view_redirects_to_file(client_with_permission):
     )
 
 
+def test_request_contents_file(client_with_permission):
+    release_request = factories.create_release_request("workspace", id="id")
+    factories.write_request_file(
+        release_request, "default", "file.txt", contents="test"
+    )
+    response = client_with_permission.get("/requests/content/id/default/file.txt")
+    assert response.status_code == 200
+    assert list(response.streaming_content) == [b"test"]
+
+
+def test_request_contents_dir(client_with_permission):
+    release_request = factories.create_release_request("workspace", id="id")
+    factories.write_request_file(
+        release_request, "default", "foo/file.txt", contents="test"
+    )
+    response = client_with_permission.get("/requests/content/id/default/foo")
+    assert response.status_code == 400
+
+
+def test_request_contents_not_exists(client_with_permission):
+    release_request = factories.create_release_request("workspace", id="id")
+    factories.write_request_file(
+        release_request, "default", "foo/file.txt", contents="test"
+    )
+    response = client_with_permission.get("/requests/content/id/default/notexists.txt")
+    assert response.status_code == 404
+
+
 def test_request_index_user_permitted_requests(client_with_user):
     permitted_client = client_with_user({"workspaces": ["test1"]})
     user = User.from_session(permitted_client.session)