Skip to content

Restrict cross-user follower changes (backport #32322 to 1.13) - #32790

Open
sonika-shah wants to merge 1 commit into
1.13from
backport-32322-1.13
Open

Restrict cross-user follower changes (backport #32322 to 1.13)#32790
sonika-shah wants to merge 1 commit into
1.13from
backport-32322-1.13

Conversation

@sonika-shah

Copy link
Copy Markdown
Collaborator

Backport of #32322 (GHSA-962g-h22r-w7hg) to 1.13.

Restrict cross-user follower changes: EntityResource owns guarded add/delete follower helpers that reject a null userId with a 400, compare the requested user with the authenticated subject via null-safe equality, and require admin authorization for cross-user mutations. All follower endpoints delegate to those helpers. Also carries the EntityRepository.buildReadBundle cache fix (skip the read-bundle cache for UNCACHED_ENTITY_TYPES).

1.13-specific adaptations (vs. the main commit 2fb1cb523a)

  • KnowledgePageResource excluded — that entity does not exist on 1.13.
  • Re-added the BadRequestException import in EntityResource — the cherry-pick dropped the import hunk (the import block diverged); the class exists on 1.13.
  • DataProductResourceIT: ForbiddenExceptionApiException — 1.13's SDK maps a 403 to the base ApiException (it has no ForbiddenException); the getStatusCode() == 403 assertions are unchanged. The 400 case keeps InvalidRequestException.

openmetadata-service + openmetadata-integration-tests build green locally.

Original PR: #32322

* fix(security): restrict cross-user follower changes

* fix(security): validate follower user IDs

* fix(cache): bypass read-bundle cache for uncached entities

buildReadBundle used the read-bundle cache for every non-deleted read,
ignoring UNCACHED_ENTITY_TYPES (user, task, workflow*, bot, domain, dataProduct).
For those types a mutation followed by an immediate re-read — e.g. the follower
add/delete then fetch in this PR's DataProductResourceIT — could serve a stale
cached bundle. Without this, the follower authorization tests fail under a
cache-enabled deployment (postgres-elasticsearch-redis / postgres-opensearch
integration lanes) with `expected <false> but was <true>` after a delete.

Gate the read-bundle cache on isCacheableEntityType(entityType) so uncached
types take the already-supported bundleCache == null path and always reflect the
current entity_relationship state. Required for the follower tests to pass on the
Redis/OpenSearch lanes (documents why this caching change is part of the security
fix, per reviewer request).

* test(followers): rename put_-prefixed delete-follower tests to delete_ prefix

The two round-trip tests exercise the DELETE follower endpoint (add is setup),
so name them with the delete_ verb prefix to match the method-name convention
(addresses the Copilot review comment).

---------

Co-authored-by: sonika-shah <58761340+sonika-shah@users.noreply.github.com>
@sonika-shah
sonika-shah requested a review from a team as a code owner September 7, 2026 03:03
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

❌ 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 github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Sep 7, 2026
@gitar-bot

gitar-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Backport of security fix #32322 to 1.13 that restricts cross-user follower changes by adding authorization checks to EntityResource follower endpoints, rejecting null userId with 400, and requiring admin rights for cross-user mutations. Includes 1.13-specific adaptations: excludes KnowledgePageResource (not present on 1.13), re-adds the BadRequestException import, and updates DataProductResourceIT to expect ApiException instead of ForbiddenException for 403 responses. Also carries the EntityRepository.buildReadBundle cache fix. No issues found.

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

@sonarqubecloud

sonarqubecloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants