feat(rbac): add inheritFromPublic flag for opt-out of public-group inheritance (#1439)

[rjzondervan]

Replaces #1440. Same change set, same commits, plus the review-feedback fixes — moved to a hotfix/... branch so the branch-protection check passes (PRs to beta must come from development/main/hotfix/*).

Summary

Adds an opt-out for the implicit "logged-in users inherit public group rights" semantics in RBAC. A new optional inheritFromPublic boolean on the schema/register authorization block — plus a tenant-wide IAppConfig default — lets administrators say "authentication is a strict gate, not a superset of public access" on a per-schema or per-register basis.

• PHP-side: PermissionHandler::resolveInheritFromPublic walks a cascade schema → register → IAppConfig openregister.rbac.inherit_from_public_default → true. null is treated as "unset"; non-boolean values at the schema/register level are also treated as unset (strict-boolean check, warning logged).
• PHP-side enforcement: PermissionHandler::hasPermission skips the hasGroupPermission(public, ...) fallback for authenticated users when the resolved value is false.
• SQL-side enforcement: MagicRbacHandler::applyRbacFilters and buildRbacConditionsSql (UNION path) honour the same flag identically. No silent fail-open — the cascade walk propagates exceptions instead of returning a permissive default.
• Frontend:
  • CnSchemaSecurityTab.vue (in @conduction/nextcloud-vue, separate PR) — per-schema checkbox.
  • RbacConfiguration.vue — tenant-default switch under the existing RBAC settings section.
  • ConfigurationSettingsHandler reads/writes the dedicated IAppConfig key on both /api/settings and /api/settings/rbac.
• Validator: Schema::validateAuthorizationRules accepts inheritFromPublic as an optional sibling of CRUD action keys; non-boolean is rejected at the schema validator and via strict normalization (filter_var FILTER_VALIDATE_BOOLEAN | FILTER_NULL_ON_FAILURE) at the API write paths.
• CSRF: updateRbacSettings no longer carries @NoCSRFRequired — this endpoint is now security-load-bearing and ADR-005 requires CSRF on state-mutating admin endpoints. Frontend uses @nextcloud/axios (sends request token automatically).
• Defaults stay backwards-compatible: unset everywhere → resolves to true → pre-change behaviour.

Review feedback addressed (vs the original #1440)

• 🔴 Blocker — silent fail-open dropped. MagicRbacHandler::resolveInheritFromPublic no longer swallows Throwable and falls back to true. The cascade now propagates failure (5xx) instead of leaking rows when the tenant explicitly opted out.
• 🟡 Strict-bool at cascade levels. Schema- and register-level reads now require literal true/false; string "false" and friends are treated as unset (with a warning), preventing the (bool) "false" foot-gun for direct mapper writes / migrations / seed JSON.
• 🟡 Strict normalization at API write paths. updateRbacSettingsOnly and updateSettings now use filter_var and reject garbage. The string "false" is correctly coerced to boolean false (matching the docs' tolerance claim) instead of silently flipping to true.
• 🟡 CSRF. @NoCSRFRequired removed from updateRbacSettings.
• 🟢 Cache docblock. resolveInheritFromPublic docblock now flags transient/in-memory schemas as a no-cache path.
• 🟢 Tasks 6.1 honest accounting. Re-opened — was marked done on a settings-endpoint round-trip; the actual DocuDesk consent-flow smoke is deferred to before promoting from beta to main (or to the QA persona pass after merge).

What's in this PR

Implementation

• lib/Service/Object/PermissionHandler.php — resolveInheritFromPublic(Schema): bool cascade with per-request cache, strict-bool helper, hasPermission gate.
• lib/Db/MagicMapper/MagicRbacHandler.php — flag plumbed through SQL emitters; no fail-open.
• lib/Db/Schema.php — inheritFromPublic accepted by the authorization validator.
• lib/Service/Settings/ConfigurationSettingsHandler.php — inheritFromPublicDefault surfaced in getSettings / getRbacSettingsOnly and persisted with strict normalization.
• lib/Controller/Settings/ConfigurationSettingsController.php — @NoCSRFRequired removed from updateRbacSettings.
• src/store/settings.js, src/views/settings/sections/RbacConfiguration.vue — tenant-default switch in the RBAC settings UI.
• CHANGELOG.md — entries under "Added" and "Behavior changes".

Tests

• tests/Unit/Service/Object/PermissionHandlerInheritFromPublicTest.php — 17 tests: 4-level cascade, null-is-unset, three new strict-bool cascade tests (string "false" at schema, string "false" at register, integer 0 at schema), four-state matrix on hasPermission.
• tests/Unit/Db/MagicMapper/MagicRbacHandlerInheritFromPublicTest.php — 10 tests: four-state matrix on buildRbacConditionsSql and applyRbacFilters smoke.
• tests/Unit/Service/Settings/ConfigurationSettingsHandlerTest.php — 3 new API write-path tests: real bool persistence, string "false" coercion, garbage rejection.
• tests/Unit/Db/MagicMapper/MagicRbacHandlerTest.php and tests/Unit/Service/Object/PermissionHandlerRbacTest.php — wired with the new constructor dependency / container mock.
• 71/71 RBAC tests + 7/7 settings tests pass against the in-container PHPUnit runner. All static analysis (Psalm, PHPStan, PHPCS) clean on changed files.

Docs

• docs/Features/access-control.md — new section covering cascade (with explicit strict-bool note), four-state matrix, worked publication-style example, 'authenticated' simple-rule alternative; inheritFromPublicDefault documented in RBAC Configuration block.
• docs/Features/property-authorization.md — "public" row of the rule-grammar table cross-references the new flag.

OpenSpec change

• openspec/changes/rbac-disable-public-inheritance/ — proposal, design, delta spec under rbac-scopes, tasks (38/40 done; 6.1 honestly re-opened, 6.3 Softwarecatalog smoke open).

Companion PR

A small frontend change for the per-schema checkbox lives in @conduction/nextcloud-vue (branch feat/inherit-from-public-checkbox). Ready to review separately by your colleague.

Test plan

• Schema-level toggle: set inheritFromPublic: false on a schema with read: [{group: "public", match: ...}]. Verify (anon, list) sees rows; (auth without explicit group, list) sees no rows; (auth in another rule's group, list) sees rows; owner and admin always see rows.
• Per-object check: same matrix on GET /api/objects/{register}/{schema}/{id} (PHP-side path).
• Tenant-default toggle: flip the new switch under RBAC Configuration, verify the IAppConfig key persists (occ config:app:get openregister rbac.inherit_from_public_default), verify schemas without an explicit value pick up the new default.
• Strict normalization: send {"inheritFromPublicDefault": "garbage"} to PUT /api/settings/rbac and confirm a 5xx with inheritFromPublicDefault must be a boolean....
• CSRF: confirm the settings UI still saves successfully (via @nextcloud/axios request-token flow). A direct curl without the request-token header should now fail.
• Cascade: schema unset, register inheritFromPublic: false → schema reads honour register's value.
• Default behaviour: any existing schema with no flag set behaves exactly as before this PR.
• DocuDesk consent / OpenCatalogi publications: regression smoke for default flag (DocuDesk consent-flow smoke is the deferred 6.1 task).

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/openregister @ f618adc

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
composer			✅	✅ 147/147
npm			✅	✅ 598/598
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-07 14:17 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

Updated 14:37Z — apologies, my initial metadata fetch missed commit 51220806 (phpcs auto-fix, pushed 14:10:57Z) so the original verdict was based on stale CI. With that commit included, the only in-scope finding (the 🔴 phpcs blocker) is resolved — see the updated comment. All quality/* checks now green, required branch-protection / check-branch ✅.

Strict re-review (resubmission of #1440) — verdict stays REQUEST_CHANGES because Strict mode flips on any open 🟡, but the author has addressed every in-scope finding. The two remaining 🟡 are explicit pre-existing / out-of-scope follow-ups that I'd be comfortable seeing merged-then-followed-up rather than blocked.

All 7 prior findings from #1440 cleanly addressed: silent fail-open dropped (catch-Throwable removed); strict-bool helper at cascade levels; filter_var normalization at both API write paths; @NoCSRFRequired removed from updateRbacSettings; cache docblock; 6 new tests pinning the strict-bool semantics; tasks.md 6.1 honestly reset. ✅

Resolved by 51220806: 🔴 PHPCS auto-fixable errors in PermissionHandler.php — phpcs job now green.

Open 🟡 (both pre-existing / out-of-scope; flagged so they don't get forgotten): 7 sibling state-mutating endpoints in the same controller still carry @NoCSRFRequired — same ADR-005 threat model applies to retention/archival/multitenancy/etc. Docblock claim "any failure must surface" is contradicted by parallel fail-open paths in resolveSchemaAuthorization and getRegisterAuthorization (both catch (\Throwable) and silently default-permissive); the entry-point fix is real but the end-to-end claim isn't delivered.

Open 🟢 (cosmetic): HTTP 500 instead of 400 for malformed inheritFromPublicDefault; frontend dep bump bundled with the security fix commit (no vulnerable swaps; review hygiene only).

Checked & clean: 13/13 protected mechanical gates green on the diff; gate-14 (route-reachability) flags 2 pre-existing methods in the controller — out-of-scope per ADR-020. PHP↔SQL parity intact; anonymous/owner/admin invariants intact; cascade null=unset semantics correct; per-schema cache key correct; frontend round-trip via @nextcloud/axios (auto-attaches CSRF token); 6 new tests pin the new strict-bool behaviour.

[SudoThijn]

FE looks fine