🔒 feat: password reset disable option; fix: account email error message (#2327)

* feat: password reset  disable option; fix: account email leak

* fix(LoginSpec): typo

* test: fixed LoginForm test

* fix: disable password reset when undefined

* refactor: use a helper function

* fix: tests

* feat: Remove unused error message in password reset process

* chore: Update password reset email message

* refactor: only allow password reset if explicitly allowed

* feat: Add password reset email service configuration check

The code changes in `checks.js` add a new function `checkPasswordReset()` that checks if the email service is configured when password reset is enabled. If the email service is not configured, a warning message is logged. This change ensures secure password reset functionality by prompting the user to configure the email service.

Co-authored-by: Berry-13 <root@Berry>
Co-authored-by: Danny Avila <messagedaniel@protonmail.com>
Co-authored-by: Danny Avila <danny@librechat.ai>

* chore: remove import order rules

* refactor: simplify password reset logic and align against Observable Response Discrepancy

* chore: make password reset warning more prominent

* chore(AuthService): better logging for password resets, refactor requestPasswordReset to use req object, fix sendEmail error when email config is not present

* refactor: fix styling of password reset email message

* chore: add missing type for passwordResetEnabled, TStartupConfig

* fix(LoginForm): prevent login form flickering

* fix(ci): Update login form to use mocked startupConfig for rendering correctly

* refactor: Improve password reset UI, applies DRY

* chore: Add logging to password reset validation middleware

* chore(CONTRIBUTING): Update import order conventions

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
Co-authored-by: Berry-13 <root@Berry>
Co-authored-by: Danny Avila <messagedaniel@protonmail.com>
This commit is contained in:
Marco Beretta 2024-06-06 17:39:36 +02:00 committed by GitHub
parent a7f5b57272
commit 5452d4c20c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 288 additions and 137 deletions

View file

@ -14,7 +14,9 @@ function Login() {
return (
<>
{error && <ErrorMessage>{localize(getLoginError(error))}</ErrorMessage>}
{startupConfig?.emailLoginEnabled && <LoginForm onSubmit={login} />}
{startupConfig?.emailLoginEnabled && (
<LoginForm onSubmit={login} startupConfig={startupConfig} />
)}
{startupConfig?.registrationEnabled && (
<p className="my-4 text-center text-sm font-light text-gray-700 dark:text-white">
{' '}

View file

@ -1,19 +1,23 @@
import React from 'react';
import { useForm } from 'react-hook-form';
import type { TLoginUser, TStartupConfig } from 'librechat-data-provider';
import { useLocalize } from '~/hooks';
import { TLoginUser } from 'librechat-data-provider';
type TLoginFormProps = {
onSubmit: (data: TLoginUser) => void;
startupConfig: TStartupConfig;
};
const LoginForm: React.FC<TLoginFormProps> = ({ onSubmit }) => {
const LoginForm: React.FC<TLoginFormProps> = ({ onSubmit, startupConfig }) => {
const localize = useLocalize();
const {
register,
handleSubmit,
formState: { errors },
} = useForm<TLoginUser>();
if (!startupConfig) {
return null;
}
const renderError = (fieldName: string) => {
const errorMessage = errors[fieldName]?.message;
@ -81,9 +85,11 @@ const LoginForm: React.FC<TLoginFormProps> = ({ onSubmit }) => {
</div>
{renderError('password')}
</div>
<a href="/forgot-password" className="text-sm text-green-500">
{localize('com_auth_password_forgot')}
</a>
{startupConfig.passwordResetEnabled && (
<a href="/forgot-password" className="text-sm text-green-500">
{localize('com_auth_password_forgot')}
</a>
)}
<div className="mt-6">
<button
aria-label="Sign in"

View file

@ -1,11 +1,37 @@
import { useForm } from 'react-hook-form';
import { useState, useEffect } from 'react';
import { useState, ReactNode } from 'react';
import { useOutletContext } from 'react-router-dom';
import { useRequestPasswordResetMutation } from 'librechat-data-provider/react-query';
import type { TRequestPasswordReset, TRequestPasswordResetResponse } from 'librechat-data-provider';
import type { FC } from 'react';
import type { TLoginLayoutContext } from '~/common';
import { useLocalize } from '~/hooks';
const BodyTextWrapper: FC<{ children: ReactNode }> = ({ children }) => {
return (
<div
className="relative mt-4 rounded border border-green-400 bg-green-100 px-4 py-3 text-green-700 dark:bg-green-900 dark:text-white"
role="alert"
>
{children}
</div>
);
};
const ResetPasswordBodyText = () => {
const localize = useLocalize();
return (
<div className="flex flex-col">
{localize('com_auth_reset_password_if_email_exists')}
<span>
<a className="text-sm text-green-500 hover:underline" href="/login">
{localize('com_auth_back_to_login')}
</a>
</span>
</div>
);
};
function RequestPasswordReset() {
const localize = useLocalize();
const {
@ -13,72 +39,39 @@ function RequestPasswordReset() {
handleSubmit,
formState: { errors },
} = useForm<TRequestPasswordReset>();
const [resetLink, setResetLink] = useState<string | undefined>(undefined);
const [bodyText, setBodyText] = useState<React.ReactNode | undefined>(undefined);
const { startupConfig, setError, setHeaderText } = useOutletContext<TLoginLayoutContext>();
const [bodyText, setBodyText] = useState<ReactNode | undefined>(undefined);
const { startupConfig, setHeaderText } = useOutletContext<TLoginLayoutContext>();
const requestPasswordReset = useRequestPasswordResetMutation();
const onSubmit = (data: TRequestPasswordReset) => {
requestPasswordReset.mutate(data, {
onSuccess: (data: TRequestPasswordResetResponse) => {
if (!startupConfig?.emailEnabled) {
setResetLink(data.link);
if (data.link && !startupConfig?.emailEnabled) {
setHeaderText('com_auth_reset_password');
setBodyText(
<span>
{localize('com_auth_click')}{' '}
<a className="text-green-500 hover:underline" href={data.link}>
{localize('com_auth_here')}
</a>{' '}
{localize('com_auth_to_reset_your_password')}
</span>,
);
} else {
setHeaderText('com_auth_reset_password_link_sent');
setBodyText(<ResetPasswordBodyText />);
}
},
onError: () => {
setError('com_auth_error_reset_password');
setTimeout(() => {
setError(null);
}, 5000);
setHeaderText('com_auth_reset_password_link_sent');
setBodyText(<ResetPasswordBodyText />);
},
});
};
useEffect(() => {
if (bodyText) {
return;
}
if (!requestPasswordReset.isSuccess) {
setHeaderText('com_auth_reset_password');
setBodyText(undefined);
return;
}
if (startupConfig?.emailEnabled) {
setHeaderText('com_auth_reset_password_link_sent');
setBodyText(localize('com_auth_reset_password_email_sent'));
return;
}
setHeaderText('com_auth_reset_password');
setBodyText(
<span>
{localize('com_auth_click')}{' '}
<a className="text-green-500 hover:underline" href={resetLink}>
{localize('com_auth_here')}
</a>{' '}
{localize('com_auth_to_reset_your_password')}
</span>,
);
}, [
requestPasswordReset.isSuccess,
startupConfig?.emailEnabled,
resetLink,
localize,
setHeaderText,
bodyText,
]);
if (bodyText) {
return (
<div
className="relative mt-4 rounded border border-green-400 bg-green-100 px-4 py-3 text-green-700 dark:bg-green-900 dark:text-white"
role="alert"
>
{bodyText}
</div>
);
return <BodyTextWrapper>{bodyText}</BodyTextWrapper>;
}
return (

View file

@ -51,7 +51,7 @@ const setup = ({
user: {},
},
},
useGetStartupCongfigReturnValue = mockStartupConfig,
useGetStartupConfigReturnValue = mockStartupConfig,
} = {}) => {
const mockUseLoginUser = jest
.spyOn(mockDataProvider, 'useLoginUserMutation')
@ -64,18 +64,18 @@ const setup = ({
const mockUseGetStartupConfig = jest
.spyOn(mockDataProvider, 'useGetStartupConfig')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useGetStartupCongfigReturnValue);
.mockReturnValue(useGetStartupConfigReturnValue);
const mockUseRefreshTokenMutation = jest
.spyOn(mockDataProvider, 'useRefreshTokenMutation')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useRefreshTokenMutationReturnValue);
const mockUseOutletContext = jest.spyOn(reactRouter, 'useOutletContext').mockReturnValue({
startupConfig: useGetStartupCongfigReturnValue.data,
startupConfig: useGetStartupConfigReturnValue.data,
});
const renderResult = render(
<AuthLayout
startupConfig={useGetStartupCongfigReturnValue.data as TStartupConfig}
isFetching={useGetStartupCongfigReturnValue.isFetching}
startupConfig={useGetStartupConfigReturnValue.data as TStartupConfig}
isFetching={useGetStartupConfigReturnValue.isFetching}
error={null}
startupConfigError={null}
header={'Welcome back'}
@ -161,7 +161,7 @@ test('Navigates to / on successful login', async () => {
isError: false,
isSuccess: true,
},
useGetStartupCongfigReturnValue: {
useGetStartupConfigReturnValue: {
...mockStartupConfig,
data: {
...mockStartupConfig.data,

View file

@ -1,17 +1,103 @@
import { render } from 'test/layout-test-utils';
import userEvent from '@testing-library/user-event';
import * as mockDataProvider from 'librechat-data-provider/react-query';
import type { TStartupConfig } from 'librechat-data-provider';
import Login from '../LoginForm';
jest.mock('librechat-data-provider/react-query');
const mockLogin = jest.fn();
const mockStartupConfig: TStartupConfig = {
socialLogins: ['google', 'facebook', 'openid', 'github', 'discord'],
discordLoginEnabled: true,
facebookLoginEnabled: true,
githubLoginEnabled: true,
googleLoginEnabled: true,
openidLoginEnabled: true,
openidLabel: 'Test OpenID',
openidImageUrl: 'http://test-server.com',
registrationEnabled: true,
emailLoginEnabled: true,
socialLoginEnabled: true,
passwordResetEnabled: true,
serverDomain: 'mock-server',
appTitle: '',
ldapLoginEnabled: false,
emailEnabled: false,
checkBalance: false,
showBirthdayIcon: false,
helpAndFaqURL: '',
};
const setup = ({
useGetUserQueryReturnValue = {
isLoading: false,
isError: false,
data: {},
},
useLoginUserReturnValue = {
isLoading: false,
isError: false,
mutate: jest.fn(),
data: {},
isSuccess: false,
},
useRefreshTokenMutationReturnValue = {
isLoading: false,
isError: false,
mutate: jest.fn(),
data: {
token: 'mock-token',
user: {},
},
},
useGetStartupConfigReturnValue = {
isLoading: false,
isError: false,
data: mockStartupConfig,
},
} = {}) => {
const mockUseLoginUser = jest
.spyOn(mockDataProvider, 'useLoginUserMutation')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useLoginUserReturnValue);
const mockUseGetUserQuery = jest
.spyOn(mockDataProvider, 'useGetUserQuery')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useGetUserQueryReturnValue);
const mockUseGetStartupConfig = jest
.spyOn(mockDataProvider, 'useGetStartupConfig')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useGetStartupConfigReturnValue);
const mockUseRefreshTokenMutation = jest
.spyOn(mockDataProvider, 'useRefreshTokenMutation')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useRefreshTokenMutationReturnValue);
return {
mockUseLoginUser,
mockUseGetUserQuery,
mockUseGetStartupConfig,
mockUseRefreshTokenMutation,
};
};
beforeEach(() => {
setup();
});
test('renders login form', () => {
const { getByLabelText } = render(<Login onSubmit={mockLogin} />);
const { getByLabelText } = render(
<Login onSubmit={mockLogin} startupConfig={mockStartupConfig} />,
);
expect(getByLabelText(/email/i)).toBeInTheDocument();
expect(getByLabelText(/password/i)).toBeInTheDocument();
});
test('submits login form', async () => {
const { getByLabelText, getByRole } = render(<Login onSubmit={mockLogin} />);
const { getByLabelText, getByRole } = render(
<Login onSubmit={mockLogin} startupConfig={mockStartupConfig} />,
);
const emailInput = getByLabelText(/email/i);
const passwordInput = getByLabelText(/password/i);
const submitButton = getByRole('button', { name: /Sign in/i });
@ -24,7 +110,9 @@ test('submits login form', async () => {
});
test('displays validation error messages', async () => {
const { getByLabelText, getByRole, getByText } = render(<Login onSubmit={mockLogin} />);
const { getByLabelText, getByRole, getByText } = render(
<Login onSubmit={mockLogin} startupConfig={mockStartupConfig} />,
);
const emailInput = getByLabelText(/email/i);
const passwordInput = getByLabelText(/password/i);
const submitButton = getByRole('button', { name: /Sign in/i });

View file

@ -50,7 +50,7 @@ const setup = ({
user: {},
},
},
useGetStartupCongfigReturnValue = mockStartupConfig,
useGetStartupConfigReturnValue = mockStartupConfig,
} = {}) => {
const mockUseRegisterUserMutation = jest
.spyOn(mockDataProvider, 'useRegisterUserMutation')
@ -63,18 +63,18 @@ const setup = ({
const mockUseGetStartupConfig = jest
.spyOn(mockDataProvider, 'useGetStartupConfig')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useGetStartupCongfigReturnValue);
.mockReturnValue(useGetStartupConfigReturnValue);
const mockUseRefreshTokenMutation = jest
.spyOn(mockDataProvider, 'useRefreshTokenMutation')
//@ts-ignore - we don't need all parameters of the QueryObserverSuccessResult
.mockReturnValue(useRefreshTokenMutationReturnValue);
const mockUseOutletContext = jest.spyOn(reactRouter, 'useOutletContext').mockReturnValue({
startupConfig: useGetStartupCongfigReturnValue.data,
startupConfig: useGetStartupConfigReturnValue.data,
});
const renderResult = render(
<AuthLayout
startupConfig={useGetStartupCongfigReturnValue.data as TStartupConfig}
isFetching={useGetStartupCongfigReturnValue.isFetching}
startupConfig={useGetStartupConfigReturnValue.data as TStartupConfig}
isFetching={useGetStartupConfigReturnValue.isFetching}
error={null}
startupConfigError={null}
header={'Create your account'}