mirror of
https://github.com/makeplane/plane.git
synced 2026-09-02 03:59:00 +02:00
7cecb466d3
Authorization in the app viewsets lives on the concrete method — an @allow_permission decorator or an inline role check. BaseViewSet subclasses DRF's ModelViewSet, which supplies list/retrieve/create/update/partial_update/ destroy for free, so when a URLconf maps a verb to an action the viewset does not implement, the request is served by the mixin with nothing but the bare default permission class. The caller is authenticated but not authorized at all, and the only thing between them and the object is whatever get_queryset() happens to filter on. Measured across the live URLconf: 225 routed actions, 27 of which resolved to a mixin under the bare default. The worst let any authenticated account with no membership in the target workspace rewrite a project it could not otherwise read — including its `workspace` field, since ProjectListSerializer declares fields="__all__" with no read_only_fields — re-parenting the project into the caller's own workspace. Others allowed overwriting or soft-deleting work items and comments with no activity record or webhook, reading project invitation tokens, and creating views in arbitrary workspaces by guessable slug. Guard it structurally in BaseViewSet.initial(): if the resolved action is one DRF's mixins provide and nothing in our own MRO implements it, refuse with 405 rather than letting the mixin operate. Three shapes are deliberately exempt — a custom @action, a perform_create override riding CreateModelMixin, and a viewset carrying a genuinely restrictive permission class. Permission classes are membership-tested rather than compared against the default, so a weaker declaration ([AllowAny], or an empty list) is not mistaken for a deliberate restrictive one. Point-fixing these one endpoint at a time is what produced two reports of the same class nine days apart, and it does not hold: of the routes that were not exploitable, most failed closed on an accident — a missing pk kwarg, or a decorator applied to a perform_create signature so it crashed before inserting — rather than on authorization. One lookup_url_kwarg change re-arms them. Also implements the five actions that clients do call and that were relying on a mixin, so they carry the same check as their siblings rather than being refused: issue and comment reaction list, project and workspace view create, workspace invitation list, and state retrieve. A contract test drives the real guard over Django's own resolver and asserts the refused set matches a reviewed manifest, in both directions, so a newly routed verb fails here instead of shipping unauthorized — and a fixed one cannot rot the list. A second manifest covers plane.api and plane.space, which define their own duplicated BaseViewSet and are not reached by this guard. Co-authored-by: Plane AI <noreply@plane.so>