kodev-org
/
vikunja
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(models): accept v2 PATCH as alias for PUT in API token matcher 

Huma's AutoPatch synthesises a PATCH counterpart for every PUT, and both
verbs collapse to the same "update" permission. PATCH is still skipped
during collection (it would clobber PUT under the shared key), but the
matcher now accepts it as an alias for the stored PUT route on the same
path, so token holders aren't forced to use PUT exclusively.
This commit is contained in:
kolaente 2026-05-30 16:46:34 +02:00 committed by kolaente
parent 8a4f5cbe11
commit b56a74d6a7
2 changed files with 84 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
pkg/models/api_routes.go
View File

@ -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 &&

85
pkg/models/api_routes_test.go
View File

@ -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.