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

Improve check admin method

https://youtrack.weseek.co.jp/issue/GW-7958
- Commonize check admin method to checkAuthStrategyHasAdmin
- Remove configNamespace param from checkAndAddActiveAuthMethodWithAdmin since it use the same namespace (crowi)
- Move configKey to checkAndAddActiveAuthMethodWithAdmin and remove from parameter
- Call checkAuthStrategyHasAdmin directly from checkAndAddActiveAuthMethodWithAdmin and remove from parameter
Mudana-Grune 2 лет назад
Родитель
Сommit
13ee2c0298
1 измененных файлов с 30 добавлено и 27 удалено
  1. 30 27
      apps/app/src/server/routes/apiv3/security-setting.js

+ 30 - 27
apps/app/src/server/routes/apiv3/security-setting.js

@@ -505,49 +505,52 @@ module.exports = (crowi) => {
       const activeAuthMethodsWithAdmin = [];
       const activeAuthMethodsWithAdmin = [];
 
 
       // Check the local auth method
       // Check the local auth method
-      await checkAndAddActiveAuthMethodWithAdmin('local', 'crowi', 'security:passport-local:isEnabled', checkLocalStrategyHasAdmin, activeAuthMethodsWithAdmin);
+      await checkAndAddActiveAuthMethodWithAdmin('local', activeAuthMethodsWithAdmin);
 
 
       // Check external auth methods
       // Check external auth methods
       const externalAuthTypes = Object.values(GrowiExternalAuthProviderType);
       const externalAuthTypes = Object.values(GrowiExternalAuthProviderType);
-      await Promise.all(externalAuthTypes.map(async(strategy) => {
-        const configKey = `security:passport-${strategy}:isEnabled`;
-        await checkAndAddActiveAuthMethodWithAdmin(strategy, 'crowi', configKey, checkExternalStrategyHasAdmin, activeAuthMethodsWithAdmin);
+      await Promise.all(externalAuthTypes.map(async (strategy) => {
+        await checkAndAddActiveAuthMethodWithAdmin(strategy, activeAuthMethodsWithAdmin);
       }));
       }));
 
 
       return activeAuthMethodsWithAdmin;
       return activeAuthMethodsWithAdmin;
     }
     }
 
 
     // Check and add an authentication method with admin to the list
     // Check and add an authentication method with admin to the list
-    async function checkAndAddActiveAuthMethodWithAdmin(strategy, configNamespace, configKey, checkHasAdminFunction, activeAuthMethodsWithAdmin) {
-      const isEnabled = configManager.getConfig(configNamespace, configKey);
-      const hasAdmin = await checkHasAdminFunction(strategy);
+    async function checkAndAddActiveAuthMethodWithAdmin(strategy, activeAuthMethodsWithAdmin) {
+      const configKey = `security:passport-${strategy}:isEnabled`;
+      const isEnabled = configManager.getConfig('crowi', configKey);
+      const hasAdmin = await checkAuthStrategyHasAdmin(strategy);
       if (isEnabled && hasAdmin && setupStrategies.includes(strategy)) {
       if (isEnabled && hasAdmin && setupStrategies.includes(strategy)) {
         activeAuthMethodsWithAdmin.push(strategy);
         activeAuthMethodsWithAdmin.push(strategy);
       }
       }
     }
     }
 
 
-    // Check if local accounts have admins
-    async function checkLocalStrategyHasAdmin() {
-      // Get all local admin accounts and filter local admins that are not in external accounts
-      const adminAccounts = await User.aggregate([
-        { $match: { admin: true } },
-        {
-          $lookup: {
-            from: 'externalaccounts',
-            localField: '_id',
-            foreignField: 'user',
-            as: 'externalAccounts',
+    // Check auth strategy has admin user
+    async function checkAuthStrategyHasAdmin(strategy) {
+      // Check if local accounts have admins
+      if (strategy === 'local') {
+        // Get all local admin accounts and filter local admins that are not in external accounts
+        const localAdmins = await User.aggregate([
+          { $match: { admin: true } },
+          {
+            $lookup: {
+              from: 'externalaccounts',
+              localField: '_id',
+              foreignField: 'user',
+              as: 'externalAccounts',
+            },
           },
           },
-        },
-        { $match: { externalAccounts: [] } },
-      ]);
-      return adminAccounts.length > 0;
-    }
+          { $match: { externalAccounts: [] } },
+        ]).exec();
+        return localAdmins.length > 0;
+      }
+
+      // Check if external accounts have admins
+      const externalAccounts = await ExternalAccount.find({ providerType: strategy }).populate('user').exec();
+      const externalAccountHasAdmin = externalAccounts.some(externalAccount => externalAccount.user && externalAccount.user.admin);
 
 
-    // Check if external accounts have admins
-    async function checkExternalStrategyHasAdmin(providerType) {
-      const externalAccounts = await ExternalAccount.find({ providerType }).populate('user').exec();
-      return externalAccounts.some(externalAccount => externalAccount.user && externalAccount.user.admin);
+      return externalAccountHasAdmin;
     }
     }