Fix listing of project members

Project member resolver may return both ProjectMember and GroupMember
members - the old :read_project permission was not accurate and didn't
work fo GroupMember

app/graphql/resolvers/project_members_resolver.rb

@@ -2,19 +2,23 @@
 
 module Resolvers
   class ProjectMembersResolver < BaseResolver
+    include Gitlab::Graphql::Authorize::AuthorizeResource
+
     argument :search, GraphQL::STRING_TYPE,
               required: false,
               description: 'Search query'
 
-    type Types::ProjectMemberType, null: true
+    type Types::MemberInterface, null: true
+
+    authorize :read_project_member
 
     alias_method :project, :object
 
     def resolve(**args)
-      return Member.none unless project.present?
+      authorize!(project)
 
       MembersFinder
-        .new(project, context[:current_user], params: args)
+        .new(project, current_user, params: args)
         .execute
     end
   end

app/graphql/types/group_member_type.rb

@@ -8,7 +8,7 @@ module Types
     implements MemberInterface
 
     graphql_name 'GroupMember'
-    description 'Represents a Group Member'
+    description 'Represents a Group Membership'
 
     field :group, Types::GroupType, null: true,
           description: 'Group that a User is a member of',

app/graphql/types/member_interface.rb

@@ -4,6 +4,9 @@ module Types
   module MemberInterface
     include BaseInterface
 
+    field :id, GraphQL::ID_TYPE, null: false,
+          description: 'ID of the member'
+
     field :access_level, Types::AccessLevelType, null: true,
           description: 'GitLab::Access level'
 
@@ -18,5 +21,22 @@ module Types
 
     field :expires_at, Types::TimeType, null: true,
           description: 'Date and time the membership expires'
+
+    field :user, Types::UserType, null: false,
+          description: 'User that is associated with the member object',
+          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.user_id).find }
+
+    definition_methods do
+      def resolve_type(object, context)
+        case object
+        when GroupMember
+          Types::GroupMemberType
+        when ProjectMember
+          Types::ProjectMemberType
+        else
+          raise ::Gitlab::Graphql::Errors::BaseError, "Unknown member type #{object.class.name}"
+        end
+      end
+    end
   end
 end

app/graphql/types/project_member_type.rb

@@ -3,7 +3,7 @@
 module Types
   class ProjectMemberType < BaseObject
     graphql_name 'ProjectMember'
-    description 'Represents a Project Member'
+    description 'Represents a Project Membership'
 
     expose_permissions Types::PermissionTypes::Project
 
@@ -11,13 +11,6 @@ module Types
 
     authorize :read_project
 
-    field :id, GraphQL::ID_TYPE, null: false,
-          description: 'ID of the member'
-
-    field :user, Types::UserType, null: false,
-          description: 'User that is associated with the member object',
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.user_id).find }
-
     field :project, Types::ProjectType, null: true,
           description: 'Project that User is a member of',
           resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, obj.source_id).find }

app/graphql/types/project_type.rb

@@ -159,7 +159,7 @@ module Types
           resolver: Resolvers::ProjectMilestonesResolver
 
     field :project_members,
-          Types::ProjectMemberType.connection_type,
+          Types::MemberInterface.connection_type,
           description: 'Members of the project',
           resolver: Resolvers::ProjectMembersResolver
 

changelogs/unreleased/project_member_list.yml

+---
+title: Include also inherited project members in GraphQL API
+merge_request: 39444
+author:
+type: fixed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -6658,7 +6658,7 @@ type Group {
 }
 
 """
-Represents a Group Member
+Represents a Group Membership
 """
 type GroupMember implements MemberInterface {
   """
@@ -6686,11 +6686,21 @@ type GroupMember implements MemberInterface {
   """
   group: Group
 
+  """
+  ID of the member
+  """
+  id: ID!
+
   """
   Date and time the membership was last updated
   """
   updatedAt: Time
 
+  """
+  User that is associated with the member object
+  """
+  user: User!
+
   """
   Permissions for the current user on the resource
   """
@@ -8340,10 +8350,55 @@ interface MemberInterface {
   """
   expiresAt: Time
 
+  """
+  ID of the member
+  """
+  id: ID!
+
   """
   Date and time the membership was last updated
   """
   updatedAt: Time
+
+  """
+  User that is associated with the member object
+  """
+  user: User!
+}
+
+"""
+The connection type for MemberInterface.
+"""
+type MemberInterfaceConnection {
+  """
+  A list of edges.
+  """
+  edges: [MemberInterfaceEdge]
+
+  """
+  A list of nodes.
+  """
+  nodes: [MemberInterface]
+
+  """
+  Information to aid in pagination.
+  """
+  pageInfo: PageInfo!
+}
+
+"""
+An edge in a connection.
+"""
+type MemberInterfaceEdge {
+  """
+  A cursor for use in pagination.
+  """
+  cursor: String!
+
+  """
+  The item at the end of the edge.
+  """
+  node: MemberInterface
 }
 
 type MergeRequest implements Noteable {
@@ -11567,7 +11622,7 @@ type Project {
     Search query
     """
     search: String
-  ): ProjectMemberConnection
+  ): MemberInterfaceConnection
 
   """
   Indicates if there is public access to pipelines and job details of the project, including output logs and artifacts
@@ -12001,7 +12056,7 @@ type ProjectEdge {
 }
 
 """
-Represents a Project Member
+Represents a Project Membership
 """
 type ProjectMember implements MemberInterface {
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -18300,7 +18300,7 @@
         {
           "kind": "OBJECT",
           "name": "GroupMember",
-          "description": "Represents a Group Member",
+          "description": "Represents a Group Membership",
           "fields": [
             {
               "name": "accessLevel",
@@ -18372,6 +18372,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "id",
+              "description": "ID of the member",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "ID",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "updatedAt",
               "description": "Date and time the membership was last updated",
@@ -18386,6 +18404,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "user",
+              "description": "User that is associated with the member object",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "User",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "userPermissions",
               "description": "Permissions for the current user on the resource",
@@ -23177,6 +23213,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "id",
+              "description": "ID of the member",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "ID",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "updatedAt",
               "description": "Date and time the membership was last updated",
@@ -23190,6 +23244,24 @@
               },
               "isDeprecated": false,
               "deprecationReason": null
+            },
+            {
+              "name": "user",
+              "description": "User that is associated with the member object",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "User",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
             }
           ],
           "inputFields": null,
@@ -23208,6 +23280,118 @@
             }
           ]
         },
+        {
+          "kind": "OBJECT",
+          "name": "MemberInterfaceConnection",
+          "description": "The connection type for MemberInterface.",
+          "fields": [
+            {
+              "name": "edges",
+              "description": "A list of edges.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "MemberInterfaceEdge",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "nodes",
+              "description": "A list of nodes.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "INTERFACE",
+                  "name": "MemberInterface",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "pageInfo",
+              "description": "Information to aid in pagination.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "OBJECT",
+                  "name": "PageInfo",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
+        {
+          "kind": "OBJECT",
+          "name": "MemberInterfaceEdge",
+          "description": "An edge in a connection.",
+          "fields": [
+            {
+              "name": "cursor",
+              "description": "A cursor for use in pagination.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "String",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "node",
+              "description": "The item at the end of the edge.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "INTERFACE",
+                "name": "MemberInterface",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "MergeRequest",
@@ -33999,7 +34183,7 @@
               ],
               "type": {
                 "kind": "OBJECT",
-                "name": "ProjectMemberConnection",
+                "name": "MemberInterfaceConnection",
                 "ofType": null
               },
               "isDeprecated": false,
@@ -35133,7 +35317,7 @@
         {
           "kind": "OBJECT",
           "name": "ProjectMember",
-          "description": "Represents a Project Member",
+          "description": "Represents a Project Membership",
           "fields": [
             {
               "name": "accessLevel",

doc/api/graphql/reference/index.md

@@ -999,7 +999,7 @@ Autogenerated return type of EpicTreeReorder
 
 ## GroupMember
 
-Represents a Group Member
+Represents a Group Membership
 
 | Name  | Type  | Description |
 | ---   |  ---- | ----------  |
@@ -1008,7 +1008,9 @@ Represents a Group Member
 | `createdBy` | User | User that authorized membership |
 | `expiresAt` | Time | Date and time the membership expires |
 | `group` | Group | Group that a User is a member of |
+| `id` | ID! | ID of the member |
 | `updatedAt` | Time | Date and time the membership was last updated |
+| `user` | User! | User that is associated with the member object |
 | `userPermissions` | GroupPermissions! | Permissions for the current user on the resource |
 
 ## GroupPermissions
@@ -1690,7 +1692,7 @@ Information about pagination in a connection.
 
 ## ProjectMember
 
-Represents a Project Member
+Represents a Project Membership
 
 | Name  | Type  | Description |
 | ---   |  ---- | ----------  |

spec/graphql/resolvers/project_members_resolver_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Resolvers::ProjectMembersResolver do
     let_it_be(:root_group)   { create(:group) }
     let_it_be(:group_1)      { create(:group, parent: root_group) }
     let_it_be(:group_2)      { create(:group, parent: root_group) }
-    let_it_be(:project)      { create(:project, :public, group: group_1) }
+    let_it_be(:project)      { create(:project, group: group_1) }
 
     let_it_be(:user_1) { create(:user, name: 'test user') }
     let_it_be(:user_2) { create(:user, name: 'test user 2') }
@@ -24,7 +24,7 @@ RSpec.describe Resolvers::ProjectMembersResolver do
     let(:args) { {} }
 
     subject do
-      resolve(described_class, obj: project, args: args, ctx: { context: user_4 })
+      resolve(described_class, obj: project, args: args, ctx: { current_user: user_4 })
     end
 
     describe '#resolve' do
@@ -50,11 +50,15 @@ RSpec.describe Resolvers::ProjectMembersResolver do
         end
       end
 
-      context 'when project is nil' do
-        let(:project) { nil }
+      context 'when user can not see project members' do
+        let_it_be(:other_user) { create(:user) }
 
-        it 'returns nil' do
-          expect(subject).to be_empty
+        subject do
+          resolve(described_class, obj: project, args: args, ctx: { current_user: other_user })
+        end
+
+        it 'raises an error' do
+          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
         end
       end
     end

spec/graphql/types/member_interface_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Types::MemberInterface do
+  it 'exposes the expected fields' do
+    expected_fields = %i[
+      id
+      access_level
+      created_by
+      created_at
+      updated_at
+      expires_at
+      user
+    ]
+
+    expect(described_class).to have_graphql_fields(*expected_fields)
+  end
+
+  describe '.resolve_type' do
+    subject { described_class.resolve_type(object, {}) }
+
+    context 'for project member' do
+      let(:object) { build(:project_member) }
+
+      it { is_expected.to be Types::ProjectMemberType }
+    end
+
+    context 'for group member' do
+      let(:object) { build(:group_member) }
+
+      it { is_expected.to be Types::GroupMemberType }
+    end
+
+    context 'for an unkown type' do
+      let(:object) { build(:user) }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(Gitlab::Graphql::Errors::BaseError)
+      end
+    end
+  end
+end

spec/graphql/types/project_type_spec.rb

@@ -108,7 +108,7 @@ RSpec.describe GitlabSchema.types['Project'] do
   describe 'members field' do
     subject { described_class.fields['projectMembers'] }
 
-    it { is_expected.to have_graphql_type(Types::ProjectMemberType.connection_type) }
+    it { is_expected.to have_graphql_type(Types::MemberInterface.connection_type) }
     it { is_expected.to have_graphql_resolver(Resolvers::ProjectMembersResolver) }
   end
 

spec/requests/api/graphql/project_query_spec.rb

@@ -5,7 +5,8 @@ require 'spec_helper'
 RSpec.describe 'getting project information' do
   include GraphqlHelpers
 
-  let(:project) { create(:project, :repository) }
+  let(:group) { create(:group) }
+  let(:project) { create(:project, :repository, group: group) }
   let(:current_user) { create(:user) }
 
   let(:query) do
@@ -60,6 +61,51 @@ RSpec.describe 'getting project information' do
         expect(graphql_data['project']['pipelines']['edges'].size).to eq(1)
       end
     end
+
+    it 'includes inherited members in project_members' do
+      group_member = create(:group_member, group: group)
+      project_member = create(:project_member, project: project)
+      member_query = <<~GQL
+        query {
+          project(fullPath: "#{project.full_path}") {
+            projectMembers {
+              nodes {
+                id
+                user {
+                  username
+                }
+                ... on ProjectMember {
+                  project {
+                    id
+                  }
+                }
+                ... on GroupMember {
+                  group {
+                    id
+                  }
+                }
+              }
+            }
+          }
+        }
+      GQL
+
+      post_graphql(member_query, current_user: current_user)
+
+      member_ids = graphql_data.dig('project', 'projectMembers', 'nodes')
+      expect(member_ids).to include(
+        a_hash_including(
+          'id' => group_member.to_global_id.to_s,
+          'group' => { 'id' => group.to_global_id.to_s }
+        )
+      )
+      expect(member_ids).to include(
+        a_hash_including(
+          'id' => project_member.to_global_id.to_s,
+          'project' => { 'id' => project.to_global_id.to_s }
+        )
+      )
+    end
   end
 
   describe 'performance' do