fix(api): export api-token permission groups in snake_case

The api-token permission group key is derived from the route slug. Every
group is snake_case except "time-entries", whose URL slug carries a hyphen.
The frontend snake_cases request payloads, rewriting that group key to
"time_entries", which the backend then rejected — so a token granted the
Time Entries scope could not be saved.

Canonicalise group and path-segment names to snake_case where they are
derived, and normalise the group key on token validation and authorisation
so any token stored under the old hyphenated key keeps resolving. No data
migration is needed: the v2 time-entries resource has never shipped in a
release.
This commit is contained in:
kolaente 2026-06-27 15:43:12 +02:00 committed by kolaente
parent e25ca7ab9a
commit 9e880e98a5
2 changed files with 47 additions and 14 deletions

View File

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

View File

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