Merge pull request #6886 from weseek/fix/108280-do-not-allow-users-to-login-when-user-registration-is-restricted

fix: Do not allow users to login when user registration is restricted

packages/app/public/static/locales/en_US/translation.json

@@ -627,7 +627,7 @@
   },
   "login": {
     "Sign in error": "Login error",
-    "Registration successful": "Registration successful",
+    "Registration successful": "Registration successful. Please wait for administrator approval.",
     "Setup": "Setup",
     "enabled_ldap_has_configuration_problem":"LDAP is enabled but the configuration has something wrong.",
     "set_env_var_for_logs": "(Please set the environment variables <code>DEBUG=crowi:service:PassportService</code> to get the logs)"

packages/app/public/static/locales/ja_JP/translation.json

@@ -621,7 +621,7 @@
   },
   "login": {
     "Sign in error": "ログインエラー",
-    "Registration successful": "登録完了",
+    "Registration successful": "登録が完了しました。管理者の承認をお待ちください。",
     "Setup": "セットアップ",
     "enabled_ldap_has_configuration_problem":"LDAPは有効ですが、設定に問題があります。",
     "set_env_var_for_logs": "(ログを取得するためには、環境変数 <code>DEBUG=crowi:service:PassportService</code> を設定してください。)"

packages/app/public/static/locales/zh_CN/translation.json

@@ -629,7 +629,7 @@
   },
 	"login": {
 		"Sign in error": "登录错误",
-		"Registration successful": "注册成功",
+		"Registration successful": "注册成功。请等待管理员批准",
 		"Setup": "安装程序",
     "enabled_ldap_has_configuration_problem":"启用了LDAP，但配置有问题。",
     "set_env_var_for_logs": "(请设置环境变量 <code>DEBUG=crowi:service:PassportService</code> 以获得日志。)"

packages/app/src/components/CompleteUserRegistration.tsx

@@ -0,0 +1,24 @@
+import React, { FC } from 'react';
+
+import { useTranslation } from 'next-i18next';
+import Link from 'next/link';
+
+export const CompleteUserRegistration: FC = () => {
+  const { t } = useTranslation();
+
+  return (
+    <div className="noLogin-dialog mx-auto" id="noLogin-dialog">
+      <div className="row mx-0">
+        <div className="col-12 mb-3 text-center">
+          <p className="alert alert-success">
+            <span>{t('login.Registration successful')}</span>
+          </p>
+          {/* If the transition source is "/login", use <a /> tag since the transition will not occur if next/link is used. */}
+          <a href='/login'>
+            <i className="icon-login mr-1" />{t('Sign in is here')}
+          </a>
+        </div>
+      </div>
+    </div>
+  );
+};

packages/app/src/components/CompleteUserRegistrationForm.tsx

@@ -1,16 +1,21 @@
 import React, { useState, useEffect, useCallback } from 'react';
 
 import { useTranslation } from 'next-i18next';
+import { useRouter } from 'next/router';
 
 import { apiv3Get, apiv3Post } from '~/client/util/apiv3-client';
 import { UserActivationErrorCode } from '~/interfaces/errors/user-activation';
+import { RegistrationMode } from '~/interfaces/registration-mode';
 
-import { toastSuccess, toastError } from '../client/util/apiNotification';
+import { toastError } from '../client/util/apiNotification';
+
+import { CompleteUserRegistration } from './CompleteUserRegistration';
 
 interface Props {
   email: string,
   token: string,
   errorCode?: UserActivationErrorCode,
+  registrationMode: RegistrationMode,
   isEmailAuthenticationEnabled: boolean,
 }
 
@@ -21,6 +26,7 @@ const CompleteUserRegistrationForm: React.FC<Props> = (props: Props) => {
     email,
     token,
     errorCode,
+    registrationMode,
     isEmailAuthenticationEnabled,
   } = props;
 
@@ -31,6 +37,9 @@ const CompleteUserRegistrationForm: React.FC<Props> = (props: Props) => {
   const [name, setName] = useState('');
   const [password, setPassword] = useState('');
   const [disableForm, setDisableForm] = useState(forceDisableForm);
+  const [isSuccessToRagistration, setIsSuccessToRagistration] = useState(false);
+
+  const router = useRouter();
 
   useEffect(() => {
     const delayDebounceFn = setTimeout(async() => {
@@ -52,17 +61,27 @@ const CompleteUserRegistrationForm: React.FC<Props> = (props: Props) => {
     e.preventDefault();
     setDisableForm(true);
     try {
-      await apiv3Post('/complete-registration', {
+      const res = await apiv3Post('/complete-registration', {
         username, name, password, token,
       });
-      toastSuccess('Registration succeed');
-      window.location.href = '/login';
+
+      setIsSuccessToRagistration(true);
+
+      const { redirectTo } = res.data;
+      if (redirectTo != null) {
+        router.push(redirectTo);
+      }
     }
     catch (err) {
       toastError(err, 'Registration failed');
       setDisableForm(false);
+      setIsSuccessToRagistration(false);
     }
-  }, [name, password, token, username]);
+  }, [username, name, password, token, router]);
+
+  if (isSuccessToRagistration && registrationMode === RegistrationMode.RESTRICTED) {
+    return <CompleteUserRegistration />;
+  }
 
   return (
     <>

packages/app/src/components/LoginForm.tsx

@@ -9,15 +9,18 @@ import ReactCardFlip from 'react-card-flip';
 import { apiv3Post } from '~/client/util/apiv3-client';
 import { LoginErrorCode } from '~/interfaces/errors/login-error';
 import { IErrorV3 } from '~/interfaces/errors/v3-error';
+import { RegistrationMode } from '~/interfaces/registration-mode';
 import { toArrayIfNot } from '~/utils/array-utils';
 
+import { CompleteUserRegistration } from './CompleteUserRegistration';
+
 type LoginFormProps = {
   username?: string,
   name?: string,
   email?: string,
   isRegistrationEnabled: boolean,
   isEmailAuthenticationEnabled: boolean,
-  registrationMode?: string,
+  registrationMode: RegistrationMode,
   registrationWhiteList: string[],
   isPasswordResetEnabled: boolean,
   isLocalStrategySetup: boolean,
@@ -51,7 +54,8 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
   const [registerErrors, setRegisterErrors] = useState<IErrorV3[]>([]);
   // For UserActivation
   const [emailForRegistrationOrder, setEmailForRegistrationOrder] = useState('');
-  const [isSuccessToSendRegistrationOrderEmail, setIsSuccessToSendRegistrationOrderEmail] = useState(false);
+
+  const [isSuccessToRagistration, setIsSuccessToRagistration] = useState(false);
 
 
   useEffect(() => {
@@ -271,7 +275,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
   const handleRegisterFormSubmit = useCallback(async(e, requestPath) => {
     e.preventDefault();
     setEmailForRegistrationOrder('');
-    setIsSuccessToSendRegistrationOrderEmail(false);
+    setIsSuccessToRagistration(false);
 
     const registerForm = {
       username: usernameForRegister,
@@ -282,14 +286,17 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
     try {
       const res = await apiv3Post(requestPath, { registerForm });
 
+      setIsSuccessToRagistration(true);
       resetRegisterErrors();
 
       const { redirectTo } = res.data;
-      router.push(redirectTo ?? '/');
+      if (redirectTo != null) {
+        router.push(redirectTo);
+      }
 
       if (isEmailAuthenticationEnabled) {
         setEmailForRegistrationOrder(emailForRegister);
-        setIsSuccessToSendRegistrationOrderEmail(true);
+        return;
       }
     }
     catch (err) {
@@ -318,7 +325,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
 
     return (
       <React.Fragment>
-        {registrationMode === 'Restricted' && (
+        {registrationMode === RegistrationMode.RESTRICTED && (
           <p className="alert alert-warning">
             {t('page_register.notice.restricted')}
             <br />
@@ -346,7 +353,7 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
         }
 
         {
-          (isEmailAuthenticationEnabled && isSuccessToSendRegistrationOrderEmail) && (
+          (isEmailAuthenticationEnabled && isSuccessToRagistration) && (
             <p className="alert alert-success">
               <span>{t('message.successfully_send_email_auth', { email: emailForRegistrationOrder })}</span>
             </p>
@@ -476,11 +483,14 @@ export const LoginForm = (props: LoginFormProps): JSX.Element => {
       </React.Fragment>
     );
   }, [
-    handleRegisterFormSubmit, isEmailAuthenticationEnabled, isMailerSetup,
-    isSuccessToSendRegistrationOrderEmail, props.email, props.name, props.username,
-    registerErrors, registrationMode, registrationWhiteList, emailForRegistrationOrder, switchForm, t,
+    t, isEmailAuthenticationEnabled, registrationMode, isMailerSetup, registerErrors, isSuccessToRagistration,
+    emailForRegistrationOrder, props.username, props.name, props.email, registrationWhiteList, switchForm, handleRegisterFormSubmit,
   ]);
 
+  if (registrationMode === RegistrationMode.RESTRICTED && isSuccessToRagistration && !isEmailAuthenticationEnabled) {
+    return <CompleteUserRegistration />;
+  }
+
   return (
     <div className="noLogin-dialog mx-auto" id="noLogin-dialog">
       <div className="row mx-0">

packages/app/src/interfaces/registration-mode.ts

@@ -0,0 +1,7 @@
+export const RegistrationMode = {
+  OPEN: 'Open',
+  RESTRICTED: 'Restricted',
+  CLOSED: 'Closed',
+} as const;
+
+export type RegistrationMode = typeof RegistrationMode[keyof typeof RegistrationMode];

packages/app/src/pages/login.page.tsx

@@ -1,6 +1,5 @@
 import React from 'react';
 
-
 import {
   NextPage, GetServerSideProps, GetServerSidePropsContext,
 } from 'next';
@@ -9,19 +8,19 @@ import { serverSideTranslations } from 'next-i18next/serverSideTranslations';
 import { NoLoginLayout } from '~/components/Layout/NoLoginLayout';
 import { LoginForm } from '~/components/LoginForm';
 import type { CrowiRequest } from '~/interfaces/crowi-request';
+import type { RegistrationMode } from '~/interfaces/registration-mode';
 
 import {
   useCsrfToken,
   useCurrentPathname,
 } from '../stores/context';
 
-
 import {
   CommonProps, getServerSideCommonProps, useCustomTitle, getNextI18NextConfig,
 } from './utils/commons';
 
 type Props = CommonProps & {
-
+  registrationMode: RegistrationMode,
   pageWithMetaStr: string,
   isMailerSetup: boolean,
   enabledStrategies: unknown,
@@ -55,6 +54,7 @@ const LoginPage: NextPage<Props> = (props: Props) => {
         registrationWhiteList={props.registrationWhiteList}
         isPasswordResetEnabled={true}
         isMailerSetup={props.isMailerSetup}
+        registrationMode={props.registrationMode}
       />
     </NoLoginLayout>
   );
@@ -106,6 +106,7 @@ async function injectServerConfigurations(context: GetServerSidePropsContext, pr
   props.isLdapSetupFailed = configManager.getConfig('crowi', 'security:passport-ldap:isEnabled') && !props.isLdapStrategySetup;
   props.registrationWhiteList = configManager.getConfig('crowi', 'security:registrationWhiteList');
   props.isEmailAuthenticationEnabled = configManager.getConfig('crowi', 'security:passport-local:isEmailAuthenticationEnabled');
+  props.registrationMode = configManager.getConfig('crowi', 'security:registrationMode');
 }
 
 export const getServerSideProps: GetServerSideProps = async(context: GetServerSidePropsContext) => {

packages/app/src/pages/user-activation.page.tsx

@@ -5,6 +5,7 @@ import CompleteUserRegistrationForm from '~/components/CompleteUserRegistrationF
 import { NoLoginLayout } from '~/components/Layout/NoLoginLayout';
 import type { CrowiRequest } from '~/interfaces/crowi-request';
 import type { UserActivationErrorCode } from '~/interfaces/errors/user-activation';
+import type { RegistrationMode } from '~/interfaces/registration-mode';
 import { IUserRegistrationOrder } from '~/server/models/user-registration-order';
 
 import {
@@ -15,6 +16,7 @@ type Props = CommonProps & {
   token: string
   email: string
   errorCode?: UserActivationErrorCode
+  registrationMode: RegistrationMode
   isEmailAuthenticationEnabled: boolean
 }
 
@@ -25,6 +27,7 @@ const UserActivationPage: NextPage<Props> = (props: Props) => {
         token={props.token}
         email={props.email}
         errorCode={props.errorCode}
+        registrationMode={props.registrationMode}
         isEmailAuthenticationEnabled={props.isEmailAuthenticationEnabled}
       />
     </NoLoginLayout>
@@ -64,6 +67,7 @@ export const getServerSideProps: GetServerSideProps = async(context: GetServerSi
     props.errorCode = context.query.errorCode as UserActivationErrorCode;
   }
 
+  props.registrationMode = req.crowi.configManager.getConfig('crowi', 'security:registrationMode');
   props.isEmailAuthenticationEnabled = req.crowi.configManager.getConfig('crowi', 'security:passport-local:isEmailAuthenticationEnabled');
 
   await injectNextI18NextConfigurations(context, props, ['translation']);

packages/app/src/server/routes/apiv3/index.js

@@ -97,6 +97,7 @@ module.exports = (crowi, app, isInstalled) => {
   router.get('/check-username', user.api.checkUsername);
 
   router.post('/complete-registration',
+    addActivity,
     injectUserRegistrationOrderByTokenMiddleware,
     userActivation.completeRegistrationRules(),
     userActivation.validateCompleteRegistration,

packages/app/src/server/routes/apiv3/user-activation.ts

@@ -4,6 +4,7 @@ import { ErrorV3 } from '@growi/core';
 import { format, subSeconds } from 'date-fns';
 import { body, validationResult } from 'express-validator';
 
+import { SupportedAction } from '~/interfaces/activity';
 import UserRegistrationOrder from '~/server/models/user-registration-order';
 import loggerFactory from '~/utils/logger';
 
@@ -64,6 +65,7 @@ async function sendEmailToAllAdmins(userData, admins, appTitle, mailService, tem
 
 export const completeRegistrationAction = (crowi) => {
   const User = crowi.model('User');
+  const activityEvent = crowi.event('activity');
   const {
     configManager,
     aclService,
@@ -127,6 +129,9 @@ export const completeRegistrationAction = (crowi) => {
           return res.apiv3Err(new ErrorV3(errorMessage, 'registration-failed'), 403);
         }
 
+        const parameters = { action: SupportedAction.ACTION_USER_REGISTRATION_SUCCESS };
+        activityEvent.emit('update', res.locals.activity._id, parameters);
+
         userRegistrationOrder.revokeOneTimeToken();
 
         if (configManager.getConfig('crowi', 'security:registrationMode') === aclService.labels.SECURITY_REGISTRATION_MODE_RESTRICTED) {
@@ -145,9 +150,28 @@ export const completeRegistrationAction = (crowi) => {
           else {
             logger.warn('E-mail Settings must be set up.');
           }
+
+          return res.apiv3({});
         }
 
-        res.apiv3({ status: 'ok' });
+        req.login(userData, (err) => {
+          if (err) {
+            logger.debug(err);
+          }
+          else {
+            // update lastLoginAt
+            userData.updateLastLoginAt(new Date(), (err) => {
+              if (err) {
+                logger.error(`updateLastLoginAt dumps error: ${err}`);
+              }
+            });
+          }
+
+          // userData.password cann't be empty but, prepare redirect because password property in User Model is optional
+          // https://github.com/weseek/growi/pull/6670
+          const redirectTo = userData.password != null ? '/' : '/me#password';
+          return res.apiv3({ redirectTo });
+        });
       });
     });
   };

packages/app/src/server/routes/login.js

@@ -16,35 +16,6 @@ module.exports = function(crowi, app) {
 
   const actions = {};
 
-  const registerSuccessHandler = function(req, res, userData) {
-    req.login(userData, (err) => {
-      if (err) {
-        logger.debug(err);
-      }
-      else {
-        // update lastLoginAt
-        userData.updateLastLoginAt(new Date(), (err) => {
-          if (err) {
-            logger.error(`updateLastLoginAt dumps error: ${err}`);
-          }
-        });
-      }
-
-
-      // userData.password cann't be empty but, prepare redirect because password property in User Model is optional
-      // https://github.com/weseek/growi/pull/6670
-      const redirectTo = userData.password ? req.session.redirectTo : '/me#password';
-
-      // remove session.redirectTo
-      delete req.session.redirectTo;
-
-      const parameters = { action: SupportedAction.ACTION_USER_REGISTRATION_SUCCESS };
-      activityEvent.emit('update', res.locals.activity._id, parameters);
-
-      return res.apiv3({ redirectTo });
-    });
-  };
-
   async function sendEmailToAllAdmins(userData) {
     // send mails to all admin users (derived from crowi) -- 2020.06.18 Yuki Takei
     const admins = await User.findAdmins();
@@ -71,6 +42,39 @@ module.exports = function(crowi, app) {
       .forEach(result => logger.error(result.reason));
   }
 
+  const registerSuccessHandler = async function(req, res, userData, registrationMode) {
+    const parameters = { action: SupportedAction.ACTION_USER_REGISTRATION_SUCCESS };
+    activityEvent.emit('update', res.locals.activity._id, parameters);
+
+    if (registrationMode === aclService.labels.SECURITY_REGISTRATION_MODE_RESTRICTED) {
+      await sendEmailToAllAdmins(userData);
+      return res.apiv3({});
+    }
+
+    req.login(userData, (err) => {
+      if (err) {
+        logger.debug(err);
+      }
+      else {
+        // update lastLoginAt
+        userData.updateLastLoginAt(new Date(), (err) => {
+          if (err) {
+            logger.error(`updateLastLoginAt dumps error: ${err}`);
+          }
+        });
+      }
+
+      // userData.password cann't be empty but, prepare redirect because password property in User Model is optional
+      // https://github.com/weseek/growi/pull/6670
+      const redirectTo = userData.password ? req.session.redirectTo : '/me#password';
+
+      // remove session.redirectTo
+      delete req.session.redirectTo;
+
+      return res.apiv3({ redirectTo });
+    });
+  };
+
   actions.error = function(req, res) {
     const reason = req.params.reason;
 
@@ -166,13 +170,7 @@ module.exports = function(crowi, app) {
           }
           return res.apiv3Err(errors, 405);
         }
-
-        if (registrationMode === aclService.labels.SECURITY_REGISTRATION_MODE_RESTRICTED) {
-          // send mail asynchronous
-          sendEmailToAllAdmins(userData);
-        }
-
-        return registerSuccessHandler(req, res, userData);
+        return registerSuccessHandler(req, res, userData, registrationMode);
       });
     });
   };