Merge pull request #1023 from shield-9/oidc-improvements

Some OIDC improvements

resource/locales/en-US/translation.json

@@ -490,9 +490,8 @@
     "xss_prevent_setting":"Prevent XSS(Cross Site Scripting)",
     "xss_prevent_setting_link":"Go to Markdown settings",
     "callback_URL": "Callback URL",
+    "providerName": "Provider Name",
     "issuerHost": "Issuer Host",
-    "authorizeURL": "Authorization URL",
-    "tokenURL": "Token URL",
     "scope": "Scope",
     "desc_of_callback_URL": "Use it in the setting of the %s provider",
     "guest_mode": {
@@ -584,11 +583,11 @@
         "name": "OpenID Connect",
         "id_detail": "Specification of the name of attribute which can identify the user in OIDC claims",
         "username_detail": "Specification of mappings for <code>username</code> when creating new users",
+        "name_detail": "Specification of mappings for <code>name</code> when creating new users",
         "mapping_detail": "Specification of mappings for %s when creating new users",
         "register_1": "Contant to OIDC IdP Administrator",
         "register_2": "Register your OIDC App with \"Authorization callback URL\" as <code>%s</code> (where <code>%s</code> is your hostname)",
-        "register_3": "Copy and paste your ClientID and Client Secret above",
-        "register_4": "Set scope to openid"
+        "register_3": "Copy and paste your ClientID and Client Secret above"
       },
       "how_to": {
         "google": "How to configure Google OAuth?",

src/server/form/admin/securityPassportOidc.js

@@ -4,11 +4,14 @@ const field = form.field;
 
 module.exports = form(
   field('settingForm[security:passport-oidc:isEnabled]').trim().toBooleanStrict().required(),
+  field('settingForm[security:passport-oidc:providerName]').trim(),
   field('settingForm[security:passport-oidc:issuerHost]').trim(),
   field('settingForm[security:passport-oidc:clientId]').trim(),
   field('settingForm[security:passport-oidc:clientSecret]').trim(),
+  field('settingForm[security:passport-oidc:attrMapId]').trim(),
   field('settingForm[security:passport-oidc:attrMapUserName]').trim(),
+  field('settingForm[security:passport-oidc:attrMapName]').trim(),
   field('settingForm[security:passport-oidc:attrMapMail]').trim(),
-  field('settingForm[security:passport-oidc:attrMapId]').trim(),
+  field('settingForm[security:passport-oidc:isSameEmailTreatedAsIdenticalUser]').trim().toBooleanStrict(),
   field('settingForm[security:passport-oidc:isSameUsernameTreatedAsIdenticalUser]').trim().toBooleanStrict(),
 );

src/server/models/config.js

@@ -347,11 +347,6 @@ module.exports = function(crowi) {
     return getValueForCrowiNS(config, key);
   };
 
-  configSchema.statics.isSameUsernameTreatedAsIdenticalUser = function(config, providerType) {
-    const key = `security:passport-${providerType}:isSameUsernameTreatedAsIdenticalUser`;
-    return getValueForCrowiNS(config, key);
-  };
-
   configSchema.statics.isUploadable = function(config) {
     const method = process.env.FILE_UPLOAD || 'aws';
 

src/server/routes/login-passport.js

@@ -365,6 +365,7 @@ module.exports = function(crowi, app) {
     const strategyName = 'oidc';
     const attrMapId = crowi.configManager.getConfig('crowi', 'security:passport-oidc:attrMapId');
     const attrMapUserName = crowi.configManager.getConfig('crowi', 'security:passport-oidc:attrMapUserName');
+    const attrMapName = crowi.configManager.getConfig('crowi', 'security:passport-oidc:attrMapName');
     const attrMapMail = crowi.configManager.getConfig('crowi', 'security:passport-oidc:attrMapMail');
 
     let response;
@@ -379,7 +380,7 @@ module.exports = function(crowi, app) {
     const userInfo = {
       id: response[attrMapId],
       username: response[attrMapUserName],
-      name: response[attrMapUserName],
+      name: response[attrMapName],
       email: response[attrMapMail],
     };
     debug('mapping response to userInfo', userInfo, response, attrMapId, attrMapUserName, attrMapMail);

src/server/service/passport.js

@@ -494,19 +494,21 @@ class PassportService {
       client_id: clientId,
       client_secret: clientSecret,
       redirect_uris: [redirectUri],
-      scope: 'openid email profile',
-      response: 'code',
+      response_types: ['code'],
     });
 
-    passport.use('oidc', new OidcStrategy({ client },
-      ((tokenset, userinfo, done) => {
-        if (userinfo) {
-          return done(null, userinfo);
-        }
+    passport.use('oidc', new OidcStrategy({
+      client,
+      params: { scope: 'openid email profile' },
+    },
+    ((tokenset, userinfo, done) => {
+      if (userinfo) {
+        return done(null, userinfo);
+      }
 
-        return done(null, false);
+      return done(null, false);
 
-      })));
+    })));
 
     this.isOidcStrategySetup = true;
     debug('OidcStrategy: setup is done');

src/server/views/admin/widget/passport/oidc.html

@@ -24,6 +24,13 @@
   </div>
   <fieldset id="passport-oidc-hide-when-disabled" {%if !isOidcEnabled %}style="display: none;"{% endif %}>
 
+    <div class="form-group">
+      <label for="settingForm[security:passport-oidc:providerName]" class="col-xs-3 control-label">{{ t("security_setting.providerName") }}</label>
+      <div class="col-xs-6">
+        <input class="form-control" type="text" name="settingForm[security:passport-oidc:providerName]" value="{{ settingForm['security:passport-oidc:providerName'] || '' }}">
+      </div>
+    </div>
+
     <div class="form-group">
       <label for="settingForm[security:passport-oidc:issuerHost]" class="col-xs-3 control-label">{{ t("security_setting.issuerHost") }}</label>
       <div class="col-xs-6">
@@ -60,6 +67,8 @@
       </div>
     </div>
 
+    <h4>Attribute Mapping ({{ t("security_setting.optional") }})</h4>
+
     <div class="form-group">
       <label for="settingForm[security:passport-oidc:attrMapId]" class="col-xs-3 control-label">Identifier</label>
       <div class="col-xs-6">
@@ -78,7 +87,19 @@
         <input class="form-control" type="text" name="settingForm[security:passport-oidc:attrMapUserName]" value="{{ settingForm['security:passport-oidc:attrMapUserName'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.OIDC.username_detail") }}
+            {{ t("security_setting.OAuth.OIDC.username_detail") }}
+          </small>
+        </p>
+      </div>
+    </div>
+
+    <div class="form-group">
+      <label for="settingForm[security:passport-oidc:attrMapName]" class="col-xs-3 control-label">Name</label>
+      <div class="col-xs-6">
+        <input class="form-control" type="text" name="settingForm[security:passport-oidc:attrMapName]" value="{{ settingForm['security:passport-oidc:attrName'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.OAuth.OIDC.name_detail") }}
           </small>
         </p>
       </div>
@@ -90,7 +111,7 @@
         <input class="form-control" type="text" name="settingForm[security:passport-oidc:attrMapMail]" value="{{ settingForm['security:passport-oidc:attrMapMail'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.OIDC.mapping_detail", t("Email")) }}
+            {{ t("security_setting.OAuth.OIDC.mapping_detail", t("Email")) }}
           </small>
         </p>
       </div>
@@ -126,6 +147,23 @@
       </div>
     </div>
 
+    <div class="form-group">
+      <div class="col-xs-6 col-xs-offset-3">
+        <div class="checkbox checkbox-info">
+          <input type="checkbox" id="bindByEmail-oidc" name="settingForm[security:passport-oidc:isSameEmailTreatedAsIdenticalUser]" value="1"
+              {% if settingForm['security:passport-oidc:isSameEmailTreatedAsIdenticalUser'] %}checked{% endif %} />
+          <label for="bindByEmail-oidc">
+            {{ t("security_setting.Treat email matching as identical", "email") }}
+          </label>
+          <p class="help-block">
+            <small>
+              {{ t("security_setting.Treat email matching as identical_warn", "email") }}
+            </small>
+          </p>
+        </div>
+      </div>
+    </div>
+
   </fieldset>
 
   <div class="form-group" id="btn-update">
@@ -149,7 +187,6 @@
     <li>{{ t("security_setting.OAuth.OIDC.register_1") }}</li>
     <li>{{ t("security_setting.OAuth.OIDC.register_2", callbackUrl) }}</li>
     <li>{{ t("security_setting.OAuth.OIDC.register_3") }}</li>
-    <li>{{ t("security_setting.OAuth.OIDC.register_4") }}</li>
   </ol>
 </div>
 

src/server/views/login.html

@@ -195,7 +195,7 @@
                 <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">by OpenID Connect</div>
+              <div class="small text-right">{{ config.crowi['security:passport-oidc:providerName'] || "OpenID Connect" }}</div>
             </form>
             {% endif %}
             {% if passportSamlLoginEnabled() %}