Fix issue with v3 tokens and group membership roles

The driver calls used by v3 token controllers to obtain roles
for a user on both project and domain were incorrectly implemented,
leading to roles being missed out of the token. v2 tokens are not
affected, since they don't use the same driver calls.

This fixes these functions and adds additonal tests to cover the
cases (all of which would fail without this patch).  As part of this
fix, the implementation of "get_roles_for_user_and_project() is
pulled up into the driver class (like the domain equivalent is already),
since, for all implementations, it is independant of backend technology.

Fixes bug 1197874

Change-Id: I59b6882d93bdc8372be03fed0b390b002a6d0320

4 files changed, 71 insertions, 66 deletions

diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py
index 2eea08cf..565a5587 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 f81feb1d..ba318c73 100644
--- a/keystone/identity/backends/sql.py
+++ b/keystone/identity/backends/sql.py
@@ -368,32 +368,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 a254470e..09324ea3 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.