Skip to content

Commit fd23c4d

Browse files
committed
feat: add structured JSON format for APIAllowListTarget
APIAllowListTarget now marshals to/from structured JSON objects `{"type":"workspace","id":"<uuid>"}` instead of colon-delimited strings. This improves type safety and frontend ergonomics. Changes: - Modified UnmarshalJSON to parse structured object representation - Extracted setValues helper for shared validation logic - Preserved UnmarshalText for backward compatibility with CLI flags and database helpers - Added MarshalJSON/UnmarshalJSON to x/wildcard/Value for proper JSON handling of wildcard values - Updated frontend mock data to use structured format - Added test coverage for both text and object unmarshaling - Added resource ID matchers to regosql converters for template and workspace ID filtering
1 parent 2155bbf commit fd23c4d

13 files changed

Lines changed: 290 additions & 45 deletions

File tree

coderd/apidoc/docs.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/apidoc/swagger.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/apikey.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ func (api *API) apiKeyByID(rw http.ResponseWriter, r *http.Request) {
215215
return
216216
}
217217

218-
httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(key))
218+
sdkKey := convertAPIKey(key)
219+
api.populateAllowListDisplayNames(ctx, sdkKey.AllowList)
220+
221+
httpapi.Write(ctx, rw, http.StatusOK, sdkKey)
219222
}
220223

221224
// @Summary Get API key by token name
@@ -250,7 +253,10 @@ func (api *API) apiKeyByName(rw http.ResponseWriter, r *http.Request) {
250253
return
251254
}
252255

253-
httpapi.Write(ctx, rw, http.StatusOK, convertAPIKey(token))
256+
sdkKey := convertAPIKey(token)
257+
api.populateAllowListDisplayNames(ctx, sdkKey.AllowList)
258+
259+
httpapi.Write(ctx, rw, http.StatusOK, sdkKey)
254260
}
255261

256262
// @Summary Update token API key
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package coderd
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/require"
9+
"go.uber.org/mock/gomock"
10+
11+
"github.com/coder/coder/v2/coderd/database"
12+
"github.com/coder/coder/v2/coderd/database/dbmock"
13+
"github.com/coder/coder/v2/coderd/rbac/policy"
14+
"github.com/coder/coder/v2/codersdk"
15+
)
16+
17+
func TestConvertAPIKeyAllowListDisplayName(t *testing.T) {
18+
t.Parallel()
19+
20+
ctrl := gomock.NewController(t)
21+
t.Cleanup(ctrl.Finish)
22+
23+
db := dbmock.NewMockStore(ctrl)
24+
templateID := uuid.New()
25+
26+
db.EXPECT().
27+
GetTemplateByID(gomock.Any(), templateID).
28+
Return(database.Template{
29+
ID: templateID,
30+
Name: "infra-template",
31+
DisplayName: "Infra Template",
32+
}, nil).
33+
Times(1)
34+
35+
key := database.APIKey{
36+
ID: "key-1",
37+
UserID: uuid.New(),
38+
LastUsed: time.Now(),
39+
ExpiresAt: time.Now().Add(time.Hour),
40+
CreatedAt: time.Now(),
41+
UpdatedAt: time.Now(),
42+
LoginType: database.LoginTypeToken,
43+
LifetimeSeconds: int64(time.Hour.Seconds()),
44+
TokenName: "cli",
45+
Scopes: database.APIKeyScopes{database.ApiKeyScopeCoderAll},
46+
AllowList: database.AllowList{
47+
{Type: string(codersdk.ResourceTemplate), ID: templateID.String()},
48+
},
49+
}
50+
51+
result := convertAPIKey(key)
52+
53+
require.Len(t, result.AllowList, 1)
54+
require.Equal(t, codersdk.ResourceTemplate, result.AllowList[0].Type)
55+
require.Equal(t, templateID.String(), result.AllowList[0].ID)
56+
require.Equal(t, "Infra Template", result.AllowList[0].DisplayName)
57+
}
58+
59+
func TestConvertAPIKeyAllowListDisplayNameWildcard(t *testing.T) {
60+
t.Parallel()
61+
62+
ctrl := gomock.NewController(t)
63+
t.Cleanup(ctrl.Finish)
64+
65+
key := database.APIKey{
66+
ID: "key-2",
67+
AllowList: database.AllowList{
68+
{Type: string(codersdk.ResourceWildcard), ID: policy.WildcardSymbol},
69+
},
70+
}
71+
72+
result := convertAPIKey(key)
73+
74+
require.Len(t, result.AllowList, 1)
75+
require.Equal(t, codersdk.ResourceWildcard, result.AllowList[0].Type)
76+
require.Equal(t, "*", result.AllowList[0].ID)
77+
require.Empty(t, result.AllowList[0].DisplayName)
78+
}

coderd/apikey_scopes_validation_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ func TestTokenCreation_AllowListValidation(t *testing.T) {
9393
// Invalid resource type should be rejected.
9494
_, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{
9595
Scopes: []codersdk.APIKeyScope{codersdk.APIKeyScopeWorkspaceRead},
96-
AllowList: []codersdk.APIAllowListTarget{
97-
{Type: codersdk.RBACResource("unknown"), ID: uuid.New().String()},
98-
},
96+
AllowList: []codersdk.APIAllowListTarget{{
97+
Type: codersdk.RBACResource("unknown"),
98+
ID: uuid.New().String(),
99+
}},
99100
})
100101
require.Error(t, err)
101102

coderd/rbac/regosql/compile_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,14 @@ func TestRegoQueries(t *testing.T) {
217217
" OR (workspaces.group_acl#>array['96c55a0e-73b4-44fc-abac-70d53c35c04c', 'permissions'] ? '*'))",
218218
VariableConverter: regosql.WorkspaceConverter(),
219219
},
220+
{
221+
Name: "WorkspaceIDMatcher",
222+
Queries: []string{
223+
`input.object.id = "a8d0f8ce-6a01-4d0d-ab1d-1d546958feae"`,
224+
},
225+
ExpectedSQL: p("workspaces.id :: text = 'a8d0f8ce-6a01-4d0d-ab1d-1d546958feae'"),
226+
VariableConverter: regosql.WorkspaceConverter(),
227+
},
220228
{
221229
Name: "NoACLConfig",
222230
Queries: []string{
@@ -262,6 +270,14 @@ neq(input.object.owner, "");
262270
p("false")),
263271
VariableConverter: regosql.TemplateConverter(),
264272
},
273+
{
274+
Name: "TemplateIDMatcher",
275+
Queries: []string{
276+
`input.object.id = "a829cb9d-7c5b-4c3b-bf78-053827a56e58"`,
277+
},
278+
ExpectedSQL: p("t.id :: text = 'a829cb9d-7c5b-4c3b-bf78-053827a56e58'"),
279+
VariableConverter: regosql.TemplateConverter(),
280+
},
265281
{
266282
Name: "UserNoOrgOwner",
267283
Queries: []string{

coderd/rbac/regosql/configs.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func userACLMatcher(m sqltypes.VariableMatcher) ACLMappingVar {
2424

2525
func TemplateConverter() *sqltypes.VariableConverter {
2626
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
27-
resourceIDMatcher(),
27+
sqltypes.StringVarMatcher("t.id :: text", []string{"input", "object", "id"}),
2828
sqltypes.StringVarMatcher("t.organization_id :: text", []string{"input", "object", "org_owner"}),
2929
// Templates have no user owner, only owner by an organization.
3030
sqltypes.AlwaysFalse(userOwnerMatcher()),
@@ -38,7 +38,7 @@ func TemplateConverter() *sqltypes.VariableConverter {
3838

3939
func WorkspaceConverter() *sqltypes.VariableConverter {
4040
matcher := sqltypes.NewVariableConverter().RegisterMatcher(
41-
resourceIDMatcher(),
41+
sqltypes.StringVarMatcher("workspaces.id :: text", []string{"input", "object", "id"}),
4242
sqltypes.StringVarMatcher("workspaces.organization_id :: text", []string{"input", "object", "org_owner"}),
4343
userOwnerMatcher(),
4444
)

coderd/users.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,3 +1616,65 @@ func convertAPIKey(k database.APIKey) codersdk.APIKey {
16161616
AllowList: allowList,
16171617
}
16181618
}
1619+
1620+
func (api *API) populateAllowListDisplayNames(ctx context.Context, allowList []codersdk.APIAllowListTarget) {
1621+
if len(allowList) == 0 {
1622+
return
1623+
}
1624+
1625+
cache := make(map[string]string, len(allowList))
1626+
for i := range allowList {
1627+
target := allowList[i]
1628+
if target.Type == codersdk.ResourceWildcard || target.ID == policy.WildcardSymbol {
1629+
continue
1630+
}
1631+
1632+
key := target.String()
1633+
name, ok := cache[key]
1634+
if !ok {
1635+
name, ok = api.allowListDisplayName(ctx, target.Type, target.ID)
1636+
if !ok {
1637+
cache[key] = ""
1638+
continue
1639+
}
1640+
cache[key] = name
1641+
}
1642+
if name != "" {
1643+
allowList[i].DisplayName = name
1644+
}
1645+
}
1646+
}
1647+
1648+
func (api *API) allowListDisplayName(ctx context.Context, resource codersdk.RBACResource, rawID string) (string, bool) {
1649+
if api == nil || api.Options == nil || api.Database == nil {
1650+
return "", false
1651+
}
1652+
if rawID == "" || rawID == policy.WildcardSymbol {
1653+
return "", false
1654+
}
1655+
1656+
id, err := uuid.Parse(rawID)
1657+
if err != nil {
1658+
return "", false
1659+
}
1660+
1661+
switch resource {
1662+
case codersdk.ResourceWorkspace:
1663+
workspace, err := api.Database.GetWorkspaceByID(ctx, id)
1664+
if err != nil {
1665+
return "", false
1666+
}
1667+
return workspace.Name, true
1668+
case codersdk.ResourceTemplate:
1669+
template, err := api.Database.GetTemplateByID(ctx, id)
1670+
if err != nil {
1671+
return "", false
1672+
}
1673+
if template.DisplayName != "" {
1674+
return template.DisplayName, true
1675+
}
1676+
return template.Name, true
1677+
default:
1678+
return "", false
1679+
}
1680+
}

codersdk/allowlist.go

Lines changed: 76 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ import (
1010
"github.com/coder/coder/v2/coderd/rbac/policy"
1111
)
1212

13-
// APIAllowListTarget represents a single allow-list entry using the canonical
14-
// string form "<resource_type>:<id>". The wildcard symbol "*" is treated as a
15-
// permissive match for either side.
13+
// APIAllowListTarget represents a single allow-list entry. The canonical string
14+
// form is "<resource_type>:<id>" with "*" acting as a wildcard for either
15+
// component. Structured JSON callers should use the object form
16+
// `{ "type": "workspace", "id": "<uuid>" }`. Optionally, servers may attach a
17+
// DisplayName to provide a human-friendly label; clients must ignore the field
18+
// when submitting data.
1619
type APIAllowListTarget struct {
17-
Type RBACResource `json:"type"`
18-
ID string `json:"id"`
20+
Type RBACResource `json:"type"`
21+
ID string `json:"id"`
22+
DisplayName string `json:"display_name,omitempty"`
1923
}
2024

2125
func AllowAllTarget() APIAllowListTarget {
@@ -30,51 +34,92 @@ func AllowResourceTarget(r RBACResource, id uuid.UUID) APIAllowListTarget {
3034
return APIAllowListTarget{Type: r, ID: id.String()}
3135
}
3236

33-
// String returns the canonical string representation "<type>:<id>" with "*" wildcards.
37+
// String returns the canonical string representation "<type>:<id>" with wildcards preserved.
3438
func (t APIAllowListTarget) String() string {
3539
return string(t.Type) + ":" + t.ID
3640
}
3741

38-
// MarshalJSON encodes as a JSON string: "<type>:<id>".
39-
func (t APIAllowListTarget) MarshalJSON() ([]byte, error) {
40-
return json.Marshal(t.String())
41-
}
42-
43-
// UnmarshalJSON decodes from a JSON string: "<type>:<id>".
42+
// UnmarshalJSON accepts either the structured object representation
43+
// `{ "type": "workspace", "id": "<uuid>" }` or the legacy string form "workspace:<uuid>".
4444
func (t *APIAllowListTarget) UnmarshalJSON(b []byte) error {
45-
var s string
46-
if err := json.Unmarshal(b, &s); err != nil {
47-
return err
45+
if len(b) == 0 {
46+
return xerrors.New("empty allow_list entry")
47+
}
48+
49+
// Attempt to decode the structured object form first.
50+
var obj struct {
51+
Type string `json:"type"`
52+
ID string `json:"id"`
53+
DisplayName string `json:"display_name"`
4854
}
49-
parts := strings.SplitN(strings.TrimSpace(s), ":", 2)
55+
if err := json.Unmarshal(b, &obj); err == nil {
56+
if obj.Type != "" || obj.ID != "" {
57+
if obj.Type == "" || obj.ID == "" {
58+
return xerrors.New("allow_list entry must include both type and id")
59+
}
60+
if err := t.setValues(obj.Type, obj.ID); err != nil {
61+
return err
62+
}
63+
// Ignore object.DisplayName on input to keep backend validation strict.
64+
return nil
65+
}
66+
}
67+
68+
var legacy string
69+
if err := json.Unmarshal(b, &legacy); err != nil {
70+
return xerrors.New("invalid allow_list entry: expected object with type/id or string")
71+
}
72+
parts := strings.SplitN(strings.TrimSpace(legacy), ":", 2)
5073
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
51-
return xerrors.Errorf("invalid allow_list entry %q: want <type>:<id>", s)
74+
return xerrors.Errorf("invalid allow_list entry %q: want <type>:<id>", legacy)
5275
}
76+
return t.setValues(parts[0], parts[1])
77+
}
5378

54-
resource, id := RBACResource(parts[0]), parts[1]
79+
func (t *APIAllowListTarget) setValues(rawType, rawID string) error {
80+
rawType = strings.TrimSpace(rawType)
81+
rawID = strings.TrimSpace(rawID)
5582

56-
// Type
57-
if resource != ResourceWildcard {
58-
if _, ok := policy.RBACPermissions[string(resource)]; !ok {
59-
return xerrors.Errorf("unknown resource type %q", resource)
60-
}
83+
if rawType == "" || rawID == "" {
84+
return xerrors.New("allow_list entry must include non-empty type and id")
6185
}
62-
t.Type = resource
6386

64-
// ID
65-
if id != policy.WildcardSymbol {
66-
if _, err := uuid.Parse(id); err != nil {
67-
return xerrors.Errorf("invalid %s ID (must be UUID): %q", resource, id)
87+
if rawType == policy.WildcardSymbol {
88+
t.Type = ResourceWildcard
89+
} else {
90+
if _, ok := policy.RBACPermissions[rawType]; !ok {
91+
return xerrors.Errorf("unknown resource type %q", rawType)
6892
}
93+
t.Type = RBACResource(rawType)
94+
}
95+
96+
if rawID == policy.WildcardSymbol {
97+
t.ID = policy.WildcardSymbol
98+
return nil
99+
}
100+
101+
if _, err := uuid.Parse(rawID); err != nil {
102+
return xerrors.Errorf("invalid %s ID (must be UUID): %q", rawType, rawID)
69103
}
70-
t.ID = id
104+
t.ID = rawID
71105
return nil
72106
}
73107

74-
// Implement encoding.TextMarshaler/Unmarshaler for broader compatibility
108+
// MarshalJSON ensures encoding/json uses the structured representation instead
109+
// of the legacy colon-delimited string form.
110+
func (t APIAllowListTarget) MarshalJSON() ([]byte, error) {
111+
type alias APIAllowListTarget
112+
return json.Marshal(alias(t))
113+
}
75114

115+
// Implement encoding.TextMarshaler/Unmarshaler for broader compatibility.
76116
func (t APIAllowListTarget) MarshalText() ([]byte, error) { return []byte(t.String()), nil }
77117

78118
func (t *APIAllowListTarget) UnmarshalText(b []byte) error {
79-
return t.UnmarshalJSON([]byte("\"" + string(b) + "\""))
119+
strTarget := strings.TrimSpace(string(b))
120+
parts := strings.SplitN(strTarget, ":", 2)
121+
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
122+
return xerrors.Errorf("invalid allow_list entry %q: want <type>:<id>", strTarget)
123+
}
124+
return t.setValues(parts[0], parts[1])
80125
}

0 commit comments

Comments
 (0)