feat(frontend): Finalize auth refactor, routing, and resolve all code review findings

This commit is contained in:
Yunxiao Xu
2026-02-12 00:11:08 -08:00
parent 7fe020f26c
commit 2545f6df13
17 changed files with 335 additions and 216 deletions

View File

@@ -1,7 +1,9 @@
import { useState, useEffect } from "react"
import { Routes, Route } from "react-router-dom"
import { MainLayout } from "./components/layout/MainLayout"
import { LoginForm } from "./components/auth/LoginForm"
import { RegisterForm } from "./components/auth/RegisterForm"
import { AuthCallback } from "./components/auth/AuthCallback"
import { AuthService, type UserResponse } from "./services/auth"
function App() {
@@ -12,48 +14,41 @@ function App() {
useEffect(() => {
const initAuth = async () => {
// Check for token in URL (OIDC callback)
const urlParams = new URLSearchParams(window.location.search)
const tokenFromUrl = urlParams.get("token")
if (tokenFromUrl) {
localStorage.setItem("token", tokenFromUrl)
// Clean URL
window.history.replaceState({}, document.title, "/")
try {
const userData = await AuthService.getMe()
setUser(userData)
setIsAuthenticated(true)
} catch (err: unknown) {
// Not logged in or session expired - this is expected if no cookie
console.log("No active session found", err)
setIsAuthenticated(false)
} finally {
setIsLoading(false)
}
const authStatus = AuthService.isAuthenticated()
setIsAuthenticated(authStatus)
if (authStatus) {
try {
const userData = await AuthService.getMe()
setUser(userData)
} catch (err) {
console.error("Failed to fetch user profile:", err)
AuthService.logout()
setIsAuthenticated(false)
}
}
setIsLoading(false)
}
initAuth()
}, [])
const handleAuthSuccess = async () => {
setIsAuthenticated(true)
try {
const userData = await AuthService.getMe()
setUser(userData)
} catch (err) {
setIsAuthenticated(true)
} catch (err: unknown) {
console.error("Failed to fetch user profile after login:", err)
}
}
const handleLogout = () => {
AuthService.logout()
setIsAuthenticated(false)
setUser(null)
const handleLogout = async () => {
try {
await AuthService.logout()
} catch (err: unknown) {
console.error("Logout failed:", err)
} finally {
setIsAuthenticated(false)
setUser(null)
}
}
if (isLoading) {
@@ -64,44 +59,52 @@ function App() {
)
}
if (!isAuthenticated) {
return (
<div className="min-h-screen flex items-center justify-center bg-background p-4">
{authMode === "login" ? (
<LoginForm
onSuccess={handleAuthSuccess}
onToggleMode={() => setAuthMode("register")}
/>
) : (
<RegisterForm
onSuccess={handleAuthSuccess}
onToggleMode={() => setAuthMode("login")}
/>
)}
</div>
)
}
return (
<MainLayout>
<div className="flex flex-col gap-4">
<div className="flex justify-between items-center">
<div>
<h1 className="text-2xl font-bold">Welcome, {user?.display_name || user?.email || "User"}!</h1>
<p className="text-sm text-muted-foreground">{user?.email}</p>
</div>
<button
onClick={handleLogout}
className="text-sm text-muted-foreground hover:text-primary underline"
>
Logout
</button>
</div>
<p className="text-muted-foreground mt-4">
Select a conversation from the sidebar or start a new one to begin your analysis.
</p>
</div>
</MainLayout>
<Routes>
<Route path="/auth/callback" element={<AuthCallback />} />
<Route
path="*"
element={
!isAuthenticated ? (
<div className="min-h-screen flex items-center justify-center bg-background p-4">
{authMode === "login" ? (
<LoginForm
onSuccess={handleAuthSuccess}
onToggleMode={() => setAuthMode("register")}
/>
) : (
<RegisterForm
onSuccess={handleAuthSuccess}
onToggleMode={() => setAuthMode("login")}
/>
)}
</div>
) : (
<MainLayout>
<div className="flex flex-col gap-4">
<div className="flex justify-between items-center">
<div>
<h1 className="text-2xl font-bold">
Welcome, {user?.display_name || user?.email || "User"}!
</h1>
<p className="text-sm text-muted-foreground">{user?.email}</p>
</div>
<button
onClick={handleLogout}
className="text-sm text-muted-foreground hover:text-primary underline"
>
Logout
</button>
</div>
<p className="text-muted-foreground mt-4">
Select a conversation from the sidebar or start a new one to begin your analysis.
</p>
</div>
</MainLayout>
)
}
/>
</Routes>
)
}

View File

@@ -0,0 +1,30 @@
import { useEffect } from "react"
import { useNavigate } from "react-router-dom"
import { AuthService } from "@/services/auth"
export function AuthCallback() {
const navigate = useNavigate()
useEffect(() => {
const verifyAuth = async () => {
try {
// The cookie should have been set by the backend redirect
await AuthService.getMe()
// Success - go to home. We use window.location.href to ensure a clean reload of App state
window.location.href = "/"
} catch (err) {
console.error("Auth callback verification failed:", err)
navigate("/?error=auth_failed", { replace: true })
}
}
verifyAuth()
}, [navigate])
return (
<div className="min-h-screen flex flex-col items-center justify-center bg-background">
<div className="animate-spin rounded-full h-8 w-8 border-b-2 border-primary mb-4"></div>
<p className="text-muted-foreground">Completing login...</p>
</div>
)
}

View File

@@ -15,6 +15,7 @@ import { Input } from "@/components/ui/input"
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"
import { useState } from "react"
import { Separator } from "@/components/ui/separator"
import axios from "axios"
interface LoginFormProps {
onSuccess: () => void
@@ -39,8 +40,12 @@ export function LoginForm({ onSuccess, onToggleMode }: LoginFormProps) {
try {
await AuthService.login(values.email, values.password)
onSuccess()
} catch (err: any) {
setError(err.response?.data?.detail || "Login failed. Please check your credentials.")
} catch (err: unknown) {
if (axios.isAxiosError(err)) {
setError(err.response?.data?.detail || "Login failed. Please check your credentials.")
} else {
setError("An unexpected error occurred.")
}
} finally {
setIsLoading(false)
}
@@ -49,7 +54,8 @@ export function LoginForm({ onSuccess, onToggleMode }: LoginFormProps) {
const handleOIDCLogin = async () => {
try {
await AuthService.loginWithOIDC()
} catch (err) {
} catch (err: unknown) {
console.error("SSO Login failed:", err)
setError("Failed to initialize SSO login.")
}
}

View File

@@ -14,6 +14,7 @@ import {
import { Input } from "@/components/ui/input"
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"
import { useState } from "react"
import axios from "axios"
interface RegisterFormProps {
onSuccess: () => void
@@ -38,12 +39,14 @@ export function RegisterForm({ onSuccess, onToggleMode }: RegisterFormProps) {
setError(null)
try {
await AuthService.register(values.email, values.password)
// Automatically login after registration or just switch to login?
// For now, let's just switch to login or notify success
alert("Registration successful! Please login.")
onToggleMode()
} catch (err: any) {
setError(err.response?.data?.detail || "Registration failed. Please try again.")
// Since backend sets cookie on registration now, we can just call onSuccess
onSuccess()
} catch (err: unknown) {
if (axios.isAxiosError(err)) {
setError(err.response?.data?.detail || "Registration failed. Please try again.")
} else {
setError("An unexpected error occurred.")
}
} finally {
setIsLoading(false)
}

View File

@@ -0,0 +1,35 @@
import { cva } from "class-variance-authority"
export const buttonVariants = cva(
"inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive",
{
variants: {
variant: {
default: "bg-primary text-primary-foreground hover:bg-primary/90",
destructive:
"bg-destructive text-white hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60",
outline:
"border bg-background shadow-xs hover:bg-accent hover:text-accent-foreground dark:bg-input/30 dark:border-input dark:hover:bg-input/50",
secondary:
"bg-secondary text-secondary-foreground hover:bg-secondary/80",
ghost:
"hover:bg-accent hover:text-accent-foreground dark:hover:bg-accent/50",
link: "text-primary underline-offset-4 hover:underline",
},
size: {
default: "h-9 px-4 py-2 has-[>svg]:px-3",
xs: "h-6 gap-1 rounded-md px-2 text-xs has-[>svg]:px-1.5 [&_svg:not([class*='size-'])]:size-3",
sm: "h-8 rounded-md gap-1.5 px-3 has-[>svg]:px-2.5",
lg: "h-10 rounded-md px-6 has-[>svg]:px-4",
icon: "size-9",
"icon-xs": "size-6 rounded-md [&_svg:not([class*='size-'])]:size-3",
"icon-sm": "size-8",
"icon-lg": "size-10",
},
},
defaultVariants: {
variant: "default",
size: "default",
},
}
)

View File

@@ -1,42 +1,9 @@
import * as React from "react"
import { cva, type VariantProps } from "class-variance-authority"
import { type VariantProps } from "class-variance-authority"
import { Slot } from "radix-ui"
import { cn } from "@/lib/utils"
const buttonVariants = cva(
"inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive",
{
variants: {
variant: {
default: "bg-primary text-primary-foreground hover:bg-primary/90",
destructive:
"bg-destructive text-white hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60",
outline:
"border bg-background shadow-xs hover:bg-accent hover:text-accent-foreground dark:bg-input/30 dark:border-input dark:hover:bg-input/50",
secondary:
"bg-secondary text-secondary-foreground hover:bg-secondary/80",
ghost:
"hover:bg-accent hover:text-accent-foreground dark:hover:bg-accent/50",
link: "text-primary underline-offset-4 hover:underline",
},
size: {
default: "h-9 px-4 py-2 has-[>svg]:px-3",
xs: "h-6 gap-1 rounded-md px-2 text-xs has-[>svg]:px-1.5 [&_svg:not([class*='size-'])]:size-3",
sm: "h-8 rounded-md gap-1.5 px-3 has-[>svg]:px-2.5",
lg: "h-10 rounded-md px-6 has-[>svg]:px-4",
icon: "size-9",
"icon-xs": "size-6 rounded-md [&_svg:not([class*='size-'])]:size-3",
"icon-sm": "size-8",
"icon-lg": "size-10",
},
},
defaultVariants: {
variant: "default",
size: "default",
},
}
)
import { buttonVariants } from "./button-variants"
function Button({
className,
@@ -61,4 +28,4 @@ function Button({
)
}
export { Button, buttonVariants }
export { Button }

View File

@@ -0,0 +1,44 @@
import * as React from "react"
import { useFormContext, useFormState, type FieldPath, type FieldValues } from "react-hook-form"
export type FormFieldContextValue<
TFieldValues extends FieldValues = FieldValues,
TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,
> = {
name: TName
}
export const FormFieldContext = React.createContext<FormFieldContextValue>(
{} as FormFieldContextValue
)
export type FormItemContextValue = {
id: string
}
export const FormItemContext = React.createContext<FormItemContextValue>(
{} as FormItemContextValue
)
export const useFormField = () => {
const fieldContext = React.useContext(FormFieldContext)
const itemContext = React.useContext(FormItemContext)
const { getFieldState } = useFormContext()
const formState = useFormState({ name: fieldContext.name })
const fieldState = getFieldState(fieldContext.name, formState)
if (!fieldContext) {
throw new Error("useFormField should be used within <FormField>")
}
const { id } = itemContext
return {
id,
name: fieldContext.name,
formItemId: `${id}-form-item`,
formDescriptionId: `${id}-form-item-description`,
formMessageId: `${id}-form-item-message`,
...fieldState,
}
}

View File

@@ -6,8 +6,6 @@ import { Slot } from "radix-ui"
import {
Controller,
FormProvider,
useFormContext,
useFormState,
type ControllerProps,
type FieldPath,
type FieldValues,
@@ -15,20 +13,10 @@ import {
import { cn } from "@/lib/utils"
import { Label } from "@/components/ui/label"
import { FormFieldContext, FormItemContext, useFormField } from "./form-context"
const Form = FormProvider
type FormFieldContextValue<
TFieldValues extends FieldValues = FieldValues,
TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,
> = {
name: TName
}
const FormFieldContext = React.createContext<FormFieldContextValue>(
{} as FormFieldContextValue
)
const FormField = <
TFieldValues extends FieldValues = FieldValues,
TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,
@@ -42,37 +30,6 @@ const FormField = <
)
}
const useFormField = () => {
const fieldContext = React.useContext(FormFieldContext)
const itemContext = React.useContext(FormItemContext)
const { getFieldState } = useFormContext()
const formState = useFormState({ name: fieldContext.name })
const fieldState = getFieldState(fieldContext.name, formState)
if (!fieldContext) {
throw new Error("useFormField should be used within <FormField>")
}
const { id } = itemContext
return {
id,
name: fieldContext.name,
formItemId: `${id}-form-item`,
formDescriptionId: `${id}-form-item-description`,
formMessageId: `${id}-form-item-message`,
...fieldState,
}
}
type FormItemContextValue = {
id: string
}
const FormItemContext = React.createContext<FormItemContextValue>(
{} as FormItemContextValue
)
function FormItem({ className, ...props }: React.ComponentProps<"div">) {
const id = React.useId()
@@ -156,7 +113,6 @@ function FormMessage({ className, ...props }: React.ComponentProps<"p">) {
}
export {
useFormField,
Form,
FormItem,
FormLabel,

View File

@@ -0,0 +1,22 @@
import * as React from "react"
export type SidebarContextProps = {
state: "expanded" | "collapsed"
open: boolean
setOpen: (open: boolean) => void
openMobile: boolean
setOpenMobile: (open: boolean) => void
isMobile: boolean
toggleSidebar: () => void
}
export const SidebarContext = React.createContext<SidebarContextProps | null>(null)
export function useSidebar() {
const context = React.useContext(SidebarContext)
if (!context) {
throw new Error("useSidebar must be used within a SidebarProvider.")
}
return context
}

View File

@@ -1,7 +1,6 @@
"use client"
import * as React from "react"
import { cva, type VariantProps } from "class-variance-authority"
import { PanelLeftIcon } from "lucide-react"
import { Slot } from "radix-ui"
@@ -24,6 +23,7 @@ import {
TooltipProvider,
TooltipTrigger,
} from "@/components/ui/tooltip"
import { SidebarContext, useSidebar, type SidebarContextProps } from "./sidebar-context"
const SIDEBAR_COOKIE_NAME = "sidebar_state"
const SIDEBAR_COOKIE_MAX_AGE = 60 * 60 * 24 * 7
@@ -32,27 +32,6 @@ const SIDEBAR_WIDTH_MOBILE = "18rem"
const SIDEBAR_WIDTH_ICON = "3rem"
const SIDEBAR_KEYBOARD_SHORTCUT = "b"
type SidebarContextProps = {
state: "expanded" | "collapsed"
open: boolean
setOpen: (open: boolean) => void
openMobile: boolean
setOpenMobile: (open: boolean) => void
isMobile: boolean
toggleSidebar: () => void
}
const SidebarContext = React.createContext<SidebarContextProps | null>(null)
function useSidebar() {
const context = React.useContext(SidebarContext)
if (!context) {
throw new Error("useSidebar must be used within a SidebarProvider.")
}
return context
}
function SidebarProvider({
defaultOpen = true,
open: openProp,
@@ -473,28 +452,6 @@ function SidebarMenuItem({ className, ...props }: React.ComponentProps<"li">) {
)
}
const sidebarMenuButtonVariants = cva(
"peer/menu-button flex w-full items-center gap-2 overflow-hidden rounded-md p-2 text-left text-sm outline-hidden ring-sidebar-ring transition-[width,height,padding] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 group-has-data-[sidebar=menu-action]/menu-item:pr-8 aria-disabled:pointer-events-none aria-disabled:opacity-50 data-[active=true]:bg-sidebar-accent data-[active=true]:font-medium data-[active=true]:text-sidebar-accent-foreground data-[state=open]:hover:bg-sidebar-accent data-[state=open]:hover:text-sidebar-accent-foreground group-data-[collapsible=icon]:size-8! group-data-[collapsible=icon]:p-2! [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0",
{
variants: {
variant: {
default: "hover:bg-sidebar-accent hover:text-sidebar-accent-foreground",
outline:
"bg-background shadow-[0_0_0_1px_hsl(var(--sidebar-border))] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground hover:shadow-[0_0_0_1px_hsl(var(--sidebar-accent))]",
},
size: {
default: "h-8 text-sm",
sm: "h-7 text-xs",
lg: "h-12 text-sm group-data-[collapsible=icon]:p-0!",
},
},
defaultVariants: {
variant: "default",
size: "default",
},
}
)
function SidebarMenuButton({
asChild = false,
isActive = false,
@@ -507,7 +464,9 @@ function SidebarMenuButton({
asChild?: boolean
isActive?: boolean
tooltip?: string | React.ComponentProps<typeof TooltipContent>
} & VariantProps<typeof sidebarMenuButtonVariants>) {
variant?: "default" | "outline"
size?: "default" | "sm" | "lg"
}) {
const Comp = asChild ? Slot.Root : "button"
const { isMobile, state } = useSidebar()
@@ -517,7 +476,15 @@ function SidebarMenuButton({
data-sidebar="menu-button"
data-size={size}
data-active={isActive}
className={cn(sidebarMenuButtonVariants({ variant, size }), className)}
className={cn(
"peer/menu-button flex w-full items-center gap-2 overflow-hidden rounded-md p-2 text-left text-sm outline-hidden ring-sidebar-ring transition-[width,height,padding] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground focus-visible:ring-2 active:bg-sidebar-accent active:text-sidebar-accent-foreground disabled:pointer-events-none disabled:opacity-50 group-has-data-[sidebar=menu-action]/menu-item:pr-8 aria-disabled:pointer-events-none aria-disabled:opacity-50 data-[active=true]:bg-sidebar-accent data-[active=true]:font-medium data-[active=true]:text-sidebar-accent-foreground data-[state=open]:hover:bg-sidebar-accent data-[state=open]:hover:text-sidebar-accent-foreground group-data-[collapsible=icon]:size-8! group-data-[collapsible=icon]:p-2! [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0",
variant === "default" && "hover:bg-sidebar-accent hover:text-sidebar-accent-foreground",
variant === "outline" && "bg-background shadow-[0_0_0_1px_hsl(var(--sidebar-border))] hover:bg-sidebar-accent hover:text-sidebar-accent-foreground hover:shadow-[0_0_0_1px_hsl(var(--sidebar-accent))]",
size === "default" && "h-8 text-sm",
size === "sm" && "h-7 text-xs",
size === "lg" && "h-12 text-sm group-data-[collapsible=icon]:p-0!",
className
)}
{...props}
/>
)
@@ -606,10 +573,8 @@ function SidebarMenuSkeleton({
}: React.ComponentProps<"div"> & {
showIcon?: boolean
}) {
// Random width between 50 to 90%.
const width = React.useMemo(() => {
return `${Math.floor(Math.random() * 40) + 50}%`
}, [])
// Use a fixed width to avoid Math.random in render path which can cause hydrations issues and lint warnings.
const width = "70%"
return (
<div
@@ -674,7 +639,7 @@ function SidebarMenuSubButton({
...props
}: React.ComponentProps<"a"> & {
asChild?: boolean
size?: "sm" | "md"
size?: "sm" | "md" | "lg"
isActive?: boolean
}) {
const Comp = asChild ? Slot.Root : "a"
@@ -722,5 +687,4 @@ export {
SidebarRail,
SidebarSeparator,
SidebarTrigger,
useSidebar,
}

View File

@@ -3,11 +3,14 @@ import { createRoot } from 'react-dom/client'
import './index.css'
import App from './App.tsx'
import { TooltipProvider } from "@/components/ui/tooltip"
import { BrowserRouter } from "react-router-dom"
createRoot(document.getElementById('root')!).render(
<StrictMode>
<TooltipProvider>
<App />
</TooltipProvider>
<BrowserRouter>
<TooltipProvider>
<App />
</TooltipProvider>
</BrowserRouter>
</StrictMode>,
)

View File

@@ -7,4 +7,18 @@ const api = axios.create({
withCredentials: true, // Crucial for HttpOnly cookies
})
// Add a response interceptor to handle 401s
api.interceptors.response.use(
(response) => response,
(error) => {
if (error.response?.status === 401) {
// Unauthorized - session likely expired
// We can't use useNavigate here as it's not a React component
// But we can redirect to home which will trigger the login view in App.tsx
window.location.href = "/"
}
return Promise.reject(error)
}
)
export default api

View File

@@ -58,7 +58,10 @@ describe("AuthService", () => {
email: "test@example.com",
display_name: "Test User"
}
mockedApi.get.mockResolvedValueOnce({ data: mockUser })
mockedApi.get.mockResolvedValueOnce({
data: mockUser,
headers: { "content-type": "application/json" }
})
const result = await AuthService.getMe()
@@ -66,6 +69,15 @@ describe("AuthService", () => {
expect(result).toEqual(mockUser)
})
it("throws error on invalid non-JSON response", async () => {
mockedApi.get.mockResolvedValueOnce({
data: "<html>Some fallback</html>",
headers: { "content-type": "text/html" }
})
await expect(AuthService.getMe()).rejects.toThrow("Invalid response from server")
})
it("logs out and calls backend", async () => {
mockedApi.post.mockResolvedValueOnce({ data: { detail: "success" } })
await AuthService.logout()

View File

@@ -38,6 +38,11 @@ export const AuthService = {
async getMe(): Promise<UserResponse> {
const response = await api.get<UserResponse>("/auth/me")
// Double check that we got JSON and not an HTML fallback
const contentType = response.headers["content-type"]
if (contentType && !contentType.includes("application/json")) {
throw new Error("Invalid response from server")
}
return response.data
},