Repository navigation
fix(use-cases): require access to the use case on destructive and lifecycle routes - #5291
Open
gorkem-bwl wants to merge 4 commits into
Open
gorkem-bwl wants to merge 4 commits into
gorkem-bwl wants to merge 4 commits into
Conversation
…ecycle routes Holding useCase.edit let an Editor with no relationship to a use case delete it, change its status, classify it or move it through its lifecycle. #5270 closed this for PATCH /api/projects/:id only. Add requireProjectAccess, one route guard for the rule that scopes the use case list: Admin, owner or member. Guarded: - DELETE /api/projects/:id - PATCH /api/projects/:id/status - POST /api/projects/:id/eu-ai-act-classification - GET /api/projects/stats/:id - POST /api/v1/use-case-lifecycle/:id/go-live, /retire, /confirm - PATCH /api/v1/use-case-lifecycle/:id/department Behaviour: - No access: 403 "Access denied", logged with logFailure, nothing written. The detail is not one that ends the client's session. - Not found or in another organization: passed to the controller, which answers its 404. - An error in the check is a 500, never allowed. A request with no organization is refused without querying. - The query is always scoped by organization_id and tells the three outcomes apart in one statement (getProjectAccessQuery). - The guard adds to the route rules. The lifecycle rule (useCase.edit or owner, Auditors never) stays in the controller. GET /api/projects/stats/:id threw on a missing use case and answered 500; it now answers 404 like the other routes. GET /api/projects/:id is left out on purpose. A task can be assigned to anyone in the organization and linked to an EU AI Act requirement. Its email opens /project-view, which needs this read, and those requirements have no page outside the use case. Scoping it needs a product decision. The controller check from #5270 stays: it runs inside the update's transaction and also covers PATCH /api/v1/projects/:id. useCaseLifecycle.test.ts asserted that an unrelated Editor could go live, which is the gap itself; that Editor is now refused until added as a member. swagger.yaml and endpoints.ts were regenerated and did not change.
canViewProjectQuery and getProjectAccessQuery ran the same visibility rule through two SQL statements that had to stay in step. getProjectAccessQuery now takes an optional transaction, and canViewProjectQuery is defined from it.
toId returns Infinity for a very long digit string, which passed the guard's NaN and positive checks and reached the access query. Ids that are not safe integers now get the guard's 400. Also: - rename the integration test that claimed the controller answers an unparsable id; the guard runs first on those routes. - document that a refused GET reaches only the file log, because logFailure skips the event log for Read.
This branch has not been 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
Holding
useCase.editlet an Editor with no relationship to a use case delete it, change its status, classify it or move it through its lifecycle. #5270 closed this forPATCH /api/projects/:idonly.This adds
requireProjectAccess, one route guard for the rule that already scopes the use case list: Admin, owner or member.Guarded routes
DELETE /api/projects/:idPATCH /api/projects/:id/statusPOST /api/projects/:id/eu-ai-act-classificationGET /api/projects/stats/:idPOST /api/v1/use-case-lifecycle/:id/go-live,/retire,/confirmPATCH /api/v1/use-case-lifecycle/:id/departmentBehaviour
GET /api/projects/stats/:idnow answers 404 for a missing use case instead of 500.Not in this PR
GET /api/projects/:idstays open for now: non-member task assignees reach it from EU AI Act task emails. A follow-up widens the rule to those assignees and then guards it.PUT, FRIAPUT, PMMstart-cycle, the framework attach and detach routes, and the remaining per-use-case reads check only the organization. They move to the same guard in a follow-up.