From 16a7b074eff5597752a86a676ae8838be32b91fb Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Thu, 8 Dec 2016 17:01:22 +0000 Subject: [PATCH] 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 (cherry picked from commit 90f2f96e69b8bfd5058628b50c9f0083e3f293e9) --- keystone/cmd/cli.py | 46 ++++++++++++++++--------- keystone/tests/unit/test_cli.py | 59 ++++++++++++++++++++++----------- 2 files changed, 70 insertions(+), 35 deletions(-) diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index d04e447a52..3335ee839f 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -218,23 +218,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 - # provide useful logging output later. - was_enabled = user['enabled'] + # If the user is not enabled, re-enable them. This also helps + # provide some useful logging output later. + update = {} + enabled = user['enabled'] + if not enabled: + update['enabled'] = True - # To keep bootstrap idempotent, try to reset the user's password - # and ensure that they are enabled. This allows bootstrap to act as - # a recovery tool, without having to create a new user. - user = self.identity_manager.update_user( - user['id'], - {'enabled': True, - 'password': self.password}) - LOG.info(_LI('Reset password for user %s.'), self.username) - if not was_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) + try: + self.identity_manager.driver.authenticate( + user['id'], self.password + ) + except AssertionError: + # This means that authentication failed and that we need to + # update the user's password. This is going to persist a + # revocation event that will make all previous tokens for the + # user invalid, which is OK because it falls within the scope + # of revocation. If a password changes, we shouldn't be able to + # use tokens obtained with an old password. + update['password'] = self.password + + # 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, diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index 591de62705..adc34a47d1 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -156,9 +156,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): - # NOTE(morganfainberg): Ensure we can run bootstrap multiple times - # without erroring. + def test_bootstrap_is_idempotent_when_password_does_not_change(self): + # NOTE(morganfainberg): Ensure we can run bootstrap with the same + # configuration multiple times without erroring. bootstrap = cli.BootStrap() self._do_test_bootstrap(bootstrap) v3_token_controller = controllers.Auth() @@ -181,23 +181,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( - is_admin=True, - headers={ - 'X-Subject-Token': token, - 'X-Auth-Token': token - } - ) + request = self.make_request(is_admin=True) request.context_dict['subject_token_id'] = token - # NOTE(lbragstad): This is currently broken because the bootstrap - # operation will automatically reset a user's password even if it is - # the same as it was before. Bootstrap has this behavior so it's - # possible to recover admin accounts, which was one of our main - # usecases for introducing the bootstrap functionality. The side-effect - # is that changing the password will create a revocation event. So if a - # token is obtained in-between two bootstrap calls, the token will no - # longer be valid after the second bootstrap operation completes, even - # if the password is the same. + # Make sure the token we authenticate for is still valid. + v3_token_controller.validate_token(request) + + def test_bootstrap_is_not_idempotent_when_password_does_change(self): + # NOTE(lbragstad): Ensure bootstrap isn't idempotent when run with + # different arguments or configuration values. + bootstrap = cli.BootStrap() + self._do_test_bootstrap(bootstrap) + 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,