Merge pull request #510 from weseek/master

release v3.1.9

CHANGES.md

@@ -1,7 +1,12 @@
 CHANGES
 ========
 
-## 3.1.8-RC
+## 3.1.9-RC
+
+* Improvement: Retrieve OAuth settings from environment variables
+* Fix: Couldn't activate GitHub settings after save in /admin/security
+
+## 3.1.8
 
 * Feature: Login with Google Account
 * Feature: Login with GitHub Account

README.md

@@ -165,6 +165,11 @@ Environment Variables
     * SECRET_TOKEN: A secret key for verifying the integrity of signed cookies.
     * SESSION_NAME: The name of the session ID cookie to set in the response by Express. default: `connect.sid`
     * FILE_UPLOAD: `aws` (default), `local`, `none`
+* **Option (Overwritable in admin page)**
+    * OAUTH_GOOGLE_CLIENT_ID: Google API client id for OAuth login
+    * OAUTH_GOOGLE_CLIENT_SECRET: Google API client secret for OAuth login
+    * OAUTH_GITHUB_CLIENT_ID: GitHub API client id for OAuth login
+    * OAUTH_GITHUB_CLIENT_SECRET: GitHub API client secret for OAuth login
 
 
 Documentation

lib/locales/en-US/translation.json

@@ -340,6 +340,7 @@
     "optional": "Optional",
     "Treat username matching as identical": "Automatically bind external accounts newly logged in to local accounts when <code>%s</code> match",
     "Treat username matching as identical_warn": "WARNING: Be aware of security because the system treats the same user as a match of <code>%s</code>.",
+    "Use env var if empty": "Use env var <code>%s</code> if empty",
     "ldap": {
       "server_url_detail": "The LDAP URL of the directory service in the format <code>ldap://host:port/DN</code> or <code>ldaps://host:port/DN</code>.",
       "bind_mode": "Binding Mode",

lib/locales/ja/translation.json

@@ -357,6 +357,7 @@
     "optional": "オプション",
     "Treat username matching as identical": "新規ログイン時、<code>%s</code> が一致したローカルアカウントが存在した場合は自動的に紐付ける",
     "Treat username matching as identical_warn": "警告: <code>%s</code> の一致を以て同一ユーザーであるとみなすので、セキュリティに注意してください",
+    "Use env var if empty": "空の場合、環境変数 <code>%s</code> を利用します",
     "ldap": {
       "server_url_detail": "LDAP URLを <code>ldap://host:port/DN</code> または <code>ldaps://host:port/DN</code> の形式で入力してください。",
       "bind_mode": "Bind モード",

lib/routes/admin.js

@@ -956,13 +956,13 @@ module.exports = function(crowi, app) {
     // reset strategy
     await crowi.passportService.resetGitHubStrategy();
     // setup strategy
-    if (Config.isEnabledPassportGoogle(config)) {
+    if (Config.isEnabledPassportGitHub(config)) {
       try {
         await crowi.passportService.setupGitHubStrategy(true);
       }
       catch (err) {
         // reset
-        await crowi.passportService.resetGoogleStrategy();
+        await crowi.passportService.resetGitHubStrategy();
         return res.json({status: false, message: err.message});
       }
     }

lib/routes/index.js

@@ -70,8 +70,8 @@ module.exports = function(crowi, app) {
   // OAuth
   app.post('/_api/admin/security/passport-google' , loginRequired(crowi, app) , middleware.adminRequired() , csrf, form.admin.securityPassportGoogle, admin.api.securityPassportGoogleSetting);
   app.post('/_api/admin/security/passport-github' , loginRequired(crowi, app) , middleware.adminRequired() , csrf, form.admin.securityPassportGitHub, admin.api.securityPassportGitHubSetting);
-  app.get('/passport/google'                      , loginPassport.loginPassportGoogle);
-  app.get('/passport/github'                      , loginPassport.loginPassportGitHub);
+  app.get('/passport/google'                      , loginPassport.loginWithGoogle);
+  app.get('/passport/github'                      , loginPassport.loginWithGitHub);
   app.get('/passport/google/callback'             , loginPassport.loginPassportGoogleCallback);
   app.get('/passport/github/callback'             , loginPassport.loginPassportGitHubCallback);
 

lib/routes/login-passport.js

@@ -1,7 +1,7 @@
 module.exports = function(crowi, app) {
   'use strict';
 
-  var debug = require('debug')('growi:routes:login-passport')
+  const debug = require('debug')('growi:routes:login-passport')
     , logger = require('@alias/logger')('growi:routes:login-passport')
     , passport = require('passport')
     , config = crowi.getConfig()
@@ -24,7 +24,7 @@ module.exports = function(crowi, app) {
       }
     });
 
-    var jumpTo = req.session.jumpTo;
+    const jumpTo = req.session.jumpTo;
     if (jumpTo) {
       req.session.jumpTo = null;
       return res.redirect(jumpTo);
@@ -101,7 +101,7 @@ module.exports = function(crowi, app) {
       'id': ldapAccountId,
       'username': usernameToBeRegistered,
       'name': nameToBeRegistered
-    }
+    };
 
     const externalAccount = await getOrCreateUser(req, res, next, userInfo, providerId);
     if (!externalAccount) {
@@ -112,7 +112,7 @@ module.exports = function(crowi, app) {
 
     // login
     await req.logIn(user, err => {
-      if (err) { return next(err) };
+      if (err) { return next(err) }
       return loginSuccess(req, res, user);
     });
   };
@@ -205,10 +205,11 @@ module.exports = function(crowi, app) {
     })(req, res, next);
   };
 
-  const loginPassportGoogle = function(req, res) {
+  const loginWithGoogle = function(req, res, next) {
     if (!passportService.isGoogleStrategySetup) {
       debug('GoogleStrategy has not been set up');
-      return;
+      req.flash('warningMessage', 'GoogleStrategy has not been set up');
+      return next();
     }
 
     passport.authenticate('google', {
@@ -224,7 +225,7 @@ module.exports = function(crowi, app) {
       'id': response.id,
       'username': response.displayName,
       'name': `${response.name.givenName} ${response.name.familyName}`
-    }
+    };
     const externalAccount = await getOrCreateUser(req, res, next, userInfo, providerId);
     if (!externalAccount) {
       return loginFailure(req, res, next);
@@ -234,15 +235,16 @@ module.exports = function(crowi, app) {
 
     // login
     req.logIn(user, err => {
-      if (err) { return next(err) };
+      if (err) { return next(err) }
       return loginSuccess(req, res, user);
     });
   };
 
-  const loginPassportGitHub = function(req, res) {
+  const loginWithGitHub = function(req, res, next) {
     if (!passportService.isGitHubStrategySetup) {
       debug('GitHubStrategy has not been set up');
-      return;
+      req.flash('warningMessage', 'GitHubStrategy has not been set up');
+      return next();
     }
 
     passport.authenticate('github')(req, res);
@@ -256,7 +258,7 @@ module.exports = function(crowi, app) {
       'id': response.id,
       'username': response.username,
       'name': response.displayName
-    }
+    };
 
     const externalAccount = await getOrCreateUser(req, res, next, userInfo, providerId);
     if (!externalAccount) {
@@ -267,7 +269,7 @@ module.exports = function(crowi, app) {
 
     // login
     req.logIn(user, err => {
-      if (err) { return next(err) };
+      if (err) { return next(err) }
       return loginSuccess(req, res, user);
     });
   };
@@ -290,7 +292,7 @@ module.exports = function(crowi, app) {
           return next();
         }
 
-        resolve(response)
+        resolve(response);
       })(req, res, next);
     });
   };
@@ -321,15 +323,15 @@ module.exports = function(crowi, app) {
         }
       }
     }
-  }
+  };
 
   return {
     loginFailure,
     loginWithLdap,
     testLdapCredentials,
     loginWithLocal,
-    loginPassportGoogle,
-    loginPassportGitHub,
+    loginWithGoogle,
+    loginWithGitHub,
     loginPassportGoogleCallback,
     loginPassportGitHubCallback,
   };

lib/service/passport.js

@@ -265,8 +265,8 @@ class PassportService {
 
     debug('GoogleStrategy: setting up..');
     passport.use(new GoogleStrategy({
-      clientId: config.crowi['security:passport-google:clientId'],
-      clientSecret: config.crowi['security:passport-google:clientSecret'],
+      clientId: config.crowi['security:passport-google:clientId'] || process.env.OAUTH_GOOGLE_CLIENT_SECRET,
+      clientSecret: config.crowi['security:passport-google:clientSecret'] || process.env.OAUTH_GOOGLE_CLIENT_SECRET,
       callbackURL: 'http://localhost:3000/passport/google/callback',  //change this
       skipUserProfile: false,
     }, function(accessToken, refreshToken, profile, done) {
@@ -311,8 +311,8 @@ class PassportService {
 
     debug('GitHubStrategy: setting up..');
     passport.use(new GitHubStrategy({
-      clientID: config.crowi['security:passport-github:clientId'],
-      clientSecret: config.crowi['security:passport-github:clientSecret'],
+      clientID: config.crowi['security:passport-github:clientId'] || process.env.OAUTH_GITHUB_CLIENT_ID,
+      clientSecret: config.crowi['security:passport-github:clientSecret'] || process.env.OAUTH_GITHUB_CLIENT_SECRET,
       callbackURL: 'http://localhost:3000/passport/github/callback',  //change this
       skipUserProfile: false,
     }, function(accessToken, refreshToken, profile, done) {

lib/views/admin/widget/passport/github.html

@@ -26,6 +26,11 @@
       <label for="settingForm[security:passport-github:clientId]" class="col-xs-3 control-label">{{ t("security_setting.clientID") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" name="settingForm[security:passport-github:clientId]" value="{{ settingForm['security:passport-github:clientId'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.Use env var if empty", "OAUTH_GITHUB_CLIENT_SECRET") }}
+          </small>
+        </p>
       </div>
     </div>
 
@@ -33,6 +38,11 @@
       <label for="settingForm[security:passport-github:clientSecret]" class="col-xs-3 control-label">{{ t("security_setting.client_secret") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" name="settingForm[security:passport-github:clientSecret]" value="{{ settingForm['security:passport-github:clientSecret'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.Use env var if empty", "OAUTH_GITHUB_CLIENT_SECRET") }}
+          </small>
+        </p>
       </div>
     </div>
     <div class="form-group">

lib/views/admin/widget/passport/google-oauth.html

@@ -26,6 +26,11 @@
       <label for="settingForm[security:passport-google:clientId]" class="col-xs-3 control-label">{{ t("security_setting.clientID") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" name="settingForm[security:passport-google:clientId]" value="{{ settingForm['security:passport-google:clientId'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.Use env var if empty", "OAUTH_GOOGLE_CLIENT_ID") }}
+          </small>
+        </p>
       </div>
     </div>
 
@@ -33,6 +38,11 @@
       <label for="settingForm[security:passport-google:clientSecret]" class="col-xs-3 control-label">{{ t("security_setting.client_secret") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" name="settingForm[security:passport-google:clientSecret]" value="{{ settingForm['security:passport-google:clientSecret'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.Use env var if empty", "OAUTH_GOOGLE_CLIENT_SECRET") }}
+          </small>
+        </p>
       </div>
     </div>
     <div class="form-group">

package.json

@@ -1,6 +1,6 @@
 {
   "name": "growi",
-  "version": "3.1.8-RC2",
+  "version": "3.1.9-RC",
   "description": "Team collaboration software using markdown",
   "tags": [
     "wiki",