[PasswordResetRequest] Eliminate info leak (#2410)

Backend/Controllers/UserController.cs

@@ -43,7 +43,8 @@ public async Task<IActionResult> ResetPasswordRequest([FromBody, BindRequired] P
 
             if (user is null)
             {
-                return NotFound(data.EmailOrUsername);
+                // Return Ok to avoid revealing to the frontend whether the user exists.
+                return Ok();
             }
 
             // Create password reset.

public/locales/en/translation.json

@@ -74,10 +74,10 @@
     "emailOrUsername": "Email OR Username",
     "resetRequestTitle": "Reset Password Request",
     "resetTitle": "Reset Password",
-    "resetRequestInstructions": "We will send a one time reset link for your account to your email",
+    "resetRequestInstructions": "We will send a one-time reset link for your account to your email.",
     "submit": "Submit",
     "resetFail": "Password reset error",
-    "notFoundError": "No match found",
+    "resetDone": "If you have correctly entered your email or username, a password reset link has been sent to your email address.",
     "backToLogin": "Back To Login"
   },
   "userMenu": {

src/components/Login/SignUpPage/SignUpComponent.tsx

@@ -298,6 +298,7 @@ export class SignUp extends React.Component<SignUpProps, SignUpState> {
                     onClick={() => {
                       router.navigate(Path.Login);
                     }}
+                    variant="outlined"
                   >
                     {this.props.t("login.backToLogin")}
                   </Button>

src/components/PasswordReset/Request.tsx

@@ -1,92 +1,103 @@
-import { Card, Grid, TextField, Typography } from "@mui/material";
+import { Button, Card, Grid, TextField, Typography } from "@mui/material";
 import { FormEvent, ReactElement, useState } from "react";
 import { useTranslation } from "react-i18next";
 import { useNavigate } from "react-router-dom";
 
-import { isEmailTaken, isUsernameTaken, resetPasswordRequest } from "backend";
+import { resetPasswordRequest } from "backend";
 import { LoadingDoneButton } from "components/Buttons";
-import { useAppDispatch } from "types/hooks";
 import { Path } from "types/path";
 
+export enum PasswordRequestIds {
+  ButtonLogin = "password-request-login",
+  ButtonSubmit = "password-request-submit",
+  FieldEmailOrUsername = "password-request-text",
+}
+
 export default function ResetRequest(): ReactElement {
-  const dispatch = useAppDispatch();
   const [emailOrUsername, setEmailOrUsername] = useState("");
   const [isDone, setIsDone] = useState(false);
+  const [isError, setIsError] = useState(false);
   const [isLoading, setIsLoading] = useState(false);
-  const [isNotFound, setIsNotFound] = useState(false);
+
   const { t } = useTranslation();
   const navigate = useNavigate();
 
   const onSubmit = (event: FormEvent<HTMLElement>): void => {
     event.preventDefault();
+    setIsError(false);
     setIsLoading(true);
-    setTimeout(() => tryResetRequest(), 1000);
-  };
-
-  const tryResetRequest = async (): Promise<void> => {
-    setIsLoading(true);
-    const exists =
-      (await isEmailTaken(emailOrUsername)) ||
-      (await isUsernameTaken(emailOrUsername));
-    if (exists) {
-      await resetPasswordRequest(emailOrUsername);
-      setIsDone(true);
-      setTimeout(() => navigate(Path.Login), 1000);
-    } else {
-      setIsNotFound(true);
-    }
-    setIsLoading(false);
-  };
-
-  const setTextField = (text: string): void => {
-    setEmailOrUsername(text);
-    setIsNotFound(false);
+    resetPasswordRequest(emailOrUsername)
+      .then(() => {
+        setIsDone(true);
+      })
+      .catch(() => {
+        setIsError(true);
+        setIsLoading(false);
+      });
   };
 
   return (
-    <div>
-      <Grid container justifyContent="center">
-        <Card style={{ padding: 10, width: 450 }}>
-          <Typography variant="h5" align="center">
-            {t("passwordReset.resetRequestTitle")}
-          </Typography>
-          <Typography variant="subtitle1" align="center">
-            {t("passwordReset.resetRequestInstructions")}
-          </Typography>
-          <form onSubmit={onSubmit}>
+    <Grid container justifyContent="center">
+      <Card style={{ padding: 10, width: 450 }}>
+        <Typography align="center" variant="h5">
+          {t("passwordReset.resetRequestTitle")}
+        </Typography>
+        {isDone ? (
+          <>
+            <Typography>{t("passwordReset.resetDone")}</Typography>
             <Grid item>
-              <TextField
-                id="password-reset-request-text"
-                required
-                type="text"
+              <Button
+                data-testid={PasswordRequestIds.ButtonLogin}
+                id={PasswordRequestIds.ButtonLogin}
+                onClick={() => navigate(Path.Login)}
+                type="button"
                 variant="outlined"
-                label={t("passwordReset.emailOrUsername")}
-                value={emailOrUsername}
-                style={{ width: "100%" }}
-                error={isNotFound}
-                helperText={isNotFound && t("passwordReset.notFoundError")}
-                margin="normal"
-                onChange={(e) => setTextField(e.target.value)}
-              />
-            </Grid>
-            <Grid item>
-              <LoadingDoneButton
-                disabled={!emailOrUsername}
-                loading={isLoading}
-                done={isDone}
-                buttonProps={{
-                  onClick: () => onSubmit,
-                  variant: "contained",
-                  color: "primary",
-                  id: "password-reset-request",
-                }}
               >
-                {t("passwordReset.submit")}
-              </LoadingDoneButton>
+                {t("login.backToLogin")}
+              </Button>
             </Grid>
-          </form>
-        </Card>
-      </Grid>
-    </div>
+          </>
+        ) : (
+          <>
+            <Typography align="center" variant="subtitle1">
+              {t("passwordReset.resetRequestInstructions")}
+            </Typography>
+            <form onSubmit={onSubmit}>
+              <Grid item>
+                <TextField
+                  helperText={isError && t("passwordReset.resetFail")}
+                  id={PasswordRequestIds.FieldEmailOrUsername}
+                  inputProps={{
+                    "data-testid": PasswordRequestIds.FieldEmailOrUsername,
+                  }}
+                  label={t("passwordReset.emailOrUsername")}
+                  margin="normal"
+                  onChange={(e) => setEmailOrUsername(e.target.value)}
+                  required
+                  type="text"
+                  style={{ width: "100%" }}
+                  value={emailOrUsername}
+                  variant="outlined"
+                />
+              </Grid>
+              <Grid item>
+                <LoadingDoneButton
+                  buttonProps={{
+                    color: "primary",
+                    id: PasswordRequestIds.ButtonSubmit,
+                    onClick: () => onSubmit,
+                    variant: "contained",
+                  }}
+                  disabled={!emailOrUsername}
+                  loading={isLoading}
+                >
+                  {t("passwordReset.submit")}
+                </LoadingDoneButton>
+              </Grid>
+            </form>
+          </>
+        )}
+      </Card>
+    </Grid>
   );
 }

src/components/PasswordReset/tests/Request.test.tsx

@@ -0,0 +1,79 @@
+import "@testing-library/jest-dom";
+import { act, cleanup, render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+
+import "tests/reactI18nextMock";
+import ResetRequest, {
+  PasswordRequestIds,
+} from "components/PasswordReset/Request";
+
+jest.mock("react-router-dom", () => ({
+  useNavigate: jest.fn(),
+}));
+
+jest.mock("backend", () => ({
+  resetPasswordRequest: (...args: any[]) => mockResetPasswordRequest(...args),
+}));
+
+const mockResetPasswordRequest = jest.fn();
+
+const setupMocks = (): void => {
+  mockResetPasswordRequest.mockResolvedValue(true);
+};
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  setupMocks();
+});
+
+afterEach(cleanup);
+
+const renderUserSettings = async (): Promise<void> => {
+  await act(async () => {
+    render(<ResetRequest />);
+  });
+};
+
+describe("ResetRequest", () => {
+  it("has disabled submit button until something is typed", async () => {
+    // Setup
+    const agent = userEvent.setup();
+    await renderUserSettings();
+
+    // Before
+    const button = screen.getByRole("button");
+    expect(button).toBeDisabled();
+
+    // Act
+    const field = screen.getByTestId(PasswordRequestIds.FieldEmailOrUsername);
+    await act(async () => {
+      await agent.type(field, "a");
+    });
+
+    // After
+    expect(button).toBeEnabled();
+  });
+
+  it("after submit, removes text field and submit button and reveals login button", async () => {
+    // Setup
+    const agent = userEvent.setup();
+    await renderUserSettings();
+
+    // Before
+    expect(screen.queryAllByRole("textbox")).toHaveLength(1);
+    expect(screen.queryAllByRole("button")).toHaveLength(1);
+    expect(screen.queryByTestId(PasswordRequestIds.ButtonLogin)).toBeNull();
+
+    // Act
+    const field = screen.getByTestId(PasswordRequestIds.FieldEmailOrUsername);
+    await act(async () => {
+      await agent.type(field, "a");
+      await agent.click(screen.getByRole("button"));
+    });
+
+    // After
+    expect(screen.queryAllByRole("textbox")).toHaveLength(0);
+    expect(screen.queryAllByRole("button")).toHaveLength(1);
+    expect(screen.queryByTestId(PasswordRequestIds.ButtonLogin)).toBeTruthy();
+  });
+});