Просмотр исходного кода

Merge pull request #6585 from weseek/feat/use-apiv3-for-post-login

feat: Modify middlewares used for POST /login with apiv3
Yohei Shiina 3 лет назад
Родитель
Сommit
5372aaed87

+ 22 - 9
packages/app/public/static/locales/en_US/translation.json

@@ -691,9 +691,19 @@
     "fail_to_save_access_token": "Failed to save access_token. Please try again.",
     "fail_to_fetch_access_token": "Failed to fetch access_token. Please do connect again.",
     "successfully_disconnected": "Successfully Disconnected!",
-    "strategy_has_not_been_set_up": "{{strategy}} has not been set up",
+    "strategy_has_not_been_set_up": {
+      "LocalStrategy"  : "LocalStrategy has not been set up",
+      "LdapStrategy"   : "LdapStrategy has not been set up",
+      "GoogleStrategy" : "GoogleStrategy has not been set up",
+      "GitHubStrategy" : "GitHubStrategy has not been set up",
+      "TwitterStrategy": "TwitterStrategy has not been set up",
+      "OidcStrategy"   : "OidcStrategy has not been set up",
+      "SamlStrategy"   : "SamlStrategy has not been set up",
+      "Basic"          : "Basic has not been set up"
+    },
+    "ldap_user_not_valid": "Ldap user is no valid",
+    "external_account_not_exist": "Failed to find or create External account",
     "maximum_number_of_users": "Can not register more than the maximum number of users.",
-    "database_error": "Database Server Error occured",
     "sign_in_failure": "Sign in failure.",
     "aws_sttings_required": "AWS settings required to use this function. Please ask the administrator.",
     "application_already_installed": "Application already installed.",
@@ -715,13 +725,16 @@
     "user_already_loggedin": "You cannot create a new account when you are logged in.",
     "registration_closed": "You are not authorized to create a new account.",
     "Username has invalid characters": "Username has invalid characters.",
-    "Username field is required": "User ID field is required",
-    "Name field is required": "Name field is required",
-    "Email format is invalid": "Email format is invalid",
-    "Email field is required": "Email field is required",
-    "Password has invalid character": "Password has invalid character",
-    "Password minimum character should be more than 8 characters": "Password minimum character should be more than 8 characters",
-    "Password field is required": "Password field is required"
+    "Username field is required": "User ID field is required.",
+    "Name field is required": "Name field is required.",
+    "Email format is invalid": "Email format is invalid.",
+    "Email field is required": "Email field is required.",
+    "Password has invalid character": "Password has invalid character.",
+    "Password minimum character should be more than 8 characters": "Password minimum character should be more than 8 characters.",
+    "Password field is required": "Password field is required.",
+    "Username or E-mail has invalid characters": "Username or E-mail has invalid characters.",
+    "Password minimum character should be more than 6 characters": "Password minimum character should be more than 6 characters.",
+    "user_not_found": "User not found."
   },
   "grid_edit":{
     "create_bootstrap_4_grid":"Create Bootstrap 4 Grid",

+ 16 - 3
packages/app/public/static/locales/ja_JP/translation.json

@@ -682,9 +682,19 @@
     "fail_to_save_access_token": "アクセストークンの保存に失敗しました、再度お試しください。",
     "fail_to_fetch_access_token": "アクセストークンの取得に失敗しました、再度お試しください。",
     "successfully_disconnected": "切断に成功しました!",
-    "strategy_has_not_been_set_up": "{{strategy}} はセットアップされていません。",
+    "strategy_has_not_been_set_up": {
+      "LocalStrategy"  : "LocalStrategy はセットアップされていません。",
+      "LdapStrategy"   : "LdapStrategy はセットアップされていません。",
+      "GoogleStrategy" : "GoogleStrategy はセットアップされていません。",
+      "GitHubStrategy" : "GitHubStrategy はセットアップされていません。",
+      "TwitterStrategy": "TwitterStrategy はセットアップされていません。",
+      "OidcStrategy"   : "OidcStrategy はセットアップされていません。",
+      "SamlStrategy"   : "SamlStrategy はセットアップされていません。",
+      "Basic"          : "Basic はセットアップされていません。"
+    },
+    "ldap_user_not_valid": "Ldap user is no valid",
+    "external_account_not_exist": "外部アカウントが見つからない、または作成に失敗しました",
     "maximum_number_of_users": "ユーザー数が上限を超えたためアクティベートできません。",
-    "database_error":"データベースサーバーに問題があります。",
     "sign_in_failure": "ログインに失敗しました。",
     "aws_sttings_required": "この機能にはAWS設定が必要です。管理者に訪ねて下さい。",
     "application_already_installed": "アプリケーションのインストールが完了しました。",
@@ -712,7 +722,10 @@
     "Email field is required": "メールアドレスは必須項目です",
     "Password has invalid character": "パスワードに無効な文字があります",
     "Password minimum character should be more than 8 characters": "パスワードの最小文字数は8文字以上です",
-    "Password field is required": "パスワードの欄は必ず入力してください"
+    "Password field is required": "パスワードの欄は必ず入力してください",
+    "Username or E-mail has invalid characters": "ユーザー名または、メールアドレスに無効な文字があります",
+    "Password minimum character should be more than 6 characters": "パスワードの最小文字数は6文字以上です",
+    "user_not_found": "ユーザーが見つかりません"
   },
   "grid_edit":{
     "create_bootstrap_4_grid":"Bootstrap 4 グリッドを作成",

+ 16 - 3
packages/app/public/static/locales/zh_CN/translation.json

@@ -738,9 +738,19 @@
 		"fail_to_save_access_token": "无法保存访问令牌。请再试一次。",
 		"fail_to_fetch_access_token": "无法获取访问令牌。请重新连接。",
 		"successfully_disconnected": "成功断开连接!",
-		"strategy_has_not_been_set_up": "{{strategy}尚未设置",
+    "strategy_has_not_been_set_up": {
+      "LocalStrategy"  : "LocalStrategy 尚未设置",
+      "LdapStrategy"   : "LdapStrategy 尚未设置",
+      "GoogleStrategy" : "GoogleStrategy 尚未设置",
+      "GitHubStrategy" : "GitHubStrategy 尚未设置",
+      "TwitterStrategy": "TwitterStrategy 尚未设置",
+      "OidcStrategy"   : "OidcStrategy 尚未设置",
+      "SamlStrategy"   : "SamlStrategy 尚未设置",
+      "Basic"          : "Basic 尚未设置"
+     },
+    "ldap_user_not_valid": "Ldap user is no valid",
+    "external_account_not_exist": "查找或创建外部账户失败",
 		"maximum_number_of_users": "注册的用户数不能超过最大值。",
-		"database_error": "发生数据库服务器错误",
 		"sign_in_failure": "登录失败。",
 		"aws_sttings_required": "使用此功能所需的AWS设置。请询问管理员。",
 		"application_already_installed": "应用程序已安装。",
@@ -768,7 +778,10 @@
     "Email field is required": "电子邮件字段是必需的",
     "Password has invalid character": "密码有无效字符",
     "Password minimum character should be more than 8 characters": "密码最小字符应超过8个字符",
-    "Password field is required": "密码字段是必需的"
+    "Password field is required": "密码字段是必需的",
+    "Username or E-mail has invalid characters": "用户名或电子邮件有无效的字符",
+    "Password minimum character should be more than 6 characters": "密码最小字符应超过6个字符",
+    "user_not_found": "未找到用户"
 	},
   "grid_edit":{
     "create_bootstrap_4_grid":"创建Bootstrap 4网格",

+ 101 - 55
packages/app/src/components/LoginForm.tsx

@@ -7,7 +7,6 @@ import { useRouter } from 'next/router';
 import ReactCardFlip from 'react-card-flip';
 
 import { apiv3Post } from '~/client/util/apiv3-client';
-import { useCsrfToken } from '~/stores/context';
 
 type LoginFormProps = {
   username?: string,
@@ -26,22 +25,25 @@ type LoginFormProps = {
 export const LoginForm = (props: LoginFormProps): JSX.Element => {
   const { t } = useTranslation();
   const router = useRouter();
-  const { data: csrfToken } = useCsrfToken();
 
   const {
     isLocalStrategySetup, isLdapStrategySetup, isPasswordResetEnabled, isRegistrationEnabled,
-    isEmailAuthenticationEnabled, registrationMode, registrationWhiteList, isMailerSetup,
+    isEmailAuthenticationEnabled, registrationMode, registrationWhiteList, isMailerSetup, objOfIsExternalAuthEnableds,
   } = props;
   const isLocalOrLdapStrategiesEnabled = isLocalStrategySetup || isLdapStrategySetup;
-  // const isSomeExternalAuthEnabled = Object.values(objOfIsExternalAuthEnableds).some(elem => elem);
-  const isSomeExternalAuthEnabled = true;
+  const isSomeExternalAuthEnabled = Object.values(objOfIsExternalAuthEnableds).some(elem => elem);
 
   // states
   const [isRegistering, setIsRegistering] = useState(false);
-  const [username, setUsername] = useState('');
-  const [name, setName] = useState('');
-  const [email, setEmail] = useState('');
-  const [password, setPassword] = useState('');
+  // For Login
+  const [usernameForLogin, setUsernameForLogin] = useState('');
+  const [passwordForLogin, setPasswordForLogin] = useState('');
+  const [loginErrors, setLoginErrors] = useState<Error[]>([]);
+  // For Register
+  const [usernameForRegister, setUsernameForRegister] = useState('');
+  const [nameForRegister, setNameForRegister] = useState('');
+  const [emailForRegister, setEmailForRegister] = useState('');
+  const [passwordForRegister, setPasswordForRegister] = useState('');
   const [registerErrors, setRegisterErrors] = useState<Error[]>([]);
 
   useEffect(() => {
@@ -57,49 +59,86 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
 
     window.location.href = `/passport/${auth}`;
   }, []);
+
+  const handleLoginWithLocalSubmit = useCallback(async(e) => {
+    e.preventDefault();
+
+    const loginForm = {
+      username: usernameForLogin,
+      password: passwordForLogin,
+    };
+
+    try {
+      const res = await apiv3Post('/login', { loginForm });
+      const { redirectTo } = res.data;
+      router.push(redirectTo);
+    }
+    catch (err) {
+      setLoginErrors(err);
+    }
+    return;
+
+  }, [passwordForLogin, router, usernameForLogin]);
+
   const renderLocalOrLdapLoginForm = useCallback(() => {
     const { isLdapStrategySetup } = props;
-
     return (
-      <form role="form" action="/login" method="post">
-        <div className="input-group">
-          <div className="input-group-prepend">
-            <span className="input-group-text">
-              <i className="icon-user"></i>
-            </span>
-          </div>
-          <input type="text" className="form-control rounded-0" data-testid="tiUsernameForLogin" placeholder="Username or E-mail" name="loginForm[username]" />
-          {isLdapStrategySetup && (
-            <div className="input-group-append">
-              <small className="input-group-text text-success">
-                <i className="icon-fw icon-check"></i> LDAP
-              </small>
+      <>
+        {
+          loginErrors != null && loginErrors.length > 0 && (
+            <p className="alert alert-danger">
+              {loginErrors.map((err, index) => {
+                return (
+                  <span key={index}>
+                    {t(err.message)}<br/>
+                  </span>
+                );
+              })}
+            </p>
+          )
+        }
+        <form role="form" onSubmit={handleLoginWithLocalSubmit} id="login-form">
+          <div className="input-group">
+            <div className="input-group-prepend">
+              <span className="input-group-text">
+                <i className="icon-user"></i>
+              </span>
             </div>
-          )}
-        </div>
+            <input type="text" className="form-control rounded-0" data-testid="tiUsernameForLogin" placeholder="Username or E-mail"
+              onChange={(e) => { setUsernameForLogin(e.target.value) }} name="usernameForLogin" />
+            {isLdapStrategySetup && (
+              <div className="input-group-append">
+                <small className="input-group-text text-success">
+                  <i className="icon-fw icon-check"></i> LDAP
+                </small>
+              </div>
+            )}
+          </div>
 
-        <div className="input-group">
-          <div className="input-group-prepend">
-            <span className="input-group-text">
-              <i className="icon-lock"></i>
-            </span>
+          <div className="input-group">
+            <div className="input-group-prepend">
+              <span className="input-group-text">
+                <i className="icon-lock"></i>
+              </span>
+            </div>
+            <input type="password" className="form-control rounded-0" data-testid="tiPasswordForLogin" placeholder="Password"
+              onChange={(e) => { setPasswordForLogin(e.target.value) }} name="passwordForLogin" />
           </div>
-          <input type="password" className="form-control rounded-0" data-testid="tiPasswordForLogin" placeholder="Password" name="loginForm[password]" />
-        </div>
 
-        <div className="input-group my-4">
-          <input type="hidden" name="_csrf" value={csrfToken} />
-          <button type="submit" id="login" className="btn btn-fill rounded-0 login mx-auto" data-testid="btnSubmitForLogin">
-            <div className="eff"></div>
-            <span className="btn-label">
-              <i className="icon-login"></i>
-            </span>
-            <span className="btn-label-text">{t('Sign in')}</span>
-          </button>
-        </div>
-      </form>
+          <div className="input-group my-4">
+            <button type="submit" id="login" className="btn btn-fill rounded-0 login mx-auto" data-testid="btnSubmitForLogin">
+              <div className="eff"></div>
+              <span className="btn-label">
+                <i className="icon-login"></i>
+              </span>
+              <span className="btn-label-text">{t('Sign in')}</span>
+            </button>
+          </div>
+        </form>
+      </>
     );
-  }, [csrfToken, props, t]);
+  }, [handleLoginWithLocalSubmit, loginErrors, props, t]);
+
   const renderExternalAuthInput = useCallback((auth) => {
     const authIconNames = {
       google: 'google',
@@ -124,6 +163,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
       </div>
     );
   }, [handleLoginWithExternalAuth, t]);
+
   const renderExternalAuthLoginForm = useCallback(() => {
     const { isLocalStrategySetup, isLdapStrategySetup, objOfIsExternalAuthEnableds } = props;
     const isExternalAuthCollapsible = isLocalStrategySetup || isLdapStrategySetup;
@@ -163,10 +203,10 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
     e.preventDefault();
 
     const registerForm = {
-      username,
-      name,
-      email,
-      password,
+      username: usernameForRegister,
+      name: nameForRegister,
+      email: emailForRegister,
+      password: passwordForRegister,
     };
     try {
       const res = await apiv3Post(requestPath, { registerForm });
@@ -180,7 +220,12 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
       }
     }
     return;
-  }, [email, name, password, router, username]);
+  }, [emailForRegister, nameForRegister, passwordForRegister, router, usernameForRegister]);
+
+  const resetLoginErrors = useCallback(() => {
+    if (loginErrors.length === 0) return;
+    setLoginErrors([]);
+  }, [loginErrors.length]);
 
   const resetRegisterErrors = useCallback(() => {
     if (registerErrors.length === 0) return;
@@ -189,8 +234,9 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
 
   const switchForm = useCallback(() => {
     setIsRegistering(!isRegistering);
+    resetLoginErrors();
     resetRegisterErrors();
-  }, [isRegistering, resetRegisterErrors]);
+  }, [isRegistering, resetLoginErrors, resetRegisterErrors]);
 
   const renderRegisterForm = useCallback(() => {
     let registerAction = '/register';
@@ -222,7 +268,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
               {registerErrors.map((err, index) => {
                 return (
                   <span key={index}>
-                    {t(`message.${err.message}`)}<br/>
+                    {t(err.message)}<br/>
                   </span>
                 );
               })}
@@ -244,7 +290,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
                 <input
                   type="text"
                   className="form-control rounded-0"
-                  onChange={(e) => { setUsername(e.target.value) }}
+                  onChange={(e) => { setUsernameForRegister(e.target.value) }}
                   placeholder={t('User ID')}
                   name="username"
                   defaultValue={props.username}
@@ -263,7 +309,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
                 {/* name */}
                 <input type="text"
                   className="form-control rounded-0"
-                  onChange={(e) => { setName(e.target.value) }}
+                  onChange={(e) => { setNameForRegister(e.target.value) }}
                   placeholder={t('Name')}
                   name="name"
                   defaultValue={props.name}
@@ -281,7 +327,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
             {/* email */}
             <input type="email"
               className="form-control rounded-0"
-              onChange={(e) => { setEmail(e.target.value) }}
+              onChange={(e) => { setEmailForRegister(e.target.value) }}
               placeholder={t('Email')}
               name="email"
               defaultValue={props.email}
@@ -315,7 +361,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
                 {/* Password */}
                 <input type="password"
                   className="form-control rounded-0"
-                  onChange={(e) => { setPassword(e.target.value) }}
+                  onChange={(e) => { setPasswordForRegister(e.target.value) }}
                   placeholder={t('Password')}
                   name="password"
                   required />

+ 9 - 7
packages/app/src/server/middlewares/login-form-validator.ts

@@ -48,18 +48,18 @@ export const loginRules = () => {
   return [
     body('loginForm.username')
       .matches(/^[\da-zA-Z\-_.+@]+$/)
-      .withMessage('Username or E-mail has invalid characters')
+      .withMessage('message.Username or E-mail has invalid characters')
       .not()
       .isEmpty()
-      .withMessage('Username field is required'),
+      .withMessage('message.Username field is required'),
     body('loginForm.password')
       .matches(/^[\x20-\x7F]*$/)
-      .withMessage('Password has invalid character')
+      .withMessage('message.Password has invalid character')
       .isLength({ min: 6 })
-      .withMessage('Password minimum character should be more than 6 characters')
+      .withMessage('message.Password minimum character should be more than 6 characters')
       .not()
       .isEmpty()
-      .withMessage('Password field is required'),
+      .withMessage('message.Password field is required'),
   ];
 };
 
@@ -77,8 +77,10 @@ export const loginValidation = (req, res, next) => {
   const extractedErrors: string[] = [];
   errors.array().map(err => extractedErrors.push(err.msg));
 
-  req.flash('errorMessages', extractedErrors);
-  Object.assign(form, { isValid: false });
+  Object.assign(form, {
+    isValid: false,
+    errors: extractedErrors,
+  });
   req.form = form;
 
   return next();

+ 8 - 8
packages/app/src/server/middlewares/register-form-validator.ts

@@ -6,24 +6,24 @@ export const registerRules = () => {
   return [
     body('registerForm.username')
       .matches(/^[\da-zA-Z\-_.]+$/)
-      .withMessage('Username has invalid characters')
+      .withMessage('message.Username has invalid characters')
       .not()
       .isEmpty()
-      .withMessage('Username field is required'),
-    body('registerForm.name').not().isEmpty().withMessage('Name field is required'),
+      .withMessage('message.Username field is required'),
+    body('registerForm.name').not().isEmpty().withMessage('message.Name field is required'),
     body('registerForm.email')
       .isEmail()
-      .withMessage('Email format is invalid')
+      .withMessage('message.Email format is invalid')
       .exists()
-      .withMessage('Email field is required'),
+      .withMessage('message.Email field is required'),
     body('registerForm.password')
       .matches(/^[\x20-\x7F]*$/)
-      .withMessage('Password has invalid character')
+      .withMessage('message.Password has invalid character')
       .isLength({ min: PASSOWRD_MINIMUM_NUMBER })
-      .withMessage('Password minimum character should be more than 8 characters')
+      .withMessage('message.Password minimum character should be more than 8 characters')
       .not()
       .isEmpty()
-      .withMessage('Password field is required'),
+      .withMessage('message.Password field is required'),
     body('registerForm[app:globalLang]'),
   ];
 };

+ 5 - 0
packages/app/src/server/routes/apiv3/index.js

@@ -2,6 +2,7 @@ import loggerFactory from '~/utils/logger';
 
 import { generateAddActivityMiddleware } from '../../middlewares/add-activity';
 import injectUserRegistrationOrderByTokenMiddleware from '../../middlewares/inject-user-registration-order-by-token-middleware';
+import * as loginFormValidator from '../../middlewares/login-form-validator';
 import * as registerFormValidator from '../../middlewares/register-form-validator';
 
 import pageListing from './page-listing';
@@ -43,6 +44,10 @@ module.exports = (crowi, app, isInstalled) => {
   const applicationInstalled = require('../../middlewares/application-installed')(crowi);
   const addActivity = generateAddActivityMiddleware(crowi);
   const login = require('../login')(crowi, app);
+  const loginPassport = require('../login-passport')(crowi, app);
+
+  routerForAuth.post('/login', applicationInstalled, loginFormValidator.loginRules(), loginFormValidator.loginValidation,
+    addActivity, loginPassport.loginWithLocal, loginPassport.loginWithLdap, loginPassport.cannotLoginErrorHadnler, loginPassport.loginFailure);
 
   routerForAuth.use('/logout', require('./logout')(crowi));
 

+ 0 - 1
packages/app/src/server/routes/index.js

@@ -82,7 +82,6 @@ module.exports = function(crowi, app) {
   app.get('/login'                    , applicationInstalled, login.preLogin, next.delegateToNext);
   app.get('/invited'                  , applicationInstalled, next.delegateToNext);
   app.post('/invited/activateInvited' , applicationInstalled, loginFormValidator.inviteRules(), loginFormValidator.inviteValidation, csrfProtection, login.invited);
-  app.post('/login'                   , applicationInstalled, loginFormValidator.loginRules(), loginFormValidator.loginValidation, csrfProtection,  addActivity, loginPassport.loginWithLocal, loginPassport.loginWithLdap, loginPassport.loginFailure);
 
   app.get('/register'                 , applicationInstalled, login.preLogin, login.register);
 

+ 113 - 90
packages/app/src/server/routes/login-passport.js

@@ -2,6 +2,8 @@ import { SupportedAction } from '~/interfaces/activity';
 import { NullUsernameToBeRegisteredError } from '~/server/models/errors';
 import loggerFactory from '~/utils/logger';
 
+import ErrorV3 from '../models/vo/error-apiv3';
+
 /* eslint-disable no-use-before-define */
 
 module.exports = function(crowi, app) {
@@ -15,6 +17,74 @@ module.exports = function(crowi, app) {
 
   const ApiResponse = require('../util/apiResponse');
 
+  const promisifiedPassportAuthentication = (strategyName, req, res) => {
+    return new Promise((resolve, reject) => {
+      passport.authenticate(strategyName, (err, response, info) => {
+        if (res.headersSent) { // dirty hack -- 2017.09.25
+          return; //              cz: somehow passport.authenticate called twice when ECONNREFUSED error occurred
+        }
+
+        logger.debug(`--- authenticate with ${strategyName} strategy ---`);
+
+        if (err) {
+          logger.error(`'${strategyName}' passport authentication error: `, err);
+          reject(err);
+        }
+
+        logger.debug('response', response);
+        logger.debug('info', info);
+
+        // authentication failure
+        if (!response) {
+          reject(response);
+        }
+
+        resolve(response);
+      })(req, res);
+    });
+  };
+
+  const getOrCreateUser = async(req, res, userInfo, providerId) => {
+    // get option
+    const isSameUsernameTreatedAsIdenticalUser = crowi.passportService.isSameUsernameTreatedAsIdenticalUser(providerId);
+    const isSameEmailTreatedAsIdenticalUser = crowi.passportService.isSameEmailTreatedAsIdenticalUser(providerId);
+
+    try {
+      // find or register(create) user
+      const externalAccount = await ExternalAccount.findOrRegister(
+        providerId,
+        userInfo.id,
+        userInfo.username,
+        userInfo.name,
+        userInfo.email,
+        isSameUsernameTreatedAsIdenticalUser,
+        isSameEmailTreatedAsIdenticalUser,
+      );
+      return externalAccount;
+    }
+    catch (err) {
+      /* eslint-disable no-else-return */
+      if (err instanceof NullUsernameToBeRegisteredError) {
+        logger.error(err.message);
+        throw Error(err.message);
+      }
+      else if (err.name === 'DuplicatedUsernameException') {
+        if (isSameEmailTreatedAsIdenticalUser || isSameUsernameTreatedAsIdenticalUser) {
+          // associate to existing user
+          debug(`ExternalAccount '${userInfo.username}' will be created and bound to the exisiting User account`);
+          return ExternalAccount.associate(providerId, userInfo.id, err.user);
+        }
+        logger.error('provider-DuplicatedUsernameException', providerId);
+        throw Error(`provider-DuplicatedUsernameException: ${providerId}`);
+      }
+      else if (err.name === 'UserUpperLimitException') {
+        logger.error(err.message);
+        throw Error(err.message);
+      }
+      /* eslint-enable no-else-return */
+    }
+  };
+
   /**
    * success handler
    * @param {*} req
@@ -44,7 +114,7 @@ module.exports = function(crowi, app) {
     };
     await crowi.activityService.createActivity(parameters);
 
-    return res.safeRedirect(redirectTo);
+    return res.apiv3({ redirectTo });
   };
 
   /**
@@ -61,13 +131,23 @@ module.exports = function(crowi, app) {
     return res.redirect('/login');
   };
 
+  const cannotLoginErrorHadnler = (req, res, next) => {
+    // this is called when all login method is somehow failed without invoking 'return next(<any Error>)'
+    const err = res.locals.err != null ? res.locals.err : Error('message.sign_in_failure');
+    return next(err);
+  };
+
   /**
    * middleware for login failure
    * @param {*} req
    * @param {*} res
    */
-  const loginFailure = (req, res) => {
-    return loginFailureHandler(req, res, req.t('message.sign_in_failure'));
+  const loginFailure = (error, req, res, next) => {
+
+    const parameters = { action: SupportedAction.ACTION_USER_LOGIN_FAILURE };
+    activityEvent.emit('update', res.locals.activity._id, parameters);
+
+    return res.apiv3Err(error, error.code);
   };
 
   /**
@@ -95,13 +175,11 @@ module.exports = function(crowi, app) {
   const loginWithLdap = async(req, res, next) => {
     if (!passportService.isLdapStrategySetup) {
       debug('LdapStrategy has not been set up');
-      return next();
+      return res.apiv3Err('message.strategy_has_not_been_set_up.LdapStrategy', 405);
     }
 
     if (!req.form.isValid) {
-      debug('invalid form');
-      return res.render('login', {
-      });
+      return next(req.form.errors);
     }
 
     const providerId = 'ldap';
@@ -113,12 +191,12 @@ module.exports = function(crowi, app) {
     }
     catch (err) {
       debug(err.message);
-      return next();
+      return next(err);
     }
 
     // check groups for LDAP
     if (!isValidLdapUserByGroupFilter(ldapAccountInfo)) {
-      return next();
+      return next(ErrorV3('message.ldap_user_not_valid', 400));
     }
 
     /*
@@ -141,16 +219,27 @@ module.exports = function(crowi, app) {
       email: mailToBeRegistered,
     };
 
-    const externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
-    if (!externalAccount) {
-      return next();
+    let externalAccount;
+    try {
+      externalAccount = await getOrCreateUser(req, res, userInfo, providerId);
+    }
+    catch (error) {
+      return next(error);
+    }
+
+    // just in case the returned value is null or undefined
+    if (externalAccount == null) {
+      return next(Error('message.external_account_not_exist'));
     }
 
     const user = await externalAccount.getPopulatedUser();
 
     // login
     await req.logIn(user, (err) => {
-      if (err) { debug(err.message); return next() }
+      if (err) {
+        debug(err.message);
+        return next(err);
+      }
 
       return loginSuccessHandler(req, res, user, SupportedAction.ACTION_USER_LOGIN_WITH_LDAP);
     });
@@ -221,13 +310,11 @@ module.exports = function(crowi, app) {
   const loginWithLocal = (req, res, next) => {
     if (!passportService.isLocalStrategySetup) {
       debug('LocalStrategy has not been set up');
-      req.flash('warningMessage', req.t('message.strategy_has_not_been_set_up', { strategy: 'LocalStrategy' }));
-      return next();
+      return res.apiv3Err('message.strategy_has_not_been_set_up.LocalStrategy', 405);
     }
 
     if (!req.form.isValid) {
-      return res.render('login', {
-      });
+      return next(req.form.errors);
     }
 
     passport.authenticate('local', (err, user, info) => {
@@ -237,12 +324,16 @@ module.exports = function(crowi, app) {
 
       if (err) { // DB Error
         logger.error('Database Server Error: ', err);
-        req.flash('warningMessage', req.t('message.database_error'));
-        return next(); // pass and the flash message is displayed when all of authentications are failed.
+        return next(err);
+      }
+      if (!user) {
+        return next();
       }
-      if (!user) { return next() }
       req.logIn(user, (err) => {
-        if (err) { debug(err.message); return next() }
+        if (err) {
+          debug(err.message);
+          return next(err);
+        }
 
         return loginSuccessHandler(req, res, user, SupportedAction.ACTION_USER_LOGIN_WITH_LOCAL);
       });
@@ -560,76 +651,8 @@ module.exports = function(crowi, app) {
     });
   };
 
-  const promisifiedPassportAuthentication = (strategyName, req, res) => {
-    return new Promise((resolve, reject) => {
-      passport.authenticate(strategyName, (err, response, info) => {
-        if (res.headersSent) { // dirty hack -- 2017.09.25
-          return; //              cz: somehow passport.authenticate called twice when ECONNREFUSED error occurred
-        }
-
-        logger.debug(`--- authenticate with ${strategyName} strategy ---`);
-
-        if (err) {
-          logger.error(`'${strategyName}' passport authentication error: `, err);
-          reject(err);
-        }
-
-        logger.debug('response', response);
-        logger.debug('info', info);
-
-        // authentication failure
-        if (!response) {
-          reject(response);
-        }
-
-        resolve(response);
-      })(req, res);
-    });
-  };
-
-  const getOrCreateUser = async(req, res, userInfo, providerId) => {
-    // get option
-    const isSameUsernameTreatedAsIdenticalUser = crowi.passportService.isSameUsernameTreatedAsIdenticalUser(providerId);
-    const isSameEmailTreatedAsIdenticalUser = crowi.passportService.isSameEmailTreatedAsIdenticalUser(providerId);
-
-    try {
-      // find or register(create) user
-      const externalAccount = await ExternalAccount.findOrRegister(
-        providerId,
-        userInfo.id,
-        userInfo.username,
-        userInfo.name,
-        userInfo.email,
-        isSameUsernameTreatedAsIdenticalUser,
-        isSameEmailTreatedAsIdenticalUser,
-      );
-      return externalAccount;
-    }
-    catch (err) {
-      /* eslint-disable no-else-return */
-      if (err instanceof NullUsernameToBeRegisteredError) {
-        req.flash('warningMessage', req.t(`message.${err.message}`));
-        return;
-      }
-      else if (err.name === 'DuplicatedUsernameException') {
-        if (isSameEmailTreatedAsIdenticalUser || isSameUsernameTreatedAsIdenticalUser) {
-          // associate to existing user
-          debug(`ExternalAccount '${userInfo.username}' will be created and bound to the exisiting User account`);
-          return ExternalAccount.associate(providerId, userInfo.id, err.user);
-        }
-
-        req.flash('provider-DuplicatedUsernameException', providerId);
-        return;
-      }
-      else if (err.name === 'UserUpperLimitException') {
-        req.flash('warningMessage', req.t('message.maximum_number_of_users'));
-        return;
-      }
-      /* eslint-enable no-else-return */
-    }
-  };
-
   return {
+    cannotLoginErrorHadnler,
     loginFailure,
     loginWithLdap,
     testLdapCredentials,

+ 1 - 9
packages/app/src/server/routes/login.js

@@ -102,14 +102,6 @@ module.exports = function(crowi, app) {
     next();
   };
 
-  actions.login = function(req, res) {
-    if (req.form) {
-      debug(req.form.errors);
-    }
-
-    return res.render('login', {});
-  };
-
   actions.register = function(req, res) {
     if (req.user != null) {
       return res.apiv3Err('user_already_logged_in', 403);
@@ -122,7 +114,7 @@ module.exports = function(crowi, app) {
 
     if (!req.form.isValid) {
       const errors = req.form.errors;
-      return res.apiv3Err(errors, 401);
+      return res.apiv3Err(errors, 400);
     }
 
     const registerForm = req.form.registerForm || {};