Merge pull request #1055 from weseek/imprv/abolish-old-config-api-plugin

Imprv/abolish old config api plugin

src/server/crowi/index.js

@@ -78,8 +78,13 @@ Crowi.prototype.init = async function() {
   await this.setupModels();
   await this.setupSessionConfig();
   await this.setupConfigManager();
-  await this.setUpApp();
-  await this.setUpXss();
+
+  // customizeService depends on AppService and XssService
+  // passportService depends on appService
+  await Promise.all([
+    this.setUpApp(),
+    this.setUpXss(),
+  ]);
 
   await Promise.all([
     this.scanRuntimeVersions(),
@@ -91,7 +96,7 @@ Crowi.prototype.init = async function() {
     this.setUpGlobalNotification(),
     this.setUpSlacklNotification(),
     this.setUpAcl(),
-    this.setUpCustomize(), // depends on AppService and XssService
+    this.setUpCustomize(),
     this.setUpRestQiitaAPI(),
   ]);
 };
@@ -383,8 +388,7 @@ Crowi.prototype.buildServer = function() {
   require('./express-init')(this, express);
 
   // import plugins
-  const Config = this.model('Config');
-  const isEnabledPlugins = Config.isEnabledPlugins(this.config);
+  const isEnabledPlugins = this.configManager.getConfig('crowi', 'plugin:isEnabledPlugins');
   if (isEnabledPlugins) {
     debug('Plugins are enabled');
     const PluginService = require('../plugins/plugin.service');

src/server/models/config.js

@@ -341,11 +341,6 @@ module.exports = function(crowi) {
     return SECURITY_RESTRICT_GUEST_MODE_READONLY === restrictGuestMode;
   };
 
-  configSchema.statics.isEnabledPlugins = function(config) {
-    const key = 'plugin:isEnabledPlugins';
-    return getValueForCrowiNS(config, key);
-  };
-
   configSchema.statics.isEnabledLinebreaks = function(config) {
     const key = 'markdown:isEnabledLinebreaks';
     return getValueForMarkdownNS(config, key);

src/server/routes/admin.js

@@ -861,7 +861,7 @@ module.exports = function(crowi, app) {
   };
 
   actions.api = {};
-  actions.api.appSetting = function(req, res) {
+  actions.api.appSetting = async function(req, res) {
     const form = req.form.settingForm;
 
     if (req.form.isValid) {
@@ -880,7 +880,8 @@ module.exports = function(crowi, app) {
         });
       }
       else {
-        return saveSetting(req, res, form);
+        await configManager.updateConfigsInTheSameNamespace('crowi', form);
+        return res.json({ status: true });
       }
     }
     else {
@@ -898,7 +899,7 @@ module.exports = function(crowi, app) {
     debug('form content', form);
 
     try {
-      await crowi.configManager.updateConfigsInTheSameNamespace('crowi', form);
+      await configManager.updateConfigsInTheSameNamespace('crowi', form);
       return res.json({ status: true });
     }
     catch (err) {
@@ -930,7 +931,7 @@ module.exports = function(crowi, app) {
     }
 
     try {
-      await crowi.configManager.updateConfigsInTheSameNamespace('crowi', form);
+      await configManager.updateConfigsInTheSameNamespace('crowi', form);
       return res.json({ status: true });
     }
     catch (err) {
@@ -947,7 +948,7 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    return saveSettingAsync(form)
+    return configManager.updateConfigsInTheSameNamespace('crowi', form)
       .then(() => {
         // reset strategy
         crowi.passportService.resetLdapStrategy();
@@ -972,12 +973,12 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    await crowi.configManager.updateConfigsInTheSameNamespace('crowi', form);
+    await configManager.updateConfigsInTheSameNamespace('crowi', form);
 
     // reset strategy
     await crowi.passportService.resetSamlStrategy();
     // setup strategy
-    if (crowi.configManager.getConfig('crowi', 'security:passport-saml:isEnabled')) {
+    if (configManager.getConfig('crowi', 'security:passport-saml:isEnabled')) {
       try {
         await crowi.passportService.setupSamlStrategy(true);
       }
@@ -999,7 +1000,7 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    await saveSettingAsync(form);
+    await configManager.updateConfigsInTheSameNamespace('crowi', form);
 
     // reset strategy
     await crowi.passportService.resetGoogleStrategy();
@@ -1026,7 +1027,7 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    await saveSettingAsync(form);
+    await configManager.updateConfigsInTheSameNamespace('crowi', form);
 
     // reset strategy
     await crowi.passportService.resetGitHubStrategy();
@@ -1053,7 +1054,7 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    await saveSettingAsync(form);
+    await configManager.updateConfigsInTheSameNamespace('crowi', form);
 
     // reset strategy
     await crowi.passportService.resetTwitterStrategy();
@@ -1080,7 +1081,7 @@ module.exports = function(crowi, app) {
     }
 
     debug('form content', form);
-    await saveSettingAsync(form);
+    await configManager.updateConfigsInTheSameNamespace('crowi', form);
 
     // reset strategy
     await crowi.passportService.resetOidcStrategy();
@@ -1375,25 +1376,6 @@ module.exports = function(crowi, app) {
     });
   }
 
-  /**
-   * save settings, update config cache ONLY. (this method don't response json)
-   *
-   * @param {any} form
-   * @returns
-   */
-  function saveSettingAsync(form) {
-    return new Promise((resolve, reject) => {
-      Config.updateNamespaceByArray('crowi', form, (err, config) => {
-        if (err) {
-          return reject(err);
-        }
-
-        Config.updateConfigCache('crowi', config);
-        return resolve();
-      });
-    });
-  }
-
   function validateMailSetting(req, form, callback) {
     const mailer = crowi.mailer;
     const option = {
@@ -1430,7 +1412,7 @@ module.exports = function(crowi, app) {
   function validateSamlSettingForm(form, t) {
     for (const key of crowi.passportService.mandatoryConfigKeysForSaml) {
       const formValue = form.settingForm[key];
-      if (crowi.configManager.getConfigFromEnvVars('crowi', key) === null && formValue === '') {
+      if (configManager.getConfigFromEnvVars('crowi', key) === null && formValue === '') {
         const formItemName = t(`security_setting.form_item_name.${key}`);
         form.errors.push(t('form_validation.required', formItemName));
       }

src/server/service/config-manager.js

@@ -32,7 +32,7 @@ class ConfigManager {
     debug('ConfigManager#loadConfigs', this.configObject);
 
     // cache all config keys
-    this.configKeys = this.getAllConfigKeys();
+    this.reloadConfigKeys();
   }
 
   /**
@@ -82,7 +82,7 @@ class ConfigManager {
   /**
    * generate an array of config keys from this.configObject
    */
-  getAllConfigKeys() {
+  getConfigKeys() {
     // type: fromDB, fromEnvVars
     const types = Object.keys(this.configObject);
     let namespaces = [];
@@ -112,6 +112,10 @@ class ConfigManager {
     return keys;
   }
 
+  reloadConfigKeys() {
+    this.configKeys = this.getConfigKeys();
+  }
+
   /**
    * get a config specified by namespace & key from configs loaded from the database
    *
@@ -178,6 +182,7 @@ class ConfigManager {
     await this.configModel.bulkWrite(queries);
 
     await this.loadConfigs();
+    this.reloadConfigKeys();
   }
 
   /**
@@ -217,7 +222,7 @@ class ConfigManager {
 
     // only exists env vars
     if (!this.configExistsInDB(namespace, key) && this.configExistsInEnvVars(namespace, key)) {
-      debug(`${namespace}.${key} only exists in db`);
+      debug(`${namespace}.${key} only exists in env vars`);
       return this.configObject.fromEnvVars[namespace][key];
     }
 

src/server/util/swigFunctions.js

@@ -95,13 +95,6 @@ module.exports = function(crowi, app, req, locals) {
     return cdnResourcesService.getHighlightJsStyleTag(styleName);
   };
 
-  /**
-   * return true if enabled
-   */
-  locals.isEnabledPassport = function() {
-    return configManager.getConfig('crowi', 'security:isEnabledPassport');
-  };
-
   /**
    * return true if local strategy has been setup successfully
    *  used whether restarting the server needed
@@ -132,14 +125,10 @@ module.exports = function(crowi, app, req, locals) {
     );
   };
 
-  locals.passportSamlLoginEnabled = function() {
-    return locals.isEnabledPassport() && locals.getConfig('crowi', 'security:passport-saml:isEnabled');
-  };
-
   locals.getSamlMissingMandatoryConfigKeys = function() {
     // return an empty array if Passport is not enabled
     // because crowi.passportService is null.
-    if (!locals.isEnabledPassport()) {
+    if (!configManager.getConfig('crowi', 'security:isEnabledPassport')) {
       return [];
     }
 
@@ -149,33 +138,16 @@ module.exports = function(crowi, app, req, locals) {
   locals.googleLoginEnabled = function() {
     // return false if Passport is enabled
     // because official crowi mechanism is not used.
-    if (locals.isEnabledPassport()) {
+    if (configManager.getConfig('crowi', 'security:isEnabledPassport')) {
       return false;
     }
 
-    const configManager = crowi.configManager;
     return (
       configManager.getConfig('crowi', 'google:clientId')
       && configManager.getConfig('crowi', 'google:clientSecret')
     );
   };
 
-  locals.passportGoogleLoginEnabled = function() {
-    return locals.isEnabledPassport() && configManager.getConfig('crowi', 'security:passport-google:isEnabled');
-  };
-
-  locals.passportGitHubLoginEnabled = function() {
-    return locals.isEnabledPassport() && configManager.getConfig('crowi', 'security:passport-github:isEnabled');
-  };
-
-  locals.passportTwitterLoginEnabled = function() {
-    return locals.isEnabledPassport() && configManager.getConfig('crowi', 'security:passport-twitter:isEnabled');
-  };
-
-  locals.passportOidcLoginEnabled = function() {
-    return locals.isEnabledPassport() && configManager.getConfig('crowi', 'security:passport-oidc:isEnabled');
-  };
-
   locals.searchConfigured = function() {
     if (crowi.getSearcher()) {
       return true;
@@ -187,11 +159,6 @@ module.exports = function(crowi, app, req, locals) {
     return process.env.HACKMD_URI != null;
   };
 
-  locals.isEnabledPlugins = function() {
-    const config = crowi.getConfig();
-    return Config.isEnabledPlugins(config);
-  };
-
   locals.isEnabledLinebreaks = function() {
     const config = crowi.getConfig();
     return Config.isEnabledLinebreaks(config);

src/server/views/admin/app.html

@@ -307,18 +307,18 @@
           <div class="col-xs-6">
 
             <div class="btn-group btn-toggle" data-toggle="buttons">
-              <label class="btn btn-default btn-rounded btn-outline {% if settingForm['plugin:isEnabledPlugins'] %}active{% endif %}" data-active-class="primary">
+              <label class="btn btn-default btn-rounded btn-outline {% if getConfig('crowi', 'plugin:isEnabledPlugins') %}active{% endif %}" data-active-class="primary">
                 <input name="settingForm[plugin:isEnabledPlugins]"
                        value="true"
                        type="radio"
-                       {% if true === settingForm['plugin:isEnabledPlugins'] %}checked{% endif %}>
+                       {% if true === getConfig('crowi', 'plugin:isEnabledPlugins') %}checked{% endif %}>
                 ON
               </label>
-              <label class="btn btn-default btn-rounded btn-outline {% if !settingForm['plugin:isEnabledPlugins'] %}active{% endif %}" data-active-class="default">
+              <label class="btn btn-default btn-rounded btn-outline {% if !getConfig('crowi', 'plugin:isEnabledPlugins') %}active{% endif %}" data-active-class="default">
                 <input name="settingForm[plugin:isEnabledPlugins]"
                        value="false"
                        type="radio"
-                       {% if !settingForm['plugin:isEnabledPlugins'] %}checked{% endif %}>
+                       {% if !getConfig('crowi', 'plugin:isEnabledPlugins') %}checked{% endif %}>
                 OFF
               </label>
             </div>

src/server/views/admin/security.html

@@ -217,7 +217,7 @@
 
         <legend>{{ t('security_setting.Authentication mechanism settings') }}</legend>
 
-        {% set isOfficialConfigurationVisible = !isEnabledPassport() %}
+        {% set isOfficialConfigurationVisible = !getConfig('crowi', 'security:isEnabledPassport') %}
         <div class="official-crowi-auth-settings" {% if !isOfficialConfigurationVisible %}style="display: none;"{% endif %}>
           {% set isRestartingServerNeeded = isPassportLocalStrategySetup() %}
           <p class="alert alert-warning"

src/server/views/layout/layout.html

@@ -27,9 +27,8 @@
 
   <script src="{{ webpack_asset('js/vendors.js') }}" defer></script>
   <script src="{{ webpack_asset('js/commons.js') }}" defer></script>
-
-  {% if isEnabledPlugins() %}
-    <script src="{{ webpack_asset('js/plugin.js') }}" defer></script>
+  {% if getConfig('crowi', 'plugin:isEnabledPlugins') %}
+  <script src="{{ webpack_asset('js/plugin.js') }}" defer></script>
   {% endif %}
   {% block html_head_loading_legacy %}
     <script src="{{ webpack_asset('js/legacy.js') }}" defer></script>
@@ -66,7 +65,7 @@
       {% if !layoutType() || 'crowi' === layoutType() %}crowi{% elseif !layoutType() || 'kibela' === layoutType() %}kibela{% else %}growi{% endif %}"
   data-me="{{ user._id.toString() }}"
   data-is-admin="{{ user.admin }}"
-  data-plugin-enabled="{{ isEnabledPlugins() }}"
+  data-plugin-enabled="{{ getConfig('crowi', 'plugin:isEnabledPlugins') }}"
   {% block html_base_attr %}{% endblock %}
   data-csrftoken="{{ csrf() }}"
   data-current-username="{% if user %}{{ user.username }}{% endif %}"

src/server/views/login.html

@@ -144,12 +144,21 @@
           </form>
         </div>
         {% endif %}
-        {% if passportGoogleLoginEnabled() || passportGitHubLoginEnabled() || passportFacebookLoginEnabled() || passportTwitterLoginEnabled() || passportOidcLoginEnabled() || passportSamlLoginEnabled() %}
+        {% if (
+          getConfig('crowi', 'security:isEnabledPassport') && (
+            getConfig('crowi', 'security:passport-google:isEnabled') ||
+            getConfig('crowi', 'security:passport-github:isEnabled') ||
+            getConfig('crowi', 'security:passport-facebook:isEnabled') ||
+            getConfig('crowi', 'security:passport-twitter:isEnabled')||
+            getConfig('crowi', 'security:passport-oidc:isEnabled') ||
+            getConfig('crowi', 'security:passport-saml:isEnabled')
+          )
+        ) %}
         <hr class="mb-1">
         <div class="collapse collapse-oauth collapse-anchor">
           <div class="spacer"></div>
           <div class="d-flex flex-row justify-content-between flex-wrap">
-            {% if passportGoogleLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-google:isEnabled') %}
             <form role="form" action="/passport/google" class="d-inline-flex flex-column">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-flex" id="google">
                 <span class="btn-label"><i class="fa fa-google"></i></span>
@@ -158,7 +167,7 @@
               <div class="small text-right">by Google Account</div>
             </form>
             {% endif %}
-            {% if passportGitHubLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-github:isEnabled') %}
             <form role="form" action="/passport/github" class="d-inline-flex flex-column">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-inline-flex" id="github">
@@ -168,7 +177,7 @@
               <div class="small text-right">by GitHub Account</div>
             </form>
             {% endif %}
-            {% if passportFacebookLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-facebook:isEnabled') %}
             <form role="form" action="/passport/facebook" class="d-inline-flex flex-column">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-inline-flex" id="facebook">
@@ -178,7 +187,7 @@
               <div class="small text-right">by Facebook Account</div>
             </form>
             {% endif %}
-            {% if passportTwitterLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-twitter:isEnabled') %}
             <form role="form" action="/passport/twitter" class="d-inline-flex flex-column">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-inline-flex" id="twitter">
@@ -188,17 +197,17 @@
               <div class="small text-right">by Twitter Account</div>
             </form>
             {% endif %}
-            {% if passportOidcLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-oidc:isEnabled') %}
             <form role="form" action="/passport/oidc" class="d-inline-flex flex-column">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-inline-flex" id="oidc">
                 <span class="btn-label"><i class="fa fa-openid"></i></span>
                 <span class="btn-label-text">{{ t('Sign in') }}</span>
               </button>
-              <div class="small text-right">{{ config.crowi['security:passport-oidc:providerName'] || "OpenID Connect" }}</div>
+              <div class="small text-right">{{ getConfig('crowi', 'security:passport-oidc:providerName') || "OpenID Connect" }}</div>
             </form>
             {% endif %}
-            {% if passportSamlLoginEnabled() %}
+            {% if getConfig('crowi', 'security:isEnabledPassport') && getConfig('crowi', 'security:passport-saml:isEnabled') %}
             <form role="form" action="/passport/saml" class="d-inline-flex flex-column">
               <input type="hidden" name="_csrf" value="{{ csrf() }}">
               <button type="submit" class="fcbtn btn btn-1b btn-login-oauth d-inline-flex" id="saml">

src/server/views/me/api_token.html

@@ -17,7 +17,7 @@
 
   <ul class="nav nav-tabs">
     <li><a href="/me"><i class="icon-user"></i> {{ t('User Information') }}</a></li>
-    {% if isEnabledPassport() %}
+    {% if getConfig('crowi', 'security:isEnabledPassport') %}
     <li><a href="/me/external-accounts"><i class="icon-share-alt"></i> {{ t('External Accounts') }}</a></li>
     {% endif %}
     <li><a href="/me/password"><i class="icon-lock"></i> {{ t('Password Settings') }}</a></li>

src/server/views/me/index.html

@@ -15,7 +15,7 @@
 
   <ul class="nav nav-tabs">
     <li class="active"><a href="/me"><i class="icon-user"></i> {{ t('User Information') }}</a></li>
-    {% if isEnabledPassport() %}
+    {% if getConfig('crowi', 'security:isEnabledPassport') %}
     <li><a href="/me/external-accounts"><i class="icon-share-alt"></i> {{ t('External Accounts') }}</a></li>
     {% endif %}
     <li><a href="/me/password"><i class="icon-lock"></i> {{ t('Password Settings') }}</a></li>

src/server/views/me/password.html

@@ -15,7 +15,7 @@
 
   <ul class="nav nav-tabs">
     <li><a href="/me"><i class="icon-user"></i> {{ t('User Information') }}</a></li>
-    {% if isEnabledPassport() %}
+    {% if getConfig('crowi', 'security:isEnabledPassport') %}
     <li><a href="/me/external-accounts"><i class="icon-share-alt"></i> {{ t('External Accounts') }}</a></li>
     {% endif %}
     <li class="active"><a href="/me/password"><i class="icon-lock"></i> {{ t('Password Settings') }}</a></li>