Просмотр исходного кода

add init method to external user group sync services

Futa Arai 2 лет назад
Родитель
Сommit
938d15b5d7

+ 4 - 2
apps/app/src/features/external-user-group/server/routes/apiv3/external-user-group.ts

@@ -315,7 +315,8 @@ module.exports = (crowi: Crowi): Router => {
     }
 
     // Do not await for sync to finish. Result (completed, failed) will be notified to the client by socket-io.
-    crowi.ldapUserGroupSyncService?.syncExternalUserGroups({ userBindUsername: req.user.name, userBindPassword: req.body.password });
+    await crowi.ldapUserGroupSyncService?.init(req.user.name, req.body.password);
+    crowi.ldapUserGroupSyncService?.syncExternalUserGroups();
 
     return res.apiv3({}, 202);
   });
@@ -356,7 +357,8 @@ module.exports = (crowi: Crowi): Router => {
     }
 
     // Do not await for sync to finish. Result (completed, failed) will be notified to the client by socket-io.
-    crowi.keycloakUserGroupSyncService?.syncExternalUserGroups(authProviderType);
+    crowi.keycloakUserGroupSyncService?.init(authProviderType);
+    crowi.keycloakUserGroupSyncService?.syncExternalUserGroups();
 
     return res.apiv3({}, 202);
   });

+ 5 - 6
apps/app/src/features/external-user-group/server/service/external-user-group-sync.ts

@@ -31,12 +31,11 @@ class ExternalUserGroupSyncS2sMessage extends S2sMessage {
 
 }
 
-// SyncParamsType: type of params to propagate and use on executing syncExternalUserGroups
-abstract class ExternalUserGroupSyncService<SyncParamsType = any> implements S2sMessageHandlable {
+abstract class ExternalUserGroupSyncService implements S2sMessageHandlable {
 
   groupProviderType: ExternalGroupProviderType; // name of external service that contains user group info (e.g: ldap, keycloak)
 
-  authProviderType: string | null; // auth provider type (e.g: ldap, oidc) has to be set before syncExternalUserGroups execution
+  authProviderType: string | null; // auth provider type (e.g: ldap, oidc). Has to be set before syncExternalUserGroups execution.
 
   socketIoService: any;
 
@@ -88,7 +87,7 @@ abstract class ExternalUserGroupSyncService<SyncParamsType = any> implements S2s
    * 2. Use createUpdateExternalUserGroup on each node in the tree using DFS
    * 3. If preserveDeletedLDAPGroups is false、delete all ExternalUserGroups that were not found during tree search
   */
-  async syncExternalUserGroups(params?: SyncParamsType): Promise<void> {
+  async syncExternalUserGroups(): Promise<void> {
     if (this.authProviderType == null) throw new Error('auth provider type is not set');
     if (this.isExecutingSync) throw new Error('External user group sync is already being executed');
     await this.switchIsExecutingSync(true);
@@ -99,7 +98,7 @@ abstract class ExternalUserGroupSyncService<SyncParamsType = any> implements S2s
     const socket = this.socketIoService?.getAdminSocket();
 
     try {
-      const trees = await this.generateExternalUserGroupTrees(params);
+      const trees = await this.generateExternalUserGroupTrees();
       const totalCount = trees.map(tree => this.getGroupCountOfTree(tree))
         .reduce((sum, current) => sum + current);
       let count = 0;
@@ -210,7 +209,7 @@ abstract class ExternalUserGroupSyncService<SyncParamsType = any> implements S2s
    * 2. Convert each group tree structure to ExternalUserGroupTreeNode
    * 3. Return the root node of each tree
   */
-  abstract generateExternalUserGroupTrees(params?: SyncParamsType): Promise<ExternalUserGroupTreeNode[]>
+  abstract generateExternalUserGroupTrees(): Promise<ExternalUserGroupTreeNode[]>
 
 }
 

+ 15 - 1
apps/app/src/features/external-user-group/server/service/keycloak-user-group-sync.ts

@@ -2,12 +2,15 @@ import { GroupRepresentation, KeycloakAdminClient, UserRepresentation } from '@s
 
 import { configManager } from '~/server/service/config-manager';
 import { S2sMessagingService } from '~/server/service/s2s-messaging/base';
+import loggerFactory from '~/utils/logger';
 import { batchProcessPromiseAll } from '~/utils/promise';
 
 import { ExternalGroupProviderType, ExternalUserGroupTreeNode, ExternalUserInfo } from '../../interfaces/external-user-group';
 
 import ExternalUserGroupSyncService from './external-user-group-sync';
 
+const logger = loggerFactory('growi:service:keycloak-user-group-sync-service');
+
 // When d = max depth of group trees
 // Max space complexity of generateExternalUserGroupTrees will be:
 // O(TREES_BATCH_SIZE * d)
@@ -21,6 +24,8 @@ export class KeycloakUserGroupSyncService extends ExternalUserGroupSyncService {
 
   groupDescriptionAttribute: string; // attribute to map to group description
 
+  isInitialized = false;
+
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
   constructor(s2sMessagingService: S2sMessagingService, socketIoService) {
     const kcHost = configManager?.getConfig('crowi', 'external-user-group:keycloak:host');
@@ -34,8 +39,17 @@ export class KeycloakUserGroupSyncService extends ExternalUserGroupSyncService {
     this.groupDescriptionAttribute = kcGroupDescriptionAttribute;
   }
 
-  override syncExternalUserGroups(authProviderType: 'oidc' | 'saml'): Promise<void> {
+  init(authProviderType: 'oidc' | 'saml'): void {
     this.authProviderType = authProviderType;
+    this.isInitialized = true;
+  }
+
+  override syncExternalUserGroups(): Promise<void> {
+    if (!this.isInitialized) {
+      const msg = 'Service not initialized';
+      logger.error(msg);
+      throw new Error(msg);
+    }
     return super.syncExternalUserGroups();
   }
 

+ 33 - 20
apps/app/src/features/external-user-group/server/service/ldap-user-group-sync.ts

@@ -1,7 +1,8 @@
 import { configManager } from '~/server/service/config-manager';
-import LdapService, { SearchResultEntry } from '~/server/service/ldap';
+import { ldapService, SearchResultEntry } from '~/server/service/ldap';
 import PassportService from '~/server/service/passport';
 import { S2sMessagingService } from '~/server/service/s2s-messaging/base';
+import loggerFactory from '~/utils/logger';
 import { batchProcessPromiseAll } from '~/utils/promise';
 
 import {
@@ -10,51 +11,63 @@ import {
 
 import ExternalUserGroupSyncService from './external-user-group-sync';
 
+const logger = loggerFactory('growi:service:ldap-user-group-sync-service');
+
 // When d = max depth of group trees
 // Max space complexity of generateExternalUserGroupTrees will be:
 // O(TREES_BATCH_SIZE * d * USERS_BATCH_SIZE)
 const TREES_BATCH_SIZE = 10;
 const USERS_BATCH_SIZE = 30;
 
-type SyncParamsType = { userBindUsername: string, userBindPassword: string };
-
-export class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncParamsType> {
+export class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
 
   passportService: PassportService;
 
-  ldapService: LdapService;
+  isInitialized = false;
 
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
   constructor(passportService: PassportService, s2sMessagingService: S2sMessagingService, socketIoService) {
     super(ExternalGroupProviderType.ldap, s2sMessagingService, socketIoService);
     this.authProviderType = 'ldap';
     this.passportService = passportService;
-    this.ldapService = new LdapService();
   }
 
-  override async generateExternalUserGroupTrees(options?: SyncParamsType): Promise<ExternalUserGroupTreeNode[]> {
+  async init(userBindUsername?: string, userBindPassword?: string): Promise<void> {
+    await ldapService.initClient(userBindUsername, userBindPassword);
+    this.isInitialized = true;
+  }
+
+  override syncExternalUserGroups(): Promise<void> {
+    if (!this.isInitialized) {
+      const msg = 'Service not initialized';
+      logger.error(msg);
+      throw new Error(msg);
+    }
+    return super.syncExternalUserGroups();
+  }
+
+  override async generateExternalUserGroupTrees(): Promise<ExternalUserGroupTreeNode[]> {
     const groupChildGroupAttribute: string = configManager.getConfig('crowi', 'external-user-group:ldap:groupChildGroupAttribute');
     const groupMembershipAttribute: string = configManager.getConfig('crowi', 'external-user-group:ldap:groupMembershipAttribute');
     const groupNameAttribute: string = configManager.getConfig('crowi', 'external-user-group:ldap:groupNameAttribute');
     const groupDescriptionAttribute: string = configManager.getConfig('crowi', 'external-user-group:ldap:groupDescriptionAttribute');
-    const groupBase: string = this.ldapService.getGroupSearchBase();
+    const groupBase: string = ldapService.getGroupSearchBase();
 
-    await this.ldapService.initClient(options?.userBindUsername, options?.userBindPassword);
-    const groupEntries = await this.ldapService.searchGroupDir();
+    const groupEntries = await ldapService.searchGroupDir();
 
     const getChildGroupDnsFromGroupEntry = (groupEntry: SearchResultEntry) => {
       // groupChildGroupAttribute and groupMembershipAttribute may be the same,
       // so filter values of groupChildGroupAttribute to ones that include groupBase
-      return this.ldapService.getArrayValFromSearchResultEntry(groupEntry, groupChildGroupAttribute).filter(attr => attr.includes(groupBase));
+      return ldapService.getArrayValFromSearchResultEntry(groupEntry, groupChildGroupAttribute).filter(attr => attr.includes(groupBase));
     };
     const getUserIdsFromGroupEntry = (groupEntry: SearchResultEntry) => {
       // groupChildGroupAttribute and groupMembershipAttribute may be the same,
       // so filter values of groupMembershipAttribute to ones that does not include groupBase
-      return this.ldapService.getArrayValFromSearchResultEntry(groupEntry, groupMembershipAttribute).filter(attr => !attr.includes(groupBase));
+      return ldapService.getArrayValFromSearchResultEntry(groupEntry, groupMembershipAttribute).filter(attr => !attr.includes(groupBase));
     };
 
     const convert = async(entry: SearchResultEntry, converted: string[]): Promise<ExternalUserGroupTreeNode | null> => {
-      const name = this.ldapService.getStringValFromSearchResultEntry(entry, groupNameAttribute);
+      const name = ldapService.getStringValFromSearchResultEntry(entry, groupNameAttribute);
       if (name == null) return null;
 
       if (converted.includes(entry.objectName)) {
@@ -67,7 +80,7 @@ export class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncP
       const userInfos = (await batchProcessPromiseAll(userIds, USERS_BATCH_SIZE, (id) => {
         return this.getUserInfo(id);
       })).filter((info): info is NonNullable<ExternalUserInfo> => info != null);
-      const description = this.ldapService.getStringValFromSearchResultEntry(entry, groupDescriptionAttribute);
+      const description = ldapService.getStringValFromSearchResultEntry(entry, groupDescriptionAttribute);
       const childGroupDNs = getChildGroupDnsFromGroupEntry(entry);
 
       const childGroupNodesWithNull: (ExternalUserGroupTreeNode | null)[] = [];
@@ -112,10 +125,10 @@ export class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncP
     // get full user info from LDAP server using externalUserInfo (DN or UID)
     const getUserEntries = async() => {
       if (groupMembershipAttributeType === LdapGroupMembershipAttributeType.dn) {
-        return this.ldapService.search(undefined, userId, 'base');
+        return ldapService.search(undefined, userId, 'base');
       }
       if (groupMembershipAttributeType === LdapGroupMembershipAttributeType.uid) {
-        return this.ldapService.search(`(uid=${userId})`, undefined);
+        return ldapService.search(`(uid=${userId})`, undefined);
       }
     };
 
@@ -123,11 +136,11 @@ export class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncP
 
     if (userEntries != null && userEntries.length > 0) {
       const userEntry = userEntries[0];
-      const uid = this.ldapService.getStringValFromSearchResultEntry(userEntry, 'uid');
+      const uid = ldapService.getStringValFromSearchResultEntry(userEntry, 'uid');
       if (uid != null) {
-        const usernameToBeRegistered = attrMapUsername === 'uid' ? uid : this.ldapService.getStringValFromSearchResultEntry(userEntry, attrMapUsername);
-        const nameToBeRegistered = this.ldapService.getStringValFromSearchResultEntry(userEntry, attrMapName);
-        const mailToBeRegistered = this.ldapService.getStringValFromSearchResultEntry(userEntry, attrMapMail);
+        const usernameToBeRegistered = attrMapUsername === 'uid' ? uid : ldapService.getStringValFromSearchResultEntry(userEntry, attrMapUsername);
+        const nameToBeRegistered = ldapService.getStringValFromSearchResultEntry(userEntry, attrMapName);
+        const mailToBeRegistered = ldapService.getStringValFromSearchResultEntry(userEntry, attrMapMail);
 
         return usernameToBeRegistered != null ? {
           id: uid,

+ 2 - 1
apps/app/src/server/service/ldap.ts

@@ -169,4 +169,5 @@ class LdapService {
 
 }
 
-export default LdapService;
+// export the singleton instance
+export const ldapService = new LdapService();

+ 3 - 3
apps/app/test/integration/service/ldap-user-group-sync.test.ts

@@ -2,7 +2,7 @@ import ldap, { Client } from 'ldapjs';
 
 import { LdapUserGroupSyncService } from '../../../src/features/external-user-group/server/service/ldap-user-group-sync';
 import { configManager } from '../../../src/server/service/config-manager';
-import LdapService from '../../../src/server/service/ldap';
+import { ldapService } from '../../../src/server/service/ldap';
 import PassportService from '../../../src/server/service/passport';
 import { getInstance } from '../setup-crowi';
 
@@ -22,8 +22,8 @@ describe('LdapUserGroupSyncService.generateExternalUserGroupTrees', () => {
   };
 
   jest.mock('../../../src/server/service/ldap');
-  const mockBind = jest.spyOn(LdapService.prototype, 'bind');
-  const mockLdapSearch = jest.spyOn(LdapService.prototype, 'search');
+  const mockBind = jest.spyOn(ldapService, 'bind');
+  const mockLdapSearch = jest.spyOn(ldapService, 'search');
   const mockLdapCreateClient = jest.spyOn(ldap, 'createClient');
 
   beforeAll(async() => {