fix: Validate template version name (#6804)

* WIP

* Update

* Validation
This commit is contained in:
Marcin Tojek
2023-03-27 13:54:01 +02:00
committed by GitHub
parent e0cc4ee7f8
commit 8187992e7f
6 changed files with 105 additions and 10 deletions

View File

@ -859,11 +859,22 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U
}
func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) {
template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID)
// An actor is allowed to update the template version if they are authorized to update the template.
tv, err := q.db.GetTemplateVersionByID(ctx, arg.ID)
if err != nil {
return database.TemplateVersion{}, err
}
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
var obj rbac.Objecter
if !tv.TemplateID.Valid {
obj = rbac.ResourceTemplate.InOrg(tv.OrganizationID)
} else {
tpl, err := q.db.GetTemplateByID(ctx, tv.TemplateID.UUID)
if err != nil {
return database.TemplateVersion{}, err
}
obj = tpl
}
if err := q.authorizeContext(ctx, rbac.ActionUpdate, obj); err != nil {
return database.TemplateVersion{}, err
}
return q.db.UpdateTemplateVersionByID(ctx, arg)

View File

@ -44,7 +44,7 @@ func init() {
valid := NameValid(str)
return valid == nil
}
for _, tag := range []string{"username", "template_name", "workspace_name"} {
for _, tag := range []string{"username", "template_name", "workspace_name", "template_version_name"} {
err := Validate.RegisterValidation(tag, nameValidator)
if err != nil {
panic(err)

View File

@ -92,12 +92,43 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) {
if params.Name != "" {
updateParams.Name = params.Name
}
// It is not allowed to "patch" the template ID, and reassign it.
updatedTemplateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, updateParams)
errTemplateVersionNameConflict := xerrors.New("template version name must be unique for a template")
var updatedTemplateVersion database.TemplateVersion
err := api.Database.InTx(func(tx database.Store) error {
if templateVersion.TemplateID.Valid && templateVersion.Name != updateParams.Name {
// User wants to rename the template version
_, err := tx.GetTemplateVersionByTemplateIDAndName(ctx, database.GetTemplateVersionByTemplateIDAndNameParams{
TemplateID: templateVersion.TemplateID,
Name: updateParams.Name,
})
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
return xerrors.Errorf("error on retrieving conflicting template version: %v", err)
}
if err == nil {
return errTemplateVersionNameConflict
}
}
// It is not allowed to "patch" the template ID, and reassign it.
var err error
updatedTemplateVersion, err = tx.UpdateTemplateVersionByID(ctx, updateParams)
if err != nil {
return xerrors.Errorf("error on patching template version: %v", err)
}
return nil
}, nil)
if errors.Is(err, errTemplateVersionNameConflict) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: err.Error(),
})
return
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Error on patching template version.",
Detail: err.Error(),
Message: err.Error(),
})
return
}

View File

@ -1347,7 +1347,7 @@ func TestTemplateVersionPatch(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
const newName = "new_name"
const newName = "new-name"
updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{
Name: newName,
})
@ -1376,6 +1376,7 @@ func TestTemplateVersionPatch(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID)
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
@ -1398,4 +1399,56 @@ func TestTemplateVersionPatch(t *testing.T) {
assert.NotEqual(t, updatedVersion1.ID, updatedVersion2.ID)
assert.Equal(t, updatedVersion1.Name, updatedVersion2.Name)
})
t.Run("Use the same name for two versions for the same templates", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID)
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
ctvr.TemplateID = template.ID
})
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
_, err := client.UpdateTemplateVersion(ctx, version2.ID, codersdk.PatchTemplateVersionRequest{
Name: version1.Name,
})
require.Error(t, err)
})
t.Run("Rename the unassigned template", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
const commonTemplateVersionName = "common-template-version-name"
updatedVersion1, err := client.UpdateTemplateVersion(ctx, version1.ID, codersdk.PatchTemplateVersionRequest{
Name: commonTemplateVersionName,
})
require.NoError(t, err)
assert.Equal(t, commonTemplateVersionName, updatedVersion1.Name)
})
t.Run("Use incorrect template version name", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
const incorrectTemplateVersionName = "incorrect/name"
_, err := client.UpdateTemplateVersion(ctx, version1.ID, codersdk.PatchTemplateVersionRequest{
Name: incorrectTemplateVersionName,
})
require.Error(t, err)
})
}