Skip to content

chore: merge provisioner key and provisioner permissions #16628

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 24, 2025
Merged

chore: merge provisioner key and provisioner permissions #16628

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0e0cd23
chore: merge provisioner key and provisioner permissions
Emyrk Feb 19, 2025
1abff51
chore: filter out invalid permissions on custom role push
Emyrk Feb 19, 2025
0f15263
linting
Emyrk Feb 19, 2025
aee3289
Update site/src/api/rbacresourcesGenerated.ts
Emyrk Feb 19, 2025
ccc3afa
Update coderd/rbac/policy/policy.go
Emyrk Feb 19, 2025
a6fcd52
make gen
Emyrk Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions coderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions coderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions coderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ var (
rbac.ResourceOrganization.Type: {policy.ActionCreate, policy.ActionRead},
rbac.ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionDelete, policy.ActionRead},
rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate},
rbac.ResourceProvisionerKeys.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionDelete},
rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(),
rbac.ResourceWorkspaceDormant.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStop},
rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH},
Expand Down Expand Up @@ -3196,7 +3195,7 @@ func (q *querier) InsertProvisionerJobTimings(ctx context.Context, arg database.
}

func (q *querier) InsertProvisionerKey(ctx context.Context, arg database.InsertProvisionerKeyParams) (database.ProvisionerKey, error) {
return insert(q.log, q.auth, rbac.ResourceProvisionerKeys.InOrg(arg.OrganizationID).WithID(arg.ID), q.db.InsertProvisionerKey)(ctx, arg)
return insert(q.log, q.auth, rbac.ResourceProvisionerDaemon.InOrg(arg.OrganizationID).WithID(arg.ID), q.db.InsertProvisionerKey)(ctx, arg)
}

func (q *querier) InsertReplica(ctx context.Context, arg database.InsertReplicaParams) (database.Replica, error) {
Expand Down
4 changes: 3 additions & 1 deletion coderd/database/modelmethods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,10 @@ func (p GetEligibleProvisionerDaemonsByProvisionerJobIDsRow) RBACObject() rbac.O
return p.ProvisionerDaemon.RBACObject()
}

// RBACObject for a provisioner key is the same as a provisioner daemon.
// Keys == provisioners from a RBAC perspective.
func (p ProvisionerKey) RBACObject() rbac.Object {
return rbac.ResourceProvisionerKeys.
return rbac.ResourceProvisionerDaemon.
WithID(p.ID).
InOrg(p.OrganizationID)
}
Expand Down
14 changes: 2 additions & 12 deletions coderd/rbac/object_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 2 additions & 9 deletions coderd/rbac/policy/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,25 +162,18 @@ var RBACPermissions = map[string]PermissionDefinition{
},
"provisioner_daemon": {
Actions: map[Action]ActionDefinition{
ActionCreate: actDef("create a provisioner daemon"),
ActionCreate: actDef("create a provisioner daemon key"),
// TODO: Move to use?
ActionRead: actDef("read provisioner daemon"),
ActionUpdate: actDef("update a provisioner daemon"),
ActionDelete: actDef("delete a provisioner daemon"),
ActionDelete: actDef("delete a provisioner daemon/key"),
},
},
"provisioner_jobs": {
Actions: map[Action]ActionDefinition{
ActionRead: actDef("read provisioner jobs"),
},
},
"provisioner_keys": {
Actions: map[Action]ActionDefinition{
ActionCreate: actDef("create a provisioner key"),
ActionRead: actDef("read provisioner keys"),
ActionDelete: actDef("delete a provisioner key"),
},
},
"organization": {
Actions: map[Action]ActionDefinition{
ActionCreate: actDef("create an organization"),
Expand Down
9 changes: 0 additions & 9 deletions coderd/rbac/roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,15 +555,6 @@ func TestRolePermissions(t *testing.T) {
false: {setOtherOrg, memberMe, userAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor},
},
},
{
Name: "ProvisionerKeys",
Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete},
Resource: rbac.ResourceProvisionerKeys.InOrg(orgID),
AuthorizeMap: map[bool][]hasAuthSubjects{
true: {owner, orgAdmin},
false: {setOtherOrg, memberMe, orgMemberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor},
},
},
{
Name: "ProvisionerJobs",
Actions: []policy.Action{policy.ActionRead},
Expand Down
2 changes: 0 additions & 2 deletions codersdk/rbacresources_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions docs/reference/api/members.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion docs/reference/api/schemas.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 24 additions & 3 deletions enterprise/coderd/roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,13 @@ func (api *API) putOrgRoles(rw http.ResponseWriter, r *http.Request) {
UUID: organization.ID,
Valid: true,
},
SitePermissions: db2sdk.List(req.SitePermissions, sdkPermissionToDB),
OrgPermissions: db2sdk.List(req.OrganizationPermissions, sdkPermissionToDB),
UserPermissions: db2sdk.List(req.UserPermissions, sdkPermissionToDB),
// Invalid permissions are filtered out. If this is changed
// to throw an error, then the story of a previously valid role
// now being invalid has to be addressed. Coder can change permissions,
// objects, and actions at any time.
SitePermissions: db2sdk.List(filterInvalidPermissions(req.SitePermissions), sdkPermissionToDB),
OrgPermissions: db2sdk.List(filterInvalidPermissions(req.OrganizationPermissions), sdkPermissionToDB),
UserPermissions: db2sdk.List(filterInvalidPermissions(req.UserPermissions), sdkPermissionToDB),
Comment on lines +154 to +156
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, custom roles with the old perm are unable to be updated.

})
if httpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
Expand Down Expand Up @@ -247,6 +251,23 @@ func (api *API) deleteOrgRole(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(ctx, rw, http.StatusNoContent, nil)
}

func filterInvalidPermissions(permissions []codersdk.Permission) []codersdk.Permission {
// Filter out any invalid permissions
var validPermissions []codersdk.Permission
for _, permission := range permissions {
err := rbac.Permission{
Negate: permission.Negate,
ResourceType: string(permission.ResourceType),
Action: policy.Action(permission.Action),
}.Valid()
if err != nil {
continue
}
validPermissions = append(validPermissions, permission)
}
return validPermissions
}

Comment on lines +254 to +270
Copy link
Member

@johnstcn johnstcn Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to filter these out? Is this in order to handle custom roles?
I'd rather have some warning of invalid permissions being in use if so.

EDIT: Now I see your above comment. This is legit, but we should probably have some way of surfacing this to admins so that they can update the role.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnstcn yea this is really unfortunate.

This is the continuous problem of "warnings" vs "errors", and our product does not support warnings very well.

I can make a follow up issue. Without this change, a stale role is a blocking thing that is impossible to fix via the UI. Custom roles unfortunately just have not been given the resources to come up with a proper UX yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue made here: #16632

func sdkPermissionToDB(p codersdk.Permission) database.CustomRolePermission {
return database.CustomRolePermission{
Negate: p.Negate,
Expand Down
9 changes: 2 additions & 7 deletions site/src/api/rbacresourcesGenerated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,14 @@ export const RBACResourceActions: Partial<
update: "update an organization member",
},
provisioner_daemon: {
create: "create a provisioner daemon",
delete: "delete a provisioner daemon",
create: "create a provisioner daemon key",
delete: "delete a provisioner daemon/key",
read: "read provisioner daemon",
update: "update a provisioner daemon",
},
provisioner_jobs: {
read: "read provisioner jobs",
},
provisioner_keys: {
create: "create a provisioner key",
delete: "delete a provisioner key",
read: "read provisioner keys",
},
replicas: {
read: "read replicas",
},
Expand Down
2 changes: 0 additions & 2 deletions site/src/api/typesGenerated.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions site/src/pages/UsersPage/storybookData/roles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,6 @@ export const MockRoles: (AssignableRoles | Role)[] = [
resource_type: "provisioner_daemon",
action: "*" as RBACAction,
},
{
negate: false,
resource_type: "provisioner_keys",
action: "*" as RBACAction,
},
{
negate: false,
resource_type: "replicas",
Expand Down
Loading