Jelajahi Sumber

Prevent ldap group sync execution when sync is already being executed

Futa Arai 2 tahun lalu
induk
melakukan
452718b589

+ 1 - 0
apps/app/public/static/locales/en_US/admin.json

@@ -1059,6 +1059,7 @@
     "invalid_sync_settings": "Invalid sync settings",
     "description_form_detail": "Please note that edited value will be overwritten on next sync if description mapper is set in sync settings",
     "only_description_edit_allowed": "Only description can be edited for external user groups",
+    "sync_being_executed": "Cannot execute sync until current sync process finishes",
     "ldap": {
       "group_sync_settings": "LDAP Group Sync Settings",
       "group_search_base_DN": "Group Search Base DN",

+ 1 - 0
apps/app/public/static/locales/ja_JP/admin.json

@@ -1067,6 +1067,7 @@
     "invalid_sync_settings": "同期設定に誤りがあります",
     "description_form_detail": "同期設定で「説明」の mapper が設定されている場合、編集内容は再同期によって上書きされることに注意してください",
     "only_description_edit_allowed": "外部グループは説明の編集のみが可能です",
+    "sync_being_executed": "現在実行されている同期が終わるまで次の実行ができません",
     "ldap": {
       "group_sync_settings": "LDAP グループ同期設定",
       "group_search_base_DN": "グループ検索ベース DN",

+ 1 - 0
apps/app/public/static/locales/zh_CN/admin.json

@@ -1067,6 +1067,7 @@
     "invalid_sync_settings": "Invalid sync settings",
     "description_form_detail": "Please note that edited value will be overwritten on next sync if description mapper is set in sync settings",
     "only_description_edit_allowed": "Only description can be edited for external user groups",
+    "sync_being_executed": "Cannot execute sync until current sync process finishes",
     "ldap": {
       "group_sync_settings": "LDAP Group Sync Settings",
       "group_search_base_DN": "Group Search Base DN",

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

@@ -17,7 +17,7 @@ import { configManager } from '~/server/service/config-manager';
 import UserGroupService from '~/server/service/user-group';
 import loggerFactory from '~/utils/logger';
 
-import LdapUserGroupSyncService from '../../service/ldap-user-group-sync';
+import { ldapUserGroupSyncService } from '../../service/ldap-user-group-sync';
 
 const logger = loggerFactory('growi:routes:apiv3:external-user-group');
 
@@ -245,9 +245,10 @@ module.exports = (crowi: Crowi): Router => {
   });
 
   router.put('/ldap/sync', loginRequiredStrictly, adminRequired, async(req: AuthorizedRequest, res: ApiV3Response) => {
+    if (ldapUserGroupSyncService?.isExecutingSync) return res.apiv3Err('external_user_group.sync_being_executed', 409);
+
     try {
-      const ldapUserGroupSyncService = new LdapUserGroupSyncService(crowi.passportService, req.user.name, req.body.password);
-      await ldapUserGroupSyncService.syncExternalUserGroups();
+      await ldapUserGroupSyncService?.syncExternalUserGroups({ userBindUsername: req.user.name, userBindPassword: req.body.password });
     }
     catch (err) {
       logger.error(err);

+ 22 - 11
apps/app/src/features/external-user-group/server/service/external-user-group-sync.ts

@@ -18,12 +18,15 @@ import ExternalUserGroupRelation from '../models/external-user-group-relation';
 const TREES_BATCH_SIZE = 10;
 const USERS_BATCH_SIZE = 30;
 
-abstract class ExternalUserGroupSyncService {
+// SyncParamsType: type of params to propagate and use on executing syncExternalUserGroups
+abstract class ExternalUserGroupSyncService<SyncParamsType = any> {
 
   groupProviderType: ExternalGroupProviderType; // name of external service that contains user group info (e.g: ldap, keycloak)
 
   authProviderType: string; // auth provider type (e.g: ldap, oidc)
 
+  isExecutingSync = false;
+
   constructor(groupProviderType: ExternalGroupProviderType, authProviderType: string) {
     this.groupProviderType = groupProviderType;
     this.authProviderType = authProviderType;
@@ -34,8 +37,9 @@ abstract class ExternalUserGroupSyncService {
    * 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(): Promise<void> {
-    const trees = await this.generateExternalUserGroupTrees();
+  async syncExternalUserGroups(params?: SyncParamsType): Promise<void> {
+    if (this.isExecutingSync) throw new Error('External user group sync is already being executed');
+    this.isExecutingSync = true;
 
     const existingExternalUserGroupIds: string[] = [];
 
@@ -49,14 +53,21 @@ abstract class ExternalUserGroupSyncService {
       }
     };
 
-    await batchProcessPromiseAll(trees, TREES_BATCH_SIZE, (root) => {
-      return syncNode(root);
-    });
+    try {
+      const trees = await this.generateExternalUserGroupTrees(params);
+
+      await batchProcessPromiseAll(trees, TREES_BATCH_SIZE, (root) => {
+        return syncNode(root);
+      });
 
-    const preserveDeletedLdapGroups: boolean = configManager?.getConfig('crowi', `external-user-group:${this.groupProviderType}:preserveDeletedGroups`);
-    if (!preserveDeletedLdapGroups) {
-      await ExternalUserGroup.deleteMany({ _id: { $nin: existingExternalUserGroupIds }, groupProviderType: this.groupProviderType });
-      await ExternalUserGroupRelation.removeAllInvalidRelations();
+      const preserveDeletedLdapGroups: boolean = configManager?.getConfig('crowi', `external-user-group:${this.groupProviderType}:preserveDeletedGroups`);
+      if (!preserveDeletedLdapGroups) {
+        await ExternalUserGroup.deleteMany({ _id: { $nin: existingExternalUserGroupIds }, groupProviderType: this.groupProviderType });
+        await ExternalUserGroupRelation.removeAllInvalidRelations();
+      }
+    }
+    finally {
+      this.isExecutingSync = false;
     }
   }
 
@@ -122,7 +133,7 @@ abstract class ExternalUserGroupSyncService {
    * 2. Convert each group tree structure to ExternalUserGroupTreeNode
    * 3. Return the root node of each tree
   */
-  abstract generateExternalUserGroupTrees(): Promise<ExternalUserGroupTreeNode[]>
+  abstract generateExternalUserGroupTrees(params?: SyncParamsType): Promise<ExternalUserGroupTreeNode[]>
 
 }
 

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

@@ -18,20 +18,26 @@ const logger = loggerFactory('growi:service:ldap-user-sync-service');
 const TREES_BATCH_SIZE = 10;
 const USERS_BATCH_SIZE = 30;
 
-class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
+type SyncParamsType = { userBindUsername: string, userBindPassword: string };
+
+class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncParamsType> {
 
   passportService: PassportService;
 
   ldapService: LdapService;
 
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  constructor(passportService, userBindUsername?: string, userBindPassword?: string) {
+  constructor(passportService) {
     super(ExternalGroupProviderType.ldap, 'ldap');
     this.passportService = passportService;
-    this.ldapService = new LdapService(userBindUsername, userBindPassword);
+    this.ldapService = new LdapService();
+  }
+
+  override async syncExternalUserGroups(options?: SyncParamsType): Promise<void> {
+    super.syncExternalUserGroups(options);
   }
 
-  async generateExternalUserGroupTrees(): Promise<ExternalUserGroupTreeNode[]> {
+  async generateExternalUserGroupTrees(options?: SyncParamsType): 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');
@@ -40,7 +46,7 @@ class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
 
     let groupEntries: SearchResultEntry[];
     try {
-      await this.ldapService.bind();
+      await this.ldapService.bind(options?.userBindUsername, options?.userBindPassword);
       groupEntries = await this.ldapService.searchGroupDir();
     }
     catch (e) {
@@ -153,4 +159,8 @@ class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
 
 }
 
-export default LdapUserGroupSyncService;
+// eslint-disable-next-line import/no-mutable-exports
+export let ldapUserGroupSyncService: LdapUserGroupSyncService | undefined; // singleton instance
+export function instanciate(passportService: PassportService): void {
+  ldapUserGroupSyncService = new LdapUserGroupSyncService(passportService);
+}

+ 7 - 0
apps/app/src/server/crowi/index.js

@@ -10,6 +10,7 @@ import next from 'next';
 
 import pkg from '^/package.json';
 
+import { instanciate as instanciateLdapUserGroupSyncService } from '~/features/external-user-group/server/service/ldap-user-group-sync';
 import QuestionnaireService from '~/features/questionnaire/server/service/questionnaire';
 import QuestionnaireCronService from '~/features/questionnaire/server/service/questionnaire-cron';
 import CdnResourcesService from '~/services/cdn-resources-service';
@@ -154,6 +155,7 @@ Crowi.prototype.init = async function() {
     this.setupQuestionnaireService(),
     this.setUpCustomize(), // depends on pluginService
     this.setupExternalAccountService(),
+    this.setupLdapUserGroupSyncService(),
   ]);
 
   // globalNotification depends on slack and mailer
@@ -787,4 +789,9 @@ Crowi.prototype.setupExternalAccountService = function() {
   instanciateExternalAccountService(this.passportService);
 };
 
+// execute after setupPassport
+Crowi.prototype.setupLdapUserGroupSyncService = function() {
+  instanciateLdapUserGroupSyncService(this.passportService);
+};
+
 export default Crowi;

+ 6 - 11
apps/app/src/server/service/ldap.ts

@@ -23,20 +23,13 @@ export interface SearchResultEntry {
 */
 class LdapService {
 
-  username?: string; // Necessary when bind type is user bind
-
-  password?: string; // Necessary when bind type is user bind
-
   client: ldap.Client;
 
   searchBase: string;
 
-  constructor(username?: string, password?: string) {
+  constructor() {
     const serverUrl = configManager?.getConfig('crowi', 'security:passport-ldap:serverUrl');
 
-    this.username = username;
-    this.password = password;
-
     // parse serverUrl
     // see: https://regex101.com/r/0tuYBB/1
     const match = serverUrl.match(/(ldaps?:\/\/[^/]+)\/(.*)?/);
@@ -56,8 +49,10 @@ class LdapService {
   /**
    * Bind to LDAP server.
    * This method is declared independently, so multiple operations can be requested to the LDAP server with a single bind.
+   * @param {string} userBindUsername Necessary when bind type is user bind
+   * @param {string} userBindPassword Necessary when bind type is user bind
    */
-  bind(): Promise<void> {
+  bind(userBindUsername?: string, userBindPassword?: string): Promise<void> {
     const isLdapEnabled = configManager?.getConfig('crowi', 'security:passport-ldap:isEnabled');
     if (!isLdapEnabled) {
       const notEnabledMessage = 'LDAP is not enabled';
@@ -72,9 +67,9 @@ class LdapService {
 
     // user bind
     const fixedBindDN = (isUserBind)
-      ? bindDN.replace(/{{username}}/, this.username)
+      ? bindDN.replace(/{{username}}/, userBindUsername)
       : bindDN;
-    const fixedBindCredentials = (isUserBind) ? this.password : bindCredentials;
+    const fixedBindCredentials = (isUserBind) ? userBindPassword : bindCredentials;
 
     return new Promise<void>((resolve, reject) => {
       this.client.bind(fixedBindDN, fixedBindCredentials, (err) => {