feat: Option to remove WorkspaceExec from owner role (#7050)

* chore: Add AllResources option for listing all RBAC objects
* Owners cannot do workspace exec site wide
* Fix FE authchecks to valid RBAC resources
This commit is contained in:
Steven Masley
2023-04-11 08:57:23 -05:00
committed by GitHub
parent ad2353c3d8
commit 9d39371ee0
28 changed files with 700 additions and 169 deletions

60
coderd/apidoc/docs.go generated
View File

@ -6290,7 +6290,11 @@ const docTemplate = `{
},
"resource_type": {
"description": "ResourceType is the name of the resource.\n` + "`" + `./coderd/rbac/object.go` + "`" + ` has the list of valid resource types.",
"type": "string"
"allOf": [
{
"$ref": "#/definitions/codersdk.RBACResource"
}
]
}
}
},
@ -6985,6 +6989,9 @@ const docTemplate = `{
"derp": {
"$ref": "#/definitions/codersdk.DERP"
},
"disable_owner_workspace_exec": {
"type": "boolean"
},
"disable_password_auth": {
"type": "boolean"
},
@ -8023,6 +8030,57 @@ const docTemplate = `{
}
}
},
"codersdk.RBACResource": {
"type": "string",
"enum": [
"workspace",
"workspace_proxy",
"workspace_execution",
"application_connect",
"audit_log",
"template",
"group",
"file",
"provisioner_daemon",
"organization",
"assign_role",
"assign_org_role",
"api_key",
"user",
"user_data",
"organization_member",
"license",
"deployment_config",
"deployment_stats",
"replicas",
"debug_info",
"system"
],
"x-enum-varnames": [
"ResourceWorkspace",
"ResourceWorkspaceProxy",
"ResourceWorkspaceExecution",
"ResourceWorkspaceApplicationConnect",
"ResourceAuditLog",
"ResourceTemplate",
"ResourceGroup",
"ResourceFile",
"ResourceProvisionerDaemon",
"ResourceOrganization",
"ResourceRoleAssignment",
"ResourceOrgRoleAssignment",
"ResourceAPIKey",
"ResourceUser",
"ResourceUserData",
"ResourceOrganizationMember",
"ResourceLicense",
"ResourceDeploymentValues",
"ResourceDeploymentStats",
"ResourceReplicas",
"ResourceDebugInfo",
"ResourceSystem"
]
},
"codersdk.RateLimitConfig": {
"type": "object",
"properties": {

View File

@ -5600,7 +5600,11 @@
},
"resource_type": {
"description": "ResourceType is the name of the resource.\n`./coderd/rbac/object.go` has the list of valid resource types.",
"type": "string"
"allOf": [
{
"$ref": "#/definitions/codersdk.RBACResource"
}
]
}
}
},
@ -6237,6 +6241,9 @@
"derp": {
"$ref": "#/definitions/codersdk.DERP"
},
"disable_owner_workspace_exec": {
"type": "boolean"
},
"disable_password_auth": {
"type": "boolean"
},
@ -7182,6 +7189,57 @@
}
}
},
"codersdk.RBACResource": {
"type": "string",
"enum": [
"workspace",
"workspace_proxy",
"workspace_execution",
"application_connect",
"audit_log",
"template",
"group",
"file",
"provisioner_daemon",
"organization",
"assign_role",
"assign_org_role",
"api_key",
"user",
"user_data",
"organization_member",
"license",
"deployment_config",
"deployment_stats",
"replicas",
"debug_info",
"system"
],
"x-enum-varnames": [
"ResourceWorkspace",
"ResourceWorkspaceProxy",
"ResourceWorkspaceExecution",
"ResourceWorkspaceApplicationConnect",
"ResourceAuditLog",
"ResourceTemplate",
"ResourceGroup",
"ResourceFile",
"ResourceProvisionerDaemon",
"ResourceOrganization",
"ResourceRoleAssignment",
"ResourceOrgRoleAssignment",
"ResourceAPIKey",
"ResourceUser",
"ResourceUserData",
"ResourceOrganizationMember",
"ResourceLicense",
"ResourceDeploymentValues",
"ResourceDeploymentStats",
"ResourceReplicas",
"ResourceDebugInfo",
"ResourceSystem"
]
},
"codersdk.RateLimitConfig": {
"type": "object",
"properties": {

View File

@ -168,7 +168,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
obj := rbac.Object{
Owner: v.Object.OwnerID,
OrgID: v.Object.OrganizationID,
Type: v.Object.ResourceType,
Type: v.Object.ResourceType.String(),
}
if obj.Owner == "me" {
obj.Owner = auth.Actor.ID
@ -188,7 +188,7 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
var dbObj rbac.Objecter
var dbErr error
// Only support referencing some resources by ID.
switch v.Object.ResourceType {
switch v.Object.ResourceType.String() {
case rbac.ResourceWorkspaceExecution.Type:
wrkSpace, err := api.Database.GetWorkspaceByID(ctx, id)
if err == nil {

View File

@ -46,34 +46,34 @@ func TestCheckPermissions(t *testing.T) {
params := map[string]codersdk.AuthorizationCheck{
readAllUsers: {
Object: codersdk.AuthorizationObject{
ResourceType: "users",
ResourceType: codersdk.ResourceUser,
},
Action: "read",
},
readMyself: {
Object: codersdk.AuthorizationObject{
ResourceType: "users",
ResourceType: codersdk.ResourceUser,
OwnerID: "me",
},
Action: "read",
},
readOwnWorkspaces: {
Object: codersdk.AuthorizationObject{
ResourceType: "workspaces",
ResourceType: codersdk.ResourceWorkspace,
OwnerID: "me",
},
Action: "read",
},
readOrgWorkspaces: {
Object: codersdk.AuthorizationObject{
ResourceType: "workspaces",
ResourceType: codersdk.ResourceWorkspace,
OrganizationID: adminUser.OrganizationID.String(),
},
Action: "read",
},
updateSpecificTemplate: {
Object: codersdk.AuthorizationObject{
ResourceType: rbac.ResourceTemplate.Type,
ResourceType: codersdk.ResourceTemplate,
ResourceID: template.ID.String(),
},
Action: "update",
@ -103,7 +103,7 @@ func TestCheckPermissions(t *testing.T) {
Client: orgAdminClient,
UserID: orgAdminUser.ID,
Check: map[string]bool{
readAllUsers: false,
readAllUsers: true,
readMyself: true,
readOwnWorkspaces: true,
readOrgWorkspaces: true,
@ -115,7 +115,7 @@ func TestCheckPermissions(t *testing.T) {
Client: memberClient,
UserID: memberUser.ID,
Check: map[string]bool{
readAllUsers: false,
readAllUsers: true,
readMyself: true,
readOwnWorkspaces: true,
readOrgWorkspaces: false,

View File

@ -171,6 +171,12 @@ func New(options *Options) *API {
options = &Options{}
}
if options.DeploymentValues.DisableOwnerWorkspaceExec {
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{
NoOwnerWorkspaceExec: true,
})
}
if options.Authorizer == nil {
options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
}

View File

@ -203,6 +203,8 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
if options.DeploymentValues == nil {
options.DeploymentValues = DeploymentValues(t)
}
// This value is not safe to run in parallel. Force it to be false.
options.DeploymentValues.DisableOwnerWorkspaceExec = false
// If no ratelimits are set, disable all rate limiting for tests.
if options.APIRateLimit == 0 {

View File

@ -14,6 +14,12 @@ type Objecter interface {
// Resources are just typed objects. Making resources this way allows directly
// passing them into an Authorize function and use the chaining api.
var (
// ResourceWildcard represents all resource types
// Try to avoid using this where possible.
ResourceWildcard = Object{
Type: WildcardSymbol,
}
// ResourceWorkspace CRUD. Org + User owner
// create/delete = make or delete workspaces
// read = access workspace
@ -136,11 +142,6 @@ var (
Type: "organization_member",
}
// ResourceWildcard represents all resource types
ResourceWildcard = Object{
Type: WildcardSymbol,
}
// ResourceLicense is the license in the 'licenses' table.
// ResourceLicense is site wide.
// create/delete = add or remove license from site.

30
coderd/rbac/object_gen.go Normal file
View File

@ -0,0 +1,30 @@
// Code generated by rbacgen/main.go. DO NOT EDIT.
package rbac
func AllResources() []Object {
return []Object{
ResourceAPIKey,
ResourceAuditLog,
ResourceDebugInfo,
ResourceDeploymentStats,
ResourceDeploymentValues,
ResourceFile,
ResourceGroup,
ResourceLicense,
ResourceOrgRoleAssignment,
ResourceOrganization,
ResourceOrganizationMember,
ResourceProvisionerDaemon,
ResourceReplicas,
ResourceRoleAssignment,
ResourceSystem,
ResourceTemplate,
ResourceUser,
ResourceUserData,
ResourceWildcard,
ResourceWorkspace,
ResourceWorkspaceApplicationConnect,
ResourceWorkspaceExecution,
ResourceWorkspaceProxy,
}
}

View File

@ -4,6 +4,7 @@ import (
"testing"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/util/slice"
)
func TestObjectEqual(t *testing.T) {
@ -174,3 +175,22 @@ func TestObjectEqual(t *testing.T) {
})
}
}
// TestAllResources ensures that all resources have a unique type name.
func TestAllResources(t *testing.T) {
t.Parallel()
var typeNames []string
resources := rbac.AllResources()
for _, r := range resources {
if r.Type == "" {
t.Errorf("empty type name: %s", r.Type)
continue
}
if slice.Contains(typeNames, r.Type) {
t.Errorf("duplicate type name: %s", r.Type)
continue
}
typeNames = append(typeNames, r.Type)
}
}

View File

@ -20,6 +20,11 @@ const (
orgMember string = "organization-member"
)
func init() {
// Always load defaults
ReloadBuiltinRoles(nil)
}
// RoleNames is a list of user assignable role names. The role names must be
// in the builtInRoles map. Any non-user assignable roles will generate an
// error on Expand.
@ -62,6 +67,33 @@ func RoleOrgMember(organizationID uuid.UUID) string {
return roleName(orgMember, organizationID.String())
}
func allPermsExcept(excepts ...Object) []Permission {
resources := AllResources()
var perms []Permission
skip := make(map[string]bool)
for _, e := range excepts {
skip[e.Type] = true
}
for _, r := range resources {
// Exceptions
if skip[r.Type] {
continue
}
// This should always be skipped.
if r.Type == ResourceWildcard.Type {
continue
}
// Owners can do everything else
perms = append(perms, Permission{
Negate: false,
ResourceType: r.Type,
Action: WildcardSymbol,
})
}
return perms
}
// builtInRoles are just a hard coded set for now. Ideally we store these in
// the database. Right now they are functions because the org id should scope
// certain roles. When we store them in the database, each organization should
@ -70,145 +102,163 @@ func RoleOrgMember(organizationID uuid.UUID) string {
//
// This map will be replaced by database storage defined by this ticket.
// https://github.com/coder/coder/issues/1194
var builtInRoles = map[string]func(orgID string) Role{
// admin grants all actions to all resources.
owner: func(_ string) Role {
return Role{
Name: owner,
DisplayName: "Owner",
Site: Permissions(map[string][]Action{
ResourceWildcard.Type: {WildcardSymbol},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
var builtInRoles map[string]func(orgID string) Role
// member grants all actions to all resources owned by the user
member: func(_ string) Role {
return Role{
Name: member,
DisplayName: "",
Site: Permissions(map[string][]Action{
// All users can read all other users and know they exist.
ResourceUser.Type: {ActionRead},
ResourceRoleAssignment.Type: {ActionRead},
// All users can see the provisioner daemons.
ResourceProvisionerDaemon.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: Permissions(map[string][]Action{
ResourceWildcard.Type: {WildcardSymbol},
}),
}
},
type RoleOptions struct {
NoOwnerWorkspaceExec bool
}
// auditor provides all permissions required to effectively read and understand
// audit log events.
// TODO: Finish the auditor as we add resources.
auditor: func(_ string) Role {
return Role{
Name: auditor,
DisplayName: "Auditor",
Site: Permissions(map[string][]Action{
// Should be able to read all template details, even in orgs they
// are not in.
ResourceTemplate.Type: {ActionRead},
ResourceAuditLog.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
// ReloadBuiltinRoles loads the static roles into the builtInRoles map.
// This can be called again with a different config to change the behavior.
//
// TODO: @emyrk This would be great if it was instanced to a coderd rather
// than a global. But that is a much larger refactor right now.
// Essentially we did not foresee different deployments needing slightly
// different role permissions.
func ReloadBuiltinRoles(opts *RoleOptions) {
if opts == nil {
opts = &RoleOptions{}
}
templateAdmin: func(_ string) Role {
return Role{
Name: templateAdmin,
DisplayName: "Template Admin",
Site: Permissions(map[string][]Action{
ResourceTemplate.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// CRUD all files, even those they did not upload.
ResourceFile.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceWorkspace.Type: {ActionRead},
// CRUD to provisioner daemons for now.
ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// Needs to read all organizations since
ResourceOrganization.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
var ownerAndAdminExceptions []Object
if opts.NoOwnerWorkspaceExec {
ownerAndAdminExceptions = append(ownerAndAdminExceptions,
ResourceWorkspaceExecution,
ResourceWorkspaceApplicationConnect,
)
}
userAdmin: func(_ string) Role {
return Role{
Name: userAdmin,
DisplayName: "User Admin",
Site: Permissions(map[string][]Action{
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// Full perms to manage org members
ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
builtInRoles = map[string]func(orgID string) Role{
// admin grants all actions to all resources.
owner: func(_ string) Role {
return Role{
Name: owner,
DisplayName: "Owner",
Site: allPermsExcept(ownerAndAdminExceptions...),
Org: map[string][]Permission{},
User: []Permission{},
}
},
// orgAdmin returns a role with all actions allows in a given
// organization scope.
orgAdmin: func(organizationID string) Role {
return Role{
Name: roleName(orgAdmin, organizationID),
DisplayName: "Organization Admin",
Site: []Permission{},
Org: map[string][]Permission{
organizationID: {
{
Negate: false,
ResourceType: "*",
Action: "*",
// member grants all actions to all resources owned by the user
member: func(_ string) Role {
return Role{
Name: member,
DisplayName: "",
Site: Permissions(map[string][]Action{
// All users can read all other users and know they exist.
ResourceUser.Type: {ActionRead},
ResourceRoleAssignment.Type: {ActionRead},
// All users can see the provisioner daemons.
ResourceProvisionerDaemon.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: allPermsExcept(),
}
},
// auditor provides all permissions required to effectively read and understand
// audit log events.
// TODO: Finish the auditor as we add resources.
auditor: func(_ string) Role {
return Role{
Name: auditor,
DisplayName: "Auditor",
Site: Permissions(map[string][]Action{
// Should be able to read all template details, even in orgs they
// are not in.
ResourceTemplate.Type: {ActionRead},
ResourceAuditLog.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
templateAdmin: func(_ string) Role {
return Role{
Name: templateAdmin,
DisplayName: "Template Admin",
Site: Permissions(map[string][]Action{
ResourceTemplate.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// CRUD all files, even those they did not upload.
ResourceFile.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceWorkspace.Type: {ActionRead},
// CRUD to provisioner daemons for now.
ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// Needs to read all organizations since
ResourceOrganization.Type: {ActionRead},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
userAdmin: func(_ string) Role {
return Role{
Name: userAdmin,
DisplayName: "User Admin",
Site: Permissions(map[string][]Action{
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// Full perms to manage org members
ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
}),
Org: map[string][]Permission{},
User: []Permission{},
}
},
// orgAdmin returns a role with all actions allows in a given
// organization scope.
orgAdmin: func(organizationID string) Role {
return Role{
Name: roleName(orgAdmin, organizationID),
DisplayName: "Organization Admin",
Site: []Permission{},
Org: map[string][]Permission{
// Org admins should not have workspace exec perms.
organizationID: allPermsExcept(ResourceWorkspaceExecution),
},
User: []Permission{},
}
},
// orgMember has an empty set of permissions, this just implies their membership
// in an organization.
orgMember: func(organizationID string) Role {
return Role{
Name: roleName(orgMember, organizationID),
DisplayName: "",
Site: []Permission{},
Org: map[string][]Permission{
organizationID: {
{
// All org members can read the other members in their org.
ResourceType: ResourceOrganizationMember.Type,
Action: ActionRead,
},
{
// All org members can read the organization
ResourceType: ResourceOrganization.Type,
Action: ActionRead,
},
{
// Can read available roles.
ResourceType: ResourceOrgRoleAssignment.Type,
Action: ActionRead,
},
{
ResourceType: ResourceGroup.Type,
Action: ActionRead,
},
},
},
},
User: []Permission{},
}
},
// orgMember has an empty set of permissions, this just implies their membership
// in an organization.
orgMember: func(organizationID string) Role {
return Role{
Name: roleName(orgMember, organizationID),
DisplayName: "",
Site: []Permission{},
Org: map[string][]Permission{
organizationID: {
{
// All org members can read the other members in their org.
ResourceType: ResourceOrganizationMember.Type,
Action: ActionRead,
},
{
// All org members can read the organization
ResourceType: ResourceOrganization.Type,
Action: ActionRead,
},
{
// Can read available roles.
ResourceType: ResourceOrgRoleAssignment.Type,
Action: ActionRead,
},
{
ResourceType: ResourceGroup.Type,
Action: ActionRead,
},
},
},
User: []Permission{},
}
},
User: []Permission{},
}
},
}
}
// assignRoles is a map of roles that can be assigned if a user has a given

View File

@ -19,6 +19,42 @@ type authSubject struct {
Actor rbac.Subject
}
//nolint:tparallel,paralleltest
func TestOwnerExec(t *testing.T) {
owner := rbac.Subject{
ID: uuid.NewString(),
Roles: rbac.RoleNames{rbac.RoleMember(), rbac.RoleOwner()},
Scope: rbac.ScopeAll,
}
t.Run("NoExec", func(t *testing.T) {
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{
NoOwnerWorkspaceExec: true,
})
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
auth := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
// Exec a random workspace
err := auth.Authorize(context.Background(), owner, rbac.ActionCreate,
rbac.ResourceWorkspaceExecution.WithID(uuid.New()).InOrg(uuid.New()).WithOwner(uuid.NewString()))
require.ErrorAsf(t, err, &rbac.UnauthorizedError{}, "expected unauthorized error")
})
t.Run("Exec", func(t *testing.T) {
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{
NoOwnerWorkspaceExec: false,
})
t.Cleanup(func() { rbac.ReloadBuiltinRoles(nil) })
auth := rbac.NewCachingAuthorizer(prometheus.NewRegistry())
// Exec a random workspace
err := auth.Authorize(context.Background(), owner, rbac.ActionCreate,
rbac.ResourceWorkspaceExecution.WithID(uuid.New()).InOrg(uuid.New()).WithOwner(uuid.NewString()))
require.NoError(t, err, "expected owner can")
})
}
// TODO: add the SYSTEM to the MATRIX
func TestRolePermissions(t *testing.T) {
t.Parallel()
@ -111,8 +147,8 @@ func TestRolePermissions(t *testing.T) {
Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete},
Resource: rbac.ResourceWorkspaceExecution.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()),
AuthorizeMap: map[bool][]authSubject{
true: {owner, orgAdmin, orgMemberMe},
false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
true: {owner, orgMemberMe},
false: {orgAdmin, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin},
},
},
{