Merge pull request #1136 from weseek/imprv/refactor-basic-auth

Imprv/refactor basic auth

package.json

@@ -68,7 +68,6 @@
     "async": "^3.0.1",
     "aws-sdk": "^2.88.0",
     "axios": "^0.19.0",
-    "basic-auth-connect": "~1.0.0",
     "body-parser": "^1.18.2",
     "bunyan": "^1.8.12",
     "bunyan-format": "^0.2.1",

resource/locales/en-US/translation.json

@@ -528,7 +528,9 @@
       "note for the only env option": "The setting item that enables or disables the SAML authentication and the highlighted setting items use only the value of environment variables.<br>To change this setting, please change to false or delete the value of the environment variable <code>%s</code> ."
     },
     "Basic": {
-      "name": "Basic Authentication"
+      "name": "Basic Authentication",
+      "desc_1": "Login with <code>username</code> in Authorization header.",
+      "desc_2": "User will be automatically generated if not exist."
     },
     "OAuth": {
       "register": "Register for %s",
@@ -538,7 +540,7 @@
         "register_1": "Access <a href=\"%s\" target=\"_blank\">%s</a>",
         "register_2": "Create Project if no projects exist",
         "register_3": "Create Credentials &rightarrow; OAuth client ID &rightarrow; Select \"Web application\"",
-        "register_4": "Register your OAuth App with one of Authorized redirect URIs as <code>%s</code> (where <code>%s</code> is your hostname)",
+        "register_4": "Register your OAuth App with one of Authorized redirect URIs as <code>%s</code>",
         "register_5": "Copy and paste your ClientID and Client Secret above"
       },
       "Facebook": {
@@ -549,13 +551,13 @@
         "register_1": "Access <a href=\"%s\" target=\"_blank\">%s</a>",
         "register_2": "Sign in Twitter",
         "register_3": "Create Credentials &rightarrow; OAuth client ID &rightarrow; Select \"Web application\"",
-        "register_4": "Register your OAuth App with one of Authorized redirect URIs as <code>%s</code> (where <code>%s</code> is your hostname)",
+        "register_4": "Register your OAuth App with one of Authorized redirect URIs as <code>%s</code>",
         "register_5": "Copy and paste your ClientID and Client Secret above"
       },
       "GitHub": {
         "name": "GitHub OAuth",
         "register_1": "Access <a href=\"%s\" target=\"_blank\">%s</a>",
-        "register_2": "Register your OAuth App with \"Authorization callback URL\" as <code>%s</code> (where <code>%s</code> is your hostname)",
+        "register_2": "Register your OAuth App with \"Authorization callback URL\" as <code>%s</code>",
         "register_3": "Copy and paste your ClientID and Client Secret above"
       },
       "OIDC": {
@@ -565,7 +567,7 @@
         "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_2": "Register your OIDC App with \"Authorization callback URL\" as <code>%s</code>",
         "register_3": "Copy and paste your ClientID and Client Secret above"
       },
       "how_to": {

resource/locales/ja/translation.json

@@ -522,7 +522,9 @@
       "note for the only env option": "現在SAML認証のON/OFFの設定値及びハイライトされている設定値は環境変数の値のみを使用するようになっています<br>この設定を変更する場合は環境変数 <code>%s</code> の値をfalseに変更もしくは削除してください"
     },
     "Basic": {
-      "name": "Basic 認証"
+      "name": "Basic 認証",
+      "desc_1": "Authorization ヘッダに格納されている <code>username</code> でログインします。",
+      "desc_2": "ユーザーが存在しなかった場合は自動生成します。"
     },
     "OAuth": {
       "register": "%sに登録",

src/server/form/admin/securityPassportBasic.js

@@ -4,6 +4,5 @@ const field = form.field;
 
 module.exports = form(
   field('settingForm[security:passport-basic:isEnabled]').trim().toBooleanStrict().required(),
-  field('settingForm[security:passport-basic:id]').trim(),
-  field('settingForm[security:passport-basic:password]').trim(),
+  field('settingForm[security:passport-basic:isSameUsernameTreatedAsIdenticalUser]').trim().toBooleanStrict(),
 );

src/server/routes/login-passport.js

@@ -476,10 +476,7 @@ module.exports = function(crowi, app) {
       userId = await promisifiedPassportAuthentication(strategyName, req, res);
     }
     catch (err) {
-      // display prompt in browser
-      res.setHeader('WWW-Authenticate', 'Basic realm="Users"');
-      res.sendStatus(401).end();
-      return;
+      return loginFailure(req, res);
     }
 
     const userInfo = {

src/server/routes/login.js

@@ -99,10 +99,7 @@ module.exports = function(crowi, app) {
   };
 
   actions.register = function(req, res) {
-    // redirect to '/' if both of these are true:
-    //  1. user has logged in
-    //  2. req.user is not username/email string (which is set by basic-auth-connect)
-    if (req.user != null && req.user instanceof Object) {
+    if (req.user != null) {
       return res.redirect('/');
     }
 

src/server/service/passport.js

@@ -621,15 +621,12 @@ class PassportService {
 
     debug('BasicStrategy: setting up..');
 
-    const configId = configManager.getConfig('crowi', 'security:passport-basic:id');
-    const configPassword = configManager.getConfig('crowi', 'security:passport-basic:password');
-
     passport.use(new BasicStrategy(
       (userId, password, done) => {
-        if (userId !== configId || password !== configPassword) {
-          return done(null, false, { message: 'Incorrect credentials.' });
+        if (userId != null) {
+          return done(null, userId);
         }
-        return done(null, userId);
+        return done(null, false, { message: 'Incorrect credentials.' });
       },
     ));
 

src/server/util/i18nUserSettingDetector.js

@@ -2,10 +2,7 @@ module.exports = {
   name: 'userSettingDetector',
 
   lookup(req, res, options) {
-    // return null if
-    //  1. user doesn't logged in
-    //  2. req.user is username/email string to login which is set by basic-auth-connect
-    if (req.user == null || !(req.user instanceof Object)) {
+    if (req.user == null) {
       return null;
     }
     return req.user.lang || null;

src/server/util/middlewares.js

@@ -170,8 +170,6 @@ module.exports = (crowi, app) => {
   };
 
   middlewares.adminRequired = function(req, res, next) {
-    // check the user logged in
-    //  make sure that req.user isn't username/email string to login which is set by basic-auth-connect
     if (req.user != null && (req.user instanceof Object) && '_id' in req.user) {
       if (req.user.admin) {
         next();
@@ -202,7 +200,6 @@ module.exports = (crowi, app) => {
       const User = crowi.model('User');
 
       // check the user logged in
-      //  make sure that req.user isn't username/email string to login which is set by basic-auth-connect
       if (req.user != null && (req.user instanceof Object) && '_id' in req.user) {
         if (req.user.status === User.STATUS_ACTIVE) {
           // Active の人だけ先に進める

src/server/views/admin/external-accounts.html

@@ -100,7 +100,7 @@
                   <i class="icon-settings"></i> <span class="caret"></span>
                 </button>
                 <ul class="dropdown-menu" role="menu">
-                  <li class="dropdown-header">{{ t('user_management.Edit_menu') }}</li>
+                  <li class="dropdown-header">{{ t('user_management.edit_menu') }}</li>
                   <form id="form_remove_{{ loop.index }}" action="/admin/users/external-accounts/{{ account._id.toString() }}/remove" method="post">
                     <input type="hidden" name="_csrf" value="{{ csrf() }}">
                   </form>

src/server/views/admin/security.html

@@ -236,7 +236,7 @@
   </div>
 
   <script>
-    $('#generalSetting, #samlSetting, #googleSetting, #mechanismSetting, #githubSetting, #twitterSetting, #oidcSetting').each(function() {
+    $('#generalSetting, #samlSetting, #basicSetting, #googleSetting, #githubSetting, #twitterSetting, #oidcSetting').each(function() {
       $(this).submit(function()
       {
         function showMessage(formId, msg, status) {

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

@@ -3,7 +3,7 @@
   <legend class="alert-anchor">{{ t("security_setting.Basic.name") }} {{ t("security_setting.configuration") }}</legend>
 
   {% set nameForIsbasicEnabled = "settingForm[security:passport-basic:isEnabled]" %}
-  {% set isbasicEnabled = settingForm['security:passport-basic:isEnabled'] %}
+  {% set isbasicEnabled = getConfig('crowi', 'security:passport-basic:isEnabled') %}
 
   <div class="form-group">
     <label for="{{nameForIsbasicEnabled}}" class="col-xs-3 control-label">{{ t("security_setting.Basic.name") }}</label>
@@ -18,21 +18,32 @@
               {% if !isbasicEnabled %}checked{% endif %}> OFF
         </label>
       </div>
+      <p class="help-block">
+        <small>
+          {{ t("security_setting.Basic.desc_1") }}<br>
+          {{ t("security_setting.Basic.desc_2") }}
+        </small>
+      </p>
     </div>
   </div>
+
+
   <fieldset id="passport-basic-hide-when-disabled" {%if !isbasicEnabled %}style="display: none;"{% endif %}>
 
     <div class="form-group">
-      <label for="settingForm[security:passport-basic:id]" class="col-xs-3 control-label">ID</label>
-      <div class="col-xs-6">
-        <input class="form-control" type="text" name="settingForm[security:passport-basic:id]" value="{{ settingForm['security:passport-basic:id'] || '' }}">
-      </div>
+    <div class="col-xs-6 col-xs-offset-3">
+      <div class="checkbox checkbox-info">
+        <input type="checkbox" id="bindByUserName-basic" name="settingForm[security:passport-basic:isSameUsernameTreatedAsIdenticalUser]" value="1"
+            {% if getConfig('crowi', 'security:passport-basic:isSameUsernameTreatedAsIdenticalUser') %}checked{% endif %} />
+        <label for="bindByUserName-basic">
+          {{ t("security_setting.Treat username matching as identical", "username") }}
+        </label>
+        <p class="help-block">
+          <small>
+            {{ t("security_setting.Treat username matching as identical_warn", "username") }}
+          </small>
+        </p>
     </div>
-
-    <div class="form-group">
-      <label for="settingForm[security:passport-basic:password]" class="col-xs-3 control-label">{{ t("Password") }}</label>
-      <div class="col-xs-6">
-        <input class="form-control" type="text" name="settingForm[security:passport-basic:password]" value="{{ settingForm['security:passport-basic:password'] || '' }}">
       </div>
     </div>
 

yarn.lock

@@ -1826,10 +1826,6 @@ base@^0.11.1:
     mixin-deep "^1.2.0"
     pascalcase "^0.1.1"
 
-basic-auth-connect@~1.0.0:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/basic-auth-connect/-/basic-auth-connect-1.0.0.tgz#fdb0b43962ca7b40456a7c2bb48fe173da2d2122"
-
 basic-auth@~2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.0.tgz#015db3f353e02e56377755f962742e8981e7bbba"