diff --git a/pkg/models/api_routes.go b/pkg/models/api_routes.go index 607274092..2f1f6e0ed 100644 --- a/pkg/models/api_routes.go +++ b/pkg/models/api_routes.go @@ -27,8 +27,15 @@ import ( var apiTokenRoutes = map[string]APITokenRoute{} +// apiTokenRoutesV2 holds /api/v2 routes under the same (group, permission) +// keys as v1, so a token granted e.g. labels.read_one authorises both +// versions. The frontend token UI still reads only apiTokenRoutes; +// CanDoAPIRoute consults both tables. +var apiTokenRoutesV2 = map[string]APITokenRoute{} + func init() { apiTokenRoutes = make(map[string]APITokenRoute) + apiTokenRoutesV2 = make(map[string]APITokenRoute) apiTokenRoutes["caldav"] = APITokenRoute{ "access": &RouteDetail{ Path: "/dav/*", @@ -50,8 +57,25 @@ type RouteDetail struct { Method string `json:"method"` } +// isV2Path reports whether the given route path lives under /api/v2. +func isV2Path(path string) bool { + return strings.HasPrefix(path, "/api/v2/") || path == "/api/v2" +} + +// stripAPIVersion removes the /api/v1/ or /api/v2/ prefix so both +// versions normalise to the same token-permission group name. +func stripAPIVersion(path string) string { + if stripped := strings.TrimPrefix(path, "/api/v1/"); stripped != path { + return stripped + } + if stripped := strings.TrimPrefix(path, "/api/v2/"); stripped != path { + return stripped + } + return path +} + func getRouteGroupName(path string) (finalName string, filteredParts []string) { - parts := strings.Split(strings.TrimPrefix(path, "/api/v1/"), "/") + parts := strings.Split(stripAPIVersion(path), "/") filteredParts = []string{} for _, part := range parts { if strings.HasPrefix(part, ":") { @@ -75,6 +99,9 @@ func getRouteGroupName(path string) (finalName string, filteredParts []string) { // getRouteDetail determines the API permission type from the route's HTTP method and path. // In Echo v5, route.Name is auto-generated as METHOD:PATH, so we derive permissions from // the HTTP method and path structure instead of the handler function name. +// +// v1 and v2 have inverted create/update verbs: v1 uses PUT for create and POST +// for update, v2 follows REST conventions (POST create, PUT/PATCH update). func getRouteDetail(route echo.RouteInfo) (method string, detail *RouteDetail) { detail = &RouteDetail{ Path: route.Path, @@ -88,6 +115,7 @@ func getRouteDetail(route echo.RouteInfo) (method string, detail *RouteDetail) { lastPart = pathParts[len(pathParts)-1] } endsWithParam := strings.HasPrefix(lastPart, ":") + v2 := isV2Path(route.Path) switch route.Method { case http.MethodGet: @@ -96,10 +124,21 @@ func getRouteDetail(route echo.RouteInfo) (method string, detail *RouteDetail) { } return "read_all", detail case http.MethodPut: - // PUT is used for creating resources in this codebase + if v2 { + // v2: PUT replaces an existing resource → update. + return "update", detail + } + // v1: PUT is used for creating resources. return "create", detail case http.MethodPost: - // POST is used for updating resources + if v2 { + // v2: POST creates a new resource on the collection. + return "create", detail + } + // v1: POST is used for updating resources. + return "update", detail + case http.MethodPatch: + // Both versions use PATCH for partial updates. return "update", detail case http.MethodDelete: return "delete", detail @@ -108,9 +147,9 @@ func getRouteDetail(route echo.RouteInfo) (method string, detail *RouteDetail) { return "", detail } -func ensureAPITokenRoutesGroup(group string) { - if _, has := apiTokenRoutes[group]; !has { - apiTokenRoutes[group] = make(APITokenRoute) +func ensureAPITokenRoutesGroup(target map[string]APITokenRoute, group string) { + if _, has := target[group]; !has { + target[group] = make(APITokenRoute) } } @@ -183,8 +222,10 @@ func isStandardCRUDRoute(routeGroupName string, routeParts []string, _ string) b return false } -// CollectRoutesForAPITokenUsage gets called for every added APITokenRoute and builds a list of all routes we can use for the api tokens. -// The requiresJWT parameter indicates if this route is protected by JWT authentication. +// CollectRoutesForAPITokenUsage records a route for token authorisation. +// v1 and v2 share group/permission keys derived from the prefix-stripped +// path; v2 entries land in apiTokenRoutesV2 so the v1-only frontend UI is +// unchanged while CanDoAPIRoute consults both tables. func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) { if route.Method == "echo_route_not_found" { @@ -205,6 +246,20 @@ func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) { return } + 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. + if route.Method == http.MethodPatch { + return + } + } + // Check if this is a standard CRUD route using path-based heuristics // In Echo v5, we can no longer rely on route.Name containing "(*WebHandler)" isCRUD := isStandardCRUDRoute(routeGroupName, routeParts, route.Method) @@ -224,67 +279,67 @@ func CollectRoutesForAPITokenUsage(route echo.RouteInfo, requiresJWT bool) { // Otherwise, we add it to the "other" key. if len(routeParts) == 1 { if routeGroupName == "notifications" && route.Method == http.MethodPost { - ensureAPITokenRoutesGroup("notifications") + ensureAPITokenRoutesGroup(target, "notifications") - apiTokenRoutes["notifications"]["mark_all_as_read"] = routeDetail + target["notifications"]["mark_all_as_read"] = routeDetail return } - ensureAPITokenRoutesGroup("other") + ensureAPITokenRoutesGroup(target, "other") - _, exists := apiTokenRoutes["other"][routeGroupName] + _, exists := target["other"][routeGroupName] if exists { routeGroupName += "_" + strings.ToLower(route.Method) } - apiTokenRoutes["other"][routeGroupName] = routeDetail + target["other"][routeGroupName] = routeDetail return } subkey := strings.Join(routeParts[1:], "_") - if _, has := apiTokenRoutes[routeParts[0]]; !has { - apiTokenRoutes[routeParts[0]] = make(APITokenRoute) + if _, has := target[routeParts[0]]; !has { + target[routeParts[0]] = make(APITokenRoute) } - if _, has := apiTokenRoutes[routeParts[0]][subkey]; has { + if _, has := target[routeParts[0]][subkey]; has { subkey += "_" + strings.ToLower(route.Method) } - apiTokenRoutes[routeParts[0]][subkey] = routeDetail + target[routeParts[0]][subkey] = routeDetail return } if strings.HasSuffix(routeGroupName, "_bulk") { parent := strings.TrimSuffix(routeGroupName, "_bulk") - ensureAPITokenRoutesGroup(parent) + ensureAPITokenRoutesGroup(target, parent) method, routeDetail := getRouteDetail(route) - apiTokenRoutes[parent][method+"_bulk"] = routeDetail + target[parent][method+"_bulk"] = routeDetail return } - _, has := apiTokenRoutes[routeGroupName] + _, has := target[routeGroupName] if !has { - apiTokenRoutes[routeGroupName] = make(APITokenRoute) + target[routeGroupName] = make(APITokenRoute) } method, routeDetail := getRouteDetail(route) if method != "" { - apiTokenRoutes[routeGroupName][method] = routeDetail + target[routeGroupName][method] = routeDetail } // Handle task attachments specially - they use custom handlers not WebHandler if routeGroupName == "tasks_attachments" { // PUT is upload (create), GET with :attachment param is download (read_one) if route.Method == http.MethodPut { - apiTokenRoutes[routeGroupName]["create"] = &RouteDetail{ + target[routeGroupName]["create"] = &RouteDetail{ Path: route.Path, Method: route.Method, } } if route.Method == http.MethodGet && strings.HasSuffix(route.Path, ":attachment") { - apiTokenRoutes[routeGroupName]["read_one"] = &RouteDetail{ + target[routeGroupName]["read_one"] = &RouteDetail{ Path: route.Path, Method: route.Method, } @@ -317,6 +372,11 @@ 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. func CanDoAPIRoute(c *echo.Context, token *APIToken) (can bool) { path := c.Path() if path == "" { @@ -327,23 +387,25 @@ func CanDoAPIRoute(c *echo.Context, token *APIToken) (can bool) { method := c.Request().Method for group, perms := range token.APIPermissions { - routes, has := apiTokenRoutes[group] - if !has { - continue - } - for _, p := range perms { - rd := routes[p] - if rd == nil { + tables := []APITokenRoute{apiTokenRoutes[group], apiTokenRoutesV2[group]} + for _, routes := range tables { + if routes == nil { continue } - if rd.Method == method && 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 && - (path == "/api/v1/tasks" || path == "/api/v1/projects/:project/tasks") { - return true + for _, p := range perms { + rd := routes[p] + if rd == nil { + continue + } + if rd.Method == method && 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 && + (path == "/api/v1/tasks" || path == "/api/v1/projects/:project/tasks") { + return true + } } } } diff --git a/pkg/models/api_routes_test.go b/pkg/models/api_routes_test.go index 3dd9e7d99..b47ee86be 100644 --- a/pkg/models/api_routes_test.go +++ b/pkg/models/api_routes_test.go @@ -54,3 +54,94 @@ func TestCanDoAPIRoute_BulkLabelTask(t *testing.T) { assert.Equal(t, "/api/v1/tasks/:projecttask/labels/bulk", bulkRoute.Path) assert.Equal(t, "POST", bulkRoute.Method) } + +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, + } + for path, want := range cases { + t.Run(path, func(t *testing.T) { + assert.Equal(t, want, isV2Path(path)) + }) + } +} + +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", + "": "", + } + for path, want := range cases { + t.Run(path, func(t *testing.T) { + assert.Equal(t, want, stripAPIVersion(path)) + }) + } +} + +// TestCollectRoutesV2 verifies that /api/v2 routes are stored in the v2 +// shadow table under the same (group, permission) keys their v1 counterparts +// would use. This is what lets a token scoped on `labels.read_one` authorise +// both /api/v1/labels/{id} and /api/v2/labels/{id}. +func TestCollectRoutesV2(t *testing.T) { + apiTokenRoutes = make(map[string]APITokenRoute) + apiTokenRoutesV2 = make(map[string]APITokenRoute) + + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "GET", Path: "/api/v2/labels"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "GET", Path: "/api/v2/labels/:id"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "POST", Path: "/api/v2/labels"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "PUT", Path: "/api/v2/labels/:id"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "DELETE", Path: "/api/v2/labels/:id"}, true) + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "PATCH", Path: "/api/v2/labels/:id"}, true) + + // v1 map stays untouched. + assert.Empty(t, apiTokenRoutes, "v2 routes must not land in the v1 table") + + labels, has := apiTokenRoutesV2["labels"] + require.True(t, has, "labels group should exist in v2 table") + assert.Equal(t, "GET", labels["read_all"].Method) + assert.Equal(t, "/api/v2/labels", labels["read_all"].Path) + assert.Equal(t, "GET", labels["read_one"].Method) + assert.Equal(t, "POST", labels["create"].Method) + // PUT is the authoritative update verb for API tokens — PATCH is + // skipped during collection so it doesn't clobber PUT. + assert.Equal(t, "PUT", labels["update"].Method) + assert.Equal(t, "DELETE", labels["delete"].Method) +} + +// TestGetRouteDetail_V2Verbs verifies the v2 verb mapping: POST→create, +// PUT/PATCH→update. v1 inverts POST and PUT so we need a separate mapping +// path. +func TestGetRouteDetail_V2Verbs(t *testing.T) { + cases := []struct { + method, path, wantPerm string + }{ + {"GET", "/api/v2/labels", "read_all"}, + {"GET", "/api/v2/labels/:id", "read_one"}, + {"POST", "/api/v2/labels", "create"}, + {"PUT", "/api/v2/labels/:id", "update"}, + {"PATCH", "/api/v2/labels/:id", "update"}, + {"DELETE", "/api/v2/labels/:id", "delete"}, + } + for _, c := range cases { + t.Run(c.method+" "+c.path, func(t *testing.T) { + perm, _ := getRouteDetail(echo.RouteInfo{Method: c.method, Path: c.path}) + assert.Equal(t, c.wantPerm, perm) + }) + } +} + +// 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.