fix(domain): scope REST lists by a global X-OpenMetadata-Domain header (server-side, central) - #31538
fix(domain): scope REST lists by a global X-OpenMetadata-Domain header (server-side, central)#31538sonika-shah wants to merge 1 commit into
Conversation
Replaces the per-API ?domain= approach with a persona-style header applied
server-side and centrally. Phase 1 (REST list path):
- ActiveDomainContext ThreadLocal + X-OpenMetadata-Domain header read in the
three auth filters into CatalogSecurityContext.activeDomain, cleared in
PerRequestContextCleaner.
- EntityUtil.addDomainQueryParam applies the nav domain STRICT (un-domained
hidden) for admins and DomainOnlyAccess users; intersects with getUserDomains()
for restricted users so it can only narrow, never bypass RBAC; guards on
supportsDomains + a governance denylist (user/team/role/policy/persona/bot).
- UI withDomainFilter sends the header (domain id) on REST GETs instead of
?domain=; search keeps its client-side query_filter for now (Phase 1.5).
Verified on a stack: admin/restricted x {B,A,inaccessible,none}, strict semantics,
RBAC no-bypass, governance lists unaffected.
github-actions
Bot
added
backend
safe to test
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
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 |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| String navDomainId = ActiveDomainContext.getActiveDomain(); | ||
| if (!nullOrEmpty(navDomainId) && entitySupportsDomains(entityType)) { | ||
| String requested = "'" + navDomainId.replace("'", "") + "'"; | ||
| // STRICT narrowing (no domainAccessControl -> un-domained assets are hidden), for admins and | ||
| // restricted users alike. For restricted users the selection is intersected with their | ||
| // accessible domains so it can only narrow, never widen (a domain they cannot access -> no | ||
| // match). | ||
| String domainId = | ||
| restricted | ||
| ? intersectWithAllowedDomains( | ||
| requested, getCommaSeparatedIdsFromRefs(subjectContext.getUserDomains())) | ||
| : requested; | ||
| filter.addQueryParam("domainId", domainId); | ||
| return; |
There was a problem hiding this comment.
⚠️ Edge Case: Sub-domain assets hidden on REST lists when parent domain selected
The central REST path resolves the header to an exact domainId IN (...) match (ListFilter.getDomainCondition builds entity_relationship.fromId IN (...) with no descendant expansion), so selecting a parent domain in the navbar hides assets that belong to its sub-domains. The search path in the same interceptor (withDomainFilter.tsx:94-98) instead matches with a prefix: '<fqn>.' clause, so Explore includes sub-domain assets. This produces inconsistent results between list views and search for the same navbar selection. Consider expanding the requested domain to include its sub-domain ids server-side before building the filter, mirroring the search prefix semantics.
Was this helpful? React with 👍 / 👎
| /** | ||
| * Holds the global (navbar-selected) domain sent on the request via the {@code X-OpenMetadata-Domain} | ||
| * header, so code paths that don't have the {@link jakarta.ws.rs.core.SecurityContext} at hand can | ||
| * still read it. Mirrors {@link ActivePersonaContext}. The value is a single domain fully-qualified | ||
| * name; it is only ever used to NARROW a listing within the user's accessible domains, never for | ||
| * authorization. | ||
| */ |
There was a problem hiding this comment.
💡 Quality: ActiveDomainContext javadoc says FQN but value is a domain id
The class comment states "The value is a single domain fully-qualified name", but the UI sends activeDomainEntityRef.id (a UUID) and EntityUtil treats the value as a domainId for entity_relationship.fromId matching. The misleading doc will confuse future maintainers into passing/comparing FQNs. Update the javadoc to say the value is the domain id (UUID).
Was this helpful? React with 👍 / 👎
| private static boolean entitySupportsDomains(String entityType) { | ||
| if (NAV_DOMAIN_FILTER_EXCLUDED_TYPES.contains(entityType)) { | ||
| return false; | ||
| } | ||
| try { | ||
| return Entity.getEntityRepository(entityType).isSupportsDomains(); | ||
| } catch (Exception e) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Quality: Broad catch(Exception) in entitySupportsDomains swallows errors silently
entitySupportsDomains catches the broad Exception and returns false with no logging, which both violates the project rule against broad catches and silently disables domain scoping for any entity type whose repository lookup throws for an unexpected reason. Narrow the catch to EntityNotFoundException (the expected case for unknown types) and at least log unexpected failures with context.
Was this helpful? React with 👍 / 👎
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source
|
| Count | Rule |
|---|---|
| 1 | sonarjs/cognitive-complexity |
| 1 | sonarjs/cyclomatic-complexity |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/hoc/withDomainFilter.tsx:30:31 |
sonarjs/cognitive-complexity |
Refactor this function to reduce its Cognitive Complexity from 30 to the 15 allowed. |
| 🟡 | src/hoc/withDomainFilter.tsx:30:31 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":30,"column":30,"endLine":30,"endColumn" |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
Summary
Replaces the per-API
?domain=approach to the global (navbar) domain filter with a singleX-OpenMetadata-Domainrequest header applied server-side and centrally, mirroring how theactive-persona header works. This is the scalable "right solution" to the gap that #31180 patched
per-endpoint (glossary/metric/tag/classification/service/dataProduct): every REST list view is
covered in one place, with no per-resource
?domainhandling.Problem
The navbar domain filter didn't scope REST list endpoints: the UI stamped
?domain=<fqn>on everylist request, but each resource had to declare
@QueryParam("domain")and apply it — only a handfuldid, so most lists ignored it. Adding the param to every API (and every future entity) doesn't scale,
and the same gap shows up on entity-page child lists.
Approach — mirror the active-persona header
withDomainFilter) sendsX-OpenMetadata-Domain: <domainId>on REST GETsinstead of
?domain=.JwtFilter,NoopFilter,CatalogOpenIdAuthorizationRequestFilter) read the header intoCatalogSecurityContext.activeDomainActiveDomainContextThreadLocal (cleared inPerRequestContextCleaner) — same shape asX-OpenMetadata-Persona/ActivePersonaContext.EntityUtil.addDomainQueryParam(already the domain chokepoint for everyEntityResource.listInternallist) reads the active domain and scopes the list.Semantics
hidden), for admins and
DomainOnlyAccessRoleusers. When "All Domains" is selected (no header),behavior is unchanged (admins unfiltered; restricted users keep their RBAC scope incl. un-domained).
subjectContext.getUserDomains()server-side; the header only narrows within it (intersection). A domain the user can't access →
no-match. The header can only shrink the result set, never widen it.
supportsDomains), and a governancedenylist (
user,team,role,policy,persona,bot) is excluded even though those carry adomainsfield — otherwise selecting a domain would empty the Users/Teams/Policies lists.Scope
securely server-side (
RBACConditionEvaluator.hasDomainfromuser.getDomains(), non-bypassable),and the nav-domain narrowing stays client-side in
query_filter(safe because RBAC bounds it). Noserver-side search change is needed.
listInternaland would need thesame central apply (
GlossaryTermResource, custom-paging resources).Verification
Built into a stack and ran an API matrix with the header, for admin and a DomainOnlyAccessRole
user with domains [A,B]; glossaries in A, in B, and un-domained:
All pass. The customer's original case (a Domain-A glossary hidden when Domain B is selected) works
for both admin and the restricted user.
Notes