Repository navigation
fix(security): third batch of backend security-hardening fixes - #36952
Conversation
- The FQN prefix renames (glossary, classification, tag, domain and container descendants), the tag-usage prefix rewrites and the thread-link rename bind every value; only the table name stays a template attribute. A guard test fails on any DAO statement that is a bare template attribute. - MySQL swaps an FQN prefix by length instead of by regular expression, and the JSON-text rewrites match names containing quotes or backslashes, so a container move below a quoted name renames its descendants. - Tag lists count usage with the bound per-tag query, so usageCount matches what a GET returns.
- A token verified against OpenMetadata's own key resolves to the account named in its sub claim, or in username when the configured claims put the email into sub, instead of through the principal-claim order configured for the identity provider's tokens. - The authorizer uses the authenticated principal name whole, not the part before the first '/' or '@'.
…nd sign-ins - Logout retires the bearer token the filter verified for the request and ignores the token in the body. The logout cache keys entries by digest, keeps only the user and logout time, skips tokens that never expire, and holds at most 100 000 entries, dropping the one that expires first. - A password reset, a user's own change, an admin's change and the reset-password ops command end the user's refresh tokens, live sessions and MCP grants. Personal access tokens and bot tokens stay.
…es in its own browser - An LDAP sign-in applies the directory groups the user is in now, a user in no group included, and reports a change of the admin flag as well as of the roles so the account is saved. A configured admin principal stays admin. - A SAML login records the id of the request it sends. The ACS accepts only the answer to that request, attaches the account to the pending login and redirects to /api/v1/saml/complete, which activates the session for the browser holding that login's session cookie.
- The security configuration GET masks secrets for every caller. - A PATCH applies its operations to the configuration as an admin reads it, with the secrets masked. - PATCH and PUT restore a masked secret only while the setting it is used with is unchanged, and otherwise ask for it to be entered again.
- The email and entity-link patterns match in linear time, accepting the same inputs as before. - A JSON Patch body is read within fixed size, nesting and operation bounds, for the REST reader and the MCP tool alike, and applying a patch refuses one that grows the document beyond a fixed budget. - A JSON Patch on the document root is refused. - The description sanitizer restores entity-link tokens in a single pass, only where the token is element text.
…ugh a re-validating patch
- A PUT naming the caller's own account is authorized explicitly: a deleted self gets 404, an unknown self is a sign-up on the same terms as POST. - A role change on a user write needs an admin in either direction; a non-admin PUT keeps the stored roles, admin flag and bot flag. - Deleting your own account needs an admin, on all three delete endpoints. - A user CSV row from a non-admin is refused for a stored admin or bot.
…BAC writes - Setting owners on configuration entities (policies, teams, apps, workflows, alerts, ingestion pipelines, classifications, services) needs EditAll; ownership tasks use the same mapping. - A reviewer is matched by user id. - A disabled policy contributes no rules. - An owning team's policies contribute only their deny rules to callers outside the team. - PolicyResource no longer rewrites OrganizationPolicy on startup. - Policy and role writes are identity endpoints; BotNonAdminImpersonationPolicy also denies bot targets, with a 2.1.0 data migration for stored policies. - Team and role policy conditions are rewritten on hard delete only; soft delete and restore drop the cached policies. ADR:2026-11-03-owners-of-configuration-entities-need-edit-all ADR:2026-11-03-a-reviewer-is-matched-by-user-id ADR:2026-11-03-a-disabled-policy-has-no-rules ADR:2026-11-03-an-owning-teams-allow-rules-stay-with-its-members ADR:2026-11-03-seeded-policies-are-not-reset-on-startup ADR:2026-10-26-impersonated-requests-never-reach-credentials-or-grant-privileges ADR:2026-10-30-deleted-policy-condition-references-become-false
- A move or copy is authorized at its from location as well as its path. - A field name selects a narrower operation only where the entity's type declares that field; below a map or an untyped object a write needs EditAll. - Tag labels are compared together with the object that carries them, on the entity the repository would store; touching tags without a net change needs EditTags, an empty patch needs ViewBasic, and a glossary term is always EditGlossaryTerms. - The user, notification-template, context-file and conversation patch guards check every touched location. - Non-admin callers cannot copy or move a value out of a service connection or a pipeline's source config. - Date fields refuse object and array values. ADR:2026-11-03-a-patch-is-authorized-at-every-location-it-touches
- On create, hasDomain() needs a domain of the stored container and every domain the request names. - A root parent reference without an id is loaded by name and a missing one fails the request; TagMapper resolves the classification by name. - A tag's parent must belong to the tag's classification. - PUT contexts see soft-deleted entities, as the write does. - A system tag's fully qualified name cannot change through its parent or classification either. ADR:2026-11-03-a-create-is-authorized-against-its-stored-container
- The FQN-prefix cleanup of tag labels and field relationships runs only for taggable types and for types, whose custom property definitions live under their name. - Custom property definitions are removed only when their type is deleted. - The tag-FQN label delete without a source is removed; tags and glossary terms keep removing their own source-bound labels. ADR:2026-11-03-a-hard-delete-removes-only-rows-of-the-deleted-entity
…check what they return - The lineage graph cache stores graphs serialized and hands each reader its own copy, so a caller's filtering never changes the graph the next caller gets. - Lineage hydrate loads entities with every attribute a policy condition reads, requires the view operations of the requested fields (shared with GET through ViewOperations), and trims the entities back to the requested fields. - The column read and the ontology endpoints authorize copies loaded with the authorization fields; ontology pack install authorizes its target glossary by id. ADR:2026-11-03-every-read-path-checks-the-fields-it-returns ADR:2026-10-30-search-lineage-returns-only-assets-the-caller-can-view
…w row-level rules - Tag conditions read the tags of inline children (columns, tasks, schema fields, features) on every read path, from a copy loaded with each repository's policy tag projection; nested struct columns and fields are merged at every depth. - Listings check each row when a row-dependent rule could refuse it, refill short cursor pages from the following pages (up to 5 reads or 1000 rows) and seal their cursors; offset and search listings, glossary-term lists and test-suite lists drop such rows. - History keeps versions of entities the caller may view; memories and files also apply their own visibility. - Reference lists (user and team owns, follows, children, descendants, parents; schema tables; database schemas) keep only entities the caller may view. ADR:2026-11-03-tag-conditions-see-the-tags-of-inline-children ADR:2026-11-03-a-listing-returns-only-rows-the-caller-may-read ADR:2026-11-03-a-reference-list-names-only-entities-the-caller-may-view
- Conversations and tasks read by id, with their replies and versions, also need ViewBasic on the entity they are about; a refused conversation answers 404. - Activity by entity link matches the link's entity type and keeps only viewable events. - Team hierarchy, task counts, domain hierarchy and the type custom-property reads need ViewBasic on their type; the team hierarchy drops teams the caller may not view and still serves joinable teams to someone signing up. The role list uses the standard listing. - The global profiler setting is always authorized on tables. ADR:2026-11-03-read-handlers-authorize-like-the-entity-reads-they-mirror
- Pipeline run documents carry the pipeline's owners, domains, tags (tasks' included), tier, certification and service; the run index is covered by rules on pipelines. A pipeline's access field change re-stamps its runs, and reindexing pipelines re-stamps the runs of each batch. Table-observability documents in the same index carry the pipeline's fields. - Table and column documents carry policyTags, every tag the table's endpoints decide on; matchAnyTag and matchAllTags match a document's own labels or its policyTags. Under access control a table's tags-only change rebuilds its column documents. - Live Data Insights charts aggregate under the caller's search access policies; snapshot charts and previews drop a restricted caller's own filter; a streaming session is joined only by its user with the same filter. - Vector search keeps only hits the caller may view. - For callers other than admins and bots, search leaves out user emails and refuses email lookups; audit-log and alert event listings mask user emails. - Run-index mappings gain the copied fields; table and column mappings gain policyTags (all languages). ADR:2026-11-03-search-applies-the-same-access-rules-as-entity-reads ADR:2026-10-08-column-entries-carry-only-their-own-tags
❌ 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 |
github-actions
Bot
added
backend
safe to test
The ACS stores the hash of a one-time code on the pending login and redirects to /api/v1/saml/complete with that code. Completing the login needs both the session cookie and the code.
For callers who do not read user emails, a query or filter that runs a script is refused like one that names an email field, since a script can build a field name at run time.
A refresh replaces its refresh token with the new one in a single UPDATE keyed by the old token, for basic and LDAP sign-ins. Only the request whose update changed the row gets new tokens; a token deleted while the request ran is not renewed.
Deleting a type still removes its custom property definitions, and no longer touches tag_usage, whose rows record no entity type.
# Conflicts: # openmetadata-service/src/main/java/org/openmetadata/service/resources/domains/DomainResource.java
# Conflicts: # openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/TeamResource.java # openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java # openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/BuildSearchIndexDocTest.java
| // Owners of a configuration entity, such as a service, need EditAll as well as EditOwners | ||
| const configEffect = | ||
| config.testId === 'edit-owner' && options.ownerEditNeedsEditAll | ||
| ? 'deny' | ||
| : effect; | ||
| await checkElementVisibility(testUserPage, config, configEffect); |
There was a problem hiding this comment.
Why one needs explicite EditAll despite EditOwner?
There was a problem hiding this comment.
Setting owners is EditOwners, but OrganizationPolicy also gives an entity's owner All on it. On configuration types (services, policies, teams, apps, workflows, ingestion pipelines, classifications, test definitions, ...) owning the entity means controlling its configuration (a service's connection, a policy's rules, a team's default roles), so this PR treats setting their owners as a configuration change: it needs EditAll as well as EditOwners, and a rule on EditOwners still applies. Data assets are unchanged; EditOwners alone is enough there.
The permissions response follows the same rule, so for a user with EditOwners but not EditAll the service page no longer offers the owner edit, which is what this spec now expects. Details are in docs/decisions/2026-11-03-owners-of-configuration-entities-need-edit-all.md. I reworded the comment in the test to say this (2133ba8).
There was a problem hiding this comment.
Update: dropped this from the PR (edc97c0). Setting owners asks for EditOwners on every entity type again, as on main; who holds it is up to the configured policies. The spec and helper are back to their main versions.
…onfigured policies Setting owners asks for EditOwners on every entity type, as before this branch. Who holds EditOwners is decided by the policies, which admins configure.
# Conflicts: # openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TeamResourceIT.java # openmetadata-service/src/main/java/org/openmetadata/service/security/ViewPermissionFilter.java
Code Review ✅ Approved 3 closed / 3 findings🔴 High risk · Hardening authorization across reads, writes, search, account changes, and SAML/session flows could expose data or permit unintended access if checks regress. Third batch of backend security-hardening fixes covering data access, sessions, policies, write authorization, search, and reads, with each area carrying its own regression tests. Resolved this session: filtered listing first page now returns a non-null ✅ 3 closed✅ Bug: Filtered listing's first page gets a non-null
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source
|
|
Summary
A third batch of backend security-hardening fixes from an internal review, one commit per area:
Each commit carries its own regression tests. The rules they set are recorded under
docs/decisions/; details are tracked internally.Upgrade notes:
Tracking: open-metadata/openmetadata-collate#8018
🤖 Generated with Claude Code