fix: redact the Authorization HTTP header from log (#462)

Author: bednar

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## 1.31.0 [unreleased]
 
+### Bug Fixes
+1. [#462](https://github.com/influxdata/influxdb-client-python/pull/462): Redact the `Authorization` HTTP header from log
+
 ## 1.30.0 [2022-06-24]
 
 ### Features

README.rst

@@ -1587,6 +1587,7 @@ The client uses uses Python's `logging <https://docs.python.org/3/library/loggin
 - ``influxdb_client.client.write.retry``
 - ``influxdb_client.client.write.dataframe_serializer``
 - ``influxdb_client.client.util.multiprocessing_helper``
+- ``influxdb_client.client.http``
 - ``influxdb_client.client.exceptions``
 
 The default logging level is `warning` without configured logger output. You can use the standard logger interface to change the log level and handler:

influxdb_client/_async/rest.py

@@ -17,22 +17,23 @@
 import aiohttp
 
 from influxdb_client.rest import ApiException
+from influxdb_client.rest import _BaseRESTClient
 from influxdb_client.rest import _UTF_8_encoding
 
 
 async def _on_request_start(session, trace_config_ctx, params):
-    print(f">>> Request: '{params.method} {params.url}'")
-    print(f">>> Headers: '{params.headers}'")
+    _BaseRESTClient.log_request(params.method, params.url)
+    _BaseRESTClient.log_headers(params.headers, '>>>')
 
 
 async def _on_request_chunk_sent(session, context, params):
     if params.chunk:
-        print(f">>> Body: {params.chunk}")
+        _BaseRESTClient.log_body(params.chunk, '>>>')
 
 
 async def _on_request_end(session, trace_config_ctx, params):
-    print(f"<<< Response: {params.response.status}")
-    print(f"<<< Headers: '{params.response.headers}")
+    _BaseRESTClient.log_response(params.response.status)
+    _BaseRESTClient.log_headers(params.headers, '<<<')
 
     response_content = params.response.content
     data = bytearray()
@@ -42,7 +43,7 @@ async def _on_request_end(session, trace_config_ctx, params):
             break
         data += chunk
     if data:
-        print(f"<<< Body: {data.decode(_UTF_8_encoding)}")
+        _BaseRESTClient.log_body(data.decode(_UTF_8_encoding), '<<<')
         response_content.unread_data(data=data)
 
 

influxdb_client/_sync/rest.py

@@ -19,6 +19,7 @@
 from urllib.parse import urlencode
 
 from influxdb_client.rest import ApiException
+from influxdb_client.rest import _BaseRESTClient
 
 try:
     import urllib3
@@ -164,6 +165,11 @@ def request(self, method, url, query_params=None, headers=None,
         if 'Content-Type' not in headers:
             headers['Content-Type'] = 'application/json'
 
+        if self.configuration.debug:
+            _BaseRESTClient.log_request(method, f"{url}?{urlencode(query_params)}")
+            _BaseRESTClient.log_headers(headers, '>>>')
+            _BaseRESTClient.log_body(body, '>>>')
+
         try:
             # For `POST`, `PUT`, `PATCH`, `OPTIONS`, `DELETE`
             if method in ['POST', 'PUT', 'PATCH', 'OPTIONS', 'DELETE']:
@@ -239,6 +245,14 @@ def request(self, method, url, query_params=None, headers=None,
             # we need to decode it to string.
             r.data = r.data.decode('utf8')
 
+        if self.configuration.debug:
+            _BaseRESTClient.log_response(r.status)
+            if hasattr(r, 'headers'):
+                _BaseRESTClient.log_headers(r.headers, '<<<')
+            if hasattr(r, 'urllib3_response'):
+                _BaseRESTClient.log_headers(r.urllib3_response.headers, '<<<')
+            _BaseRESTClient.log_body(r.data, '<<<')
+
         if not 200 <= r.status <= 299:
             raise ApiException(http_resp=r)
 

influxdb_client/client/_base.py

@@ -38,7 +38,8 @@
     'influxdb_client.client.write.retry',
     'influxdb_client.client.write.dataframe_serializer',
     'influxdb_client.client.util.multiprocessing_helper',
-    'influxdb_client.client.exceptions'
+    'influxdb_client.client.http',
+    'influxdb_client.client.exceptions',
 ]
 
 

influxdb_client/configuration.py

@@ -13,7 +13,6 @@
 from __future__ import absolute_import
 
 import copy
-import http.client as httplib
 import logging
 import multiprocessing
 import sys
@@ -164,17 +163,19 @@ def debug(self, value):
         self.__debug = value
         if self.__debug:
             # if debug status is True, turn on debug logging
-            for _, logger in self.loggers.items():
+            for name, logger in self.loggers.items():
                 logger.setLevel(logging.DEBUG)
-            # turn on httplib debug
-            httplib.HTTPConnection.debuglevel = 1
+                if name == 'influxdb_client.client.http':
+                    logger.addHandler(logging.StreamHandler(sys.stdout))
+            # we use 'influxdb_client.client.http' logger instead of this
+            # httplib.HTTPConnection.debuglevel = 1
         else:
             # if debug status is False, turn off debug logging,
             # setting log level to default `logging.WARNING`
             for _, logger in self.loggers.items():
                 logger.setLevel(logging.WARNING)
-            # turn off httplib debug
-            httplib.HTTPConnection.debuglevel = 0
+            # we use 'influxdb_client.client.http' logger instead of this
+            # httplib.HTTPConnection.debuglevel = 0
 
     @property
     def logger_format(self):

influxdb_client/rest.py

@@ -11,6 +11,9 @@
 
 from __future__ import absolute_import
 
+import logging
+from typing import Dict
+
 from influxdb_client.client.exceptions import InfluxDBError
 from influxdb_client.configuration import Configuration
 
@@ -52,6 +55,30 @@ def __str__(self):
         return error_message
 
 
+class _BaseRESTClient(object):
+    logger = logging.getLogger('influxdb_client.client.http')
+
+    @staticmethod
+    def log_request(method: str, url: str):
+        _BaseRESTClient.logger.debug(f">>> Request: '{method} {url}'")
+
+    @staticmethod
+    def log_response(status: str):
+        _BaseRESTClient.logger.debug(f"<<< Response: {status}")
+
+    @staticmethod
+    def log_body(body: object, prefix: str):
+        _BaseRESTClient.logger.debug(f"{prefix} Body: {body}")
+
+    @staticmethod
+    def log_headers(headers: Dict[str, str], prefix: str):
+        for key, v in headers.items():
+            value = v
+            if 'authorization' == key.lower():
+                value = '***'
+            _BaseRESTClient.logger.debug(f"{prefix} {key}: {value}")
+
+
 def _requires_create_user_session(configuration: Configuration, cookie: str, resource_path: str):
     _unauthorized = ['/api/v2/signin', '/api/v2/signout']
     return configuration.username and configuration.password and not cookie and resource_path not in _unauthorized

tests/test_InfluxDBClient.py

@@ -1,8 +1,10 @@
 import http.server
 import json
+import logging
 import os
 import threading
 import unittest
+from io import StringIO
 
 import httpretty
 import pytest
@@ -262,6 +264,21 @@ def test_init_without_token(self):
         self.assertIsNotNone(self.influxdb_client)
         self.influxdb_client.query_api().query("buckets()", "my-org")
 
+    def test_redacted_auth_header(self):
+        httpretty.register_uri(httpretty.POST, uri="http://localhost/api/v2/query", status=200, body="")
+        self.influxdb_client = InfluxDBClient("http://localhost", "my-token", debug=True)
+
+        log_stream = StringIO()
+        logger = logging.getLogger("influxdb_client.client.http")
+        logger.addHandler(logging.StreamHandler(log_stream))
+
+        self.influxdb_client.query_api().query("buckets()", "my-org")
+        requests = httpretty.httpretty.latest_requests
+        self.assertEqual(1, len(requests))
+        self.assertEqual("Token my-token", requests[0].headers["Authorization"])
+
+        self.assertIn("Authorization: ***", log_stream.getvalue())
+
 
 class ServerWithSelfSingedSSL(http.server.SimpleHTTPRequestHandler):
     def _set_headers(self):

tests/test_InfluxDBClientAsync.py

@@ -1,7 +1,9 @@
 import asyncio
+import logging
 import unittest
 import os
 from datetime import datetime
+from io import StringIO
 
 import pytest
 from aioresponses import aioresponses
@@ -247,6 +249,19 @@ async def test_init_without_token(self, mocked):
         self.client = InfluxDBClientAsync("http://localhost")
         await self.client.query_api().query("buckets()", "my-org")
 
+    @async_test
+    async def test_redacted_auth_header(self):
+        await self.client.close()
+        self.client = InfluxDBClientAsync(url="http://localhost:8086", token="my-token", org="my-org", debug=True)
+
+        log_stream = StringIO()
+        logger = logging.getLogger("influxdb_client.client.http")
+        logger.addHandler(logging.StreamHandler(log_stream))
+
+        await self.client.query_api().query("buckets()", "my-org")
+
+        self.assertIn("Authorization: ***", log_stream.getvalue())
+
     async def _prepare_data(self, measurement: str):
         _point1 = Point(measurement).tag("location", "Prague").field("temperature", 25.3)
         _point2 = Point(measurement).tag("location", "New York").field("temperature", 24.3)