add description to groups

fixes #39172 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> Signed-off-by: Alexander Schwartz <aschwart@redhat.com> Co-authored-by: Alexander Schwartz <aschwart@redhat.com>
2025-12-21 14:30:05 -06:00 · 2025-05-14 12:41:01 +02:00
parent 7776e8c587
commit cbd0d18f6a
21 changed files with 142 additions and 23 deletions
--- a/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java
+++ b/core/src/main/java/org/keycloak/representations/idm/GroupRepresentation.java
@@ -34,6 +34,7 @@ public class GroupRepresentation {
    // to identify a group and operate on it in a basic way
    protected String id;
    protected String name;
+    protected String description;
    protected String path;
    protected String parentId;
    protected Long subGroupCount;
@@ -62,6 +63,14 @@ public class GroupRepresentation {
        this.name = name;
    }

+    public String getDescription() {
+        return description;
+    }
+
+    public void setDescription(String description) {
+        this.description = description;
+    }
+
    public String getPath() {
        return path;
    }
--- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
+++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
@@ -526,7 +526,6 @@ showRemaining=Show ${remaining}
 searchProfile=Search profile
 eventTypes.UPDATE_EMAIL_ERROR.name=Update email error
 removeConfirm_other=Are you sure you want to remove these groups?
-renameGroup=Rename group
 configure=Configure
 searchScopeHelp=For one level, the search applies only for users in the DNs specified by User DNs. For subtree, the search applies to the whole subtree. See LDAP documentation for more details.
 jumpToSection=Jump to section
@@ -2101,7 +2100,7 @@ tableView=Table view
 addClientProfile=Add client profile
 maxFailureWaitSeconds=Max wait
 userEventsRegistered=User events registered
-renameAGroup=Rename group
+editGroup=Edit group
 eventConfigError=Could not save event configuration\: {{error}}
 confirmAccessTokenTitle=Regenerate registration access token?
 target=Target
--- a/js/apps/admin-ui/src/groups/GroupTable.tsx
+++ b/js/apps/admin-ui/src/groups/GroupTable.tsx
@@ -159,7 +159,7 @@ export const GroupTable = ({ refresh: viewRefresh }: GroupTableProps) => {
            ? []
            : [
                {
-                  title: t("rename"),
+                  title: t("edit"),
                  onRowClick: async (group) => {
                    setRename(group);
                    return false;
--- a/js/apps/admin-ui/src/groups/GroupsModal.tsx
+++ b/js/apps/admin-ui/src/groups/GroupsModal.tsx
@@ -54,7 +54,10 @@ export const GroupsModal = ({
    useState<GroupRepresentation | null>(null);

  const form = useForm({
-    defaultValues: { name: "" },
+    defaultValues: {
+      name: rename?.name || "",
+      description: rename?.description || "",
+    },
  });
  const { handleSubmit, formState } = form;

@@ -235,13 +238,13 @@ export const GroupsModal = ({
      } else if (rename) {
        await adminClient.groups.update(
          { id },
-          { ...rename, name: group.name },
+          { ...rename, name: group.name, description: group.description },
        );
      } else {
        await adminClient.groups.updateChildGroup({ id }, group);
      }

-      refresh(rename ? { ...rename, name: group.name } : undefined);
+      refresh(rename ? { ...rename, ...group } : undefined);
      handleModalToggle();
      addAlert(
        t(
@@ -263,12 +266,12 @@ export const GroupsModal = ({
      variant={ModalVariant.small}
      title={
        rename
-          ? t("renameAGroup")
+          ? t("editGroup")
          : duplicateId
            ? t("duplicateAGroup")
            : t("createAGroup")
      }
-      isOpen={true}
+      isOpen
      onClose={handleModalToggle}
      actions={[
        <FormSubmitButton
@@ -279,7 +282,7 @@ export const GroupsModal = ({
          allowInvalid
          allowNonDirty
        >
-          {t(rename ? "rename" : duplicateId ? "duplicate" : "create")}
+          {t(rename ? "edit" : duplicateId ? "duplicate" : "create")}
        </FormSubmitButton>,
        <Button
          id="modal-cancel"
@@ -308,6 +311,7 @@ export const GroupsModal = ({
            rules={{ required: t("required") }}
            autoFocus
          />
+          <TextControl name="description" label={t("description")} />
        </Form>
      </FormProvider>
    </Modal>
--- a/js/apps/admin-ui/src/groups/GroupsSection.tsx
+++ b/js/apps/admin-ui/src/groups/GroupsSection.tsx
@@ -169,7 +169,7 @@ export default function GroupsSection() {
                          key="renameGroup"
                          onClick={() => setRename(currentGroup())}
                        >
-                          {t("renameGroup")}
+                          {t("edit")}
                        </DropdownItem>,
                        <DropdownItem
                          data-testid="deleteGroup"
@@ -182,6 +182,9 @@ export default function GroupsSection() {
                    : undefined
                }
              />
+              <PageSection className="pf-v5-u-pt-0">
+                {currentGroup()?.description}
+              </PageSection>
              {subGroups.length > 0 && (
                <Tabs
                  inset={{
--- a/js/apps/admin-ui/src/groups/components/GroupTree.tsx
+++ b/js/apps/admin-ui/src/groups/components/GroupTree.tsx
@@ -125,7 +125,7 @@ const GroupTreeContextMenu = ({
      >
        <DropdownList>
          <DropdownItem key="rename" onClick={toggleRenameOpen}>
-            {t("rename")}
+            {t("edit")}
          </DropdownItem>
          <DropdownItem key="move" onClick={toggleMoveOpen}>
            {t("moveTo")}
--- a/js/apps/admin-ui/test/groups/list.spec.ts
+++ b/js/apps/admin-ui/test/groups/list.spec.ts
@@ -1,4 +1,4 @@
-import { test } from "@playwright/test";
+import { expect, test } from "@playwright/test";
 import { v4 as uuid } from "uuid";
 import adminClient from "../utils/AdminClient";
 import { login } from "../utils/login";
@@ -14,10 +14,11 @@ import {
  assertRowExists,
  clickRowKebabItem,
  clickSelectRow,
+  clickTableRowItem,
  clickTableToolbarItem,
  searchItem,
 } from "../utils/table";
-import { createGroup, renameGroup, searchGroup } from "./list";
+import { createGroup, editGroup, searchGroup } from "./list";
 import { goToGroupDetails } from "./util";

 test.describe("Group test", () => {
@@ -48,15 +49,18 @@ test.describe("Group test", () => {
  });

  test("Create group test", async ({ page }) => {
-    await createGroup(page, groupName, true);
+    await createGroup(page, groupName, "", true);
    await assertNotificationMessage(page, "Group created");
    await searchGroup(page, groupName);
    await assertRowExists(page, groupName, true);

    // create group from search bar
    const secondGroupName = `group-second-${uuid()}`;
-    await createGroup(page, secondGroupName, false);
+    await createGroup(page, secondGroupName, "some sort of description", false);
    await assertNotificationMessage(page, "Group created");
+    await clickTableRowItem(page, secondGroupName);
+    await expect(page.getByText("some sort of description")).toBeVisible();
+    await page.goBack();

    await searchGroup(page, secondGroupName);
    await assertRowExists(page, secondGroupName, true);
@@ -64,7 +68,7 @@ test.describe("Group test", () => {
  });

  test("Fail to create group with empty name", async ({ page }) => {
-    await createGroup(page, " ", true);
+    await createGroup(page, " ", "", true);
    await assertNotificationMessage(
      page,
      "Could not create group Group name is missing",
@@ -72,8 +76,8 @@ test.describe("Group test", () => {
  });

  test("Fail to create group with duplicated name", async ({ page }) => {
-    await createGroup(page, groupName, true);
-    await createGroup(page, groupName, false);
+    await createGroup(page, groupName, "", true);
+    await createGroup(page, groupName, "", false);
    await assertNotificationMessage(
      page,
      `Could not create group Top level group named '${groupName}' already exists.`,
@@ -130,10 +134,11 @@ test.describe("Search group under current group", () => {
    await assertRowExists(page, predefinedGroups[2], false);
  });

-  test("Rename group", async ({ page }) => {
+  test("Edit group", async ({ page }) => {
    const newGroupName = "new_group_name";
-    await clickRowKebabItem(page, predefinedGroups[3], "Rename");
-    await renameGroup(page, newGroupName);
+    const description = "new description";
+    await clickRowKebabItem(page, predefinedGroups[3], "Edit");
+    await editGroup(page, newGroupName, description);
    await assertNotificationMessage(page, "Group updated");
    await assertRowExists(page, newGroupName);
    await assertRowExists(page, predefinedGroups[3], false);
--- a/js/apps/admin-ui/test/groups/list.ts
+++ b/js/apps/admin-ui/test/groups/list.ts
@@ -3,6 +3,7 @@ import { Page } from "@playwright/test";
 export async function createGroup(
  page: Page,
  name: string,
+  description: string,
  fromEmptyState = false,
 ) {
  if (fromEmptyState) {
@@ -11,6 +12,7 @@ export async function createGroup(
    await page.getByTestId("openCreateGroupModal").click();
  }
  await page.getByTestId("name").fill(name);
+  await page.getByTestId("description").fill(description);
  await page.getByTestId("createGroup").click();
 }

@@ -22,7 +24,8 @@ export async function searchGroup(page: Page, name: string) {
    .click();
 }

-export async function renameGroup(page: Page, name: string) {
+export async function editGroup(page: Page, name: string, description: string) {
  await page.getByTestId("name").fill(name);
+  await page.getByTestId("description").fill(description);
  await page.getByTestId("renameGroup").click();
 }
--- a/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts
+++ b/js/libs/keycloak-admin-client/src/defs/groupRepresentation.ts
@@ -5,6 +5,7 @@
 export default interface GroupRepresentation {
  id?: string;
  name?: string;
+  description?: string;
  path?: string;
  subGroupCount?: number;
  subGroups?: GroupRepresentation[];
--- a/js/libs/keycloak-admin-client/test/groups.spec.ts
+++ b/js/libs/keycloak-admin-client/test/groups.spec.ts
@@ -70,7 +70,7 @@ describe("Groups", () => {
    const groupId = currentGroup.id;
    await kcAdminClient.groups.update(
      { id: groupId! },
-      { name: "another-group-name" },
+      { name: "another-group-name", description: "another-group-description" },
    );

    const group = await kcAdminClient.groups.findOne({
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java
@@ -112,6 +112,18 @@ public class GroupAdapter implements GroupModel {

    }

+    @Override
+    public String getDescription() {
+        if (isUpdated()) return updated.getDescription();
+        return cached.getDescription();
+    }
+
+    @Override
+    public void setDescription(String description) {
+        getDelegateForUpdate();
+        updated.setDescription(description);
+    }
+
    @Override
    public void setSingleAttribute(String name, String value) {
        getDelegateForUpdate();
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java
@@ -39,6 +39,7 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {

    private final String realm;
    private final String name;
+    private final String description;
    private final String parentId;
    private final LazyLoader<GroupModel, MultivaluedHashMap<String, String>> attributes;
    private final LazyLoader<GroupModel, Set<String>> roleMappings;
@@ -49,6 +50,7 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {
        super(revision, group.getId());
        this.realm = realm.getId();
        this.name = group.getName();
+        this.description = group.getDescription();
        this.parentId = group.getParentId();
        this.attributes = new DefaultLazyLoader<>(source -> new MultivaluedHashMap<>(source.getAttributes()), MultivaluedHashMap::new);
        this.roleMappings = new DefaultLazyLoader<>(source -> source.getRoleMappingsStream().map(RoleModel::getId).collect(Collectors.toSet()), Collections::emptySet);
@@ -76,6 +78,10 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {
        return name;
    }

+    public String getDescription() {
+        return description;
+    }
+
    public String getParentId() {
        return parentId;
    }
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java
@@ -89,6 +89,17 @@ public class GroupAdapter implements GroupModel , JpaModel<GroupEntity> {
        fireGroupUpdatedEvent();
    }

+    @Override
+    public String getDescription() {
+        return group.getDescription();
+    }
+
+    @Override
+    public void setDescription(String description) {
+        group.setDescription(description);
+        fireGroupUpdatedEvent();
+    }
+
    @Override
    public GroupModel getParent() {
        String parentId = this.getParentId();
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java
@@ -51,6 +51,10 @@ public class GroupEntity {
    @Column(name = "NAME")
    protected String name;

+    @Nationalized
+    @Column(name = "DESCRIPTION")
+    protected String description;
+
    @Column(name = "PARENT_GROUP")
    private String parentId;

@@ -92,6 +96,14 @@ public class GroupEntity {
        this.name = name;
    }

+    public String getDescription() {
+        return description;
+    }
+
+    public void setDescription(String description) {
+        this.description = description;
+    }
+
    public String getRealm() {
        return realm;
    }
--- a/model/jpa/src/main/resources/META-INF/jpa-changelog-26.3.0.xml
+++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-26.3.0.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!--
+  ~ * Copyright 2024 Red Hat, Inc. and/or its affiliates
+  ~ * and other contributors as indicated by the @author tags.
+  ~ *
+  ~ * Licensed under the Apache License, Version 2.0 (the "License");
+  ~ * you may not use this file except in compliance with the License.
+  ~ * You may obtain a copy of the License at
+  ~ *
+  ~ * http://www.apache.org/licenses/LICENSE-2.0
+  ~ *
+  ~ * Unless required by applicable law or agreed to in writing, software
+  ~ * distributed under the License is distributed on an "AS IS" BASIS,
+  ~ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ * See the License for the specific language governing permissions and
+  ~ * limitations under the License.
+  -->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">
+
+    <changeSet author="keycloak" id="26.3.0-groups-description">
+        <addColumn tableName="KEYCLOAK_GROUP">
+            <column name="DESCRIPTION" type="NVARCHAR(255)" />
+        </addColumn>
+    </changeSet>
+
+</databaseChangeLog>
--- a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml
+++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml
@@ -86,5 +86,6 @@
    <include file="META-INF/jpa-changelog-26.0.0.xml"/>
    <include file="META-INF/jpa-changelog-26.1.0.xml"/>
    <include file="META-INF/jpa-changelog-26.2.0.xml"/>
+    <include file="META-INF/jpa-changelog-26.3.0.xml"/>

 </databaseChangeLog>
--- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
+++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -150,6 +150,7 @@ public class ModelToRepresentation {
        GroupRepresentation rep = new GroupRepresentation();
        rep.setId(group.getId());
        rep.setName(group.getName());
+        rep.setDescription(group.getDescription());
        rep.setPath(buildGroupPath(group));
        rep.setParentId(group.getParentId());
        if (!full) return rep;
--- a/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java
+++ b/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java
@@ -143,6 +143,7 @@ public class KeycloakModelUtilsTest {
        static boolean escapeSlashes = false;

        private String name;
+        private String description;
        private GroupModel parent;

        public GroupAdapterTest(String name, GroupModel parent) {
@@ -165,6 +166,16 @@ public class KeycloakModelUtilsTest {
            this.name = name;
        }

+        @Override
+        public String getDescription() {
+            return description;
+        }
+
+        @Override
+        public void setDescription(String description) {
+            this.description = description;
+        }
+
        @Override
        public void setSingleAttribute(String name, String value) {
        }
--- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java
+++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java
@@ -193,6 +193,10 @@ public interface GroupModel extends RoleMapperModel {

    void setName(String name);

+    String getDescription();
+
+    void setDescription(String description);
+
    /**
     * Set single value of specified attribute. Remove all other existing values
     *
--- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java
@@ -269,6 +269,8 @@ public class GroupResource {
                model.removeAttribute(attr);
            }
        }
+
+        model.setDescription(rep.getDescription());
    }

    @Path("role-mappings")
--- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedGroupStorageProvider.java
+++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/HardcodedGroupStorageProvider.java
@@ -114,6 +114,15 @@ public class HardcodedGroupStorageProvider implements GroupStorageProvider {
            return groupName;
        }

+        @Override
+        public String getDescription() {
+            return null;
+        }
+
+        public void setDescription(String description) {
+            throw new ReadOnlyException("group is read only");
+        }
+
        @Override
        public Stream<RoleModel> getRealmRoleMappingsStream() {
            return Stream.empty();