Repository navigation
[2.0] fix(security): third batch of backend security-hardening fixes - #37162
Merged
Merged
Conversation
The FQN prefix renames, 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. Tag lists count usage with the bound per-tag query. ListFilter.escapeForMySqlRegexReplacement stays, deprecated, for extensions.
A token verified against OpenMetadata's own signing key, on a request or a WebSocket connection, resolves to the user name it was minted for instead of the configured principal claims, whose email local part can belong to another account. SecurityUtil.getUserName uses the principal name whole.
…nd sign-ins Logout denylists the verified bearer token of the request, not a token named in the body. Denylist entries are keyed by a digest of the token's signed part, hold only who signed out and when, and are capped. A new password, from a reset link, the user, an admin or the reset-password command, ends the user's refresh tokens, live sessions and MCP grants; personal access tokens and bot tokens stay.
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.
The login keeps its AuthnRequest id, and the ACS accepts only a response whose signed assertion answers it. The ACS no longer signs anyone in: it attaches the account to the pending login and redirects to /api/v1/saml/complete on the login's own origin with a one-time code. Completing the login needs both the browser's session cookie and that code, read from the session store.
The email pattern matches in linear time and accepts the same addresses. The message parser's entity-link pattern cannot backtrack across unclosed links. JSON Patch bodies are read with Jackson limits and an operation cap, and a patch on the document root is refused. Description placeholders are restored in one pass, in text positions only.
A PUT naming the caller's own account refuses a deleted self with 404 and treats an unknown self as a sign-up, allowed only when self sign-up is. Deleting your own account needs an admin. A role list that differs from the stored one, in either direction, is an admin change, and a non-admin PUT keeps the stored admin and bot flags and, when omitted, the stored roles. A non-admin CSV row fails for a stored admin or bot.
Rules of a disabled policy no longer apply. An owning team's policies give their allow rules to its members only. A reviewer is matched by user id. Startup no longer rewrites seeded policies. Impersonated requests cannot write policies or roles, and BotNonAdminImpersonationPolicy denies bot targets, added to stored copies by the 2.0.8 migration. A soft-deleted team or role keeps the policy conditions that name it; only a hard delete rewrites them.
A JSON Patch is authorized at every location it touches, the from location of a move or copy included. Narrow edit operations apply only at real fields of the entity; tag changes are compared with their location, and an operation beside a tag change keeps that change's tag operation. The user PATCH guards and the domain check read the same locations. A patch's copies are bounded in size, and a date field refuses an object or array.
…lt; refuse impersonated team grants A user's team changes are decided on the patched result, whatever shape the PATCH operations take, and joining or leaving a closed team needs an admin; a non-admin PUT that omits teams keeps the stored ones. The by-name self-delete check resolves the user and compares ids. Team writes that change default roles or policies, and team CSV imports, refuse impersonated requests.
… membership The team page shows Join and Leave only on a joinable team, or to an admin, as the server allows joining and leaving a closed team only for admins.
- 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 field relationships runs only for taggable types and for types, whose custom property definitions live under their name; tag labels are removed by FQN prefix only for taggable types. - 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
- Tasks read by id, and their versions, also need ViewBasic on the entity they are about. - Activity by entity link matches the link's entity type as well as its FQN. - Team hierarchy, task counts, domain hierarchy and the type custom-property reads need ViewBasic on their type; the team hierarchy 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
…hardening-3 # Conflicts: # openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java # openmetadata-service/src/main/java/org/openmetadata/service/util/JsonPatchUtils.java
mohityadav766
added
safe to test
mohityadav766
requested review from
harsh-vador
and removed request for
a team
November 5, 2026 12:20
Contributor
❌ 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 |
Contributor
Jest test CoverageUI tests summary
|
Code Review ✅ Approved🔴 High risk · Reworks authorization, account privileges, sessions, and SAML login; mistakes could permit unauthorized access or disrupt legitimate sign-ins. Backports the third batch of backend security hardening to 2.0, covering prefix-rename statements, token principal resolution, session and password handling, SAML login completion, input limits, user and team membership authorization, create and PUT authorization, hard-delete cleanup, read handlers, and Team page Join and Leave visibility. No issues found. VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
|
This branch was previously deployed
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
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.
Summary
Backport to 2.0 of the third batch of backend security hardening (from #36952), one commit per area:
Only what applies to 2.0 is carried. Each commit carries its own regression tests. Details are tracked internally.
🤖 Generated with Claude Code