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

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

@@ -27,7 +27,7 @@ abstract class ExternalUserGroupSyncService<SyncParamsType = any> {
 
   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)
+  authProviderType: string | null; // auth provider type (e.g: ldap, oidc) has to be set before syncExternalUserGroups execution
 
   socketIoService: any;
 
@@ -45,6 +45,7 @@ abstract class ExternalUserGroupSyncService<SyncParamsType = any> {
    * 3. If preserveDeletedLDAPGroups is false、delete all ExternalUserGroups that were not found during tree search
   */
   async syncExternalUserGroups(params?: SyncParamsType): 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');
     this.isExecutingSync = true;
 

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

@@ -38,7 +38,7 @@ class LdapUserGroupSyncService extends ExternalUserGroupSyncService<SyncParamsTy
     const groupDescriptionAttribute: string = configManager.getConfig('crowi', 'external-user-group:ldap:groupDescriptionAttribute');
     const groupBase: string = this.ldapService.getGroupSearchBase();
 
-    await this.ldapService.bind(options?.userBindUsername, options?.userBindPassword);
+    await this.ldapService.initClient(options?.userBindUsername, options?.userBindPassword);
     const groupEntries = await this.ldapService.searchGroupDir();
 
     const getChildGroupDnsFromGroupEntry = (groupEntry: SearchResultEntry) => {

+ 17 - 5
apps/app/src/server/service/ldap.ts

@@ -23,11 +23,16 @@ export interface SearchResultEntry {
 */
 class LdapService {
 
-  client: ldap.Client;
+  client: ldap.Client | null;
 
   searchBase: string;
 
-  constructor() {
+  /**
+   * Initialize LDAP client and bind.
+   * @param {string} userBindUsername Necessary when bind type is user bind
+   * @param {string} userBindPassword Necessary when bind type is user bind
+   */
+  initClient(userBindUsername?: string, userBindPassword?: string): void {
     const serverUrl = configManager?.getConfig('crowi', 'security:passport-ldap:serverUrl');
 
     // parse serverUrl
@@ -44,6 +49,7 @@ class LdapService {
     this.client = ldap.createClient({
       url,
     });
+    this.bind(userBindUsername, userBindPassword);
   }
 
   /**
@@ -53,6 +59,9 @@ class LdapService {
    * @param {string} userBindPassword Necessary when bind type is user bind
    */
   bind(userBindUsername?: string, userBindPassword?: string): Promise<void> {
+    const client = this.client;
+    if (client == null) throw new Error('LDAP client is not initialized');
+
     const isLdapEnabled = configManager?.getConfig('crowi', 'security:passport-ldap:isEnabled');
     if (!isLdapEnabled) {
       const notEnabledMessage = 'LDAP is not enabled';
@@ -72,7 +81,7 @@ class LdapService {
     const fixedBindCredentials = (isUserBind) ? userBindPassword : bindCredentials;
 
     return new Promise<void>((resolve, reject) => {
-      this.client.bind(fixedBindDN, fixedBindCredentials, (err) => {
+      client.bind(fixedBindDN, fixedBindCredentials, (err) => {
         if (err != null) {
           reject(err);
         }
@@ -89,15 +98,18 @@ class LdapService {
    * @returns {SearchEntry[]} Search result. Default scope is set to 'sub'.
    */
   search(filter?: string, base?: string, scope: 'sub' | 'base' | 'one' = 'sub'): Promise<SearchResultEntry[]> {
+    const client = this.client;
+    if (client == null) throw new Error('LDAP client is not initialized');
+
     const searchResults: SearchResultEntry[] = [];
 
     return new Promise((resolve, reject) => {
       // reject on client connection error (occures when not binded or host is not found)
-      this.client.on('error', (err) => {
+      client.on('error', (err) => {
         reject(err);
       });
 
-      this.client.search(base || this.searchBase, {
+      client.search(base || this.searchBase, {
         scope, filter, paged: true, sizeLimit: 200,
       }, (err, res) => {
         if (err != null) {

+ 2 - 1
apps/app/test/integration/service/external-user-group-sync.test.ts

@@ -17,7 +17,8 @@ import { getInstance } from '../setup-crowi';
 class TestExternalUserGroupSyncService extends ExternalUserGroupSyncService {
 
   constructor(socketIoService) {
-    super(ExternalGroupProviderType.ldap, 'ldap', socketIoService);
+    super('ldap', socketIoService);
+    this.authProviderType = ExternalGroupProviderType.ldap;
   }
 
   async generateExternalUserGroupTrees(): Promise<ExternalUserGroupTreeNode[]> {

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

@@ -256,7 +256,7 @@ describe('LdapUserGroupSyncService.generateExternalUserGroupTrees', () => {
         return Promise.reject(new Error('not found'));
       });
 
-      await expect(ldapUserGroupSyncService?.generateExternalUserGroupTrees()).rejects.toThrow('external_user_group.ldap.circular_reference');
+      await expect(ldapUserGroupSyncService?.generateExternalUserGroupTrees()).rejects.toThrow('Circular reference inside LDAP group tree');
     });
   });
 });