Skip to content

Commit d8f959a

Browse files
aslilacaslilac
committed
fix: don't allow "new" or "create" as url-friendly names
1 parent d0b2f61 commit d8f959a

File tree

6 files changed

+76
-63
lines changed

6 files changed

+76
-63
lines changed

‎coderd/httpapi/httpapi.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func init() {
4646
valid := NameValid(str)
4747
return valid == nil
4848
}
49-
for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name", "oauth2_app_name"} {
49+
for _, tag := range []string{"username", "organization_name", "template_name", "group_name", "workspace_name", "oauth2_app_name"} {
5050
err := Validate.RegisterValidation(tag, nameValidator)
5151
if err != nil {
5252
panic(err)
@@ -62,7 +62,7 @@ func init() {
6262
valid := DisplayNameValid(str)
6363
return valid == nil
6464
}
65-
for _, displayNameTag := range []string{"organization_display_name", "template_display_name"} {
65+
for _, displayNameTag := range []string{"organization_display_name", "template_display_name", "group_display_name"} {
6666
err := Validate.RegisterValidation(displayNameTag, displayNameValidator)
6767
if err != nil {
6868
panic(err)

‎coderd/httpapi/name.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ func NameValid(str string) error {
4646
if len(str) < 1 {
4747
return xerrors.New("must be >= 1 character")
4848
}
49+
if str == "new" || str == "create" {
50+
return xerrors.Errorf("cannot use %q as a name", str)
51+
}
4952
matched := UsernameValidRegex.MatchString(str)
5053
if !matched {
5154
return xerrors.New("must be alphanumeric with hyphens")

‎coderd/templates_test.go

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ func TestTemplate(t *testing.T) {
3737
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
3838
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
3939

40-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
41-
defer cancel()
40+
ctx := testutil.Context(t, testutil.WaitLong)
4241

4342
_, err := client.Template(ctx, template.ID)
4443
require.NoError(t, err)
@@ -63,8 +62,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
6362
})
6463
assert.Equal(t, (3 * time.Hour).Milliseconds(), expected.ActivityBumpMillis)
6564

66-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
67-
defer cancel()
65+
ctx := testutil.Context(t, testutil.WaitLong)
6866

6967
got, err := user.Template(ctx, expected.ID)
7068
require.NoError(t, err)
@@ -86,8 +84,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
8684
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
8785
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
8886

89-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
90-
defer cancel()
87+
ctx := testutil.Context(t, testutil.WaitLong)
9188

9289
_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
9390
Name: template.Name,
@@ -98,15 +95,30 @@ func TestPostTemplateByOrganization(t *testing.T) {
9895
require.Equal(t, http.StatusConflict, apiErr.StatusCode())
9996
})
10097

101-
t.Run("DefaultTTLTooLow", func(t *testing.T) {
98+
t.Run("ReservedName", func(t *testing.T) {
10299
t.Parallel()
103100
client := coderdtest.New(t, nil)
104101
user := coderdtest.CreateFirstUser(t, client)
105102
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
106103

107-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
108-
defer cancel()
104+
ctx := testutil.Context(t, testutil.WaitShort)
109105

106+
_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
107+
Name: "new",
108+
VersionID: version.ID,
109+
})
110+
var apiErr *codersdk.Error
111+
require.ErrorAs(t, err, &apiErr)
112+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
113+
})
114+
115+
t.Run("DefaultTTLTooLow", func(t *testing.T) {
116+
t.Parallel()
117+
client := coderdtest.New(t, nil)
118+
user := coderdtest.CreateFirstUser(t, client)
119+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
120+
121+
ctx := testutil.Context(t, testutil.WaitLong)
110122
_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
111123
Name: "testing",
112124
VersionID: version.ID,
@@ -124,9 +136,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
124136
user := coderdtest.CreateFirstUser(t, client)
125137
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
126138

127-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
128-
defer cancel()
129-
139+
ctx := testutil.Context(t, testutil.WaitLong)
130140
got, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
131141
Name: "testing",
132142
VersionID: version.ID,
@@ -143,15 +153,13 @@ func TestPostTemplateByOrganization(t *testing.T) {
143153
owner := coderdtest.CreateFirstUser(t, client)
144154
user, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
145155
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
146-
147156
expected := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
148157
request.DisableEveryoneGroupAccess = true
149158
})
150159

151-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
152-
defer cancel()
153-
160+
ctx := testutil.Context(t, testutil.WaitLong)
154161
_, err := user.Template(ctx, expected.ID)
162+
155163
var apiErr *codersdk.Error
156164
require.ErrorAs(t, err, &apiErr)
157165
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
@@ -161,9 +169,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
161169
t.Parallel()
162170
client := coderdtest.New(t, nil)
163171

164-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
165-
defer cancel()
166-
172+
ctx := testutil.Context(t, testutil.WaitLong)
167173
_, err := client.CreateTemplate(ctx, uuid.New(), codersdk.CreateTemplateRequest{
168174
Name: "test",
169175
VersionID: uuid.New(),
@@ -241,8 +247,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
241247
client := coderdtest.New(t, nil)
242248
user := coderdtest.CreateFirstUser(t, client)
243249

244-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
245-
defer cancel()
250+
ctx := testutil.Context(t, testutil.WaitLong)
246251

247252
_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
248253
Name: "test",
@@ -398,8 +403,7 @@ func TestTemplatesByOrganization(t *testing.T) {
398403
client := coderdtest.New(t, nil)
399404
user := coderdtest.CreateFirstUser(t, client)
400405

401-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
402-
defer cancel()
406+
ctx := testutil.Context(t, testutil.WaitLong)
403407

404408
templates, err := client.TemplatesByOrganization(ctx, user.OrganizationID)
405409
require.NoError(t, err)
@@ -414,8 +418,7 @@ func TestTemplatesByOrganization(t *testing.T) {
414418
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
415419
coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
416420

417-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
418-
defer cancel()
421+
ctx := testutil.Context(t, testutil.WaitLong)
419422

420423
templates, err := client.TemplatesByOrganization(ctx, user.OrganizationID)
421424
require.NoError(t, err)
@@ -430,8 +433,7 @@ func TestTemplatesByOrganization(t *testing.T) {
430433
coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
431434
coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID)
432435

433-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
434-
defer cancel()
436+
ctx := testutil.Context(t, testutil.WaitLong)
435437

436438
templates, err := client.TemplatesByOrganization(ctx, user.OrganizationID)
437439
require.NoError(t, err)
@@ -446,8 +448,7 @@ func TestTemplateByOrganizationAndName(t *testing.T) {
446448
client := coderdtest.New(t, nil)
447449
user := coderdtest.CreateFirstUser(t, client)
448450

449-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
450-
defer cancel()
451+
ctx := testutil.Context(t, testutil.WaitLong)
451452

452453
_, err := client.TemplateByName(ctx, user.OrganizationID, "something")
453454
var apiErr *codersdk.Error
@@ -462,8 +463,7 @@ func TestTemplateByOrganizationAndName(t *testing.T) {
462463
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
463464
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
464465

465-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
466-
defer cancel()
466+
ctx := testutil.Context(t, testutil.WaitLong)
467467

468468
_, err := client.TemplateByName(ctx, user.OrganizationID, template.Name)
469469
require.NoError(t, err)
@@ -497,8 +497,7 @@ func TestPatchTemplateMeta(t *testing.T) {
497497
// updatedAt is too close together.
498498
time.Sleep(time.Millisecond * 5)
499499

500-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
501-
defer cancel()
500+
ctx := testutil.Context(t, testutil.WaitLong)
502501

503502
updated, err := client.UpdateTemplateMeta(ctx, template.ID, req)
504503
require.NoError(t, err)
@@ -542,8 +541,7 @@ func TestPatchTemplateMeta(t *testing.T) {
542541
DeprecationMessage: ptr.Ref("APGL cannot deprecate"),
543542
}
544543

545-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
546-
defer cancel()
544+
ctx := testutil.Context(t, testutil.WaitLong)
547545

548546
updated, err := client.UpdateTemplateMeta(ctx, template.ID, req)
549547
require.NoError(t, err)
@@ -566,8 +564,8 @@ func TestPatchTemplateMeta(t *testing.T) {
566564
// updatedAt is too close together.
567565
time.Sleep(time.Millisecond * 5)
568566

569-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
570-
defer cancel()
567+
ctx := testutil.Context(t, testutil.WaitLong)
568+
571569
// nolint:gocritic // Setting up unit test data
572570
err := db.UpdateTemplateAccessControlByID(dbauthz.As(ctx, coderdtest.AuthzUserSubject(tplAdmin, user.OrganizationID)), database.UpdateTemplateAccessControlByIDParams{
573571
ID: template.ID,
@@ -607,8 +605,7 @@ func TestPatchTemplateMeta(t *testing.T) {
607605
MaxPortShareLevel: &level,
608606
}
609607

610-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
611-
defer cancel()
608+
ctx := testutil.Context(t, testutil.WaitLong)
612609

613610
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
614611
// AGPL cannot change max port sharing level
@@ -643,8 +640,7 @@ func TestPatchTemplateMeta(t *testing.T) {
643640
// We're too fast! Sleep so we can be sure that updatedAt is greater
644641
time.Sleep(time.Millisecond * 5)
645642

646-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
647-
defer cancel()
643+
ctx := testutil.Context(t, testutil.WaitLong)
648644

649645
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
650646
require.NoError(t, err)
@@ -675,8 +671,7 @@ func TestPatchTemplateMeta(t *testing.T) {
675671
DefaultTTLMillis: -1,
676672
}
677673

678-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
679-
defer cancel()
674+
ctx := testutil.Context(t, testutil.WaitLong)
680675

681676
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
682677
require.ErrorContains(t, err, "default_ttl_ms: Must be a positive integer")
@@ -886,8 +881,7 @@ func TestPatchTemplateMeta(t *testing.T) {
886881
ctr.DefaultTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
887882
})
888883

889-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
890-
defer cancel()
884+
ctx := testutil.Context(t, testutil.WaitLong)
891885

892886
req := codersdk.UpdateTemplateMeta{
893887
Name: template.Name,
@@ -921,8 +915,7 @@ func TestPatchTemplateMeta(t *testing.T) {
921915
ctr.DefaultTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
922916
})
923917

924-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
925-
defer cancel()
918+
ctx := testutil.Context(t, testutil.WaitLong)
926919

927920
req := codersdk.UpdateTemplateMeta{
928921
DefaultTTLMillis: -int64(time.Hour),
@@ -956,8 +949,7 @@ func TestPatchTemplateMeta(t *testing.T) {
956949
Icon: "",
957950
}
958951

959-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
960-
defer cancel()
952+
ctx := testutil.Context(t, testutil.WaitLong)
961953

962954
updated, err := client.UpdateTemplateMeta(ctx, template.ID, req)
963955
require.NoError(t, err)
@@ -1164,8 +1156,7 @@ func TestDeleteTemplate(t *testing.T) {
11641156
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
11651157
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
11661158

1167-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1168-
defer cancel()
1159+
ctx := testutil.Context(t, testutil.WaitLong)
11691160

11701161
err := client.DeleteTemplate(ctx, template.ID)
11711162
require.NoError(t, err)
@@ -1183,8 +1174,7 @@ func TestDeleteTemplate(t *testing.T) {
11831174
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
11841175
coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
11851176

1186-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1187-
defer cancel()
1177+
ctx := testutil.Context(t, testutil.WaitLong)
11881178

11891179
err := client.DeleteTemplate(ctx, template.ID)
11901180
var apiErr *codersdk.Error

‎codersdk/groups.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@ const (
1818
)
1919

2020
type CreateGroupRequest struct {
21-
Name string `json:"name"`
22-
DisplayName string `json:"display_name"`
21+
Name string `json:"name" validate:"required,group_name"`
22+
DisplayName string `json:"display_name" validate:"omitempty,group_display_name"`
2323
AvatarURL string `json:"avatar_url"`
2424
QuotaAllowance int `json:"quota_allowance"`
2525
}
2626

2727
type Group struct {
2828
ID uuid.UUID `json:"id" format:"uuid"`
29-
Name string `json:"name"`
30-
DisplayName string `json:"display_name"`
29+
Name string `json:"name" validate:"required,group_name"`
30+
DisplayName string `json:"display_name" validate:"required,group_display_name"`
3131
OrganizationID uuid.UUID `json:"organization_id" format:"uuid"`
3232
Members []ReducedUser `json:"members"`
3333
AvatarURL string `json:"avatar_url"`
@@ -111,8 +111,8 @@ func (c *Client) Group(ctx context.Context, group uuid.UUID) (Group, error) {
111111
type PatchGroupRequest struct {
112112
AddUsers []string `json:"add_users"`
113113
RemoveUsers []string `json:"remove_users"`
114-
Name string `json:"name"`
115-
DisplayName *string `json:"display_name"`
114+
Name string `json:"name" validate:"omitempty,group_name"`
115+
DisplayName *string `json:"display_name" validate:"omitempty,group_display_name"`
116116
AvatarURL *string `json:"avatar_url"`
117117
QuotaAllowance *int `json:"quota_allowance"`
118118
}

‎codersdk/organizations.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ func ProvisionerTypeValid[T ProvisionerType | string](pt T) error {
4141
// Organization is the JSON representation of a Coder organization.
4242
type Organization struct {
4343
ID uuid.UUID `table:"id" json:"id" validate:"required" format:"uuid"`
44-
Name string `table:"name,default_sort" json:"name" validate:"required,username"`
45-
DisplayName string `table:"display_name" json:"display_name" validate:"required"`
44+
Name string `table:"name,default_sort" json:"name"`
45+
DisplayName string `table:"display_name" json:"display_name"`
4646
Description string `table:"description" json:"description"`
4747
CreatedAt time.Time `table:"created_at" json:"created_at" validate:"required" format:"date-time"`
4848
UpdatedAt time.Time `table:"updated_at" json:"updated_at" validate:"required" format:"date-time"`

‎enterprise/coderd/groups_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,26 @@ func TestCreateGroup(t *testing.T) {
101101
require.Equal(t, http.StatusConflict, cerr.StatusCode())
102102
})
103103

104+
t.Run("ReservedName", func(t *testing.T) {
105+
t.Parallel()
106+
107+
client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{
108+
Features: license.Features{
109+
codersdk.FeatureTemplateRBAC: 1,
110+
},
111+
}})
112+
userAdminClient, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleUserAdmin())
113+
ctx := testutil.Context(t, testutil.WaitLong)
114+
_, err := userAdminClient.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
115+
Name: "new",
116+
})
117+
118+
require.Error(t, err)
119+
var apiErr *codersdk.Error
120+
require.ErrorAs(t, err, &apiErr)
121+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
122+
})
123+
104124
t.Run("allUsers", func(t *testing.T) {
105125
t.Parallel()
106126

0 commit comments

Comments
 (0)