errors: hide credentials in urls in error messages (#34793)

passerelle/base/models.py

@@ -19,6 +19,7 @@ from django.db.models import Q
 from django.test import override_settings
 from django.utils.text import slugify
 from django.utils import timezone, six
+from django.utils.six.moves.urllib.parse import urlparse
 from django.utils.translation import ugettext_lazy as _
 from django.utils.timezone import now
 from django.core.files.base import ContentFile
@@ -750,6 +751,18 @@ class ProxyLogger(object):
         if self.connector.down() and not force:
             # don't log if the connector is known to be down
             return
+
+        # hide credentials in urls
+        for url in re.findall(r'(https?://\S+)', message):
+            try:
+                parsed = urlparse(url)
+            except Exception:
+                continue
+            if not parsed.username and not parsed.password:
+                continue
+            replaced = parsed._replace(netloc="{}:{}@{}".format('***', '***', parsed.hostname))
+            message = message.replace(url, replaced.geturl())
+
         levelno = getattr(logging, levelname)
         if self._logger.level <= levelno:
 

tests/test_proxylogger.py

@@ -8,6 +8,7 @@ import pytest
 from httmock import HTTMock
 
 from django.core.exceptions import ValidationError
+from django.utils.log import AdminEmailHandler
 
 from passerelle.base.models import ProxyLogger, ResourceLog
 from passerelle.apps.feeds.models import Feed
@@ -24,6 +25,18 @@ def connector():
     return connector
 
 
+@pytest.fixture
+def email_handler():
+    root = logging.getLogger()
+    handler = AdminEmailHandler(include_html=True)
+    handler.level = logging.ERROR
+    root.handlers.append(handler)
+    try:
+        yield
+    finally:
+        root.handlers.remove(handler)
+
+
 def test_proxy_logger_basic(db, connector):
     pr = ProxyLogger(connector)
     pr.debug(u'some message')
@@ -61,6 +74,7 @@ def test_proxy_logger_ignore(db, connector):
     pr.debug(u'some message')
     assert len(ResourceLog.objects.all()) == 0
 
+
 def test_proxy_logger_ignore_when_down(db, connector):
     with HTTMock(down_mock):  # set connector as down
         connector.availability()
@@ -71,6 +85,33 @@ def test_proxy_logger_ignore_when_down(db, connector):
     assert len(ResourceLog.objects.all()) == 0
 
 
+@pytest.mark.parametrize('error_msg, expected', [
+    ('Foo Bar', 'Foo Bar'),
+    ('http://badurl', 'http://badurl'),
+    ('GET http://tcl.example.net/tclpassagearret (=> 502)',
+     'GET http://tcl.example.net/tclpassagearret (=> 502)'),
+    ('GET https://tcl.example.net/tclpassagearret (=> 502)',
+     'GET https://tcl.example.net/tclpassagearret (=> 502)'),
+    ('GET http://username:password@tcl.example.net/tclpassagearret (=> 502)',
+     'GET http://***:***@tcl.example.net/tclpassagearret (=> 502)'),
+    ('GET https://username:password@tcl.example.net/tclpassagearret (=> 502)',
+     'GET https://***:***@tcl.example.net/tclpassagearret (=> 502)'),
+])
+def test_proxy_logger_hide_url_credentials(db, settings, email_handler, mailoutbox,
+                                           connector, error_msg, expected):
+    settings.ADMINS = [('admin', 'admin@example.net')]
+
+    pr = ProxyLogger(connector)
+    pr.error(error_msg)
+    assert len(mailoutbox) == 1
+    msg = mailoutbox[0]
+    assert msg.to == ['admin@example.net']
+    assert msg.subject == '[Django] ERROR: %s' % expected
+    assert expected in msg.body
+    rl = ResourceLog.objects.latest('pk')
+    assert rl.message == expected
+
+
 def test_validate_notification_delays(db, connector):
     def take(iterator, count):
         return list(itertools.compress(iterator, range(1, count + 1)))