Group-based app grants do not make an Application visible to its editors/viewers

[rubenvdlinde]

Symptom

An Application granted to a group is still invisible to members of that group: GET /api/applications returns 200 with an empty list for both an editor and a viewer.

Measured on a live instance

Everything openbuild-side checks out:

Checked	Result
Stored grant	{owners:['user:admin'], editors:['group:rbac-editors'], viewers:['group:rbac-viewers']} — reads back correctly
Group membership	rbac-viewer ∈ rbac-viewers, rbac-editor ∈ rbac-editors (OCS cloud/users/{uid}/groups)
Groups exist	admin, openregister, rbac-editors, rbac-viewers
Principal grammar	PermissionResolver::matchesCaller() classifies group: principals and intersects them with the caller's groups
Result	GET /api/applications → 200, empty, for both users

So the manifest permissions block is correct and the matcher supports it, yet the object never reaches the filter. Most likely candidate: OpenRegister-level object visibility — a grant separate from openbuild's permissions block. Worth checking whether the list is scoped by OR organisation/RBAC before openbuild's own check runs.

Why it matters beyond tests

If group grants don't confer visibility, then in the product an owner can add a team to an app and those users still see nothing — the affordance exists and does not work.

Blocks

• tests/e2e/versionRouting.spec.ts 9.2 (viewer sees version-not-found, not a stack trace)
• tests/e2e/schema-access-scopes-rbac.spec.ts (3 scenarios: lock-out warning, draft-vs-production scope isolation, disabled Access sub-editor on production)
• the real viewer-gating assertion omitted from save-as-template.spec.ts

Groundwork is already in the repo: tests/e2e/support/appRoles.ts writes the grant correctly (carrying the whole record forward — OR saves are PUT-semantic) and documents these findings inline. The RBAC users and one stored session per role are provisioned by globalSetup (#66). Once a member can see the app, those four scenarios should be straightforward.

[rubenvdlinde]

Narrowed: the objects never reach openbuild's filter

Measured at the OpenRegister layer, same endpoint, same moment:

GET /apps/openregister/api/objects/openbuild/application?_limit=50
  admin        -> 200, 21 objects
  rbac-editor  -> 200,  0 objects

So OpenRegister itself returns nothing to a non-admin member. openbuild's own filter is correct and irrelevant here — I read it to be sure:

ApplicationsController::filterApplicationsByRole() calls
PermissionResolver::matchesCaller(..., roles: ['owners','editors','viewers'])
with the caller's groups and allowAdminBypass: false, i.e. it already checks all three role buckets for list visibility. It just never receives the objects.

This is therefore an OpenRegister-side issue, not an openbuild one. Likely candidates, in order of suspicion:

1. organisation scoping on the objects (an OR organisation is a UUID, not an NC group — objects created by admin land in admin's organisation);
2. register/schema-level RBAC on openbuild/application.

Retitling suggestion: the fix belongs in OpenRegister or in how openbuild provisions the register's access, not in the permissions block.

Consequence worth stating plainly

Granting a group editor/viewer on an app currently does nothing observable: the users still see no app. The manifest permissions block is written correctly and the matcher supports it, so the feature looks implemented end to end while being unreachable in practice.

Unblocks when fixed

versionRouting 9.2, schema-access-scopes-rbac (3), and the real viewer-gating assertion in save-as-template. Groundwork is in tests/e2e/support/appRoles.ts (PR #77); RBAC users and per-role sessions come from globalSetup (PR #66).

[rubenvdlinde]

Root cause found — and the obvious fix is the wrong one

The mechanism

OR applies RBAC for non-admins. Its own response metadata says so, same endpoint, different caller:

admin        -> "rbac": false   (admin bypass)  21 objects
rbac-editor  -> "rbac": true                     0 objects

And the openbuild/application schema (id 18) authorises:

{"create": ["admin"], "update": ["admin"], "delete": ["admin"]}

There is no read entry, and OR treats absent-read as closed once RBAC engages. Admins get through on the bypass; every other user gets an empty set. Organisation scoping is NOT involved — both users resolve to the same "Default Organisation" (edf2f535…), which the object belongs to.

Why "just add read" would be a security bug

Opening read to authenticated users would make
GET /apps/openregister/api/objects/openbuild/application return every Application, including its permissions roster, to every logged-in user — exactly the IDOR that appinfo/routes.php warns about at the applications route:

the editor list MUST go through this […] which would leak every Application + permissions to every authed user (IDOR)

So the closed read is deliberate, and it is not the defect.

The actual defect

ApplicationsController queries OR as the calling user and then filters the result with filterApplicationsByRole() — a careful three-role check (owners/editors/viewers, allowAdminBypass: false) that is correct and, for non-admins, is handed an empty array. The app-level permission model is therefore unreachable for exactly the users it exists to serve.

The consistent design is for openbuild to read the Applications with system/elevated privileges and let its own filter be the authority — which is what that filter was written for — rather than being gated twice, once by a mechanism that cannot see app roles.

Why I stopped here rather than patching it

This is a security boundary: the fix moves the trust decision, so it needs the filter proven to run on every path that returns Applications (index, detail, manifest, versions, insights), not just the list. That deserves its own change with tests per path, not a one-line swap at the end of a session.

Unblocks when fixed

versionRouting 9.2, schema-access-scopes-rbac (3), and the viewer-gating assertion in save-as-template. Fixture groundwork is already in tests/e2e/support/appRoles.ts (#77), users and per-role sessions in globalSetup (#66).

[rubenvdlinde]

The real root cause: read visibility is gated on the CREATE permission

PermissionHandler::filterObjectsForPermissions() (openregister, lib/Service/Object/PermissionHandler.php ~line 911) is the object-level visibility filter on the read path. It does this:

// Property-level RBAC ... this loop only decides object-level visibility.
if ($this->hasPermission(
        schema: $schema,
        action: 'create',   // <-- gating READ visibility on CREATE
        userId: $userId,
        objectOwner: $objectOwner,
        _rbac: $_rbac
    ) === false
) {
    continue;   // object hidden
}

The comment directly above states the loop decides visibility — and it asks for create.

This explains the symptom exactly

The openbuild/application schema authorises create: ["admin"]. So every non-admin fails the check and the object is skipped — which is why rbac-editor saw 0 objects while admin saw 21, with no read rule involved at all. The missing read key was a red herring, including in my earlier comments here.

Consequences beyond openbuild

This is schema-agnostic: on any register, a user who may not create objects of a schema cannot see them either, unless something else broadcasts. Read-only consumers are the normal case, so this likely affects far more than Applications.

Revised fix

1. action: 'create' -> action: 'read' in that filter. That alone probably resolves this issue, without opening anything.
2. Then, separately, the users sentinel: OR recognises public (broadcast, includes anonymous) and admin as broadcast in resolveReadGroupIds(), but has no authenticated-only value. Adding users there and in extractGroupIdsFromReadEntries() gives "any logged-in caller, not anonymous" — the thing that could not be expressed.

Not applied here, deliberately

Changing which permission gates visibility is a security-boundary edit in the foundation app every Conduction app depends on. It needs: verification that hasPermission(action:'read') behaves as expected for owner/group/public paths, a check of the other _rbac call sites (filterUuidsForPermissions and the single-object path look similar), and regression coverage across dependent apps. Worth doing properly rather than as a one-word swap.

Also unresolved

Why adding read: ['rbac-editors'] flipped rbac-editor from 0 to 21 objects, when the gate above asks for create. hasPermission() evidently consults read rules somewhere in that path; that interaction should be understood before either change lands.

[rubenvdlinde]

Execution plan for the permanent fix

Recorded here so it survives the session. Phases 0–1 are the fix; 2 is the feature you asked for; 3 is what makes it safe to ship.

Phase 0 — ground truth (must precede any edit)

1. Trace PermissionHandler::hasPermission() end to end: which actions exist, how owner / group / public are resolved, and whether read is honoured the same way create is.
2. Explain the anomaly first: adding read: ['rbac-editors'] flipped rbac-editor from 0 to 21 objects even though the visibility gate asks for create. Something in that path consults read rules. Until that is understood, any fix here is a guess that happens to work.

Phase 1 — the correctness fix

• lib/Service/Object/PermissionHandler.php, filterObjectsForPermissions(): action: 'create' → action: 'read'.
• Audit the sibling call sites for the same mistake before changing any of them:
  • filterUuidsForPermissions()
  • the single-object read path (see the isolation comment in MagicMapper ~5265)
  • RenderObject ~1766
• Preserve fail-closed behaviour: an unresolvable authorization must still hide the object (resolveReadGroupIds already logs "fail-closed" at ~1565).

Phase 2 — the users sentinel

• resolveReadGroupIds() and extractGroupIdsFromReadEntries() special-case public and admin as broadcast. Add users = any authenticated caller, never anonymous.
• The anonymous path (groupId: 'public', ~516/538/647) must NOT match users. That distinction is the entire point of the new value.

Phase 3 — verification matrix (the part that makes it shippable)

Caller	read rule	Expected
anonymous	public	visible
anonymous	users	not visible
authenticated non-member	users	visible at OR, filtered to nothing by openbuild
authenticated	named group	visible only if in that group
admin	any	unchanged
owner-only object	any	unchanged

Plus: openbuild's e2e suite (30+ tests currently green), and the suites of other apps on this register — OR is the foundation, so a wrong gate here hides or exposes data in every Conduction app.

Phase 4 — openbuild follow-through

• Set read: ['users'] on the application schema in lib/Settings/openbuild_register.json (⚠️ register changes need a FORCED import to apply).
• Un-skip versionRouting 9.2, schema-access-scopes-rbac (3), and add the viewer-gating assertion to save-as-template.
• Revert the experimental read: ['rbac-editors'] currently set on schema 18 on the :8099 instance.

Not executed in this session

Phases 1–2 are ~15 lines of code. Phase 0 and Phase 3 are the work, and neither fits in what remains of my context. Shipping an unverified change to the permission gate of the foundation app is the one outcome worse than leaving this open — a wrong read gate either hides data everywhere or exposes it everywhere, silently, exactly like the create gate has been doing.

[rubenvdlinde]

Closed by #81 — and the earlier diagnosis in this thread was wrong

Correction first. The diagnosis I posted above — that OpenRegister's read path gates visibility on the create permission (PermissionHandler::filterObjectsForPermissions, action: 'create') — is not the cause. That function has no production caller: the only repo-wide reference invokes it with _rbac: false, so the check never ran. It read as the live gate because it is public, documented, @spec-annotated, unit-tested and named in the feature docs. One grep for callers would have settled it.

Full correction and cleanup: ConductionNL/openregister#2252.

The actual cause

A non-empty authorization block that omits read fails closed to owner-only rows — deliberately, commented as such at MagicRbacHandler:1031. buildRbacConditionsSql bypasses filtering only for an empty block; a populated block with no read rule produces $rules = [] and falls through to the owner condition alone.

Every openbuild schema declared {create, update, delete: ["admin"]} and no read. So every non-admin saw zero — the app's own permissions grants landed correctly and were then filtered out one layer below.

This also explains the anomaly I filed as an open question: adding read: ['rbac-editors'] flipped 0 → 21 because it supplied the missing rule to the SQL gate. Nothing consulted the create gate.

Fix

read: ["authenticated"] on all 15 schemas — 6 in the monolith, 9 in register.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). Anonymous stays excluded: authenticated requires $userId !== null.

Verified live after a forced re-import:

caller	OR object API	openbuild /api/applications
admin	21	21
rbac-editor (editor on pw-verchain)	0 → 21	1
rbac-viewer (viewer on pw-verchain)	0 → 21	1
rbac-outsider	0 → 21	0
anonymous	0	401

Both layers behave as designed — OR grants the coarse authenticated read, openbuild's endpoint filters by app role.

Two follow-ups this exposed

Neither was reachable before, because non-admins could not see anything at all:

1. fix(setup): a 403 from setup/status is an answer, not a failure nextcloud-vue#574 (merged) — useSetupStatus treated the admin-only endpoint's 403 as "setup unfinished", so every non-admin met a first-time-setup wizard they could not complete instead of the app. openbuild needs a published nc-vue bump to pick this up.
2. Access editor: lock-out warning fires for group MEMBERS too — prefixed vs bare gids #83 — the Access editor's lock-out warning fires for group members too: the dropdown carries group:-prefixed values while the membership check compares bare gids.

versionRouting 9.2 is un-skipped and green as part of this (4 scenarios, with an editor-gets-200 positive control). schema-access-scopes-rbac stays skipped on (1), with its stale blockers replaced by the measured ones.