From 514b1b452b1bb418e7c6169934129dbd31bd87c7 Mon Sep 17 00:00:00 2001
From: Erik Jan de Wit <erikjan.dewit@gmail.com>
Date: Fri, 11 Apr 2025 13:53:06 +0200
Subject: [PATCH] make group select required when specific group is selected
 (#38768)

* make group select required when specific group is selected

fixes: #38767
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

* small refactor to make labels not dependant on route

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

* fixed tests

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

---------

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
---
 .../src/clients/authorization/policy/Role.tsx |  2 +-
 .../role-mapping/AddRoleMappingModal.tsx      | 30 ++++++-------------
 .../PermissionsEvaluationTab.tsx              |  1 +
 .../resource-types/GroupSelect.tsx            | 15 ++++------
 .../resource-types/ResourceType.tsx           |  1 +
 .../resource-types/RoleSelect.tsx             | 22 +++++++-------
 .../admin-ui/test/permissions/main.spec.ts    |  4 ++-
 js/apps/admin-ui/test/permissions/main.ts     |  7 +++++
 8 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/js/apps/admin-ui/src/clients/authorization/policy/Role.tsx b/js/apps/admin-ui/src/clients/authorization/policy/Role.tsx
index 99c1d5abd8f..5b6c7c6c157 100644
--- a/js/apps/admin-ui/src/clients/authorization/policy/Role.tsx
+++ b/js/apps/admin-ui/src/clients/authorization/policy/Role.tsx
@@ -66,6 +66,7 @@ export const Role = () => {
                 <AddRoleMappingModal
                   id="role"
                   type="roles"
+                  title={t("assignRole")}
                   onAssign={(rows) => {
                     field.onChange([
                       ...(field.value || []),
@@ -77,7 +78,6 @@ export const Role = () => {
                   onClose={() => {
                     setOpen(false);
                   }}
-                  isLDAPmapper
                 />
               )}
               <Button
diff --git a/js/apps/admin-ui/src/components/role-mapping/AddRoleMappingModal.tsx b/js/apps/admin-ui/src/components/role-mapping/AddRoleMappingModal.tsx
index 510f32a8a94..26b8fed32af 100644
--- a/js/apps/admin-ui/src/components/role-mapping/AddRoleMappingModal.tsx
+++ b/js/apps/admin-ui/src/components/role-mapping/AddRoleMappingModal.tsx
@@ -24,8 +24,6 @@ import useLocaleSort from "../../utils/useLocaleSort";
 import { ResourcesKey, Row, ServiceRole } from "./RoleMapping";
 import { getAvailableRoles } from "./queries";
 import { getAvailableClientRoles } from "./resource";
-import { PermissionsConfigurationTabsParams } from "../../permissions-configuration/routes/PermissionsConfigurationTabs";
-import { useParams } from "react-router-dom";
 
 type AddRoleMappingModalProps = {
   id: string;
@@ -34,7 +32,8 @@ type AddRoleMappingModalProps = {
   isRadio?: boolean;
   onAssign: (rows: Row[]) => void;
   onClose: () => void;
-  isLDAPmapper?: boolean;
+  title?: string;
+  actionLabel?: string;
 };
 
 type FilterType = "roles" | "clients";
@@ -52,9 +51,11 @@ export const AddRoleMappingModal = ({
   id,
   name,
   type,
-  isLDAPmapper,
+  isRadio,
   onAssign,
   onClose,
+  title,
+  actionLabel,
 }: AddRoleMappingModalProps) => {
   const { adminClient } = useAdminClient();
 
@@ -71,7 +72,6 @@ export const AddRoleMappingModal = ({
 
   const localeSort = useLocaleSort();
   const compareRow = ({ role: { name } }: Row) => name?.toUpperCase();
-  const { tab } = useParams<PermissionsConfigurationTabsParams>();
 
   const loader = async (
     first?: number,
@@ -123,13 +123,7 @@ export const AddRoleMappingModal = ({
   return (
     <Modal
       variant={ModalVariant.large}
-      title={
-        tab !== "evaluation"
-          ? isLDAPmapper
-            ? t("assignRole")
-            : t("assignRolesTo", { client: name })
-          : t("selectRole")
-      }
+      title={title || t("assignRolesTo", { client: name })}
       isOpen
       onClose={onClose}
       actions={[
@@ -143,7 +137,7 @@ export const AddRoleMappingModal = ({
             onClose();
           }}
         >
-          {tab !== "evaluation" ? t("assign") : t("select")}
+          {actionLabel || t("assign")}
         </Button>,
         <Button
           data-testid="cancel"
@@ -157,13 +151,7 @@ export const AddRoleMappingModal = ({
     >
       <KeycloakDataTable
         key={key}
-        onSelect={(rows) => {
-          if (tab === "evaluation") {
-            setSelectedRows(rows.length > 0 ? [rows[0]] : []);
-          } else {
-            setSelectedRows([...rows]);
-          }
-        }}
+        onSelect={(rows) => setSelectedRows([...rows])}
         searchPlaceholderKey="searchByRoleName"
         isPaginated={!(filterType === "roles" && type !== "roles")}
         searchTypeComponent={
@@ -202,7 +190,7 @@ export const AddRoleMappingModal = ({
           )
         }
         canSelectAll
-        isRadio={tab === "evaluation"}
+        isRadio={isRadio}
         loader={filterType === "roles" ? loader : clientRolesLoader}
         ariaLabelKey="associatedRolesText"
         columns={[
diff --git a/js/apps/admin-ui/src/permissions-configuration/permission-evaluation/PermissionsEvaluationTab.tsx b/js/apps/admin-ui/src/permissions-configuration/permission-evaluation/PermissionsEvaluationTab.tsx
index c22a806d1f5..52a2d66392f 100644
--- a/js/apps/admin-ui/src/permissions-configuration/permission-evaluation/PermissionsEvaluationTab.tsx
+++ b/js/apps/admin-ui/src/permissions-configuration/permission-evaluation/PermissionsEvaluationTab.tsx
@@ -196,6 +196,7 @@ const PermissionEvaluateContent = ({ client }: Props) => {
                       defaultValue={[]}
                       variant="typeahead"
                       isRequired
+                      isRadio
                     />
                   )}
                   <SelectControl
diff --git a/js/apps/admin-ui/src/permissions-configuration/resource-types/GroupSelect.tsx b/js/apps/admin-ui/src/permissions-configuration/resource-types/GroupSelect.tsx
index e95e21adddb..e03c5b1a844 100644
--- a/js/apps/admin-ui/src/permissions-configuration/resource-types/GroupSelect.tsx
+++ b/js/apps/admin-ui/src/permissions-configuration/resource-types/GroupSelect.tsx
@@ -69,14 +69,11 @@ export const GroupSelect = ({
         name={name!}
         control={control}
         defaultValue={defaultValue}
-        rules={
-          isRequired
-            ? {
-                validate: (value?: GroupRepresentation[]) =>
-                  value && value.length > 0,
-              }
-            : undefined
-        }
+        rules={{
+          validate: (value?: string[]) => {
+            return isRequired && value && value.length > 0;
+          },
+        }}
         render={({ field }) => (
           <>
             {open && (
@@ -147,7 +144,7 @@ export const GroupSelect = ({
           </Tbody>
         </Table>
       )}
-      {errors.groups && <FormErrorText message={t("requiredGroups")} />}
+      {errors[name!] && <FormErrorText message={t("requiredGroups")} />}
     </FormGroup>
   );
 };
diff --git a/js/apps/admin-ui/src/permissions-configuration/resource-types/ResourceType.tsx b/js/apps/admin-ui/src/permissions-configuration/resource-types/ResourceType.tsx
index 57bf96f35f8..80c9fe60bcc 100644
--- a/js/apps/admin-ui/src/permissions-configuration/resource-types/ResourceType.tsx
+++ b/js/apps/admin-ui/src/permissions-configuration/resource-types/ResourceType.tsx
@@ -96,6 +96,7 @@ export const ResourceType = ({
           })}
           defaultValue={[]}
           variant="typeaheadMulti"
+          isRequired={withEnforceAccessTo}
         />
       )}
     </>
diff --git a/js/apps/admin-ui/src/permissions-configuration/resource-types/RoleSelect.tsx b/js/apps/admin-ui/src/permissions-configuration/resource-types/RoleSelect.tsx
index 27c79fb44c1..ce0d83fe608 100644
--- a/js/apps/admin-ui/src/permissions-configuration/resource-types/RoleSelect.tsx
+++ b/js/apps/admin-ui/src/permissions-configuration/resource-types/RoleSelect.tsx
@@ -12,14 +12,13 @@ import {
 } from "@keycloak/keycloak-ui-shared";
 import { AddRoleMappingModal } from "../../components/role-mapping/AddRoleMappingModal";
 import { Row, ServiceRole } from "../../components/role-mapping/RoleMapping";
-import { PermissionsConfigurationTabsParams } from "../routes/PermissionsConfigurationTabs";
-import { useParams } from "react-router-dom";
 
 type RoleSelectorProps = {
   name: string;
+  isRadio?: boolean;
 };
 
-export const RoleSelect = ({ name }: RoleSelectorProps) => {
+export const RoleSelect = ({ name, isRadio = false }: RoleSelectorProps) => {
   const { adminClient } = useAdminClient();
   const { t } = useTranslation();
   const {
@@ -30,7 +29,6 @@ export const RoleSelect = ({ name }: RoleSelectorProps) => {
   const values = getValues(name) || [];
   const [isModalOpen, setIsModalOpen] = useState(false);
   const [selectedRoles, setSelectedRoles] = useState<Row[]>([]);
-  const { tab } = useParams<PermissionsConfigurationTabsParams>();
 
   useFetch(
     async () => {
@@ -55,12 +53,10 @@ export const RoleSelect = ({ name }: RoleSelectorProps) => {
 
   return (
     <FormGroup
-      label={tab !== "evaluation" ? t("roles") : t("role")}
+      label={isRadio ? t("role") : t("roles")}
       labelIcon={
         <HelpItem
-          helpText={
-            tab !== "evaluation" ? t("policyRolesHelp") : t("selectRole")
-          }
+          helpText={isRadio ? t("selectRole") : t("policyRolesHelp")}
           fieldLabelId="roles"
         />
       }
@@ -71,19 +67,21 @@ export const RoleSelect = ({ name }: RoleSelectorProps) => {
         <AddRoleMappingModal
           id="role"
           type="roles"
+          title={t("selectRole")}
+          actionLabel={t("select")}
+          isRadio={isRadio}
           onAssign={(rows) => {
             setValue(name, [
-              ...values,
+              ...(!isRadio ? values : []),
               ...rows
                 .filter((row) => row.role.id !== undefined)
                 .map((row) => row.role.id!),
             ]);
 
-            setSelectedRoles([...selectedRoles, ...rows]);
+            setSelectedRoles(isRadio ? rows : [...selectedRoles, ...rows]);
             setIsModalOpen(false);
           }}
           onClose={() => setIsModalOpen(false)}
-          isLDAPmapper
         />
       )}
       <Button
@@ -91,7 +89,7 @@ export const RoleSelect = ({ name }: RoleSelectorProps) => {
         variant="secondary"
         onClick={() => setIsModalOpen(true)}
       >
-        {tab !== "evaluation" ? t("addRoles") : t("selectRole")}
+        {isRadio ? t("selectRole") : t("addRoles")}
       </Button>
       {selectedRoles.length > 0 && (
         <Table variant="compact">
diff --git a/js/apps/admin-ui/test/permissions/main.spec.ts b/js/apps/admin-ui/test/permissions/main.spec.ts
index 3eee6be48b3..ec8c527f995 100644
--- a/js/apps/admin-ui/test/permissions/main.spec.ts
+++ b/js/apps/admin-ui/test/permissions/main.spec.ts
@@ -11,6 +11,7 @@ import {
   clickCreatePermission,
   clickCreatePolicySaveButton,
   clickSearchButton,
+  deletePermission,
   fillPermissionForm,
   goToEvaluation,
   goToPermissions,
@@ -76,6 +77,7 @@ test.describe("Permissions section tests", () => {
 
     await goToPermissions(page);
     await assertRowExists(page, "test-permission");
+    await deletePermission(page, "test-permission");
     await goToPolicies(page);
     await assertRowExists(page, "test-policy");
   });
@@ -95,7 +97,7 @@ test.describe("Permissions section tests", () => {
     await fillPolicyForm(
       page,
       {
-        name: "test-policy",
+        name: "test-policy2",
         description: "test-description",
         type: "User",
         user: "test-user",
diff --git a/js/apps/admin-ui/test/permissions/main.ts b/js/apps/admin-ui/test/permissions/main.ts
index cfe065fe396..9743350a432 100644
--- a/js/apps/admin-ui/test/permissions/main.ts
+++ b/js/apps/admin-ui/test/permissions/main.ts
@@ -1,6 +1,8 @@
 import PolicyRepresentation from "@keycloak/keycloak-admin-client/lib/defs/policyRepresentation";
 import { Page } from "@playwright/test";
 import { selectItem } from "../utils/form";
+import { confirmModal } from "../utils/modal";
+import { clickRowKebabItem } from "../utils/table";
 
 type PermissionForm = PolicyRepresentation & {
   enforcementMode?: "allResources" | "specificResources";
@@ -68,3 +70,8 @@ export async function openSearchPanel(page: Page) {
 export async function clickSearchButton(page: Page) {
   await page.getByTestId("search-btn").click();
 }
+
+export async function deletePermission(page: Page, name: string) {
+  await clickRowKebabItem(page, name, "Delete");
+  await confirmModal(page);
+}