Refactor TokenValidator

Moves some attributes out of the constructor which makes testing easier.

Author: steventux

manage_breast_screening/dicom/tests/test_api.py

@@ -6,6 +6,7 @@
 import pytest
 from django.core.files.uploadedfile import SimpleUploadedFile
 from ninja.testing import TestClient
+from pydicom.uid import generate_uid
 
 from manage_breast_screening.core.api import api
 from manage_breast_screening.gateway.models import GatewayActionStatus
@@ -21,8 +22,10 @@
 
 
 @pytest.fixture(autouse=True)
-def enable_api(monkeypatch):
+def setup(monkeypatch):
     monkeypatch.setenv("API_ENABLED", "true")
+    monkeypatch.setenv("API_AUDIENCE", "test_audience")
+    monkeypatch.setenv("TENANT_ID", "test_tenant_id")
 
 
 @pytest.fixture
@@ -181,6 +184,26 @@ def test_upload_invalid_auth(dicom_file):
     }
 
 
+def test_upload_bypass_token_validation(dicom_file):
+    with patch.object(TokenValidator, "bypass_authentication", return_value=True):
+        with patch.object(
+            DicomRecorder,
+            "get_or_create_records",
+            return_value=(
+                MagicMock(study_instance_uid=generate_uid()),
+                MagicMock(series_instance_uid=generate_uid()),
+                MagicMock(sop_instance_uid=generate_uid(), id=1),
+            ),
+        ):
+            response = client.put(
+                "/dicom/abc123",
+                FILES={"file": dicom_file},
+                headers={"Authorization": "Bearer anytoken"},
+            )
+
+    assert response.status_code == 201
+
+
 @pytest.mark.django_db
 def test_report_failure(mock_token_validator):
     action = GatewayActionFactory()

manage_breast_screening/dicom/tests/test_token_validator.py

@@ -3,6 +3,7 @@
 
 import jwt
 import pytest
+from django.conf import settings
 
 from ..token_validator import TokenValidator
 
@@ -133,7 +134,7 @@ def test_with_valid_token(
         }
 
     def test_authentication_bypass_enabled(self, mock_logger):
-        with patch.dict("os.environ", {"BYPASS_API_TOKEN_AUTH": "true"}):
+        with patch.object(settings, "BYPASS_API_TOKEN_AUTH", return_value=True):
             validator = TokenValidator()
             assert validator(Mock(headers={"Authorization": "Bearer anytoken"})) == {
                 "sub": "bypass_user"

manage_breast_screening/dicom/token_validator.py

@@ -4,6 +4,7 @@
 from urllib.request import urlopen
 
 import jwt
+from django.conf import settings
 from ninja.security import HttpBearer
 
 logger = logging.getLogger(__name__)
@@ -12,19 +13,8 @@
 
 
 class TokenValidator(HttpBearer):
-    def __init__(self):
-        self.bypass_auth = os.getenv("BYPASS_API_TOKEN_AUTH", "false").lower() == "true"
-        self.api_audience = os.environ["API_AUDIENCE"]
-        self.tenant_id = os.environ["TENANT_ID"]
-        self.discovery_keys_url = (
-            "https://login.microsoftonline.com/"
-            + self.tenant_id
-            + "/discovery/v2.0/keys"
-        )
-        self.issuer_url = "https://sts.windows.net/" + self.tenant_id + "/"
-
     def authenticate(self, request, token) -> dict | None:
-        if self.bypass_auth:
+        if self.bypass_authentication:
             logger.warning("Authentication bypass is enabled.")
             return {"sub": "bypass_user"}
 
@@ -49,16 +39,16 @@ def _rsa_key(self, token) -> dict | None:
                         "e": key["e"],
                     }
         except Exception:
-            logger.error("Unable to parse authentication token.", exc_info=True)
+            logger.exception("Unable to parse authentication token.")
 
     def _decode(self, token: str, rsa_key: dict) -> dict | None:
         try:
             payload = jwt.decode(
                 token,
                 rsa_key,
                 algorithms=ALLOWED_ALGORITHMS,
-                audience=self.api_audience,
-                issuer=self.issuer_url,
+                audience=os.getenv("API_AUDIENCE"),
+                issuer=f"https://sts.windows.net/{self.tenant_id}/",
             )
             return payload
         except jwt.ExpiredSignatureError:
@@ -69,3 +59,15 @@ def _decode(self, token: str, rsa_key: dict) -> dict | None:
             logger.exception("Token is invalid")
         except Exception:
             logger.exception("Unable to parse authentication token.")
+
+    @property
+    def discovery_keys_url(self) -> str:
+        return f"https://login.microsoftonline.com/{self.tenant_id}/discovery/v2.0/keys"
+
+    @property
+    def tenant_id(self) -> str | None:
+        return os.getenv("TENANT_ID", "")
+
+    @property
+    def bypass_authentication(self) -> bool:
+        return getattr(settings, "BYPASS_API_TOKEN_AUTH", False)