mirror of
https://github.com/makeplane/plane.git
synced 2026-09-02 12:09:14 +02:00
83d5bda1ce
The project-membership rule for asset access lived as a method on
WorkspaceFileAssetEndpoint with three call sites. Every other asset route is a
sibling BaseAPIView subclass and therefore could not reach it, so four
workspace-level routes in the app and the whole external-API asset surface
resolved assets on the workspace alone. A workspace member or guest belonging to
none of the asset's projects could download another project's uploads, copy them
into a project they controlled, reverse a deletion its owner performed, probe
for asset ids, and flip is_uploaded to take an attachment offline.
Move the rule onto the model as FileAsset.is_project_accessible_to so any
surface that can load a FileAsset can ask the question, and a route added later
cannot silently omit it -- which is how this gap arose.
app/views/asset/v2.py
- AssetRestoreEndpoint.post, AssetCheckEndpoint.get,
WorkspaceAssetDownloadEndpoint.get and DuplicateAssetEndpoint.post now check
the asset's project. Check answers exists=false rather than confirming a
foreign asset, so it stops being a cross-project existence oracle.
- DuplicateAssetEndpoint also requires active membership of the destination
project from the request body; existence in the workspace was being treated
as authorization.
api/views/asset.py
- GenericAssetEndpoint.get and .patch gained the same check. is_uploaded gates
every download path, so an unscoped patch is a takedown primitive.
- .post validates the body-supplied project_id against the URL workspace and
the caller's membership; it was stored unvalidated, so a row in one
workspace could point at a project in another -- exactly the inconsistency
the access check has to defend against downstream.
Also fixes three unconditional 500s in the same file: S3Storage.__init__ is
(self, request=None) and never accepted is_server, so S3Storage(request=request,
is_server=True) at :338, :451 and :581 raised TypeError for every caller.
Passing no request is what selects the internal endpoint, making the keyword
both wrong and redundant. This ships with the authorization checks on purpose:
repairing the crash alone would have exposed a cross-project asset read on a
route that currently only looks harmless.
Contract tests cover each route from a non-member, a member, and a
workspace-level asset whose project_id is NULL, so the fix cannot over-reach.
The external-API positive paths patch S3Storage with autospec=True so the
constructor signature is validated and the crash cannot regress unnoticed.
Adds plane/tests/contract/api/conftest.py to reset the ApiKeyRateThrottle bucket
around each external-API contract test. That throttle keys on the token string,
which is a constant across the package, so all of those tests shared one
60/minute budget for the whole run. Adding tests here pushed the package past it
and produced 429s in unrelated files. Only this throttle's key is cleared,
following the existing narrowly-scoped auth-throttle helper rather than
cache.clear().
Co-authored-by: Plane AI <noreply@plane.so>