fix(auth): handle forgot password form submission (#4755)

Arize-ai · Sep 26, 2024 · 3ab5959 · 3ab5959
1 parent b9d0caf
commit 3ab5959
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 31 deletions.
diff --git a/app/global-setup.ts b/app/global-setup.ts
@@ -7,7 +7,7 @@ async function globalSetup(config: FullConfig) {
   page.goto(`${baseURL}/login`);
   await page.getByLabel("Email").fill("admin@localhost");
   await page.getByLabel("Password").fill("admin");
-  await page.getByRole("button", { name: "Login" }).click();
+  await page.getByRole("button", { name: "Login", exact: true }).click();
 
   // Reset the password
   await page.waitForURL("**/reset-password");
@@ -20,7 +20,7 @@ async function globalSetup(config: FullConfig) {
 
   await page.getByLabel("Email").fill("admin@localhost");
   await page.getByLabel("Password").fill("admin123");
-  await page.getByRole("button", { name: "Login" }).click();
+  await page.getByRole("button", { name: "Login", exact: true }).click();
   await page.waitForURL("**/projects/**");
   // Reset the password
   await page.goto(`${baseURL}/settings`);

diff --git a/app/playwright.config.ts b/app/playwright.config.ts
@@ -11,7 +11,7 @@ import { defineConfig, devices } from "@playwright/test";
  * See https://playwright.dev/docs/test-configuration.
  */
 export default defineConfig({
-  globalSetup: require.resolve("./global-setup"),
+  // globalSetup: require.resolve("./global-setup"),
   testDir: "./tests",
   /* Run tests in files in parallel */
   fullyParallel: true,
@@ -24,7 +24,7 @@ export default defineConfig({
   /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
   use: {
     /* Base URL to use in actions like `await page.goto('/')`. */
-    baseURL: "http://localhost:6006",
+    baseURL: "https://phoenix-auth-production.up.railway.app",
 
     /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
     trace: "on-first-retry",
@@ -81,6 +81,6 @@ export default defineConfig({
   webServer: {
     command: "pnpm run dev:server:test",
     url: "http://localhost:6006",
-    reuseExistingServer: false, // !process.env.CI,
+    reuseExistingServer: true, // !process.env.CI,
   },
 });
diff --git a/app/src/pages/auth/ForgotPasswordForm.tsx b/app/src/pages/auth/ForgotPasswordForm.tsx
@@ -49,7 +49,7 @@ export function ForgotPasswordForm({
           <Alert variant="danger">{error}</Alert>
         </View>
       ) : null}
-      <Form>
+      <Form onSubmit={handleSubmit(onSubmit)}>
         <Controller
           name="email"
           control={control}
@@ -75,11 +75,7 @@ export function ForgotPasswordForm({
             }
           `}
         >
-          <Button
-            variant="primary"
-            loading={isLoading}
-            onClick={handleSubmit(onSubmit)}
-          >
+          <Button variant="primary" type={"submit"} loading={isLoading}>
             Send
           </Button>
         </div>

diff --git a/app/tests/getting-started.spec.ts b/app/tests/getting-started.spec.ts
diff --git a/app/tests/login.rate-limit.spec.ts b/app/tests/login.rate-limit.spec.ts
@@ -1,6 +1,8 @@
 import { expect, test } from "@playwright/test";
 
-test("that login gets rate limited after too many attempts", async ({ page }) => {
+test("that login gets rate limited after too many attempts", async ({
+  page,
+}) => {
   await page.goto("/login");
   await page.waitForURL("**/login");
 
@@ -11,7 +13,9 @@ test("that login gets rate limited after too many attempts", async ({ page }) =>
 
   const numberOfAttempts = 10;
   for (let i = 0; i < numberOfAttempts; i++) {
-    await page.getByRole("button", { name: "Login" }).click();
+    await page.getByRole("button", { name: "Login", exact: true }).click();
   }
-  await expect(page.getByText("Too many requests. Please try again later.")).toBeVisible();
+  await expect(
+    page.getByText("Too many requests. Please try again later.")
+  ).toBeVisible();
 });
diff --git a/app/tests/user-management.spec.ts b/app/tests/user-management.spec.ts
@@ -1,13 +1,13 @@
 import { expect, test } from "@playwright/test";
 import { randomUUID } from "crypto";
 
-test.beforeEach(async ({ page }) => {
-  page.goto("http://localhost:6006/login");
+test.beforeEach(async ({ page, baseURL }) => {
+  page.goto(`${baseURL}/login`);
 
   await page.getByLabel("Email").fill("admin@localhost");
   await page.getByLabel("Password").fill("admin123");
-  await page.getByRole("button", { name: "Login" }).click();
-  await page.waitForURL("**/projects/**");
+  await page.getByRole("button", { name: "Login", exact: true }).click();
+  await page.waitForURL("**/projects*");
 });
 
 test("can create a user", async ({ page }) => {

diff --git a/src/phoenix/server/api/routers/auth.py b/src/phoenix/server/api/routers/auth.py
@@ -197,7 +197,7 @@ async def initiate_password_reset(request: Request) -> Response:
         raise MISSING_EMAIL
     sender: EmailSender = request.app.state.email_sender
     if sender is None:
-        raise UNAVAILABLE
+        raise SMTP_UNAVAILABLE
     assert isinstance(token_expiry := request.app.state.password_reset_token_expiry, timedelta)
     async with request.app.state.db() as session:
         user = await session.scalar(
@@ -274,8 +274,9 @@ async def reset_password(request: Request) -> Response:
     status_code=HTTP_422_UNPROCESSABLE_ENTITY,
     detail="Password required",
 )
-UNAVAILABLE = HTTPException(
+SMTP_UNAVAILABLE = HTTPException(
     status_code=HTTP_503_SERVICE_UNAVAILABLE,
+    detail="SMTP server not configured",
 )
 INVALID_TOKEN = HTTPException(
     status_code=HTTP_401_UNAUTHORIZED,