Skip to content

Commit faac672

Browse files
committed
test(nav): specify the manifest permission gate and prove its server input in E2E
Gate 16 failed this PR on four changed methods with no @SPEC. They are the behavioural core of the fail-open fix, so they get a real requirement rather than an exclusion: REQ-RBAC-008 in authorization-via-or-rbac says a manifest permission gates both the nav entry and the route, that the route guard fails closed, and that the permission list comes from the server and is never empty. The scenario a browser can observe is covered by a new Playwright spec: an administrator's dashboard receives initial state isAdmin true, and a freshly created account in no group receives false, asserted with toBe(false) so an absent value cannot pass as a no. The account's empty group list is proven before it is relied on. The other three scenarios carry reason-bearing exclusions naming the vitest cases that pin them: no shipped page declares a permission and the manifest is bundled at build time, so a browser has no gated page to visit; and the guard and list inputs they cover cannot be produced by a real page load.
1 parent 849de09 commit faac672

5 files changed

Lines changed: 234 additions & 0 deletions

File tree

lib/Controller/DashboardController.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ public function catchAll(): TemplateResponse {
114114
* half-booted request denies rather than permits.
115115
*
116116
* @return TemplateResponse The rendered Decidiq index template.
117+
*
118+
* @spec openspec/specs/authorization-via-or-rbac/spec.md#requirement-req-rbac-008-a-manifest-permission-gates-the-nav-entry-and-the-route-and-fails-closed
117119
*/
118120
protected function renderIndex(): TemplateResponse {
119121
$user = $this->userSession->getUser();

openspec/specs/authorization-via-or-rbac/spec.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,44 @@ object and property rules alike and is not constrained by this requirement.
206206
- **GIVEN** an authenticated account in no group at all
207207
- **WHEN** that account creates a Decision through OpenRegister's object API
208208
- **THEN** the Decision is created.
209+
210+
### Requirement: REQ-RBAC-008 A manifest `permission` gates the nav entry and the route, and fails closed
211+
A `permission` declared on a manifest menu entry or page SHALL restrict both halves of the SPA: the
212+
nav entry SHALL NOT render, and direct navigation to the page's route SHALL NOT render it, for an
213+
account that does not hold that permission. The permission list SHALL come from the server: the
214+
dashboard page SHALL publish `isAdmin` into initial state from Nextcloud's own admin test on the
215+
acting user, defaulting to `false` when there is no session. The frontend SHALL build a list that is
216+
never empty (`user`, plus `admin` only when initial state is exactly `true`), because the library's
217+
nav filter reads an empty list as "the app did not say" and renders the entry.
218+
219+
The route guard SHALL fail closed: a page that declares a permission SHALL be refused when the list
220+
is absent, empty or not an array, and a refused navigation SHALL redirect to the app root. This is
221+
presentation only. It stops a page rendering in the SPA; every endpoint that page calls SHALL still
222+
enforce its own access server-side, and nothing SHALL depend on the guard for authorization.
223+
224+
#### Scenario: The server tells the SPA whether the account is an administrator
225+
- **GIVEN** a Nextcloud administrator and an account in no group at all
226+
- **WHEN** each opens the decidiq dashboard
227+
- **THEN** initial state `isAdmin` is `true` for the administrator and `false` for the other account
228+
229+
#### Scenario: A page that declares a permission is refused on direct navigation
230+
- **GIVEN** a manifest page that declares `permission: "admin"`
231+
- **WHEN** an account without `admin` navigates straight to that page's route
232+
- **THEN** the page does not render and the router redirects to the app root
233+
- **AND** the page's nav entry is not rendered for that account
234+
235+
@e2e exclude No page or menu entry in decidiq's shipped manifests declares a `permission` today, and the manifest is bundled at build time (`require.context` in `src/main.js`), so a browser test has no gated page to navigate to and cannot add one. The behaviour is driven through both halves, the router guard and CnAppNav's filter, for a gated manifest page by `tests/vitest/navPermissions.spec.js` ("a manifest page, end to end through both halves"). Add a Playwright test here in the same change that first gates a real page.
236+
237+
#### Scenario: A gated route is refused when the permission list is missing
238+
- **GIVEN** a manifest page that declares a permission
239+
- **WHEN** the route guard receives no permission list, an empty list, or a value that is not an array
240+
- **THEN** the navigation is refused
241+
242+
@e2e exclude These inputs cannot occur in a browser: `currentPermissions()` always hands the guard a non-empty array, so a page load can never deliver an empty or malformed list to it. The fail-closed contract is pinned on the guard function itself by `tests/vitest/navPermissions.spec.js` ("fails CLOSED on an empty or malformed list, unlike CnAppNav").
243+
244+
#### Scenario: The permission list is never empty and grants admin only on a real boolean true
245+
- **GIVEN** initial state `isAdmin` of boolean `true`, boolean `false`, absent, or the string `"false"`
246+
- **WHEN** the frontend builds the permission list
247+
- **THEN** the list is `["user", "admin"]` for boolean `true` and `["user"]` for every other value, never empty
248+
249+
@e2e exclude The server only ever publishes a real boolean (see the scenario above, which a Playwright test covers), so the absent and string values this scenario guards against cannot be produced by a page load, and the list itself is a module-local value no browser test can read. Pinned by `tests/vitest/navPermissions.spec.js` ("never returns an empty list" and "grants admin only for a real boolean true").

src/utils/manifestRoutes.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
* @param {object} manifest The merged manifest (with `pages[]`).
3333
* @param {object} component The component every route renders.
3434
* @return {Array<object>} vue-router 4 routes config.
35+
* @spec openspec/specs/authorization-via-or-rbac/spec.md#requirement-req-rbac-008-a-manifest-permission-gates-the-nav-entry-and-the-route-and-fails-closed
3536
*/
3637
export function routesFromManifest(manifest, component) {
3738
const routes = (manifest.pages ?? []).map((page) => ({
@@ -69,6 +70,7 @@ export function routesFromManifest(manifest, component) {
6970
* @param {object} to The vue-router target route.
7071
* @param {Array<string>} permissions The permissions this account holds.
7172
* @return {boolean|object} `true` to allow, or a redirect location to refuse.
73+
* @spec openspec/specs/authorization-via-or-rbac/spec.md#requirement-req-rbac-008-a-manifest-permission-gates-the-nav-entry-and-the-route-and-fails-closed
7274
*/
7375
export function permissionGuard(to, permissions) {
7476
const required = to?.meta?.permission

src/utils/permissions.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export const ADMIN_PERMISSION = 'admin'
4545
*
4646
* @param {boolean} isAdmin The server's own answer, from initial state.
4747
* @return {Array<string>} A never-empty list of permission strings.
48+
* @spec openspec/specs/authorization-via-or-rbac/spec.md#requirement-req-rbac-008-a-manifest-permission-gates-the-nav-entry-and-the-route-and-fails-closed
4849
*/
4950
export function currentPermissions(isAdmin) {
5051
const permissions = [BASE_PERMISSION]
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2026 Decidiq Contributors
3+
* SPDX-License-Identifier: EUPL-1.2
4+
*
5+
* Where the SPA's permission list comes from (decidiq#1267).
6+
*
7+
* The nav filter and the route guard both read one list, built in
8+
* `src/utils/permissions.js` from a single server-provided input: initial
9+
* state `isAdmin`, published by `DashboardController::renderIndex()` from
10+
* Nextcloud's own admin test on the acting user. Before #1267 the list was
11+
* derived from `window.OC.currentUser.permissions`, a property that does not
12+
* exist (OC.currentUser is the uid STRING), so the list was always `[]` and
13+
* the nav filter failed open for everyone.
14+
*
15+
* This spec asserts the half a browser can observe: what each account's page
16+
* load actually receives. How that value becomes a list, and how the guard
17+
* treats the list, is pinned in `tests/vitest/navPermissions.spec.js`; the
18+
* scenarios for those carry reason-bearing exclusions in the spec.
19+
*
20+
* WHY A REAL, NON-ADMIN ACCOUNT. The project's storage state is the `admin`
21+
* session, and admin is the one account for which the old fail-open bug and
22+
* a correct implementation look identical: both show everything. Only a
23+
* second account in no group at all can tell "the server said false" from
24+
* "nobody said anything". Every assertion names the account it is about.
25+
*
26+
* WHY A BROWSER LOGIN, NOT HTTP BASIC. The scenario is about what a person's
27+
* page load receives, so each account logs in through the login form in its
28+
* own context, exactly as the global setup does for admin.
29+
*
30+
* @e2e openspec/specs/authorization-via-or-rbac/spec.md#the-server-tells-the-spa-whether-the-account-is-an-administrator
31+
*/
32+
import type { APIResponse, Browser, Page } from '@playwright/test'
33+
34+
import { expect, test } from '@playwright/test'
35+
import { randomBytes } from 'node:crypto'
36+
import { BASE_URL as BASE } from '../base-url.ts'
37+
38+
const ADMIN_USER = process.env.NC_ADMIN_USER ?? 'admin'
39+
const ADMIN_PASS = process.env.NC_ADMIN_PASS ?? 'admin'
40+
41+
// From a CSPRNG, not Math.random(): the password below is built from it.
42+
const RUN_ID = `${Date.now()}-${randomBytes(6).toString('hex')}`
43+
44+
/** An account in no group at all, created for this run and deleted after it. */
45+
const MEMBER = {
46+
uid: `dq1267-member-${RUN_ID}`,
47+
password: `Dq1267-member-${RUN_ID}-pw`,
48+
}
49+
50+
/**
51+
* OCS headers. `OCS-APIRequest: true` is required: without it Nextcloud answers
52+
* an OCS call with 412 (CSRF) instead of processing it.
53+
*/
54+
const OCS = { Accept: 'application/json', 'OCS-APIRequest': 'true' }
55+
56+
/**
57+
* Read a response body for an assertion message without letting a non-JSON
58+
* body throw inside the message builder.
59+
*
60+
* @param resp The response to describe.
61+
* @return A short printable description.
62+
*/
63+
async function summarise(resp: APIResponse): Promise<string> {
64+
const text = await resp.text().catch(() => '<unreadable body>')
65+
return `HTTP ${resp.status()} ${text.slice(0, 400)}`
66+
}
67+
68+
/**
69+
* Log in through the form in a fresh context, with no inherited cookies, and
70+
* open the decidiq dashboard.
71+
*
72+
* @param browser The Playwright browser.
73+
* @param username The account to log in as.
74+
* @param password Its password.
75+
* @return The page, parked on the decidiq dashboard.
76+
*/
77+
async function openDashboardAs(
78+
browser: Browser,
79+
username: string,
80+
password: string,
81+
): Promise<Page> {
82+
const context = await browser.newContext({
83+
baseURL: BASE,
84+
storageState: { cookies: [], origins: [] },
85+
})
86+
const page = await context.newPage()
87+
await page.goto('/index.php/login', { timeout: 60_000 })
88+
await page.locator('input[name="user"]').fill(username)
89+
await page.locator('input[name="password"]').fill(password)
90+
await page.locator('button[type="submit"]').first().click()
91+
await page.waitForURL((url) => !/\/login(\?|$|\/)/.test(url.pathname), {
92+
timeout: 60_000,
93+
})
94+
await page.goto('/index.php/apps/decidiq/', { timeout: 60_000 })
95+
return page
96+
}
97+
98+
/**
99+
* The value Nextcloud rendered for decidiq's `isAdmin` initial state.
100+
*
101+
* Nextcloud writes each initial state as a hidden input whose value is the
102+
* base64 of the JSON-encoded value. Read it from the DOM the server sent,
103+
* not from any variable the app derives from it.
104+
*
105+
* @param page A page on the decidiq dashboard.
106+
* @param account The account name, for the assertion message.
107+
* @return The decoded value.
108+
*/
109+
async function isAdminStateOf(page: Page, account: string): Promise<unknown> {
110+
const input = page.locator('#initial-state-decidiq-isAdmin')
111+
await expect(
112+
input,
113+
`${account}: the dashboard must publish initial state decidiq/isAdmin`,
114+
).toHaveCount(1, { timeout: 30_000 })
115+
const raw = await input.getAttribute('value')
116+
expect(raw, `${account}: initial state decidiq/isAdmin has no value`).not.toBeNull()
117+
return JSON.parse(Buffer.from(raw as string, 'base64').toString('utf8'))
118+
}
119+
120+
test.describe('decidiq#1267: the server tells the SPA whether the account is an administrator', () => {
121+
test.beforeAll(async ({ playwright }) => {
122+
const admin = await playwright.request.newContext({
123+
httpCredentials: { password: ADMIN_PASS, send: 'always', username: ADMIN_USER },
124+
storageState: { cookies: [], origins: [] },
125+
})
126+
const created = await admin.post(`${BASE}/ocs/v2.php/cloud/users?format=json`, {
127+
data: { password: MEMBER.password, userid: MEMBER.uid },
128+
headers: OCS,
129+
})
130+
expect(created.ok(), `creating account ${MEMBER.uid}: ${await summarise(created)}`).toBe(
131+
true,
132+
)
133+
134+
// The whole point of this account is that it holds nothing. Prove it
135+
// before relying on it, so a stray default group cannot make the
136+
// "false" assertion below pass for the wrong reason, or fail for one.
137+
const groups = await admin.get(
138+
`${BASE}/ocs/v2.php/cloud/users/${MEMBER.uid}/groups?format=json`,
139+
{ headers: OCS },
140+
)
141+
expect(groups.ok(), `reading the groups of ${MEMBER.uid}: ${await summarise(groups)}`).toBe(
142+
true,
143+
)
144+
const body = await groups.json()
145+
expect(
146+
body?.ocs?.data?.groups ?? [],
147+
`${MEMBER.uid} must be in no group at all, in particular not in "admin"`,
148+
).toEqual([])
149+
await admin.dispose()
150+
})
151+
152+
test.afterAll(async ({ playwright }) => {
153+
const admin = await playwright.request.newContext({
154+
httpCredentials: { password: ADMIN_PASS, send: 'always', username: ADMIN_USER },
155+
storageState: { cookies: [], origins: [] },
156+
})
157+
await admin.delete(`${BASE}/ocs/v2.php/cloud/users/${MEMBER.uid}?format=json`, {
158+
headers: OCS,
159+
})
160+
await admin.dispose()
161+
})
162+
163+
test('an administrator receives isAdmin true', async ({ browser }) => {
164+
const page = await openDashboardAs(browser, ADMIN_USER, ADMIN_PASS)
165+
try {
166+
expect(
167+
await isAdminStateOf(page, ADMIN_USER),
168+
`${ADMIN_USER} (a Nextcloud administrator) must receive isAdmin === true`,
169+
).toBe(true)
170+
} finally {
171+
await page.context().close()
172+
}
173+
})
174+
175+
test('an account in no group receives isAdmin false, not nothing', async ({ browser }) => {
176+
const page = await openDashboardAs(browser, MEMBER.uid, MEMBER.password)
177+
try {
178+
// toBe(false), not toBeFalsy(): an absent value is exactly the
179+
// fail-open input #1267 removed, and must not pass as a "no".
180+
expect(
181+
await isAdminStateOf(page, MEMBER.uid),
182+
`${MEMBER.uid} (in no group) must receive isAdmin === false`,
183+
).toBe(false)
184+
} finally {
185+
await page.context().close()
186+
}
187+
})
188+
})

0 commit comments

Comments
 (0)