Browse Source

Mask passwords in debug log on user password operations

When a user is created, they change their password, or admin
changes their password and debug logging is enabled, the value of
the user's password was logged. The value should be masked.

Conflicts:
	keystone/common/controller.py
	keystone/tests/unit/test_v3_identity.py

Change-Id: I07b7441378fb630f01204d6b656b218f6b94dd5a
Closes-Bug: #1465922
(cherry picked from commit fbdb100e65)
(cherry picked from commit c4dc1331e1)
Brant Knudson 3 years ago
parent
commit
c15cbc48d6
2 changed files with 9 additions and 15 deletions
  1. 5
    1
      keystone/common/controller.py
  2. 4
    14
      keystone/tests/test_v3_identity.py

+ 5
- 1
keystone/common/controller.py View File

@@ -25,6 +25,7 @@ from keystone import exception
25 25
 from keystone.i18n import _
26 26
 from keystone.models import token_model
27 27
 from keystone.openstack.common import log
28
+from keystone.openstack.common import strutils
28 29
 
29 30
 
30 31
 LOG = log.getLogger(__name__)
@@ -50,9 +51,12 @@ def v2_deprecated(f):
50 51
 
51 52
 
52 53
 def _build_policy_check_credentials(self, action, context, kwargs):
54
+    kwargs_str = ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs])
55
+    kwargs_str = strutils.mask_password(kwargs_str)
56
+
53 57
     LOG.debug('RBAC: Authorizing %(action)s(%(kwargs)s)', {
54 58
         'action': action,
55
-        'kwargs': ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs])})
59
+        'kwargs': kwargs_str})
56 60
 
57 61
     # see if auth context has already been created. If so use it.
58 62
     if ('environment' in context and

+ 4
- 14
keystone/tests/test_v3_identity.py View File

@@ -1662,8 +1662,6 @@ class IdentityTestCase(test_v3.RestfulTestCase):
1662 1662
     def test_create_user_password_not_logged(self):
1663 1663
         # When a user is created, the password isn't logged at any level.
1664 1664
 
1665
-        # FIXME(blk-u): This doesn't work as expected, see bug 1465922
1666
-
1667 1665
         log_fix = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG))
1668 1666
 
1669 1667
         ref = self.new_user_ref(domain_id=self.domain_id)
@@ -1671,15 +1669,12 @@ class IdentityTestCase(test_v3.RestfulTestCase):
1671 1669
             '/users',
1672 1670
             body={'user': ref})
1673 1671
 
1674
-        # This should be assert*Not*In, see bug 1465922
1675
-        self.assertIn(ref['password'], log_fix.output)
1672
+        self.assertNotIn(ref['password'], log_fix.output)
1676 1673
 
1677 1674
     def test_update_password_not_logged(self):
1678 1675
         # When admin modifies user password, the password isn't logged at any
1679 1676
         # level.
1680 1677
 
1681
-        # FIXME(blk-u): This doesn't work as expected, see bug 1465922
1682
-
1683 1678
         log_fix = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG))
1684 1679
 
1685 1680
         # bootstrap a user as admin
@@ -1694,9 +1689,7 @@ class IdentityTestCase(test_v3.RestfulTestCase):
1694 1689
                    expected_status=200)
1695 1690
 
1696 1691
         self.assertNotIn(password, log_fix.output)
1697
-
1698
-        # This should be assert*Not*In, see bug 1465922
1699
-        self.assertIn(new_password, log_fix.output)
1692
+        self.assertNotIn(new_password, log_fix.output)
1700 1693
 
1701 1694
 
1702 1695
 class IdentityInheritanceTestCase(test_v3.RestfulTestCase):
@@ -2346,8 +2339,6 @@ class UserSelfServiceChangingPasswordsTestCase(test_v3.RestfulTestCase):
2346 2339
         # When a user changes their password, the password isn't logged at any
2347 2340
         # level.
2348 2341
 
2349
-        # FIXME(blk-u): This doesn't work as expected, see bug 1465922
2350
-
2351 2342
         log_fix = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG))
2352 2343
 
2353 2344
         # change password
@@ -2356,6 +2347,5 @@ class UserSelfServiceChangingPasswordsTestCase(test_v3.RestfulTestCase):
2356 2347
                              original_password=self.user_ref['password'],
2357 2348
                              expected_status=204)
2358 2349
 
2359
-        # These should be assert*Not*In, see bug 1465922
2360
-        self.assertIn(self.user_ref['password'], log_fix.output)
2361
-        self.assertIn(new_password, log_fix.output)
2350
+        self.assertNotIn(self.user_ref['password'], log_fix.output)
2351
+        self.assertNotIn(new_password, log_fix.output)

Loading…
Cancel
Save