Make bootstrap idempotent when it needs to be

This commit makes `keystone-manage bootstrap` completely idempotent
when configuration values or environment variables haven't changed
between runs. If they have changed, then `bootstrap` shouldn't be
as idempotent becuase it's changing the state of the deployment.

This commit addresses these issues and adds tests to ensure the
proper behavior is tested.

Change-Id: I053b27e881f5bb67db1ace01e6d06aead10b1e47
Closes-Bug: 1647800

keystone/cmd/cli.py

@@ -219,23 +219,37 @@ class BootStrap(BaseApp):
             LOG.info(_LI('User %s already exists, skipping creation.'),
                      self.username)
 
-            # Remember whether the user was enabled or not, so that we can
+            # If the user is not enabled, re-enable them. This also helps
-            # provide useful logging output later.
+            # provide some useful logging output later.
-            was_enabled = user['enabled']
+            update = {}
+            enabled = user['enabled']
+            if not enabled:
+                update['enabled'] = True
 
-            # To keep bootstrap idempotent, try to reset the user's password
+            try:
-            # and ensure that they are enabled. This allows bootstrap to act as
+                self.identity_manager.driver.authenticate(
-            # a recovery tool, without having to create a new user.
+                    user['id'], self.password
-            user = self.identity_manager.update_user(
+                )
-                user['id'],
+            except AssertionError:
-                {'enabled': True,
+                # This means that authentication failed and that we need to
-                 'password': self.password})
+                # update the user's password. This is going to persist a
-            LOG.info(_LI('Reset password for user %s.'), self.username)
+                # revocation event that will make all previous tokens for the
-            if not was_enabled and user['enabled']:
+                # user invalid, which is OK because it falls within the scope
-                # Although we always try to enable the user, this log message
+                # of revocation. If a password changes, we shouldn't be able to
-                # only makes sense if we know that the user was previously
+                # use tokens obtained with an old password.
-                # disabled.
+                update['password'] = self.password
-                LOG.info(_LI('Enabled user %s.'), self.username)
+
+            # Only make a call to update the user if the password has changed
+            # or the user was previously disabled. This allows bootstrap to act
+            # as a recovery tool, without having to create a new user.
+            if update:
+                user = self.identity_manager.update_user(user['id'], update)
+                LOG.info(_LI('Reset password for user %s.'), self.username)
+                if not enabled and user['enabled']:
+                    # Although we always try to enable the user, this log
+                    # message only makes sense if we know that the user was
+                    # previously disabled.
+                    LOG.info(_LI('Enabled user %s.'), self.username)
         except exception.UserNotFound:
             user = self.identity_manager.create_user(
                 user_ref={'name': self.username,

keystone/tests/unit/test_cli.py

@@ -159,9 +159,9 @@ class CliBootStrapTestCase(unit.SQLDriverOverrides, unit.TestCase):
                 self.assertEqual(svc['id'], endpoint['service_id'])
                 self.assertEqual(interface, endpoint['interface'])
 
-    def test_bootstrap_is_idempotent(self):
+    def test_bootstrap_is_idempotent_when_password_does_not_change(self):
-        # NOTE(morganfainberg): Ensure we can run bootstrap multiple times
+        # NOTE(morganfainberg): Ensure we can run bootstrap with the same
-        # without erroring.
+        # configuration multiple times without erroring.
         bootstrap = cli.BootStrap()
         self._do_test_bootstrap(bootstrap)
         v3_token_controller = controllers.Auth()
@@ -184,23 +184,44 @@ class CliBootStrapTestCase(unit.SQLDriverOverrides, unit.TestCase):
         token = auth_response.headers['X-Subject-Token']
         self._do_test_bootstrap(bootstrap)
         # build validation request
-        request = self.make_request(
+        request = self.make_request(is_admin=True)
-            is_admin=True,
-            headers={
-                'X-Subject-Token': token,
-                'X-Auth-Token': token
-            }
-        )
         request.context_dict['subject_token_id'] = token
-        # NOTE(lbragstad): This is currently broken because the bootstrap
+        # Make sure the token we authenticate for is still valid.
-        # operation will automatically reset a user's password even if it is
+        v3_token_controller.validate_token(request)
-        # the same as it was before. Bootstrap has this behavior so it's
+
-        # possible to recover admin accounts, which was one of our main
+    def test_bootstrap_is_not_idempotent_when_password_does_change(self):
-        # usecases for introducing the bootstrap functionality. The side-effect
+        # NOTE(lbragstad): Ensure bootstrap isn't idempotent when run with
-        # is that changing the password will create a revocation event. So if a
+        # different arguments or configuration values.
-        # token is obtained in-between two bootstrap calls, the token will no
+        bootstrap = cli.BootStrap()
-        # longer be valid after the second bootstrap operation completes, even
+        self._do_test_bootstrap(bootstrap)
-        # if the password is the same.
+        v3_token_controller = controllers.Auth()
+        v3_password_data = {
+            'identity': {
+                "methods": ["password"],
+                "password": {
+                    "user": {
+                        "name": bootstrap.username,
+                        "password": bootstrap.password,
+                        "domain": {
+                            "id": CONF.identity.default_domain_id
+                        }
+                    }
+                }
+            }
+        }
+        auth_response = v3_token_controller.authenticate_for_token(
+            self.make_request(), v3_password_data)
+        token = auth_response.headers['X-Subject-Token']
+        os.environ['OS_BOOTSTRAP_PASSWORD'] = uuid.uuid4().hex
+        self._do_test_bootstrap(bootstrap)
+        # build validation request
+        request = self.make_request(is_admin=True)
+        request.context_dict['subject_token_id'] = token
+        # Since the user account was recovered with a different password, we
+        # shouldn't be able to validate this token. Bootstrap should have
+        # persisted a revocation event because the user's password was updated.
+        # Since this token was obtained using the original password, it should
+        # now be invalid.
         self.assertRaises(
             exception.TokenNotFound,
             v3_token_controller.validate_token,