From 66bc4c5a45c209479de8801c1d558044bb2f69a0 Mon Sep 17 00:00:00 2001 From: counterweight Date: Thu, 18 Dec 2025 23:54:51 +0100 Subject: [PATCH] review --- .gitignore | 1 - Makefile | 2 +- backend/auth.py | 25 ----- frontend/app/audit/page.test.tsx | 172 +++++++++++++++++++++++++++++++ frontend/app/audit/page.tsx | 162 ++++++++++++----------------- frontend/app/auth-context.tsx | 14 +-- frontend/app/page.tsx | 78 +------------- frontend/app/styles/shared.ts | 82 +++++++++++++++ frontend/app/sum/page.tsx | 99 ++++-------------- frontend/e2e/permissions.spec.ts | 52 +++------- 10 files changed, 367 insertions(+), 320 deletions(-) create mode 100644 frontend/app/audit/page.test.tsx create mode 100644 frontend/app/styles/shared.ts diff --git a/.gitignore b/.gitignore index a397da0..aa5f5c4 100644 --- a/.gitignore +++ b/.gitignore @@ -12,7 +12,6 @@ node_modules/ # Env .env -.env # IDE .idea/ diff --git a/Makefile b/Makefile index d06cbfe..e5fd5fe 100644 --- a/Makefile +++ b/Makefile @@ -51,4 +51,4 @@ test-frontend: test-e2e: ./scripts/e2e.sh -test: test-backend test-frontend +test: test-backend test-frontend test-e2e diff --git a/backend/auth.py b/backend/auth.py index a358fbd..87bfe07 100644 --- a/backend/auth.py +++ b/backend/auth.py @@ -128,31 +128,6 @@ def require_permission(*required_permissions: Permission): return permission_checker -def require_any_permission(*required_permissions: Permission): - """ - Dependency factory that checks if user has ANY of the required permissions. - - Usage: - @app.get("/api/resource") - async def get_resource(user: User = Depends(require_any_permission(Permission.VIEW, Permission.ADMIN))): - ... - """ - async def permission_checker( - request: Request, - db: AsyncSession = Depends(get_db), - ) -> User: - user = await get_current_user(request, db) - user_permissions = await user.get_permissions(db) - - if not any(p in user_permissions for p in required_permissions): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"Requires one of: {', '.join(p.value for p in required_permissions)}", - ) - return user - return permission_checker - - async def build_user_response(user: User, db: AsyncSession) -> UserResponse: """Build a UserResponse with roles and permissions.""" permissions = await user.get_permissions(db) diff --git a/frontend/app/audit/page.test.tsx b/frontend/app/audit/page.test.tsx new file mode 100644 index 0000000..dee1b8f --- /dev/null +++ b/frontend/app/audit/page.test.tsx @@ -0,0 +1,172 @@ +import { render, screen, waitFor, cleanup } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import AuditPage from "./page"; + +// Mock next/navigation +const mockPush = vi.fn(); +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: mockPush }), +})); + +// Default mock values for admin user +let mockUser: { id: number; email: string; roles: string[]; permissions: string[] } | null = { + id: 1, + email: "admin@example.com", + roles: ["admin"], + permissions: ["view_audit"], +}; +let mockIsLoading = false; +const mockLogout = vi.fn(); +const mockHasPermission = vi.fn((permission: string) => + mockUser?.permissions.includes(permission) ?? false +); + +vi.mock("../auth-context", () => ({ + useAuth: () => ({ + user: mockUser, + isLoading: mockIsLoading, + logout: mockLogout, + hasPermission: mockHasPermission, + }), + Permission: { + VIEW_COUNTER: "view_counter", + INCREMENT_COUNTER: "increment_counter", + USE_SUM: "use_sum", + VIEW_AUDIT: "view_audit", + }, +})); + +// Mock fetch +const mockFetch = vi.fn(); +global.fetch = mockFetch; + +beforeEach(() => { + vi.clearAllMocks(); + mockUser = { + id: 1, + email: "admin@example.com", + roles: ["admin"], + permissions: ["view_audit"], + }; + mockIsLoading = false; + mockHasPermission.mockImplementation((permission: string) => + mockUser?.permissions.includes(permission) ?? false + ); + // Default: successful empty response + mockFetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ records: [], total: 0, page: 1, per_page: 10, total_pages: 1 }), + }); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe("AuditPage", () => { + it("shows loading state", () => { + mockIsLoading = true; + render(); + expect(screen.getByText("Loading...")).toBeTruthy(); + }); + + it("redirects to login when not authenticated", async () => { + mockUser = null; + render(); + await waitFor(() => { + expect(mockPush).toHaveBeenCalledWith("/login"); + }); + }); + + it("redirects to home when user lacks audit permission", async () => { + mockUser = { + id: 1, + email: "user@example.com", + roles: ["regular"], + permissions: ["view_counter"], + }; + mockHasPermission.mockReturnValue(false); + render(); + await waitFor(() => { + expect(mockPush).toHaveBeenCalledWith("/"); + }); + }); + + it("displays error message when API fetch fails", async () => { + mockFetch.mockRejectedValue(new Error("Network error")); + render(); + + await waitFor(() => { + // Both tables should show errors since both calls fail + const errors = screen.getAllByText("Network error"); + expect(errors.length).toBeGreaterThan(0); + }); + }); + + it("displays error when API returns non-ok response", async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 500, + json: () => Promise.resolve({ detail: "Internal server error" }), + }); + + render(); + + await waitFor(() => { + expect(screen.getByText("Failed to load counter records")).toBeTruthy(); + }); + }); + + it("displays records when fetch succeeds", async () => { + const counterResponse = { + records: [ + { + id: 1, + user_email: "recorduser@example.com", + value_before: 0, + value_after: 1, + created_at: "2024-01-01T00:00:00Z", + }, + ], + total: 1, + page: 1, + per_page: 10, + total_pages: 1, + }; + + const sumResponse = { + records: [], + total: 0, + page: 1, + per_page: 10, + total_pages: 1, + }; + + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve(counterResponse), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve(sumResponse), + }); + + render(); + + await waitFor(() => { + expect(screen.getByText("recorduser@example.com")).toBeTruthy(); + }); + }); + + it("shows table headers", async () => { + render(); + + await waitFor(() => { + // Check for counter table headers + expect(screen.getByText("Counter Activity")).toBeTruthy(); + expect(screen.getByText("Sum Activity")).toBeTruthy(); + }); + }); +}); diff --git a/frontend/app/audit/page.tsx b/frontend/app/audit/page.tsx index 621d9f4..df37354 100644 --- a/frontend/app/audit/page.tsx +++ b/frontend/app/audit/page.tsx @@ -1,9 +1,10 @@ "use client"; -import { useEffect, useState } from "react"; +import { useEffect, useState, useCallback } from "react"; import { useRouter } from "next/navigation"; import { useAuth, Permission } from "../auth-context"; import { API_URL } from "../config"; +import { sharedStyles } from "../styles/shared"; interface CounterRecord { id: number; @@ -33,6 +34,8 @@ interface PaginatedResponse { export default function AuditPage() { const [counterData, setCounterData] = useState | null>(null); const [sumData, setSumData] = useState | null>(null); + const [counterError, setCounterError] = useState(null); + const [sumError, setSumError] = useState(null); const [counterPage, setCounterPage] = useState(1); const [sumPage, setSumPage] = useState(1); const { user, isLoading, logout, hasPermission } = useAuth(); @@ -50,41 +53,51 @@ export default function AuditPage() { } }, [isLoading, user, router, canViewAudit]); + const fetchCounterRecords = useCallback(async (page: number) => { + setCounterError(null); + try { + const res = await fetch(`${API_URL}/api/audit/counter?page=${page}&per_page=10`, { + credentials: "include", + }); + if (!res.ok) { + throw new Error("Failed to load counter records"); + } + const data = await res.json(); + setCounterData(data); + } catch (err) { + setCounterData(null); + setCounterError(err instanceof Error ? err.message : "Failed to load counter records"); + } + }, []); + + const fetchSumRecords = useCallback(async (page: number) => { + setSumError(null); + try { + const res = await fetch(`${API_URL}/api/audit/sum?page=${page}&per_page=10`, { + credentials: "include", + }); + if (!res.ok) { + throw new Error("Failed to load sum records"); + } + const data = await res.json(); + setSumData(data); + } catch (err) { + setSumData(null); + setSumError(err instanceof Error ? err.message : "Failed to load sum records"); + } + }, []); + useEffect(() => { if (user && canViewAudit) { fetchCounterRecords(counterPage); } - }, [user, counterPage, canViewAudit]); + }, [user, counterPage, canViewAudit, fetchCounterRecords]); useEffect(() => { if (user && canViewAudit) { fetchSumRecords(sumPage); } - }, [user, sumPage, canViewAudit]); - - const fetchCounterRecords = async (page: number) => { - try { - const res = await fetch(`${API_URL}/api/audit/counter?page=${page}&per_page=10`, { - credentials: "include", - }); - const data = await res.json(); - setCounterData(data); - } catch { - setCounterData(null); - } - }; - - const fetchSumRecords = async (page: number) => { - try { - const res = await fetch(`${API_URL}/api/audit/sum?page=${page}&per_page=10`, { - credentials: "include", - }); - const data = await res.json(); - setSumData(data); - } catch { - setSumData(null); - } - }; + }, [user, sumPage, canViewAudit, fetchSumRecords]); const handleLogout = async () => { await logout(); @@ -142,7 +155,12 @@ export default function AuditPage() { - {counterData?.records.map((record) => ( + {counterError && ( + + {counterError} + + )} + {!counterError && counterData?.records.map((record) => ( {record.user_email} {record.value_before} @@ -150,7 +168,7 @@ export default function AuditPage() { {formatDate(record.created_at)} ))} - {(!counterData || counterData.records.length === 0) && ( + {!counterError && (!counterData || counterData.records.length === 0) && ( No records yet @@ -201,7 +219,12 @@ export default function AuditPage() { - {sumData?.records.map((record) => ( + {sumError && ( + + {sumError} + + )} + {!sumError && sumData?.records.map((record) => ( {record.user_email} {record.a} @@ -210,7 +233,7 @@ export default function AuditPage() { {formatDate(record.created_at)} ))} - {(!sumData || sumData.records.length === 0) && ( + {!sumError && (!sumData || sumData.records.length === 0) && ( No records yet @@ -246,73 +269,8 @@ export default function AuditPage() { ); } -const styles: Record = { - main: { - minHeight: "100vh", - background: "linear-gradient(135deg, #0f0f23 0%, #1a1a3e 50%, #2d1b4e 100%)", - display: "flex", - flexDirection: "column", - }, - loader: { - flex: 1, - display: "flex", - alignItems: "center", - justifyContent: "center", - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "1.125rem", - }, - header: { - padding: "1.5rem 2rem", - borderBottom: "1px solid rgba(255, 255, 255, 0.06)", - display: "flex", - justifyContent: "space-between", - alignItems: "center", - }, - nav: { - display: "flex", - alignItems: "center", - gap: "0.75rem", - }, - navLink: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "0.875rem", - textDecoration: "none", - transition: "color 0.2s", - }, - navDivider: { - color: "rgba(255, 255, 255, 0.2)", - fontSize: "0.75rem", - }, - navCurrent: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "#a78bfa", - fontSize: "0.875rem", - fontWeight: 600, - }, - userInfo: { - display: "flex", - alignItems: "center", - gap: "1rem", - }, - userEmail: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.6)", - fontSize: "0.875rem", - }, - logoutBtn: { - fontFamily: "'DM Sans', system-ui, sans-serif", - padding: "0.5rem 1rem", - fontSize: "0.875rem", - fontWeight: 500, - background: "rgba(255, 255, 255, 0.05)", - color: "rgba(255, 255, 255, 0.7)", - border: "1px solid rgba(255, 255, 255, 0.1)", - borderRadius: "8px", - cursor: "pointer", - transition: "all 0.2s", - }, +const pageStyles: Record = { + // Override content for audit-specific layout content: { flex: 1, padding: "2rem", @@ -401,6 +359,12 @@ const styles: Record = { color: "rgba(255, 255, 255, 0.3)", fontSize: "0.875rem", }, + errorRow: { + padding: "2rem 1rem", + textAlign: "center", + color: "#f87171", + fontSize: "0.875rem", + }, pagination: { display: "flex", justifyContent: "center", @@ -428,3 +392,5 @@ const styles: Record = { }, }; +const styles = { ...sharedStyles, ...pageStyles }; + diff --git a/frontend/app/auth-context.tsx b/frontend/app/auth-context.tsx index 98b129e..2884b26 100644 --- a/frontend/app/auth-context.tsx +++ b/frontend/app/auth-context.tsx @@ -1,6 +1,6 @@ "use client"; -import { createContext, useContext, useState, useEffect, ReactNode } from "react"; +import { createContext, useContext, useState, useEffect, useCallback, ReactNode } from "react"; import { API_URL } from "./config"; @@ -100,17 +100,17 @@ export function AuthProvider({ children }: { children: ReactNode }) { setUser(null); }; - const hasPermission = (permission: PermissionType): boolean => { + const hasPermission = useCallback((permission: PermissionType): boolean => { return user?.permissions.includes(permission) ?? false; - }; + }, [user]); - const hasAnyPermission = (...permissions: PermissionType[]): boolean => { + const hasAnyPermission = useCallback((...permissions: PermissionType[]): boolean => { return permissions.some((p) => user?.permissions.includes(p) ?? false); - }; + }, [user]); - const hasRole = (role: string): boolean => { + const hasRole = useCallback((role: string): boolean => { return user?.roles.includes(role) ?? false; - }; + }, [user]); return ( (null); @@ -90,80 +91,7 @@ export default function Home() { ); } -const styles: Record = { - main: { - minHeight: "100vh", - background: "linear-gradient(135deg, #0f0f23 0%, #1a1a3e 50%, #2d1b4e 100%)", - display: "flex", - flexDirection: "column", - }, - loader: { - flex: 1, - display: "flex", - alignItems: "center", - justifyContent: "center", - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "1.125rem", - }, - header: { - padding: "1.5rem 2rem", - borderBottom: "1px solid rgba(255, 255, 255, 0.06)", - display: "flex", - justifyContent: "space-between", - alignItems: "center", - }, - nav: { - display: "flex", - alignItems: "center", - gap: "0.75rem", - }, - navLink: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "0.875rem", - textDecoration: "none", - transition: "color 0.2s", - }, - navDivider: { - color: "rgba(255, 255, 255, 0.2)", - fontSize: "0.75rem", - }, - navCurrent: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "#a78bfa", - fontSize: "0.875rem", - fontWeight: 600, - }, - userInfo: { - display: "flex", - alignItems: "center", - gap: "1rem", - }, - userEmail: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.6)", - fontSize: "0.875rem", - }, - logoutBtn: { - fontFamily: "'DM Sans', system-ui, sans-serif", - padding: "0.5rem 1rem", - fontSize: "0.875rem", - fontWeight: 500, - background: "rgba(255, 255, 255, 0.05)", - color: "rgba(255, 255, 255, 0.7)", - border: "1px solid rgba(255, 255, 255, 0.1)", - borderRadius: "8px", - cursor: "pointer", - transition: "all 0.2s", - }, - content: { - flex: 1, - display: "flex", - alignItems: "center", - justifyContent: "center", - padding: "2rem", - }, +const pageStyles: Record = { counterCard: { background: "rgba(255, 255, 255, 0.03)", backdropFilter: "blur(10px)", @@ -216,3 +144,5 @@ const styles: Record = { fontWeight: 400, }, }; + +const styles = { ...sharedStyles, ...pageStyles }; diff --git a/frontend/app/styles/shared.ts b/frontend/app/styles/shared.ts new file mode 100644 index 0000000..ecaef96 --- /dev/null +++ b/frontend/app/styles/shared.ts @@ -0,0 +1,82 @@ +import React from "react"; + +/** + * Shared styles used across multiple pages. + * These styles define the common layout and theming for the app. + */ +export const sharedStyles: Record = { + main: { + minHeight: "100vh", + background: "linear-gradient(135deg, #0f0f23 0%, #1a1a3e 50%, #2d1b4e 100%)", + display: "flex", + flexDirection: "column", + }, + loader: { + flex: 1, + display: "flex", + alignItems: "center", + justifyContent: "center", + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "rgba(255, 255, 255, 0.5)", + fontSize: "1.125rem", + }, + header: { + padding: "1.5rem 2rem", + borderBottom: "1px solid rgba(255, 255, 255, 0.06)", + display: "flex", + justifyContent: "space-between", + alignItems: "center", + }, + nav: { + display: "flex", + alignItems: "center", + gap: "0.75rem", + }, + navLink: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "rgba(255, 255, 255, 0.5)", + fontSize: "0.875rem", + textDecoration: "none", + transition: "color 0.2s", + }, + navDivider: { + color: "rgba(255, 255, 255, 0.2)", + fontSize: "0.75rem", + }, + navCurrent: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "#a78bfa", + fontSize: "0.875rem", + fontWeight: 600, + }, + userInfo: { + display: "flex", + alignItems: "center", + gap: "1rem", + }, + userEmail: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "rgba(255, 255, 255, 0.6)", + fontSize: "0.875rem", + }, + logoutBtn: { + fontFamily: "'DM Sans', system-ui, sans-serif", + padding: "0.5rem 1rem", + fontSize: "0.875rem", + fontWeight: 500, + background: "rgba(255, 255, 255, 0.05)", + color: "rgba(255, 255, 255, 0.7)", + border: "1px solid rgba(255, 255, 255, 0.1)", + borderRadius: "8px", + cursor: "pointer", + transition: "all 0.2s", + }, + content: { + flex: 1, + display: "flex", + alignItems: "center", + justifyContent: "center", + padding: "2rem", + }, +}; + diff --git a/frontend/app/sum/page.tsx b/frontend/app/sum/page.tsx index 15d0c81..075ec84 100644 --- a/frontend/app/sum/page.tsx +++ b/frontend/app/sum/page.tsx @@ -4,12 +4,14 @@ import { useEffect, useState } from "react"; import { useRouter } from "next/navigation"; import { useAuth, Permission } from "../auth-context"; import { API_URL } from "../config"; +import { sharedStyles } from "../styles/shared"; export default function SumPage() { const [a, setA] = useState(""); const [b, setB] = useState(""); const [result, setResult] = useState(null); const [showResult, setShowResult] = useState(false); + const [error, setError] = useState(null); const { user, isLoading, logout, hasPermission } = useAuth(); const router = useRouter(); @@ -28,6 +30,7 @@ export default function SumPage() { const handleSum = async () => { const numA = parseFloat(a) || 0; const numB = parseFloat(b) || 0; + setError(null); try { const res = await fetch(`${API_URL}/api/sum`, { @@ -36,13 +39,14 @@ export default function SumPage() { credentials: "include", body: JSON.stringify({ a: numA, b: numB }), }); + if (!res.ok) { + throw new Error("Calculation failed"); + } const data = await res.json(); setResult(data.result); setShowResult(true); - } catch { - // Fallback to local calculation if API fails - setResult(numA + numB); - setShowResult(true); + } catch (err) { + setError(err instanceof Error ? err.message : "Calculation failed"); } }; @@ -51,6 +55,7 @@ export default function SumPage() { setB(""); setResult(null); setShowResult(false); + setError(null); }; const handleLogout = async () => { @@ -123,6 +128,9 @@ export default function SumPage() { = Calculate + {error && ( +
{error}
+ )} ) : (
@@ -145,80 +153,7 @@ export default function SumPage() { ); } -const styles: Record = { - main: { - minHeight: "100vh", - background: "linear-gradient(135deg, #0f0f23 0%, #1a1a3e 50%, #2d1b4e 100%)", - display: "flex", - flexDirection: "column", - }, - loader: { - flex: 1, - display: "flex", - alignItems: "center", - justifyContent: "center", - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "1.125rem", - }, - header: { - padding: "1.5rem 2rem", - borderBottom: "1px solid rgba(255, 255, 255, 0.06)", - display: "flex", - justifyContent: "space-between", - alignItems: "center", - }, - nav: { - display: "flex", - alignItems: "center", - gap: "0.75rem", - }, - navLink: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.5)", - fontSize: "0.875rem", - textDecoration: "none", - transition: "color 0.2s", - }, - navDivider: { - color: "rgba(255, 255, 255, 0.2)", - fontSize: "0.75rem", - }, - navCurrent: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "#a78bfa", - fontSize: "0.875rem", - fontWeight: 600, - }, - userInfo: { - display: "flex", - alignItems: "center", - gap: "1rem", - }, - userEmail: { - fontFamily: "'DM Sans', system-ui, sans-serif", - color: "rgba(255, 255, 255, 0.6)", - fontSize: "0.875rem", - }, - logoutBtn: { - fontFamily: "'DM Sans', system-ui, sans-serif", - padding: "0.5rem 1rem", - fontSize: "0.875rem", - fontWeight: 500, - background: "rgba(255, 255, 255, 0.05)", - color: "rgba(255, 255, 255, 0.7)", - border: "1px solid rgba(255, 255, 255, 0.1)", - borderRadius: "8px", - cursor: "pointer", - transition: "all 0.2s", - }, - content: { - flex: 1, - display: "flex", - alignItems: "center", - justifyContent: "center", - padding: "2rem", - }, +const pageStyles: Record = { card: { background: "rgba(255, 255, 255, 0.03)", backdropFilter: "blur(10px)", @@ -344,5 +279,13 @@ const styles: Record = { resetIcon: { fontSize: "1.25rem", }, + error: { + fontFamily: "'DM Sans', system-ui, sans-serif", + color: "#f87171", + fontSize: "0.875rem", + marginTop: "0.5rem", + }, }; +const styles = { ...sharedStyles, ...pageStyles }; + diff --git a/frontend/e2e/permissions.spec.ts b/frontend/e2e/permissions.spec.ts index e189241..c832e5f 100644 --- a/frontend/e2e/permissions.spec.ts +++ b/frontend/e2e/permissions.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, Page, APIRequestContext } from "@playwright/test"; +import { test, expect, Page } from "@playwright/test"; /** * Permission-based E2E tests @@ -14,14 +14,23 @@ const API_URL = process.env.NEXT_PUBLIC_API_URL || "http://localhost:8000"; // Test credentials - must match what's seeded in the database via seed.py // These come from environment variables DEV_USER_EMAIL/PASSWORD and DEV_ADMIN_EMAIL/PASSWORD +// Tests will fail fast if these are not set +function getRequiredEnv(name: string): string { + const value = process.env[name]; + if (!value) { + throw new Error(`Required environment variable ${name} is not set. Run 'source .env' or set it in your environment.`); + } + return value; +} + const REGULAR_USER = { - email: process.env.DEV_USER_EMAIL || "user@example.com", - password: process.env.DEV_USER_PASSWORD || "user123", + email: getRequiredEnv("DEV_USER_EMAIL"), + password: getRequiredEnv("DEV_USER_PASSWORD"), }; const ADMIN_USER = { - email: process.env.DEV_ADMIN_EMAIL || "admin@example.com", - password: process.env.DEV_ADMIN_PASSWORD || "admin123", + email: getRequiredEnv("DEV_ADMIN_EMAIL"), + password: getRequiredEnv("DEV_ADMIN_PASSWORD"), }; // Helper to clear auth cookies @@ -29,17 +38,6 @@ async function clearAuth(page: Page) { await page.context().clearCookies(); } -// Helper to create a user with specific role via API -async function createUserWithRole( - request: APIRequestContext, - email: string, - password: string, - roleName: string -): Promise { - // This requires direct DB access or a test endpoint - // For now, we'll use the seeded users from conftest -} - // Helper to login a user async function loginUser(page: Page, email: string, password: string) { await page.goto("/login"); @@ -149,19 +147,9 @@ test.describe("Regular User Access", () => { }); test.describe("Admin User Access", () => { - // Skip these tests if admin user isn't set up - // In real scenario, you'd create admin user in beforeAll - test.skip( - !process.env.DEV_ADMIN_EMAIL, - "Admin tests require DEV_ADMIN_EMAIL and DEV_ADMIN_PASSWORD env vars" - ); - - const adminEmail = process.env.DEV_ADMIN_EMAIL || ADMIN_USER.email; - const adminPassword = process.env.DEV_ADMIN_PASSWORD || ADMIN_USER.password; - test.beforeEach(async ({ page }) => { await clearAuth(page); - await loginUser(page, adminEmail, adminPassword); + await loginUser(page, ADMIN_USER.email, ADMIN_USER.password); }); test("redirected from counter page to audit", async ({ page }) => { @@ -258,17 +246,9 @@ test.describe("Permission Boundary via API", () => { }); test("admin user API call to counter returns 403", async ({ page, request }) => { - const adminEmail = process.env.DEV_ADMIN_EMAIL; - const adminPassword = process.env.DEV_ADMIN_PASSWORD; - - if (!adminEmail || !adminPassword) { - test.skip(); - return; - } - // Login as admin await clearAuth(page); - await loginUser(page, adminEmail, adminPassword); + await loginUser(page, ADMIN_USER.email, ADMIN_USER.password); // Get cookies const cookies = await page.context().cookies();