GC-1014: fix attribute mappings

resource/locales/en-US/translation.json

@@ -383,6 +383,10 @@
       "name": "SAML",
       "entry_point": "Entry Point",
       "issuer": "Issuer",
+      "First Name": "First Name",
+      "Last Name": "Last Name",
+      "id_detail": "Specification of the name of attribute which can identify the user in SAML Identity Provider",
+      "username_detail": "Specification of mappings for <code>username</code> when creating new users",
       "mapping_detail": "Specification of mappings for %s when creating new users"
     },
     "OAuth": {

resource/locales/ja/translation.json

@@ -401,6 +401,10 @@
       "name": "SAML",
       "entry_point": "エントリーポイント",
       "issuer": "発行者",
+      "First Name": "姓",
+      "Last Name": "名",
+      "id_detail": "SAML Identity プロバイダ内で一意に識別可能な値を格納している属性",
+      "username_detail": "新規ユーザーのアカウント名(<code>username</code>)に関連付ける属性",
       "mapping_detail": "新規ユーザーの%sに関連付ける属性"
     },
     "OAuth": {

src/server/form/admin/securityPassportSaml.js

@@ -5,12 +5,12 @@ const field = form.field;
 
 module.exports = form(
   field('settingForm[security:passport-saml:isEnabled]').trim().toBooleanStrict().required(),
-  field('settingForm[security:passport-saml:entryPoint]').trim(),
-  field('settingForm[security:passport-saml:issuer]').trim(),
-  field('settingForm[security:passport-saml:attrMapId]'),
-  field('settingForm[security:passport-saml:attrMapUsername]'),
-  field('settingForm[security:passport-saml:attrMapMail]'),
-  field('settingForm[security:passport-saml:attrMapFirstName]'),
-  field('settingForm[security:passport-saml:attrMapLastName]'),
+  field('settingForm[security:passport-saml:entryPoint]').trim().required().isUrl(),
+  field('settingForm[security:passport-saml:issuer]').trim().required(),
+  field('settingForm[security:passport-saml:attrMapId]').trim().required(),
+  field('settingForm[security:passport-saml:attrMapUsername]').trim().required(),
+  field('settingForm[security:passport-saml:attrMapMail]').trim().required(),
+  field('settingForm[security:passport-saml:attrMapFirstName]').trim(),
+  field('settingForm[security:passport-saml:attrMapLastName]').trim(),
   field('settingForm[security:passport-saml:isSameUsernameTreatedAsIdenticalUser]').trim().toBooleanStrict(),
 );

src/server/routes/login-passport.js

@@ -324,17 +324,25 @@ module.exports = function(crowi, app) {
   const loginPassportSamlCallback = async(req, res, next) => {
     const providerId = 'saml';
     const strategyName = 'saml';
-    const attrMapId = config.crowi['security:passport-saml:attrMapId'] || 'id';
-    const attrMapUsername = config.crowi['security:passport-saml:attrMapUsername'] || 'userName';
+    const attrMapId = config.crowi['security:passport-saml:attrMapId'];
+    const attrMapUsername = config.crowi['security:passport-saml:attrMapUsername'];
+    const attrMapMail = config.crowi['security:passport-saml:attrMapMail'];
     const attrMapFirstName = config.crowi['security:passport-saml:attrMapFirstName'] || 'firstName';
     const attrMapLastName = config.crowi['security:passport-saml:attrMapLastName'] || 'lastName';
     const response = await promisifiedPassportAuthentication(req, res, next, strategyName);
     const userInfo = {
       'id': response[attrMapId],
       'username': response[attrMapUsername],
-      'name': `${response[attrMapFirstName]} ${response[attrMapLastName]}`,
+      'email': response[attrMapMail]
     };
 
+    // determine name
+    const firstName = response[attrMapFirstName];
+    const lastName = response[attrMapLastName];
+    if (firstName != null || lastName != null) {
+      userInfo['name'] = `${response[attrMapFirstName]} ${response[attrMapLastName]}`.trim();
+    }
+
     const externalAccount = await getOrCreateUser(req, res, next, userInfo, providerId);
     if (!externalAccount) {
       return loginFailure(req, res, next);

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

@@ -62,13 +62,13 @@
     <h4>Attribute Mapping</h4>
 
     <div class="form-group">
-      <label for="settingForm[security:passport-saml:attrMapId]" class="col-xs-3 control-label">User ID</label>
+      <label for="settingForm[security:passport-saml:attrMapId]" class="col-xs-3 control-label">Identifier</label>
       <div class="col-xs-6">
-        <input class="form-control" type="text" placeholder="Default: id"
+        <input class="form-control" type="text"
             name="settingForm[security:passport-saml:attrMapId]" value="{{ settingForm['security:passport-saml:attrMapId'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.SAML.mapping_detail", "User ID") }}
+            {{ t("security_setting.SAML.id_detail") }}
           </small>
         </p>
       </div>
@@ -77,11 +77,11 @@
     <div class="form-group">
       <label for="settingForm[security:passport-saml:attrMapUsername]" class="col-xs-3 control-label">Username</label>
       <div class="col-xs-6">
-        <input class="form-control" type="text" placeholder="Default: username"
+        <input class="form-control" type="text"
             name="settingForm[security:passport-saml:attrMapUsername]" value="{{ settingForm['security:passport-saml:attrMapUsername'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.SAML.mapping_detail", "Username") }}
+            {{ t("security_setting.SAML.username_detail") }}
           </small>
         </p>
       </div>
@@ -105,26 +105,39 @@
     </div>
 
     <div class="form-group">
-      <label for="settingForm[security:passport-saml:attrMapFirstName]" class="col-xs-3 control-label">First Name</label>
+      <label for="settingForm[security:passport-saml:attrMapMail]" class="col-xs-3 control-label">Mail</label>
+      <div class="col-xs-6">
+        <input class="form-control" type="text"
+            name="settingForm[security:passport-saml:attrMapMail]" value="{{ settingForm['security:passport-saml:attrMapMail'] || '' }}">
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.SAML.mapping_detail", t("Email")) }}
+          </small>
+        </p>
+      </div>
+    </div>
+
+    <div class="form-group">
+      <label for="settingForm[security:passport-saml:attrMapFirstName]" class="col-xs-3 control-label">{{ t("security_setting.SAML.First Name") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" placeholder="Default: firstName"
             name="settingForm[security:passport-saml:attrMapFirstName]" value="{{ settingForm['security:passport-saml:attrMapFirstName'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.SAML.mapping_detail", "First Name") }}
+            {{ t("security_setting.SAML.mapping_detail", t("security_setting.SAML.First Name")) }}
           </small>
         </p>
       </div>
     </div>
 
     <div class="form-group">
-      <label for="settingForm[security:passport-saml:attrMapLastName]" class="col-xs-3 control-label">Last Name</label>
+      <label for="settingForm[security:passport-saml:attrMapLastName]" class="col-xs-3 control-label">{{ t("security_setting.SAML.Last Name") }}</label>
       <div class="col-xs-6">
         <input class="form-control" type="text" placeholder="Default: lastName"
             name="settingForm[security:passport-saml:attrMapLastName]" value="{{ settingForm['security:passport-saml:attrMapLastName'] || '' }}">
         <p class="help-block">
           <small>
-            {{ t("security_setting.SAML.mapping_detail", "Last Name") }}
+            {{ t("security_setting.SAML.mapping_detail", t("security_setting.SAML.Last Name")) }}
           </small>
         </p>
       </div>