Skip to content

Commit 7d0b0c9

Browse files
mtojekmtojek
committed
WIP
1 parent 1d34942 commit 7d0b0c9

File tree

5 files changed

+47
-14
lines changed

5 files changed

+47
-14
lines changed

‎coderd/database/dbauthz/querier.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -851,11 +851,14 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U
851851
}
852852

853853
func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) {
854-
template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID)
854+
// Must do an authorized fetch to prevent leaking template ids this way.
855+
tpl, err := q.GetTemplateByID(ctx, arg.TemplateID.UUID)
855856
if err != nil {
856857
return database.TemplateVersion{}, err
857858
}
858-
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
859+
// Check the create permission on the template.
860+
err = q.authorizeContext(ctx, rbac.ActionUpdate, tpl)
861+
if err != nil {
859862
return database.TemplateVersion{}, err
860863
}
861864
return q.db.UpdateTemplateVersionByID(ctx, arg)

‎coderd/database/dbauthz/querier_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ func (s *MethodTestSuite) TestTemplate() {
721721
check.Args(database.UpdateTemplateVersionByIDParams{
722722
ID: tv.ID,
723723
TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true},
724+
Name: tv.Name,
725+
UpdatedAt: tv.UpdatedAt,
724726
}).Asserts(t1, rbac.ActionUpdate).Returns(tv)
725727
}))
726728
s.Run("UpdateTemplateVersionDescriptionByJobID", s.Subtest(func(db database.Store, check *expects) {

‎coderd/templateversions.go

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,21 +80,46 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) {
8080
if !httpapi.Read(ctx, rw, r, &params) {
8181
return
8282
}
83-
if params.Name == "" {
84-
params.Name = templateVersion.Name
83+
84+
updateParams := database.UpdateTemplateVersionByIDParams{
85+
ID: templateVersion.ID,
86+
TemplateID: templateVersion.TemplateID,
87+
UpdatedAt: database.Now(),
88+
Name: templateVersion.Name,
8589
}
86-
templateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{
87-
ID: templateVersion.ID,
88-
Name: params.Name,
89-
})
90+
91+
if params.Name != "" {
92+
updateParams.Name = params.Name
93+
}
94+
// It is not allowed to "patch" the template ID, and reassign it.
95+
updatedTemplateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, updateParams)
9096
if err != nil {
9197
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
9298
Message: "Error on patching template version.",
9399
Detail: err.Error(),
94100
})
95101
return
96102
}
97-
httpapi.Write(ctx, rw, http.StatusNoContent, templateVersion)
103+
104+
job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID)
105+
if err != nil {
106+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
107+
Message: "Internal error fetching provisioner job.",
108+
Detail: err.Error(),
109+
})
110+
return
111+
}
112+
113+
user, err := api.Database.GetUserByID(ctx, templateVersion.CreatedBy)
114+
if err != nil {
115+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
116+
Message: "Internal error on fetching user.",
117+
Detail: err.Error(),
118+
})
119+
return
120+
}
121+
122+
httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(updatedTemplateVersion, convertProvisionerJob(job), user))
98123
}
99124

100125
// @Summary Cancel template version by ID

‎coderd/templateversions_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,30 +1342,33 @@ func TestTemplateVersionPatch(t *testing.T) {
13421342
client := coderdtest.New(t, nil)
13431343
user := coderdtest.CreateFirstUser(t, client)
13441344
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1345+
coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
13451346

13461347
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
13471348
defer cancel()
13481349

1350+
const newName = "new_name"
13491351
updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{
1350-
Name: "new name",
1352+
Name: newName,
13511353
})
13521354

13531355
require.NoError(t, err)
1354-
require.Equal(t, updatedVersion.Name, "new name")
1356+
assert.Equal(t, newName, updatedVersion.Name)
1357+
assert.NotEqual(t, updatedVersion.Name, version.Name)
13551358
})
13561359

13571360
t.Run("Use the same name if a new name is not passed", func(t *testing.T) {
13581361
t.Parallel()
13591362
client := coderdtest.New(t, nil)
13601363
user := coderdtest.CreateFirstUser(t, client)
13611364
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1365+
coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
13621366

13631367
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
13641368
defer cancel()
13651369

13661370
updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{})
1367-
13681371
require.NoError(t, err)
1369-
require.Equal(t, updatedVersion.Name, version.Name)
1372+
assert.Equal(t, version.Name, updatedVersion.Name)
13701373
})
13711374
}

‎codersdk/templateversions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func (c *Client) UpdateTemplateVersion(ctx context.Context, versionID uuid.UUID,
302302
return TemplateVersion{}, err
303303
}
304304
defer res.Body.Close()
305-
if res.StatusCode != http.StatusNoContent {
305+
if res.StatusCode != http.StatusOK {
306306
return TemplateVersion{}, ReadBodyAsError(res)
307307
}
308308
var version TemplateVersion

0 commit comments

Comments
 (0)