diff --git a/pkg/models/api_routes.go b/pkg/models/api_routes.go index d7a8a437d..8a12b9c03 100644 --- a/pkg/models/api_routes.go +++ b/pkg/models/api_routes.go @@ -74,6 +74,13 @@ func stripAPIVersion(path string) string { return path } +// canonicalAPITokenGroup snake_cases a permission group name. The frontend +// snake_cases request payloads, so a hyphenated group slug (e.g. from +// /api/v2/time-entries) can't round-trip and fails validation on save. +func canonicalAPITokenGroup(group string) string { + return strings.ReplaceAll(group, "-", "_") +} + func getRouteGroupName(path string) (finalName string, filteredParts []string) { parts := strings.Split(stripAPIVersion(path), "/") filteredParts = []string{} @@ -82,7 +89,7 @@ func getRouteGroupName(path string) (finalName string, filteredParts []string) { continue } - filteredParts = append(filteredParts, part) + filteredParts = append(filteredParts, canonicalAPITokenGroup(part)) } finalName = strings.Join(filteredParts, "_") @@ -183,7 +190,7 @@ func isStandardCRUDRoute(routeGroupName string, routeParts []string, _ string) b "comments": true, "relations": true, "attachments": true, - "time-entries": true, + "time_entries": true, "projects_views": true, "projects_teams": true, "projects_users": true, @@ -403,7 +410,8 @@ func CanDoAPIRoute(c *echo.Context, token *APIToken) (can bool) { } method := c.Request().Method - for group, perms := range token.APIPermissions { + for rawGroup, perms := range token.APIPermissions { + group := canonicalAPITokenGroup(rawGroup) tables := []APITokenRoute{apiTokenRoutes[group], apiTokenRoutesV2[group]} for _, routes := range tables { if routes == nil { @@ -448,8 +456,9 @@ func PermissionsAreValid(permissions APIPermissions) (err error) { // resources (no v1 counterpart) live solely in apiTokenRoutesV2, so // validating against the union lets tokens grant them. CanDoAPIRoute // already consults both tables when authorising. - v1Routes := apiTokenRoutes[key] - v2Routes := apiTokenRoutesV2[key] + group := canonicalAPITokenGroup(key) + v1Routes := apiTokenRoutes[group] + v2Routes := apiTokenRoutesV2[group] if v1Routes == nil && v2Routes == nil { return &ErrInvalidAPITokenPermission{ Group: key, diff --git a/pkg/models/api_routes_test.go b/pkg/models/api_routes_test.go index 1cbe6c5fb..65f31a897 100644 --- a/pkg/models/api_routes_test.go +++ b/pkg/models/api_routes_test.go @@ -121,9 +121,9 @@ func TestCollectRoutesV2(t *testing.T) { assert.Equal(t, "DELETE", labels["delete"].Method) } -// TestCollectRoutes_TimeEntriesV2 verifies the v2-only time-entries resource -// lands under a clean "time-entries" group rather than the "other" catch-all, -// so its scopes read sensibly for token clients. +// TestCollectRoutes_TimeEntriesV2 pins the v2-only time-entries resource to a +// snake_case "time_entries" group (not the "other" catch-all, not a hyphenated +// key the frontend's snake_case transform would mangle on save). func TestCollectRoutes_TimeEntriesV2(t *testing.T) { apiTokenRoutes = make(map[string]APITokenRoute) apiTokenRoutesV2 = make(map[string]APITokenRoute) @@ -137,8 +137,11 @@ func TestCollectRoutes_TimeEntriesV2(t *testing.T) { _, isOther := apiTokenRoutesV2["other"] assert.False(t, isOther, "time-entries CRUD must not fall into the 'other' bucket") - te, has := apiTokenRoutesV2["time-entries"] - require.True(t, has, "time-entries group should exist in the v2 table") + _, hyphenated := apiTokenRoutesV2["time-entries"] + assert.False(t, hyphenated, "group key must be canonicalised to snake_case") + + te, has := apiTokenRoutesV2["time_entries"] + require.True(t, has, "time_entries group should exist in the v2 table") assert.Equal(t, "GET", te["read_all"].Method) assert.Equal(t, "/api/v2/time-entries", te["read_all"].Path) assert.Equal(t, "GET", te["read_one"].Method) @@ -148,7 +151,7 @@ func TestCollectRoutes_TimeEntriesV2(t *testing.T) { } // TestGetAPITokenRoutes_ExposesV2Only verifies the /routes payload merges -// v2-only groups (time-entries has no v1 counterpart) so token clients can +// v2-only groups (time_entries has no v1 counterpart) so token clients can // discover and grant them, without mutating the v1 table itself. func TestGetAPITokenRoutes_ExposesV2Only(t *testing.T) { apiTokenRoutes = make(map[string]APITokenRoute) @@ -162,14 +165,35 @@ func TestGetAPITokenRoutes_ExposesV2Only(t *testing.T) { _, hasLabels := routes["labels"] assert.True(t, hasLabels, "v1 groups stay exposed") - te, hasTE := routes["time-entries"] - require.True(t, hasTE, "v2-only time-entries must be exposed via /routes") + te, hasTE := routes["time_entries"] + require.True(t, hasTE, "v2-only time_entries must be exposed via /routes") assert.Equal(t, "GET", te["read_all"].Method) - _, v1HasTE := apiTokenRoutes["time-entries"] + _, v1HasTE := apiTokenRoutes["time_entries"] assert.False(t, v1HasTE, "the merge must not mutate the v1 table") } +// TestCanDoAPIRoute_TimeEntriesHyphenLegacy proves a token stored under the old +// hyphenated "time-entries" key still validates and authorises — no migration. +func TestCanDoAPIRoute_TimeEntriesHyphenLegacy(t *testing.T) { + apiTokenRoutes = make(map[string]APITokenRoute) + apiTokenRoutesV2 = make(map[string]APITokenRoute) + + CollectRoutesForAPITokenUsage(echo.RouteInfo{Method: "GET", Path: "/api/v2/time-entries"}, true) + + for _, key := range []string{"time_entries", "time-entries"} { + t.Run(key, func(t *testing.T) { + perms := APIPermissions{key: []string{"read_all"}} + require.NoError(t, PermissionsAreValid(perms), "%s must validate", key) + + token := &APIToken{APIPermissions: perms} + req := httptest.NewRequest("GET", "/api/v2/time-entries", nil) + c := echo.New().NewContext(req, httptest.NewRecorder()) + assert.True(t, CanDoAPIRoute(c, token), "%s must authorise", key) + }) + } +} + // TestGetRouteDetail_V2Verbs verifies the v2 verb mapping: POST→create, // PUT/PATCH→update. v1 inverts POST and PUT so we need a separate mapping // path.