Fixes 31740: Enforce caller policies on the Incident Manager listing (1.13 backport) (#31741)

[ShaileshParmar11]

Describe your changes:

Fixes #31740

1.13 backport of #31741.

A domain-restricted user saw failed test case incidents from every domain on the Incident Manager page. Opening one was correctly blocked, but the listed row already leaked the test case name, the table name and the full FQN.

RuleEvaluator#hasDomain short-circuits to true for list operations (a listing has no single resource to evaluate the condition against) and defers enforcement to search-side RBAC filtering. The two search-backed listing paths in EntityTimeSeriesRepository never received a SubjectContext, so that filtering never ran and no policy could restrict the listing — including with enableSearchAccessControl on.

This threads the caller's SubjectContext through both paths so the existing RBAC search machinery filters the query, same approach as the upstream fix.

Not a straight cherry-pick

8df0bb573d doesn't apply cleanly to 1.13:

• ElasticSearchAggregationManager / OpenSearchAggregationManager have diverged from main — 1.13 doesn't have the "ContextMemory" feature main's aggregate() wraps queries with (restrictToOrgWideMemories). The SubjectContext-aware aggregate() overload and its applyRbacQuery helper are hand-adapted onto 1.13's actual (undiverged) aggregate() implementation instead of reusing main's refactor verbatim.
• The upstream PR's DomainIsolationIT.java depends on test infrastructure from Feature Request: Improve domain isolation in UI and lineage for multi-tenant setups #24180 that was never backported to 1.13 (the file doesn't exist on this branch). This adds a standalone IncidentManagerDomainIsolationIT instead, following the enableSearchAccessControl/restoreSearchAccessControl + TestNamespace pattern already established by MultiDomainHasDomainIT on 1.13.
• All other touched backend files (EntityTimeSeriesRepository, TestCaseResolutionStatusResource, SearchRepository, AggregationManagementClient, ElasticSearchClient, OpenSearchClient) matched 1.13's current code closely enough to apply the same change directly.

TestCaseResolutionStatusResource on 1.13 has two GET listings worth noting since they're easy to conflate: the base path (repository.list, DB-backed via ListFilter/JDBI — not touched by this fix, and known to silently ignore its own domain query param, a separate pre-existing bug out of scope here) and /search/list (repository.listFromSearchWithOffset / listLatestFromSearch — the SubjectContext-aware path this fix actually changes, and what the Incident Manager UI page calls).

Playwright

Initially left out on the assumption it needed new CI lane wiring, based on a pattern from an unrelated ImportExport-lane investigation on a different branch. That doesn't apply to 1.13: this branch has no impact-map/path-filter test-selection layer — playwright-postgresql-e2e.yml triggers on any non-paths-ignored change and runs the full chromium project (the default project, no explicit testMatch) across a fixed 6-way shard, so any new spec under playwright/e2e/Features/** is picked up automatically. The DomainIsolation project in playwright.config.ts is an unrelated compatibility shim for --project=DomainIsolation invocations and doesn't gate this.

Added DomainIncidentIsolation.spec.ts (1.13 port of main's spec), plus its two missing dependencies:

• utils/domainIsolationUtils.ts — ported verbatim.
• incidentManager.ts#seedFailedIncidents — adapted to poll /search/list instead of the base path, since that's the endpoint the spec's assertions and this fix both depend on (polling the base, DB-backed path wouldn't actually confirm Elasticsearch indexing).

The rest of main's DomainIsolation suite (search/lineage/task/dropdown/listing isolation) still depends on #24180 infrastructure not backported to 1.13 and remains out of scope.

Type of change:

• Bug fix

Tests:

Backend integration tests

• Added IncidentManagerDomainIsolationIT#test_incidentListing_restrictedUserSeesOnlyOwnDomainIncidents in openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IncidentManagerDomainIsolationIT.java. Seeds two domains, a domain-restricted user (DomainOnlyAccessRole), and incidents in each domain; asserts /search/list is filtered for both latest=false and latest=true — the aggregation path that carried the same gap.

Not yet run against a live server in this session; this is its first execution in CI. It's a standard *IT.java under openmetadata-integration-tests, picked up automatically by the existing Maven integration-test phase.

Playwright (UI) tests

• Added openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DomainIsolation/DomainIncidentIsolation.spec.ts, ported from main. Runs under the default chromium project — no new CI lane needed (see above).

Manual testing performed

Verified mvn compile / mvn test-compile succeed for openmetadata-service and openmetadata-integration-tests, mvn spotless:apply makes no further changes, yarn tsc:playwright shows no new type errors from the added/modified files, and eslint/prettier pass clean on them. Not run against a live server in this session.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #<issue-number> above.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: not applicable, no schema changes.
• For UI changes: Playwright spec added, see above.
• I have added tests (unit / integration / Playwright as applicable) and listed them above.

Bug fix:

• I have added a test that covers the exact scenario we are fixing, adapted to what already exists on 1.13.

🤖 Generated with Claude Code

Greptile Summary

This backport threads the authenticated subject through time-series search and aggregation paths so Incident Manager listings honor caller policies.

• Adds subject-aware search and aggregation overloads for Elasticsearch and OpenSearch.
• Passes the request subject from the incident-status resource into both normal and latest-result listings.
• Adds backend and Playwright coverage for domain-isolated incident listings.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResolutionStatusResource.java	Resolves the caller subject and supplies it to both search-backed incident listing branches.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java	Adds subject-aware overloads for offset and latest aggregation searches while retaining compatibility overloads.
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchAggregationManager.java	Applies the caller's RBAC query to Elasticsearch aggregation requests.
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchAggregationManager.java	Applies equivalent caller-policy filtering to OpenSearch aggregation requests.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DomainIsolation/DomainIncidentIsolation.spec.ts	Covers Incident Manager visibility for a domain-restricted user through the UI.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IncidentManagerDomainIsolationIT.java	Exercises both normal and latest incident-listing paths with domain-restricted credentials.

Sequence Diagram

sequenceDiagram
  participant U as Restricted caller
  participant R as Incident status resource
  participant T as Time-series repository
  participant S as Search repository
  participant A as Search aggregation manager
  U->>R: GET /search/list
  R->>R: Resolve SubjectContext
  alt "latest=false"
    R->>T: listFromSearchWithOffset(..., subject)
    T->>S: listWithOffset(..., subject)
    S-->>U: Policy-filtered incidents
  else "latest=true"
    R->>T: listLatestFromSearch(..., subject)
    T->>S: aggregate(..., subject)
    S->>A: Apply RBAC query
    A-->>U: Policy-filtered latest incidents
  end

Loading

_{Reviews (6): Last reviewed commit: "Serialize IT tests that mutate global se..." | Re-trigger Greptile}

Context used (3)

• Knowledge Base — Metadata platform service
• Knowledge Base — Service API resources
• Knowledge Base — Platform security and governance

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[github-actions]

❌ UI Checkstyle Failed

❌ Core Components - I18n Sync

Core-components t() keys, locale files, or language-set are out of sync. Run yarn check-i18n-all locally.

❌ Antd + Less Deprecation Guard

A new antd import or new .less file was added. Use UntitledUI + Tailwind for new work.

Affected files

at Function._resolveFilename (node:internal/modules/cjs/loader:1401:15)
at defaultResolveImpl (node:internal/modules/cjs/loader:1057:19)
at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1062:22)
at Function._load (node:internal/modules/cjs/loader:1211:37)
at TracingChannel.traceSync (node:diagnostics_channel:322:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5)
at node:internal/main/run_main_module:36:49 {

---

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

[gitar-bot]

Code Review ✅ Approved 5 resolved / 5 findings

Threads the caller subject through time-series search and aggregation paths to enforce Incident Manager listing policies, accompanied by integration and UI tests addressing multiple concurrency findings.

✅ 5 resolved

✅ Quality: applyRbacQuery uses 4 scattered returns (style guideline)

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchAggregationManager.java:74-88 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchAggregationManager.java:73-87
The new applyRbacQuery helper has four return statements, which violates the project's "one return statement per method, placed at the end" Java standard. Behaviorally it is correct (and the ES/OS copies are near-identical, differing only in the ElasticQueryBuilder/OpenSearchQueryBuilder cast). Consider restructuring to a single trailing result return with if/else; a shared helper is impractical here because the two Query types come from different client libraries.

✅ Bug: Incident isolation spec toggles global search RBAC in parallel project

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DomainIsolation/DomainIncidentIsolation.spec.ts:166 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DomainIsolation/DomainIncidentIsolation.spec.ts:177 📄 openmetadata-ui/src/main/resources/ui/playwright.config.ts:86-98 📄 openmetadata-ui/src/main/resources/ui/playwright.config.ts:163-177
DomainIncidentIsolation.spec.ts runs under the default chromium project, which is fullyParallel: true with 3 CI workers (playwright.config.ts:42,86-99). Its beforeAll calls enableDisableSearchRBAC(apiContext, true) and afterAll disables it (lines 166,177) — a global server setting (globalSettings.enableAccessControl). Every other chromium test running concurrently is exposed to search RBAC being toggled on/off mid-run, which can flake unrelated search-dependent tests and pollute state. This is exactly why the existing SearchRBAC.spec.ts is excluded from chromium (testIgnore at playwright.config.ts:98) and given its own dedicated non-parallel SearchRBAC project. Add this spec to the chromium testIgnore list and run it in its own isolated (non-parallel) project, mirroring the SearchRBAC setup, rather than the default parallel chromium project.

✅ Quality: Unused exported helpers added to domainIsolationUtils.ts

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/domainIsolationUtils.ts:90-104
searchDomainInDropdownTree and searchDomainInListing (and the waitForDomainSearch helper) are exported/defined but not referenced by DomainIncidentIsolation.spec.ts or anything else backported to 1.13 — only assignDomainOnlyAccess, assignDomainToTable, and safeDelete are used. These add dead code that lint/tsc may not flag since they are exported. Either remove them until the specs that need them are backported, or reference them where intended.

✅ Bug: IT mutates global search settings under concurrent execution

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IncidentManagerDomainIsolationIT.java:57 📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IncidentManagerDomainIsolationIT.java:93-94 📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IncidentManagerDomainIsolationIT.java:240-254
IncidentManagerDomainIsolationIT is annotated @execution(ExecutionMode.CONCURRENT) yet enableSearchAccessControl() flips the process-wide SEARCH_SETTINGS.globalSettings.enableAccessControl to true for the duration of the test. TestNamespace only isolates entities, not this global flag, so any other IT running concurrently will suddenly have search RBAC enforced against its queries (unexpectedly filtered results) — a flaky cross-test failure. The teardown compounds it: restoreSearchAccessControl() calls the /reset/SEARCH_SETTINGS endpoint, which resets the entire search configuration to defaults (not just the one flag) and can run while another instance still relies on the setting. Serialize this test against the shared setting (e.g. @ResourceLock / @execution(SAME_THREAD)) and restore only the single flag rather than resetting all search settings.

✅ Security: Subject-aware aggregate() default overload fails open

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/AggregationManagementClient.java:77-85
AggregationManagementClient.aggregate(..., SubjectContext) defaults to delegating to the subject-less overload, which applies no RBAC filtering. Today only ElasticSearchAggregationManager and OpenSearchAggregationManager override it, so behaviour is correct, but any future implementation that forgets to override this method would silently leak documents the caller cannot read while appearing to honor the subject-aware API. Consider making the subject-aware method abstract (no default), or having the default fail closed / log a warning so a missing override is caught rather than silently bypassing policy.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}