feat(feed): per-entity CreateTask/EditTask authorization for Feed task threads#29166
feat(feed): per-entity CreateTask/EditTask authorization for Feed task threads#29166yan-3005 wants to merge 6 commits into
Conversation
…eed thread tasks Adds two new MetadataOperation verbs and wires them into the Feed API so that filing or editing a Task thread is now authorized against the target entity (the entity referenced by `about`) instead of the abstract `task` resource. This lets policy authors write conditional rules like "only owners of this table can file tasks against it" using the standard `isOwner()` / `matchAnyTag()` expressions, which previously could not be evaluated at the resource-level `task.Create` check because no entity instance was in scope. Changes: - `resourceDescriptor.json`: add `CreateTask` and `EditTask` to the MetadataOperation enum. - `ResourceRegistry`: expose both verbs on every entity via COMMON_OPERATIONS, so policies can target table/topic/dashboard/etc. `EditTask` is auto-granted by `EditAll` via existing Edit* subsumption. - `FeedResource.createThread` / `updateThread`: when the thread type is `Task`, resolve the `about` entity link and authorize `CreateTask` (POST) or `EditTask` (PATCH) against the target entity. Other thread types (Conversation) are unaffected. - `DataConsumerPolicy.json`: preserve current UX by keeping the legacy `task.Create` rule (so the existing UI resource-level gate keeps returning true) and adding a new `all`-scoped rule granting `CreateTask` and `EditTask` to every authenticated user by default. Admins restrict by overriding the new rule with conditions. Tests: - `ResourceRegistryTaskOperationsTest`: unit test asserting both verbs appear on every entity descriptor. - `FeedTaskAuthzIT`: integration tests covering admin happy paths (POST + PATCH) and DENY-policy enforcement for both verbs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yan-3005
added
the
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
✅ TypeScript Types Auto-UpdatedThe generated TypeScript types have been automatically updated based on JSON schema changes in this PR. |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…sumerPolicy Seed policies are create-if-not-exists in initializeEntity, so the new DataConsumerPolicy-TaskRule added in the previous commit only takes effect on fresh installations. Existing tenants would silently lose task-filing for non-admin users on upgrade because FeedResource.authorizeThreadCreate / authorizeThreadEdit now require CreateTask / EditTask grants that aren't present in the persisted DataConsumerPolicy row. Adds addTaskRuleToDataConsumerPolicy() to v1130 MigrationUtil mirroring the v200 addCreateTaskRuleToDataConsumerPolicy() precedent. The migration: - locates the system DataConsumerPolicy - checks for an existing DataConsumerPolicy-TaskRule - if missing, appends an allow rule for CreateTask + EditTask on resources=["all"] - swallows EntityNotFoundException (fresh installs may not have it yet) and logs other failures rather than failing the migration Wired into mysql and postgres v1130 Migration.runDataMigration(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…v200 The development version is now 2.0.0-SNAPSHOT, so the migration belongs in v200 alongside the existing addCreateTaskRuleToDataConsumerPolicy backfill it follows. Mirrors the v200 precedent: locate DataConsumerPolicy by name, append DataConsumerPolicy-TaskRule (CreateTask + EditTask on resources=["all"]) if absent, idempotent existence check, swallow EntityNotFoundException for fresh installs, log other failures without aborting the migration. Wired into runDataMigration() in mysql/v200/Migration and postgres/v200/Migration. Removes the v1130 placement and the unused imports it pulled into v1130/MigrationUtil. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Jest test CoverageUI tests summary
|
Code Review ✅ Approved 2 resolved / 2 findingsEnables per-entity authorization for Feed tasks by adding CreateTask and EditTask operations, successfully migrating existing policies to maintain backward compatibility. The implementation secures task creation and editing workflows while allowing for granular conditional rules. ✅ 2 resolved✅ Bug: Upgrade regression: existing tenants lose task-filing without policy migration
✅ Edge Case: Backfill failure is silently swallowed, regression can persist
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
Summary
Background
When a user files a Data Access Request (or any task) against table T, today's authorization checks operation `Create` on the global `task` resource. There is no entity in scope, so a rule like `isOwner()` can never be meaningfully evaluated — the rule's condition treats the would-be task itself as the subject, but the task doesn't exist yet and has no owner. Result: `CONDITIONAL_ALLOW` at resource level, UI converts to `false`, button hidden.
The fix is structural: the verb that gates task creation must live on the target entity's resource so the condition evaluates against table T (which has owners, tags, etc.). Existing parallel pattern: `CreateTests` / `EditTests` on table for test-case workflows.
Changes
`EditTask` is auto-granted by anyone holding `EditAll` on the entity via the existing `Edit*` subsumption logic in `CompiledRule`. `CreateTask` is not subsumed — must be granted explicitly (matches `CreateTests` precedent).
Tests
Local run: `Tests run: 4, Failures: 0, Errors: 0, Skipped: 0`.
Test plan
Notes for downstream UI work (not in this PR)
The current UI gate (`getResourcePermission(TASK).Create`) continues to work because the legacy `task.Create` seed rule is preserved. A follow-up UI change can switch to entity-level `getEntityPermissionByFqn(aboutType, aboutFqn).CreateTask` to also hide the button for non-owners — without that change, non-owners see the button and get a 403 toast on submit.
🤖 Generated with Claude Code
Greptile Summary
This PR adds two new
MetadataOperationverbs —CreateTaskandEditTask— and wires them into the Feed API so task thread creation and editing are authorized against the target entity referenced byabout, not the abstracttaskresource. A matching migration step and seed-policy update ensure that existing deployments preserve default UX while admins can now apply per-entity conditional rules likeisOwner().POST /v1/feed(Task type) andPATCH /v1/feed/{id}(Task type) callauthorizeAgainstAbout, which builds an entity-scopedResourceContextandOperationContextand runs them through the standardAuthorizer, enabling conditions that inspect actual entity attributes.addTaskRuleToDataConsumerPolicyinv200/MigrationUtilensures upgraded tenants receive the newDataConsumerPolicy-TaskRule; the seed JSON receives the same rule for fresh installs.CreateTask/EditTaskenum values are added toresourceDescriptor.jsonand five TypeScript-generated files, completing the type surface across stack layers.Confidence Score: 5/5
Safe to merge — the new authorization gates follow established patterns, the migration is idempotent, and the default policy preserves existing UX for all non-admin users.
The core authorization change is narrowly scoped: two new helper methods in FeedResource gate only Task-type threads, leaving all other thread types untouched. The migration step is idempotent (name-check before insert) and covered by both MySQL and Postgres paths. The only cosmetic concern is that the seed JSON renames the legacy task.Create rule entry while the existing migration constant still uses the old name, which can create a redundant rule entry depending on startup order — no functional impact, no security implication.
openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java — the CREATE_TASK_RULE_NAME constant is out of sync with the renamed seed entry.
Important Files Changed
Comments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/FeedResource.java, line 297-337 (link)EditTaskgate bypassed by resolve/close endpointsPATCH /v1/feed/{id}now enforcesEditTask, but the task-specificPUT /tasks/{id}/resolveandPUT /tasks/{id}/closeendpoints both calldao.checkPermissionsForResolveTask(), which checks admin, owner, assignee, and creator roles — it has no awareness of the newEditTaskoperation. A user explicitly deniedEditTaskon a target entity can still mutate task state (resolve or close) through these endpoints, bypassing the intent of the new per-entity authorization model.Reviews (6): Last reviewed commit: "Merge branch 'main' into dar-edit-tasks-..." | Re-trigger Greptile