Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions frontend/public/components/group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ const getImpersonateAction = (
startImpersonate: StartImpersonate,
navigate: NavigateFunction,
): KebabAction => (kind: K8sKind, group: GroupKind) => ({
label: i18next.t('public~Impersonate Group {{name}}', group.metadata),
label: i18next.t('public~Impersonate Group {{name}}', { name: group?.metadata?.name }),
callback: () => {
startImpersonate('Group', group.metadata.name);
startImpersonate('Group', group?.metadata?.name);
navigate(window.SERVER_FLAGS.basePath);
},
// Must use API group authorization.k8s.io, NOT user.openshift.io
// See https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation
accessReview: {
group: 'authorization.k8s.io',
resource: 'groups',
name: group.metadata.name,
name: group?.metadata?.name,
verb: 'impersonate',
Comment on lines +60 to 71
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard impersonation when group name is missing.

At Line 62, startImpersonate can still receive undefined via group?.metadata?.name, which can trigger a runtime failure in the impersonation path. Line 70 propagates the same risk into accessReview.name.

Suggested fix
-const getImpersonateAction = (
-  startImpersonate: StartImpersonate,
-  navigate: NavigateFunction,
-): KebabAction => (kind: K8sKind, group: GroupKind) => ({
-  label: i18next.t('public~Impersonate Group {{name}}', { name: group?.metadata?.name }),
-  callback: () => {
-    startImpersonate('Group', group?.metadata?.name);
-    navigate(window.SERVER_FLAGS.basePath);
-  },
-  accessReview: {
-    group: 'authorization.k8s.io',
-    resource: 'groups',
-    name: group?.metadata?.name,
-    verb: 'impersonate',
-  },
-});
+const getImpersonateAction = (
+  startImpersonate: StartImpersonate,
+  navigate: NavigateFunction,
+): KebabAction => (kind: K8sKind, group: GroupKind) => {
+  const groupName = group?.metadata?.name;
+  return {
+    label: i18next.t('public~Impersonate Group {{name}}', { name: groupName ?? '' }),
+    callback: () => {
+      if (!groupName) {
+        return;
+      }
+      startImpersonate('Group', groupName);
+      navigate(window.SERVER_FLAGS.basePath);
+    },
+    accessReview: {
+      group: 'authorization.k8s.io',
+      resource: 'groups',
+      name: groupName,
+      verb: 'impersonate',
+    },
+  };
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/public/components/group.tsx` around lines 60 - 71, The callback
passes group?.metadata?.name (possibly undefined) into startImpersonate and
accessReview.name which can cause runtime errors; add a guard that checks for a
non-empty group.metadata.name before calling startImpersonate or constructing
the accessReview, e.g., disable or no-op the callback when name is falsy, and
ensure accessReview.name and the i18n label use a safe fallback only when a real
name exists; update the anonymous callback in the menu entry (the function that
calls startImpersonate and navigate) and the accessReview object to only
run/contain values when group?.metadata?.name is defined.

},
});
Expand Down
17 changes: 11 additions & 6 deletions frontend/public/components/modals/add-users-modal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as _ from 'lodash-es';
import * as React from 'react';

import { GroupModel } from '../../models';
Expand All @@ -17,22 +16,28 @@ import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
export const AddUsersModal = (props: AddUsersModalProps) => {
const [values, setValues] = React.useState(['']);
const [handlePromise, inProgress, errorMessage] = usePromiseHandler();
const { t } = useTranslation();

const submit: React.FormEventHandler<HTMLFormElement> = (e) => {
e.preventDefault();
// Append to an existing array, but handle the special case when the array is null.
if (!props.group?.metadata?.name) {
return;
}
const validUsers = values.map((v) => v.trim()).filter((v) => v.length > 0);
if (validUsers.length === 0) {
return;
}
const patch = props.group.users
? _.map(values, (value: string) => ({ op: 'add', path: '/users/-', value }))
: [{ op: 'add', path: '/users', value: values }];
? validUsers.map((value: string) => ({ op: 'add', path: '/users/-', value }))
: [{ op: 'add', path: '/users', value: validUsers }];
handlePromise(k8sPatch(GroupModel, props.group, patch)).then(() => props.close());
};
const { t } = useTranslation();

return (
<form onSubmit={submit} name="form" className="modal-content ">
<ModalTitle>{t('public~Add Users')}</ModalTitle>
<ModalBody>
<p>{t('public~Add new Users to Group {{name}}.', props.group.metadata)}</p>
<p>{t('public~Add new users to group {{name}}', { name: props.group?.metadata?.name })}</p>
<ListInput label={t('public~Users')} required initialValues={values} onChange={setValues} />
</ModalBody>
<ModalSubmitFooter
Expand Down
2 changes: 1 addition & 1 deletion frontend/public/locales/en/public.json
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@
"Add secret as": "Add secret as",
"Prefix": "Prefix",
"Volume": "Volume",
"Add new Users to Group {{name}}.": "Add new Users to Group {{name}}.",
"Add new users to group {{name}}": "Add new users to group {{name}}",
"Edit routing configuration": "Edit routing configuration",
"Group by": "Group by",
"Group wait": "Group wait",
Expand Down