diff --git a/pkg/models/api_routes.go b/pkg/models/api_routes.go index 2f1f6e0ed..e601515ab 100644 --- a/pkg/models/api_routes.go +++ b/pkg/models/api_routes.go @@ -249,12 +249,9 @@ func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) { target := apiTokenRoutes if isV2Path(route.Path) { target = apiTokenRoutesV2 - // AutoPatch synthesises a PATCH counterpart for every PUT route in - // the /api/v2 surface. Both methods derive the same "update" - // permission, so storing the PATCH one would clobber the PUT one - // (last-write-wins on the map). Skip PATCH during collection — - // PUT is the authoritative update verb for API tokens; JWT clients - // still get PATCH because auth isn't gated on this table. + // AutoPatch's synthesised PATCH and the original PUT both derive the + // "update" permission and would clobber each other on the map. Store + // only PUT; CanDoAPIRoute accepts PATCH as its alias on the same path. if route.Method == http.MethodPatch { return } @@ -372,11 +369,10 @@ func GetAvailableAPIRoutesForToken(c *echo.Context) error { // stored (Path, Method) for that permission matches exactly. This closes // GHSA-v479-vf79-mg83 and the wider method/sub-resource confusion it // enabled. The one exception is the tasks.read_all quirk handled below. -// -// Tokens are granted by (group, permission) name (e.g. labels.read_one), -// so a single permission can legitimately match both the v1 and v2 routes -// for the same resource. We consult apiTokenRoutes for v1 and the -// apiTokenRoutesV2 shadow map for v2. +// One (group, permission) pair can legitimately match both v1 and v2 +// routes; we walk apiTokenRoutes and apiTokenRoutesV2 in turn. On v2, +// PATCH is accepted as an alias for the stored PUT on the same path +// (AutoPatch collapses both onto the "update" permission). func CanDoAPIRoute(c *echo.Context, token *APIToken) (can bool) { path := c.Path() if path == "" { @@ -400,6 +396,13 @@ func CanDoAPIRoute(c *echo.Context, token *APIToken) (can bool) { if rd.Method == method && rd.Path == path { return true } + // v2: AutoPatch mirrors every PUT as a PATCH on the same + // path. PATCH isn't stored (it would clobber PUT under + // the same "update" key), so accept it as an alias here. + if isV2Path(rd.Path) && rd.Method == http.MethodPut && + method == http.MethodPatch && rd.Path == path { + return true + } // Two list endpoints share tasks.read_all but only one // survives collection, so allow either explicitly. if group == "tasks" && p == "read_all" && method == http.MethodGet && diff --git a/pkg/models/api_routes_test.go b/pkg/models/api_routes_test.go index b47ee86be..b4b0e1661 100644 --- a/pkg/models/api_routes_test.go +++ b/pkg/models/api_routes_test.go @@ -17,6 +17,7 @@ package models import ( + "net/http/httptest" "testing" "github.com/labstack/echo/v5" @@ -57,14 +58,14 @@ func TestCanDoAPIRoute_BulkLabelTask(t *testing.T) { func TestIsV2Path(t *testing.T) { cases := map[string]bool{ - "/api/v2": true, - "/api/v2/": true, - "/api/v2/labels": true, - "/api/v1/labels": false, - "/api/v1/api/v2": false, // prefix is authoritative - "": false, - "/api/v20/labels": false, // only exact /api/v2 prefix counts - "/api/v2labels": false, + "/api/v2": true, + "/api/v2/": true, + "/api/v2/labels": true, + "/api/v1/labels": false, + "/api/v1/api/v2": false, // prefix is authoritative + "": false, + "/api/v20/labels": false, // only exact /api/v2 prefix counts + "/api/v2labels": false, } for path, want := range cases { t.Run(path, func(t *testing.T) { @@ -75,13 +76,13 @@ func TestIsV2Path(t *testing.T) { func TestStripAPIVersion(t *testing.T) { cases := map[string]string{ - "/api/v1/labels": "labels", - "/api/v2/labels": "labels", - "/api/v2/labels/42": "labels/42", - "/api/v1/tasks/bulk": "tasks/bulk", - "/api/v3/labels": "/api/v3/labels", // unknown versions pass through - "/labels": "/labels", - "": "", + "/api/v1/labels": "labels", + "/api/v2/labels": "labels", + "/api/v2/labels/42": "labels/42", + "/api/v1/tasks/bulk": "tasks/bulk", + "/api/v3/labels": "/api/v3/labels", // unknown versions pass through + "/labels": "/labels", + "": "", } for path, want := range cases { t.Run(path, func(t *testing.T) { @@ -142,6 +143,60 @@ func TestGetRouteDetail_V2Verbs(t *testing.T) { } } +// TestCanDoAPIRoute_V2PatchAliasesPut verifies that a token granted the +// "update" permission on a v2 resource can issue PATCH requests against +// the same path as the stored PUT route. Huma's AutoPatch synthesises +// PATCH for every PUT — the matcher accepts it as an alias so token +// holders aren't forced to use PUT exclusively. +func TestCanDoAPIRoute_V2PatchAliasesPut(t *testing.T) { + apiTokenRoutes = make(map[string]APITokenRoute) + apiTokenRoutesV2 = make(map[string]APITokenRoute) + apiTokenRoutes["caldav"] = APITokenRoute{ + "access": &RouteDetail{Path: "/dav/*", Method: "ANY"}, + } + + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "PUT", Path: "/api/v2/labels/:id"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "PATCH", Path: "/api/v2/labels/:id"}, true) + + token := &APIToken{ + APIPermissions: APIPermissions{"labels": []string{"update"}}, + } + + e := echo.New() + + t.Run("PUT is allowed (stored verb)", func(t *testing.T) { + req := httptest.NewRequest("PUT", "/api/v2/labels/:id", nil) + c := e.NewContext(req, httptest.NewRecorder()) + assert.True(t, CanDoAPIRoute(c, token)) + }) + + t.Run("PATCH is allowed via alias", func(t *testing.T) { + req := httptest.NewRequest("PATCH", "/api/v2/labels/:id", nil) + c := e.NewContext(req, httptest.NewRecorder()) + assert.True(t, CanDoAPIRoute(c, token)) + }) + + t.Run("PATCH on a different path is rejected", func(t *testing.T) { + req := httptest.NewRequest("PATCH", "/api/v2/projects/:id", nil) + c := e.NewContext(req, httptest.NewRecorder()) + assert.False(t, CanDoAPIRoute(c, token)) + }) + + t.Run("v1 PATCH stays rejected", func(t *testing.T) { + // The alias must not bleed onto v1 — v1 has no AutoPatch and + // never registers PATCH on update routes. + apiTokenRoutes["labels"] = APITokenRoute{ + "update": &RouteDetail{Path: "/api/v1/labels/:id", Method: "POST"}, + } + v1Token := &APIToken{ + APIPermissions: APIPermissions{"labels": []string{"update"}}, + } + req := httptest.NewRequest("PATCH", "/api/v1/labels/:id", nil) + c := e.NewContext(req, httptest.NewRecorder()) + assert.False(t, CanDoAPIRoute(c, v1Token)) + }) +} + // End-to-end CanDoAPIRoute coverage for /api/v2 is provided by the Label // integration test in pkg/webtests/huma_label_test.go (see the token-auth // scenarios in that file) which exercises the full auth pipeline.