Restrict cross-user follower changes (backport #32322 to 2.0) - #32789
Restrict cross-user follower changes (backport #32322 to 2.0)#32789sonika-shah wants to merge 1 commit into
Conversation
* 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>
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 |
Code Review ✅ ApprovedSecurity backport restricting cross-user follower changes to OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
✅ Playwright Results — workflow succeededValidated commit ✅ 998 passed · ❌ 0 failed · 🟡 2 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 51m 46s ⏱️ Max setup 3m 9s · max shard execution 18m 52s · max shard-job elapsed before upload 22m 6s · reporting 7s 🌐 193.38 requests/attempt · 2.42 app boots/UI scenario · 24.29% common-shard skew Optimization targets still in progress:
🟡 2 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
2 participants
Backport of #32322 (GHSA-962g-h22r-w7hg) to
2.0.Restrict cross-user follower changes:
EntityResourceowns guarded add/delete follower helpers that reject a nulluserIdwith 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 before calling the repository. Also carries the requiredEntityRepository.buildReadBundlecache fix (skip the read-bundle cache forUNCACHED_ENTITY_TYPESso a follower mutation + immediate re-read isn't stale on cache-enabled deployments).2fb1cb523a(44 files, no conflicts).openmetadata-service+openmetadata-integration-testsbuild green locally.Original PR: #32322