PB-1351 Filter the logged headers

Provide a white list that can be overwritten by the environment for which headers we allow to go to the log
geoadmin · Jan 28, 2025 · 85a3a25 · 85a3a25
1 parent 8c0d075
commit 85a3a25
Show file tree

Hide file tree

Showing 4 changed files with 202 additions and 112 deletions.
diff --git a/app/config/api.py b/app/config/api.py
@@ -1,12 +1,6 @@
-import sys
-from logging import getLogger
-from typing import Any
-from typing import List
-from typing import Optional
-from typing import TypedDict
-
 from access.api import router as access_router
 from botocore.exceptions import EndpointConnectionError
+from config.logging import LoggedNinjaAPI
 from distributions.api import router as distributions_router
 from ninja import NinjaAPI
 from ninja.errors import AuthenticationError
@@ -22,111 +16,6 @@
 from django.http import HttpRequest
 from django.http import HttpResponse
 
-logger = getLogger(__name__)
-
-LogExtra = TypedDict(
-    'LogExtra',
-    {
-        'http': {
-            'request': {
-                'method': str, 'header': dict[str, str]
-            },
-            'response': {
-                'status_code': int, 'header': dict[str, str]
-            }
-        },
-        'url': {
-            'path': str, 'scheme': str
-        }
-    }
-)
-
-
-def generate_log_extra(request: HttpRequest, response: HttpResponse) -> LogExtra:
-    """Generate the extra dict for the logging calls
-
-    This will format the following extra fields to be sent to the logger:
-
-    request:
-        http:
-            request:
-                method: GET | POST | PUT | ...
-                header: LIST OF HEADERS
-            response:
-                header: LIST OF HEADERS
-                status_code: STATUS_CODE
-
-    url:
-        path: REQUEST_PATH
-        scheme: REQUEST_SCHEME
-
-    Args:
-        request (HttpRequest): Request object
-        response (HttpResponse): Response object
-
-    Returns:
-        dict: dict of extras
-    """
-    return {
-        'http': {
-            'request': {
-                'method': request.method or 'UNKNOWN',
-                'header': {
-                    k.lower(): v for k, v in request.headers.items()
-                }
-            },
-            'response': {
-                'status_code': response.status_code,
-                'header': {
-                    k.lower(): v for k, v in response.headers.items()
-                },
-            }
-        },
-        'url': {
-            'path': request.path or 'UNKNOWN', 'scheme': request.scheme or 'UNKNOWN'
-        }
-    }
-
-
-class LoggedNinjaAPI(NinjaAPI):
-    """Extension for the NinjaAPI to log the requests to elastic
-
-    Overwriting the method that creates a response. The only thing done then
-    is that depending on the status, a log entry will be triggered.
-    """
-
-    def create_response(
-        self,
-        request: HttpRequest,
-        data: Any,
-        *args: List[Any],
-        status: Optional[int] = None,
-        temporal_response: Optional[HttpResponse] = None,
-    ) -> HttpResponse:
-        response = super().create_response(
-            request, data, *args, status=status, temporal_response=temporal_response
-        )
-
-        if response.status_code >= 200 and response.status_code < 400:
-            logger.info(
-                "Response %s on %s",
-                response.status_code,  # parameter for %s
-                request.path,  # parameter for %s
-                extra=generate_log_extra(request, response)
-            )
-        elif response.status_code >= 400 and response.status_code < 500:
-            logger.warning(
-                "Response %s on %s",
-                response.status_code,  # parameter for %s
-                request.path,  # parameter for %s
-                extra=generate_log_extra(request, response)
-            )
-        else:
-            logger.exception(repr(sys.exc_info()[1]), extra=generate_log_extra(request, response))
-
-        return response
-
-
 api = LoggedNinjaAPI()
 
 api.add_router("", provider_router)

diff --git a/app/config/logging.py b/app/config/logging.py
@@ -0,0 +1,118 @@
+import sys
+from logging import getLogger
+from typing import Any
+from typing import List
+from typing import Optional
+from typing import TypedDict
+
+from ninja import NinjaAPI
+
+from django.conf import settings
+from django.http import HttpRequest
+from django.http import HttpResponse
+
+logger = getLogger(__name__)
+
+LogExtra = TypedDict(
+    'LogExtra',
+    {
+        'http': {
+            'request': {
+                'method': str, 'header': dict[str, str]
+            },
+            'response': {
+                'status_code': int, 'header': dict[str, str]
+            }
+        },
+        'url': {
+            'path': str, 'scheme': str
+        }
+    }
+)
+
+
+def generate_log_extra(request: HttpRequest, response: HttpResponse) -> LogExtra:
+    """Generate the extra dict for the logging calls
+
+    This will format the following extra fields to be sent to the logger:
+
+    request:
+        http:
+            request:
+                method: GET | POST | PUT | ...
+                header: LIST OF HEADERS
+            response:
+                header: LIST OF HEADERS
+                status_code: STATUS_CODE
+
+    url:
+        path: REQUEST_PATH
+        scheme: REQUEST_SCHEME
+
+    Args:
+        request (HttpRequest): Request object
+        response (HttpResponse): Response object
+
+    Returns:
+        dict: dict of extras
+    """
+    return {
+        'http': {
+            'request': {
+                'method': request.method or 'UNKNOWN',
+                'header': {
+                    k.lower(): v for k,
+                    v in request.headers.items() if k.lower() in settings.LOG_ALLOWED_HEADERS
+                }
+            },
+            'response': {
+                'status_code': response.status_code,
+                'header': {
+                    k.lower(): v for k,
+                    v in response.headers.items() if k.lower() in settings.LOG_ALLOWED_HEADERS
+                },
+            }
+        },
+        'url': {
+            'path': request.path or 'UNKNOWN', 'scheme': request.scheme or 'UNKNOWN'
+        }
+    }
+
+
+class LoggedNinjaAPI(NinjaAPI):
+    """Extension for the NinjaAPI to log the requests to elastic
+
+    Overwriting the method that creates a response. The only thing done then
+    is that depending on the status, a log entry will be triggered.
+    """
+
+    def create_response(
+        self,
+        request: HttpRequest,
+        data: Any,
+        *args: List[Any],
+        status: Optional[int] = None,
+        temporal_response: Optional[HttpResponse] = None,
+    ) -> HttpResponse:
+        response = super().create_response(
+            request, data, *args, status=status, temporal_response=temporal_response
+        )
+
+        if response.status_code >= 200 and response.status_code < 400:
+            logger.info(
+                "Response %s on %s",
+                response.status_code,  # parameter for %s
+                request.path,  # parameter for %s
+                extra=generate_log_extra(request, response)
+            )
+        elif response.status_code >= 400 and response.status_code < 500:
+            logger.warning(
+                "Response %s on %s",
+                response.status_code,  # parameter for %s
+                request.path,  # parameter for %s
+                extra=generate_log_extra(request, response)
+            )
+        else:
+            logger.exception(repr(sys.exc_info()[1]), extra=generate_log_extra(request, response))
+
+        return response
diff --git a/app/config/settings_base.py b/app/config/settings_base.py
@@ -184,3 +184,52 @@ def get_logging_config() -> dict[str, object]:
 
 
 LOGGING = get_logging_config()
+
+# list of headers that are allowed to be logged
+_DEFAULT_LOG_ALLOWED_HEADERS = [
+
+    # Standard headers
+    "accept",
+    "accept-encoding",
+    "accept-language",
+    "accept-ranges",
+    "cache-control",
+    "connection",
+    "content-length",
+    "content-security-policy",
+    "content-type",
+    "etag",
+    "host",
+    "if-match",
+    "if-none-match",
+    "origin",
+    "referer",
+    "referrer-policy",
+    "transfer-encoding",
+    "user-agent",
+    "vary",
+    "x-content-type-options",
+    "x-forwarded-for",
+    "x-forwarded-host",
+    "x-forwarded-port",
+    "x-forwarded-proto",
+
+  # Cloudfront headers
+    "cloudfront-is-android-viewer",
+    "cloudfront-is-desktop-viewer",
+    "cloudfront-is-ios-viewer",
+    "cloudfront-is-mobile-viewer",
+    "cloudfront-is-smarttv-viewer",
+    "cloudfront-is-tablet-viewer",
+
+  # PPBGDI headers
+    "x-e2e-testing",
+    # API GW Headers
+    "geoadmin-authenticated"
+    "geoadmin-username",
+    "apigw-requestid"
+]
+LOG_ALLOWED_HEADERS = [
+    str(header).lower()
+    for header in env.list('LOG_ALLOWED_HEADERS', default=_DEFAULT_LOG_ALLOWED_HEADERS)
+]
diff --git a/app/tests/config/test_request_logging.py b/app/tests/config/test_request_logging.py
@@ -7,14 +7,48 @@
 
 import mock_api  # pylint: disable=unused-import
 import pytest
+from config.logging import generate_log_extra
 from ecs_logging import StdlibFormatter
 
+from django.http import HttpRequest
+from django.http import HttpResponse
+
 
 @pytest.fixture(name='configure_logger')
 def fixture_configure_logger(caplog):
     caplog.handler.setFormatter(StdlibFormatter())
 
 
+def test_generate_log_extra(settings):
+    settings.LOG_ALLOWED_HEADERS = [h.lower for h in ['Content-Type', 'Lebowski', 'x-apigw-id']]
+
+    request = HttpRequest()
+    # overwrite the headers
+    request.headers = {
+        'opinion': 'just like yours',
+        'content-type': 'bowling',
+        'secret-header': 'remove this',
+        'Lebowski': 'Jeffrey'
+    }
+
+    response = HttpResponse()
+    response['Content-type'] = "Nihilism"
+    response['Set-Cookie'] = "Rug"
+
+    out = generate_log_extra(request, response)
+
+    request_headers = out['http']['request']['header'].keys()
+    response_headers = out['http']['response']['header'].keys()
+
+    assert 'content-type' in request_headers
+    assert 'lebowski' in request_headers
+    assert 'secret-header' not in request_headers
+    assert 'opinion' not in request_headers
+
+    assert 'content-type' in response_headers
+    assert 'set-cookie' not in response_headers
+
+
 def test_404_logging(client, caplog, configure_logger):
     path = '/api/v1/trigger-not-found'
     client.get(path)