Skip to content

Commit f34e6fd

Browse files
EmyrkEmyrk
authored
chore: implement 'use' verb to template object, read has less scope now (#16075)
Template `use` is now a verb. - Template admins can `use` all templates (org template admins same in org) - Members get the `use` perm from the `everyone` group in the `group_acl`.
1 parent 3217cb8 commit f34e6fd

17 files changed

+128
-28
lines changed

‎coderd/database/db2sdk/db2sdk.go

+11
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/coder/coder/v2/coderd/database"
1919
"github.com/coder/coder/v2/coderd/rbac"
20+
"github.com/coder/coder/v2/coderd/rbac/policy"
2021
"github.com/coder/coder/v2/coderd/render"
2122
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
2223
"github.com/coder/coder/v2/codersdk"
@@ -694,3 +695,13 @@ func MatchedProvisioners(provisionerDaemons []database.ProvisionerDaemon, now ti
694695
}
695696
return matched
696697
}
698+
699+
func TemplateRoleActions(role codersdk.TemplateRole) []policy.Action {
700+
switch role {
701+
case codersdk.TemplateRoleAdmin:
702+
return []policy.Action{policy.WildcardSymbol}
703+
case codersdk.TemplateRoleUse:
704+
return []policy.Action{policy.ActionRead, policy.ActionUse}
705+
}
706+
return []policy.Action{}
707+
}

‎coderd/database/dbauthz/dbauthz.go

+8
Original file line numberDiff line numberDiff line change
@@ -3169,6 +3169,14 @@ func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLin
31693169

31703170
func (q *querier) InsertWorkspace(ctx context.Context, arg database.InsertWorkspaceParams) (database.WorkspaceTable, error) {
31713171
obj := rbac.ResourceWorkspace.WithOwner(arg.OwnerID.String()).InOrg(arg.OrganizationID)
3172+
tpl, err := q.GetTemplateByID(ctx, arg.TemplateID)
3173+
if err != nil {
3174+
return database.WorkspaceTable{}, xerrors.Errorf("verify template by id: %w", err)
3175+
}
3176+
if err := q.authorizeContext(ctx, policy.ActionUse, tpl); err != nil {
3177+
return database.WorkspaceTable{}, xerrors.Errorf("use template for workspace: %w", err)
3178+
}
3179+
31723180
return insert(q.log, q.auth, obj, q.db.InsertWorkspace)(ctx, arg)
31733181
}
31743182

‎coderd/database/dbauthz/dbauthz_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2459,7 +2459,7 @@ func (s *MethodTestSuite) TestWorkspace() {
24592459
OrganizationID: o.ID,
24602460
AutomaticUpdates: database.AutomaticUpdatesNever,
24612461
TemplateID: tpl.ID,
2462-
}).Asserts(rbac.ResourceWorkspace.WithOwner(u.ID.String()).InOrg(o.ID), policy.ActionCreate)
2462+
}).Asserts(tpl, policy.ActionRead, tpl, policy.ActionUse, rbac.ResourceWorkspace.WithOwner(u.ID.String()).InOrg(o.ID), policy.ActionCreate)
24632463
}))
24642464
s.Run("Start/InsertWorkspaceBuild", s.Subtest(func(db database.Store, check *expects) {
24652465
u := dbgen.User(s.T(), db, database.User{})

‎coderd/database/dbgen/dbgen.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@ import (
2020
"golang.org/x/xerrors"
2121

2222
"github.com/coder/coder/v2/coderd/database"
23+
"github.com/coder/coder/v2/coderd/database/db2sdk"
2324
"github.com/coder/coder/v2/coderd/database/dbauthz"
2425
"github.com/coder/coder/v2/coderd/database/dbtime"
2526
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2627
"github.com/coder/coder/v2/coderd/database/pubsub"
2728
"github.com/coder/coder/v2/coderd/rbac"
28-
"github.com/coder/coder/v2/coderd/rbac/policy"
29+
"github.com/coder/coder/v2/codersdk"
2930
"github.com/coder/coder/v2/cryptorand"
3031
"github.com/coder/coder/v2/testutil"
3132
)
@@ -75,7 +76,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database.
7576
if seed.GroupACL == nil {
7677
// By default, all users in the organization can read the template.
7778
seed.GroupACL = database.TemplateACL{
78-
seed.OrganizationID.String(): []policy.Action{policy.ActionRead},
79+
seed.OrganizationID.String(): db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse),
7980
}
8081
}
8182
if seed.UserACL == nil {

‎coderd/database/migrations/000287_template_read_to_use.down.sql

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
UPDATE
2+
templates
3+
SET
4+
group_acl = replace(group_acl::text, '["read", "use"]', '["read"]')::jsonb,
5+
user_acl = replace(user_acl::text, '["read", "use"]', '["read"]')::jsonb

‎coderd/database/migrations/000287_template_read_to_use.up.sql

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-- With the "use" verb now existing for templates, we need to update the acl's to
2+
-- include "use" where the permissions set ["read"] is present.
3+
-- The other permission set is ["*"] which is unaffected.
4+
5+
UPDATE
6+
templates
7+
SET
8+
-- Instead of trying to write a complicated SQL query to update the JSONB
9+
-- object, a string replace is much simpler and easier to understand.
10+
-- Both pieces of text are JSON arrays, so this safe to do.
11+
group_acl = replace(group_acl::text, '["read"]', '["read", "use"]')::jsonb,
12+
user_acl = replace(user_acl::text, '["read"]', '["read", "use"]')::jsonb

‎coderd/insights_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ import (
2323
agentproto "github.com/coder/coder/v2/agent/proto"
2424
"github.com/coder/coder/v2/coderd/coderdtest"
2525
"github.com/coder/coder/v2/coderd/database"
26+
"github.com/coder/coder/v2/coderd/database/db2sdk"
2627
"github.com/coder/coder/v2/coderd/database/dbauthz"
2728
"github.com/coder/coder/v2/coderd/database/dbgen"
2829
"github.com/coder/coder/v2/coderd/database/dbrollup"
2930
"github.com/coder/coder/v2/coderd/database/dbtestutil"
3031
"github.com/coder/coder/v2/coderd/rbac"
31-
"github.com/coder/coder/v2/coderd/rbac/policy"
3232
"github.com/coder/coder/v2/coderd/workspaceapps"
3333
"github.com/coder/coder/v2/coderd/workspacestats"
3434
"github.com/coder/coder/v2/codersdk"
@@ -675,7 +675,7 @@ func TestTemplateInsights_Golden(t *testing.T) {
675675
OrganizationID: firstUser.OrganizationID,
676676
CreatedBy: firstUser.UserID,
677677
GroupACL: database.TemplateACL{
678-
firstUser.OrganizationID.String(): []policy.Action{policy.ActionRead},
678+
firstUser.OrganizationID.String(): db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse),
679679
},
680680
})
681681
err := db.UpdateTemplateVersionByID(context.Background(), database.UpdateTemplateVersionByIDParams{
@@ -1573,7 +1573,7 @@ func TestUserActivityInsights_Golden(t *testing.T) {
15731573
OrganizationID: firstUser.OrganizationID,
15741574
CreatedBy: firstUser.UserID,
15751575
GroupACL: database.TemplateACL{
1576-
firstUser.OrganizationID.String(): []policy.Action{policy.ActionRead},
1576+
firstUser.OrganizationID.String(): db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse),
15771577
},
15781578
})
15791579
err := db.UpdateTemplateVersionByID(context.Background(), database.UpdateTemplateVersionByIDParams{

‎coderd/rbac/object_gen.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/rbac/policy/policy.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ var RBACPermissions = map[string]PermissionDefinition{
133133
},
134134
"template": {
135135
Actions: map[Action]ActionDefinition{
136-
ActionCreate: actDef("create a template"),
137-
// TODO: Create a use permission maybe?
136+
ActionCreate: actDef("create a template"),
137+
ActionUse: actDef("use the template to initially create a workspace, then workspace lifecycle permissions take over"),
138138
ActionRead: actDef("read template"),
139139
ActionUpdate: actDef("update a template"),
140140
ActionDelete: actDef("delete a template"),

‎coderd/rbac/roles.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
318318
Identifier: RoleTemplateAdmin(),
319319
DisplayName: "Template Admin",
320320
Site: Permissions(map[string][]policy.Action{
321-
ResourceTemplate.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionViewInsights},
321+
ResourceTemplate.Type: ResourceTemplate.AvailableActions(),
322322
// CRUD all files, even those they did not upload.
323323
ResourceFile.Type: {policy.ActionCreate, policy.ActionRead},
324324
ResourceWorkspace.Type: {policy.ActionRead},
@@ -476,7 +476,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
476476
Site: []Permission{},
477477
Org: map[string][]Permission{
478478
organizationID.String(): Permissions(map[string][]policy.Action{
479-
ResourceTemplate.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionViewInsights},
479+
ResourceTemplate.Type: ResourceTemplate.AvailableActions(),
480480
ResourceFile.Type: {policy.ActionCreate, policy.ActionRead},
481481
ResourceWorkspace.Type: {policy.ActionRead},
482482
// Assigning template perms requires this permission.

‎coderd/rbac/roles_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,17 @@ func TestRolePermissions(t *testing.T) {
232232
false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, userAdmin, orgMemberMe},
233233
},
234234
},
235+
{
236+
Name: "UseTemplates",
237+
Actions: []policy.Action{policy.ActionUse},
238+
Resource: rbac.ResourceTemplate.InOrg(orgID).WithGroupACL(map[string][]policy.Action{
239+
groupID.String(): {policy.ActionUse},
240+
}),
241+
AuthorizeMap: map[bool][]hasAuthSubjects{
242+
true: {owner, orgAdmin, templateAdmin, orgTemplateAdmin, groupMemberMe},
243+
false: {setOtherOrg, orgAuditor, orgUserAdmin, memberMe, userAdmin, orgMemberMe},
244+
},
245+
},
235246
{
236247
Name: "Files",
237248
Actions: []policy.Action{policy.ActionCreate},

‎coderd/templates.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"golang.org/x/xerrors"
1515

1616
"cdr.dev/slog"
17+
"github.com/coder/coder/v2/coderd/database/db2sdk"
1718

1819
"github.com/coder/coder/v2/coderd/audit"
1920
"github.com/coder/coder/v2/coderd/database"
@@ -382,7 +383,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
382383
if !createTemplate.DisableEveryoneGroupAccess {
383384
// The organization ID is used as the group ID for the everyone group
384385
// in this organization.
385-
defaultsGroups[organization.ID.String()] = []policy.Action{policy.ActionRead}
386+
defaultsGroups[organization.ID.String()] = db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse)
386387
}
387388
err = api.Database.InTx(func(tx database.Store) error {
388389
now := dbtime.Now()

‎coderd/workspaces.go

+12
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,18 @@ func createWorkspace(
525525
httpapi.ResourceNotFound(rw)
526526
return
527527
}
528+
// The user also needs permission to use the template. At this point they have
529+
// read perms, but not necessarily "use". This is also checked in `db.InsertWorkspace`.
530+
// Doing this up front can save some work below if the user doesn't have permission.
531+
if !api.Authorize(r, policy.ActionUse, template) {
532+
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
533+
Message: fmt.Sprintf("Unauthorized access to use the template %q.", template.Name),
534+
Detail: "Although you are able to view the template, you are unable to create a workspace using it. " +
535+
"Please contact an administrator about your permissions if you feel this is an error.",
536+
Validations: nil,
537+
})
538+
return
539+
}
528540

529541
templateAccessControl := (*(api.AccessControlStore.Load())).GetTemplateAccessControl(template)
530542
if templateAccessControl.IsDeprecated() {

‎codersdk/rbacresources_gen.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎enterprise/coderd/templates.go

+6-16
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/coder/coder/v2/coderd/httpapi"
1717
"github.com/coder/coder/v2/coderd/httpmw"
1818
"github.com/coder/coder/v2/coderd/rbac/policy"
19+
"github.com/coder/coder/v2/coderd/util/slice"
1920
"github.com/coder/coder/v2/codersdk"
2021
)
2122

@@ -222,7 +223,7 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) {
222223
delete(template.UserACL, id)
223224
continue
224225
}
225-
template.UserACL[id] = convertSDKTemplateRole(role)
226+
template.UserACL[id] = db2sdk.TemplateRoleActions(role)
226227
}
227228
}
228229

@@ -234,7 +235,7 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) {
234235
delete(template.GroupACL, id)
235236
continue
236237
}
237-
template.GroupACL[id] = convertSDKTemplateRole(role)
238+
template.GroupACL[id] = db2sdk.TemplateRoleActions(role)
238239
}
239240
}
240241

@@ -316,8 +317,8 @@ func convertTemplateUsers(tus []database.TemplateUser, orgIDsByUserIDs map[uuid.
316317
}
317318

318319
func validateTemplateRole(role codersdk.TemplateRole) error {
319-
actions := convertSDKTemplateRole(role)
320-
if actions == nil && role != codersdk.TemplateRoleDeleted {
320+
actions := db2sdk.TemplateRoleActions(role)
321+
if len(actions) == 0 && role != codersdk.TemplateRoleDeleted {
321322
return xerrors.Errorf("role %q is not a valid Template role", role)
322323
}
323324

@@ -326,7 +327,7 @@ func validateTemplateRole(role codersdk.TemplateRole) error {
326327

327328
func convertToTemplateRole(actions []policy.Action) codersdk.TemplateRole {
328329
switch {
329-
case len(actions) == 1 && actions[0] == policy.ActionRead:
330+
case len(actions) == 2 && slice.SameElements(actions, []policy.Action{policy.ActionUse, policy.ActionRead}):
330331
return codersdk.TemplateRoleUse
331332
case len(actions) == 1 && actions[0] == policy.WildcardSymbol:
332333
return codersdk.TemplateRoleAdmin
@@ -335,17 +336,6 @@ func convertToTemplateRole(actions []policy.Action) codersdk.TemplateRole {
335336
return ""
336337
}
337338

338-
func convertSDKTemplateRole(role codersdk.TemplateRole) []policy.Action {
339-
switch role {
340-
case codersdk.TemplateRoleAdmin:
341-
return []policy.Action{policy.WildcardSymbol}
342-
case codersdk.TemplateRoleUse:
343-
return []policy.Action{policy.ActionRead}
344-
}
345-
346-
return nil
347-
}
348-
349339
// TODO move to api.RequireFeatureMW when we are OK with changing the behavior.
350340
func (api *API) templateRBACEnabledMW(next http.Handler) http.Handler {
351341
return api.RequireFeatureMW(codersdk.FeatureTemplateRBAC)(next)

‎enterprise/coderd/workspaces_test.go

+47
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,53 @@ func TestCreateWorkspace(t *testing.T) {
193193
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
194194
require.Contains(t, apiErr.Message, "doesn't exist")
195195
})
196+
197+
// Auditors cannot "use" templates, they can only read them.
198+
t.Run("Auditor", func(t *testing.T) {
199+
t.Parallel()
200+
201+
owner, first := coderdenttest.New(t, &coderdenttest.Options{
202+
Options: &coderdtest.Options{
203+
IncludeProvisionerDaemon: true,
204+
},
205+
LicenseOptions: &coderdenttest.LicenseOptions{
206+
Features: license.Features{
207+
codersdk.FeatureTemplateRBAC: 1,
208+
codersdk.FeatureMultipleOrganizations: 1,
209+
},
210+
},
211+
})
212+
213+
// A member of the org as an auditor
214+
auditor, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleAuditor())
215+
216+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
217+
defer cancel()
218+
219+
// Given: a template with a version without the "use" permission on everyone
220+
version := coderdtest.CreateTemplateVersion(t, owner, first.OrganizationID, nil)
221+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, owner, version.ID)
222+
template := coderdtest.CreateTemplate(t, owner, first.OrganizationID, version.ID)
223+
224+
//nolint:gocritic // This should be run as the owner user.
225+
err := owner.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{
226+
UserPerms: nil,
227+
GroupPerms: map[string]codersdk.TemplateRole{
228+
first.OrganizationID.String(): codersdk.TemplateRoleDeleted,
229+
},
230+
})
231+
require.NoError(t, err)
232+
233+
_, err = auditor.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{
234+
TemplateID: template.ID,
235+
Name: "workspace",
236+
})
237+
require.Error(t, err)
238+
var apiErr *codersdk.Error
239+
require.ErrorAs(t, err, &apiErr)
240+
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
241+
require.Contains(t, apiErr.Message, "Unauthorized access to use the template")
242+
})
196243
}
197244

198245
func TestCreateUserWorkspace(t *testing.T) {

‎site/src/api/rbacresourcesGenerated.ts

+1
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export const RBACResourceActions: Partial<
144144
delete: "delete a template",
145145
read: "read template",
146146
update: "update a template",
147+
use: "use the template to initially create a workspace, then workspace lifecycle permissions take over",
147148
view_insights: "view insights",
148149
},
149150
user: {

0 commit comments

Comments
 (0)