fix(auth): make admin pages reachable — async capability guard + sticky dev role + nav
CI / frontend (push) Successful in 1m44s
CI / storybook-a11y (push) Failing after 4m28s
CI / backend (push) Successful in 1m28s
CI / e2e (push) Successful in 2m49s
CI / codeql (csharp) (push) Failing after 2m8s
CI / codeql (javascript-typescript) (push) Failing after 1m30s
CI / api-client-drift (push) Successful in 2m6s
CI / frontend (push) Successful in 1m44s
CI / storybook-a11y (push) Failing after 4m28s
CI / backend (push) Successful in 1m28s
CI / e2e (push) Successful in 2m49s
CI / codeql (csharp) (push) Failing after 2m8s
CI / codeql (javascript-typescript) (push) Failing after 1m30s
CI / api-client-drift (push) Successful in 2m6s
The admin pages (/beheer/stamdata, /brief/huisstijl) were unreachable in the browser, for three compounding reasons — all fixed here: - **Guard raced /me.** capabilityGuard read can() synchronously while /me was still loading, so it denied even an entitled admin (deny-by-default) and bounced to /login. It's now async: awaits AccessStore.whenReady() (new — resolves once /me settles), then allows if entitled; an authenticated-but-unentitled user goes to /dashboard, anonymous to /login. + auth.guard.spec (the missing test that let this ship). - **Dev role wasn't sticky.** currentRole() read ?role= from the URL on every request, but login/nav drop the param, silently reverting admin→drafter mid-session and 403-ing the admin endpoints. It now persists the role per-tab (sessionStorage), so every role-aware request keeps it. Dev-only (the interceptor is wired only under isDevMode). - **No way in.** Added capability-gated Huisstijl + Stamdata links to the header (shown only when /me grants the cap); injecting AccessStore there also warms /me early. New en translations for the two labels; site-header story stubs AccessStore (+ AsAdmin variant) so it needs no HTTP. Verified live: with ?role=admin the header shows both links, clicking Stamdata loads the grid (GET /api/v1/stamdata → 200, was 403→redirect); a non-admin sees no link. Full `npm run ci` green (310 tests); site-header stories pass axe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,62 @@
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { Router } from '@angular/router';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { AccessStore } from '@shared/application/access.store';
|
||||
import { SessionStore } from './application/session.store';
|
||||
import { authGuard, capabilityGuard } from './auth.guard';
|
||||
|
||||
type Opts = {
|
||||
authed: boolean;
|
||||
can?: (c: string) => boolean;
|
||||
whenReady?: () => Promise<void>;
|
||||
};
|
||||
|
||||
function setup({ authed, can = () => false, whenReady = () => Promise.resolve() }: Opts) {
|
||||
const createUrlTree = vi.fn((cmds: string[]) => ({ tree: cmds }));
|
||||
const readySpy = vi.fn(whenReady);
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
{ provide: SessionStore, useValue: { isAuthenticated: () => authed } },
|
||||
{ provide: AccessStore, useValue: { whenReady: readySpy, can } },
|
||||
{ provide: Router, useValue: { createUrlTree } },
|
||||
],
|
||||
});
|
||||
return { createUrlTree, readySpy };
|
||||
}
|
||||
|
||||
// The guards ignore their (route, state) args; cast to call with none.
|
||||
const call = <T>(fn: unknown) => TestBed.runInInjectionContext(() => (fn as () => T)());
|
||||
|
||||
describe('authGuard', () => {
|
||||
it('allows an authenticated user', () => {
|
||||
setup({ authed: true });
|
||||
expect(call(authGuard)).toBe(true);
|
||||
});
|
||||
|
||||
it('redirects an anonymous user to /login', () => {
|
||||
const { createUrlTree } = setup({ authed: false });
|
||||
expect(call(authGuard)).toEqual({ tree: ['/login'] });
|
||||
expect(createUrlTree).toHaveBeenCalledWith(['/login']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('capabilityGuard', () => {
|
||||
const guard = () => capabilityGuard('stamdata:edit');
|
||||
|
||||
it('waits for /me, then allows an entitled admin', async () => {
|
||||
const { readySpy } = setup({ authed: true, can: (c) => c === 'stamdata:edit' });
|
||||
await expect(call<Promise<unknown>>(guard())).resolves.toBe(true);
|
||||
expect(readySpy).toHaveBeenCalledOnce(); // it awaited caps before deciding
|
||||
});
|
||||
|
||||
it('sends an authenticated-but-unentitled user to /dashboard (not a login loop)', async () => {
|
||||
setup({ authed: true, can: () => false });
|
||||
await expect(call<Promise<unknown>>(guard())).resolves.toEqual({ tree: ['/dashboard'] });
|
||||
});
|
||||
|
||||
it('redirects an anonymous user to /login without waiting for caps', async () => {
|
||||
const { readySpy } = setup({ authed: false, can: () => true });
|
||||
await expect(call<Promise<unknown>>(guard())).resolves.toEqual({ tree: ['/login'] });
|
||||
expect(readySpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -13,19 +13,22 @@ export const authGuard: CanActivateFn = () => {
|
||||
|
||||
/**
|
||||
* Route guard factory (PRD-0002 §6): authenticated AND holding `capability`, else
|
||||
* redirect. No route in this app currently needs a capability gate — brief's
|
||||
* canApprove/canReject/canSend are per-action, not per-page (both actors land on
|
||||
* the same `/brief` page and see different actions) — so this exists as the
|
||||
* available building block for the day a route-level gate is needed, e.g. a future
|
||||
* approver-only page.
|
||||
* redirect. Used by the admin pages (`/brief/huisstijl`, `/beheer/stamdata`).
|
||||
*
|
||||
* **Async on purpose:** `can()` is deny-by-default, so it must not be read while `/me`
|
||||
* is still loading — it would deny an entitled admin and bounce them. We await
|
||||
* `AccessStore.whenReady()` (caps resolved) before deciding. An unauthenticated user
|
||||
* goes to `/login`; an authenticated-but-unentitled user goes to `/dashboard` (they're
|
||||
* logged in, just not allowed here — no re-login loop). The backend re-enforces
|
||||
* regardless (403); this guard is the UX pre-gate.
|
||||
*/
|
||||
export function capabilityGuard(capability: Capability): CanActivateFn {
|
||||
return () => {
|
||||
return async () => {
|
||||
const session = inject(SessionStore);
|
||||
const access = inject(AccessStore);
|
||||
const router = inject(Router);
|
||||
return session.isAuthenticated() && access.can(capability)
|
||||
? true
|
||||
: router.createUrlTree(['/login']);
|
||||
if (!session.isAuthenticated()) return router.createUrlTree(['/login']);
|
||||
await access.whenReady();
|
||||
return access.can(capability) ? true : router.createUrlTree(['/dashboard']);
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { Injectable, computed, inject } from '@angular/core';
|
||||
import { toObservable } from '@angular/core/rxjs-interop';
|
||||
import { filter, firstValueFrom } from 'rxjs';
|
||||
import { RemoteData, fromResource } from '@shared/application/remote-data';
|
||||
import { Capability } from '@shared/domain/capability';
|
||||
import { MeAdapter, parseMe } from '@shared/infrastructure/me.adapter';
|
||||
@@ -40,4 +42,13 @@ export class AccessStore {
|
||||
const tag = this.capabilities().tag;
|
||||
return tag === 'Success' || tag === 'Failure';
|
||||
});
|
||||
|
||||
private ready$ = toObservable(this.ready);
|
||||
/** Resolves once `/me` has settled (success or failure). The `capabilityGuard` awaits
|
||||
this before deciding — otherwise it reads `can()` while `/me` is still loading and
|
||||
wrongly denies (deny-by-default), bouncing even an entitled user. */
|
||||
async whenReady(): Promise<void> {
|
||||
if (this.ready()) return;
|
||||
await firstValueFrom(this.ready$.pipe(filter((r) => r)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,12 +1,17 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { roleInterceptor } from './role.interceptor';
|
||||
|
||||
// currentRole() reads window.location; pin it so the test is about routing, not the shim.
|
||||
vi.mock('./role', () => ({ currentRole: () => 'admin' }));
|
||||
// currentRole() reads window.location.search; set it via the real URL rather than
|
||||
// vi.mock (the Angular unit-test system forbids mocking relative imports).
|
||||
beforeEach(() => window.history.replaceState({}, '', '/?role=admin'));
|
||||
afterEach(() => {
|
||||
window.history.replaceState({}, '', '/');
|
||||
sessionStorage.clear(); // currentRole() now persists the dev role; don't leak across tests
|
||||
});
|
||||
|
||||
// Minimal stand-in for HttpRequest — the interceptor only reads `url` and calls
|
||||
// `clone({ setHeaders })`. Avoids importing @angular/common/http at runtime (its XHR
|
||||
// chunk needs the JIT compiler under vitest).
|
||||
// `clone({ setHeaders })`. Avoids importing @angular/common/http (its XHR chunk needs
|
||||
// the JIT compiler under vitest).
|
||||
function fakeReq(url: string) {
|
||||
const make = (headers: Map<string, string>) => ({
|
||||
url,
|
||||
|
||||
@@ -3,14 +3,29 @@ import { Role } from '@shared/domain/role';
|
||||
/**
|
||||
* Dev-only role stand-in (the reading MECHANISM; the `Role` type is domain). This
|
||||
* POC has one faked self-service user and no real identities, so the two-person
|
||||
* letter workflow (drafter vs approver) is driven by a `?role=` query param —
|
||||
* exactly the pattern of the `?scenario=` toggle. The backend receives it as an
|
||||
* `X-Role` header (see role.interceptor), resolves it into a `Principal`
|
||||
* server-side, and is the sole authority on what that principal may do (PRD-0002
|
||||
* phase P1, `Authz.Can`) — the FE only renders the resulting decision flags, it no
|
||||
* longer derives permission from this value itself.
|
||||
* letter workflow (drafter vs approver) plus admin is driven by a `?role=` query
|
||||
* param. The backend receives it as an `X-Role` header (see role.interceptor),
|
||||
* resolves it into a `Principal` server-side, and is the sole authority on what that
|
||||
* principal may do (PRD-0002 phase P1, `Authz.Can`) — the FE only renders the
|
||||
* resulting decision flags, it no longer derives permission from this value itself.
|
||||
*
|
||||
* **Sticky within the tab (sessionStorage):** the interceptor reads this per request,
|
||||
* but navigation drops the query param (login redirects to /dashboard, RouterLinks
|
||||
* don't carry it), which would silently revert an admin to drafter mid-session and
|
||||
* 403 the admin endpoints. So a `?role=` seen in the URL is remembered for the tab;
|
||||
* later requests use the remembered value. Set `?role=drafter` (or a fresh tab) to
|
||||
* reset. Dev-only — the interceptor itself is only wired under `isDevMode()`.
|
||||
*/
|
||||
const STORAGE_KEY = 'dev-role';
|
||||
const isRole = (v: string | null): v is Role =>
|
||||
v === 'drafter' || v === 'approver' || v === 'admin';
|
||||
|
||||
export function currentRole(): Role {
|
||||
const role = new URLSearchParams(window.location.search).get('role');
|
||||
return role === 'approver' || role === 'admin' ? role : 'drafter';
|
||||
const fromUrl = new URLSearchParams(window.location.search).get('role');
|
||||
if (isRole(fromUrl)) {
|
||||
sessionStorage.setItem(STORAGE_KEY, fromUrl);
|
||||
return fromUrl;
|
||||
}
|
||||
const stored = sessionStorage.getItem(STORAGE_KEY);
|
||||
return isRole(stored) ? stored : 'drafter';
|
||||
}
|
||||
|
||||
@@ -3,6 +3,8 @@ import { toSignal } from '@angular/core/rxjs-interop';
|
||||
import { NavigationEnd, Router, RouterLink, RouterLinkActive } from '@angular/router';
|
||||
import { filter, map } from 'rxjs/operators';
|
||||
import { SESSION_PORT } from '@shared/application/session.port';
|
||||
import { AccessStore } from '@shared/application/access.store';
|
||||
import { Capability } from '@shared/domain/capability';
|
||||
import { BreadcrumbComponent } from '@shared/layout/breadcrumb/breadcrumb.component';
|
||||
import { trailFor } from '@shared/layout/breadcrumb/breadcrumb-trail';
|
||||
|
||||
@@ -18,6 +20,21 @@ const NAV_ITEMS: readonly HeaderNavItem[] = [
|
||||
{ label: $localize`:@@header.nav.inschrijven:Inschrijven`, to: '/registreren' },
|
||||
];
|
||||
|
||||
/** Admin-only nav, shown only when `/me` grants the matching capability — the pages
|
||||
are otherwise reachable by URL alone. */
|
||||
const ADMIN_NAV_ITEMS: readonly (HeaderNavItem & { readonly cap: Capability })[] = [
|
||||
{
|
||||
label: $localize`:@@header.nav.huisstijl:Huisstijl`,
|
||||
to: '/brief/huisstijl',
|
||||
cap: 'orgtemplate:edit',
|
||||
},
|
||||
{
|
||||
label: $localize`:@@header.nav.stamdata:Stamdata`,
|
||||
to: '/beheer/stamdata',
|
||||
cap: 'stamdata:edit',
|
||||
},
|
||||
];
|
||||
|
||||
/** Organism: CIBG Huisstijl site header — logo block, robijn titlebar (breadcrumb +
|
||||
user menu), horizontal nav. ponytail: text wordmark, not the licensed Rijksoverheid
|
||||
beeldmerk; no search box (no search feature yet). */
|
||||
@@ -90,6 +107,11 @@ const NAV_ITEMS: readonly HeaderNavItem[] = [
|
||||
<a [routerLink]="item.to">{{ item.label }}</a>
|
||||
</li>
|
||||
}
|
||||
@for (item of adminItems(); track item.to) {
|
||||
<li routerLinkActive="active" [routerLinkActiveOptions]="{ exact: true }">
|
||||
<a [routerLink]="item.to">{{ item.label }}</a>
|
||||
</li>
|
||||
}
|
||||
</ul>
|
||||
</div>
|
||||
</nav>
|
||||
@@ -101,6 +123,10 @@ export class SiteHeaderComponent {
|
||||
|
||||
private router = inject(Router);
|
||||
private sessionPort = inject(SESSION_PORT, { optional: true });
|
||||
private access = inject(AccessStore);
|
||||
/** Injecting AccessStore here also warms `/me` at app start (the header renders on
|
||||
every page), so the admin routes' guard usually finds caps already resolved. */
|
||||
protected adminItems = computed(() => ADMIN_NAV_ITEMS.filter((i) => this.access.can(i.cap)));
|
||||
|
||||
readonly session = computed(() => this.sessionPort?.session() ?? null);
|
||||
private url = toSignal(
|
||||
|
||||
@@ -1,12 +1,24 @@
|
||||
import type { Meta, StoryObj } from '@storybook/angular';
|
||||
import { applicationConfig } from '@storybook/angular';
|
||||
import { provideRouter } from '@angular/router';
|
||||
import { AccessStore } from '@shared/application/access.store';
|
||||
import { Capability } from '@shared/domain/capability';
|
||||
import { SiteHeaderComponent } from './site-header.component';
|
||||
|
||||
// The header injects AccessStore for the capability-gated admin links; stub it so the
|
||||
// story needs no HTTP/ApiClient. `can` decides which admin links appear.
|
||||
const withCaps = (caps: Capability[]) =>
|
||||
applicationConfig({
|
||||
providers: [
|
||||
provideRouter([]),
|
||||
{ provide: AccessStore, useValue: { can: (c: Capability) => caps.includes(c) } },
|
||||
],
|
||||
});
|
||||
|
||||
const meta: Meta<SiteHeaderComponent> = {
|
||||
title: 'Design System/Organisms/Site Header',
|
||||
component: SiteHeaderComponent,
|
||||
decorators: [applicationConfig({ providers: [provideRouter([])] })],
|
||||
decorators: [withCaps([])],
|
||||
render: (args) => ({
|
||||
props: args,
|
||||
template: `<app-site-header />`,
|
||||
@@ -15,4 +27,10 @@ const meta: Meta<SiteHeaderComponent> = {
|
||||
export default meta;
|
||||
type Story = StoryObj<SiteHeaderComponent>;
|
||||
|
||||
/** Standard user — no admin links. */
|
||||
export const Default: Story = {};
|
||||
|
||||
/** Admin — the capability-gated Huisstijl + Stamdata links appear. */
|
||||
export const AsAdmin: Story = {
|
||||
decorators: [withCaps(['orgtemplate:edit', 'stamdata:edit'])],
|
||||
};
|
||||
|
||||
@@ -3582,6 +3582,22 @@
|
||||
<context context-type="linenumber">102</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.huisstijl" datatype="html">
|
||||
<source>Huisstijl</source>
|
||||
<target datatype="html">House style</target>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">26</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.stamdata" datatype="html">
|
||||
<source>Stamdata</source>
|
||||
<target datatype="html">Master data</target>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">27</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
</body>
|
||||
</file>
|
||||
</xliff>
|
||||
|
||||
+22
-8
@@ -2592,56 +2592,70 @@
|
||||
<source>Overzicht</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">15</context>
|
||||
<context context-type="linenumber">17</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.gegevens" datatype="html">
|
||||
<source>Mijn gegevens</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">16</context>
|
||||
<context context-type="linenumber">18</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.herregistratie" datatype="html">
|
||||
<source>Herregistratie</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">17</context>
|
||||
<context context-type="linenumber">19</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.inschrijven" datatype="html">
|
||||
<source>Inschrijven</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">18</context>
|
||||
<context context-type="linenumber">20</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.huisstijl" datatype="html">
|
||||
<source>Huisstijl</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">26</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.nav.stamdata" datatype="html">
|
||||
<source>Stamdata</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">27</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.sender" datatype="html">
|
||||
<source>BIG-register</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">53,54</context>
|
||||
<context context-type="linenumber">62,63</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.ministry" datatype="html">
|
||||
<source>Ministerie van Volksgezondheid, Welzijn en Sport</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">55,57</context>
|
||||
<context context-type="linenumber">64,66</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.uitloggen" datatype="html">
|
||||
<source> Uitloggen </source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">77,78</context>
|
||||
<context context-type="linenumber">86,87</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="header.navAria" datatype="html">
|
||||
<source>Hoofdnavigatie</source>
|
||||
<context-group purpose="location">
|
||||
<context context-type="sourcefile">src/app/shared/layout/site-header/site-header.component.ts</context>
|
||||
<context context-type="linenumber">85,86</context>
|
||||
<context context-type="linenumber">94,95</context>
|
||||
</context-group>
|
||||
</trans-unit>
|
||||
<trans-unit id="wizard.naarStap" datatype="html">
|
||||
|
||||
Reference in New Issue
Block a user