Don't log the credentials by default

Even in case of DEBUG level logging credentials (especially those that
give admin level access) should not be saved into log files.

This way of handling it has the side effect that if someone uses
password "password", it will be replaced in another place too... but
password "password" or some other keyword that can be found in the
request itself was a pretty bad idea to begin with.

Shell utilities are not affected and the verbose mode will still
display the passwords to make debugging easy.

Implements: blueprint limit-credentials-logging
Change-Id: I50d0ebbfbd44c7a5b162d9334b4fdbda67e5c28d

neutronclient/client.py

@@ -99,7 +99,8 @@ class HTTPClient(httplib2.Http):
                  token=None, region_name=None, timeout=None,
                  endpoint_url=None, insecure=False,
                  endpoint_type='publicURL',
-                 auth_strategy='keystone', ca_cert=None, **kwargs):
+                 auth_strategy='keystone', ca_cert=None, log_credentials=False,
+                 **kwargs):
         super(HTTPClient, self).__init__(timeout=timeout, ca_certs=ca_cert)
 
         self.username = username
@@ -113,6 +114,7 @@ class HTTPClient(httplib2.Http):
         self.content_type = 'application/json'
         self.endpoint_url = endpoint_url
         self.auth_strategy = auth_strategy
+        self.log_credentials = log_credentials
         # httplib2 overrides
         self.disable_ssl_certificate_validation = insecure
 
@@ -132,7 +134,13 @@ class HTTPClient(httplib2.Http):
             kargs['body'] = kwargs['body']
         args = utils.safe_encode_list(args)
         kargs = utils.safe_encode_dict(kargs)
-        utils.http_log_req(_logger, args, kargs)
+
+        if self.log_credentials:
+            log_kargs = kargs
+        else:
+            log_kargs = self._strip_credentials(kargs)
+
+        utils.http_log_req(_logger, args, log_kargs)
         try:
             resp, body = self.request(*args, **kargs)
         except httplib2.SSLHandshakeError as e:
@@ -150,6 +158,15 @@ class HTTPClient(httplib2.Http):
             raise exceptions.Forbidden(message=body)
         return resp, body
 
+    def _strip_credentials(self, kwargs):
+        if kwargs.get('body') and self.password:
+            log_kwargs = kwargs.copy()
+            log_kwargs['body'] = kwargs['body'].replace(self.password,
+                                                        'REDACTED')
+            return log_kwargs
+        else:
+            return kwargs
+
     def authenticate_and_fetch_endpoint_url(self):
         if not self.auth_token:
             self.authenticate()

neutronclient/common/clientmanager.py

@@ -57,6 +57,7 @@ class ClientManager(object):
                  auth_strategy=None,
                  insecure=False,
                  ca_cert=None,
+                 log_credentials=False,
                  ):
         self._token = token
         self._url = url
@@ -72,11 +73,13 @@ class ClientManager(object):
         self._auth_strategy = auth_strategy
         self._insecure = insecure
         self._ca_cert = ca_cert
+        self._log_credentials = log_credentials
         return
 
     def initialize(self):
         if not self._url:
-            httpclient = client.HTTPClient(username=self._username,
+            httpclient = client.HTTPClient(
+                username=self._username,
                 tenant_name=self._tenant_name,
                 tenant_id=self._tenant_id,
                 password=self._password,
@@ -84,7 +87,8 @@ class ClientManager(object):
                 auth_url=self._auth_url,
                 endpoint_type=self._endpoint_type,
                 insecure=self._insecure,
-                                           ca_cert=self._ca_cert)
+                ca_cert=self._ca_cert,
+                log_credentials=self._log_credentials)
             httpclient.authenticate()
             # Populate other password flow attributes
             self._token = httpclient.auth_token

neutronclient/shell.py

@@ -529,7 +529,8 @@ class NeutronShell(app.App):
             auth_strategy=self.options.os_auth_strategy,
             endpoint_type=self.options.endpoint_type,
             insecure=self.options.insecure,
-            ca_cert=self.options.os_cacert, )
+            ca_cert=self.options.os_cacert,
+            log_credentials=True)
         return
 
     def initialize_app(self, argv):

tests/unit/test_auth.py

@@ -25,6 +25,7 @@ import testtools
 
 from neutronclient import client
 from neutronclient.common import exceptions
+from neutronclient.common import utils
 
 
 USERNAME = 'testuser'
@@ -323,6 +324,34 @@ class CLITestAuthKeystone(testtools.TestCase):
                           self.client._extract_service_catalog,
                           resources)
 
+    def test_strip_credentials_from_log(self):
+        def verify_no_credentials(kwargs):
+            return ('REDACTED' in kwargs['body']) and (
+                self.client.password not in kwargs['body'])
+
+        def verify_credentials(body):
+            return 'REDACTED' not in body and self.client.password in body
+
+        self.mox.StubOutWithMock(self.client, "request")
+        self.mox.StubOutWithMock(utils, "http_log_req")
+
+        res200 = self.mox.CreateMock(httplib2.Response)
+        res200.status = 200
+
+        utils.http_log_req(mox.IgnoreArg(), mox.IgnoreArg(), mox.Func(
+            verify_no_credentials))
+        self.client.request(
+            mox.IsA(str), mox.IsA(str), body=mox.Func(verify_credentials),
+            headers=mox.IgnoreArg()
+        ).AndReturn((res200, json.dumps(KS_TOKEN_RESULT)))
+        utils.http_log_req(mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg())
+        self.client.request(
+            mox.IsA(str), mox.IsA(str), headers=mox.IsA(dict)
+        ).AndReturn((res200, ''))
+        self.mox.ReplayAll()
+
+        self.client.do_request('/resource', 'GET')
+
 
 class CLITestAuthKeystoneWithId(CLITestAuthKeystone):
 

tests/unit/test_ssl.py

@@ -60,7 +60,8 @@ class TestSSL(testtools.TestCase):
             tenant_name=mox.IgnoreArg(),
             token=mox.IgnoreArg(),
             url=mox.IgnoreArg(),
-            username=mox.IgnoreArg()
+            username=mox.IgnoreArg(),
+            log_credentials=mox.IgnoreArg(),
         )
         openstack_shell.NeutronShell.interact().AndReturn(0)
         self.mox.ReplayAll()
@@ -88,7 +89,8 @@ class TestSSL(testtools.TestCase):
             tenant_name=mox.IgnoreArg(),
             token=mox.IgnoreArg(),
             url=mox.IgnoreArg(),
-            username=mox.IgnoreArg()
+            username=mox.IgnoreArg(),
+            log_credentials=mox.IgnoreArg(),
         )
         openstack_shell.NeutronShell.interact().AndReturn(0)
         self.mox.ReplayAll()