Fixes #29835: make read authorization independent of the fields projection - #30691
Fixes #29835: make read authorization independent of the fields projection#30691sonika-shah wants to merge 1 commit into
Conversation
…ection
ResourceContext loaded the entity for policy evaluation using the caller's
`fields` query parameter. A policy attribute that was not requested read as
absent, so a conditional Deny rule silently failed open — GET /v1/tables/{id}
without `fields=tags` returned a table that a matchAnyTag Deny rule should
have blocked, while the same request with `fields=tags` was correctly denied.
Always resolve the authorization entity with the policy-relevant fields
(owners, tags, domains, reviewers) instead of the caller-supplied projection.
The response load is unchanged, so payloads are unaffected. Removing the
projection also makes the authorization read cache-eligible again, since
useRepositoryCache() disabled caching whenever requestedFields was set.
Fixes open-metadata#29835
❌ 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 |
Code Review ✅ ApprovedMakes read authorization independent of the caller's fields projection by resolving policy entities with all required attributes, preventing conditional deny rules from failing open. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
|



Describe your changes:
Fixes #29835
ResourceContext.resolveEntity()loaded the entity used for policy evaluation with the caller'sfieldsquery parameter. Any policy-relevant attribute the caller did not request read as absent, so a conditionalDenyrule silently failed open —matchAnyTagreturnsfalsewhen tags were never loaded.Measured on a table tagged
PII.Sensitivewith aDENY VIEW_ALL where matchAnyTag('PII.Sensitive')rule, as a non-admin user:GET /v1/tables/{id}GET /v1/tables/{id}?fields=tagsGET /v1/tables/{id}?fields=ownersGET /v1/tables/name/{fqn}fieldsThe authorization entity is now always resolved with the policy-relevant fields (
owners,tags,domains,reviewers) rather than the caller-supplied projection. The response load is untouched, so response payloads are byte-identical.Side effect: removing the projection makes the authorization read cache-eligible again —
useRepositoryCache()returnedfalsewheneverrequestedFieldswas set, so this path was previously uncached.Scope
This closes the projection-dependence gap. It does not change column-level tag enforcement: column tags hydrate only when
columnsandtagsare both loaded (TableRepository#setFields), which is unchanged here and tracked separately.Type of change:
High-level design:
N/A — removes a branch, no new components.
Tests:
Use cases covered
Denyrule is enforced onGETby id and by name regardless of thefieldsparameter.fields=owners) does not bypass the rule.Unit tests
Backend integration tests
openmetadata-integration-tests.QueryVisibilityPolicyIT#test_tagDenyPolicy_enforcedWhenFieldsParamOmitted(added to the existing tag-policy IT rather than a new file).Ingestion integration tests
Playwright (UI) tests
Manual testing performed
PII.Sensitive, aDENY VIEW_ALLpolicy conditioned onmatchAnyTag, and a non-admin user carrying it.fields, and denied withfields=tags— the reported inconsistency.UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes #29835.Greptile Summary
Authorization entity loading is now independent of the caller’s response projection.
Confidence Score: 5/5
The PR appears safe to merge and closes the field-projection-dependent authorization gap.
Authorization now loads policy-relevant relationships independently from the response projection, while field-sensitive request caching remains isolated by field and relation keys and the response load remains separate.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Resource as EntityResource participant Auth as Authorizer participant Context as ResourceContext participant Repo as EntityRepository Client->>Resource: "GET entity?fields=<projection>" Resource->>Auth: authorize(operation, resource context) Auth->>Context: resolve policy entity Context->>Repo: load owners/tags/domains/reviewers Repo-->>Context: policy-complete entity Context-->>Auth: authorization attributes Auth-->>Resource: allow or deny alt allowed Resource->>Repo: load requested response projection Repo-->>Client: projected entity else denied Resource-->>Client: 403 Forbidden endReviews (1): Last reviewed commit: "fix(security): make read authorization i..." | Re-trigger Greptile
Context used: