chore(release): backport keycloak fixes but without upgrade keycloak …

…client (#2429) * chore(release): backport keycloak-backend fixes for RHDH 1.3.1 Signed-off-by: Oleksandr Andriienko <[email protected]> * chore(release): add changeset and bump new version Signed-off-by: Oleksandr Andriienko <[email protected]> * chore(release): fix remaining tests Signed-off-by: Oleksandr Andriienko <[email protected]> --------- Signed-off-by: Oleksandr Andriienko <[email protected]>
janus-idp · Oct 23, 2024 · 1650b05 · 1650b05
1 parent e02e4d5
commit 1650b05
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 43 deletions.
diff --git a/.changeset/early-trainers-dance.md b/.changeset/early-trainers-dance.md
@@ -0,0 +1,7 @@
+---
+"@janus-idp/backstage-plugin-keycloak-backend": major
+---
+
+Provide keycloak-backend fixes:
+- avoid undefined values for keycloak group members
+- retrieve full list group members using pagination
diff --git a/plugins/keycloak-backend/__fixtures__/data.ts b/plugins/keycloak-backend/__fixtures__/data.ts
@@ -90,9 +90,5 @@ export const users = [
   },
 ];
 
-export const groupMembers = [
-  ['jamesdoe'],
-  [],
-  [],
-  ['jamesdoe', 'joedoe', 'johndoe'],
-];
+export const groupMembers1 = ['jamesdoe'];
+export const groupMembers2 = ['jamesdoe', 'joedoe', 'johndoe'];
diff --git a/plugins/keycloak-backend/__fixtures__/helpers.ts b/plugins/keycloak-backend/__fixtures__/helpers.ts
@@ -2,7 +2,7 @@ import { getVoidLogger } from '@backstage/backend-common';
 import { TaskInvocationDefinition, TaskRunner } from '@backstage/backend-tasks';
 import { EntityProviderConnection } from '@backstage/plugin-catalog-node';
 
-import { groupMembers, groups, users } from './data';
+import { groupMembers1, groupMembers2, groups, users } from './data';
 
 export const BASIC_VALID_CONFIG = {
   catalog: {
@@ -55,10 +55,25 @@ export class KeycloakAdminClientMock {
     count: jest.fn().mockResolvedValue(groups.length),
     listMembers: jest
       .fn()
-      .mockResolvedValueOnce(groupMembers[0].map(username => ({ username })))
-      .mockResolvedValueOnce(groupMembers[1].map(username => ({ username })))
-      .mockResolvedValueOnce(groupMembers[2].map(username => ({ username })))
-      .mockResolvedValueOnce(groupMembers[3].map(username => ({ username }))),
+      .mockImplementation(
+        async (payload?: {
+          id: string;
+          _max?: number;
+          _realm?: string;
+          first?: number;
+        }) => {
+          const { id, first } = payload || {};
+          if (id === '9cf51b5d-e066-4ed8-940c-dc6da77f81a5' && first === 0) {
+            // biggroup - first members page
+            return groupMembers1.map(username => ({ username }));
+          }
+          if (id === 'bb10231b-2939-4b1a-b8bb-9249ed7b76f7' && first === 0) {
+            // testgroup - first members page
+            return groupMembers2.map(username => ({ username }));
+          }
+          return [];
+        },
+      ),
   };
 
   auth = authMock;
@@ -74,6 +89,14 @@ class FakeAbortSignal implements AbortSignal {
   dispatchEvent() {
     return true;
   }
+  any(signals: Iterable<AbortSignal>): AbortSignal {
+    for (const signal of signals) {
+      if (signal.aborted) {
+        return signal;
+      }
+    }
+    throw new Error('No abort signal found');
+  }
 }
 
 export class ManualTaskRunner implements TaskRunner {

diff --git a/plugins/keycloak-backend/package.json b/plugins/keycloak-backend/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@janus-idp/backstage-plugin-keycloak-backend",
-  "version": "1.13.3",
+  "version": "1.13.4",
   "description": "A Backend backend plugin for Keycloak",
   "main": "src/index.ts",
   "types": "src/index.ts",

diff --git a/plugins/keycloak-backend/src/lib/read.test.ts b/plugins/keycloak-backend/src/lib/read.test.ts
@@ -1,3 +1,5 @@
+import { mockServices } from '@backstage/backend-test-utils';
+
 import KcAdminClient from '@keycloak/keycloak-admin-client';
 
 import {
@@ -21,10 +23,12 @@ const config: KeycloakProviderConfig = {
   baseUrl: 'http://mock-url',
 };
 
+const logger = mockServices.logger.mock();
+
 describe('readKeycloakRealm', () => {
   it('should return the correct number of users and groups', async () => {
     const client = new KeycloakAdminClientMock() as unknown as KcAdminClient;
-    const { users, groups } = await readKeycloakRealm(client, config);
+    const { users, groups } = await readKeycloakRealm(client, config, logger);
 
     expect(users).toHaveLength(3);
     expect(groups).toHaveLength(4);
@@ -41,7 +45,7 @@ describe('readKeycloakRealm', () => {
     };
 
     const client = new KeycloakAdminClientMock() as unknown as KcAdminClient;
-    const { users, groups } = await readKeycloakRealm(client, config, {
+    const { users, groups } = await readKeycloakRealm(client, config, logger, {
       userTransformer,
       groupTransformer,
     });
@@ -144,11 +148,15 @@ describe('getEntities', () => {
   it('should fetch all users', async () => {
     const client = new KeycloakAdminClientMock() as unknown as KcAdminClient;
 
-    const users = await getEntities(client.users, {
-      id: '',
-      baseUrl: '',
-      realm: '',
-    });
+    const users = await getEntities(
+      client.users,
+      {
+        id: '',
+        baseUrl: '',
+        realm: '',
+      },
+      logger,
+    );
 
     expect(users).toHaveLength(3);
   });
@@ -163,6 +171,7 @@ describe('getEntities', () => {
         baseUrl: '',
         realm: '',
       },
+      logger,
       1,
     );
 

diff --git a/plugins/keycloak-backend/src/lib/read.ts b/plugins/keycloak-backend/src/lib/read.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { LoggerService } from '@backstage/backend-plugin-api';
 import { GroupEntity, UserEntity } from '@backstage/catalog-model';
 
 import KeycloakAdminClient from '@keycloak/keycloak-admin-client';
@@ -120,6 +121,7 @@ export function* traverseGroups(
 export async function getEntities<T extends Users | Groups>(
   entities: T,
   config: KeycloakProviderConfig,
+  logger: LoggerService,
   entityQuerySize: number = KEYCLOAK_ENTITY_QUERY_SIZE,
 ): Promise<Awaited<ReturnType<T['find']>>> {
   const rawEntityCount = await entities.count({ realm: config.realm });
@@ -132,11 +134,15 @@ export async function getEntities<T extends Users | Groups>(
   const entityPromises = Array.from(
     { length: pageCount },
     (_, i) =>
-      entities.find({
-        realm: config.realm,
-        max: entityQuerySize,
-        first: i * entityQuerySize,
-      }) as ReturnType<T['find']>,
+      entities
+        .find({
+          realm: config.realm,
+          max: entityQuerySize,
+          first: i * entityQuerySize,
+        })
+        .catch(err =>
+          logger.warn('Failed to retieve Keycloak entities.', err),
+        ) as ReturnType<T['find']>,
   );
 
   const entityResults = (await Promise.all(entityPromises)).flat() as Awaited<
@@ -146,9 +152,43 @@ export async function getEntities<T extends Users | Groups>(
   return entityResults;
 }
 
+async function getAllGroupMembers<T extends Groups>(
+  groups: T,
+  groupId: string,
+  config: KeycloakProviderConfig,
+  options?: { userQuerySize?: number },
+): Promise<string[]> {
+  const querySize = options?.userQuerySize || 100;
+
+  let allMembers: string[] = [];
+  let page = 0;
+  let totalMembers = 0;
+
+  do {
+    const members = await groups.listMembers({
+      id: groupId,
+      max: querySize,
+      realm: config.realm,
+      first: page * querySize,
+    });
+
+    if (members.length > 0) {
+      allMembers = allMembers.concat(members.map(m => m.username!));
+      totalMembers = members.length; // Get the number of members retrieved
+    } else {
+      totalMembers = 0; // No members retrieved
+    }
+
+    page++;
+  } while (totalMembers > 0);
+
+  return allMembers;
+}
+
 export const readKeycloakRealm = async (
   client: KeycloakAdminClient,
   config: KeycloakProviderConfig,
+  logger: LoggerService,
   options?: {
     userQuerySize?: number;
     groupQuerySize?: number;
@@ -162,12 +202,14 @@ export const readKeycloakRealm = async (
   const kUsers = await getEntities(
     client.users,
     config,
+    logger,
     options?.userQuerySize,
   );
 
   const rawKGroups = await getEntities(
     client.groups,
     config,
+    logger,
     options?.groupQuerySize,
   );
   const flatKGroups = rawKGroups.reduce((acc, g) => {
@@ -176,13 +218,12 @@ export const readKeycloakRealm = async (
   }, [] as GroupRepresentationWithParent[]);
   const kGroups = await Promise.all(
     flatKGroups.map(async g => {
-      g.members = (
-        await client.groups.listMembers({
-          id: g.id!,
-          max: options?.userQuerySize,
-          realm: config.realm,
-        })
-      ).map(m => m.username!);
+      g.members = await getAllGroupMembers(
+        client.groups as Groups,
+        g.id!,
+        config,
+        options,
+      );
       return g;
     }),
   );
@@ -228,13 +269,17 @@ export const readKeycloakRealm = async (
   const groups = parsedGroups.map(g => {
     const entity = g.entity;
     entity.spec.members =
-      g.entity.spec.members?.map(
-        m => parsedUsers.find(p => p.username === m)?.entity.metadata.name!,
-      ) ?? [];
+      g.entity.spec.members?.flatMap(m => {
+        const name = parsedUsers.find(p => p.username === m)?.entity.metadata
+          .name;
+        return name ? [name] : [];
+      }) ?? [];
     entity.spec.children =
-      g.entity.spec.children?.map(
-        c => parsedGroups.find(p => p.name === c)?.entity.metadata.name!,
-      ) ?? [];
+      g.entity.spec.children?.flatMap(c => {
+        const child = parsedGroups.find(p => p.name === c)?.entity.metadata
+          .name;
+        return child ? [child] : [];
+      }) ?? [];
     entity.spec.parent = parsedGroups.find(
       p => p.name === entity.spec.parent,
     )?.entity.metadata.name;

diff --git a/plugins/keycloak-backend/src/providers/KeycloakOrgEntityProvider.ts b/plugins/keycloak-backend/src/providers/KeycloakOrgEntityProvider.ts
@@ -217,12 +217,17 @@ export class KeycloakOrgEntityProvider implements EntityProvider {
 
     await kcAdminClient.auth(credentials);
 
-    const { users, groups } = await readKeycloakRealm(kcAdminClient, provider, {
-      userQuerySize: provider.userQuerySize,
-      groupQuerySize: provider.groupQuerySize,
-      userTransformer: this.options.userTransformer,
-      groupTransformer: this.options.groupTransformer,
-    });
+    const { users, groups } = await readKeycloakRealm(
+      kcAdminClient,
+      provider,
+      logger,
+      {
+        userQuerySize: provider.userQuerySize,
+        groupQuerySize: provider.groupQuerySize,
+        userTransformer: this.options.userTransformer,
+        groupTransformer: this.options.groupTransformer,
+      },
+    );
 
     const { markCommitComplete } = markReadComplete({ users, groups });