fix(keycloak): fix #591: Cleanup some small code smells in Keycloak p…

…lugin (#1022) fix #591: Cleanup some small code smells
janus-idp · Dec 21, 2023 · 74cb7b1 · 74cb7b1
1 parent bae9269
commit 74cb7b1
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 64 deletions.
diff --git a/plugins/keycloak-backend/src/lib/config.ts b/plugins/keycloak-backend/src/lib/config.ts
@@ -94,6 +94,60 @@ export type KeycloakProviderConfig = {
   groupQuerySize?: number;
 };
 
+const readProviderConfig = (
+  id: string,
+  providerConfigInstance: Config,
+): KeycloakProviderConfig => {
+  const baseUrl = providerConfigInstance.getString('baseUrl');
+  const realm = providerConfigInstance.getOptionalString('realm') ?? 'master';
+  const loginRealm =
+    providerConfigInstance.getOptionalString('loginRealm') ?? 'master';
+  const username = providerConfigInstance.getOptionalString('username');
+  const password = providerConfigInstance.getOptionalString('password');
+  const clientId = providerConfigInstance.getOptionalString('clientId');
+  const clientSecret = providerConfigInstance.getOptionalString('clientSecret');
+  const userQuerySize =
+    providerConfigInstance.getOptionalNumber('userQuerySize');
+  const groupQuerySize =
+    providerConfigInstance.getOptionalNumber('groupQuerySize');
+
+  if (clientId && !clientSecret) {
+    throw new Error(`clientSecret must be provided when clientId is defined.`);
+  }
+
+  if (clientSecret && !clientId) {
+    throw new Error(`clientId must be provided when clientSecret is defined.`);
+  }
+
+  if (username && !password) {
+    throw new Error(`password must be provided when username is defined.`);
+  }
+
+  if (password && !username) {
+    throw new Error(`username must be provided when password is defined.`);
+  }
+
+  const schedule = providerConfigInstance.has('schedule')
+    ? readTaskScheduleDefinitionFromConfig(
+        providerConfigInstance.getConfig('schedule'),
+      )
+    : undefined;
+
+  return {
+    id,
+    baseUrl,
+    loginRealm,
+    realm,
+    username,
+    password,
+    clientId,
+    clientSecret,
+    schedule,
+    userQuerySize,
+    groupQuerySize,
+  };
+};
+
 export const readProviderConfigs = (
   config: Config,
 ): KeycloakProviderConfig[] => {
@@ -103,62 +157,8 @@ export const readProviderConfigs = (
   if (!providersConfig) {
     return [];
   }
-
   return providersConfig.keys().map(id => {
     const providerConfigInstance = providersConfig.getConfig(id);
-
-    const baseUrl = providerConfigInstance.getString('baseUrl');
-    const realm = providerConfigInstance.getOptionalString('realm') ?? 'master';
-    const loginRealm =
-      providerConfigInstance.getOptionalString('loginRealm') ?? 'master';
-    const username = providerConfigInstance.getOptionalString('username');
-    const password = providerConfigInstance.getOptionalString('password');
-    const clientId = providerConfigInstance.getOptionalString('clientId');
-    const clientSecret =
-      providerConfigInstance.getOptionalString('clientSecret');
-    const userQuerySize =
-      providerConfigInstance.getOptionalNumber('userQuerySize');
-    const groupQuerySize =
-      providerConfigInstance.getOptionalNumber('groupQuerySize');
-
-    if (clientId && !clientSecret) {
-      throw new Error(
-        `clientSecret must be provided when clientId is defined.`,
-      );
-    }
-
-    if (clientSecret && !clientId) {
-      throw new Error(
-        `clientId must be provided when clientSecret is defined.`,
-      );
-    }
-
-    if (username && !password) {
-      throw new Error(`password must be provided when username is defined.`);
-    }
-
-    if (password && !username) {
-      throw new Error(`username must be provided when password is defined.`);
-    }
-
-    const schedule = providerConfigInstance.has('schedule')
-      ? readTaskScheduleDefinitionFromConfig(
-          providerConfigInstance.getConfig('schedule'),
-        )
-      : undefined;
-
-    return {
-      id,
-      baseUrl,
-      loginRealm,
-      realm,
-      username,
-      password,
-      clientId,
-      clientSecret,
-      schedule,
-      userQuerySize,
-      groupQuerySize,
-    };
+    return readProviderConfig(id, providerConfigInstance);
   });
 };
diff --git a/plugins/keycloak-backend/src/lib/read.test.ts b/plugins/keycloak-backend/src/lib/read.test.ts
@@ -88,7 +88,8 @@ describe('parseGroup', () => {
     };
     const entity = await parseGroup(groupsFixture[0], 'test', transformer);
 
-    expect(entity!.metadata.name).toEqual('biggroup_test');
+    expect(entity).toBeDefined();
+    expect(entity?.metadata.name).toEqual('biggroup_test');
   });
 });
 
@@ -134,7 +135,8 @@ describe('parseUser', () => {
     };
     const entity = await parseUser(usersFixture[0], 'test', [], transformer);
 
-    expect(entity!.metadata.name).toEqual('jamesdoe_test');
+    expect(entity).toBeDefined();
+    expect(entity?.metadata.name).toEqual('jamesdoe_test');
   });
 });
 
@@ -150,6 +152,7 @@ describe('getEntities', () => {
 
     expect(users).toHaveLength(3);
   });
+
   it('should fetch all users with pagination', async () => {
     const client = new KeycloakAdminClientMock() as unknown as KcAdminClient;
 

diff --git a/plugins/keycloak-backend/src/lib/read.ts b/plugins/keycloak-backend/src/lib/read.ts
@@ -42,7 +42,7 @@ export const parseGroup = async (
   realm: string,
   groupTransformer?: GroupTransformer,
 ): Promise<GroupEntity | undefined> => {
-  const transformer = groupTransformer || noopGroupTransformer;
+  const transformer = groupTransformer ?? noopGroupTransformer;
   const entity: GroupEntity = {
     apiVersion: 'backstage.io/v1beta1',
     kind: 'Group',
@@ -59,7 +59,7 @@ export const parseGroup = async (
         displayName: keycloakGroup.name!,
       },
       // children, parent and members are updated again after all group and user transformers applied.
-      children: keycloakGroup.subGroups?.map(g => g.name!) || [],
+      children: keycloakGroup.subGroups?.map(g => g.name!) ?? [],
       parent: keycloakGroup.parent,
       members: keycloakGroup.members,
     },
@@ -75,7 +75,7 @@ export const parseUser = async (
 
   userTransformer?: UserTransformer,
 ): Promise<UserEntity | undefined> => {
-  const transformer = userTransformer || noopUserTransformer;
+  const transformer = userTransformer ?? noopUserTransformer;
   const entity: UserEntity = {
     apiVersion: 'backstage.io/v1beta1',
     kind: 'User',
@@ -230,11 +230,11 @@ export const readKeycloakRealm = async (
     entity.spec.members =
       g.entity.spec.members?.map(
         m => parsedUsers.find(p => p.username === m)?.entity.metadata.name!,
-      ) || [];
+      ) ?? [];
     entity.spec.children =
       g.entity.spec.children?.map(
         c => parsedGroups.find(p => p.name === c)?.entity.metadata.name!,
-      ) || [];
+      ) ?? [];
     entity.spec.parent = parsedGroups.find(p => p.name === entity.spec.parent)
       ?.entity.metadata.name;
     return entity;

diff --git a/plugins/keycloak-backend/src/providers/KeycloakOrgEntityProvider.ts b/plugins/keycloak-backend/src/providers/KeycloakOrgEntityProvider.ts
@@ -203,12 +203,16 @@ export class KeycloakOrgEntityProvider implements EntityProvider {
         username: provider.username,
         password: provider.password,
       };
-    } else {
+    } else if (provider.clientId && provider.clientSecret) {
       credentials = {
         grantType: 'client_credentials',
-        clientId: provider.clientId!,
+        clientId: provider.clientId,
         clientSecret: provider.clientSecret,
       };
+    } else {
+      throw new Error(
+        `username and password or clientId and clientSecret must be provided.`,
+      );
     }
 
     await kcAdminClient.auth(credentials);