Merge "Fix issue with v3 tokens and group membership roles"

4 files changed, 71 insertions, 66 deletions

diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py
index 77a86789..89c05aaf 100644
--- a/keystone/identity/backends/kvs.py
+++ b/keystone/identity/backends/kvs.py
@@ -131,15 +131,6 @@ class Identity(kvs.Base, identity.Driver):
         user_ref = self._get_user(user_id)
         return user_ref.get('tenants', [])
 
-    def get_roles_for_user_and_project(self, user_id, tenant_id):
-        self.get_user(user_id)
-        self.get_project(tenant_id)
-        try:
-            metadata_ref = self.get_metadata(user_id, tenant_id)
-        except exception.MetadataNotFound:
-            metadata_ref = {}
-        return metadata_ref.get('roles', [])
-
     def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
         self.get_user(user_id)
         self.get_project(tenant_id)
diff --git a/keystone/identity/backends/ldap.py b/keystone/identity/backends/ldap.py
index dffbf835..2387c84e 100644
--- a/keystone/identity/backends/ldap.py
+++ b/keystone/identity/backends/ldap.py
@@ -149,12 +149,23 @@ class Identity(identity.Driver):
 
     def get_metadata(self, user_id=None, tenant_id=None,
                      domain_id=None, group_id=None):
+
+        def _get_roles_for_just_user_and_project(user_id, tenant_id):
+            self.get_user(user_id)
+            self.get_project(tenant_id)
+            user_dn = self.user._id_to_dn(user_id)
+            return [self.role._dn_to_id(a.role_dn)
+                    for a in self.role.get_role_assignments
+                    (self.project._id_to_dn(tenant_id))
+                    if a.user_dn == user_dn]
+
         if domain_id is not None:
-            raise NotImplemented('Domain metadata not supported by LDAP.')
+            msg = 'Domain metadata not supported by LDAP'
+            raise exception.NotImplemented(message=msg)
         if not self.get_project(tenant_id) or not self.get_user(user_id):
             return {}
 
-        metadata_ref = self.get_roles_for_user_and_project(user_id, tenant_id)
+        metadata_ref = _get_roles_for_just_user_and_project(user_id, tenant_id)
         if not metadata_ref:
             return {}
         return {'roles': metadata_ref}
@@ -182,15 +193,6 @@ class Identity(identity.Driver):
                  self.project.get_user_dns(tenant_id, rolegrants)]
         return self._set_default_domain(users)
 
-    def get_roles_for_user_and_project(self, user_id, tenant_id):
-        self.get_user(user_id)
-        self.get_project(tenant_id)
-        user_dn = self.user._id_to_dn(user_id)
-        return [self.role._dn_to_id(a.role_dn)
-                for a in self.role.get_role_assignments
-                (self.project._id_to_dn(tenant_id))
-                if a.user_dn == user_dn]
-
     def _subrole_id_to_dn(self, role_id, tenant_id):
         if tenant_id is None:
             return self.role._id_to_dn(role_id)
diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py
index 2f9d89f3..c02ffd0b 100644
--- a/keystone/identity/backends/sql.py
+++ b/keystone/identity/backends/sql.py
@@ -405,32 +405,6 @@ class Identity(sql.Base, identity.Driver):
         membership_refs = query.all()
         return [x.project_id for x in membership_refs]
 
-    def _get_user_group_project_roles(self, metadata_ref, user_id, project_id):
-        group_refs = self.list_groups_for_user(user_id=user_id)
-        for x in group_refs:
-            try:
-                metadata_ref.update(
-                    self.get_metadata(group_id=x['id'],
-                                      tenant_id=project_id))
-            except exception.MetadataNotFound:
-                # no group grant, skip
-                pass
-
-    def _get_user_project_roles(self, metadata_ref, user_id, project_id):
-        try:
-            metadata_ref.update(self.get_metadata(user_id, project_id))
-        except exception.MetadataNotFound:
-            pass
-
-    def get_roles_for_user_and_project(self, user_id, tenant_id):
-        session = self.get_session()
-        self._get_user(session, user_id)
-        self._get_project(session, tenant_id)
-        metadata_ref = {}
-        self._get_user_project_roles(metadata_ref, user_id, tenant_id)
-        self._get_user_group_project_roles(metadata_ref, user_id, tenant_id)
-        return list(set(metadata_ref.get('roles', [])))
-
     def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
         session = self.get_session()
         self._get_user(session, user_id)
diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index 77870fda..1e2ed733 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -186,50 +186,88 @@ class Driver(object):
     def get_roles_for_user_and_project(self, user_id, tenant_id):
         """Get the roles associated with a user within given tenant.
 
+        This includes roles directly assigned to the user on the
+        project, as well as those by virtue of group membership.
+
         :returns: a list of role ids.
         :raises: keystone.exception.UserNotFound,
                  keystone.exception.ProjectNotFound
 
         """
-        raise exception.NotImplemented()
+        def _get_group_project_roles(user_id, tenant_id):
+            role_list = []
+            group_refs = self.list_groups_for_user(user_id=user_id)
+            for x in group_refs:
+                try:
+                    metadata_ref = self.get_metadata(group_id=x['id'],
+                                                     tenant_id=tenant_id)
+                    role_list += metadata_ref.get('roles', [])
+                except exception.MetadataNotFound:
+                    # no group grant, skip
+                    pass
+            return role_list
+
+        def _get_user_project_roles(user_id, tenant_id):
+            metadata_ref = {}
+            try:
+                metadata_ref = self.get_metadata(user_id=user_id,
+                                                 tenant_id=tenant_id)
+            except exception.MetadataNotFound:
+                pass
+            return metadata_ref.get('roles', [])
+
+        self.get_user(user_id)
+        self.get_project(tenant_id)
+        user_role_list = _get_user_project_roles(user_id, tenant_id)
+        group_role_list = _get_group_project_roles(user_id, tenant_id)
+        # Use set() to process the list to remove any duplicates
+        return list(set(user_role_list + group_role_list))
 
     def get_roles_for_user_and_domain(self, user_id, domain_id):
         """Get the roles associated with a user within given domain.
 
+        This includes roles directly assigned to the user on the
+        domain, as well as those by virtue of group membership.
+
         :returns: a list of role ids.
         :raises: keystone.exception.UserNotFound,
-                 keystone.exception.ProjectNotFound
+                 keystone.exception.DomainNotFound
 
         """
 
-        def update_metadata_for_group_domain_roles(self, metadata_ref,
-                                                   user_id, domain_id):
+        def _get_group_domain_roles(user_id, domain_id):
+            role_list = []
             group_refs = self.list_groups_for_user(user_id=user_id)
             for x in group_refs:
                 try:
-                    metadata_ref.update(
-                        self.get_metadata(group_id=x['id'],
-                                          domain_id=domain_id))
-                except exception.MetadataNotFound:
-                    # no group grant, skip
+                    metadata_ref = self.get_metadata(group_id=x['id'],
+                                                     domain_id=domain_id)
+                    role_list += metadata_ref.get('roles', [])
+                except (exception.MetadataNotFound, exception.NotImplemented):
+                    # MetadataNotFound implies no group grant, so skip.
+                    # Ignore NotImplemented since not all backends support
+                    # domains.
                     pass
+            return role_list
 
-        def update_metadata_for_user_domain_roles(self, metadata_ref,
-                                                  user_id, domain_id):
+        def _get_user_domain_roles(user_id, domain_id):
+            metadata_ref = {}
             try:
-                metadata_ref.update(self.get_metadata(user_id=user_id,
-                                                      domain_id=domain_id))
-            except exception.MetadataNotFound:
+                metadata_ref = self.get_metadata(user_id=user_id,
+                                                 domain_id=domain_id)
+            except (exception.MetadataNotFound, exception.NotImplemented):
+                # MetadataNotFound implies no user grants.
+                # Ignore NotImplemented since not all backends support
+                # domains.
                 pass
+            return metadata_ref.get('roles', [])
 
         self.get_user(user_id)
         self.get_domain(domain_id)
-        metadata_ref = {}
-        update_metadata_for_user_domain_roles(self, metadata_ref,
-                                              user_id, domain_id)
-        update_metadata_for_group_domain_roles(self, metadata_ref,
-                                               user_id, domain_id)
-        return list(set(metadata_ref.get('roles', [])))
+        user_role_list = _get_user_domain_roles(user_id, domain_id)
+        group_role_list = _get_group_domain_roles(user_id, domain_id)
+        # Use set() to process the list to remove any duplicates
+        return list(set(user_role_list + group_role_list))
 
     def add_role_to_user_and_project(self, user_id, tenant_id, role_id):
         """Add a role to a user within given tenant.