From 8a8c8952f24c6dca2c40c1f8d9cf863b0b3eb2a8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 12:16:51 -0500 Subject: [PATCH 01/18] chore: Add AllResources option for listing all RBAC objects --- Makefile | 5 ++ scripts/rbacgen/main.go | 91 +++++++++++++++++++++++++++++++++++ scripts/rbacgen/object.gotmpl | 12 +++++ 3 files changed, 108 insertions(+) create mode 100644 scripts/rbacgen/main.go create mode 100644 scripts/rbacgen/object.gotmpl diff --git a/Makefile b/Makefile index 5162d358a06b9..bcdc4c89923f8 100644 --- a/Makefile +++ b/Makefile @@ -423,6 +423,7 @@ gen: \ provisionersdk/proto/provisioner.pb.go \ provisionerd/proto/provisionerd.pb.go \ site/src/api/typesGenerated.ts \ + coderd/rbac/object_gen.go \ docs/admin/prometheus.md \ docs/cli.md \ docs/admin/audit-logs.md \ @@ -443,6 +444,7 @@ gen/mark-fresh: provisionersdk/proto/provisioner.pb.go \ provisionerd/proto/provisionerd.pb.go \ site/src/api/typesGenerated.ts \ + coderd/rbac/object_gen.go \ docs/admin/prometheus.md \ docs/cli.md \ docs/admin/audit-logs.md \ @@ -495,6 +497,9 @@ site/src/api/typesGenerated.ts: scripts/apitypings/main.go $(shell find ./coders cd site yarn run format:types +coderd/rbac/object_gen.go: scripts/rbacgen/main.go coderd/rbac/object.go + go run scripts/rbacgen/main.go ./coderd/rbac > coderd/rbac/object_gen.go + docs/admin/prometheus.md: scripts/metricsdocgen/main.go scripts/metricsdocgen/metrics go run scripts/metricsdocgen/main.go cd site diff --git a/scripts/rbacgen/main.go b/scripts/rbacgen/main.go new file mode 100644 index 0000000000000..c7ea65f778b47 --- /dev/null +++ b/scripts/rbacgen/main.go @@ -0,0 +1,91 @@ +package main + +import ( + "bytes" + "context" + _ "embed" + "fmt" + "go/format" + "go/types" + "html/template" + "log" + "os" + "sort" + + "golang.org/x/tools/go/packages" +) + +//go:embed object.gotmpl +var objectGoTpl string + +type TplState struct { + ResourceNames []string +} + +// main will generate a file that lists all rbac objects. +// This is to provide an "AllResources" function that is always +// in sync. +func main() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + path := "." + if len(os.Args) > 1 { + path = os.Args[1] + } + + cfg := &packages.Config{ + Mode: packages.NeedTypes | packages.NeedName | packages.NeedTypesInfo | packages.NeedDeps, + Tests: false, + Context: ctx, + } + + pkgs, err := packages.Load(cfg, path) + if err != nil { + log.Fatalf("Failed to load package: %s", err.Error()) + } + + if len(pkgs) != 1 { + log.Fatalf("Expected 1 package, got %d", len(pkgs)) + } + + rbacPkg := pkgs[0] + if rbacPkg.Name != "rbac" { + log.Fatalf("Expected rbac package, got %q", rbacPkg.Name) + } + + tpl, err := template.New("object.gotmpl").Parse(objectGoTpl) + if err != nil { + log.Fatalf("Failed to parse templates: %s", err.Error()) + } + + var out bytes.Buffer + err = tpl.Execute(&out, TplState{ + ResourceNames: allResources(rbacPkg), + }) + + if err != nil { + log.Fatalf("Execute template: %s", err.Error()) + } + + formatted, err := format.Source(out.Bytes()) + if err != nil { + log.Fatalf("Format template: %s", err.Error()) + } + + _, _ = fmt.Fprint(os.Stdout, string(formatted)) +} + +func allResources(pkg *packages.Package) []string { + var resources []string + names := pkg.Types.Scope().Names() + for _, name := range names { + obj, ok := pkg.Types.Scope().Lookup(name).(*types.Var) + if ok && obj.Type().String() == "github.com/coder/coder/coderd/rbac.Object" { + resources = append(resources, obj.Name()) + } + + } + sort.Strings(resources) + return resources +} diff --git a/scripts/rbacgen/object.gotmpl b/scripts/rbacgen/object.gotmpl new file mode 100644 index 0000000000000..281acbc581925 --- /dev/null +++ b/scripts/rbacgen/object.gotmpl @@ -0,0 +1,12 @@ +// Code generated by rbacgen/main.go. DO NOT EDIT. +package rbac + +func AllResources() []Object { + return []Object{ + {{- range .ResourceNames }} + {{ . }}, + {{- end }} + } +} + + From 9113b16a474d9f9ad3c02e84c2467fcfa5b42206 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 12:23:50 -0500 Subject: [PATCH 02/18] Owners cannot do workspace exec site wide --- coderd/rbac/object_gen.go | 30 ++++++++++++++++++++++++++++++ coderd/rbac/object_test.go | 18 ++++++++++++++++++ coderd/rbac/roles.go | 23 ++++++++++++++++++++--- 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 coderd/rbac/object_gen.go diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go new file mode 100644 index 0000000000000..9af80010cf753 --- /dev/null +++ b/coderd/rbac/object_gen.go @@ -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, + } +} diff --git a/coderd/rbac/object_test.go b/coderd/rbac/object_test.go index 386a1e98f5477..3203ced4c3aee 100644 --- a/coderd/rbac/object_test.go +++ b/coderd/rbac/object_test.go @@ -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,20 @@ func TestObjectEqual(t *testing.T) { }) } } + +// TestAllResources ensures that all resources have a unique type name. +func TestAllResources(t *testing.T) { + 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) + } +} diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index f5b5736db0f72..10e65ce0d27e8 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -76,9 +76,26 @@ var builtInRoles = map[string]func(orgID string) Role{ return Role{ Name: owner, DisplayName: "Owner", - Site: Permissions(map[string][]Action{ - ResourceWildcard.Type: {WildcardSymbol}, - }), + Site: func() []Permission { + // Owner can do all actions on all resources, minus some exceptions. + resources := AllResources() + var perms []Permission + + for _, r := range resources { + // Exceptions + if r.Equal(ResourceWildcard) || + r.Equal(ResourceWorkspaceExecution) { + continue + } + // Owners can do everything else + perms = append(perms, Permission{ + Negate: false, + ResourceType: r.Type, + Action: WildcardSymbol, + }) + } + return perms + }(), Org: map[string][]Permission{}, User: []Permission{}, } From 57cb9b75c0d4e472b1d2e7036318cc5cdea538d5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 13:25:51 -0500 Subject: [PATCH 03/18] Rena,e --- coderd/coderd.go | 4 + coderd/rbac/roles.go | 330 ++++++++++++++++++++++++------------------- 2 files changed, 185 insertions(+), 149 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index afc87b20bd73e..304ba91991bb5 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -171,6 +171,10 @@ func New(options *Options) *API { options = &Options{} } + rbac.ReloadBuiltinRoles(&rbac.RoleOptions{ + NoOwnerWorkspaceExec: true, + }) + if options.Authorizer == nil { options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry) } diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 10e65ce0d27e8..d72cfe11a3599 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -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 + } + // Do not include the wildcard + 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,162 +102,162 @@ 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: func() []Permission { - // Owner can do all actions on all resources, minus some exceptions. - resources := AllResources() - var perms []Permission - - for _, r := range resources { - // Exceptions - if r.Equal(ResourceWildcard) || - r.Equal(ResourceWorkspaceExecution) { - continue - } - // Owners can do everything else - perms = append(perms, Permission{ - Negate: false, - ResourceType: r.Type, - Action: WildcardSymbol, - }) - } - return perms - }(), - 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}, - }), - } - }, - - // 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{}, - } - }, +type RoleOptions struct { + NoOwnerWorkspaceExec bool +} - 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{}, - } - }, +// 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{} + } - 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{}, - } - }, + var ownerAndAdminExceptions []Object + if opts.NoOwnerWorkspaceExec { + ownerAndAdminExceptions = append(ownerAndAdminExceptions, ResourceWorkspaceExecution) + } - // 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: "*", - }, + 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{}, + } + }, + + // 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}, + }), + } + }, + + // 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 From 659800c1f8fc31f0f08d61547c6779394fc3ca16 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 13:35:11 -0500 Subject: [PATCH 04/18] Add deployment config --- coderd/coderd.go | 8 +++++--- coderd/coderdtest/coderdtest.go | 2 ++ coderd/rbac/object.go | 5 ----- coderd/rbac/roles.go | 10 ++-------- codersdk/deployment.go | 10 ++++++++++ 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 304ba91991bb5..48b97a98d5c13 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -171,9 +171,11 @@ func New(options *Options) *API { options = &Options{} } - rbac.ReloadBuiltinRoles(&rbac.RoleOptions{ - NoOwnerWorkspaceExec: true, - }) + if options.DeploymentValues.DisableOwnerWorkspaceExec { + rbac.ReloadBuiltinRoles(&rbac.RoleOptions{ + NoOwnerWorkspaceExec: true, + }) + } if options.Authorizer == nil { options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 91f7948407cab..dca66b7eba23d 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -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 { diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index ead5fad556a9e..01ffa96cb0b32 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -136,11 +136,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. diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index d72cfe11a3599..534e737fd787e 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -80,10 +80,6 @@ func allPermsExcept(excepts ...Object) []Permission { if skip[r.Type] { continue } - // Do not include the wildcard - if r.Type == ResourceWildcard.Type { - continue - } // Owners can do everything else perms = append(perms, Permission{ Negate: false, @@ -149,10 +145,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // All users can see the provisioner daemons. ResourceProvisionerDaemon.Type: {ActionRead}, }), - Org: map[string][]Permission{}, - User: Permissions(map[string][]Action{ - ResourceWildcard.Type: {WildcardSymbol}, - }), + Org: map[string][]Permission{}, + User: allPermsExcept(), } }, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 8f9537729d013..1ee2db9710a50 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -162,6 +162,7 @@ type DeploymentValues struct { GitAuthProviders clibase.Struct[[]GitAuthConfig] `json:"git_auth,omitempty" typescript:",notnull"` SSHConfig SSHConfig `json:"config_ssh,omitempty" typescript:",notnull"` WgtunnelHost clibase.String `json:"wgtunnel_host,omitempty" typescript:",notnull"` + DisableOwnerWorkspaceExec clibase.Bool `json:"disable_owner_workspace_exec,omitempty" typescript:",notnull"` Config clibase.String `json:"config,omitempty" typescript:",notnull"` WriteConfig clibase.Bool `json:"write_config,omitempty" typescript:",notnull"` @@ -1302,6 +1303,15 @@ when required by your organization's security policy.`, Value: &c.DisablePathApps, YAML: "disablePathApps", }, + { + Name: "Disable Owner Workspace Execution", + Description: "Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user permissions to access their own workspaces.", + Flag: "disable-owner-workspace-exec", + Env: "CODER_DISABLE_OWNER_WORKSPACE_EXEC", + + Value: &c.DisableOwnerWorkspaceExec, + YAML: "disableOwnerWorkspaceExec", + }, { Name: "Session Duration", Description: "The token expiry duration for browser sessions. Sessions may last longer if they are actively making requests, but this functionality can be disabled via --disable-session-expiry-refresh.", From e6970e1908fb254a8ab6b35532710e0880561e52 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 14:03:58 -0500 Subject: [PATCH 05/18] Add rbac resource enums --- coderd/rbac/object.go | 6 ++++ codersdk/rbacresources.go | 28 +++++++++++++++++++ site/src/api/typesGenerated.ts | 1 + .../SecuritySettingsPageView.tsx | 1 + 4 files changed, 36 insertions(+) create mode 100644 codersdk/rbacresources.go diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 01ffa96cb0b32..e867abfb69685 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -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 diff --git a/codersdk/rbacresources.go b/codersdk/rbacresources.go new file mode 100644 index 0000000000000..d6fb32788adb4 --- /dev/null +++ b/codersdk/rbacresources.go @@ -0,0 +1,28 @@ +package codersdk + +type RBACResource string + +const ( + ResourceWorkspace RBACResource = "workspace" + ResourceWorkspaceProxy RBACResource = "workspace_proxy" + ResourceWorkspaceExecution RBACResource = "workspace_execution" + ResourceWorkspaceApplicationConnect RBACResource = "application_connect" + ResourceAuditLog RBACResource = "audit_log" + ResourceTemplate RBACResource = "template" + ResourceGroup RBACResource = "group" + ResourceFile RBACResource = "file" + ResourceProvisionerDaemon RBACResource = "provisioner_daemon" + ResourceOrganization RBACResource = "organization" + ResourceRoleAssignment RBACResource = "assign_role" + ResourceOrgRoleAssignment RBACResource = "assign_org_role" + ResourceAPIKey RBACResource = "api_key" + ResourceUser RBACResource = "user" + ResourceUserData RBACResource = "user_data" + ResourceOrganizationMember RBACResource = "organization_member" + ResourceLicense RBACResource = "license" + ResourceDeploymentValues RBACResource = "deployment_config" + ResourceDeploymentStats RBACResource = "deployment_stats" + ResourceReplicas RBACResource = "replicas" + ResourceDebugInfo RBACResource = "debug_info" + ResourceSystem RBACResource = "system" +) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 22a0b2626fd99..527d049a089db 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -379,6 +379,7 @@ export interface DeploymentValues { readonly git_auth?: any readonly config_ssh?: SSHConfig readonly wgtunnel_host?: string + readonly disable_owner_workspace_exec?: boolean readonly config?: string readonly write_config?: boolean // Named type "github.com/coder/coder/cli/clibase.HostPort" unknown, using "any" diff --git a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx index bb80e5de87e95..fabc8027032f6 100644 --- a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx @@ -36,6 +36,7 @@ export const SecuritySettingsPageView = ({ options, "SSH Keygen Algorithm", "Secure Auth Cookie", + "Disable Owner Workspace Execution" )} /> From b53035892f17c5c03f1e9f47ccfe6d7b53d74413 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 14:14:12 -0500 Subject: [PATCH 06/18] Fix FE authchecks to valid RBAC resources --- coderd/apidoc/docs.go | 60 ++++++++++++++++++++++++- coderd/apidoc/swagger.json | 60 ++++++++++++++++++++++++- coderd/authorize.go | 4 +- codersdk/authorization.go | 2 +- codersdk/rbacresources.go | 4 ++ docs/api/authorization.md | 4 +- docs/api/general.md | 1 + docs/api/schemas.md | 58 +++++++++++++++++++----- docs/cli/server.md | 9 ++++ site/src/api/typesGenerated.ts | 51 ++++++++++++++++++++- site/src/xServices/auth/authXService.ts | 4 +- 11 files changed, 237 insertions(+), 20 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8a7001ac9f14e..cb52911df84e8 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6270,7 +6270,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" + } + ] } } }, @@ -6965,6 +6969,9 @@ const docTemplate = `{ "derp": { "$ref": "#/definitions/codersdk.DERP" }, + "disable_owner_workspace_exec": { + "type": "boolean" + }, "disable_password_auth": { "type": "boolean" }, @@ -8003,6 +8010,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": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index df9e2cf608f8b..f5ba0033026ac 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5586,7 +5586,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" + } + ] } } }, @@ -6223,6 +6227,9 @@ "derp": { "$ref": "#/definitions/codersdk.DERP" }, + "disable_owner_workspace_exec": { + "type": "boolean" + }, "disable_password_auth": { "type": "boolean" }, @@ -7168,6 +7175,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": { diff --git a/coderd/authorize.go b/coderd/authorize.go index ab1f3a39fd542..670e284af8d1f 100644 --- a/coderd/authorize.go +++ b/coderd/authorize.go @@ -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 { diff --git a/codersdk/authorization.go b/codersdk/authorization.go index 2f4365e79396b..4e8a6eed7019f 100644 --- a/codersdk/authorization.go +++ b/codersdk/authorization.go @@ -43,7 +43,7 @@ type AuthorizationCheck struct { type AuthorizationObject struct { // ResourceType is the name of the resource. // `./coderd/rbac/object.go` has the list of valid resource types. - ResourceType string `json:"resource_type"` + ResourceType RBACResource `json:"resource_type"` // OwnerID (optional) adds the set constraint to all resources owned by a given user. OwnerID string `json:"owner_id,omitempty"` // OrganizationID (optional) adds the set constraint to all resources owned by a given organization. diff --git a/codersdk/rbacresources.go b/codersdk/rbacresources.go index d6fb32788adb4..7db5fc0ec1c76 100644 --- a/codersdk/rbacresources.go +++ b/codersdk/rbacresources.go @@ -26,3 +26,7 @@ const ( ResourceDebugInfo RBACResource = "debug_info" ResourceSystem RBACResource = "system" ) + +func (r RBACResource) String() string { + return string(r) +} diff --git a/docs/api/authorization.md b/docs/api/authorization.md index a4cdec1659204..a75a477656224 100644 --- a/docs/api/authorization.md +++ b/docs/api/authorization.md @@ -25,7 +25,7 @@ curl -X POST http://coder-server:8080/api/v2/authcheck \ "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } }, "property2": { @@ -34,7 +34,7 @@ curl -X POST http://coder-server:8080/api/v2/authcheck \ "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } } } diff --git a/docs/api/general.md b/docs/api/general.md index 0fbaf9eb3647e..b07a1688bffd4 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -188,6 +188,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "stun_addresses": ["string"] } }, + "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, "disable_session_expiry_refresh": true, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 549bccd9425d8..d39853e57b32c 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1039,7 +1039,7 @@ "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } } ``` @@ -1069,7 +1069,7 @@ AuthorizationCheck is used to check if the currently authenticated user (or the "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } ``` @@ -1077,12 +1077,12 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | ------ | -------- | ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `organization_id` | string | false | | Organization ID (optional) adds the set constraint to all resources owned by a given organization. | -| `owner_id` | string | false | | Owner ID (optional) adds the set constraint to all resources owned by a given user. | -| `resource_id` | string | false | | Resource ID (optional) reduces the set to a singular resource. This assigns a resource ID to the resource type, eg: a single workspace. The rbac library will not fetch the resource from the database, so if you are using this option, you should also set the owner ID and organization ID if possible. Be as specific as possible using all the fields relevant. | -| `resource_type` | string | false | | Resource type is the name of the resource. `./coderd/rbac/object.go` has the list of valid resource types. | +| Name | Type | Required | Restrictions | Description | +| ----------------- | ---------------------------------------------- | -------- | ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `organization_id` | string | false | | Organization ID (optional) adds the set constraint to all resources owned by a given organization. | +| `owner_id` | string | false | | Owner ID (optional) adds the set constraint to all resources owned by a given user. | +| `resource_id` | string | false | | Resource ID (optional) reduces the set to a singular resource. This assigns a resource ID to the resource type, eg: a single workspace. The rbac library will not fetch the resource from the database, so if you are using this option, you should also set the owner ID and organization ID if possible. Be as specific as possible using all the fields relevant. | +| `resource_type` | [codersdk.RBACResource](#codersdkrbacresource) | false | | Resource type is the name of the resource. `./coderd/rbac/object.go` has the list of valid resource types. | ## codersdk.AuthorizationRequest @@ -1095,7 +1095,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } }, "property2": { @@ -1104,7 +1104,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "organization_id": "string", "owner_id": "string", "resource_id": "string", - "resource_type": "string" + "resource_type": "workspace" } } } @@ -1814,6 +1814,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "stun_addresses": ["string"] } }, + "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, "disable_session_expiry_refresh": true, @@ -2162,6 +2163,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a "stun_addresses": ["string"] } }, + "disable_owner_workspace_exec": true, "disable_password_auth": true, "disable_path_apps": true, "disable_session_expiry_refresh": true, @@ -2350,6 +2352,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a | `config_ssh` | [codersdk.SSHConfig](#codersdksshconfig) | false | | | | `dangerous` | [codersdk.DangerousConfig](#codersdkdangerousconfig) | false | | | | `derp` | [codersdk.DERP](#codersdkderp) | false | | | +| `disable_owner_workspace_exec` | boolean | false | | | | `disable_password_auth` | boolean | false | | | | `disable_path_apps` | boolean | false | | | | `disable_session_expiry_refresh` | boolean | false | | | @@ -3362,6 +3365,41 @@ Parameter represents a set value for the scope. | ---------- | ------ | -------- | ------------ | ----------- | | `deadline` | string | true | | | +## codersdk.RBACResource + +```json +"workspace" +``` + +### Properties + +#### Enumerated Values + +| Value | +| --------------------- | +| `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` | + ## codersdk.RateLimitConfig ```json diff --git a/docs/cli/server.md b/docs/cli/server.md index 26b7e3a4e9d9d..3572954d4b024 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -143,6 +143,15 @@ An HTTP URL that is accessible by other replicas to relay DERP traffic. Required Addresses for STUN servers to establish P2P connections. Set empty to disable P2P connections. +### --disable-owner-workspace-exec + +| | | +| ----------- | ------------------------------------------------ | +| Type | bool | +| Environment | $CODER_DISABLE_OWNER_WORKSPACE_EXEC | + +Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user permissions to access their own workspaces. + ### --disable-password-auth | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 527d049a089db..4cd2c8470575e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -117,7 +117,7 @@ export interface AuthorizationCheck { // From codersdk/authorization.go export interface AuthorizationObject { - readonly resource_type: string + readonly resource_type: RBACResource readonly owner_id?: string readonly organization_id?: string readonly resource_id?: string @@ -1420,6 +1420,55 @@ export const ProvisionerStorageMethods: ProvisionerStorageMethod[] = ["file"] export type ProvisionerType = "echo" | "terraform" export const ProvisionerTypes: ProvisionerType[] = ["echo", "terraform"] +// From codersdk/rbacresources.go +export type RBACResource = + | "api_key" + | "application_connect" + | "assign_org_role" + | "assign_role" + | "audit_log" + | "debug_info" + | "deployment_config" + | "deployment_stats" + | "file" + | "group" + | "license" + | "organization" + | "organization_member" + | "provisioner_daemon" + | "replicas" + | "system" + | "template" + | "user" + | "user_data" + | "workspace" + | "workspace_execution" + | "workspace_proxy" +export const RBACResources: RBACResource[] = [ + "api_key", + "application_connect", + "assign_org_role", + "assign_role", + "audit_log", + "debug_info", + "deployment_config", + "deployment_stats", + "file", + "group", + "license", + "organization", + "organization_member", + "provisioner_daemon", + "replicas", + "system", + "template", + "user", + "user_data", + "workspace", + "workspace_execution", + "workspace_proxy", +] + // From codersdk/audit.go export type ResourceType = | "api_key" diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index db73f54c39a59..2619c845c38c8 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -59,7 +59,7 @@ export const permissionsToCheck = { }, [checks.viewDeploymentValues]: { object: { - resource_type: "deployment_flags", + resource_type: "deployment_config" }, action: "read", }, @@ -71,7 +71,7 @@ export const permissionsToCheck = { }, [checks.viewUpdateCheck]: { object: { - resource_type: "update_check", + resource_type: "deployment_config", }, action: "read", }, From 8595f1510838cbc90b421d3623a2c43f6458ca8a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 14:29:34 -0500 Subject: [PATCH 07/18] Add unit test --- coderd/rbac/roles.go | 9 ++++++++- coderd/rbac/roles_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 534e737fd787e..dd65886c04ed2 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -80,6 +80,10 @@ func allPermsExcept(excepts ...Object) []Permission { 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, @@ -118,7 +122,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { var ownerAndAdminExceptions []Object if opts.NoOwnerWorkspaceExec { - ownerAndAdminExceptions = append(ownerAndAdminExceptions, ResourceWorkspaceExecution) + ownerAndAdminExceptions = append(ownerAndAdminExceptions, + ResourceWorkspaceExecution, + ResourceWorkspaceApplicationConnect, + ) } builtInRoles = map[string]func(orgID string) Role{ diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 0a83f987f7244..0c1eb7682376f 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -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() From a535b2caf6e749e6d8d66159f2eb1b165a07a440 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 14:35:23 -0500 Subject: [PATCH 08/18] make fmt --- .../SecuritySettingsPage/SecuritySettingsPageView.tsx | 2 +- site/src/xServices/auth/authXService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx index fabc8027032f6..a68634da6bf38 100644 --- a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.tsx @@ -36,7 +36,7 @@ export const SecuritySettingsPageView = ({ options, "SSH Keygen Algorithm", "Secure Auth Cookie", - "Disable Owner Workspace Execution" + "Disable Owner Workspace Execution", )} /> diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 2619c845c38c8..132b01d496e84 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -59,7 +59,7 @@ export const permissionsToCheck = { }, [checks.viewDeploymentValues]: { object: { - resource_type: "deployment_config" + resource_type: "deployment_config", }, action: "read", }, From 9203a42b31e16d3740282445fb785676857d8093 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 15:30:20 -0500 Subject: [PATCH 09/18] Lint --- coderd/authorize_test.go | 2 +- enterprise/coderd/authorize_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/authorize_test.go b/coderd/authorize_test.go index f19cd361ca70f..738ffd2d61dc5 100644 --- a/coderd/authorize_test.go +++ b/coderd/authorize_test.go @@ -73,7 +73,7 @@ func TestCheckPermissions(t *testing.T) { }, updateSpecificTemplate: { Object: codersdk.AuthorizationObject{ - ResourceType: rbac.ResourceTemplate.Type, + ResourceType: codersdk.ResourceTemplate, ResourceID: template.ID.String(), }, Action: "update", diff --git a/enterprise/coderd/authorize_test.go b/enterprise/coderd/authorize_test.go index 5a39675721993..e176fb2c47ac4 100644 --- a/enterprise/coderd/authorize_test.go +++ b/enterprise/coderd/authorize_test.go @@ -58,7 +58,7 @@ func TestCheckACLPermissions(t *testing.T) { params := map[string]codersdk.AuthorizationCheck{ updateSpecificTemplate: { Object: codersdk.AuthorizationObject{ - ResourceType: rbac.ResourceTemplate.Type, + ResourceType: codersdk.ResourceTemplate, ResourceID: template.ID.String(), }, Action: "write", From 2438f67efbfb4728bf4e870294b91db28eb95e07 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Apr 2023 15:38:22 -0500 Subject: [PATCH 10/18] Fix test --- cli/testdata/coder_server_--help.golden | 6 ++++++ coderd/rbac/roles_test.go | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 7b31f8355723d..5cade0737de72 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -16,6 +16,12 @@ Start a Coder server $CACHE_DIRECTORY is set, it will be used for compatibility with systemd. + --disable-owner-workspace-exec bool, $CODER_DISABLE_OWNER_WORKSPACE_EXEC + Remove the permission for the 'owner' role to have workspace execution + on all workspaces. This prevents the 'owner' from ssh, apps, and + terminal access based on the 'owner' role. They still have their user + permissions to access their own workspaces. + --disable-path-apps bool, $CODER_DISABLE_PATH_APPS Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 0c1eb7682376f..a35db7d83416b 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -147,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}, }, }, { From bafafe425e96546b813ce3a3dddb7794add4129d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Apr 2023 15:36:36 -0500 Subject: [PATCH 11/18] Fix unit test --- coderd/authorize_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/coderd/authorize_test.go b/coderd/authorize_test.go index 738ffd2d61dc5..7c325cb7ffa47 100644 --- a/coderd/authorize_test.go +++ b/coderd/authorize_test.go @@ -46,27 +46,27 @@ 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", @@ -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, From ae7c1307c537fd241a7f17d81d1dea42264633a5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Apr 2023 15:38:44 -0500 Subject: [PATCH 12/18] Goldne files --- cli/testdata/server-config.yaml.golden | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index c552999c7e8a1..c9a9a36dd3f81 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -315,6 +315,12 @@ agentFallbackTroubleshootingURL: https://coder.com/docs/coder-oss/latest/templat # --wildcard-access-url is configured. # (default: , type: bool) disablePathApps: false +# Remove the permission for the 'owner' role to have workspace execution on all +# workspaces. This prevents the 'owner' from ssh, apps, and terminal access based +# on the 'owner' role. They still have their user permissions to access their own +# workspaces. +# (default: , type: bool) +disableOwnerWorkspaceExec: false # These options change the behavior of how clients interact with the Coder. # Clients include the coder cli, vs code extension, and the web UI. client: From 449dfa5e5ac8f3b57ff27b352c2018e70469cfcf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Apr 2023 15:54:52 -0500 Subject: [PATCH 13/18] Linter --- coderd/rbac/object_test.go | 2 ++ scripts/rbacgen/main.go | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/rbac/object_test.go b/coderd/rbac/object_test.go index 3203ced4c3aee..cbd043c753983 100644 --- a/coderd/rbac/object_test.go +++ b/coderd/rbac/object_test.go @@ -178,6 +178,8 @@ 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 { diff --git a/scripts/rbacgen/main.go b/scripts/rbacgen/main.go index c7ea65f778b47..ee06a49f21c31 100644 --- a/scripts/rbacgen/main.go +++ b/scripts/rbacgen/main.go @@ -84,7 +84,6 @@ func allResources(pkg *packages.Package) []string { if ok && obj.Type().String() == "github.com/coder/coder/coderd/rbac.Object" { resources = append(resources, obj.Name()) } - } sort.Strings(resources) return resources From 5839794d583bc6047a5d6c36ef368f8430455753 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Apr 2023 16:12:07 -0500 Subject: [PATCH 14/18] Fix make gen deps --- Makefile | 4 +-- site/src/api/typesGenerated.ts | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 04198d1344840..e5a67084dfa18 100644 --- a/Makefile +++ b/Makefile @@ -510,12 +510,12 @@ docs/cli.md: scripts/clidocgen/main.go $(GO_SRC_FILES) docs/manifest.json cd site yarn run format:write:only ../docs/cli.md ../docs/cli/*.md ../docs/manifest.json -docs/admin/audit-logs.md: scripts/auditdocgen/main.go enterprise/audit/table.go +docs/admin/audit-logs.md: scripts/auditdocgen/main.go enterprise/audit/table.go coderd/rbac/object_gen.go go run scripts/auditdocgen/main.go cd site yarn run format:write:only ../docs/admin/audit-logs.md -coderd/apidoc/swagger.json: $(shell find ./scripts/apidocgen $(FIND_EXCLUSIONS) -type f) $(wildcard coderd/*.go) $(wildcard enterprise/coderd/*.go) $(wildcard codersdk/*.go) .swaggo docs/manifest.json +coderd/apidoc/swagger.json: $(shell find ./scripts/apidocgen $(FIND_EXCLUSIONS) -type f) $(wildcard coderd/*.go) $(wildcard enterprise/coderd/*.go) $(wildcard codersdk/*.go) .swaggo docs/manifest.json coderd/rbac/object_gen.go ./scripts/apidocgen/generate.sh yarn run --cwd=site format:write:only ../docs/api ../docs/manifest.json ../coderd/apidoc/swagger.json diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d5dbf2041e404..a024312cdda69 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -379,6 +379,8 @@ export interface DeploymentValues { readonly git_auth?: any readonly config_ssh?: SSHConfig readonly wgtunnel_host?: string + readonly disable_owner_workspace_exec?: boolean + // This is likely an enum in an external package ("github.com/coder/coder/cli/clibase.YAMLConfigPath") readonly config?: string readonly write_config?: boolean // Named type "github.com/coder/coder/cli/clibase.HostPort" unknown, using "any" @@ -1419,6 +1421,55 @@ export const ProvisionerStorageMethods: ProvisionerStorageMethod[] = ["file"] export type ProvisionerType = "echo" | "terraform" export const ProvisionerTypes: ProvisionerType[] = ["echo", "terraform"] +// From codersdk/rbacresources.go +export type RBACResource = + | "api_key" + | "application_connect" + | "assign_org_role" + | "assign_role" + | "audit_log" + | "debug_info" + | "deployment_config" + | "deployment_stats" + | "file" + | "group" + | "license" + | "organization" + | "organization_member" + | "provisioner_daemon" + | "replicas" + | "system" + | "template" + | "user" + | "user_data" + | "workspace" + | "workspace_execution" + | "workspace_proxy" +export const RBACResources: RBACResource[] = [ + "api_key", + "application_connect", + "assign_org_role", + "assign_role", + "audit_log", + "debug_info", + "deployment_config", + "deployment_stats", + "file", + "group", + "license", + "organization", + "organization_member", + "provisioner_daemon", + "replicas", + "system", + "template", + "user", + "user_data", + "workspace", + "workspace_execution", + "workspace_proxy", +] + // From codersdk/audit.go export type ResourceType = | "api_key" From ade0d4ecb2b34e8eb05d90e9061766f66df8126e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 10 Apr 2023 16:24:59 -0500 Subject: [PATCH 15/18] Fix FE test --- .../SecuritySettingsPageView.stories.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.stories.tsx index 1839c3ce4db50..330c497b95f41 100644 --- a/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/SecuritySettingsPage/SecuritySettingsPageView.stories.tsx @@ -21,6 +21,11 @@ export default { usage: "something", value: "1234", }, + { + name: "Disable Owner Workspace Execution", + usage: "something", + value: false, + }, { name: "TLS Version", usage: "something", @@ -52,6 +57,10 @@ NoTLS.args = { name: "SSH Keygen Algorithm", value: "1234", } as DeploymentOption, + { + name: "Disable Owner Workspace Execution", + value: false, + } as DeploymentOption, { name: "Secure Auth Cookie", value: "1234", From 3449edc2c97af3f3c5cd0d07998b3a2af5e99ddf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 11 Apr 2023 08:02:33 -0500 Subject: [PATCH 16/18] Rename flag/env var --- codersdk/deployment.go | 6 +++--- docs/cli/server.md | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 41458aab9bdb2..eccfe4cb4d0bc 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1322,10 +1322,10 @@ when required by your organization's security policy.`, YAML: "disablePathApps", }, { - Name: "Disable Owner Workspace Execution", + Name: "Disable Owner Workspace Access", Description: "Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user permissions to access their own workspaces.", - Flag: "disable-owner-workspace-exec", - Env: "CODER_DISABLE_OWNER_WORKSPACE_EXEC", + Flag: "disable-owner-workspace-access", + Env: "CODER_DISABLE_OWNER_WORKSPACE_ACCESS", Value: &c.DisableOwnerWorkspaceExec, YAML: "disableOwnerWorkspaceExec", diff --git a/docs/cli/server.md b/docs/cli/server.md index aefe80e487b7d..111a2171262cb 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -173,13 +173,13 @@ An HTTP URL that is accessible by other replicas to relay DERP traffic. Required Addresses for STUN servers to establish P2P connections. Set empty to disable P2P connections. -### --disable-owner-workspace-exec +### --disable-owner-workspace-access -| | | -| ----------- | ------------------------------------------------ | -| Type | bool | -| Environment | $CODER_DISABLE_OWNER_WORKSPACE_EXEC | -| YAML | disableOwnerWorkspaceExec | +| | | +| ----------- | -------------------------------------------------- | +| Type | bool | +| Environment | $CODER_DISABLE_OWNER_WORKSPACE_ACCESS | +| YAML | disableOwnerWorkspaceExec | Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user permissions to access their own workspaces. From cc04d934eb8e79aa8ace3aa38387803e691ff20a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 11 Apr 2023 08:03:49 -0500 Subject: [PATCH 17/18] fixup! Rename flag/env var --- codersdk/deployment.go | 2 +- docs/cli/server.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index eccfe4cb4d0bc..71b643e32290e 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1328,7 +1328,7 @@ when required by your organization's security policy.`, Env: "CODER_DISABLE_OWNER_WORKSPACE_ACCESS", Value: &c.DisableOwnerWorkspaceExec, - YAML: "disableOwnerWorkspaceExec", + YAML: "disableOwnerWorkspaceAccess", }, { Name: "Session Duration", diff --git a/docs/cli/server.md b/docs/cli/server.md index 111a2171262cb..cd42ef026a13e 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -179,7 +179,7 @@ Addresses for STUN servers to establish P2P connections. Set empty to disable P2 | ----------- | -------------------------------------------------- | | Type | bool | | Environment | $CODER_DISABLE_OWNER_WORKSPACE_ACCESS | -| YAML | disableOwnerWorkspaceExec | +| YAML | disableOwnerWorkspaceAccess | Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user permissions to access their own workspaces. From 9ed9fdc968f888b8048445a170d43d9bc26154c2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 11 Apr 2023 08:24:09 -0500 Subject: [PATCH 18/18] Update golden files --- cli/testdata/coder_server_--help.golden | 2 +- cli/testdata/server-config.yaml.golden | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 3e1166af5f7f2..be3274f8bf1a2 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -16,7 +16,7 @@ Start a Coder server $CACHE_DIRECTORY is set, it will be used for compatibility with systemd. - --disable-owner-workspace-exec bool, $CODER_DISABLE_OWNER_WORKSPACE_EXEC + --disable-owner-workspace-access bool, $CODER_DISABLE_OWNER_WORKSPACE_ACCESS Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and terminal access based on the 'owner' role. They still have their user diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index c9a9a36dd3f81..5876107294df8 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -320,7 +320,7 @@ disablePathApps: false # on the 'owner' role. They still have their user permissions to access their own # workspaces. # (default: , type: bool) -disableOwnerWorkspaceExec: false +disableOwnerWorkspaceAccess: false # These options change the behavior of how clients interact with the Coder. # Clients include the coder cli, vs code extension, and the web UI. client: