Conversation
…-table-fix fix: use platform-aware table_schema in MagicMapper to support MariaDB
…tMapper - Fix constructor signature mismatches in 16 controller/service test files (OrganisationController, FilesController, ConsumersController, etc.) - Replace non-existent UnifiedObjectMapper with MagicMapper in 49 test files - Fix Newman integration tests: add file upload/delete routes, update publish/depublish/restore assertions, fix bulk operation tests - Fix IL10N mock injection in SettingsController tests - Fix named parameter errors (setInterval, extractOpenRegisterProperties, etc.) - Add missing AuthorizationService constants (HMAC/PKCS1/PSS_ALGORITHMS) - Fix MailAppScriptListener test event class mock - Fix EmailLinkTest property name (setDate → setMailDate) - Re-enable PHPUnit and Newman in quality workflow Newman: 192 assertions, 0 failures (was 42 failures) PHPUnit: constructor mismatches resolved for controller layer, remaining Service-layer mismatches are pre-existing from feature branches
- RegistersControllerTest: add ContainerInterface mock (17th param) - SchemasControllerTest: add ContainerInterface mock (14th param) - DeleteObjectTest: add IDBConnection mock (8th param) - RenderObjectTest/CoverageTest/DeepTest: add ComputedFieldHandler, TranslationHandler, LinkedEntityEnricher mocks (params 12-14)
…sues - ObjectsControllerTest: remove objectMapper param (no longer in constructor) - ObjectsControllerCoverageTest: same fix for all 3 constructor calls - ChatControllerTest: add missing IL10N mock before userId param
- CacheHandler: remove obsolete MagicMapper param from 16 constructor calls across 3 test files - SaveObjects: remove 5 retired sub-handler params from 2 test files - BulkOperationsHandler: delete test for removed class - ObjectsController: remove objectMapper param, fix coverage test - ChatController: add missing IL10N mock - EmailsController: rewrite tests for current API, fix source bugs (wrong named params in validateObject, wrong unlinkEmail args) - RelationsController: fix named params in validateObject call - ContactsController: add missing service mocks - FilesControllerFileActions: use real ObjectEntity for magic methods - SettingsController: fix l10n mock for vsprintf interpolation - SourcesController: add willReturnArgument to l10n mock - RelationHandler: fix objectMapper→objectEntityMapper named param
- Replace ObjectEntityMapper with MagicMapper in 15 test files - Add missing LinkedEntityPropertyHandler, ComputedFieldHandler, TranslationHandler, TmloService params to SaveObject constructors - Fix PermissionHandlerRbacTest to use real Entity instances - SaveObjectRefactoredMethodsTest now passes all 13 tests
Source code fixes: - DestructionList/SelectionList: remove named args from Entity setters - ActionListener: fix getNestedValue named param (array→data) Test fixes: - ObjectsControllerTest: remove publish/depublish tests, fix canDelete mocks, fix clearBlob test, fix constructor without webhookService - ChatControllerTest: configure IL10N mock callback - ConfigurationsControllerTest: add missing 'objects' key in mock - ConsumersControllerTest: configure IL10N mock - OrganisationControllerTest: fix findAll expectation - BlobMigrationJobTest: fix Doctrine→OCP mocks, setValueString return - ObjectEntityTest/SearchTrailTest: remove published/depublished asserts - RegisterMapperTest: fix constructor mock type - ObjectCleanupListenerTest: use real ObjectEntity for magic methods Result: Controller/BackgroundJob/Db/Listener dirs all pass (589 tests)
- RenderObject: configure TranslationHandler mock to pass through data instead of returning empty array (wiped object data during render) - SaveObject: same TranslationHandler fix for save path - SaveObject/Deep/Additional/Coverage: update 31 tests referencing removed published/depublished Entity properties - ReferentialIntegrityServiceTest: fix objectMapper→objectEntityMapper named param (36 errors resolved) - PermissionHandlerRbacTest: fix assertion for permissive auth model - ObjectHandlers/SaveObjectTest: TranslationHandler pass-through mock Result: All 986 tests in these 9 files pass with 0 errors
- SaveObjectsTest: remove 5 deleted handler mocks, remove tests for deleted private methods, fix chunk/bulk assertions (44+44→0) - SaveObjectsBranchCoverageTest: rewrite for new constructor (3→0) - CacheHandlerTest: add IAppContainer mock for lazy MagicMapper (27→0) - CacheHandlerCoverageTest: add container services map (1+2→0) - CacheHandlerBranchCoverageTest: add IAppContainer mock (3→0) - SaveObjectsRefactoredMethodsTest: rewrite for current API (17→0) - IntegratedFileUploadTest: add missing constructor params (9→2)
- ObjectServiceTest: remove PublishHandler/BulkOperationsHandler refs, remove publish/depublish tests, fix search pagination params - ImportServiceTest: fix constructor (remove extra MagicMapper) - SchemaServiceTest: switch named params to positional - ExportServiceTest: fix constructor, update depublished→updated ref - HookExecutorTest: add missing WorkflowExecutionMapper param - SchemaServiceRefactoredMethodsTest: fix named params - ObjectServiceDeepTest: remove PublishHandler, fix 37-param constructor - MagicMapperTest: fix constructor, remove published/depublished cols - ExportServiceCoverageTest: fix constructor - LogServiceTest: remove unifiedObjectMapper
Source code fixes: - ActionService: fix named param array→data in getNestedValue call - ActionExecutor: add (int) cast for getEngine() call Test fixes (17 files): - DeepLinkRegistryServiceTest: fix constructor (ContainerInterface) - ExportServiceGapTest: remove extra MagicMapper arg - ObjectServiceRefactoredMethodsTest: remove PublishHandler - ContactMatchingServiceTest: use real entities for magic methods - AuthorizationServiceTest: fix constructor, remove deleted methods - SettingsServiceTest: add DB platform mock, fix getDatabaseStats - RelationHandlerDeepTest: use real ObjectEntity instances - DeckCardService/EmailService/ContactService: fix mapper mock pattern - FileSidebarServiceTest: use real entity instances - ActionExecutorTest/ActionServiceTest: fix type mismatches - DashboardServiceTest: remove published key assertion - RegisterServiceTest: add tableExists mock - TaskServiceTest: update expected exception message Remaining: BulkMetadataHandlingTest (7 failures) needs full rewrite
…ister into feature/combined-outstanding-prs
Resolve .gitignore conflict: keep image ignore entries from the enrichment branch, remove duplicate block, drop redundant docs/node_modules/ entry (already present earlier in file).
Resolve merge conflicts in 8 test files. All conflicts were about the UnifiedObjectMapper→MagicMapper rename on development vs the ContainerInterface refactoring on the PR branch. Kept PR branch versions since they match the actual constructor signatures. Deleted BulkOperationsHandlerTest.php (retired with blob objects table).
- ReferentialIntegrityServiceTest: fix deleteObjects mock (2 params not 4 after refactor) - IntegratedFileUploadTest: add TranslationHandler pass-through mock - BulkMetadataHandlingTest: rewrite for refactored SaveObjects (removed sub-handlers, use ultraFastBulkSave, fix org service mock) - TextExtractionServiceTest: update getStats assertion (totalObjects now hardcoded to 0) Result: 0 errors, 0 failures across all test directories
- Create NoVtodoCalendarException to replace fragile str_contains() guard
in TasksController (replaces string-matching on exception messages)
- Update TaskService to throw NoVtodoCalendarException instead of generic Exception
- Add testIndexReturnsEmptyWhenNoVtodoCalendar test in TasksControllerTest
- Add setVerb('comment') assertion in NoteServiceTest::testCreateNoteSuccess
- Update TaskServiceTest to expect NoVtodoCalendarException
php cs fix in other branceh
fix: Proper NcAppSidebar on detail pages
fix: Show schema and register names in search results instead of IDs
fix: Detect and sync missing columns even when schema version is unchanged
feat: Archival destruction workflow — NEN 15489 compliant destruction
feat: Retention management — bewaartermijnen and selectielijsten
- Replace container->get(IGroupManager::class) with constructor injection in RegistersController (deprecated Nextcloud pattern) - Fix PermissionMatrix.vue: fetchRegisterList/fetchSchemaList -> refreshRegisterList/refreshSchemaList (correct store method names) - Use return values from refresh methods instead of store property access
…ncement feat: Authorization RBAC Enhancement
Keep archival destruction workflow routes from development alongside the SaaS multi-tenant changes.
Resolve conflict in appinfo/routes.php by keeping both the archival destruction workflow routes from development and the e-Depot transfer routes from this branch.
retrofit: full Bucket 1 + 2a/2b/3b spec coverage for openregister (~500 methods annotated, 6 new specs)
…asset only) (#1386) The central Quality workflow (ConductionNL/.github#34) now publishes SBOMs exclusively as release assets — see SECURITY.md "Software Bill of Materials". This PR cleans up the per-app remnants: - delete .github/workflows/sbom.yml (the central job replaces it) - delete the checked-in sbom.cdx.json (release asset is the source of truth) - gitignore SBOM files so future generations don't accidentally land in repo Stable URL for clients: https://github.com/ConductionNL/openregister/releases/latest/download/sbom.cdx.json Co-authored-by: SBOM Cleanup <ops@conduction.nl>
…ame) (#1388) Per CONVENTIONS.md (ConductionNL/.github#35). The previous filename collided with the central reusable workflow's name (also quality.yml in .github/workflows/), which was a recurring source of confusion when searching across repos. Pure file rename; the workflow's 'name:' field is unchanged so branch-protection required-status-check identity is preserved. Co-authored-by: convention-cleanup <ops@conduction.nl>
) Updates 8 active-spec references from `.github/workflows/quality.yml` to `.github/workflows/code-quality.yml` per the canonical filename established by ConductionNL/.github CONVENTIONS.md (#35). Files touched (live specs only — archived spec copies under openspec/changes/archive/ are intentionally NOT modified to preserve historical accuracy): - openspec/specs/mariadb-ci-matrix/spec.md (5 refs) - openspec/changes/api-test-coverage/specs/api-test-coverage/spec.md (3 refs) The requirement title at line 95 ("Matrix Strategy Configuration in quality.yml") is intentionally preserved — that string is the requirement's addressable identity and renaming it would break any cross-references. Co-authored-by: convention-cleanup <ops@conduction.nl>
…-properties fix(GIT-1358): Fix extend-schema property visibility and allOf composition
Development
…-update Updated package
Add an opt-out for the "logged-in users inherit public group rights" semantics in OR's RBAC. Schemas and registers gain an optional inheritFromPublic boolean (default true, backwards-compatible). When false, authenticated users do NOT qualify for public rules — they qualify only via their own group memberships. Anonymous users see no behaviour change. Cascade: schema → register → IAppConfig openregister.rbac.inherit_from_public_default → hard-coded true. Implementation touches both RBAC layers identically: - PHP-side PermissionHandler::hasPermission inheritance fallback (line 229-241) - SQL-side MagicRbacHandler::processConditionalRule + processSimpleRule (and their UNION-mode siblings buildRbacConditionsSql + processConditionalRuleSql) Modified capability: rbac-scopes. Tracks GitHub issue #1439.
Adds an opt-out for the implicit "logged-in users inherit public group
rights" semantics. Schemas (and registers, via cascade) gain an optional
inheritFromPublic boolean, default true (preserves pre-change behaviour).
Cascade:
schema.authorization.inheritFromPublic
→ register.authorization.inheritFromPublic
→ IAppConfig openregister.rbac.inherit_from_public_default
→ hard-coded true
null is treated as "unset" — cascade falls through.
PermissionHandler:
- new constructor dep IAppConfig
- new public resolveInheritFromPublic(Schema): bool with per-request cache
- hasPermission line 229-241 inheritance fallback now gated on the flag
MagicRbacHandler:
- resolveInheritFromPublic(Schema) helper delegating to PermissionHandler
via existing container DI
- applyRbacFilters resolves the flag once at the top, plumbs through
processAuthorizationRule → processConditionalRule + processSimpleRule
- same plumbing in the UNION-mode path: buildRbacConditionsSql →
processAuthorizationRuleSql → processConditionalRuleSql, and the shared
processSimpleRule
- the per-object hasPermission method (separate from PermissionHandler's)
also gated identically
Behaviour:
- inheritFromPublic = true (default): unchanged from pre-change.
- inheritFromPublic = false + anonymous user: still qualifies for public.
- inheritFromPublic = false + authenticated user: does NOT qualify for
public rules; only own-group / owner / admin grants apply.
Tests:
- new PermissionHandlerInheritFromPublicTest covers cascade resolution
(4 levels + null=unset semantics) and the four-state matrix on
hasPermission, plus owner/admin shortcut invariance.
- existing PermissionHandlerRbacTest updated for new constructor sig.
Quality:
- PHPCS clean on touched files (auto-fix + manual passes).
- PHPStan clean.
- Psalm clean.
- openspec validate clean.
Deferred (tracked in tasks.md as not-yet-checked):
- 3.6, 3.7: SQL-side unit tests (need fixture DB).
- 6.x: cross-app smoke tests against running stacks.
- 7.3-7.5: integration tests against running services.
- 8.1, 8.2: docs extension + worked example.
- 9.1, 9.4: full unit suite (PHPUnit needs the NC docker bootstrap)
+ manual live-stack smoke.
Closes (partially) #1439.
Surfaces the tenant-wide `rbac.inherit_from_public_default` IAppConfig key through the existing settings payload as `rbac.inheritFromPublicDefault`, and renders a toggle for it in the RBAC configuration section. Also updates the rbacOptions store default so the switch hydrates correctly on first load. Pairs with the schema-level checkbox in @conduction/nextcloud-vue (CnSchemaSecurityTab). Backend cascade was added in 3c05b62.
…dator (#1439) Two gaps surfaced during /opsx:verify against the live stack: 1. Schema::validateAuthorizationRules rejected `inheritFromPublic` because it only allowed CRUD action keys. The validator now treats it as an optional sibling of the action keys and verifies it is a boolean (or null = unset). 2. ConfigurationSettingsHandler::getRbacSettingsOnly / updateRbacSettingsOnly handle the dedicated `/api/settings/rbac` endpoint that the frontend store actually calls. They now read and write the `rbac.inherit_from_public_default` IAppConfig key, matching the unified `getSettings` / `updateSettings` paths added earlier. Verified end-to-end via the four-state matrix on /api/objects: with inheritFromPublic=true (default) anon and authenticated users both see public-conditional rows; with false set per-schema, anon still sees them but authenticated users without explicit group membership do not.
Verified manually against the Docker NC stack:
- /api/settings/rbac round-trip (read + write) for inheritFromPublicDefault
- schema-level inheritFromPublic accepted by validator and round-trips
through /api/schemas/{id}
- four-state matrix on /api/objects: (anon|auth) × (true|false) yields
counts that match spec — only (auth, false) is denied; the other three
states see the public-match objects
Tasks 6.1, 6.2, 9.4 set to done. Remaining open items are unit-test
extensions (3.6, 3.7, 7.x), additional docs (5.2, 8.1, 8.2), the broader
suite run (9.1), and the Softwarecatalog smoke (6.3).
Adds tests/Unit/Db/MagicMapper/MagicRbacHandlerInheritFromPublicTest.php covering buildRbacConditionsSql and applyRbacFilters under the four-state matrix (anon|auth × inheritFromPublic true|false), plus parity checks for the simple-string `'public'` rule, the `'authenticated'` rule, and admin bypass. 10 new tests, all green via the in-container PHPUnit runner. Also fixes a regression in the existing PermissionHandlerRbacTest where buildHandlerWithRealMatcher() didn't pass the new IAppConfig dependency into PermissionHandler, causing 10 errors during the full suite run. Knocks out tasks 3.6, 3.7, 7.1, 7.2, 7.3, 7.4, 7.5, 9.1 in the change's tasks.md — the SQL-side matrix is now unit-tested and the integration scenarios are covered by either the cascade unit tests (cascade fall- through paths) or the live-stack matrix run during /opsx:verify.
Extends docs/Features/access-control.md with:
- The optional `inheritFromPublic` boolean on the schema authorization
JSON example
- A new section "Disabling public-group inheritance for authenticated
users" covering the cascade (schema → register → IAppConfig → true),
the four-state matrix, a worked publication-style example, and the
`'authenticated'` simple-rule alternative
- The new `inheritFromPublicDefault` field in the RBAC Configuration
block, including the IAppConfig key and the occ command
Also cross-references the new flag from the `"public"` row of the rule
table in docs/Features/property-authorization.md, since the previous
phrasing ("matches any authenticated user") was unconditional.
Closes tasks 5.2, 8.1, 8.2 in the rbac-disable-public-inheritance change.
…#1439) Addresses the blocker + 3 concerns + 1 minor flagged in WilcoLouwerse's strict review of PR #1440. 🔴 Blocker — drop the silent fail-open in MagicRbacHandler::resolveInheritFromPublic. The previous try/catch returned `true` on any Throwable from the cascade walk, which silently undid the gate the tenant explicitly opted out of and let this SQL path diverge from the PHP per-object check (which propagates). Spec invariant "per-object checks and listing membership cannot drift" now holds even under failure: the request fails (5xx) instead of leaking rows. 🟡 Concern 1 — strict-boolean check at schema/register cascade levels. PHP's `(bool) "false"` is `true`, so a register persisted via direct mapper write / migration / seed JSON could store a string and silently invert the gate. Both schema (line 716) and register (line 728) now require literal `true` or `false`; anything else (string, int, etc.) is treated as "unset" and logged as a warning. Three new cascade tests pin the strict-equality contract. 🟡 Concern 2 — strict normalization on the API write paths. `updateRbacSettingsOnly` and `updateSettings` now use `filter_var` with `FILTER_VALIDATE_BOOLEAN | FILTER_NULL_ON_FAILURE` (matching the docs' boolean-tolerance claim) and throw on garbage rather than silently coercing `(bool) "false" === true`. Three new tests pin the normalize-and-persist contract (real bool, "false" string, garbage rejection). 🟡 Concern 3 — drop @NoCSRFRequired from updateRbacSettings. This endpoint is now security-load-bearing (it flips a tenant-wide RBAC default); CSRF protection on state-mutating admin endpoints is required by ADR-005. Frontend uses @nextcloud/axios which sends the request token automatically; no UI change needed. 🟢 Minor — docblock note on resolveInheritFromPublic about transient schemas (no-cache path) so future readers don't expect cache hits on in-memory drafts. Tasks 6.1 (DocuDesk smoke) re-opened — the previous justification was a settings-endpoint round-trip, not a behavioural exercise of DocuDesk's consent-fetch endpoint. Honest accounting per reviewer's note. Tests: 71 RBAC + 7 settings (was 68 + 4) — all green via the in-container PHPUnit runner.
…ublic-inheritance feat(rbac): add inheritFromPublic flag for opt-out of public-group inheritance (#1439)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| jobs: | ||
| branch-protection: | ||
| uses: ConductionNL/.github/.github/workflows/branch-protection.yml@main |
Comment on lines
+17
to
+32
| if: github.event_name != 'push' || github.event.created != true | ||
| uses: ConductionNL/.github/.github/workflows/quality.yml@main | ||
| with: | ||
| app-name: openregister | ||
| php-version: "8.3" | ||
| php-test-versions: '["8.3", "8.4"]' | ||
| nextcloud-test-refs: '["stable32"]' | ||
| enable-psalm: true | ||
| enable-phpstan: true | ||
| enable-phpmetrics: true | ||
| enable-frontend: true | ||
| enable-eslint: true | ||
| enable-phpunit: true | ||
| enable-newman: true | ||
| enable-coverage-guard: true | ||
| enable-sbom: true |
Comment on lines
+11
to
+13
| uses: ConductionNL/.github/.github/workflows/documentation.yml@main | ||
| with: | ||
| cname: openregisters.app |
Comment on lines
+15
to
+20
| uses: ConductionNL/.github/.github/workflows/issue-triage.yml@feature/openspec-project-sync | ||
| with: | ||
| app-name: openregister | ||
| backlog-existing: ${{ github.event_name == 'workflow_dispatch' && inputs.backlog-existing || false }} | ||
| secrets: | ||
| PROJECT_TOKEN: ${{ secrets.PROJECT_TOKEN }} |
Comment on lines
+9
to
+18
| if: github.ref == 'refs/heads/development' | ||
| uses: ConductionNL/.github/.github/workflows/release.yml@main | ||
| with: | ||
| release-type: unstable | ||
| app-name: openregister | ||
| verify-vendor-deps: true | ||
| vendor-check-paths: 'openai-php/client/src,theodo-group/llphant/src' | ||
| secrets: inherit | ||
|
|
||
| beta: |
Comment on lines
+19
to
+28
| if: github.ref == 'refs/heads/beta' | ||
| uses: ConductionNL/.github/.github/workflows/release.yml@main | ||
| with: | ||
| release-type: beta | ||
| app-name: openregister | ||
| verify-vendor-deps: true | ||
| vendor-check-paths: 'openai-php/client/src,theodo-group/llphant/src' | ||
| secrets: inherit | ||
|
|
||
| stable: |
Comment on lines
+29
to
+36
| if: github.ref == 'refs/heads/main' | ||
| uses: ConductionNL/.github/.github/workflows/release.yml@main | ||
| with: | ||
| release-type: stable | ||
| app-name: openregister | ||
| verify-vendor-deps: true | ||
| vendor-check-paths: 'openai-php/client/src,theodo-group/llphant/src' | ||
| secrets: inherit |
Member
Author
|
TEST COMPLETE |
Contributor
Quality Report — ConductionNL/openregister @
|
| 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-20 13:49 UTC
Download the full PDF report from the workflow artifacts.
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
6 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.
No description provided.