From e3f2c7c4c8e3f6950234716673d554f49a041c81 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 09:28:17 +0000 Subject: [PATCH 1/3] Initial plan From c4287d8825fd046bbe7cc6dff8b3cfaad2c1e4c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 09:43:40 +0000 Subject: [PATCH 2/3] Add warning for groups bound to roles when deleting Co-authored-by: nwmac <1955897+nwmac@users.noreply.github.com> --- shell/assets/translations/en-us.yaml | 25 +- .../mixin/__tests__/roleDeletionCheck.test.ts | 264 ++++++++++++++++++ shell/promptRemove/mixin/roleDeletionCheck.js | 12 +- 3 files changed, 294 insertions(+), 7 deletions(-) create mode 100644 shell/promptRemove/mixin/__tests__/roleDeletionCheck.test.ts diff --git a/shell/assets/translations/en-us.yaml b/shell/assets/translations/en-us.yaml index 1576a7ba73b..c0b9c62d764 100644 --- a/shell/assets/translations/en-us.yaml +++ b/shell/assets/translations/en-us.yaml @@ -5802,18 +5802,35 @@ rbac: noContext: label: No Context globalRoles: - notBound: 'No users bound ' - unableToCheck: Unable to check if any user is bound to the role(s). Please try again. + notBound: 'No users or groups bound ' + unableToCheck: Unable to check if any users or groups are bound to the role(s). Please try again. waiting: |- {count, plural, - =1 { Checking if there are any users bound to this role } - other { Checking if there are any users bound to these roles } + =1 { Checking if there are any users or groups bound to this role } + other { Checking if there are any users or groups bound to these roles } } usersBound: |- {count, plural, =1 { Caution: There is 1 user bound to the role(s) about to be deleted. Do you want to proceed? } other { Caution: There are {count} users bound to the role(s) about to be deleted. Do you want to proceed? } } + bound: |- + {users, plural, + =0 {{groups, plural, + =1 { Caution: There is 1 group bound to the role(s) about to be deleted. Do you want to proceed? } + other { Caution: There are {groups} groups bound to the role(s) about to be deleted. Do you want to proceed? } + }} + =1 {{groups, plural, + =0 { Caution: There is 1 user bound to the role(s) about to be deleted. Do you want to proceed? } + =1 { Caution: There is 1 user and 1 group bound to the role(s) about to be deleted. Do you want to proceed? } + other { Caution: There is 1 user and {groups} groups bound to the role(s) about to be deleted. Do you want to proceed? } + }} + other {{groups, plural, + =0 { Caution: There are {users} users bound to the role(s) about to be deleted. Do you want to proceed? } + =1 { Caution: There are {users} users and 1 group bound to the role(s) about to be deleted. Do you want to proceed? } + other { Caution: There are {users} users and {groups} groups bound to the role(s) about to be deleted. Do you want to proceed? } + }} + } types: global: label: Global Permissions diff --git a/shell/promptRemove/mixin/__tests__/roleDeletionCheck.test.ts b/shell/promptRemove/mixin/__tests__/roleDeletionCheck.test.ts new file mode 100644 index 00000000000..5a1dc28b737 --- /dev/null +++ b/shell/promptRemove/mixin/__tests__/roleDeletionCheck.test.ts @@ -0,0 +1,264 @@ +import RoleDeletionCheck from '@shell/promptRemove/mixin/roleDeletionCheck'; +import { MANAGEMENT } from '@shell/config/types'; + +describe('roleDeletionCheck mixin', () => { + describe('handleRoleDeletionCheck', () => { + it('should show "notBound" when no bindings exist', async() => { + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: [] }) // role bindings + .mockResolvedValueOnce({ data: [] }); // users + + const mockT = jest.fn().mockImplementation((key: string) => { + if (key === 'rbac.globalRoles.notBound') { + return 'No users or groups bound'; + } + + return key; + }); + + // Create a context object that mimics the component's `this` + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.notBound', null, true); + expect(context.info).toContain('No users or groups bound'); + expect(context.warning).toBe(''); + }); + + it('should warn when only users are bound to roles', async() => { + const roleBindings = [ + { globalRoleName: 'test-role', userName: 'user-1' }, + { globalRoleName: 'test-role', userName: 'user-2' } + ]; + const users = [ + { id: 'user-1', username: 'admin' }, + { id: 'user-2', username: 'viewer' } + ]; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 2, groups: 0 }); + expect(context.warning).toContain('2 users'); + expect(context.warning).toContain('0 groups'); + expect(context.info).toBe(''); + }); + + it('should warn when only groups are bound to roles', async() => { + const roleBindings = [ + { globalRoleName: 'test-role', groupPrincipalName: 'github_team://123' }, + { globalRoleName: 'test-role', groupPrincipalName: 'github_team://456' } + ]; + const users: any[] = []; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 0, groups: 2 }); + expect(context.warning).toContain('0 users'); + expect(context.warning).toContain('2 groups'); + expect(context.info).toBe(''); + }); + + it('should warn when both users and groups are bound to roles', async() => { + const roleBindings = [ + { globalRoleName: 'test-role', userName: 'user-1' }, + { globalRoleName: 'test-role', groupPrincipalName: 'github_team://123' } + ]; + const users = [ + { id: 'user-1', username: 'admin' } + ]; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 1, groups: 1 }); + expect(context.warning).toContain('1 users'); + expect(context.warning).toContain('1 groups'); + expect(context.info).toBe(''); + }); + + it('should count unique groups across multiple roles', async() => { + const roleBindings = [ + { globalRoleName: 'role-1', groupPrincipalName: 'github_team://123' }, + { globalRoleName: 'role-2', groupPrincipalName: 'github_team://123' }, // same group, different role + { globalRoleName: 'role-2', groupPrincipalName: 'github_team://456' } + ]; + const users: any[] = []; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [ + { id: 'role-1', type: MANAGEMENT.GLOBAL_ROLE }, + { id: 'role-2', type: MANAGEMENT.GLOBAL_ROLE } + ]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 0, groups: 2 }); + expect(context.warning).toContain('2 groups'); + expect(context.info).toBe(''); + }); + + it('should handle single group bound to a role', async() => { + const roleBindings = [ + { globalRoleName: 'test-role', groupPrincipalName: 'github_team://123' } + ]; + const users: any[] = []; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 0, groups: 1 }); + expect(context.warning).toContain('1 groups'); + expect(context.info).toBe(''); + }); + + it('should not count groups when binding has no groupPrincipalName', async() => { + const roleBindings = [ + { + globalRoleName: 'test-role', userName: 'user-1', groupPrincipalName: undefined + }, + { + globalRoleName: 'test-role', userName: 'user-2', groupPrincipalName: null + } + ]; + const users = [ + { id: 'user-1', username: 'admin' }, + { id: 'user-2', username: 'viewer' } + ]; + + const mockDispatch = jest.fn() + .mockResolvedValueOnce({ data: roleBindings }) + .mockResolvedValueOnce({ data: users }); + + const mockT = jest.fn().mockImplementation((key: string, params?: any) => { + if (key === 'rbac.globalRoles.bound') { + return `Caution: ${ params?.users } users and ${ params?.groups } groups bound`; + } + + return key; + }); + + const context = { + t: mockT, + warning: '', + info: '', + $store: { dispatch: mockDispatch }, + }; + + const rolesToRemove = [{ id: 'test-role', type: MANAGEMENT.GLOBAL_ROLE }]; + + await RoleDeletionCheck.methods.handleRoleDeletionCheck.call(context, rolesToRemove, MANAGEMENT.GLOBAL_ROLE, ''); + + expect(mockT).toHaveBeenCalledWith('rbac.globalRoles.bound', { users: 2, groups: 0 }); + expect(context.warning).toContain('2 users'); + expect(context.warning).toContain('0 groups'); + }); + }); +}); diff --git a/shell/promptRemove/mixin/roleDeletionCheck.js b/shell/promptRemove/mixin/roleDeletionCheck.js index 7f38d0e12ab..3391158ee92 100644 --- a/shell/promptRemove/mixin/roleDeletionCheck.js +++ b/shell/promptRemove/mixin/roleDeletionCheck.js @@ -36,6 +36,7 @@ export default { let propToMatch; let numberOfRolesWithBinds = 0; const uniqueUsersWithBinds = new Set(); + const uniqueGroupsWithBinds = new Set(); this.info = this.t('rbac.globalRoles.waiting', { count: rolesToRemove.length }); @@ -80,17 +81,22 @@ export default { if (usedRoles.length) { const uniqueUsers = [...new Set(usedRoles.map((item) => item.userName).filter((user) => userMap[user]))]; + const uniqueGroups = [...new Set(usedRoles.map((item) => item.groupPrincipalName).filter(Boolean))]; - if (uniqueUsers.length) { + if (uniqueUsers.length || uniqueGroups.length) { numberOfRolesWithBinds++; uniqueUsers.forEach((user) => uniqueUsersWithBinds.add(user)); + uniqueGroups.forEach((group) => uniqueGroupsWithBinds.add(group)); } } }); - if (numberOfRolesWithBinds && uniqueUsersWithBinds.size) { + if (numberOfRolesWithBinds && (uniqueUsersWithBinds.size || uniqueGroupsWithBinds.size)) { this.info = ''; - this.warning = this.t('rbac.globalRoles.usersBound', { count: uniqueUsersWithBinds.size }); + this.warning = this.t('rbac.globalRoles.bound', { + users: uniqueUsersWithBinds.size, + groups: uniqueGroupsWithBinds.size + }); } else { this.info = this.t('rbac.globalRoles.notBound', null, true); } From f4662f58ea328835b41cd54e95b9fc509bb850de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 09:47:26 +0000 Subject: [PATCH 3/3] Remove deprecated usersBound translation key Co-authored-by: nwmac <1955897+nwmac@users.noreply.github.com> --- shell/assets/translations/en-us.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/shell/assets/translations/en-us.yaml b/shell/assets/translations/en-us.yaml index c0b9c62d764..8ebad690282 100644 --- a/shell/assets/translations/en-us.yaml +++ b/shell/assets/translations/en-us.yaml @@ -5809,11 +5809,6 @@ rbac: =1 { Checking if there are any users or groups bound to this role } other { Checking if there are any users or groups bound to these roles } } - usersBound: |- - {count, plural, - =1 { Caution: There is 1 user bound to the role(s) about to be deleted. Do you want to proceed? } - other { Caution: There are {count} users bound to the role(s) about to be deleted. Do you want to proceed? } - } bound: |- {users, plural, =0 {{groups, plural,