openstack
/
keystone
Code Issues Proposed changes

extracting user and trust ids into normalized fields 

These fields are used for queries, and may need to be indexed
Also moves the delete token for... functions into the base class
for controllers.

Removed the token API revoke token call as that needed access to other
APIs.  Logic was moved into the controller.

Bug 1152801

Change-Id: I59c360fe5aef905dfa30cb55ee54ff1fbe64dc58
This commit is contained in:
Adam Young 2013-03-08 21:19:25 -05:00
parent a79a7c1ddb
commit eb4dd4afbf
14 changed files with 214 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
etc/policy.json
View File

@ -25,8 +25,7 @@
"identity:get_project": [["rule:admin_required"]],
"identity:list_projects": [["rule:admin_required"]],
"identity:list_user_projects": [["rule:admin_required"],
["user_id:%(user_id)s"]],
"identity:list_user_projects": [["rule:admin_or_owner"]],
"identity:create_project": [["rule:admin_or_owner"]],
"identity:update_project": [["rule:admin_required"]],
"identity:delete_project": [["rule:admin_required"]],
@ -34,7 +33,7 @@
"identity:get_user": [["rule:admin_required"]],
"identity:list_users": [["rule:admin_required"]],
"identity:create_user": [["rule:admin_required"]],
"identity:update_user": [["rule:admin_required"]],
"identity:update_user": [["rule:admin_or_owner"]],
"identity:delete_user": [["rule:admin_required"]],
"identity:get_group": [["rule:admin_required"]],

3
keystone/auth/token_factory.py
View File

@ -285,7 +285,8 @@ def create_token(context, auth_context, auth_info):
user=token_data['token']['user'],
tenant=token_data['token'].get('project'),
metadata=metadata_ref,
token_data=token_data)
token_data=token_data,
trust_id=trust['id'] if trust else None)
token_api.create_token(context, token_id, data)
except Exception as e:
# an identical token may have been created already.

31
keystone/common/controller.py
View File

@ -153,6 +153,32 @@ def filterprotected(*filters):
class V2Controller(wsgi.Application):
"""Base controller class for Identity API v2."""
def _delete_tokens_for_trust(self, context, user_id, trust_id):
try:
token_list = self.token_api.list_tokens(context, user_id,
trust_id=trust_id)
for token in token_list:
self.token_api.delete_token(context, token)
except exception.NotFound:
pass
def _delete_tokens_for_user(self, context, user_id, project_id=None):
#First delete tokens that could get other tokens.
for token_id in self.token_api.list_tokens(context,
user_id,
tenant_id=project_id):
try:
self.token_api.delete_token(context, token_id)
except exception.NotFound:
pass
#delete tokens generated from trusts
for trust in self.trust_api.list_trusts_for_trustee(context, user_id):
self._delete_tokens_for_trust(context, user_id, trust['id'])
for trust in self.trust_api.list_trusts_for_trustor(context, user_id):
self._delete_tokens_for_trust(context,
trust['trustee_user_id'],
trust['id'])
def _require_attribute(self, ref, attr):
"""Ensures the reference contains the specified attribute."""
if ref.get(attr) is None or ref.get(attr) == '':
@ -188,6 +214,11 @@ class V3Controller(V2Controller):
collection_name = 'entities'
member_name = 'entity'
def _delete_tokens_for_group(self, context, group_id):
user_refs = self.identity_api.list_users_in_group(context, group_id)
for user in user_refs:
self._delete_tokens_for_user(context, user['id'])
@classmethod
def base_url(cls, path=None):
endpoint = CONF.public_endpoint % CONF

82
keystone/common/sql/migrate_repo/versions/021_add_trust_to_token.py Normal file
View File

@ -0,0 +1,82 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# Copyright 2013 OpenStack LLC
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import sqlalchemy
from sqlalchemy import exc
from sqlalchemy.orm import sessionmaker
from keystone import config
def downgrade_token_table_with_column_drop(meta, migrate_engine):
token_table = sqlalchemy.Table('token', meta, autoload=True)
#delete old tokens, as the format has changed.
#We don't guarantee that existing tokens will be
#usable after a migration
token_table.delete()
token_table.drop_column(
sqlalchemy.Column('trust_id',
sqlalchemy.String(64),
nullable=True))
token_table.drop_column(
sqlalchemy.Column('user_id',
sqlalchemy.String(64)))
def create_column_forgiving(migrate_engine, table, column):
try:
table.create_column(column)
except exc.OperationalError as e:
if (e.args[0].endswith('duplicate column name: %s' % column.name)
and migrate_engine.name == "sqlite"):
#sqlite does not drop columns, so if we have already
#done a downgrade and are now upgrading, we will hit
#this: the SQLite driver previously reported success
#dropping the columns but it hasn't.
pass
else:
raise
def upgrade_token_table(meta, migrate_engine):
#delete old tokens, as the format has changed.
#The existing tokens will not
#support some of the list functions
token_table = sqlalchemy.Table('token', meta, autoload=True)
token_table.delete()
create_column_forgiving(
migrate_engine, token_table,
sqlalchemy.Column('trust_id',
sqlalchemy.String(64),
nullable=True))
create_column_forgiving(
migrate_engine, token_table,
sqlalchemy.Column('user_id', sqlalchemy.String(64)))
def upgrade(migrate_engine):
meta = sqlalchemy.MetaData()
meta.bind = migrate_engine
upgrade_token_table(meta, migrate_engine)
def downgrade(migrate_engine):
meta = sqlalchemy.MetaData()
meta.bind = migrate_engine
downgrade_token_table_with_column_drop(meta, migrate_engine)

78
keystone/identity/controllers.py
View File

@ -161,32 +161,6 @@ class Tenant(controller.V2Controller):
return o
def delete_tokens_for_user(context, token_api, trust_api, user_id):
try:
#First delete tokens that could get other tokens.
for token_id in token_api.list_tokens(context, user_id):
token_api.delete_token(context, token_id)
#now delete trust tokens
for trust in trust_api.list_trusts_for_trustee(context, user_id):
token_list = token_api.list_tokens(context, user_id,
trust_id=trust['id'])
for token in token_list:
token_api.delete_token(context, token)
except exception.NotImplemented:
# The users status has been changed but tokens remain valid for
# backends that can't list tokens for users
LOG.warning(_('User %s status has changed, but existing tokens '
'remain valid') % user_id)
def delete_tokens_for_group(context, identity_api, token_api, trust_api,
group_id):
user_refs = identity_api.list_users_in_group(context, group_id)
for user in user_refs:
delete_tokens_for_user(
context, token_api, trust_api, user['id'])
class User(controller.V2Controller):
def get_user(self, context, user_id):
self.assert_admin(context)
@ -243,8 +217,7 @@ class User(controller.V2Controller):
if user.get('password') or not user.get('enabled', True):
# If the password was changed or the user was disabled we clear tokens
delete_tokens_for_user(context, self.token_api, self.trust_api,
user_id)
self._delete_tokens_for_user(context, user_id)
return {'user': self._filter_domain_id(user_ref)}
def delete_user(self, context, user_id):
@ -326,7 +299,7 @@ class Role(controller.V2Controller):
self.identity_api.add_role_to_user_and_project(
context, user_id, tenant_id, role_id)
self.token_api.revoke_tokens(context, user_id, tenant_id)
self._delete_tokens_for_user(context, user_id)
role_ref = self.identity_api.get_role(context, role_id)
return {'role': role_ref}
@ -347,8 +320,7 @@ class Role(controller.V2Controller):
# a user also adds them to a tenant, so we must follow up on that
self.identity_api.remove_role_from_user_and_project(
context, user_id, tenant_id, role_id)
delete_tokens_for_user(
context, self.token_api, self.trust_api, user_id)
self._delete_tokens_for_user(context, user_id)
# COMPAT(diablo): CRUD extension
def get_role_refs(self, context, user_id):
@ -390,7 +362,7 @@ class Role(controller.V2Controller):
role_id = role.get('roleId')
self.identity_api.add_role_to_user_and_project(
context, user_id, tenant_id, role_id)
self.token_api.revoke_tokens(context, user_id, tenant_id)
self._delete_tokens_for_user(context, user_id)
role_ref = self.identity_api.get_role(context, role_id)
return {'role': role_ref}
@ -416,7 +388,7 @@ class Role(controller.V2Controller):
context, user_id, tenant_id, role_id)
roles = self.identity_api.get_roles_for_user_and_project(
context, user_id, tenant_id)
self.token_api.revoke_tokens(context, user_id, tenant_id)
self._delete_tokens_for_user(context, user_id)
class DomainV3(controller.V3Controller):
@ -462,17 +434,14 @@ class DomainV3(controller.V3Controller):
"""
# revoke all tokens for users owned by this domain
if user.get('domain_id') == domain_id:
self.token_api.revoke_tokens(
context,
user_id=user['id'])
self._delete_tokens_for_user(
context, user['id'])
else:
# only revoke tokens on projects owned by this domain
for project in projects:
self.token_api.revoke_tokens(
context,
user_id=user['id'],
tenant_id=project['id'])
self._delete_tokens_for_user(
context, user['id'],
project_id=project['id'])
return DomainV3.wrap_member(context, ref)
@controller.protected
@ -568,9 +537,7 @@ class UserV3(controller.V3Controller):
if user.get('password') or not user.get('enabled', True):
# revoke all tokens owned by this user
self.token_api.revoke_tokens(
context,
user_id=ref['id'])
self._delete_tokens_for_user(context, user_id)
return UserV3.wrap_member(context, ref)
@ -580,8 +547,7 @@ class UserV3(controller.V3Controller):
context, user_id, group_id)
# Delete any tokens so that group membership can have an
# immediate effect
delete_tokens_for_user(
context, self.token_api, self.trust_api, user_id)
self._delete_tokens_for_user(context, user_id)
@controller.protected
def check_user_in_group(self, context, user_id, group_id):
@ -592,8 +558,7 @@ class UserV3(controller.V3Controller):
def remove_user_from_group(self, context, user_id, group_id):
self.identity_api.remove_user_from_group(
context, user_id, group_id)
delete_tokens_for_user(
context, self.token_api, self.trust_api, user_id)
self._delete_tokens_for_user(context, user_id)
@controller.protected
def delete_user(self, context, user_id):
@ -644,8 +609,7 @@ class GroupV3(controller.V3Controller):
user_refs = self.identity_api.list_users_in_group(context, group_id)
self.identity_api.delete_group(context, group_id)
for user in user_refs:
delete_tokens_for_user(
context, self.token_api, self.trust_api, user['id'])
self._delete_tokens_for_user(context, user['id'])
class CredentialV3(controller.V3Controller):
@ -738,12 +702,9 @@ class RoleV3(controller.V3Controller):
# delete any tokens for this user or, in the case of a group,
# tokens from all the uses who are members of this group.
if user_id:
delete_tokens_for_user(
context, self.token_api, self.trust_api, user_id)
self._delete_tokens_for_user(context, user_id)
else:
delete_tokens_for_group(
context, self.identity_api, self.token_api, self.trust_api,
group_id)
self._delete_tokens_for_group(context, group_id)
@controller.protected
def list_grants(self, context, user_id=None, group_id=None,
@ -779,9 +740,6 @@ class RoleV3(controller.V3Controller):
# Now delete any tokens for this user or, in the case of a group,
# tokens from all the uses who are members of this group.
if user_id:
delete_tokens_for_user(
context, self.token_api, self.trust_api, user_id)
self._delete_tokens_for_user(context, user_id)
else:
delete_tokens_for_group(
context, self.identity_api, self.token_api,
self.trust_api, group_id)
self._delete_tokens_for_group(context, group_id)

4
keystone/token/backends/kvs.py
View File

@ -45,8 +45,8 @@ class Token(kvs.Base, token.Driver):
data_copy = copy.deepcopy(data)
if not data_copy.get('expires'):
data_copy['expires'] = token.default_expire_time()
if 'trust_id' in data and data['trust_id'] is None:
data_copy.pop('trust_id')
if not data_copy.get('user_id'):
data_copy['user_id'] = data_copy['user']['id']
self.db.set('token-%s' % token_id, data_copy)
return copy.deepcopy(data_copy)

2
keystone/token/backends/memcache.py
View File

@ -66,6 +66,8 @@ class Token(token.Driver):
ptk = self._prefix_token_id(token.unique_id(token_id))
if not data_copy.get('expires'):
data_copy['expires'] = token.default_expire_time()
if not data_copy.get('user_id'):
data_copy['user_id'] = data_copy['user']['id']
kwargs = {}
if data_copy['expires'] is not None:
expires_ts = utils.unixtime(data_copy['expires'])

27
keystone/token/backends/sql.py
View File

@ -26,11 +26,13 @@ from keystone import token
class TokenModel(sql.ModelBase, sql.DictBase):
__tablename__ = 'token'
attributes = ['id', 'expires']
attributes = ['id', 'expires', 'user_id', 'trust_id']
id = sql.Column(sql.String(64), primary_key=True)
expires = sql.Column(sql.DateTime(), default=None)
extra = sql.Column(sql.JsonBlob())
valid = sql.Column(sql.Boolean(), default=True)
user_id = sql.Column(sql.String(64))
trust_id = sql.Column(sql.String(64), nullable=True)
class Token(sql.Base, token.Driver):
@ -55,6 +57,9 @@ class Token(sql.Base, token.Driver):
data_copy = copy.deepcopy(data)
if not data_copy.get('expires'):
data_copy['expires'] = token.default_expire_time()
if not data_copy.get('user_id'):
data_copy['user_id'] = data_copy['user']['id']
token_ref = TokenModel.from_dict(data_copy)
token_ref.id = token.unique_id(token_id)
token_ref.valid = True
@ -76,27 +81,20 @@ class Token(sql.Base, token.Driver):
session.flush()
def _list_tokens_for_trust(self, trust_id):
def trust_matches(trust_id, token_ref_dict):
return (token_ref_dict.get('trust_id') and
token_ref_dict['trust_id'] == trust_id)
session = self.get_session()
tokens = []
now = timeutils.utcnow()
query = session.query(TokenModel)
query = query.filter(TokenModel.expires > now)
query = query.filter(TokenModel.trust_id == trust_id)
token_references = query.filter_by(valid=True)
for token_ref in token_references:
token_ref_dict = token_ref.to_dict()
if trust_matches(trust_id, token_ref_dict):
tokens.append(token_ref['id'])
tokens.append(token_ref['id'])
return tokens
def _list_tokens_for_user(self, user_id, tenant_id=None):
def user_matches(user_id, token_ref_dict):
return (token_ref_dict.get('user') and
token_ref_dict['user'].get('id') == user_id)
def tenant_matches(tenant_id, token_ref_dict):
return ((tenant_id is None) or
(token_ref_dict.get('tenant') and
@ -107,12 +105,13 @@ class Token(sql.Base, token.Driver):
now = timeutils.utcnow()
query = session.query(TokenModel)
query = query.filter(TokenModel.expires > now)
query = query.filter(TokenModel.user_id == user_id)
token_references = query.filter_by(valid=True)
for token_ref in token_references:
token_ref_dict = token_ref.to_dict()
if (user_matches(user_id, token_ref_dict) and
tenant_matches(tenant_id, token_ref_dict)):
tokens.append(token_ref['id'])
if tenant_matches(tenant_id, token_ref_dict):
tokens.append(token_ref['id'])
return tokens
def list_tokens(self, user_id, tenant_id=None, trust_id=None):

17
keystone/token/core.py
View File

@ -121,15 +121,6 @@ class Manager(manager.Manager):
def __init__(self):
super(Manager, self).__init__(CONF.token.driver)
def revoke_tokens(self, context, user_id, tenant_id=None):
"""Invalidates all tokens held by a user (optionally for a tenant).
If a specific tenant ID is not provided, *all* tokens held by user will
be revoked.
"""
for token_id in self.list_tokens(context, user_id, tenant_id):
self.delete_token(context, token_id)
class Driver(object):
"""Interface description for a Token driver."""
@ -200,11 +191,3 @@ class Driver(object):
"""
raise exception.NotImplemented()
def revoke_tokens(self, user_id, tenant_id=None):
"""Invalidates all tokens held by a user (optionally for a tenant).
:raises: keystone.exception.UserNotFound,
keystone.exception.ProjectNotFound
"""
raise exception.NotImplemented()

4
tests/test_auth.py
View File

@ -687,10 +687,8 @@ class AuthWithTrust(AuthTest):
self.assert_token_count_for_trust(0)
auth_response = self.fetch_v2_token_from_trust()
self.assert_token_count_for_trust(1)
identity.controllers.delete_tokens_for_user(
self.trust_controller._delete_tokens_for_user(
{},
self.trust_controller.token_api,
self.trust_controller.trust_api,
self.trustee['id'])
self.assert_token_count_for_trust(0)

6
tests/test_backend.py
View File

@ -1953,14 +1953,18 @@ class TokenTests(object):
def test_token_crud(self):
token_id = uuid.uuid4().hex
data = {'id': token_id, 'a': 'b',
'trust_id': None,
'user': {'id': 'testuserid'}}
data_ref = self.token_api.create_token(token_id, data)
expires = data_ref.pop('expires')
data_ref.pop('user_id')
self.assertTrue(isinstance(expires, datetime.datetime))
self.assertDictEqual(data_ref, data)
new_data_ref = self.token_api.get_token(token_id)
expires = new_data_ref.pop('expires')
new_data_ref.pop('user_id')
self.assertTrue(isinstance(expires, datetime.datetime))
self.assertEquals(new_data_ref, data)
@ -2045,8 +2049,10 @@ class TokenTests(object):
expire_time = timeutils.utcnow() - datetime.timedelta(minutes=1)
data = {'id_hash': token_id, 'id': token_id, 'a': 'b',
'expires': expire_time,
'trust_id': None,
'user': {'id': 'testuserid'}}
data_ref = self.token_api.create_token(token_id, data)
data_ref.pop('user_id')
self.assertDictEqual(data_ref, data)
self.assertRaises(exception.TokenNotFound,
self.token_api.get_token, token_id)

8
tests/test_sql_upgrade.py
View File

@ -658,7 +658,10 @@ class SqlUpgradeTests(test.TestCase):
def test_upgrade_trusts(self):
self.assertEqual(self.schema.version, 0, "DB is at version 0")
self.upgrade(18)
self.upgrade(20)
self.assertTableColumns("token",
["id", "expires", "extra", "valid"])
self.upgrade(21)
self.assertTableColumns("trust",
["id", "trustor_user_id",
"trustee_user_id",
@ -667,6 +670,9 @@ class SqlUpgradeTests(test.TestCase):
"expires_at", "extra"])
self.assertTableColumns("trust_role",
["trust_id", "role_id"])
self.assertTableColumns("token",
["id", "expires", "extra", "valid",
"trust_id", "user_id"])
def test_fixup_role(self):
session = self.Session()

7
tests/test_v3.py
View File

@ -214,12 +214,13 @@ class RestfulTestCase(test_content_types.RestfulTestCase):
def v3_request(self, path, **kwargs):
# Check if the caller has passed in auth details for
# use in requesting the token
auth = kwargs.get('auth', None)
auth = kwargs.pop('auth', None)
if auth:
kwargs.pop('auth')
token = self.get_requested_token(auth)
else:
token = self.get_scoped_token()
token = kwargs.pop('token', None)
if not token:
token = self.get_scoped_token()
path = '/v3' + path
return self.admin_request(
path=path,

48
tests/test_v3_auth.py
View File

@ -35,8 +35,8 @@ class TestAuthInfo(test_v3.RestfulTestCase):
# building helper functions, they cause backend databases and fixtures
# to be loaded unnecessarily. Separating out the helper functions from
# this base class would improve efficiency (Bug #1134836)
def setUp(self):
super(TestAuthInfo, self).setUp(load_sample_data=False)
def setUp(self, load_sample_data=False):
super(TestAuthInfo, self).setUp(load_sample_data=load_sample_data)
def test_missing_auth_methods(self):
auth_data = {'identity': {}}
@ -815,9 +815,9 @@ class TestAuthXML(TestAuthJSON):
content_type = 'xml'
class TestTrustAuth(test_v3.RestfulTestCase):
class TestTrustAuth(TestAuthInfo):
def setUp(self):
super(TestTrustAuth, self).setUp()
super(TestTrustAuth, self).setUp(load_sample_data=True)
# create a trustee to delegate stuff to
self.trustee_user_id = uuid.uuid4().hex
@ -1065,3 +1065,43 @@ class TestTrustAuth(test_v3.RestfulTestCase):
self.user_id, expected_status=200)
trusts = r.body['trusts']
self.assertEqual(len(trusts), 0)
def test_change_password_invalidates_trust_tokens(self):
ref = self.new_trust_ref(
trustor_user_id=self.user_id,
trustee_user_id=self.trustee_user_id,
project_id=self.project_id,
impersonation=True,
expires=dict(minutes=1),
role_ids=[self.role_id])
del ref['id']
r = self.post('/trusts', body={'trust': ref})
trust = self.assertValidTrustResponse(r)
auth_data = self.build_authentication_request(
user_id=self.trustee_user['id'],
password=self.trustee_user['password'],
trust_id=trust['id'])
r = self.post('/auth/tokens', body=auth_data)
self.assertValidProjectTrustScopedTokenResponse(r, self.user)
trust_token = r.getheader('X-Subject-Token')
self.get('/trusts?trustor_user_id=%s' %
self.user_id, expected_status=200,
token=trust_token)
auth_data = self.build_authentication_request(
user_id=self.trustee_user['id'],
password=self.trustee_user['password'])
self.assertValidUserResponse(
self.patch('/users/%s' % self.trustee_user['id'],
body={'user': {'password': uuid.uuid4().hex}},
auth=auth_data,
expected_status=200))
self.get('/trusts?trustor_user_id=%s' %
self.user_id, expected_status=401,
token=trust_token)