-
Notifications
You must be signed in to change notification settings - Fork 874
chore: implement 'use' verb to template object, read
has less scope now
#16075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
162b3a5
a0f7f25
d6740c0
4234b83
0c08bc1
bdb29b8
ed6de5e
8dea6c0
4586398
7df41c5
36d329c
9b04ab6
5da005f
389768b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3169,6 +3169,14 @@ func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLin | |
|
||
func (q *querier) InsertWorkspace(ctx context.Context, arg database.InsertWorkspaceParams) (database.WorkspaceTable, error) { | ||
obj := rbac.ResourceWorkspace.WithOwner(arg.OwnerID.String()).InOrg(arg.OrganizationID) | ||
tpl, err := q.GetTemplateByID(ctx, arg.TemplateID) | ||
if err != nil { | ||
return database.WorkspaceTable{}, xerrors.Errorf("verify template by id: %w", err) | ||
} | ||
Comment on lines
+3172
to
+3175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does check the template There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we cache the resources themselves, or just the authz decisions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only cache the authz decisions. The template fetch will happen again unfortunately (from a performance POV). |
||
if err := q.authorizeContext(ctx, policy.ActionUse, tpl); err != nil { | ||
return database.WorkspaceTable{}, xerrors.Errorf("use template for workspace: %w", err) | ||
} | ||
|
||
return insert(q.log, q.auth, obj, q.db.InsertWorkspace)(ctx, arg) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
UPDATE | ||
templates | ||
SET | ||
group_acl = replace(group_acl::text, '["read", "use"]', '["read"]')::jsonb, | ||
user_acl = replace(user_acl::text, '["read", "use"]', '["read"]')::jsonb |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
-- With the "use" verb now existing for templates, we need to update the acl's to | ||
-- include "use" where the permissions set ["read"] is present. | ||
-- The other permission set is ["*"] which is unaffected. | ||
|
||
UPDATE | ||
templates | ||
SET | ||
-- Instead of trying to write a complicated SQL query to update the JSONB | ||
-- object, a string replace is much simpler and easier to understand. | ||
-- Both pieces of text are JSON arrays, so this safe to do. | ||
group_acl = replace(group_acl::text, '["read"]', '["read", "use"]')::jsonb, | ||
user_acl = replace(user_acl::text, '["read"]', '["read", "use"]')::jsonb |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -193,6 +193,53 @@ func TestCreateWorkspace(t *testing.T) { | |
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) | ||
require.Contains(t, apiErr.Message, "doesn't exist") | ||
}) | ||
|
||
// Auditors cannot "use" templates, they can only read them. | ||
t.Run("Auditor", func(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This passes without an error on |
||
t.Parallel() | ||
|
||
owner, first := coderdenttest.New(t, &coderdenttest.Options{ | ||
Options: &coderdtest.Options{ | ||
IncludeProvisionerDaemon: true, | ||
}, | ||
LicenseOptions: &coderdenttest.LicenseOptions{ | ||
Features: license.Features{ | ||
codersdk.FeatureTemplateRBAC: 1, | ||
codersdk.FeatureMultipleOrganizations: 1, | ||
}, | ||
}, | ||
}) | ||
|
||
// A member of the org as an auditor | ||
auditor, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleAuditor()) | ||
|
||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) | ||
defer cancel() | ||
|
||
// Given: a template with a version without the "use" permission on everyone | ||
version := coderdtest.CreateTemplateVersion(t, owner, first.OrganizationID, nil) | ||
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, owner, version.ID) | ||
template := coderdtest.CreateTemplate(t, owner, first.OrganizationID, version.ID) | ||
|
||
//nolint:gocritic // This should be run as the owner user. | ||
err := owner.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{ | ||
UserPerms: nil, | ||
GroupPerms: map[string]codersdk.TemplateRole{ | ||
first.OrganizationID.String(): codersdk.TemplateRoleDeleted, | ||
}, | ||
}) | ||
require.NoError(t, err) | ||
|
||
_, err = auditor.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ | ||
TemplateID: template.ID, | ||
Name: "workspace", | ||
}) | ||
require.Error(t, err) | ||
var apiErr *codersdk.Error | ||
require.ErrorAs(t, err, &apiErr) | ||
require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) | ||
require.Contains(t, apiErr.Message, "Unauthorized access to use the template") | ||
}) | ||
} | ||
|
||
func TestCreateUserWorkspace(t *testing.T) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to place this in
codersdk
, but then I'd have to importpolicy
. And we already havecodersdk.RBACAction
.TemplateRole
should probably be a database enum. At present it only exists incodersdk.