fix(rbac): grant authenticated read on every openbuild schema (#76) - #81
Merged
Merged
Conversation
Every openbuild schema declared
"authorization": { "create": ["admin"], "update": ["admin"], "delete": ["admin"] }
— non-empty, and with no `read` key. OpenRegister treats that as fail-closed:
`MagicRbacHandler::buildRbacConditionsSql()` bypasses filtering only for an
EMPTY block; a populated block with no `read` rule falls through to the owner
condition alone, so every non-admin caller saw zero rows. Not a bug in OR —
deliberate, and commented as such at MagicRbacHandler:1031.
That is the whole of #76. An owner could grant a colleague editor or viewer on
an app and they still saw an empty list, because OR filtered the objects out
one layer below openbuild's own permission check.
Adds `"read": ["authenticated"]` to all 15 schemas — 6 in the monolith and 9
across the register.d fragments, which were missed by the first pass and would
have left business rules, automations, component blocks and the agent
workspace owner-only.
`authenticated` requires $userId !== null (MagicRbacHandler:414), so anonymous
callers are NOT granted. This is intentionally the coarse layer: appinfo/routes.php
already documents that OR's schema read rule is a group ACL, not a row filter,
and that the per-app `permissions` block is enforced by /api/applications. Both
layers verified live.
Measured on the disposable instance after a FORCED re-import:
caller OR object API openbuild /api/applications
admin 21 21
rbac-editor 0 -> 21 1 (granted editor on pw-verchain)
rbac-viewer 0 -> 21 1 (granted viewer on pw-verchain)
rbac-outsider 0 -> 21 0 (no grant)
anonymous 0 401
Diagnosis trail: ConductionNL/openregister#2252.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 659/659 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Coverage: 89.4% (17/19 statements)
Quality workflow — 2026-08-01 21:24 UTC
Download the full PDF report from the workflow artifacts.
This was referenced Aug 1, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 2, 2026
…ns can use the app (#87) Picks up ConductionNL/nextcloud-vue#576: a 401/403 from /api/setup/status now empties the unmet-steps lists, so a non-admin is no longer shown the first-time-setup wizard. Why this bump matters more than the version delta suggests: after #81 gave non-admins OpenRegister-level read on the schemas, an editor still could not use OpenBuild. They landed on "Welcome to OpenBuild / Set up this app" with .openbuild-schema-list count 0 — the setup endpoints are admin-only, they answer 403, and useSetupStatus read that as "setup unfinished". Also corrects a drift found on the way: node_modules held 2.1.0-vue3.7 while package.json and the lockfile both said 2.1.0-vue3.13. Nobody had run an install since that bump, so local builds were linking a library three versions behind the pin. This commit was built from a clean `npm ci`. New: tests/e2e/non-admin-access.spec.ts, a regression suite for the OUTCOME rather than any one layer. Two consecutive fixes for this looked green while the app stayed broken for the user — openbuild#76's grant, then a nc-vue fix that short-circuited `completed`, which CnAppRoot never reads. The only assertion that would have caught both is "a non-admin sees the app", so that is what these three tests assert: - an editor reaches the schema designer AND no setup wizard (both halves — asserting the wizard's absence alone passes on a blank page) - an editor sees the app they were granted - an outsider sees none — the control proving openbuild's row-level filter still runs on top of OR's coarse `authenticated` read grant Verified live against the disposable instance: 3 passed. Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #76.
Root cause
Every openbuild schema declared a non-empty authorization block with no
readkey:OpenRegister treats that as fail-closed.
MagicRbacHandler::buildRbacConditionsSql()bypasses filtering only for an empty block; a populated block with noreadrule produces$rules = []and falls through to the owner condition alone — so every non-admin caller matched nothing and saw zero rows.That is not an OR bug. It is deliberate, and commented as such at
MagicRbacHandler:1031.It is the whole of #76: an owner could grant a colleague editor or viewer on an app, the grant landed and read back correctly, and they still saw an empty list — because OR filtered the objects out one layer below openbuild's own permission check.
The fix
"read": ["authenticated"]on all 15 schemas — 6 inopenbuild_register.jsonand 9 acrossregister.d/fragments. The fragments were missed on the first pass and would have left business rules, automations, component blocks and the agent workspace owner-only.authenticatedrequires$userId !== null(MagicRbacHandler:414), so anonymous callers are not granted.This is intentionally the coarse layer.
appinfo/routes.phpalready documents that OR's schema read rule is a group ACL rather than a row filter, and that the per-apppermissionsblock is enforced by/api/applications. Both layers are verified below.Verification — live, after a FORCED re-import
A normal import advances the version without applying the change; this used
POST /api/settings/load./api/applicationsrbac-editor(editor onpw-verchain)rbac-viewer(viewer onpw-verchain)rbac-outsider(no grant)Both layers behave as designed: OR grants the coarse authenticated read, openbuild's endpoint filters it down by app role, and anonymous is excluded at both.
All 15 schemas re-read from the live API to confirm the authorization actually applied, rather than trusting the JSON.
🤖 Generated with Claude Code