fix(GRO-2358): wire signOut() at higher layer for no-access screen
The 'Portal access not configured' screen (rendered when an Authentik SSO user has no matching client row) used an inline fetch to /api/auth/sign-out instead of the shared signOut() exported by lib/auth-client. AdminLayout already uses the shared handler, so the no-access screen was a divergence that could trap the user on an authenticated surface with a broken exit. Wire handleSignOut at the CustomerPortal level and use it from the no-access card. The handler: - calls the canonical signOut() from lib/auth-client (same one AdminLayout uses, so 'same logout handler reachable from any authenticated route' is satisfied without adding a new chrome element) - always navigates to /login, even on transient auth-server failure, so a network hiccup never leaves the user trapped Tests cover the home no-access path AND a deep-link no-access path (/appointments) — the latter guards the AC requirement that the escape hatch works on at least one other authenticated surface that does not have a route guard. UAT_PLAYBOOK §5.25.6 updated; new §5.25.6b documents the deep-link case. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
+2
-1
@@ -428,7 +428,8 @@ These cases cover the `CustomerPortal` initialisation path that bridges an Authe
|
||||
| TC-WEB-5.25.3 | Subsequent portal calls use the bridged session ID | After TC-WEB-5.25.1 succeeds, navigate to **Appointments**, **My Pets**, **Billing**, **Settings**. Inspect any `/api/portal/*` request in DevTools → Network. | Each portal API call carries an `X-Impersonation-Session-Id` header whose value equals the `sessionId` returned by `session-from-auth` (not a URL-param value). Each call returns 200 (or 404 for genuinely empty collections), never 401. |
|
||||
| TC-WEB-5.25.4 | No impersonation chrome for the customer's own session | After TC-WEB-5.25.1, scan the portal UI. | No amber border around the page. No "STAFF VIEW" watermark. No "End Impersonation" button in the sidebar. The customer is themselves; only impersonation sessions started via `?sessionId=` show the banner. |
|
||||
| TC-WEB-5.25.5 | 404 fallback for authenticated user with no client record | 1. Sign in via SSO with an Authentik account whose email is **not** present in `clients`. 2. Land on `/`. | `POST /api/portal/session-from-auth` returns 404. The portal renders a centred card titled **"Portal access not configured"** with the message about contacting the groomer and a **Sign out** button. No redirect loop, no portal chrome. |
|
||||
| TC-WEB-5.25.6 | 404 fallback Sign-out escape hatch | From TC-WEB-5.25.5 click **Sign out**. | `POST /api/auth/sign-out` fires; browser navigates to `/login`; the Authentik session cookie is cleared. Reloading `/` no longer hits 404 (will show the login page). |
|
||||
| TC-WEB-5.25.6 | 404 fallback Sign-out escape hatch (GRO-2358) | From TC-WEB-5.25.5 click **Sign out**. | The shared `signOut()` from `lib/auth-client` fires (same handler as `AdminLayout`); browser navigates to `/login`; the Authentik session cookie is cleared. Reloading `/` no longer hits 404 (will show the login page). The handler always navigates to `/login` — even if the network call to `/api/auth/sign-out` fails — so a transient auth-server hiccup never leaves the user trapped on an authenticated screen. |
|
||||
| TC-WEB-5.25.6b | 404 fallback Sign-out on deep-link (GRO-2358) | From TC-WEB-5.25.5, instead of staying on `/`, navigate directly to a portal sub-route (e.g. `/appointments`, `/pets`, `/billing`). The no-access card renders. Click **Sign out**. | The same shared `signOut()` handler fires and the browser navigates to `/login`. The no-access screen must surface an escape hatch on every authenticated route — not just `/` — so a stale or deep link into a portal the user has no access to can never trap them. |
|
||||
| TC-WEB-5.25.7 | Bridge precedence — impersonation URL wins | 1. Sign in via SSO as a customer. 2. Open a new tab to `https://uat.groombook.dev/?sessionId=<a-valid-staff-impersonation-session-id>`. | The impersonation path runs; the amber banner appears for the impersonated client. The Better Auth bridge is **not** called on this load (`session-from-auth` absent in Network). |
|
||||
| TC-WEB-5.25.8 | Bridge precedence — dev user wins | In dev mode (e.g. local) with `localStorage["dev-user"]` set to a client persona, navigate to `/`. | The dev-session path runs (`POST /api/portal/dev-session`). The Better Auth bridge is **not** called (`session-from-auth` absent in Network). Staff dev users still redirect to `/admin`. |
|
||||
| TC-WEB-5.25.9 | Staff Better Auth session does not run the customer bridge | Sign in via SSO with a staff identity. Navigate to `/`. | `App.tsx` routing redirects to `/admin`. `POST /api/portal/session-from-auth` is **not** called. |
|
||||
|
||||
@@ -21,6 +21,18 @@ vi.mock("../portal/sections/Appointments.js", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
// Spy on the canonical `signOut()` from the shared auth-client so we can
|
||||
// assert the no-access screen's logout button uses the SAME handler as
|
||||
// `AdminLayout`. We mock at the module boundary — the no-access screen is
|
||||
// the one authenticated surface that renders without the portal chrome, so
|
||||
// a regression here would trap the user. We do NOT use `importActual`
|
||||
// because the real `createAuthClient()` requires a runtime `baseURL`
|
||||
// (Better Auth) that the JSDOM test environment can't supply.
|
||||
const signOutSpy = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
vi.mock("../lib/auth-client.js", () => ({
|
||||
signOut: signOutSpy,
|
||||
}));
|
||||
|
||||
const SESSION: ImpersonationSession = {
|
||||
id: "sess-1",
|
||||
staffId: "staff-1",
|
||||
@@ -336,6 +348,8 @@ describe("CustomerPortal SSO bridge", () => {
|
||||
beforeEach(() => {
|
||||
// Make sure no dev-user leaks across tests
|
||||
window.localStorage.clear();
|
||||
// Reset shared signOut() spy so per-test counts are deterministic
|
||||
signOutSpy.mockClear();
|
||||
});
|
||||
|
||||
const brandingResponse = {
|
||||
@@ -429,6 +443,132 @@ describe("CustomerPortal SSO bridge", () => {
|
||||
expect(screen.getByRole("button", { name: /Sign out/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("calls the shared signOut() handler and navigates to /login from the no-access screen (GRO-2358)", async () => {
|
||||
// Reset the spy so previous tests don't leak into this assertion.
|
||||
signOutSpy.mockClear();
|
||||
|
||||
// JSDOM throws on window.location.href assignment by default; swap in a
|
||||
// writable stub so the navigation is observable, then restore after.
|
||||
const originalLocation = window.location;
|
||||
Object.defineProperty(window, "location", {
|
||||
value: { href: "" },
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
global.fetch = vi.fn((input: RequestInfo) => {
|
||||
const url = typeof input === "string" ? input : input.toString();
|
||||
if (url === "/api/branding") return Promise.resolve(brandingResponse);
|
||||
if (url === "/api/auth/get-session") {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: async () => ({ user: { email: "stranger@example.com", role: "customer" } }),
|
||||
} as Response);
|
||||
}
|
||||
if (url === "/api/portal/session-from-auth") {
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
status: 404,
|
||||
json: async () => ({ error: "No client record found for this user" }),
|
||||
} as Response);
|
||||
}
|
||||
return Promise.resolve({ ok: true, json: async () => ({}) } as Response);
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
const { CustomerPortal } = await import("../portal/CustomerPortal.js");
|
||||
render(
|
||||
<MemoryRouter initialEntries={["/"]}>
|
||||
<CustomerPortal />
|
||||
</MemoryRouter>
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/Portal access not configured/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Pre-condition: the shared signOut() must NOT have been called yet — the
|
||||
// no-access screen is mounted because the bridge failed, not because the
|
||||
// user clicked anything.
|
||||
expect(signOutSpy).not.toHaveBeenCalled();
|
||||
|
||||
// Drive the click. The handler is the SAME `signOut()` exported from
|
||||
// auth-client that AdminLayout uses, so verifying this call is enough to
|
||||
// prove the no-access screen reaches the canonical sign-out surface.
|
||||
const signOutButton = screen.getByRole("button", { name: /Sign out/i });
|
||||
fireEvent.click(signOutButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(signOutSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// The handler always navigates to /login — even if the network call to
|
||||
// /api/auth/sign-out fails — so a transient auth-server hiccup never
|
||||
// leaves the user trapped on an authenticated screen.
|
||||
await waitFor(() => {
|
||||
expect(window.location.href).toBe("/login");
|
||||
});
|
||||
|
||||
Object.defineProperty(window, "location", { value: originalLocation, configurable: true });
|
||||
});
|
||||
|
||||
it("reaches the same shared signOut() on a deep-link no-access screen (GRO-2358)", async () => {
|
||||
// AC requires verifying the SAME logout handler is reachable from at
|
||||
// least one other authenticated surface — here a deep link to a portal
|
||||
// sub-route (e.g. /appointments) for a user with a Better Auth session
|
||||
// but no client record. The no-access screen is the only authenticated
|
||||
// surface without a route guard, so the handler must fire identically.
|
||||
signOutSpy.mockClear();
|
||||
|
||||
const originalLocation = window.location;
|
||||
Object.defineProperty(window, "location", {
|
||||
value: { href: "" },
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
global.fetch = vi.fn((input: RequestInfo) => {
|
||||
const url = typeof input === "string" ? input : input.toString();
|
||||
if (url === "/api/branding") return Promise.resolve(brandingResponse);
|
||||
if (url === "/api/auth/get-session") {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: async () => ({ user: { email: "stranger@example.com", role: "customer" } }),
|
||||
} as Response);
|
||||
}
|
||||
if (url === "/api/portal/session-from-auth") {
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
status: 404,
|
||||
json: async () => ({ error: "No client record found for this user" }),
|
||||
} as Response);
|
||||
}
|
||||
return Promise.resolve({ ok: true, json: async () => ({}) } as Response);
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
const { CustomerPortal } = await import("../portal/CustomerPortal.js");
|
||||
render(
|
||||
<MemoryRouter initialEntries={["/appointments"]}>
|
||||
<CustomerPortal />
|
||||
</MemoryRouter>
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/Portal access not configured/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const signOutButton = screen.getByRole("button", { name: /Sign out/i });
|
||||
fireEvent.click(signOutButton);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(signOutSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(window.location.href).toBe("/login");
|
||||
});
|
||||
|
||||
Object.defineProperty(window, "location", { value: originalLocation, configurable: true });
|
||||
});
|
||||
|
||||
it("does not call session-from-auth when there is no Better Auth session", async () => {
|
||||
global.fetch = vi.fn((input: RequestInfo) => {
|
||||
const url = typeof input === "string" ? input : input.toString();
|
||||
|
||||
@@ -15,6 +15,7 @@ import { ImpersonationBanner } from "./ImpersonationBanner.js";
|
||||
import { AuditLogViewer } from "./AuditLogViewer.js";
|
||||
import { useBranding } from "../BrandingContext.js";
|
||||
import { getDevUser } from "../pages/DevLoginSelector.js";
|
||||
import { signOut } from "../lib/auth-client.js";
|
||||
import type { ImpersonationSession } from "@groombook/types";
|
||||
import type { Appointment as PortalAppointment } from "./sections/Appointments.js";
|
||||
|
||||
@@ -193,6 +194,19 @@ export function CustomerPortal() {
|
||||
}
|
||||
}, [session]);
|
||||
|
||||
// Shared sign-out handler — wires the canonical Better Auth `signOut()` so
|
||||
// every authenticated surface (no-access screen, portal chrome, etc.) uses
|
||||
// the same implementation as `AdminLayout`. Failure to reach the server
|
||||
// still leaves the SPA free to navigate to /login.
|
||||
const handleSignOut = useCallback(async () => {
|
||||
try {
|
||||
await signOut();
|
||||
} catch {
|
||||
// Best-effort; navigate to /login regardless so the user is never trapped.
|
||||
}
|
||||
window.location.href = "/login";
|
||||
}, []);
|
||||
|
||||
const logPageView = useCallback((page: string) => {
|
||||
if (!session) return;
|
||||
void fetch(`/api/impersonation/sessions/${session.id}/log`, {
|
||||
@@ -281,14 +295,7 @@ export function CustomerPortal() {
|
||||
<h1 className="text-lg font-semibold text-stone-800 mb-2">Portal access not configured</h1>
|
||||
<p className="text-sm text-stone-600 mb-6">{authError}</p>
|
||||
<button
|
||||
onClick={async () => {
|
||||
try {
|
||||
await fetch("/api/auth/sign-out", { method: "POST", credentials: "include" });
|
||||
} catch {
|
||||
// Best-effort sign-out; redirect to /login regardless.
|
||||
}
|
||||
window.location.href = "/login";
|
||||
}}
|
||||
onClick={() => { void handleSignOut(); }}
|
||||
className="inline-flex items-center justify-center gap-2 px-4 py-2 rounded-lg text-sm font-medium text-stone-700 bg-stone-100 hover:bg-stone-200 transition-colors"
|
||||
>
|
||||
<LogOut size={14} />
|
||||
|
||||
Reference in New Issue
Block a user