From ca4006847b477773ebafa9eb3fc5cd0d420738d9 Mon Sep 17 00:00:00 2001
From: Brian Helba <brian.helba@kitware.com>
Date: Thu, 26 Jun 2025 17:04:17 -0400
Subject: [PATCH 1/2] Return falsy values from failed authentication, per Ninja
 expectations

---
 isic/zip_download/api.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/isic/zip_download/api.py b/isic/zip_download/api.py
index a8401d5d..27fc96a6 100644
--- a/isic/zip_download/api.py
+++ b/isic/zip_download/api.py
@@ -63,20 +63,20 @@ def authenticate(self, request: HttpRequest, username: str, password: str) -> Li
 class ZipDownloadTokenAuth(APIKeyQuery):
     param_name = "token"
 
-    def authenticate(self, request: HttpRequest, key: str | None) -> dict:
+    def authenticate(self, request: HttpRequest, key: str | None) -> dict | None:
         if not key:
-            raise AuthenticationError
+            return None
 
         try:
             token_dict = TimestampSigner().unsign_object(key, max_age=timedelta(days=1))
         except BadSignature:
-            raise AuthenticationError from None
+            return None
 
         token_dict["token"] = key
         return token_dict
 
 
-def zip_api_auth(request: HttpRequest):
+def zip_api_auth(request: HttpRequest) -> bool:
     """
     Protects the zip listing endpoint with basic auth.
 
@@ -88,10 +88,10 @@ def zip_api_auth(request: HttpRequest):
     """
     # the default auth argument for ninja routes checks if ANY of the backends validate,
     # but we want to check that ALL of them validate.
-    if not ZipDownloadBasicAuth()(request):
-        return False
-
-    return ZipDownloadTokenAuth()(request)
+    return all((
+        ZipDownloadBasicAuth()(request),
+        ZipDownloadTokenAuth()(request),
+    ))
 
 
 @csrf_exempt

From 227c0fe230ab59ba6702dfb354ffffdc0e80fe9c Mon Sep 17 00:00:00 2001
From: Dan LaManna <dan.lamanna@kitware.com>
Date: Fri, 18 Jul 2025 14:08:00 -0400
Subject: [PATCH 2/2] Return the result of ZipDownloadTokenAuth

---
 isic/zip_download/api.py                     | 12 ++++-----
 isic/zip_download/tests/test_zip_download.py | 26 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/isic/zip_download/api.py b/isic/zip_download/api.py
index 27fc96a6..97182e55 100644
--- a/isic/zip_download/api.py
+++ b/isic/zip_download/api.py
@@ -4,7 +4,7 @@
 import json
 import logging
 from pathlib import PurePosixPath
-from typing import TYPE_CHECKING, Literal
+from typing import TYPE_CHECKING, Any, Literal
 
 from botocore.signers import CloudFrontSigner
 from django.conf import settings
@@ -76,7 +76,7 @@ def authenticate(self, request: HttpRequest, key: str | None) -> dict | None:
         return token_dict
 
 
-def zip_api_auth(request: HttpRequest) -> bool:
+def zip_api_auth(request: HttpRequest) -> Any:
     """
     Protects the zip listing endpoint with basic auth.
 
@@ -88,10 +88,10 @@ def zip_api_auth(request: HttpRequest) -> bool:
     """
     # the default auth argument for ninja routes checks if ANY of the backends validate,
     # but we want to check that ALL of them validate.
-    return all((
-        ZipDownloadBasicAuth()(request),
-        ZipDownloadTokenAuth()(request),
-    ))
+    if not ZipDownloadBasicAuth()(request):
+        return False
+
+    return ZipDownloadTokenAuth()(request)
 
 
 @csrf_exempt
diff --git a/isic/zip_download/tests/test_zip_download.py b/isic/zip_download/tests/test_zip_download.py
index 322bf02a..185e6f2d 100644
--- a/isic/zip_download/tests/test_zip_download.py
+++ b/isic/zip_download/tests/test_zip_download.py
@@ -3,6 +3,8 @@
 from urllib.parse import ParseResult, parse_qs, urlparse
 
 from django.conf import settings
+from django.core.signing import TimestampSigner
+from ninja.errors import AuthenticationError
 import pytest
 
 from isic.core.models import Image
@@ -48,6 +50,30 @@ def zip_basic_auth():
     }
 
 
+@pytest.mark.django_db
+def test_zip_api_auth(mocker, authenticated_client, zip_basic_auth):
+    from isic.zip_download.api import zip_api_auth
+
+    token = TimestampSigner().sign_object({"token": "foo"})
+
+    request = mocker.MagicMock(
+        headers={"Authorization": "Basic " + b64encode(b":badcredentials").decode()}
+    )
+    with pytest.raises(AuthenticationError):
+        zip_api_auth(request)
+
+    request = mocker.MagicMock(
+        headers={"Authorization": zip_basic_auth["HTTP_AUTHORIZATION"]},
+        GET={"token": token},
+    )
+    assert zip_api_auth(request)
+
+    request = mocker.MagicMock(
+        headers={"Authorization": zip_basic_auth["HTTP_AUTHORIZATION"]}, GET={"token": "badtoken"}
+    )
+    assert not zip_api_auth(request)
+
+
 @pytest.mark.django_db(transaction=True)
 @pytest.mark.usefixtures("_random_images_with_licenses")
 def test_zip_download_licenses(authenticated_client, zip_basic_auth):