first round of review
This commit is contained in:
parent
7ebfb7a2dd
commit
da5a0d03eb
14 changed files with 362 additions and 244 deletions
|
|
@ -2,6 +2,8 @@
|
|||
|
||||
import { createContext, useContext, useState, useEffect, ReactNode } from "react";
|
||||
|
||||
const API_URL = process.env.NEXT_PUBLIC_API_URL || "http://localhost:8000";
|
||||
|
||||
interface User {
|
||||
id: number;
|
||||
email: string;
|
||||
|
|
@ -9,54 +11,43 @@ interface User {
|
|||
|
||||
interface AuthContextType {
|
||||
user: User | null;
|
||||
token: string | null;
|
||||
isLoading: boolean;
|
||||
login: (email: string, password: string) => Promise<void>;
|
||||
register: (email: string, password: string) => Promise<void>;
|
||||
logout: () => void;
|
||||
logout: () => Promise<void>;
|
||||
}
|
||||
|
||||
const AuthContext = createContext<AuthContextType | null>(null);
|
||||
|
||||
export function AuthProvider({ children }: { children: ReactNode }) {
|
||||
const [user, setUser] = useState<User | null>(null);
|
||||
const [token, setToken] = useState<string | null>(null);
|
||||
const [isLoading, setIsLoading] = useState(true);
|
||||
|
||||
useEffect(() => {
|
||||
const storedToken = localStorage.getItem("token");
|
||||
if (storedToken) {
|
||||
setToken(storedToken);
|
||||
fetchUser(storedToken);
|
||||
} else {
|
||||
setIsLoading(false);
|
||||
}
|
||||
checkAuth();
|
||||
}, []);
|
||||
|
||||
const fetchUser = async (authToken: string) => {
|
||||
const checkAuth = async () => {
|
||||
try {
|
||||
const res = await fetch("http://localhost:8000/api/auth/me", {
|
||||
headers: { Authorization: `Bearer ${authToken}` },
|
||||
const res = await fetch(`${API_URL}/api/auth/me`, {
|
||||
credentials: "include",
|
||||
});
|
||||
if (res.ok) {
|
||||
const userData = await res.json();
|
||||
setUser(userData);
|
||||
} else {
|
||||
localStorage.removeItem("token");
|
||||
setToken(null);
|
||||
}
|
||||
} catch {
|
||||
localStorage.removeItem("token");
|
||||
setToken(null);
|
||||
// Not authenticated
|
||||
} finally {
|
||||
setIsLoading(false);
|
||||
}
|
||||
};
|
||||
|
||||
const login = async (email: string, password: string) => {
|
||||
const res = await fetch("http://localhost:8000/api/auth/login", {
|
||||
const res = await fetch(`${API_URL}/api/auth/login`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
credentials: "include",
|
||||
body: JSON.stringify({ email, password }),
|
||||
});
|
||||
|
||||
|
|
@ -65,16 +56,15 @@ export function AuthProvider({ children }: { children: ReactNode }) {
|
|||
throw new Error(error.detail || "Login failed");
|
||||
}
|
||||
|
||||
const data = await res.json();
|
||||
localStorage.setItem("token", data.access_token);
|
||||
setToken(data.access_token);
|
||||
setUser(data.user);
|
||||
const userData = await res.json();
|
||||
setUser(userData);
|
||||
};
|
||||
|
||||
const register = async (email: string, password: string) => {
|
||||
const res = await fetch("http://localhost:8000/api/auth/register", {
|
||||
const res = await fetch(`${API_URL}/api/auth/register`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
credentials: "include",
|
||||
body: JSON.stringify({ email, password }),
|
||||
});
|
||||
|
||||
|
|
@ -83,20 +73,20 @@ export function AuthProvider({ children }: { children: ReactNode }) {
|
|||
throw new Error(error.detail || "Registration failed");
|
||||
}
|
||||
|
||||
const data = await res.json();
|
||||
localStorage.setItem("token", data.access_token);
|
||||
setToken(data.access_token);
|
||||
setUser(data.user);
|
||||
const userData = await res.json();
|
||||
setUser(userData);
|
||||
};
|
||||
|
||||
const logout = () => {
|
||||
localStorage.removeItem("token");
|
||||
setToken(null);
|
||||
const logout = async () => {
|
||||
await fetch(`${API_URL}/api/auth/logout`, {
|
||||
method: "POST",
|
||||
credentials: "include",
|
||||
});
|
||||
setUser(null);
|
||||
};
|
||||
|
||||
return (
|
||||
<AuthContext.Provider value={{ user, token, isLoading, login, register, logout }}>
|
||||
<AuthContext.Provider value={{ user, isLoading, login, register, logout }}>
|
||||
{children}
|
||||
</AuthContext.Provider>
|
||||
);
|
||||
|
|
@ -109,4 +99,3 @@ export function useAuth() {
|
|||
}
|
||||
return context;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,14 +12,12 @@ vi.mock("next/navigation", () => ({
|
|||
|
||||
// Default mock values
|
||||
let mockUser: { id: number; email: string } | null = { id: 1, email: "test@example.com" };
|
||||
let mockToken: string | null = "valid-token";
|
||||
let mockIsLoading = false;
|
||||
const mockLogout = vi.fn();
|
||||
|
||||
vi.mock("./auth-context", () => ({
|
||||
useAuth: () => ({
|
||||
user: mockUser,
|
||||
token: mockToken,
|
||||
isLoading: mockIsLoading,
|
||||
logout: mockLogout,
|
||||
}),
|
||||
|
|
@ -29,7 +27,6 @@ beforeEach(() => {
|
|||
vi.clearAllMocks();
|
||||
// Reset to authenticated state
|
||||
mockUser = { id: 1, email: "test@example.com" };
|
||||
mockToken = "valid-token";
|
||||
mockIsLoading = false;
|
||||
});
|
||||
|
||||
|
|
@ -64,14 +61,18 @@ describe("Home - Authenticated", () => {
|
|||
expect(screen.getByText("Sign out")).toBeDefined();
|
||||
});
|
||||
|
||||
test("clicking sign out calls logout", async () => {
|
||||
test("clicking sign out calls logout and redirects", async () => {
|
||||
vi.spyOn(global, "fetch").mockResolvedValue({
|
||||
json: () => Promise.resolve({ value: 42 }),
|
||||
} as Response);
|
||||
|
||||
render(<Home />);
|
||||
fireEvent.click(screen.getByText("Sign out"));
|
||||
expect(mockLogout).toHaveBeenCalled();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockLogout).toHaveBeenCalled();
|
||||
expect(mockPush).toHaveBeenCalledWith("/login");
|
||||
});
|
||||
});
|
||||
|
||||
test("renders counter value after fetch", async () => {
|
||||
|
|
@ -85,7 +86,7 @@ describe("Home - Authenticated", () => {
|
|||
});
|
||||
});
|
||||
|
||||
test("fetches counter with auth header", async () => {
|
||||
test("fetches counter with credentials", async () => {
|
||||
const fetchSpy = vi.spyOn(global, "fetch").mockResolvedValue({
|
||||
json: () => Promise.resolve({ value: 0 }),
|
||||
} as Response);
|
||||
|
|
@ -96,7 +97,7 @@ describe("Home - Authenticated", () => {
|
|||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
"http://localhost:8000/api/counter",
|
||||
expect.objectContaining({
|
||||
headers: { Authorization: "Bearer valid-token" },
|
||||
credentials: "include",
|
||||
})
|
||||
);
|
||||
});
|
||||
|
|
@ -111,7 +112,7 @@ describe("Home - Authenticated", () => {
|
|||
expect(screen.getByText("Increment")).toBeDefined();
|
||||
});
|
||||
|
||||
test("clicking increment button calls API with auth header", async () => {
|
||||
test("clicking increment button calls API with credentials", async () => {
|
||||
const fetchSpy = vi
|
||||
.spyOn(global, "fetch")
|
||||
.mockResolvedValueOnce({ json: () => Promise.resolve({ value: 0 }) } as Response)
|
||||
|
|
@ -127,7 +128,7 @@ describe("Home - Authenticated", () => {
|
|||
"http://localhost:8000/api/counter/increment",
|
||||
expect.objectContaining({
|
||||
method: "POST",
|
||||
headers: { Authorization: "Bearer valid-token" },
|
||||
credentials: "include",
|
||||
})
|
||||
);
|
||||
});
|
||||
|
|
@ -149,7 +150,6 @@ describe("Home - Authenticated", () => {
|
|||
describe("Home - Unauthenticated", () => {
|
||||
test("redirects to login when not authenticated", async () => {
|
||||
mockUser = null;
|
||||
mockToken = null;
|
||||
|
||||
render(<Home />);
|
||||
|
||||
|
|
@ -160,16 +160,14 @@ describe("Home - Unauthenticated", () => {
|
|||
|
||||
test("returns null when not authenticated", () => {
|
||||
mockUser = null;
|
||||
mockToken = null;
|
||||
|
||||
const { container } = render(<Home />);
|
||||
// Should render nothing (just redirects)
|
||||
expect(container.querySelector("main")).toBeNull();
|
||||
});
|
||||
|
||||
test("does not fetch counter when no token", () => {
|
||||
test("does not fetch counter when no user", () => {
|
||||
mockUser = null;
|
||||
mockToken = null;
|
||||
const fetchSpy = vi.spyOn(global, "fetch");
|
||||
|
||||
render(<Home />);
|
||||
|
|
@ -182,7 +180,6 @@ describe("Home - Loading State", () => {
|
|||
test("does not redirect while loading", () => {
|
||||
mockIsLoading = true;
|
||||
mockUser = null;
|
||||
mockToken = null;
|
||||
|
||||
render(<Home />);
|
||||
|
||||
|
|
|
|||
|
|
@ -4,9 +4,11 @@ import { useEffect, useState } from "react";
|
|||
import { useRouter } from "next/navigation";
|
||||
import { useAuth } from "./auth-context";
|
||||
|
||||
const API_URL = process.env.NEXT_PUBLIC_API_URL || "http://localhost:8000";
|
||||
|
||||
export default function Home() {
|
||||
const [count, setCount] = useState<number | null>(null);
|
||||
const { user, token, isLoading, logout } = useAuth();
|
||||
const { user, isLoading, logout } = useAuth();
|
||||
const router = useRouter();
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -16,26 +18,30 @@ export default function Home() {
|
|||
}, [isLoading, user, router]);
|
||||
|
||||
useEffect(() => {
|
||||
if (token) {
|
||||
fetch("http://localhost:8000/api/counter", {
|
||||
headers: { Authorization: `Bearer ${token}` },
|
||||
if (user) {
|
||||
fetch(`${API_URL}/api/counter`, {
|
||||
credentials: "include",
|
||||
})
|
||||
.then((res) => res.json())
|
||||
.then((data) => setCount(data.value))
|
||||
.catch(() => setCount(null));
|
||||
}
|
||||
}, [token]);
|
||||
}, [user]);
|
||||
|
||||
const increment = async () => {
|
||||
if (!token) return;
|
||||
const res = await fetch("http://localhost:8000/api/counter/increment", {
|
||||
const res = await fetch(`${API_URL}/api/counter/increment`, {
|
||||
method: "POST",
|
||||
headers: { Authorization: `Bearer ${token}` },
|
||||
credentials: "include",
|
||||
});
|
||||
const data = await res.json();
|
||||
setCount(data.value);
|
||||
};
|
||||
|
||||
const handleLogout = async () => {
|
||||
await logout();
|
||||
router.push("/login");
|
||||
};
|
||||
|
||||
if (isLoading) {
|
||||
return (
|
||||
<main style={styles.main}>
|
||||
|
|
@ -53,7 +59,7 @@ export default function Home() {
|
|||
<div style={styles.header}>
|
||||
<div style={styles.userInfo}>
|
||||
<span style={styles.userEmail}>{user.email}</span>
|
||||
<button onClick={logout} style={styles.logoutBtn}>
|
||||
<button onClick={handleLogout} style={styles.logoutBtn}>
|
||||
Sign out
|
||||
</button>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -5,9 +5,9 @@ function uniqueEmail(): string {
|
|||
return `test-${Date.now()}-${Math.random().toString(36).substring(7)}@example.com`;
|
||||
}
|
||||
|
||||
// Helper to clear localStorage
|
||||
// Helper to clear auth cookies
|
||||
async function clearAuth(page: Page) {
|
||||
await page.evaluate(() => localStorage.clear());
|
||||
await page.context().clearCookies();
|
||||
}
|
||||
|
||||
test.describe("Authentication Flow", () => {
|
||||
|
|
@ -83,7 +83,7 @@ test.describe("Signup", () => {
|
|||
await page.click('button[type="submit"]');
|
||||
await expect(page).toHaveURL("/");
|
||||
|
||||
// Clear and try again with same email
|
||||
// Clear cookies and try again with same email
|
||||
await clearAuth(page);
|
||||
await page.goto("/signup");
|
||||
await page.fill('input[type="email"]', email);
|
||||
|
|
@ -248,7 +248,7 @@ test.describe("Session Persistence", () => {
|
|||
await expect(page.getByText(email)).toBeVisible();
|
||||
});
|
||||
|
||||
test("token is stored in localStorage", async ({ page }) => {
|
||||
test("auth cookie is set after login", async ({ page }) => {
|
||||
const email = uniqueEmail();
|
||||
|
||||
await page.goto("/signup");
|
||||
|
|
@ -258,13 +258,14 @@ test.describe("Session Persistence", () => {
|
|||
await page.click('button[type="submit"]');
|
||||
await expect(page).toHaveURL("/");
|
||||
|
||||
// Check localStorage
|
||||
const token = await page.evaluate(() => localStorage.getItem("token"));
|
||||
expect(token).toBeTruthy();
|
||||
expect(token!.length).toBeGreaterThan(10);
|
||||
// Check cookies
|
||||
const cookies = await page.context().cookies();
|
||||
const authCookie = cookies.find((c) => c.name === "auth_token");
|
||||
expect(authCookie).toBeTruthy();
|
||||
expect(authCookie!.httpOnly).toBe(true);
|
||||
});
|
||||
|
||||
test("token is cleared on logout", async ({ page }) => {
|
||||
test("auth cookie is cleared on logout", async ({ page }) => {
|
||||
const email = uniqueEmail();
|
||||
|
||||
await page.goto("/signup");
|
||||
|
|
@ -276,8 +277,9 @@ test.describe("Session Persistence", () => {
|
|||
|
||||
await page.click("text=Sign out");
|
||||
|
||||
const token = await page.evaluate(() => localStorage.getItem("token"));
|
||||
expect(token).toBeNull();
|
||||
const cookies = await page.context().cookies();
|
||||
const authCookie = cookies.find((c) => c.name === "auth_token");
|
||||
// Cookie should be deleted or have empty value
|
||||
expect(!authCookie || authCookie.value === "").toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ function uniqueEmail(): string {
|
|||
// Helper to authenticate a user
|
||||
async function authenticate(page: Page): Promise<string> {
|
||||
const email = uniqueEmail();
|
||||
await page.evaluate(() => localStorage.clear());
|
||||
await page.context().clearCookies();
|
||||
await page.goto("/signup");
|
||||
await page.fill('input[type="email"]', email);
|
||||
await page.fill('input[type="password"]', "password123");
|
||||
|
|
@ -95,13 +95,13 @@ test.describe("Counter - Authenticated", () => {
|
|||
|
||||
test.describe("Counter - Unauthenticated", () => {
|
||||
test("redirects to login when accessing counter without auth", async ({ page }) => {
|
||||
await page.evaluate(() => localStorage.clear());
|
||||
await page.context().clearCookies();
|
||||
await page.goto("/");
|
||||
await expect(page).toHaveURL("/login");
|
||||
});
|
||||
|
||||
test("shows login form when redirected", async ({ page }) => {
|
||||
await page.evaluate(() => localStorage.clear());
|
||||
await page.context().clearCookies();
|
||||
await page.goto("/");
|
||||
await expect(page.locator("h1")).toHaveText("Welcome back");
|
||||
});
|
||||
|
|
@ -138,11 +138,11 @@ test.describe("Counter - Session Integration", () => {
|
|||
test("counter API requires authentication", async ({ page }) => {
|
||||
// Try to access counter API directly without auth
|
||||
const response = await page.request.get("http://localhost:8000/api/counter");
|
||||
expect(response.status()).toBe(403);
|
||||
expect(response.status()).toBe(401);
|
||||
});
|
||||
|
||||
test("counter increment API requires authentication", async ({ page }) => {
|
||||
const response = await page.request.post("http://localhost:8000/api/counter/increment");
|
||||
expect(response.status()).toBe(403);
|
||||
expect(response.status()).toBe(401);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
7
frontend/env.example
Normal file
7
frontend/env.example
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
# Environment variables for the frontend
|
||||
# For local dev: use direnv with the root .envrc file (recommended)
|
||||
# For production: set these in your deployment environment
|
||||
|
||||
# API URL for the backend
|
||||
NEXT_PUBLIC_API_URL=http://localhost:8000
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue