Add authentication requirement to /bugzilla_webhook endpoint (#828)

* Add token authentication to Webhook endpoint

* Create `authenticated_client` fixture

Use client fixtures in tests, rather than manually setting up clients

* Update .secrets.baseline

Author: grahamalama

.env.example

@@ -4,6 +4,7 @@ LOG_FORMAT=text
 LOG_LEVEL=debug
 APP_RELOAD=True
 APP_DEBUG=True
+JBI_API_KEY="fake_api_key"
 
 # Jira API Secrets
 JIRA_USERNAME="fake_jira_username"

.secrets.baseline

@@ -101,21 +101,28 @@
   ],
   "results": {
     ".env.example": [
+      {
+        "type": "Secret Keyword",
+        "filename": ".env.example",
+        "hashed_secret": "7dfe63b6762fc69b8e486a2bafa43b8f7d23b788",
+        "is_verified": false,
+        "line_number": 7
+      },
       {
         "type": "Secret Keyword",
         "filename": ".env.example",
         "hashed_secret": "4b9a4ce92b6a01a4cd6ee1672d31c043f2ae79ab",
         "is_verified": false,
-        "line_number": 10
+        "line_number": 11
       },
       {
         "type": "Secret Keyword",
         "filename": ".env.example",
         "hashed_secret": "77ea6398f252999314d609a708842a49fc43e055",
         "is_verified": false,
-        "line_number": 13
+        "line_number": 14
       }
     ]
   },
-  "generated_at": "2023-04-25T15:55:52Z"
+  "generated_at": "2024-01-29T20:59:20Z"
 }

jbi/environment.py

@@ -31,6 +31,7 @@ class Settings(BaseSettings):
     max_retries: int = 3
     # https://github.com/python/mypy/issues/12841
     env: Environment = Environment.NONPROD  # type: ignore
+    jbi_api_key: str
 
     # Jira
     jira_base_url: str = "https://mozit-test.atlassian.net/"

jbi/router.py

@@ -1,12 +1,14 @@
 """
 Core FastAPI app (setup, middleware)
 """
+import secrets
 from pathlib import Path
 from typing import Annotated, Optional
 
-from fastapi import APIRouter, Body, Depends, Request, Response
+from fastapi import APIRouter, Body, Depends, HTTPException, Request, Response, status
 from fastapi.encoders import jsonable_encoder
 from fastapi.responses import HTMLResponse
+from fastapi.security import APIKeyHeader
 from fastapi.templating import Jinja2Templates
 
 from jbi import bugzilla, jira
@@ -20,6 +22,7 @@
 VersionDep = Annotated[dict, Depends(get_version)]
 BugzillaServiceDep = Annotated[bugzilla.BugzillaService, Depends(bugzilla.get_service)]
 JiraServiceDep = Annotated[jira.JiraService, Depends(jira.get_service)]
+
 router = APIRouter()
 
 
@@ -72,7 +75,22 @@ def version(version_json: VersionDep):
     return version_json
 
 
-@router.post("/bugzilla_webhook")
+header_scheme = APIKeyHeader(name="X-Api-Key")
+
+
+def api_key_auth(
+    settings: SettingsDep, api_key: Annotated[str, Depends(header_scheme)]
+):
+    if not secrets.compare_digest(api_key, settings.jbi_api_key):
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED, detail="Forbidden"
+        )
+
+
+@router.post(
+    "/bugzilla_webhook",
+    dependencies=[Depends(api_key_auth)],
+)
 def bugzilla_webhook(
     request: Request,
     actions: ActionsDep,

tests/conftest.py

@@ -78,6 +78,13 @@ def anon_client():
     return TestClient(app)
 
 
+@pytest.fixture
+def authenticated_client():
+    """An test client with a valid API key."""
+    # api key for tests defined in .env.example
+    return TestClient(app, headers={"X-Api-Key": "fake_api_key"})
+
+
 @pytest.fixture
 def settings():
     """A test Settings object"""

tests/unit/test_app.py

@@ -2,9 +2,8 @@
 from unittest.mock import patch
 
 import pytest
-from fastapi.testclient import TestClient
 
-from jbi.app import app, traces_sampler
+from jbi.app import traces_sampler
 from jbi.environment import get_settings
 
 
@@ -36,12 +35,15 @@ def test_request_summary_defaults_user_agent_to_empty_string(caplog, anon_client
         assert summary.agent == ""
 
 
-def test_422_errors_are_logged(webhook_request_factory, caplog):
+def test_422_errors_are_logged(authenticated_client, webhook_request_factory, caplog):
     webhook = webhook_request_factory.build(bug=None)
 
-    with TestClient(app) as anon_client:
-        with caplog.at_level(logging.INFO):
-            anon_client.post("/bugzilla_webhook", data=webhook.model_dump_json())
+    with caplog.at_level(logging.INFO):
+        authenticated_client.post(
+            "/bugzilla_webhook",
+            headers={"X-Api-Key": "fake_api_key"},
+            data=webhook.model_dump_json(),
+        )
 
     logged = [r for r in caplog.records if r.name == "jbi.app"][0]
     assert logged.errors[0]["loc"] == ("body", "bug")
@@ -82,7 +84,9 @@ def test_errors_are_reported_to_sentry(anon_client, bugzilla_webhook_request):
         with patch("jbi.router.execute_action", side_effect=ValueError):
             with pytest.raises(ValueError):
                 anon_client.post(
-                    "/bugzilla_webhook", data=bugzilla_webhook_request.model_dump_json()
+                    "/bugzilla_webhook",
+                    headers={"X-Api-Key": "fake_api_key"},
+                    data=bugzilla_webhook_request.model_dump_json(),
                 )
 
     assert mocked.called, "Sentry captured the exception"
@@ -91,6 +95,7 @@ def test_errors_are_reported_to_sentry(anon_client, bugzilla_webhook_request):
 def test_request_id_is_passed_down_to_logger_contexts(
     caplog,
     bugzilla_webhook_request,
+    authenticated_client,
     mocked_jira,
     mocked_bugzilla,
 ):
@@ -99,14 +104,13 @@ def test_request_id_is_passed_down_to_logger_contexts(
         "key": "JBI-1922",
     }
     with caplog.at_level(logging.DEBUG):
-        with TestClient(app) as anon_client:
-            anon_client.post(
-                "/bugzilla_webhook",
-                data=bugzilla_webhook_request.model_dump_json(),
-                headers={
-                    "X-Request-Id": "foo-bar",
-                },
-            )
+        authenticated_client.post(
+            "/bugzilla_webhook",
+            data=bugzilla_webhook_request.model_dump_json(),
+            headers={
+                "X-Request-Id": "foo-bar",
+            },
+        )
 
     runner_logs = [r for r in caplog.records if r.name == "jbi.runner"]
     assert runner_logs[0].rid == "foo-bar"

tests/unit/test_router.py

@@ -4,9 +4,7 @@
 from unittest import mock
 
 import pytest
-from fastapi.testclient import TestClient
 
-from jbi.app import app
 from jbi.environment import get_settings
 
 
@@ -80,9 +78,7 @@ def test_statics_are_served(anon_client):
 
 
 def test_webhook_is_200_if_action_succeeds(
-    bugzilla_webhook_request,
-    mocked_jira,
-    mocked_bugzilla,
+    bugzilla_webhook_request, mocked_jira, mocked_bugzilla, authenticated_client
 ):
     mocked_bugzilla.get_bug.return_value = bugzilla_webhook_request.bug
     mocked_bugzilla.update_bug.return_value = {
@@ -106,38 +102,62 @@ def test_webhook_is_200_if_action_succeeds(
         "self": f"{get_settings().jira_base_url}rest/api/2/issue/JBI-1922/remotelink/18936",
     }
 
-    with TestClient(app) as anon_client:
-        response = anon_client.post(
-            "/bugzilla_webhook", data=bugzilla_webhook_request.model_dump_json()
-        )
-        assert response
-        assert response.status_code == 200
+    response = authenticated_client.post(
+        "/bugzilla_webhook",
+        data=bugzilla_webhook_request.model_dump_json(),
+    )
+    assert response
+    assert response.status_code == 200
 
 
 def test_webhook_is_200_if_action_raises_IgnoreInvalidRequestError(
-    webhook_request_factory,
-    mocked_bugzilla,
+    webhook_request_factory, mocked_bugzilla, authenticated_client
 ):
     webhook = webhook_request_factory(bug__whiteboard="unmatched")
     mocked_bugzilla.get_bug.return_value = webhook.bug
 
-    with TestClient(app) as anon_client:
-        response = anon_client.post("/bugzilla_webhook", data=webhook.model_dump_json())
-        assert response
-        assert response.status_code == 200
-        assert (
-            response.json()["error"]
-            == "no bug whiteboard matching action tags: devtest"
-        )
+    response = authenticated_client.post(
+        "/bugzilla_webhook",
+        data=webhook.model_dump_json(),
+    )
+    assert response
+    assert response.status_code == 200
+    assert response.json()["error"] == "no bug whiteboard matching action tags: devtest"
+
+
+def test_webhook_is_401_if_unathenticated(
+    webhook_request_factory, mocked_bugzilla, anon_client
+):
+    response = anon_client.post(
+        "/bugzilla_webhook",
+        data={},
+    )
+    assert response.status_code == 403
+
+
+def test_webhook_is_401_if_wrong_key(
+    webhook_request_factory, mocked_bugzilla, anon_client
+):
+    response = anon_client.post(
+        "/bugzilla_webhook",
+        headers={"X-Api-Key": "not the right key"},
+        data={},
+    )
+    assert response.status_code == 401
 
 
-def test_webhook_is_422_if_bug_information_missing(webhook_request_factory):
+def test_webhook_is_422_if_bug_information_missing(
+    webhook_request_factory, authenticated_client
+):
     webhook = webhook_request_factory.build(bug=None)
 
-    with TestClient(app) as anon_client:
-        response = anon_client.post("/bugzilla_webhook", data=webhook.model_dump_json())
-        assert response.status_code == 422
-        assert response.json()["detail"][0]["loc"] == ["body", "bug"]
+    response = authenticated_client.post(
+        "/bugzilla_webhook",
+        headers={"X-Api-Key": "fake_api_key"},
+        data=webhook.model_dump_json(),
+    )
+    assert response.status_code == 422
+    assert response.json()["detail"][0]["loc"] == ["body", "bug"]
 
 
 def test_read_version(anon_client):