From 3408515e8c1ea0c7716de0d65824e64016d021f7 Mon Sep 17 00:00:00 2001 From: Henry Nash Date: Sun, 28 Dec 2014 17:12:59 +0000 Subject: [PATCH] Split roles into their own backend within assignments This is the first part of the more comprehensive split of assignments, which rationalizes both the backend and controllers. In order to make this change easier for reviewers, it is divided into a number of smaller patches. Follow-on patches will: - Fix incorrect doc strings for grant driver methods - Update unit tests to call the new role manager - Update the assignment controller to call the role manager - Refactor assignment manager and driver methods to logically separate project/domains from the actual assignments - Split projects and domains into their own backend - Split the controllers so they call the correct manager - Update the tests to call the new correct manager Partially implements: bp pluggable-assignments Change-Id: I41fc23a049c26e514222a966c1847e183448be00 --- doc/source/architecture.rst | 1 + doc/source/configuration.rst | 48 ++- etc/keystone.conf.sample | 22 ++ keystone/assignment/backends/ldap.py | 108 ++---- keystone/assignment/backends/sql.py | 122 ++---- keystone/assignment/core.py | 360 ++++++++++++------ keystone/assignment/role_backends/__init__.py | 0 keystone/assignment/role_backends/ldap.py | 124 ++++++ keystone/assignment/role_backends/sql.py | 80 ++++ keystone/backends.py | 1 + keystone/common/config.py | 16 + .../versions/062_drop_assignment_role_fk.py | 41 ++ keystone/tests/test_backend.py | 13 +- keystone/tests/test_backend_ldap.py | 20 +- keystone/tests/test_backend_sql.py | 34 ++ keystone/tests/test_sql_upgrade.py | 17 + 16 files changed, 680 insertions(+), 327 deletions(-) create mode 100644 keystone/assignment/role_backends/__init__.py create mode 100644 keystone/assignment/role_backends/ldap.py create mode 100644 keystone/assignment/role_backends/sql.py create mode 100644 keystone/common/sql/migrate_repo/versions/062_drop_assignment_role_fk.py diff --git a/doc/source/architecture.rst b/doc/source/architecture.rst index 8df7f71e70..6026cc1367 100644 --- a/doc/source/architecture.rst +++ b/doc/source/architecture.rst @@ -135,6 +135,7 @@ abstract base class for any implementations, identifying the expected service implementations. The drivers for the services are: * :mod:`keystone.assignment.core.Driver` +* :mod:`keystone.assignment.core.RoleDriver` * :mod:`keystone.catalog.core.Driver` * :mod:`keystone.identity.core.Driver` * :mod:`keystone.policy.core.Driver` diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index a2884bc372..671b0ec155 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -109,6 +109,7 @@ configuration file is organized into the following sections: * ``[paste_deploy]`` - Pointer to the PasteDeploy configuration file * ``[policy]`` - Policy system driver configuration for RBAC * ``[revoke]`` - Revocation system driver configuration +* ``[role]`` - Role system driver configuration * ``[saml]`` - SAML configuration options * ``[signing]`` - Cryptographic signatures for PKI based tokens * ``[ssl]`` - SSL configuration @@ -505,9 +506,9 @@ Current Keystone systems that have caching capabilities: from the other systems in ``Keystone``. This option is set in the ``[assignment]`` section of the configuration file. - Currently ``assignment`` has caching for ``project``, ``domain``, and - ``role`` specific requests (primarily around the CRUD actions). Caching - is currently not implemented on grants. The list (``list_projects``, + Currently ``assignment`` has caching for ``project`` and ``domain`` + specific requests (primarily around the CRUD actions). Caching is + currently not implemented on grants. The list (``list_projects``, ``list_domains``, etc) methods are not subject to caching. .. WARNING:: @@ -521,6 +522,24 @@ Current Keystone systems that have caching capabilities: recommended that caching be disabled on ``assignment``. To disable caching specifically on ``assignment``, in the ``[assignment]`` section of the configuration set ``caching`` to ``False``. + * ``role`` + Currently ``role`` has caching for ``get_role``, but not for ``list_roles``. + The role system has a separate ``cache_time`` configuration option, + that can be set to a value above or below the global ``expiration_time`` + default, allowing for different caching behavior from the other systems in + ``Keystone``. This option is set in the ``[role]`` section of the + configuration file. + + .. WARNING:: + Be aware that if a read-only ``role`` backend is in use, the cache + will not immediately reflect changes on the back end. Any given change + may take up to the ``cache_time`` (if set in the ``[role]`` + section of the configuration) or the global ``expiration_time`` (set in + the ``[cache]`` section of the configuration) before it is reflected. + If this type of delay (when using a read-only ``role`` backend) is + an issue, it is recommended that caching be disabled on ``role``. + To disable caching specifically on ``role``, in the ``[role]`` + section of the configuration set ``caching`` to ``False``. For more information about the different backends (and configuration options): * `dogpile.cache.backends.memory`_ @@ -1508,8 +1527,8 @@ directories in conjunction with reading user and group information. Keystone now provides an option whereby these read-only directories can be easily integrated as it now enables its identity entities (which comprises users, groups, and group memberships) to be served out of directories while -assignments (which comprises projects, roles, role assignments, and domains) -are to be served from a different Keystone backend (i.e. SQL). To enable this +assignments (which comprises projects, role assignments, and domains) and roles +are to be served from different Keystone backends (i.e. SQL). To enable this option, you must have the following ``keystone.conf`` options set: .. code-block:: ini @@ -1520,17 +1539,20 @@ option, you must have the following ``keystone.conf`` options set: [assignment] driver = keystone.assignment.backends.sql.Assignment + [role] + driver = keystone.assignment.role_backends.sql.Role + With the above configuration, Keystone will only lookup identity related information such users, groups, and group membership from the directory, while assignment related information will be provided by the SQL backend. Also note -that if there is an LDAP Identity, and no assignment backend is specified, the -assignment backend will default to LDAP. Although this may seem -counterintuitive, it is provided for backwards compatibility. Nonetheless, the -explicit option will always override the implicit option, so specifying the -options as shown above will always be correct. Finally, it is also worth noting -that whether or not the LDAP accessible directory is to be considered read only -is still configured as described in a previous section above by setting values -such as the following in the ``[ldap]`` configuration section: +that if there is an LDAP Identity, and no assignment or role backend is +specified, they will default to LDAP. Although this may seem counterintuitive, +it is provided for backwards compatibility. Nonetheless, the explicit option +will always override the implicit option, so specifying the options as shown +above will always be correct. Finally, it is also worth noting that whether or +not the LDAP accessible directory is to be considered read only is still +configured as described in a previous section above by setting values such as +the following in the ``[ldap]`` configuration section: .. code-block:: ini diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample index eac27eef31..8b7081e6fc 100644 --- a/etc/keystone.conf.sample +++ b/etc/keystone.conf.sample @@ -1289,6 +1289,28 @@ #caching = true +[role] + +# +# From keystone +# + +# Role backend driver. (string value) +#driver = + +# Toggle for role caching. This has no effect unless global caching is enabled. +# (boolean value) +#caching = true + +# TTL (in seconds) to cache role data. This has no effect unless global caching +# is enabled. (integer value) +#cache_time = + +# Maximum number of entities that will be returned in a role collection. +# (integer value) +#list_limit = + + [saml] # diff --git a/keystone/assignment/backends/ldap.py b/keystone/assignment/backends/ldap.py index 03a21a53ac..4d9fb90df3 100644 --- a/keystone/assignment/backends/ldap.py +++ b/keystone/assignment/backends/ldap.py @@ -19,6 +19,7 @@ import ldap as ldap import ldap.filter from keystone import assignment +from keystone.assignment.role_backends import ldap as ldap_role from keystone import clean from keystone.common import driver_hints from keystone.common import ldap as common_ldap @@ -51,6 +52,9 @@ class Assignment(assignment.Driver): self.project = ProjectApi(CONF) self.role = RoleApi(CONF, self.user) + def default_role_driver(self): + return 'keystone.assignment.role_backends.ldap.Role' + def _set_default_parent_project(self, ref): """If the parent project ID has not been set, set it to None.""" if isinstance(ref, dict): @@ -129,7 +133,8 @@ class Assignment(assignment.Driver): return self._set_default_attributes( self.project.update(tenant_id, tenant)) - def get_group_project_roles(self, groups, project_id, project_domain_id): + def list_role_ids_for_groups_on_project( + self, groups, project_id, project_domain_id): self.get_project(project_id) group_dns = [self.group._id_to_dn(group_id) for group_id in groups] role_list = [self.role._dn_to_id(role_assignment.role_dn) @@ -178,12 +183,6 @@ class Assignment(assignment.Driver): return {} return {'roles': [self._role_to_dict(r, False) for r in metadata_ref]} - def get_role(self, role_id): - return self.role.get(role_id) - - def list_roles(self, hints): - return self.role.get_all() - def list_projects_for_user(self, user_id, group_ids, hints): user_dn = self.user._id_to_dn(user_id) associations = (self.role.list_project_roles_for_user @@ -201,7 +200,8 @@ class Assignment(assignment.Driver): return [self._set_default_attributes(x) for x in self.project.get_user_projects(user_dn, associations)] - def get_roles_for_groups(self, group_ids, project_id=None, domain_id=None): + def list_role_ids_for_groups( + self, group_ids, project_id=None, domain_id=None): raise exception.NotImplemented() def list_projects_for_groups(self, group_ids): @@ -230,7 +230,6 @@ class Assignment(assignment.Driver): def add_role_to_user_and_project(self, user_id, tenant_id, role_id): self.get_project(tenant_id) - self.get_role(role_id) user_dn = self.user._id_to_dn(user_id) role_dn = self._subrole_id_to_dn(role_id, tenant_id) self.role.add_user(role_id, role_dn, user_dn, user_id, tenant_id) @@ -241,7 +240,6 @@ class Assignment(assignment.Driver): def _add_role_to_group_and_project(self, group_id, tenant_id, role_id): self.get_project(tenant_id) - self.get_role(role_id) group_dn = self.group._id_to_dn(group_id) role_dn = self._subrole_id_to_dn(role_id, tenant_id) self.role.add_user(role_id, role_dn, group_dn, group_id, tenant_id) @@ -250,30 +248,6 @@ class Assignment(assignment.Driver): role_dn=role_dn, tenant_dn=tenant_dn) - def create_role(self, role_id, role): - self.role.check_allow_create() - try: - self.get_role(role_id) - except exception.NotFound: - pass - else: - msg = _('Duplicate ID, %s.') % role_id - raise exception.Conflict(type='role', details=msg) - - try: - self.role.get_by_name(role['name']) - except exception.NotFound: - pass - else: - msg = _('Duplicate name, %s.') % role['name'] - raise exception.Conflict(type='role', details=msg) - - return self.role.create(role) - - def delete_role(self, role_id): - self.role.check_allow_delete() - return self.role.delete(role_id, self.project.tree_dn) - def delete_project(self, tenant_id): self.project.check_allow_delete() if self.project.subtree_delete_enabled: @@ -294,11 +268,6 @@ class Assignment(assignment.Driver): return self.role.delete_user(role_dn, self.group._id_to_dn(group_id), role_id) - def update_role(self, role_id, role): - self.role.check_allow_update() - self.get_role(role_id) - return self.role.update(role_id, role) - def create_domain(self, domain_id, domain): if domain_id == CONF.identity.default_domain_id: msg = _('Duplicate ID, %s.') % domain_id @@ -347,7 +316,6 @@ class Assignment(assignment.Driver): def create_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, inherited_to_projects=False): - self.get_role(role_id) if domain_id: self.get_domain(domain_id) @@ -367,10 +335,9 @@ class Assignment(assignment.Driver): metadata_ref['roles'] = self.add_role_to_user_and_project( user_id, project_id, role_id) - def get_grant(self, role_id, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): - role_ref = self.get_role(role_id) + def check_grant_role_id(self, role_id, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): if domain_id: self.get_domain(domain_id) @@ -386,12 +353,10 @@ class Assignment(assignment.Driver): metadata_ref.get('roles', []), inherited_to_projects)) if role_id not in role_ids: raise exception.RoleNotFound(role_id=role_id) - return role_ref def delete_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, inherited_to_projects=False): - self.get_role(role_id) if domain_id: self.get_domain(domain_id) @@ -415,9 +380,9 @@ class Assignment(assignment.Driver): except KeyError: raise exception.RoleNotFound(role_id=role_id) - def list_grants(self, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): + def list_grant_role_ids(self, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): if domain_id: self.get_domain(domain_id) if project_id: @@ -429,9 +394,8 @@ class Assignment(assignment.Driver): except exception.MetadataNotFound: metadata_ref = {} - return [self.get_role(role_id) for role_id in - self._roles_from_role_dicts(metadata_ref.get('roles', []), - inherited_to_projects)] + return self._roles_from_role_dicts(metadata_ref.get('roles', []), + inherited_to_projects) def get_domain_by_name(self, domain_name): default_domain = assignment.calc_default_domain() @@ -455,6 +419,9 @@ class Assignment(assignment.Driver): role_assignments.append(assignment) return role_assignments + def delete_role_assignments(self, role_id): + self.role.roles_delete_subtree_by_role(role_id, self.project.tree_dn) + # TODO(termie): turn this into a data object and move logic to driver class ProjectApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): @@ -540,16 +507,9 @@ class GroupRoleAssociation(object): # TODO(termie): turn this into a data object and move logic to driver -class RoleApi(common_ldap.BaseLdap): - DEFAULT_OU = 'ou=Roles' - DEFAULT_STRUCTURAL_CLASSES = [] - DEFAULT_OBJECTCLASS = 'organizationalRole' - DEFAULT_MEMBER_ATTRIBUTE = 'roleOccupant' - NotFound = exception.RoleNotFound - options_name = 'role' - attribute_options_names = {'name': 'name'} - immutable_attrs = ['id'] - model = models.Role +# NOTE(heny-nash): The RoleLdapStructureMixin class enables the sharing of the +# LDAP structure between here and the role backend LDAP, no methods are shared. +class RoleApi(ldap_role.RoleLdapStructureMixin, common_ldap.BaseLdap): def __init__(self, conf, user_api): super(RoleApi, self).__init__(conf) @@ -557,13 +517,6 @@ class RoleApi(common_ldap.BaseLdap): or self.DEFAULT_MEMBER_ATTRIBUTE) self._user_api = user_api - def get(self, role_id, role_filter=None): - model = super(RoleApi, self).get(role_id, role_filter) - return model - - def create(self, values): - return super(RoleApi, self).create(values) - def add_user(self, role_id, role_dn, user_dn, user_id, tenant_id=None): try: super(RoleApi, self).add_member(user_dn, role_dn) @@ -682,22 +635,9 @@ class RoleApi(common_ldap.BaseLdap): def roles_delete_subtree_by_project(self, tenant_dn): self._delete_tree_nodes(tenant_dn, ldap.SCOPE_ONELEVEL) - def update(self, role_id, role): - new_name = role.get('name') - if new_name is not None: - try: - old_role = self.get_by_name(new_name) - if old_role['id'] != role_id: - raise exception.Conflict( - _('Cannot duplicate name %s') % old_role) - except exception.NotFound: - pass - return super(RoleApi, self).update(role_id, role) - - def delete(self, role_id, tenant_dn): - self._delete_tree_nodes(tenant_dn, ldap.SCOPE_SUBTREE, query_params={ + def roles_delete_subtree_by_role(self, role_id, tree_dn): + self._delete_tree_nodes(tree_dn, ldap.SCOPE_SUBTREE, query_params={ self.id_attr: role_id}) - super(RoleApi, self).delete(role_id) def list_role_assignments(self, project_tree_dn): """Returns a list of all the role assignments linked to project_tree_dn diff --git a/keystone/assignment/backends/sql.py b/keystone/assignment/backends/sql.py index 65e7336323..14fee729dd 100644 --- a/keystone/assignment/backends/sql.py +++ b/keystone/assignment/backends/sql.py @@ -38,6 +38,9 @@ class AssignmentType(object): class Assignment(keystone_assignment.Driver): + def default_role_driver(self): + return "keystone.assignment.role_backends.sql.Role" + def _get_project(self, session, project_id): project_ref = session.query(Project).get(project_id) if project_ref is None: @@ -135,7 +138,6 @@ class Assignment(keystone_assignment.Driver): 'User, Group, Project, Domain: %s') % message_data) with sql.transaction() as session: - self._get_role(session, role_id) if domain_id: self._get_domain(session, domain_id) @@ -155,21 +157,20 @@ class Assignment(keystone_assignment.Driver): # The v3 grant APIs are silent if the assignment already exists pass - def list_grants(self, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): + def list_grant_role_ids(self, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): with sql.transaction() as session: if domain_id: self._get_domain(session, domain_id) if project_id: self._get_project(session, project_id) - q = session.query(Role).join(RoleAssignment) + q = session.query(RoleAssignment.role_id) q = q.filter(RoleAssignment.actor_id == (user_id or group_id)) q = q.filter(RoleAssignment.target_id == (project_id or domain_id)) q = q.filter(RoleAssignment.inherited == inherited_to_projects) - q = q.filter(Role.id == RoleAssignment.role_id) - return [x.to_dict() for x in q.all()] + return [x.role_id for x in q.all()] def _build_grant_filter(self, session, role_id, user_id, group_id, domain_id, project_id, inherited_to_projects): @@ -180,11 +181,10 @@ class Assignment(keystone_assignment.Driver): q = q.filter_by(inherited=inherited_to_projects) return q - def get_grant(self, role_id, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): + def check_grant_role_id(self, role_id, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): with sql.transaction() as session: - role_ref = self._get_role(session, role_id) if domain_id: self._get_domain(session, domain_id) if project_id: @@ -198,13 +198,10 @@ class Assignment(keystone_assignment.Driver): except sql.NotFound: raise exception.RoleNotFound(role_id=role_id) - return role_ref.to_dict() - def delete_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, inherited_to_projects=False): with sql.transaction() as session: - self._get_role(session, role_id) if domain_id: self._get_domain(session, domain_id) if project_id: @@ -376,48 +373,37 @@ class Assignment(keystone_assignment.Driver): project_refs = self._get_children(session, [project_id]) return not project_refs - def get_roles_for_groups(self, group_ids, project_id=None, domain_id=None): + def list_role_ids_for_groups( + self, group_ids, project_id=None, domain_id=None): - def _get_roles_for_groups_on_domain(group_ids, domain_id): + def _get_role_ids_for_groups_on_domain(group_ids, domain_id): sql_constraints = sqlalchemy.and_( RoleAssignment.type == AssignmentType.GROUP_DOMAIN, RoleAssignment.target_id == domain_id, RoleAssignment.inherited == false(), - Role.id == RoleAssignment.role_id, RoleAssignment.actor_id.in_(group_ids)) session = sql.get_session() with session.begin(): - query = session.query(Role).filter( + query = session.query(RoleAssignment.role_id).filter( sql_constraints).distinct() - return [role.to_dict() for role in query.all()] - - def _get_roles_for_groups_on_project(group_ids, project_id): - - def _role_ids_to_dicts(session, ids): - if not ids: - return [] - else: - query = session.query(Role) - query = query.filter(Role.id.in_(ids)) - role_refs = query.all() - return [role_ref.to_dict() for role_ref in role_refs] + return [assignment.role_id for assignment in query.all()] + def _get_role_ids_for_groups_on_project(group_ids, project_id): with sql.transaction() as session: project = self._get_project(session, project_id) - role_ids = self._get_group_project_roles( + return self._get_group_project_role_ids( session, group_ids, project_id, project.domain_id) - return _role_ids_to_dicts(session, role_ids) if project_id is not None: - return _get_roles_for_groups_on_project(group_ids, project_id) + return _get_role_ids_for_groups_on_project(group_ids, project_id) elif domain_id is not None: - return _get_roles_for_groups_on_domain(group_ids, domain_id) + return _get_role_ids_for_groups_on_domain(group_ids, domain_id) else: raise AttributeError(_("Must specify either domain or project")) - def _get_group_project_roles(self, session, groups, project_id, - project_domain_id): + def _get_group_project_role_ids(self, session, groups, project_id, + project_domain_id): if not groups: # If there's no groups then there will be no project roles. return [] @@ -457,10 +443,11 @@ class Assignment(keystone_assignment.Driver): return [result.role_id for result in query.all()] - def get_group_project_roles(self, groups, project_id, project_domain_id): + def list_role_ids_for_groups_on_project( + self, groups, project_id, project_domain_id): with sql.transaction() as session: - return self._get_group_project_roles(session, groups, project_id, - project_domain_id) + return self._get_group_project_role_ids( + session, groups, project_id, project_domain_id) def list_projects_for_groups(self, group_ids): with sql.transaction() as session: @@ -516,7 +503,6 @@ class Assignment(keystone_assignment.Driver): def add_role_to_user_and_project(self, user_id, tenant_id, role_id): with sql.transaction() as session: self._get_project(session, tenant_id) - self._get_role(session, role_id) try: with sql.transaction() as session: @@ -667,53 +653,11 @@ class Assignment(keystone_assignment.Driver): session.delete(ref) - # role crud - - @sql.handle_conflicts(conflict_type='role') - def create_role(self, role_id, role): + def delete_role_assignments(self, role_id): with sql.transaction() as session: - ref = Role.from_dict(role) - session.add(ref) - return ref.to_dict() - - @sql.truncated - def list_roles(self, hints): - with sql.transaction() as session: - query = session.query(Role) - refs = sql.filter_limit_query(Role, query, hints) - return [ref.to_dict() for ref in refs] - - def _get_role(self, session, role_id): - ref = session.query(Role).get(role_id) - if ref is None: - raise exception.RoleNotFound(role_id=role_id) - return ref - - def get_role(self, role_id): - with sql.transaction() as session: - return self._get_role(session, role_id).to_dict() - - @sql.handle_conflicts(conflict_type='role') - def update_role(self, role_id, role): - with sql.transaction() as session: - ref = self._get_role(session, role_id) - old_dict = ref.to_dict() - for k in role: - old_dict[k] = role[k] - new_role = Role.from_dict(old_dict) - for attr in Role.attributes: - if attr != 'id': - setattr(ref, attr, getattr(new_role, attr)) - ref.extra = new_role.extra - return ref.to_dict() - - def delete_role(self, role_id): - with sql.transaction() as session: - ref = self._get_role(session, role_id) q = session.query(RoleAssignment) q = q.filter_by(role_id=role_id) q.delete(False) - session.delete(ref) def delete_user(self, user_id): with sql.transaction() as session: @@ -755,15 +699,6 @@ class Project(sql.ModelBase, sql.DictBase): __table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {}) -class Role(sql.ModelBase, sql.DictBase): - __tablename__ = 'role' - attributes = ['id', 'name'] - id = sql.Column(sql.String(64), primary_key=True) - name = sql.Column(sql.String(255), unique=True, nullable=False) - extra = sql.Column(sql.JsonBlob()) - __table_args__ = (sql.UniqueConstraint('name'), {}) - - class RoleAssignment(sql.ModelBase, sql.DictBase): __tablename__ = 'assignment' attributes = ['type', 'actor_id', 'target_id', 'role_id', 'inherited'] @@ -775,8 +710,7 @@ class RoleAssignment(sql.ModelBase, sql.DictBase): nullable=False) actor_id = sql.Column(sql.String(64), nullable=False, index=True) target_id = sql.Column(sql.String(64), nullable=False) - role_id = sql.Column(sql.String(64), sql.ForeignKey('role.id'), - nullable=False) + role_id = sql.Column(sql.String(64), nullable=False) inherited = sql.Column(sql.Boolean, default=False, nullable=False) __table_args__ = (sql.PrimaryKeyConstraint('type', 'actor_id', 'target_id', 'role_id'), {}) diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index bd11962e1d..763be0dd9b 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -29,6 +29,7 @@ from keystone.i18n import _ from keystone.i18n import _LE, _LI from keystone import notifications from keystone.openstack.common import log +from keystone.openstack.common import versionutils CONF = config.CONF @@ -48,9 +49,26 @@ def calc_default_domain(): 'name': u'Default'} +def deprecated_to_role_api(f): + """Specialized deprecation wrapper for assignment to role api. + + This wraps the standard deprecation wrapper and fills in the method + names automatically. + + """ + @six.wraps(f) + def wrapper(*args, **kwargs): + x = versionutils.deprecated( + what='assignment.' + f.__name__ + '()', + as_of=versionutils.deprecated.KILO, + in_favor_of='role.' + f.__name__ + '()') + return x(f) + return wrapper() + + @dependency.provider('assignment_api') @dependency.optional('revoke_api') -@dependency.requires('credential_api', 'identity_api') +@dependency.requires('credential_api', 'identity_api', 'role_api') class Manager(manager.Manager): """Default pivot point for the Assignment backend. @@ -239,7 +257,7 @@ class Manager(manager.Manager): """ def _get_group_project_roles(user_id, project_ref): group_ids = self._get_group_ids_for_user_id(user_id) - return self.driver.get_group_project_roles( + return self.driver.list_role_ids_for_groups_on_project( group_ids, project_ref['id'], project_ref['domain_id']) @@ -322,6 +340,15 @@ class Manager(manager.Manager): # Use set() to process the list to remove any duplicates return list(set(user_role_list + group_role_list)) + def get_roles_for_groups(self, group_ids, project_id=None, domain_id=None): + """Get a list of roles for this group on domain and/or project.""" + + # TODO(henry-nash): We should pull up the algorithm for this method + # from the drivers to here. + role_ids = self.list_role_ids_for_groups( + group_ids, project_id, domain_id) + return self.role_api.list_roles_from_ids(role_ids) + def add_user_to_project(self, tenant_id, user_id): """Add user to a tenant by creating a default role relationship. @@ -330,6 +357,7 @@ class Manager(manager.Manager): """ try: + self.role_api.get_role(config.CONF.member_role_id) self.driver.add_role_to_user_and_project( user_id, tenant_id, @@ -340,13 +368,17 @@ class Manager(manager.Manager): config.CONF.member_role_id) role = {'id': CONF.member_role_id, 'name': CONF.member_role_name} - self.driver.create_role(config.CONF.member_role_id, role) + self.role_api.create_role(config.CONF.member_role_id, role) # now that default role exists, the add should succeed self.driver.add_role_to_user_and_project( user_id, tenant_id, config.CONF.member_role_id) + def add_role_to_user_and_project(self, user_id, tenant_id, role_id): + self.role_api.get_role(role_id) + self.driver.add_role_to_user_and_project(user_id, tenant_id, role_id) + def remove_user_from_project(self, tenant_id, user_id): """Remove user from a tenant @@ -598,45 +630,6 @@ class Manager(manager.Manager): def get_project_by_name(self, tenant_name, domain_id): return self.driver.get_project_by_name(tenant_name, domain_id) - @cache.on_arguments(should_cache_fn=SHOULD_CACHE, - expiration_time=EXPIRATION_TIME) - def get_role(self, role_id): - return self.driver.get_role(role_id) - - @notifications.created('role') - def create_role(self, role_id, role): - ret = self.driver.create_role(role_id, role) - if SHOULD_CACHE(ret): - self.get_role.set(ret, self, role_id) - return ret - - @manager.response_truncated - def list_roles(self, hints=None): - return self.driver.list_roles(hints or driver_hints.Hints()) - - @notifications.updated('role') - def update_role(self, role_id, role): - ret = self.driver.update_role(role_id, role) - self.get_role.invalidate(self, role_id) - return ret - - @notifications.deleted('role') - def delete_role(self, role_id): - try: - self._delete_tokens_for_role(role_id) - except exception.NotImplemented: - # FIXME(morganfainberg): Not all backends (ldap) implement - # `list_role_assignments_for_role` which would have previously - # caused a NotImplmented error to be raised when called through - # the controller. Now error or proper action will always come from - # the `delete_role` method logic. Work needs to be done to make - # the behavior between drivers consistent (capable of revoking - # tokens for the same circumstances). This is related to the bug - # https://bugs.launchpad.net/keystone/+bug/1221805 - pass - self.driver.delete_role(role_id) - self.get_role.invalidate(self, role_id) - def list_role_assignments_for_role(self, role_id=None): # NOTE(henry-nash): Currently the efficiency of the key driver # implementation (SQL) of list_role_assignments is severely hampered by @@ -664,9 +657,26 @@ class Manager(manager.Manager): def create_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, inherited_to_projects=False, context=None): + self.role_api.get_role(role_id) self.driver.create_grant(role_id, user_id, group_id, domain_id, project_id, inherited_to_projects) + def get_grant(self, role_id, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): + role_ref = self.role_api.get_role(role_id) + self.driver.check_grant_role_id( + role_id, user_id, group_id, domain_id, project_id, + inherited_to_projects) + return role_ref + + def list_grants(self, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): + grant_ids = self.driver.list_grant_role_ids( + user_id, group_id, domain_id, project_id, inherited_to_projects) + return self.role_api.list_roles_from_ids(grant_ids) + @notifications.role_assignment('deleted') def delete_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, @@ -693,12 +703,17 @@ class Manager(manager.Manager): LOG.debug('Group %s not found, no tokens to invalidate.', group_id) + # TODO(henry-nash): While having the call to get_role here mimics the + # previous behavior (when it was buried inside the driver delete call), + # this seems an odd place to have this check, given what we have + # already done so far in this method. See Bug #1406776. + self.role_api.get_role(role_id) self.driver.delete_grant(role_id, user_id, group_id, domain_id, project_id, inherited_to_projects) if user_id is not None: self._emit_invalidate_user_token_persistence(user_id) - def _delete_tokens_for_role(self, role_id): + def delete_tokens_for_role_assignments(self, role_id): assignments = self.list_role_assignments_for_role(role_id=role_id) # Iterate over the assignments for this role and build the list of @@ -770,6 +785,26 @@ class Manager(manager.Manager): # from persistence if persistence is enabled. pass + @deprecated_to_role_api + def create_role(self, role_id, role): + return self.role_api.create_role(role_id, role) + + @deprecated_to_role_api + def get_role(self, role_id): + return self.role_api.get_role(role_id) + + @deprecated_to_role_api + def update_role(self, role_id, role): + return self.role_api.update_role(role_id, role) + + @deprecated_to_role_api + def delete_role(self, role_id): + return self.role_api.delete_role(role_id) + + @deprecated_to_role_api + def list_roles(self, hints=None): + return self.role_api.list_roles(hints=hints) + @six.add_metaclass(abc.ABCMeta) class Driver(object): @@ -811,6 +846,10 @@ class Driver(object): def _get_list_limit(self): return CONF.assignment.list_limit or CONF.list_limit + # TODO(henry-nash): A number of the abstract methods incorrectly list + # User/GroupNotFound as possible exceptions, even though we no longer + # check for these in the driver methods. This is raised as bug #1406393. + @abc.abstractmethod def get_project_by_name(self, tenant_name, domain_id): """Get a tenant by name. @@ -836,8 +875,8 @@ class Driver(object): """Add a role to a user within given tenant. :raises: keystone.exception.UserNotFound, - keystone.exception.ProjectNotFound, - keystone.exception.RoleNotFound + keystone.exception.ProjectNotFound + """ raise exception.NotImplemented() # pragma: no cover @@ -865,38 +904,37 @@ class Driver(object): the OS-INHERIT extension to be enabled). :raises: keystone.exception.DomainNotFound, - keystone.exception.ProjectNotFound, - keystone.exception.RoleNotFound + keystone.exception.ProjectNotFound """ raise exception.NotImplemented() # pragma: no cover @abc.abstractmethod - def list_grants(self, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): - """Lists assignments/grants. - - :raises: keystone.exception.UserNotFound, - keystone.exception.GroupNotFound, - keystone.exception.ProjectNotFound, - keystone.exception.DomainNotFound, - keystone.exception.RoleNotFound - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_grant(self, role_id, user_id=None, group_id=None, - domain_id=None, project_id=None, - inherited_to_projects=False): + def list_grant_role_ids(self, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): """Lists assignments/grants. + :raises: keystone.exception.UserNotFound, + keystone.exception.GroupNotFound, + keystone.exception.ProjectNotFound, + keystone.exception.DomainNotFound + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def check_grant_role_id(self, role_id, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): + """Checks an assignment/grant role id. + :raises: keystone.exception.UserNotFound, keystone.exception.GroupNotFound, keystone.exception.ProjectNotFound, keystone.exception.DomainNotFound, keystone.exception.RoleNotFound + :returns: None or raises an exception if grant not found """ raise exception.NotImplemented() # pragma: no cover @@ -1070,8 +1108,9 @@ class Driver(object): raise exception.NotImplemented() @abc.abstractmethod - def get_roles_for_groups(self, group_ids, project_id=None, domain_id=None): - """List all the roles assigned to groups on either domain or + def list_role_ids_for_groups( + self, group_ids, project_id=None, domain_id=None): + """List all the role ids assigned to groups on either domain or project. If the project_id is not None, this value will be used, no matter what @@ -1084,7 +1123,7 @@ class Driver(object): :raises: AttributeError: In case both project_id and domain_id are set to None - :returns: a list of Role entities matching groups and + :returns: a list of Role ids matching groups and project_id or domain_id """ @@ -1155,32 +1194,10 @@ class Driver(object): """ raise exception.NotImplemented() # pragma: no cover - # role crud - @abc.abstractmethod - def create_role(self, role_id, role): - """Creates a new role. - - :raises: keystone.exception.Conflict - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def list_roles(self, hints): - """List roles in the system. - - :param hints: filter hints which the driver should - implement if at all possible. - - :returns: a list of role_refs or an empty list. - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_group_project_roles(self, groups, project_id, project_domain_id): - """Get group roles for a specific project. + def list_role_ids_for_groups_on_project( + self, groups, project_id, project_domain_id): + """List group role ids for a specific project. Supports the ``OS-INHERIT`` role inheritance from the project's domain if supported by the assignment driver. @@ -1191,38 +1208,15 @@ class Driver(object): :type project_id: str :param project_domain_id: project's domain identifier :type project_domain_id: str - :returns: list of role_refs for the project + :returns: list of role ids for the project :rtype: list """ raise exception.NotImplemented() @abc.abstractmethod - def get_role(self, role_id): - """Get a role by ID. + def delete_role_assignments(self, role_id): + """Deletes all assignments for a role.""" - :returns: role_ref - :raises: keystone.exception.RoleNotFound - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def update_role(self, role_id, role): - """Updates an existing role. - - :raises: keystone.exception.RoleNotFound, - keystone.exception.Conflict - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def delete_role(self, role_id): - """Deletes an existing role. - - :raises: keystone.exception.RoleNotFound - - """ raise exception.NotImplemented() # pragma: no cover # TODO(ayoung): determine what else these two functions raise @@ -1277,3 +1271,131 @@ class Driver(object): """ if domain_id != CONF.identity.default_domain_id: raise exception.DomainNotFound(domain_id=domain_id) + + +@dependency.provider('role_api') +@dependency.requires('assignment_api') +class RoleManager(manager.Manager): + """Default pivot point for the Role backend.""" + + def __init__(self): + # If there is a specific driver specified for role, then use it. + # Otherwise retrieve the driver type from the assignment driver. + role_driver = CONF.role.driver + + if role_driver is None: + assignment_driver = dependency.REGISTRY['assignment_api'].driver + role_driver = assignment_driver.default_role_driver() + + super(RoleManager, self).__init__(role_driver) + + @cache.on_arguments(should_cache_fn=SHOULD_CACHE, + expiration_time=EXPIRATION_TIME) + def get_role(self, role_id): + return self.driver.get_role(role_id) + + @notifications.created('role') + def create_role(self, role_id, role): + ret = self.driver.create_role(role_id, role) + if SHOULD_CACHE(ret): + self.get_role.set(ret, self, role_id) + return ret + + @manager.response_truncated + def list_roles(self, hints=None): + return self.driver.list_roles(hints or driver_hints.Hints()) + + @notifications.updated('role') + def update_role(self, role_id, role): + ret = self.driver.update_role(role_id, role) + self.get_role.invalidate(self, role_id) + return ret + + @notifications.deleted('role') + def delete_role(self, role_id): + try: + self.assignment_api.delete_tokens_for_role_assignments(role_id) + except exception.NotImplemented: + # FIXME(morganfainberg): Not all backends (ldap) implement + # `list_role_assignments_for_role` which would have previously + # caused a NotImplmented error to be raised when called through + # the controller. Now error or proper action will always come from + # the `delete_role` method logic. Work needs to be done to make + # the behavior between drivers consistent (capable of revoking + # tokens for the same circumstances). This is related to the bug + # https://bugs.launchpad.net/keystone/+bug/1221805 + pass + self.assignment_api.delete_role_assignments(role_id) + self.driver.delete_role(role_id) + self.get_role.invalidate(self, role_id) + + +@six.add_metaclass(abc.ABCMeta) +class RoleDriver(object): + + def _get_list_limit(self): + return CONF.role.list_limit or CONF.list_limit + + @abc.abstractmethod + def create_role(self, role_id, role): + """Creates a new role. + + :raises: keystone.exception.Conflict + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def list_roles(self, hints): + """List roles in the system. + + :param hints: filter hints which the driver should + implement if at all possible. + + :returns: a list of role_refs or an empty list. + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def list_roles_from_ids(self, role_ids): + """List roles for the provided list of ids. + + :param role_ids: list of ids + + :returns: a list of role_refs. + + This method is used internally by the assignment manager to bulk read + a set of roles given their ids. + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def get_role(self, role_id): + """Get a role by ID. + + :returns: role_ref + :raises: keystone.exception.RoleNotFound + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def update_role(self, role_id, role): + """Updates an existing role. + + :raises: keystone.exception.RoleNotFound, + keystone.exception.Conflict + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def delete_role(self, role_id): + """Deletes an existing role. + + :raises: keystone.exception.RoleNotFound + + """ + raise exception.NotImplemented() # pragma: no cover diff --git a/keystone/assignment/role_backends/__init__.py b/keystone/assignment/role_backends/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/keystone/assignment/role_backends/ldap.py b/keystone/assignment/role_backends/ldap.py new file mode 100644 index 0000000000..674f47579d --- /dev/null +++ b/keystone/assignment/role_backends/ldap.py @@ -0,0 +1,124 @@ +# 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. + +from __future__ import absolute_import + +from keystone import assignment +from keystone.common import ldap as common_ldap +from keystone.common import models +from keystone import config +from keystone import exception +from keystone.i18n import _ +from keystone.identity.backends import ldap as ldap_identity +from keystone.openstack.common import log + + +CONF = config.CONF +LOG = log.getLogger(__name__) + + +class Role(assignment.RoleDriver): + + def __init__(self): + super(Role, self).__init__() + self.LDAP_URL = CONF.ldap.url + self.LDAP_USER = CONF.ldap.user + self.LDAP_PASSWORD = CONF.ldap.password + self.suffix = CONF.ldap.suffix + + # This is the only deep dependency from resource back + # to identity. The assumption is that if you are using + # LDAP for resource, you are using it for identity as well. + self.user = ldap_identity.UserApi(CONF) + self.role = RoleApi(CONF, self.user) + + def get_role(self, role_id): + return self.role.get(role_id) + + def list_roles(self, hints): + return self.role.get_all() + + def list_roles_from_ids(self, ids): + return [self.get_role(id) for id in ids] + + def create_role(self, role_id, role): + self.role.check_allow_create() + try: + self.get_role(role_id) + except exception.NotFound: + pass + else: + msg = _('Duplicate ID, %s.') % role_id + raise exception.Conflict(type='role', details=msg) + + try: + self.role.get_by_name(role['name']) + except exception.NotFound: + pass + else: + msg = _('Duplicate name, %s.') % role['name'] + raise exception.Conflict(type='role', details=msg) + + return self.role.create(role) + + def delete_role(self, role_id): + self.role.check_allow_delete() + return self.role.delete(role_id) + + def update_role(self, role_id, role): + self.role.check_allow_update() + self.get_role(role_id) + return self.role.update(role_id, role) + + +# NOTE(heny-nash): A mixin class to enable the sharing of the LDAP structure +# between here and the assignment LDAP. +class RoleLdapStructureMixin(object): + DEFAULT_OU = 'ou=Roles' + DEFAULT_STRUCTURAL_CLASSES = [] + DEFAULT_OBJECTCLASS = 'organizationalRole' + DEFAULT_MEMBER_ATTRIBUTE = 'roleOccupant' + NotFound = exception.RoleNotFound + options_name = 'role' + attribute_options_names = {'name': 'name'} + immutable_attrs = ['id'] + model = models.Role + + +# TODO(termie): turn this into a data object and move logic to driver +class RoleApi(RoleLdapStructureMixin, common_ldap.BaseLdap): + + def __init__(self, conf, user_api): + super(RoleApi, self).__init__(conf) + self._user_api = user_api + + def get(self, role_id, role_filter=None): + model = super(RoleApi, self).get(role_id, role_filter) + return model + + def create(self, values): + return super(RoleApi, self).create(values) + + def update(self, role_id, role): + new_name = role.get('name') + if new_name is not None: + try: + old_role = self.get_by_name(new_name) + if old_role['id'] != role_id: + raise exception.Conflict( + _('Cannot duplicate name %s') % old_role) + except exception.NotFound: + pass + return super(RoleApi, self).update(role_id, role) + + def delete(self, role_id): + super(RoleApi, self).delete(role_id) diff --git a/keystone/assignment/role_backends/sql.py b/keystone/assignment/role_backends/sql.py new file mode 100644 index 0000000000..f19d182776 --- /dev/null +++ b/keystone/assignment/role_backends/sql.py @@ -0,0 +1,80 @@ +# 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. + +from keystone import assignment +from keystone.common import sql +from keystone import exception + + +class Role(assignment.RoleDriver): + + @sql.handle_conflicts(conflict_type='role') + def create_role(self, role_id, role): + with sql.transaction() as session: + ref = RoleTable.from_dict(role) + session.add(ref) + return ref.to_dict() + + @sql.truncated + def list_roles(self, hints): + with sql.transaction() as session: + query = session.query(RoleTable) + refs = sql.filter_limit_query(RoleTable, query, hints) + return [ref.to_dict() for ref in refs] + + def list_roles_from_ids(self, ids): + if not ids: + return [] + else: + with sql.transaction() as session: + query = session.query(RoleTable) + query = query.filter(RoleTable.id.in_(ids)) + role_refs = query.all() + return [role_ref.to_dict() for role_ref in role_refs] + + def _get_role(self, session, role_id): + ref = session.query(RoleTable).get(role_id) + if ref is None: + raise exception.RoleNotFound(role_id=role_id) + return ref + + def get_role(self, role_id): + with sql.transaction() as session: + return self._get_role(session, role_id).to_dict() + + @sql.handle_conflicts(conflict_type='role') + def update_role(self, role_id, role): + with sql.transaction() as session: + ref = self._get_role(session, role_id) + old_dict = ref.to_dict() + for k in role: + old_dict[k] = role[k] + new_role = RoleTable.from_dict(old_dict) + for attr in RoleTable.attributes: + if attr != 'id': + setattr(ref, attr, getattr(new_role, attr)) + ref.extra = new_role.extra + return ref.to_dict() + + def delete_role(self, role_id): + with sql.transaction() as session: + ref = self._get_role(session, role_id) + session.delete(ref) + + +class RoleTable(sql.ModelBase, sql.DictBase): + __tablename__ = 'role' + attributes = ['id', 'name'] + id = sql.Column(sql.String(64), primary_key=True) + name = sql.Column(sql.String(255), unique=True, nullable=False) + extra = sql.Column(sql.JsonBlob()) + __table_args__ = (sql.UniqueConstraint('name'), {}) diff --git a/keystone/backends.py b/keystone/backends.py index 265daec295..eca8756cf3 100644 --- a/keystone/backends.py +++ b/keystone/backends.py @@ -43,6 +43,7 @@ def load_backends(): id_mapping_api=identity.MappingManager(), identity_api=_IDENTITY_API, policy_api=policy.Manager(), + role_api=assignment.RoleManager(), token_api=token.persistence.Manager(), trust_api=trust.Manager(), token_provider_api=token.provider.Manager()) diff --git a/keystone/common/config.py b/keystone/common/config.py index 56f9535a0c..4d5714ce14 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -444,6 +444,22 @@ FILE_OPTIONS = { help='Maximum number of entities that will be returned ' 'in an assignment collection.'), ], + 'role': [ + # The role driver has no default for backward compatibility reasons. + # If role driver is not specified, the assignment driver chooses + # the backend + cfg.StrOpt('driver', + help='Role backend driver.'), + cfg.BoolOpt('caching', default=True, + help='Toggle for role caching. This has no effect ' + 'unless global caching is enabled.'), + cfg.IntOpt('cache_time', + help='TTL (in seconds) to cache role data. This has ' + 'no effect unless global caching is enabled.'), + cfg.IntOpt('list_limit', + help='Maximum number of entities that will be returned ' + 'in a role collection.'), + ], 'credential': [ cfg.StrOpt('driver', default=('keystone.credential.backends' diff --git a/keystone/common/sql/migrate_repo/versions/062_drop_assignment_role_fk.py b/keystone/common/sql/migrate_repo/versions/062_drop_assignment_role_fk.py new file mode 100644 index 0000000000..5a33486c03 --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/062_drop_assignment_role_fk.py @@ -0,0 +1,41 @@ +# 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 keystone.common.sql import migration_helpers + + +def list_constraints(migrate_engine): + meta = sqlalchemy.MetaData() + meta.bind = migrate_engine + assignment_table = sqlalchemy.Table('assignment', meta, autoload=True) + role_table = sqlalchemy.Table('role', meta, autoload=True) + + constraints = [{'table': assignment_table, + 'fk_column': 'role_id', + 'ref_column': role_table.c.id}] + return constraints + + +def upgrade(migrate_engine): + # SQLite does not support constraints, and querying the constraints + # raises an exception + if migrate_engine.name == 'sqlite': + return + migration_helpers.remove_constraints(list_constraints(migrate_engine)) + + +def downgrade(migrate_engine): + if migrate_engine.name == 'sqlite': + return + migration_helpers.add_constraints(list_constraints(migrate_engine)) diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py index 08d9e59f53..5c7347d910 100644 --- a/keystone/tests/test_backend.py +++ b/keystone/tests/test_backend.py @@ -3226,13 +3226,12 @@ class IdentityTests(object): role_ref = self.assignment_api.get_role(role_id) updated_role_ref = copy.deepcopy(role_ref) updated_role_ref['name'] = uuid.uuid4().hex - # Update role, bypassing the assignment api manager - self.assignment_api.driver.update_role(role_id, updated_role_ref) + # Update role, bypassing the role api manager + self.role_api.driver.update_role(role_id, updated_role_ref) # Verify get_role still returns old ref self.assertDictEqual(role_ref, self.assignment_api.get_role(role_id)) # Invalidate Cache - self.assignment_api.get_role.invalidate(self.assignment_api, - role_id) + self.role_api.get_role.invalidate(self.role_api, role_id) # Verify get_role returns the new role_ref self.assertDictEqual(updated_role_ref, self.assignment_api.get_role(role_id)) @@ -3240,12 +3239,12 @@ class IdentityTests(object): self.assignment_api.update_role(role_id, role_ref) # Verify get_role returns the original role ref self.assertDictEqual(role_ref, self.assignment_api.get_role(role_id)) - # Delete role bypassing the assignment api manager - self.assignment_api.driver.delete_role(role_id) + # Delete role bypassing the role api manager + self.role_api.driver.delete_role(role_id) # Verify get_role still returns the role_ref self.assertDictEqual(role_ref, self.assignment_api.get_role(role_id)) # Invalidate cache - self.assignment_api.get_role.invalidate(self.assignment_api, role_id) + self.role_api.get_role.invalidate(self.role_api, role_id) # Verify RoleNotFound is now raised self.assertRaises(exception.RoleNotFound, self.assignment_api.get_role, diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py index 1c428a469d..865c58df52 100644 --- a/keystone/tests/test_backend_ldap.py +++ b/keystone/tests/test_backend_ldap.py @@ -996,8 +996,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): # could affect what the drivers would return up to the manager. This # solves this assumption when working with aggressive (on-create) # cache population. - self.assignment_api.get_role.invalidate(self.assignment_api, - self.role_member['id']) + self.role_api.get_role.invalidate(self.assignment_api, + self.role_member['id']) self.assignment_api.get_role(self.role_member['id']) self.assignment_api.get_project.invalidate(self.assignment_api, self.tenant_bar['id']) @@ -1018,8 +1018,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): # could affect what the drivers would return up to the manager. This # solves this assumption when working with aggressive (on-create) # cache population. - self.assignment_api.get_role.invalidate(self.assignment_api, - self.role_member['id']) + self.role_api.get_role.invalidate(self.assignment_api, + self.role_member['id']) self.assertRaises(exception.RoleNotFound, self.assignment_api.get_role, self.role_member['id']) @@ -1112,8 +1112,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): # could affect what the drivers would return up to the manager. This # solves this assumption when working with aggressive (on-create) # cache population. - self.assignment_api.get_role.invalidate(self.assignment_api, - self.role_member['id']) + self.role_api.get_role.invalidate(self.role_api, + self.role_member['id']) role_ref = self.assignment_api.get_role(self.role_member['id']) self.assertEqual(self.role_member['id'], role_ref['id']) self.assertEqual(self.role_member['name'], role_ref['name']) @@ -1126,8 +1126,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): # could affect what the drivers would return up to the manager. This # solves this assumption when working with aggressive (on-create) # cache population. - self.assignment_api.get_role.invalidate(self.assignment_api, - self.role_member['id']) + self.role_api.get_role.invalidate(self.role_api, + self.role_member['id']) role_ref = self.assignment_api.get_role(self.role_member['id']) self.assertEqual(self.role_member['id'], role_ref['id']) self.assertNotIn('name', role_ref) @@ -1144,8 +1144,8 @@ class LDAPIdentity(BaseLDAPIdentity, tests.TestCase): # could affect what the drivers would return up to the manager. This # solves this assumption when working with aggressive (on-create) # cache population. - self.assignment_api.get_role.invalidate(self.assignment_api, - self.role_member['id']) + self.role_api.get_role.invalidate(self.role_api, + self.role_member['id']) role_ref = self.assignment_api.get_role(self.role_member['id']) self.assertEqual(self.role_member['id'], role_ref['id']) self.assertNotIn('name', role_ref) diff --git a/keystone/tests/test_backend_sql.py b/keystone/tests/test_backend_sql.py index 0864709b1a..eec41954b0 100644 --- a/keystone/tests/test_backend_sql.py +++ b/keystone/tests/test_backend_sql.py @@ -28,6 +28,7 @@ from keystone.common import sql from keystone import config from keystone import exception from keystone.identity.backends import sql as identity_sql +from keystone.openstack.common import log as logging from keystone import tests from keystone.tests import default_fixtures from keystone.tests.ksfixtures import database @@ -776,3 +777,36 @@ class SqlCredential(SqlTests): credentials = self.credential_api.list_credentials_for_user( self.user_foo['id']) self._validateCredentialList(credentials, self.user_credentials) + + +class DeprecatedDecorators(SqlTests): + + def test_assignment_to_role_api(self): + """Test that calling one of the methods does call LOG.deprecated. + + This method is really generic to the type of backend, but we need + one to execute the test, so the SQL backend is as good as any. + + """ + + # Rather than try and check that a log message is issued, we + # enabled fatal_deprecations so that we can check for the + # raising of the exception. + + # First try to create a role without enabling fatal deprecations, + # which should work due to the cross manager deprecated calls. + role_ref = { + 'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex} + self.assignment_api.create_role(role_ref['id'], role_ref) + self.role_api.get_role(role_ref['id']) + + # Now enable fatal exceptions - creating a role by calling the + # old manager should now fail. + self.config_fixture.config(fatal_deprecations=True) + role_ref = { + 'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex} + self.assertRaises(logging.DeprecatedConfig, + self.assignment_api.create_role, + role_ref['id'], role_ref) diff --git a/keystone/tests/test_sql_upgrade.py b/keystone/tests/test_sql_upgrade.py index 0422355fa1..907b9f6733 100644 --- a/keystone/tests/test_sql_upgrade.py +++ b/keystone/tests/test_sql_upgrade.py @@ -1506,6 +1506,23 @@ class SqlUpgradeTests(SqlMigrateBase): project = session.query(proj_table)[0] self.assertRaises(AttributeError, getattr, project, 'parent_id') + def test_drop_assignment_role_fk(self): + self.upgrade(61) + self.assertTrue(self.does_fk_exist('assignment', 'role_id')) + self.upgrade(62) + if self.engine.name != 'sqlite': + # sqlite does not support FK deletions (or enforcement) + self.assertFalse(self.does_fk_exist('assignment', 'role_id')) + self.downgrade(61) + self.assertTrue(self.does_fk_exist('assignment', 'role_id')) + + def does_fk_exist(self, table, fk_column): + inspector = reflection.Inspector.from_engine(self.engine) + for fk in inspector.get_foreign_keys(table): + if fk_column in fk['constrained_columns']: + return True + return False + def populate_user_table(self, with_pass_enab=False, with_pass_enab_domain=False): # Populate the appropriate fields in the user