mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 17:46:06 +00:00
Second pass over the suite, driven by the inventory in the gaps document. Nine agents wrote suites in parallel against private databases, then a tenth read all of it adversarially and five of its findings were fixed. unit + component 2076 -> 2079 (+888 over the round) api 647 -> 1015 e2e 18 -> 29 What was closed: - lib/route-access.ts, the page-level authorization layer, went from zero tests to 48. Every API route was guarded and none of the pages were. - The five media proxy routes now have a real 2xx beside every 403. The blocker was the positive control, solved by stubbing r2Client.send() and leaving lib/r2-media-proxy.ts itself real. - Every remaining server-side lib module: invitations, email verification, the upload tokens, the logger, request origin, the whole R2 and Bunny lifecycle, notifications and admin stats. - Six video-page hooks, and the chunking arithmetic extracted out of lib/client/r2-video-upload.ts as a pure module. - Five end-to-end flows: workspace members, bulk operations, the admin area, player interaction and failure recovery. Three things about the harness itself turned out to be wrong: - Two @/lib/r2 stubs in tests/setup/api.ts had the wrong return shape, so every route reaching finalizeR2VideoUpload silently took the "not a valid video" branch and no test noticed. - The auth matrix asserted only "not 2xx", which two entries satisfied without their guard existing. It now requires 401 or 403, which makes both load-bearing, and all 60 routes pass the stricter form. - Both admin API routes had no positive control anywhere: replacing their guard with an unconditional refusal left the entire suite green. Found by the adversarial review, now covered. Process: - bun run test:mutation runs StrykerJS over the authorization and validation modules. Diagnostic, not a gate, weekly in CI rather than on a push. - playwright.config.ts gains an opt-in webkit project for the player spec. - AGENTS.md now requires a batch of new tests to be reviewed by somebody who did not write them. Only two production files change, both deliberate: lib/auth.ts loses a verbatim copy of its own permission formulas, and lib/client/r2-video-upload.ts calls the extracted arithmetic. No behaviour change in either.
145 lines
6.5 KiB
TypeScript
145 lines
6.5 KiB
TypeScript
// The /admin area, which nothing covered before.
|
|
//
|
|
// Admin is not a database column. lib/auth.ts:144 derives `token.isAdmin` on
|
|
// every request by looking the signed-in address up in the ADMIN_EMAILS
|
|
// environment variable of the *app under test*. That has two consequences for
|
|
// this file:
|
|
//
|
|
// 1. The privileged account has to use a fixed address rather than the
|
|
// per-test unique one every other spec gets, hence ADMIN_EMAIL below.
|
|
// Because that address is unique in the database, the two tests that use it
|
|
// must not run at the same time, hence the serial describe.
|
|
// 2. That variable therefore has to be set for the app under test, and
|
|
// playwright.config.ts sets it in APP_ENV to exactly ADMIN_EMAIL below.
|
|
// Remove it and the privileged half of this spec has nothing to sign in as,
|
|
// so the probe below **fails** rather than skipping: this is the only
|
|
// positive coverage of the admin area in the repo outside
|
|
// tests/api/auth-matrix.test.ts, and a config change should not be able to
|
|
// quietly delete it.
|
|
//
|
|
// The probe reads /api/auth/session, which is a question about configuration,
|
|
// not about the authorization being tested: if the /admin guard itself
|
|
// regressed, isAdmin would still be true and the tests below would fail on
|
|
// their own assertions.
|
|
import type { APIRequestContext, Page } from '@playwright/test';
|
|
import { anonTest, expect, E2E_PASSWORD, signInPage } from './fixtures';
|
|
import { createUser } from '../factories';
|
|
import { db } from '@/lib/db';
|
|
|
|
/**
|
|
* The address that must appear in the app's ADMIN_EMAILS for the privileged
|
|
* tests to run. Lower case, because lib/auth.ts lower-cases both sides.
|
|
*/
|
|
const ADMIN_EMAIL = '[email protected]';
|
|
|
|
const ADMIN_SETUP_HINT =
|
|
`The app under test does not treat ${ADMIN_EMAIL} as an admin. ` +
|
|
`APP_ENV in playwright.config.ts must set ADMIN_EMAILS to '${ADMIN_EMAIL}'; ` +
|
|
`this is a failure rather than a skip because it is the only place the admin ` +
|
|
`area is exercised from a browser.`;
|
|
|
|
/** Whether the app considers the signed-in account an admin. */
|
|
async function sessionIsAdmin(request: APIRequestContext): Promise<boolean> {
|
|
const response = await request.get('/api/auth/session');
|
|
if (!response.ok()) return false;
|
|
const session = (await response.json()) as { user?: { isAdmin?: boolean } };
|
|
return session.user?.isAdmin === true;
|
|
}
|
|
|
|
/** Creates the fixed-address admin account and signs `page` in as it. */
|
|
async function signInAsAdmin(page: Page): Promise<void> {
|
|
await db.user.deleteMany({ where: { email: ADMIN_EMAIL } });
|
|
await createUser({ name: 'E2E Admin', email: ADMIN_EMAIL, password: E2E_PASSWORD });
|
|
await signInPage(page, ADMIN_EMAIL);
|
|
}
|
|
|
|
anonTest.describe('the admin area', () => {
|
|
// ADMIN_EMAIL is a unique column, so only one test may hold it at a time.
|
|
anonTest.describe.configure({ mode: 'serial' });
|
|
|
|
anonTest.afterEach(async () => {
|
|
await db.user.deleteMany({ where: { email: ADMIN_EMAIL } });
|
|
});
|
|
|
|
anonTest('an ordinary signed-in user is refused every admin page', async ({ page, seed }) => {
|
|
const user = await seed.user({ name: 'Not An Admin' });
|
|
await signInPage(page, user.email ?? '');
|
|
|
|
// The session is real: without this the redirects below would prove nothing
|
|
// more than that /admin is behind a login.
|
|
await page.goto('/dashboard');
|
|
await expect(page).toHaveURL(/\/dashboard$/);
|
|
expect(await sessionIsAdmin(page.context().request)).toEqual(false);
|
|
|
|
for (const route of ['/admin', '/admin/users', '/admin/feedback']) {
|
|
await page.goto(route);
|
|
// app/admin/layout.tsx sends a non-admin to the marketing root.
|
|
await expect(page).toHaveURL(/\/$/);
|
|
await expect(page.getByRole('heading', { name: 'Dashboard Overview' })).toHaveCount(0);
|
|
}
|
|
|
|
// Refused, not missing. A route that did not exist would answer 404, and a
|
|
// 404 would satisfy every assertion above for the wrong reason.
|
|
const missing = await page.request.get('/definitely-not-a-route', { maxRedirects: 0 });
|
|
expect(missing.status()).toEqual(404);
|
|
const admin = await page.request.get('/admin', { maxRedirects: 0 });
|
|
expect(admin.status()).not.toEqual(404);
|
|
|
|
// The write side is guarded independently of the pages.
|
|
const refresh = await page.request.post('/api/admin/stats/refresh-r2');
|
|
expect(refresh.status()).toEqual(403);
|
|
});
|
|
|
|
anonTest(
|
|
'an admin reaches the dashboard and can search the user list',
|
|
async ({ page, seed }) => {
|
|
// Seeded before the probe so the search below has something to find.
|
|
const target = await seed.user({ name: 'Findable Person' });
|
|
const targetEmail = target.email ?? '';
|
|
|
|
await signInAsAdmin(page);
|
|
expect(await sessionIsAdmin(page.context().request), ADMIN_SETUP_HINT).toBe(true);
|
|
|
|
await page.goto('/admin');
|
|
await expect(page).toHaveURL(/\/admin$/);
|
|
await expect(page.getByRole('heading', { name: 'Dashboard Overview' })).toBeVisible();
|
|
|
|
// The dashboard is not a static shell: it counts rows, and the count has to
|
|
// be at least the two accounts this test created.
|
|
const totalUsers = Number(
|
|
(await page.getByText('Total Users').locator('xpath=../..').innerText())
|
|
.replace(/[^0-9]/g, '')
|
|
.trim()
|
|
);
|
|
expect(totalUsers).toBeGreaterThanOrEqual(2);
|
|
|
|
// --- one action: search the user list -----------------------------------
|
|
await page.getByRole('link', { name: 'Users' }).first().click();
|
|
await expect(page).toHaveURL(/\/admin\/users$/);
|
|
|
|
// Scoped to the form: the global header carries an icon button whose
|
|
// accessible name is also "Search".
|
|
const searchForm = page.locator('form[action="/admin/users"]');
|
|
const search = searchForm.getByLabel('Search users by name or email');
|
|
const submit = searchForm.getByRole('button', { name: 'Search' });
|
|
|
|
await search.fill(targetEmail);
|
|
await submit.click();
|
|
|
|
await expect(page).toHaveURL(/[?&]q=/);
|
|
await expect(page.getByText(targetEmail, { exact: true })).toBeVisible();
|
|
|
|
// The filter really filters: the admin's own row is in the unfiltered list
|
|
// and must be absent from this one.
|
|
await expect(page.getByText(ADMIN_EMAIL, { exact: true })).toHaveCount(0);
|
|
|
|
// And a query that matches nobody says so, rather than falling back to
|
|
// everybody.
|
|
await search.fill(`no-such-person-${Date.now()}@example.invalid`);
|
|
await submit.click();
|
|
await expect(page.getByText('No users match these filters.')).toBeVisible();
|
|
await expect(page.getByText(targetEmail, { exact: true })).toHaveCount(0);
|
|
}
|
|
);
|
|
});
|