fix(dar): reject duplicate active Data Access Requests per user and entity#28853
fix(dar): reject duplicate active Data Access Requests per user and entity#28853yan-3005 wants to merge 11 commits into
Conversation
…ntity Enforce the one-active-request-per-entity rule in the backend so it holds regardless of entry point (Entity Details page or the Data Access Request page). On task creation, a Data Access Request is rejected when the same creator already has an active request for the same entity. A request is "active" in every status except the terminal ones (Rejected, Revoked, Expired); Expired is kept as a string literal until the enum is extended. Different users requesting the same entity, and the same user requesting a different entity, remain allowed.
❌ 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 |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
yan-3005
added
safe to test
There was a problem hiding this comment.
Pull request overview
This PR enforces a backend invariant for Data Access Requests (DAR): a single user can have at most one active DAR per target entity, preventing duplicate submissions via POST /api/v1/tasks (not just UI-guarded entry points).
Changes:
- Added a backend validation on task creation to reject duplicate active DARs for the same
(creator, entity)pair, returning a 400 with a message referencing the existing task id. - Added a DAO query to look up an existing active DAR by
aboutFqnHash + type + createdById, excluding terminal statuses. - Added integration tests covering same-user duplicate rejection and allowed cases for different users / different entities.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskRepository.java | Adds create-time validation to prevent duplicate active DARs per creator+entity. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java | Adds a query to find an existing active DAR for a given about-entity and creator. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/DataAccessRequestValidationIT.java | Adds IT coverage for duplicate DAR rejection and allowed non-duplicate scenarios. |
…kStatus Address review feedback: the hand-written terminal-status denylist (Rejected/Revoked/Expired) wrongly treated Completed/Cancelled/Failed as active, which would block a user from re-requesting an entity whose prior request had already ended in one of those states. Derive the active (non-terminal) status set from the canonical workflow definition CreateTask.isTerminalTaskStatus so the duplicate check can never drift from it. Any status the workflow treats as terminal frees the user to file a new request. Drops the Expired string literal: a future Expired status is terminal by construction and needs no special-casing.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Add DataAccessRequestValidationIT.testDarAfterPreviousRequestTerminal_allowed: create a Data Access Request, close it (resolves to the terminal Cancelled status), then confirm the same user can file a fresh request for the same entity. Locks in the derived active/terminal semantics so future changes to CreateTask.isTerminalTaskStatus cannot silently regress the duplicate guard.
…okup, fix capability tests - Decouple from CreateTask.isTerminalTaskStatus (Copilot): replace the static initializer that force-loaded the Flowable listener class with a local ACTIVE_TASK_STATUSES constant (Open/InProgress/Pending/Approved/Granted); revert isTerminalTaskStatus to package-private. The list is documented to stay in sync with that canonical predicate and ListFilter's active group. - Make the duplicate lookup deterministic (Copilot): ORDER BY createdAt DESC before LIMIT 1 so the rejection message references the newest active request. - Fix three capability tests that legitimately created multiple DARs (different access types) on the same entity, which the one-active-request-per-entity rule now forbids; each access-type request now targets its own table/data product.
Code Review ✅ Approved 2 resolved / 2 findingsEnforces Data Access Request uniqueness by rejecting duplicate active requests per user and entity at the backend level. The implementation addresses potential status list drift by deriving active statuses directly from the canonical terminal task predicate. ✅ 2 resolved✅ Quality: Broadened terminal-status set is untested
✅ Quality: Manual active-status list can silently drift from terminal predicate
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Fixes https://github.com/open-metadata/openmetadata-collate/issues/4463
Problem
A user should be able to have only one active Data Access Request per entity. Today that's only guarded in the UI on the Entity Details page. The
POST /api/v1/tasksendpoint has no such check, so the Data Access Request page (entity dropdown) lets the same user submit a second request for an entity they already have a pending request for, creating duplicates.Fix
Enforce the rule in the backend (
TaskRepository.prepare, create path only) so it holds for every entry point. On creation of aDataAccessRequest, the request is rejected with400when the same creator already has an active request for the same entity.Rejected,Revoked,Expired.Expiredis a forthcoming status not yet inTaskEntityStatus, so it is kept as a string literal until the enum is extended.aboutFqnHash) and per-creator (viacreatedById).Tests
DataAccessRequestValidationIT:Known limitation
Check-then-insert is not atomic — two concurrent POSTs can both pass the guard. This matches the existing connector-capability validator's behavior. A unique DB index would be the airtight fix but is out of scope here.