Skip to content

Commit f56fcf9

Browse files
sreyasreya
committed
add patch endpoint tests
1 parent d533a16 commit f56fcf9

File tree

6 files changed

+261
-38
lines changed

6 files changed

+261
-38
lines changed

‎coderd/database/modelmethods.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ func templateRoleToActions(t TemplateRole) []rbac.Action {
4646
case TemplateRoleWrite:
4747
return []rbac.Action{rbac.ActionRead, rbac.ActionUpdate}
4848
case TemplateRoleAdmin:
49-
return []rbac.Action{rbac.WildcardSymbol}
49+
// TODO: Why does rbac.Wildcard not work here?
50+
return []rbac.Action{rbac.ActionRead, rbac.ActionUpdate, rbac.ActionCreate, rbac.ActionDelete}
5051
}
5152
return nil
5253
}

‎coderd/templates.go

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,9 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
457457

458458
// Only users who are able to create templates (aka template admins)
459459
// are able to control user permissions.
460-
// TODO: It'd be nice to also assert delete since a template admin
461-
// should be able to do both.
460+
// TODO: It might be cleaner to control template perms access
461+
// via a separate RBAC resource, and restrict all actions to the template
462+
// admin role.
462463
if len(req.UserPerms) > 0 && !api.Authorize(r, rbac.ActionCreate, template) {
463464
httpapi.ResourceNotFound(rw)
464465
return
@@ -475,9 +476,23 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
475476
validErrs = append(validErrs, codersdk.ValidationError{Field: "max_ttl_ms", Detail: "Cannot be greater than " + maxTTLDefault.String()})
476477
}
477478

478-
for _, v := range req.UserPerms {
479+
for k, v := range req.UserPerms {
479480
if err := validateTemplateRole(v); err != nil {
480481
validErrs = append(validErrs, codersdk.ValidationError{Field: "user_perms", Detail: err.Error()})
482+
continue
483+
}
484+
485+
userID, err := uuid.Parse(k)
486+
if err != nil {
487+
validErrs = append(validErrs, codersdk.ValidationError{Field: "user_perms", Detail: "User ID " + k + "must be a valid UUID."})
488+
continue
489+
}
490+
491+
// This could get slow if we get a ton of user perm updates.
492+
_, err = api.Database.GetUserByID(r.Context(), userID)
493+
if err != nil {
494+
validErrs = append(validErrs, codersdk.ValidationError{Field: "user_perms", Detail: fmt.Sprintf("Failed to find user with ID %q: %v", k, err.Error())})
495+
continue
481496
}
482497
}
483498

@@ -509,7 +524,8 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
509524
req.Description == template.Description &&
510525
req.Icon == template.Icon &&
511526
req.MaxTTLMillis == time.Duration(template.MaxTtl).Milliseconds() &&
512-
req.MinAutostartIntervalMillis == time.Duration(template.MinAutostartInterval).Milliseconds() {
527+
req.MinAutostartIntervalMillis == time.Duration(template.MinAutostartInterval).Milliseconds() &&
528+
len(req.UserPerms) == 0 {
513529
return nil
514530
}
515531

@@ -530,23 +546,12 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
530546
minAutostartInterval = time.Duration(template.MinAutostartInterval)
531547
}
532548

533-
updated, err = tx.UpdateTemplateMetaByID(r.Context(), database.UpdateTemplateMetaByIDParams{
534-
ID: template.ID,
535-
UpdatedAt: database.Now(),
536-
Name: name,
537-
Description: desc,
538-
Icon: icon,
539-
MaxTtl: int64(maxTTL),
540-
MinAutostartInterval: int64(minAutostartInterval),
541-
})
542-
if err != nil {
543-
return err
544-
}
545-
546549
if len(req.UserPerms) > 0 {
547550
userACL := template.UserACL()
548551
for k, v := range req.UserPerms {
549-
if len(v) == 0 {
552+
// A user with an empty string implies
553+
// deletion.
554+
if v == "" {
550555
delete(userACL, k)
551556
continue
552557
}
@@ -559,6 +564,19 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
559564
}
560565
}
561566

567+
updated, err = tx.UpdateTemplateMetaByID(r.Context(), database.UpdateTemplateMetaByIDParams{
568+
ID: template.ID,
569+
UpdatedAt: database.Now(),
570+
Name: name,
571+
Description: desc,
572+
Icon: icon,
573+
MaxTtl: int64(maxTTL),
574+
MinAutostartInterval: int64(minAutostartInterval),
575+
})
576+
if err != nil {
577+
return err
578+
}
579+
562580
return nil
563581
})
564582
if err != nil {
@@ -839,22 +857,9 @@ func convertSDKTemplateRole(role codersdk.TemplateRole) database.TemplateRole {
839857
return ""
840858
}
841859

842-
func templateRoleToActions(role codersdk.TemplateRole) []string {
843-
switch role {
844-
case codersdk.TemplateRoleAdmin:
845-
return []string{rbac.WildcardSymbol}
846-
case codersdk.TemplateRoleWrite:
847-
return []string{rbac.ActionRead, rbac.ActionUpdate}
848-
case codersdk.TemplateRoleRead:
849-
return []string{rbac.ActionRead}
850-
}
851-
852-
return nil
853-
}
854-
855860
func validateTemplateRole(role codersdk.TemplateRole) error {
856861
dbRole := convertSDKTemplateRole(role)
857-
if dbRole == "" {
862+
if dbRole == "" && role != codersdk.TemplateRoleDeleted {
858863
return xerrors.Errorf("role %q is not a valid Template role", role)
859864
}
860865

‎coderd/templates_test.go

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,217 @@ func TestPatchTemplateMeta(t *testing.T) {
519519
require.NoError(t, err)
520520
assert.Equal(t, updated.Icon, "")
521521
})
522+
523+
t.Run("UserPerms", func(t *testing.T) {
524+
t.Parallel()
525+
526+
t.Run("OK", func(t *testing.T) {
527+
t.Parallel()
528+
529+
client := coderdtest.New(t, nil)
530+
user := coderdtest.CreateFirstUser(t, client)
531+
_, user2 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
532+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
533+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
534+
req := codersdk.UpdateTemplateMeta{
535+
UserPerms: map[string]codersdk.TemplateRole{
536+
user2.ID.String(): codersdk.TemplateRoleRead,
537+
},
538+
}
539+
540+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
541+
defer cancel()
542+
543+
template, err := client.UpdateTemplateMeta(ctx, template.ID, req)
544+
require.NoError(t, err)
545+
546+
role, ok := template.UserRoles[user2.ID.String()]
547+
require.True(t, ok, "User not contained within user_roles map")
548+
require.Equal(t, codersdk.TemplateRoleRead, role)
549+
})
550+
551+
t.Run("DeleteUser", func(t *testing.T) {
552+
t.Parallel()
553+
554+
client := coderdtest.New(t, nil)
555+
user := coderdtest.CreateFirstUser(t, client)
556+
_, user2 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
557+
_, user3 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
558+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
559+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
560+
req := codersdk.UpdateTemplateMeta{
561+
UserPerms: map[string]codersdk.TemplateRole{
562+
user2.ID.String(): codersdk.TemplateRoleRead,
563+
user3.ID.String(): codersdk.TemplateRoleWrite,
564+
},
565+
}
566+
567+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
568+
defer cancel()
569+
570+
template, err := client.UpdateTemplateMeta(ctx, template.ID, req)
571+
require.NoError(t, err)
572+
573+
role, ok := template.UserRoles[user2.ID.String()]
574+
require.True(t, ok, "User not contained within user_roles map")
575+
require.Equal(t, codersdk.TemplateRoleRead, role)
576+
577+
role, ok = template.UserRoles[user3.ID.String()]
578+
require.True(t, ok, "User not contained within user_roles map")
579+
require.Equal(t, codersdk.TemplateRoleWrite, role)
580+
581+
req = codersdk.UpdateTemplateMeta{
582+
UserPerms: map[string]codersdk.TemplateRole{
583+
user2.ID.String(): codersdk.TemplateRoleAdmin,
584+
user3.ID.String(): codersdk.TemplateRoleDeleted,
585+
},
586+
}
587+
588+
template, err = client.UpdateTemplateMeta(ctx, template.ID, req)
589+
require.NoError(t, err)
590+
591+
role, ok = template.UserRoles[user2.ID.String()]
592+
require.True(t, ok, "User not contained within user_roles map")
593+
require.Equal(t, codersdk.TemplateRoleAdmin, role)
594+
595+
_, ok = template.UserRoles[user3.ID.String()]
596+
require.False(t, ok, "User should have been deleted from user_roles map")
597+
})
598+
599+
t.Run("InvalidUUID", func(t *testing.T) {
600+
t.Parallel()
601+
602+
client := coderdtest.New(t, nil)
603+
user := coderdtest.CreateFirstUser(t, client)
604+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
605+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
606+
req := codersdk.UpdateTemplateMeta{
607+
UserPerms: map[string]codersdk.TemplateRole{
608+
"hi": "admin",
609+
},
610+
}
611+
612+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
613+
defer cancel()
614+
615+
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
616+
require.Error(t, err)
617+
cerr, _ := codersdk.AsError(err)
618+
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
619+
})
620+
621+
t.Run("InvalidUser", func(t *testing.T) {
622+
t.Parallel()
623+
624+
client := coderdtest.New(t, nil)
625+
user := coderdtest.CreateFirstUser(t, client)
626+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
627+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
628+
req := codersdk.UpdateTemplateMeta{
629+
UserPerms: map[string]codersdk.TemplateRole{
630+
uuid.NewString(): "admin",
631+
},
632+
}
633+
634+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
635+
defer cancel()
636+
637+
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
638+
require.Error(t, err)
639+
cerr, _ := codersdk.AsError(err)
640+
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
641+
})
642+
643+
t.Run("InvalidRole", func(t *testing.T) {
644+
t.Parallel()
645+
646+
client := coderdtest.New(t, nil)
647+
user := coderdtest.CreateFirstUser(t, client)
648+
_, user2 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
649+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
650+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
651+
req := codersdk.UpdateTemplateMeta{
652+
UserPerms: map[string]codersdk.TemplateRole{
653+
user2.ID.String(): "updater",
654+
},
655+
}
656+
657+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
658+
defer cancel()
659+
660+
_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
661+
require.Error(t, err)
662+
cerr, _ := codersdk.AsError(err)
663+
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
664+
})
665+
666+
t.Run("RegularUserCannotUpdatePerms", func(t *testing.T) {
667+
t.Parallel()
668+
669+
client := coderdtest.New(t, nil)
670+
user := coderdtest.CreateFirstUser(t, client)
671+
client2, user2 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
672+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
673+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
674+
req := codersdk.UpdateTemplateMeta{
675+
UserPerms: map[string]codersdk.TemplateRole{
676+
user2.ID.String(): codersdk.TemplateRoleWrite,
677+
},
678+
}
679+
680+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
681+
defer cancel()
682+
683+
template, err := client.UpdateTemplateMeta(ctx, template.ID, req)
684+
require.NoError(t, err)
685+
686+
req = codersdk.UpdateTemplateMeta{
687+
UserPerms: map[string]codersdk.TemplateRole{
688+
user2.ID.String(): codersdk.TemplateRoleAdmin,
689+
},
690+
}
691+
692+
template, err = client2.UpdateTemplateMeta(ctx, template.ID, req)
693+
require.Error(t, err)
694+
cerr, _ := codersdk.AsError(err)
695+
require.Equal(t, http.StatusNotFound, cerr.StatusCode())
696+
})
697+
698+
t.Run("RegularUserWithAdminCanUpdate", func(t *testing.T) {
699+
t.Parallel()
700+
701+
client := coderdtest.New(t, nil)
702+
user := coderdtest.CreateFirstUser(t, client)
703+
client2, user2 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
704+
_, user3 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
705+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
706+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
707+
req := codersdk.UpdateTemplateMeta{
708+
UserPerms: map[string]codersdk.TemplateRole{
709+
user2.ID.String(): codersdk.TemplateRoleAdmin,
710+
},
711+
}
712+
713+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
714+
defer cancel()
715+
716+
template, err := client.UpdateTemplateMeta(ctx, template.ID, req)
717+
require.NoError(t, err)
718+
719+
req = codersdk.UpdateTemplateMeta{
720+
UserPerms: map[string]codersdk.TemplateRole{
721+
user3.ID.String(): codersdk.TemplateRoleRead,
722+
},
723+
}
724+
725+
template, err = client2.UpdateTemplateMeta(ctx, template.ID, req)
726+
require.NoError(t, err)
727+
728+
role, ok := template.UserRoles[user3.ID.String()]
729+
require.True(t, ok, "User not contained within user_roles map")
730+
require.Equal(t, codersdk.TemplateRoleRead, role)
731+
})
732+
})
522733
}
523734

524735
func TestDeleteTemplate(t *testing.T) {

‎coderd/workspaces_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ import (
2525
"github.com/coder/coder/testutil"
2626
)
2727

28-
func TestWorkspace(t *testing.T) {
28+
func TestWorkspaces(t *testing.T) {
2929
t.Parallel()
3030

31-
t.Run("OK", func(t *testing.T) {
31+
t.Run("OKK", func(t *testing.T) {
3232
t.Parallel()
3333
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
3434
user := coderdtest.CreateFirstUser(t, client)

‎codersdk/error.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,8 @@ func IsConnectionErr(err error) bool {
5151

5252
return xerrors.As(err, &dnsErr) || xerrors.As(err, &opErr)
5353
}
54+
55+
func AsError(err error) (*Error, bool) {
56+
var e *Error
57+
return e, xerrors.As(err, &e)
58+
}

‎codersdk/templates.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,10 @@ type TemplateUserACL map[string]TemplateRole
4242
type TemplateRole string
4343

4444
var (
45-
TemplateRoleAdmin TemplateRole = "admin"
46-
TemplateRoleWrite TemplateRole = "write"
47-
TemplateRoleRead TemplateRole = "read"
45+
TemplateRoleAdmin TemplateRole = "admin"
46+
TemplateRoleWrite TemplateRole = "write"
47+
TemplateRoleRead TemplateRole = "read"
48+
TemplateRoleDeleted TemplateRole = ""
4849
)
4950

5051
type UpdateTemplateMeta struct {

0 commit comments

Comments
 (0)