Merge branch '299179-edit-permission-not-possibe-in-subgroup-of-group-with-ldap-sync' into 'master'

Remove LDAP "Edit Permissions" button for inherited group members [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!52847

ee/app/assets/javascripts/members/utils.js

@@ -2,6 +2,7 @@ import { __ } from '~/locale';
 import {
   generateBadges as CEGenerateBadges,
   parseDataAttributes as CEParseDataAttributes,
+  isDirectMember,
 } from '~/members/utils';
 
 export {
@@ -37,7 +38,7 @@ export const generateBadges = (member, isCurrentUser) => [
   },
 ];
 
-export const canOverride = (member) => member.canOverride;
+export const canOverride = (member) => member.canOverride && isDirectMember(member);
 
 export const parseDataAttributes = (el) => {
   const { ldapOverridePath } = el.dataset;

ee/changelogs/unreleased/299179-edit-permission-not-possibe-in-subgroup-of-group-with-ldap-sync.yml

+---
+title: Remove LDAP "Edit Permissions" button for inherited group members
+merge_request: 52847
+author:
+type: fixed

ee/spec/features/groups/members/override_ldap_memberships_spec.rb

@@ -10,7 +10,8 @@ RSpec.describe 'Groups > Members > Maintainer/Owner can override LDAP access lev
   let(:maryjane) { create(:user, name: 'Mary Jane') }
   let(:owner)    { create(:user) }
   let(:group)    { create(:group_with_ldap_group_link, :public) }
-  let(:project)  { create(:project, namespace: group)  }
+  let(:subgroup) { create(:group, :public, parent: group) }
+  let(:project) { create(:project, namespace: group) }
 
   let!(:owner_member)   { create(:group_member, :owner, group: group, user: owner) }
   let!(:ldap_member)    { create(:group_member, :guest, group: group, user: johndoe, ldap: true) }
@@ -23,12 +24,18 @@ RSpec.describe 'Groups > Members > Maintainer/Owner can override LDAP access lev
     sign_in(owner)
   end
 
-  it 'override not available on project members page', :js do
+  it 'does not allow override on project members page', :js do
     visit namespace_project_project_members_path(group, project)
 
     expect(page).not_to have_button 'Edit permissions'
   end
 
+  it 'does not allow override of inherited group members', :js do
+    visit group_group_members_path(subgroup)
+
+    expect(page).not_to have_button 'Edit permissions'
+  end
+
   it 'owner cannot override LDAP access level', :js do
     stub_application_setting(allow_group_owners_to_manage_ldap: false)
 

ee/spec/frontend/members/components/table/member_table_cell_spec.js

 import { mount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
-import { member as memberMock } from 'jest/members/mock_data';
+import { member as memberMock, directMember } from 'jest/members/mock_data';
 import MembersTableCell from 'ee/members/components/table/members_table_cell.vue';
 
 describe('MemberTableCell', () => {
@@ -80,7 +80,7 @@ describe('MemberTableCell', () => {
     describe('canOverride', () => {
       it('returns `true` when `canOverride` is `true`', () => {
         createComponent({
-          member: { ...memberMock, canOverride: true },
+          member: { ...directMember, canOverride: true },
         });
 
         expect(findWrappedComponent().props('permissions').canOverride).toBe(true);
@@ -88,7 +88,7 @@ describe('MemberTableCell', () => {
 
       it('returns `false` when `canOverride` is `false`', () => {
         createComponent({
-          member: { ...memberMock, canOverride: false },
+          member: { ...directMember, canOverride: false },
         });
 
         expect(findWrappedComponent().props('permissions').canOverride).toBe(false);

ee/spec/frontend/members/components/table/members_table_spec.js

 import { within } from '@testing-library/dom';
 import { mount, createLocalVue, createWrapper } from '@vue/test-utils';
 import Vuex from 'vuex';
-import { member as memberMock, members } from 'jest/members/mock_data';
+import { member as memberMock, directMember, members } from 'jest/members/mock_data';
 import MembersTable from '~/members/components/table/members_table.vue';
 
 const localVue = createLocalVue();
@@ -58,8 +58,7 @@ describe('MemberList', () => {
   describe('fields', () => {
     describe('"Actions" field', () => {
       const memberCanOverride = {
-        ...memberMock,
-        source: { ...memberMock.source, id: 1 },
+        ...directMember,
         canOverride: true,
       };
 

ee/spec/frontend/members/utils_spec.js

-import { member as memberMock, membersJsonString, members } from 'jest/members/mock_data';
+import {
+  member as memberMock,
+  directMember,
+  inheritedMember,
+  membersJsonString,
+  members,
+} from 'jest/members/mock_data';
 import { generateBadges, canOverride, parseDataAttributes } from 'ee/members/utils';
 
 describe('Members Utils', () => {
@@ -30,9 +36,11 @@ describe('Members Utils', () => {
 
   describe('canOverride', () => {
     test.each`
-      member                                  | expected
-      ${{ ...memberMock, canOverride: true }} | ${true}
-      ${memberMock}                           | ${false}
+      member                                        | expected
+      ${{ ...directMember, canOverride: true }}     | ${true}
+      ${{ ...inheritedMember, canOverride: true }}  | ${false}
+      ${{ ...directMember, canOverride: false }}    | ${false}
+      ${{ ...inheritedMember, canOverride: false }} | ${false}
     `('returns $expected', ({ member, expected }) => {
       expect(canOverride(member)).toBe(expected);
     });