Skip to content

Commit 229604a

Browse files
Leo6Leologonoffclaude
authored
CONSOLE-4734: Multi-group impersonation Combined (#15612)
* docs: clarify comment for multi-group impersonation feature flag * Add groups to the redux store and actions * CONSOLE-4784: WebSocket Subprotocol Parsing for multi-group impersonation Support multiple Impersonate-Group subprotocols on WebSocket connections. Maintains backward compatibility for single-group impersonation. * CONSOLE-4788: Backend Request Headers Updates for multi-group impersonation Handle X-Console-Impersonate-Groups header by splitting comma-separated groups into multiple Impersonate-Group headers for Kubernetes API. * CONSOLE-4785: GraphQL Context Updates for multi-group impersonation Update GraphQL context to support array-valued headers for multi-group impersonation. Change headers type from map[string]string to map[string]interface{} to support both single string values and []string arrays. * CONSOLE-4789: UI Actions for multi-group impersonation fetch utilities Update fetch utilities to handle multi-group impersonation: - Support array-valued Impersonate-Group headers - Convert array to X-Console-Impersonate-Groups for fetch() API compatibility - Add UserWithGroups kind support in header generation * CONSOLE-4787: Redux Actions & States Updates for multi-group impersonation Add multi-group support to Redux actions: - Update startImpersonate to accept groups parameter - Encode multiple groups as WebSocket subprotocols in UserWithGroups mode - Pass groups to beginImpersonate action - Expose store and UIActions for testing (temporary) * CONSOLE-4782: Group Impersonation Modal Add modal component for multi-group user impersonation: - TypeScript React component with group selection - Multi-select group input with search/filter - Chip display for selected groups - Form validation for username - Internationalization support * CONSOLE-4782: UI Integration for multi-group impersonation Integrate multi-group impersonation into the UI: - Update impersonation banner to display multiple groups - Add modal trigger in masthead toolbar - Update user component to show impersonated user - Add feature flag checks in app component - Add masthead menu items for start/stop impersonation - Add enabled parameter to usePackageManifestCheck hook * fix: fix logonoff's review comment * fix: fix the suggestions from Kevin regarding the frontend UI changes in the multi-group impersonation modal. * nit: formatting change * fix: fix the review comment regarding the frontend changes provided by logonoff Co-authored-by: logonoff <[email protected]> * feat: integrate dynamic group fetching in ImpersonateUserModal * test: Add IMPERSONATE flag to featureReducer tests * fix: revert the debug work * fix: fix logonoff's review comment * i18n: run translation * refactor: use useCallBack to prevent unnecessary rerendering * feat: add error alert for group loading failure in ImpersonateUserModal * feat: enhance accessibility in ImpersonateUserModal with ARIA labels and descriptions * fix: remove outdated comment * refactor: simplify impersonation group handling with backward compatibility Consolidate ImpersonateGroup and ImpersonateGroups fields into a single ImpersonateGroup []string field with custom UnmarshalJSON method that supports both string (single group) and array (multiple groups) formats for backward compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * CONSOLE-4782: Remove IMPERSONATE feature flag Multi-group impersonation is now always enabled. Removed the feature flag and all associated conditional checks to simplify the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: refresh the page after stop impersonation * fix: address the review comments from @TheRealJon * test: add comprehensive unit tests for ImpersonateUserModal Add 21 unit tests covering: - Modal rendering in open/closed states - Username input handling (typing, prefilled, readonly, edge cases) - Groups multi-select field functionality - Form validation (empty username, disabled submit button) - Loading and error states - Form submission with trimming and modal closing - Modal lifecycle and reset behavior - Expandable groups feature (>5 selected groups) * Display all groups when ≤5 selected * Hide groups beyond 5 and show "+N" button * Expand to show all groups when "+N" clicked * Auto-collapse when count reduces to ≤5 All tests use data-test attributes for stable selectors and properly handle PatternFly v6 modal portal rendering. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: add integration tests for ImpersonateUserModal with Redux Add 10 integration tests covering complete workflows with Redux state: - Redux action dispatching (startImpersonate with user only) - Redux action dispatching (startImpersonate with user and groups) - Complete group selection workflow * Select single group * Select multiple groups * Deselect groups - Search and filter groups functionality - Error handling workflow with Redux - Modal lifecycle with state management - Readonly mode behavior Integration tests use Provider wrapper with mock Redux store to verify complete user flows from UI interaction through to Redux action dispatch. All tests properly handle async operations and PatternFly v6 components. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: update navigation handling in MastheadToolbar and ImpersonateUserModal * refactor: remove unnecessary React imports from impersonate user modal tests * test: add smoke tests for Multi-Group Impersonation functionality Introduce a comprehensive suite of smoke tests for the impersonation feature, ensuring critical paths are validated. The tests cover: - Modal opening and user impersonation - Banner persistence across navigation - Stopping impersonation - Validation of username input and group selection These tests are designed to quickly identify major breaking changes and ensure the core functionality remains intact. Expected run time is approximately 2-3 minutes. * test: add E2E Cypress tests for Multi-Group Impersonation with RBAC Introduce a comprehensive suite of end-to-end tests for the Multi-Group Impersonation feature, focusing on role-based access control (RBAC). The tests cover: - Setup of Kubernetes resources (namespaces, users, groups, role bindings) - Verification of access control when impersonating users across different groups - Cleanup of resources post-testing These tests ensure that the impersonation functionality adheres to expected RBAC permissions and provides a robust validation of user access across multiple namespaces. * nit: the word "more" is mistreated as noun in the i18n * fix: apply CodeRabbit's suggestion * feat: enhance impersonation handling and feature refresh - the feature didn't work on firefox, but this fix it, and on both chrome and firefox it works. * fix: apply the review comment from coderabbit * i18n: run i18n to fix the failing CI * i18n: running on a different laptop * refactor: extract impersonation feature refresh logic into a custom hook * fix: Implement normalizeConsoleHeaders utility to ensure compatibility with fetch API. * test: fix the flaky guided tour issue * test: thanks to @yapei suggestion, I added more guidedTour.close to the test to reduce the flake * test: thanks to @yapei's suggestion, put guidedTour.close after the impersonation button is clicked: since after staritng the impersonation, the modal might show up and blocking the test * test: remove the cypress test per discussion with @yapei --------- Co-authored-by: logonoff <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 2243c06 commit 229604a

File tree

21 files changed

+1745
-80
lines changed

21 files changed

+1745
-80
lines changed

frontend/packages/console-app/src/actions/hooks/useGroupActions.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { useCallback, useMemo } from 'react';
22
import { useTranslation } from 'react-i18next';
3-
import { useDispatch } from 'react-redux';
3+
import { useDispatch, useSelector } from 'react-redux';
44
import { useNavigate } from 'react-router-dom-v5-compat';
5-
import { Action } from '@console/dynamic-plugin-sdk';
5+
import { Action, getImpersonate } from '@console/dynamic-plugin-sdk';
66
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
77
import * as UIActions from '@console/internal/actions/ui';
88
import { asAccessReview } from '@console/internal/components/utils/rbac';
99
import { GroupModel } from '@console/internal/models';
1010
import { GroupKind } from '@console/internal/module/k8s';
11+
import { RootState } from '@console/internal/redux';
1112
import AddGroupUsersModal from '../../components/modals/add-group-users-modal';
1213

1314
/**
@@ -18,14 +19,25 @@ export const useGroupActions = (obj: GroupKind): Action[] => {
1819
const dispatch = useDispatch();
1920
const navigate = useNavigate();
2021
const launchOverlay = useOverlay();
22+
const impersonate = useSelector((state: RootState) => getImpersonate(state));
2123

2224
const startImpersonate = useCallback(
2325
(kind: string, name: string) => dispatch(UIActions.startImpersonate(kind, name)),
2426
[dispatch],
2527
);
2628

29+
const stopImpersonate = useCallback(() => dispatch(UIActions.stopImpersonate()), [dispatch]);
30+
2731
const factory = useMemo(
2832
() => ({
33+
stopImpersonate: (): Action => ({
34+
id: 'stop-impersonate',
35+
label: t('public~Stop impersonating'),
36+
cta: () => {
37+
stopImpersonate();
38+
navigate(window.SERVER_FLAGS.basePath);
39+
},
40+
}),
2941
impersonate: (): Action => ({
3042
id: 'impersonate-group',
3143
label: t('public~Impersonate Group {{name}}', obj.metadata),
@@ -42,8 +54,12 @@ export const useGroupActions = (obj: GroupKind): Action[] => {
4254
accessReview: asAccessReview(GroupModel, obj, 'patch'),
4355
}),
4456
}),
45-
[navigate, obj, startImpersonate, t, launchOverlay],
57+
[navigate, obj, startImpersonate, stopImpersonate, t, launchOverlay],
4658
);
4759

48-
return useMemo<Action[]>(() => [factory.impersonate(), factory.addUsers()], [factory]);
60+
return useMemo<Action[]>(() => {
61+
// Determine which impersonation action to show based on impersonation state
62+
const impersonationAction = impersonate ? factory.stopImpersonate() : factory.impersonate();
63+
return [impersonationAction, factory.addUsers()];
64+
}, [impersonate, factory]);
4965
};

frontend/packages/console-dynamic-plugin-sdk/src/app/core/actions/core.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ export enum ActionType {
1616
export const setUser = (userInfo: UserInfo) => action(ActionType.SetUser, { userInfo });
1717
export const setUserResource = (userResource: UserKind) =>
1818
action(ActionType.SetUserResource, { userResource });
19-
export const beginImpersonate = (kind: string, name: string, subprotocols: string[]) =>
20-
action(ActionType.BeginImpersonate, { kind, name, subprotocols });
19+
export const beginImpersonate = (
20+
kind: string,
21+
name: string,
22+
subprotocols: string[],
23+
groups?: string[],
24+
) => action(ActionType.BeginImpersonate, { kind, name, subprotocols, groups });
2125
export const endImpersonate = () => action(ActionType.EndImpersonate);
2226
export const setAdmissionWebhookWarning = (id: string, warning: AdmissionWebhookWarning) =>
2327
action(ActionType.SetAdmissionWebhookWarning, { id, warning });

frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/core.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export const coreReducer = (
2727
kind: action.payload.kind,
2828
name: action.payload.name,
2929
subprotocols: action.payload.subprotocols,
30+
groups: action.payload.groups,
3031
},
3132
};
3233
case ActionType.EndImpersonate: {

frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export type ImpersonateKind = {
1313
kind: string;
1414
name: string;
1515
subprotocols: string[];
16+
groups?: string[];
1617
};
1718

1819
export type CoreState = {

frontend/packages/console-dynamic-plugin-sdk/src/utils/fetch/console-fetch-utils.ts

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { getImpersonate } from '../../app/core/reducers';
22
import storeHandler from '../../app/storeHandler';
33

44
type ConsoleRequestHeaders = {
5-
'Impersonate-Group'?: string;
5+
'Impersonate-Group'?: string | string[];
66
'Impersonate-User'?: string;
77
'X-CSRFToken'?: string;
88
};
@@ -35,14 +35,54 @@ export const getConsoleRequestHeaders = (): ConsoleRequestHeaders => {
3535
};
3636

3737
// Set impersonation headers
38-
const { kind, name } = getImpersonate(state) || {};
39-
if ((kind === 'User' || kind === 'Group') && name) {
40-
// Even if we are impersonating a group, we still need to set Impersonate-User to something or k8s will complain
41-
headers['Impersonate-User'] = name;
42-
if (kind === 'Group') {
38+
const impersonateData = getImpersonate(state);
39+
if (impersonateData) {
40+
const { kind, name, groups } = impersonateData;
41+
42+
if (kind === 'User' && name) {
43+
// Simple user impersonation
44+
headers['Impersonate-User'] = name;
45+
} else if (kind === 'Group' && name) {
46+
// Single group impersonation (backward compatibility)
47+
// Even if we are impersonating a group, we still need to set Impersonate-User to something or k8s will complain
48+
headers['Impersonate-User'] = name;
4349
headers['Impersonate-Group'] = name;
50+
} else if (kind === 'UserWithGroups' && name && groups && groups.length > 0) {
51+
// User with multiple groups impersonation
52+
headers['Impersonate-User'] = name;
53+
// Note: This creates an array of values for the same header key
54+
headers['Impersonate-Group'] = groups;
4455
}
4556
}
4657

4758
return headers;
4859
};
60+
61+
/**
62+
* Normalizes console headers to be compatible with fetch API's HeadersInit.
63+
* Converts array values (like Impersonate-Group) to a format that fetch() accepts.
64+
* @param headers - Headers object that may contain array values
65+
* @returns Normalized headers object with only string values
66+
*/
67+
export const normalizeConsoleHeaders = (
68+
headers: Record<string, string | string[] | undefined>,
69+
): Record<string, string> => {
70+
const normalized: Record<string, string> = {};
71+
72+
Object.entries(headers || {}).forEach(([key, value]) => {
73+
if (Array.isArray(value)) {
74+
// For multiple Impersonate-Group headers, we need special handling
75+
// because fetch() API combines them into a single comma-separated header
76+
// which doesn't work for Kubernetes impersonation
77+
if (key === 'Impersonate-Group') {
78+
// Send as a special header that the backend will split
79+
normalized['X-Console-Impersonate-Groups'] = value.join(',');
80+
}
81+
// Skip other array values as they're not supported by fetch HeadersInit
82+
} else if (value) {
83+
normalized[key] = value;
84+
}
85+
});
86+
87+
return normalized;
88+
};

frontend/packages/console-dynamic-plugin-sdk/src/utils/fetch/console-fetch.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { setAdmissionWebhookWarning } from '../../app/core/actions';
44
import storeHandler from '../../app/storeHandler';
55
import { ConsoleFetchText, ConsoleFetchJSON, ConsoleFetch } from '../../extensions/console-types';
66
import { TimeoutError } from '../error/http-error';
7-
import { getConsoleRequestHeaders } from './console-fetch-utils';
7+
import { getConsoleRequestHeaders, normalizeConsoleHeaders } from './console-fetch-utils';
88

99
/**
1010
* A custom wrapper around `fetch` that adds console-specific headers and allows for retries and timeouts.
@@ -50,9 +50,13 @@ const consoleFetchCommon = async (
5050
options: RequestInit = {},
5151
timeout?: number,
5252
): Promise<Response | string> => {
53-
const headers = getConsoleRequestHeaders();
54-
// Pass headers last to let callers to override Accept.
55-
const allOptions = _.defaultsDeep({ method }, options, { headers });
53+
const consoleHeaders = getConsoleRequestHeaders();
54+
const normalizedConsoleHeaders = normalizeConsoleHeaders(consoleHeaders);
55+
56+
// Merge headers properly - console headers first, then let options override
57+
const mergedHeaders = { ...normalizedConsoleHeaders, ...options.headers };
58+
const allOptions = _.defaultsDeep({ method, headers: mergedHeaders }, options);
59+
5660
const response = await consoleFetch(url, allOptions, timeout);
5761
const dataPromise = parseData(response);
5862
const warning = response.headers.get('Warning');
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
export { consoleFetch, consoleFetchJSON, consoleFetchText } from './console-fetch';
2-
export { getConsoleRequestHeaders } from './console-fetch-utils';
2+
export { getConsoleRequestHeaders, normalizeConsoleHeaders } from './console-fetch-utils';

frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { useState, useMemo, useCallback, useRef, useEffect } from 'react';
22
import { CatalogItem } from '@console/dynamic-plugin-sdk/src/extensions/catalog';
33
import { consoleFetch } from '@console/dynamic-plugin-sdk/src/lib-core';
4-
import { getConsoleRequestHeaders } from '@console/dynamic-plugin-sdk/src/utils/fetch';
4+
import {
5+
getConsoleRequestHeaders,
6+
normalizeConsoleHeaders,
7+
} from '@console/dynamic-plugin-sdk/src/utils/fetch';
58
import { ONE_SECOND } from '@console/shared/src/constants/time';
69
import { usePoll } from '@console/shared/src/hooks/usePoll';
710
import { OLMCatalogItem } from '../types';
@@ -22,8 +25,9 @@ const useCatalogItems: UseCatalogItems = () => {
2225

2326
const headers = useMemo(() => {
2427
const consoleHeaders = getConsoleRequestHeaders();
28+
const normalizedHeaders = normalizeConsoleHeaders(consoleHeaders);
2529
return {
26-
...consoleHeaders,
30+
...normalizedHeaders,
2731
'If-Modified-Since': lastModified,
2832
'Cache-Control': 'max-age=300',
2933
};

frontend/public/actions/ui.ts

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,21 @@ export const setActiveNamespace = (namespace: string = '') => {
188188
return action(ActionType.SetActiveNamespace, { namespace });
189189
};
190190

191-
export const startImpersonate = (kind: string, name: string) => async (dispatch, getState) => {
191+
/**
192+
* Encodes a string for use in Kubernetes impersonation subprotocols.
193+
* Subprotocols are comma-separated, so commas aren't allowed. Also "="
194+
* and "/" aren't allowed, so we base64 encode and replace illegal chars.
195+
*/
196+
const encodeImpersonationValue = (value: string, textEncoder: TextEncoder): string => {
197+
return Base64.encode(String.fromCharCode.apply(String, textEncoder.encode(value)))
198+
.replace(/=/g, '_')
199+
.replace(/\//g, '-');
200+
};
201+
202+
export const startImpersonate = (kind: string, name: string, groups?: string[]) => async (
203+
dispatch,
204+
getState,
205+
) => {
192206
const textEncoder = new TextEncoder();
193207

194208
const imp = getImpersonate(getState());
@@ -198,25 +212,38 @@ export const startImpersonate = (kind: string, name: string) => async (dispatch,
198212
return;
199213
}
200214

201-
/**
202-
* Subprotocols are comma-separated, so commas aren't allowed. Also "="
203-
* and "/" aren't allowed, so base64 but replace illegal chars.
204-
*/
205-
const encodedName = Base64.encode(String.fromCharCode.apply(String, textEncoder.encode(name)))
206-
.replace(/=/g, '_')
207-
.replace(/\//g, '-');
215+
const encodedName = encodeImpersonationValue(name, textEncoder);
208216

209217
let subprotocols;
210218
if (kind === 'User') {
211219
subprotocols = [`Impersonate-User.${encodedName}`];
212-
}
213-
if (kind === 'Group') {
220+
} else if (kind === 'Group') {
214221
subprotocols = [`Impersonate-Group.${encodedName}`];
222+
} else if (kind === 'UserWithGroups' && groups && groups.length > 0) {
223+
// User with multiple groups impersonation
224+
// Encode user subprotocol
225+
subprotocols = [`Impersonate-User.${encodedName}`];
226+
// Encode each group as a separate subprotocol
227+
groups.forEach((group) => {
228+
const encodedGroup = encodeImpersonationValue(group, textEncoder);
229+
subprotocols.push(`Impersonate-Group.${encodedGroup}`);
230+
});
215231
}
216232

217-
dispatch(beginImpersonate(kind, name, subprotocols));
233+
dispatch(beginImpersonate(kind, name, subprotocols, groups));
234+
235+
// Close WebSocket to trigger reconnection with new impersonation headers
236+
// Wait for the close to complete before proceeding
218237
subsClient.close(false, true);
219-
dispatch(clearSSARFlags());
238+
239+
// Don't clear/refresh flags here - the App component's useLayoutEffect will handle it
240+
// This ensures flags refresh happens in sync with React's render cycle
241+
};
242+
243+
// Action to refresh features after impersonation change
244+
// Don't clear flags - just re-detect them. Old values remain until new ones are fetched.
245+
// This prevents components from seeing PENDING state and showing loading spinners.
246+
export const refreshFeaturesAfterImpersonation = () => (dispatch) => {
220247
dispatch(detectFeatures());
221248
};
222249
export const stopImpersonate = () => (dispatch) => {

frontend/public/co-fetch.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,33 @@
11
import * as _ from 'lodash';
22
import { HttpError, RetryError } from '@console/dynamic-plugin-sdk/src/utils/error/http-error';
33
import { authSvc } from './module/auth';
4-
import { getCSRFToken } from '@console/dynamic-plugin-sdk/src/utils/fetch/console-fetch-utils';
4+
import { getConsoleRequestHeaders } from '@console/dynamic-plugin-sdk/src/utils/fetch/console-fetch-utils';
55

66
export const applyConsoleHeaders = (url, options) => {
7-
const token = getCSRFToken();
8-
if (options.headers) {
9-
options.headers['X-CSRFToken'] = token;
10-
} else {
11-
options.headers = { 'X-CSRFToken': token };
7+
const consoleHeaders = getConsoleRequestHeaders();
8+
9+
if (!options.headers) {
10+
options.headers = {};
1211
}
1312

13+
// Apply console headers, handling array values for multiple headers
14+
Object.entries(consoleHeaders || {}).forEach(([key, value]) => {
15+
if (Array.isArray(value)) {
16+
// For multiple Impersonate-Group headers, we need special handling
17+
// because fetch() API combines them into a single comma-separated header
18+
// which doesn't work for Kubernetes impersonation
19+
if (key === 'Impersonate-Group') {
20+
// Send as a special header that the backend will split
21+
options.headers['X-Console-Impersonate-Groups'] = value.join(',');
22+
} else {
23+
// For other array headers, store as array
24+
options.headers[key] = value;
25+
}
26+
} else if (value) {
27+
options.headers[key] = value;
28+
}
29+
});
30+
1431
// X-CSRFToken is used only for non-GET requests targeting bridge
1532
if (options.method === 'GET' || url.indexOf('://') >= 0) {
1633
delete options.headers['X-CSRFToken'];

0 commit comments

Comments
 (0)