fix(security): schema-rbac-hardening — close cross-org schema reads (#379, #390, #378) - #395
Merged
Merged
Conversation
…tie/contract Closes three related read-leak paths in lib/Settings/softwarecatalogus_register.json and lib/Controller/AanbodController.php (softwarecatalog #379, #390, #378): - gebruik, koppeling, and organisatie each granted gebruik-beheerder an unscoped (bare) schema-RBAC read. Neither koppeling nor organisatie has an app-local controller, so a read of either schema through OpenRegister's standard object API was gated solely by this config — any gebruik-beheerder in any organisation could read every other organisation's koppelingen and organisaties. gebruik-beheerder is now match-scoped to _organisation (all three schemas) plus afnemer (gebruik only, since it is gebruik's explicit consumer-relation field). - contract still carried eight other bare, unscoped roles beyond the aanbod-beheerder grant already fixed in vendor-visibility-rbac (REQ-006): functioneel-beheerder, gebruik-beheerder, vng-raadpleger, software-catalog-users, organisatie-beheerder, organisaties-beheerder, and gebruik-raadpleger are now match-scoped to _organisation. ambtenaar and software-catalog-admins (the app's designated super-user group, wired into setSuperUserGroups() alongside admin) stay deliberately unscoped. - AanbodController::getAanbod() relied entirely on AanbodService's internal getCurrentOrganisation() returning null for an anonymous session; it now has the same explicit getUser() === null guard REQ-004 already added to getGebruiksWhereAfnemer(), returning the empty envelope before the service is ever invoked. The gebruik.deelnemers array-membership sharing case is a deliberate, documented residual: OpenRegister's OperatorEvaluator has no array-contains operator, so it cannot be expressed as a schema-RBAC match condition today. It stays enforced by the existing app-level deelnames-gebruik bypass path (regression-tested separately) — not silently dropped. Note: this fix has no runtime effect on an already-installed instance until softwarecatalog #391 (register-import-reliability) lands — the repair-step importer currently no-ops on a re-edited, already-imported register/schema.
…-hardening Covers every requirement this change adds or extends, including the mandated negative scenarios (schema-rbac-hardening tasks.md Task 1-4): - SchemaRbacTest (new): asserts gebruik-beheerder is no longer a bare unscoped read grant on gebruik/koppeling/organisatie, is match-scoped to _organisation on all three (plus afnemer on gebruik), and that the pre-existing aanbod-beheerder/public grants are untouched. - ContractRbacTest (extended): asserts each newly-scoped contract role (functioneel-beheerder, gebruik-beheerder, vng-raadpleger, software-catalog-users, organisatie-beheerder, organisaties-beheerder, gebruik-raadpleger) is match-scoped to _organisation, and that ambtenaar and software-catalog-admins retain their deliberate unrestricted read. - AanbodControllerTest (new): asserts an unauthenticated caller is rejected before AanbodService::getAanbod() is ever invoked (never() assertion), mirroring the existing AangebodenGebruikControllerTest pattern. - AangebodenGebruikServiceTest (extended): regression coverage for getGebruiksWhereDeelnemers() confirming the schema-RBAC edits did not affect the RBAC-disabled, session-scoped deelnemers bypass path that stands in for the documented array-contains residual. 36 tests / 181 assertions in this slice, all green. Full suite: 428 tests, 1 pre-existing failure unrelated to this change (PortfolioReportControllerTest::testCsvFormatReturnsDownloadResponse, an environment-only Symfony class gap in the OCP test harness, untouched by this change).
Archives the schema-rbac-hardening change into openspec/specs/vendor-visibility-rbac/spec.md as the canonical spec home (openspec archive schema-rbac-hardening --yes): adds REQ-008 (gebruik/ koppeling/organisatie schema-level RBAC) and REQ-009 (AanbodController explicit auth guard), and extends REQ-006 with the contract schema's remaining scoped roles plus the software-catalog-admins exception. openspec validate --specs --strict passes (52/52). Extends docs/security/vendor-visibility-rbac.md with the schema-level RBAC layer section, updates the audit table entries for /api/aanbod and contract reads, adds koppeling/organisatie schema-RBAC rows, and documents the gebruik.deelnemers array-membership residual with the reason it cannot be expressed at the schema-RBAC layer today (no array-contains operator in OpenRegister's OperatorEvaluator) plus the deployment caveat that this fix has no effect on an installed instance until softwarecatalog #391 (register-import-reliability) lands.
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
1 participant
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.
Closes #379, #390, #378 as one change (same fix shape, same file, same tests).
#379 — the loud one
gebruik,koppelingandorganisatiecarried a bare, unscopedgebruik-beheerderinauthorization.read.koppelingandorganisatiehave no app-local controller, so reads go through OpenRegister's generic object API where schema RBAC is the sole enforcement point — agebruik-beheerderin one municipality could read every other organisation's koppelingen and organisaties. Now match-scoped on_organisation(plusafnemerforgebruik, its explicit consumer field).Safe to fix: every app service already passes
_rbac:falseand scopes in PHP, so schema RBAC was dead for the app's own routes.#390 — same gap, more roles
contract's remaining 7 bare roles scoped on_organisation. Two kept unscoped deliberately and justified in design.md:ambtenaar(spec-locked) andsoftware-catalog-admins(the app's real super-user group, wired intosetSuperUserGroups()).#378 — implicit guard
AanbodController::getAanbod()now has an explicitgetUser() === nullguard returning the empty envelope before the service is invoked (REQ-004 pattern).Documented residual — deelnemers
gebruik.deelnemersarray-membership sharing cannot be expressed as a schema-RBAC match: OpenRegister'sOperatorEvaluatorhas no array-contains operator. Recorded explicitly in the proposal, design (Decision 6), spec REQ-008 anddocs/security/vendor-visibility-rbac.md— not silently dropped, no operator invented. The app-leveldeelnames-gebruikpath stays the enforcement point and is regression-tested unchanged.This has zero runtime effect on an installed instance until #391 lands — monolith register edits are currently a silent no-op on upgrade. Do not treat as deployed-effective until #391 is merged and live-verified.
Tests: 36 new/extended (181 assertions) incl. negative cross-org denial tests for all four schemas; full suite 428. One pre-existing unrelated failure (#393, CSV test-env
HeaderUtils).openspec validate --specs --strict52/52. Archived.🤖 Generated with Claude Code