Skip to content

Commit 708557d

Browse files
EmyrkEmyrk
authored
chore: always use new codepath over deprecated (#14050)
* chore: always use new codepath over deprecated * add unit test for lack of template access
1 parent 6b75cbf commit 708557d

File tree

5 files changed

+74
-40
lines changed

5 files changed

+74
-40
lines changed

‎coderd/audit/request.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ type Request[T Auditable] struct {
5151
Action database.AuditAction
5252
}
5353

54+
// UpdateOrganizationID can be used if the organization ID is not known
55+
// at the initiation of an audit log request.
5456
func (r *Request[T]) UpdateOrganizationID(id uuid.UUID) {
5557
r.params.OrganizationID = id
5658
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,25 +1064,6 @@ func (w WorkspaceAgentWaiter) Wait() []codersdk.WorkspaceResource {
10641064
// CreateWorkspace creates a workspace for the user and template provided.
10651065
// A random name is generated for it.
10661066
// To customize the defaults, pass a mutator func.
1067-
//
1068-
// Deprecated: Use CreateWorkspace.
1069-
func CreateWorkspaceByOrganization(t testing.TB, client *codersdk.Client, organization uuid.UUID, templateID uuid.UUID, mutators ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace {
1070-
t.Helper()
1071-
req := codersdk.CreateWorkspaceRequest{
1072-
TemplateID: templateID,
1073-
Name: RandomUsername(t),
1074-
AutostartSchedule: ptr.Ref("CRON_TZ=US/Central 30 9 * * 1-5"),
1075-
TTLMillis: ptr.Ref((8 * time.Hour).Milliseconds()),
1076-
AutomaticUpdates: codersdk.AutomaticUpdatesNever,
1077-
}
1078-
for _, mutator := range mutators {
1079-
mutator(&req)
1080-
}
1081-
workspace, err := client.CreateWorkspace(context.Background(), organization, codersdk.Me, req)
1082-
require.NoError(t, err)
1083-
return workspace
1084-
}
1085-
10861067
func CreateWorkspace(t testing.TB, client *codersdk.Client, templateID uuid.UUID, mutators ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace {
10871068
t.Helper()
10881069
req := codersdk.CreateWorkspaceRequest{

‎coderd/workspaces.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ func createWorkspace(
464464
templateID := req.TemplateID
465465
if templateID == uuid.Nil {
466466
templateVersion, err := api.Database.GetTemplateVersionByID(ctx, req.TemplateVersionID)
467-
if errors.Is(err, sql.ErrNoRows) {
467+
if httpapi.Is404Error(err) {
468468
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
469469
Message: fmt.Sprintf("Template version %q doesn't exist.", templateID.String()),
470470
Validations: []codersdk.ValidationError{{
@@ -498,7 +498,7 @@ func createWorkspace(
498498
}
499499

500500
template, err := api.Database.GetTemplateByID(ctx, templateID)
501-
if errors.Is(err, sql.ErrNoRows) {
501+
if httpapi.Is404Error(err) {
502502
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
503503
Message: fmt.Sprintf("Template %q doesn't exist.", templateID.String()),
504504
Validations: []codersdk.ValidationError{{
@@ -521,8 +521,18 @@ func createWorkspace(
521521
})
522522
return
523523
}
524+
525+
// Update audit log's organization
524526
auditReq.UpdateOrganizationID(template.OrganizationID)
525527

528+
// Do this upfront to save work. If this fails, the rest of the work
529+
// would be wasted.
530+
if !api.Authorize(r, policy.ActionCreate,
531+
rbac.ResourceWorkspace.InOrg(template.OrganizationID).WithOwner(owner.ID.String())) {
532+
httpapi.ResourceNotFound(rw)
533+
return
534+
}
535+
526536
templateAccessControl := (*(api.AccessControlStore.Load())).GetTemplateAccessControl(template)
527537
if templateAccessControl.IsDeprecated() {
528538
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -578,7 +588,7 @@ func createWorkspace(
578588
// read other workspaces. Ideally we check the error on create and look for
579589
// a postgres conflict error.
580590
workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
581-
OwnerID: user.ID,
591+
OwnerID: owner.ID,
582592
Name: req.Name,
583593
})
584594
if err == nil {
@@ -611,7 +621,7 @@ func createWorkspace(
611621
ID: uuid.New(),
612622
CreatedAt: now,
613623
UpdatedAt: now,
614-
OwnerID: user.ID,
624+
OwnerID: owner.ID,
615625
OrganizationID: template.OrganizationID,
616626
TemplateID: template.ID,
617627
Name: req.Name,
@@ -679,8 +689,8 @@ func createWorkspace(
679689
ProvisionerJob: *provisionerJob,
680690
QueuePosition: 0,
681691
},
682-
user.Username,
683-
user.AvatarURL,
692+
owner.Username,
693+
owner.AvatarURL,
684694
[]database.WorkspaceResource{},
685695
[]database.WorkspaceResourceMetadatum{},
686696
[]database.WorkspaceAgent{},
@@ -702,8 +712,8 @@ func createWorkspace(
702712
workspace,
703713
apiBuild,
704714
template,
705-
user.Username,
706-
user.AvatarURL,
715+
owner.Username,
716+
owner.AvatarURL,
707717
api.Options.AllowWorkspaceRenames,
708718
)
709719
if err != nil {

‎codersdk/organizations.go

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -475,19 +475,8 @@ func (c *Client) TemplateByName(ctx context.Context, organizationID uuid.UUID, n
475475
// CreateWorkspace creates a new workspace for the template specified.
476476
//
477477
// Deprecated: Use CreateUserWorkspace instead.
478-
func (c *Client) CreateWorkspace(ctx context.Context, organizationID uuid.UUID, user string, request CreateWorkspaceRequest) (Workspace, error) {
479-
res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/organizations/%s/members/%s/workspaces", organizationID, user), request)
480-
if err != nil {
481-
return Workspace{}, err
482-
}
483-
defer res.Body.Close()
484-
485-
if res.StatusCode != http.StatusCreated {
486-
return Workspace{}, ReadBodyAsError(res)
487-
}
488-
489-
var workspace Workspace
490-
return workspace, json.NewDecoder(res.Body).Decode(&workspace)
478+
func (c *Client) CreateWorkspace(ctx context.Context, _ uuid.UUID, user string, request CreateWorkspaceRequest) (Workspace, error) {
479+
return c.CreateUserWorkspace(ctx, user, request)
491480
}
492481

493482
// CreateUserWorkspace creates a new workspace for the template specified.

‎enterprise/coderd/workspaces_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,58 @@ func TestCreateWorkspace(t *testing.T) {
135135
_, err = client1.CreateWorkspace(ctx, user.OrganizationID, user1.ID.String(), req)
136136
require.Error(t, err)
137137
})
138+
139+
t.Run("NoTemplateAccess", func(t *testing.T) {
140+
t.Parallel()
141+
ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{
142+
Options: &coderdtest.Options{
143+
IncludeProvisionerDaemon: true,
144+
},
145+
LicenseOptions: &coderdenttest.LicenseOptions{
146+
Features: license.Features{
147+
codersdk.FeatureTemplateRBAC: 1,
148+
},
149+
},
150+
})
151+
152+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
153+
user, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleMember())
154+
155+
version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, nil)
156+
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID)
157+
template := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
158+
159+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
160+
defer cancel()
161+
162+
// Remove everyone access
163+
err := templateAdmin.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{
164+
UserPerms: map[string]codersdk.TemplateRole{},
165+
GroupPerms: map[string]codersdk.TemplateRole{
166+
owner.OrganizationID.String(): codersdk.TemplateRoleDeleted,
167+
},
168+
})
169+
require.NoError(t, err)
170+
171+
// Test "everyone" access is revoked to the regular user
172+
_, err = user.Template(ctx, template.ID)
173+
require.Error(t, err)
174+
var apiErr *codersdk.Error
175+
require.ErrorAs(t, err, &apiErr)
176+
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
177+
178+
_, err = user.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{
179+
TemplateID: template.ID,
180+
Name: "random",
181+
AutostartSchedule: ptr.Ref("CRON_TZ=US/Central 30 9 * * 1-5"),
182+
TTLMillis: ptr.Ref((8 * time.Hour).Milliseconds()),
183+
AutomaticUpdates: codersdk.AutomaticUpdatesNever,
184+
})
185+
require.Error(t, err)
186+
require.ErrorAs(t, err, &apiErr)
187+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
188+
require.Contains(t, apiErr.Message, "doesn't exist")
189+
})
138190
}
139191

140192
func TestCreateUserWorkspace(t *testing.T) {

0 commit comments

Comments
 (0)