diff --git a/keystone/common/sql/contract_repo/versions/022_contract_add_default_project_id_index.py b/keystone/common/sql/contract_repo/versions/022_contract_add_default_project_id_index.py new file mode 100644 index 0000000000..8aa15c1ef2 --- /dev/null +++ b/keystone/common/sql/contract_repo/versions/022_contract_add_default_project_id_index.py @@ -0,0 +1,15 @@ +# 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. + + +def upgrade(migrate_engine): + pass diff --git a/keystone/common/sql/data_migration_repo/versions/022_migrate_add_default_project_id_index.py b/keystone/common/sql/data_migration_repo/versions/022_migrate_add_default_project_id_index.py new file mode 100644 index 0000000000..8aa15c1ef2 --- /dev/null +++ b/keystone/common/sql/data_migration_repo/versions/022_migrate_add_default_project_id_index.py @@ -0,0 +1,15 @@ +# 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. + + +def upgrade(migrate_engine): + pass diff --git a/keystone/common/sql/expand_repo/versions/022_expand_add_default_project_id_index.py b/keystone/common/sql/expand_repo/versions/022_expand_add_default_project_id_index.py new file mode 100644 index 0000000000..37413d0f9c --- /dev/null +++ b/keystone/common/sql/expand_repo/versions/022_expand_add_default_project_id_index.py @@ -0,0 +1,21 @@ +# 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 as sql + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + user = sql.Table('user', meta, autoload=True) + sql.Index('ix_default_project_id', user.c.default_project_id).create() diff --git a/keystone/identity/backends/base.py b/keystone/identity/backends/base.py index eb600be9eb..c4252db53f 100644 --- a/keystone/identity/backends/base.py +++ b/keystone/identity/backends/base.py @@ -221,6 +221,15 @@ class IdentityDriverBase(object): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def unset_default_project_id(self, project_id): + """Unset a users default project given a specific project ID. + + :param str project_id: project ID + + """ + raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod def list_users_in_group(self, group_id, hints): """List users in a group. diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py index d029ddcd5e..a146647175 100644 --- a/keystone/identity/backends/ldap/core.py +++ b/keystone/identity/backends/ldap/core.py @@ -87,6 +87,11 @@ class Identity(base.IdentityDriverBase): def list_users(self, hints): return self.user.get_all_filtered(hints) + def unset_default_project_id(self, project_id): + # This function is not implemented for the LDAP backend + # LDAP backend is readonly. + self._disallow_write() + def get_user_by_name(self, user_name, domain_id): # domain_id will already have been handled in the Manager layer, # parameter left in so this matches the Driver specification diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index 70c8b32517..ad5b016b23 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -170,6 +170,14 @@ class Identity(base.IdentityDriverBase): user_refs = sql.filter_limit_query(model.User, query, hints) return [base.filter_user(x.to_dict()) for x in user_refs] + def unset_default_project_id(self, project_id): + with sql.session_for_write() as session: + query = session.query(model.User) + query = query.filter(model.User.default_project_id == project_id) + + for user in query: + user.default_project_id = None + def _get_user(self, session, user_id): user_ref = session.query(model.User).get(user_id) if not user_ref: diff --git a/keystone/identity/backends/sql_model.py b/keystone/identity/backends/sql_model.py index 8dd9c9bb88..fe9ebe8ddb 100644 --- a/keystone/identity/backends/sql_model.py +++ b/keystone/identity/backends/sql_model.py @@ -39,7 +39,7 @@ class User(sql.ModelBase, sql.DictBase): domain_id = sql.Column(sql.String(64), nullable=False) _enabled = sql.Column('enabled', sql.Boolean) extra = sql.Column(sql.JsonBlob()) - default_project_id = sql.Column(sql.String(64)) + default_project_id = sql.Column(sql.String(64), index=True) _resource_option_mapper = orm.relationship( 'UserOption', single_parent=True, diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 2b9cea8f5e..7da04c49a3 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -490,6 +490,7 @@ class Manager(manager.Manager): self.event_callbacks = { notifications.ACTIONS.deleted: { 'domain': [self._domain_deleted], + 'project': [self._set_default_project_to_none], }, } @@ -520,8 +521,21 @@ class Manager(manager.Manager): 'cleanup.'), {'userid': user['id'], 'domainid': domain_id}) - # Domain ID normalization methods + def _set_default_project_to_none(self, service, resource_type, operation, + payload): + """Callback, clears user default_project_id after project deletion. + Notification approach was used instead of using a FK constraint. + Reason being, operators are allowed to have separate backends for + various keystone subsystems. This doesn't guarantee that projects and + users will be stored in the same backend, meaning we can't rely on FK + constraints to do this work for us. + + """ + project_id = payload['resource_info'] + self.driver.unset_default_project_id(project_id) + + # Domain ID normalization methods def _set_domain_id_and_mapping(self, ref, domain_id, driver, entity_type): """Patch the domain_id/public_id into the resulting entity(ies). diff --git a/keystone/releasenotes/notes/unset-user-default_project_id-when-project-is-deleted-4d42c841b6e7e54e.yaml b/keystone/releasenotes/notes/unset-user-default_project_id-when-project-is-deleted-4d42c841b6e7e54e.yaml new file mode 100644 index 0000000000..569abc9b63 --- /dev/null +++ b/keystone/releasenotes/notes/unset-user-default_project_id-when-project-is-deleted-4d42c841b6e7e54e.yaml @@ -0,0 +1,14 @@ +fixes: + - | + [`bug 1523369 `_] + Currently, if a project is deleted, it is not removed as a user's default + project id. Now the default project id is set to none, however changes may + not be visible until memcache end of life. + +upgrade: + - | + The identity backend driver interface has changed. We've added a new + ``unset_default_project_id(project_id)`` method to unset a users default + project id matching the given project id. If you have a custom + implementation for the identity driver, you will need to implement this + new method. diff --git a/keystone/tests/unit/resource/test_backends.py b/keystone/tests/unit/resource/test_backends.py index 102606c61f..bf0c5d12d8 100644 --- a/keystone/tests/unit/resource/test_backends.py +++ b/keystone/tests/unit/resource/test_backends.py @@ -1505,8 +1505,8 @@ class ResourceTests(object): project_ref = self.resource_api.get_project(project['id']) self.assertDictEqual(updated_project_ref, project_ref) - @test_utils.wip('waiting for fix to bug #1523369') def test_delete_project_clears_default_project_id(self): + self.config_fixture.config(group='cache', enabled=False) project = unit.new_project_ref( domain_id=CONF.identity.default_domain_id) user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id, @@ -1514,13 +1514,14 @@ class ResourceTests(object): self.resource_api.create_project(project['id'], project) user = self.identity_api.create_user(user) user = self.identity_api.get_user(user['id']) - self.assertIsNotNone(user['default_project_id']) - self.resource_api.delete_project(project['id']) - user = self.identity_api.get_user(user['id']) - self.assertIsNone(user['default_project_id']) + # LDAP is read only default_project_id doesn't exist + if 'default_project_id' in user: + self.assertIsNotNone(user['default_project_id']) + self.resource_api.delete_project(project['id']) + user = self.identity_api.get_user(user['id']) + self.assertNotIn('default_project_id', user) - @test_utils.wip('waiting for fix to bug #1523369') def test_delete_project_with_roles_clears_default_project_id(self): project = unit.new_project_ref( domain_id=CONF.identity.default_domain_id) @@ -1533,10 +1534,9 @@ class ResourceTests(object): self.assignment_api.create_grant(user_id=user['id'], project_id=project['id'], role_id=role['id']) - self.resource_api.delete_project(project['id']) user = self.identity_api.get_user(user['id']) - self.assertIsNone(user['default_project_id']) + self.assertNotIn('default_project_id', user) class ResourceDriverTests(object):