From 7ce0b462b885d4e07846e4baca95b100572f813d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 11:32:06 -0500 Subject: [PATCH 01/11] feat: Add template-manager role for managing templates site-wide Might want to make this an org role in the future. Also also shows assignable roles in the ui. --- coderd/rbac/builtin.go | 30 ++++++++++++++++++++-------- coderd/rbac/builtin_internal_test.go | 1 + coderd/roles.go | 23 ++++++++++++++++----- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 8fd60fc6c2c93..d97157c6478d6 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -9,9 +9,10 @@ import ( ) const ( - admin string = "admin" - member string = "member" - auditor string = "auditor" + admin string = "admin" + member string = "member" + templateManager string = "template-manager" + auditor string = "auditor" orgAdmin string = "organization-admin" orgMember string = "organization-member" @@ -93,6 +94,18 @@ var ( } }, + templateManager: func(_ string) Role { + return Role{ + Name: templateManager, + DisplayName: "Template Manager", + Site: permissions(map[Object][]Action{ + ResourceTemplate: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + // CRUD all files, even those they did not upload. + ResourceFile: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + }), + } + }, + // orgAdmin returns a role with all actions allows in a given // organization scope. orgAdmin: func(organizationID string) Role { @@ -153,11 +166,12 @@ var ( // map[actor_role][assign_role] assignRoles = map[string]map[string]bool{ admin: { - admin: true, - auditor: true, - member: true, - orgAdmin: true, - orgMember: true, + admin: true, + auditor: true, + member: true, + orgAdmin: true, + orgMember: true, + templateManager: true, }, orgAdmin: { orgAdmin: true, diff --git a/coderd/rbac/builtin_internal_test.go b/coderd/rbac/builtin_internal_test.go index 3ffe8879a2548..1cc8d44be1b63 100644 --- a/coderd/rbac/builtin_internal_test.go +++ b/coderd/rbac/builtin_internal_test.go @@ -18,6 +18,7 @@ func TestRoleByName(t *testing.T) { }{ {Role: builtInRoles[admin]("")}, {Role: builtInRoles[member]("")}, + {Role: builtInRoles[templateManager]("")}, {Role: builtInRoles[auditor]("")}, {Role: builtInRoles[orgAdmin](uuid.New().String())}, diff --git a/coderd/roles.go b/coderd/roles.go index 05013f696cd94..623ab7ad0768f 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -13,16 +13,21 @@ import ( // assignableSiteRoles returns all site wide roles that can be assigned. func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) { - // TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the - // role of the user. - + actorRoles := httpmw.AuthorizationUserRoles(r) if !api.Authorize(r, rbac.ActionRead, rbac.ResourceRoleAssignment) { httpapi.Forbidden(rw) return } roles := rbac.SiteRoles() - httpapi.Write(rw, http.StatusOK, convertRoles(roles)) + assignable := make([]rbac.Role, 0) + for _, role := range roles { + if rbac.CanAssignRole(actorRoles.Roles, role.Name) { + assignable = append(assignable, role) + } + } + + httpapi.Write(rw, http.StatusOK, convertRoles(assignable)) } // assignableSiteRoles returns all site wide roles that can be assigned. @@ -30,6 +35,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { // TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the // role of the user. organization := httpmw.OrganizationParam(r) + actorRoles := httpmw.AuthorizationUserRoles(r) if !api.Authorize(r, rbac.ActionRead, rbac.ResourceOrgRoleAssignment.InOrg(organization.ID)) { httpapi.Forbidden(rw) @@ -37,7 +43,14 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { } roles := rbac.OrganizationRoles(organization.ID) - httpapi.Write(rw, http.StatusOK, convertRoles(roles)) + assignable := make([]rbac.Role, 0) + for _, role := range roles { + if rbac.CanAssignRole(actorRoles.Roles, role.Name) { + assignable = append(assignable, role) + } + } + + httpapi.Write(rw, http.StatusOK, convertRoles(assignable)) } func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) { From e9e4a42736f9018ca15220926806d38853b974b2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 11:43:29 -0500 Subject: [PATCH 02/11] Fix unit tests --- coderd/rbac/builtin_test.go | 3 +++ coderd/roles_test.go | 9 +++++---- codersdk/roles.go | 6 ++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index 231780e9eed56..cd5d95228d6dc 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -396,10 +396,13 @@ func TestListRoles(t *testing.T) { // If this test is ever failing, just update the list to the roles // expected from the builtin set. + // Always use constant strings, as if the names change, we need to write + // a SQL migration to change the name on the backend. require.ElementsMatch(t, []string{ "admin", "member", "auditor", + "template-manager", }, siteRoleNames) diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 1e1b3b177fab8..5e023d552032c 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -120,7 +120,7 @@ func TestListRoles(t *testing.T) { require.NoError(t, err, "create org") const forbidden = "Forbidden" - siteRoles := convertRoles(rbac.RoleAdmin(), "auditor") + siteRoles := convertRoles(rbac.RoleAdmin(), "auditor", "template-manager") orgRoles := convertRoles(rbac.RoleOrgAdmin(admin.OrganizationID)) testCases := []struct { @@ -131,19 +131,20 @@ func TestListRoles(t *testing.T) { AuthorizedError string }{ { + // Members cannot assign any roles Name: "MemberListSite", APICall: func(ctx context.Context) ([]codersdk.Role, error) { x, err := member.ListSiteRoles(ctx) return x, err }, - ExpectedRoles: siteRoles, + ExpectedRoles: []codersdk.Role{}, }, { Name: "OrgMemberListOrg", APICall: func(ctx context.Context) ([]codersdk.Role, error) { return member.ListOrganizationRoles(ctx, admin.OrganizationID) }, - ExpectedRoles: orgRoles, + ExpectedRoles: []codersdk.Role{}, }, { Name: "NonOrgMemberListOrg", @@ -158,7 +159,7 @@ func TestListRoles(t *testing.T) { APICall: func(ctx context.Context) ([]codersdk.Role, error) { return orgAdmin.ListSiteRoles(ctx) }, - ExpectedRoles: siteRoles, + ExpectedRoles: []codersdk.Role{}, }, { Name: "OrgAdminListOrg", diff --git a/codersdk/roles.go b/codersdk/roles.go index 377565c06d404..d6e34c7e48127 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -14,8 +14,7 @@ type Role struct { DisplayName string `json:"display_name"` } -// ListSiteRoles lists all available site wide roles. -// This is not user specific. +// ListSiteRoles lists all assignable site wide roles. func (c *Client) ListSiteRoles(ctx context.Context) ([]Role, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/users/roles", nil) if err != nil { @@ -29,8 +28,7 @@ func (c *Client) ListSiteRoles(ctx context.Context) ([]Role, error) { return roles, json.NewDecoder(res.Body).Decode(&roles) } -// ListOrganizationRoles lists all available roles for a given organization. -// This is not user specific. +// ListOrganizationRoles lists all assignable roles for a given organization. func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]Role, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/roles", org.String()), nil) if err != nil { From 6309ab88fce1d721808942df663b60fd60d5361b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 13:56:52 -0500 Subject: [PATCH 03/11] Add user-admin role --- coderd/rbac/builtin.go | 52 ++++++++++++++------- coderd/rbac/builtin_internal_test.go | 3 +- coderd/rbac/builtin_test.go | 67 +++++++++++++++++----------- coderd/rbac/object.go | 9 ++++ coderd/roles.go | 2 - coderd/workspaceapps.go | 4 +- 6 files changed, 92 insertions(+), 45 deletions(-) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index d97157c6478d6..fe3c5645105c3 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -9,10 +9,11 @@ import ( ) const ( - admin string = "admin" - member string = "member" - templateManager string = "template-manager" - auditor string = "auditor" + admin string = "admin" + member string = "member" + templateAdmin string = "template-admin" + userAdmin string = "user-admin" + auditor string = "auditor" orgAdmin string = "organization-admin" orgMember string = "organization-member" @@ -27,6 +28,14 @@ func RoleAdmin() string { return roleName(admin, "") } +func RoleTemplateAdmin() string { + return roleName(templateAdmin, "") +} + +func RoleUserAdmin() string { + return roleName(userAdmin, "") +} + func RoleMember() string { return roleName(member, "") } @@ -73,7 +82,8 @@ var ( ResourceProvisionerDaemon: {ActionRead}, }), User: permissions(map[Object][]Action{ - ResourceWildcard: {WildcardSymbol}, + ResourceWildcard: {WildcardSymbol}, + ResourceWorkspaceExecution: {WildcardSymbol}, }), } }, @@ -94,14 +104,25 @@ var ( } }, - templateManager: func(_ string) Role { + templateAdmin: func(_ string) Role { return Role{ - Name: templateManager, - DisplayName: "Template Manager", + Name: templateAdmin, + DisplayName: "Template Admin", Site: permissions(map[Object][]Action{ ResourceTemplate: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, // CRUD all files, even those they did not upload. - ResourceFile: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceFile: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceWorkspace: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + }), + } + }, + + userAdmin: func(_ string) Role { + return Role{ + Name: userAdmin, + DisplayName: "User Admin", + Site: permissions(map[Object][]Action{ + ResourceUser: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, }), } }, @@ -166,12 +187,13 @@ var ( // map[actor_role][assign_role] assignRoles = map[string]map[string]bool{ admin: { - admin: true, - auditor: true, - member: true, - orgAdmin: true, - orgMember: true, - templateManager: true, + admin: true, + auditor: true, + member: true, + orgAdmin: true, + orgMember: true, + templateAdmin: true, + userAdmin: true, }, orgAdmin: { orgAdmin: true, diff --git a/coderd/rbac/builtin_internal_test.go b/coderd/rbac/builtin_internal_test.go index 1cc8d44be1b63..2e49949d6cc04 100644 --- a/coderd/rbac/builtin_internal_test.go +++ b/coderd/rbac/builtin_internal_test.go @@ -18,7 +18,8 @@ func TestRoleByName(t *testing.T) { }{ {Role: builtInRoles[admin]("")}, {Role: builtInRoles[member]("")}, - {Role: builtInRoles[templateManager]("")}, + {Role: builtInRoles[templateAdmin]("")}, + {Role: builtInRoles[userAdmin]("")}, {Role: builtInRoles[auditor]("")}, {Role: builtInRoles[orgAdmin](uuid.New().String())}, diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index cd5d95228d6dc..780658a5c2808 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -111,6 +111,7 @@ func TestRolePermissions(t *testing.T) { // currentUser is anything that references "me", "mine", or "my". currentUser := uuid.New() adminID := uuid.New() + templateAdminID := uuid.New() orgID := uuid.New() otherOrg := uuid.New() @@ -124,9 +125,12 @@ func TestRolePermissions(t *testing.T) { otherOrgMember := authSubject{Name: "org_member_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg)}} otherOrgAdmin := authSubject{Name: "org_admin_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg), rbac.RoleOrgAdmin(otherOrg)}} + templateAdmin := authSubject{Name: "template-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleTemplateAdmin()}} + userAdmin := authSubject{Name: "template-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleUserAdmin()}} + // requiredSubjects are required to be asserted in each test case. This is // to make sure one is not forgotten. - requiredSubjects := []authSubject{memberMe, admin, orgMemberMe, orgAdmin, otherOrgAdmin, otherOrgMember} + requiredSubjects := []authSubject{memberMe, admin, orgMemberMe, orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin} testCases := []struct { // Name the test case to better locate the failing test case. @@ -146,7 +150,7 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead}, Resource: rbac.ResourceUser, AuthorizeMap: map[bool][]authSubject{ - true: {admin, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin}, + true: {admin, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin}, false: {}, }, }, @@ -155,8 +159,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, Resource: rbac.ResourceUser, AuthorizeMap: map[bool][]authSubject{ - true: {admin}, - false: {memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin}, + true: {admin, userAdmin}, + false: {memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin}, }, }, { @@ -165,8 +169,18 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, Resource: rbac.ResourceWorkspace.InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ - true: {admin, orgMemberMe, orgAdmin}, - false: {memberMe, otherOrgAdmin, otherOrgMember}, + true: {admin, orgMemberMe, orgAdmin, templateAdmin}, + false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin}, + }, + }, + { + Name: "MyWorkspaceInOrgExecution", + // When creating the WithID won't be set, but it does not change the result. + Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, + Resource: rbac.ResourceWorkspaceExecution.InOrg(orgID).WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]authSubject{ + true: {admin, orgAdmin, orgMemberMe}, + false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, { @@ -174,8 +188,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, Resource: rbac.ResourceTemplate.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ - true: {admin, orgAdmin}, - false: {memberMe, orgMemberMe, otherOrgAdmin, otherOrgMember}, + true: {admin, orgAdmin, templateAdmin}, + false: {memberMe, orgMemberMe, otherOrgAdmin, otherOrgMember, userAdmin}, }, }, { @@ -183,8 +197,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead}, Resource: rbac.ResourceTemplate.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ - true: {admin, orgMemberMe, orgAdmin}, - false: {memberMe, otherOrgAdmin, otherOrgMember}, + true: {admin, orgMemberMe, orgAdmin, templateAdmin}, + false: {memberMe, otherOrgAdmin, otherOrgMember, userAdmin}, }, }, { @@ -192,8 +206,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionCreate}, Resource: rbac.ResourceFile, AuthorizeMap: map[bool][]authSubject{ - true: {admin}, - false: {orgMemberMe, orgAdmin, memberMe, otherOrgAdmin, otherOrgMember}, + true: {admin, templateAdmin}, + false: {orgMemberMe, orgAdmin, memberMe, otherOrgAdmin, otherOrgMember, userAdmin}, }, }, { @@ -201,8 +215,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead, rbac.ActionUpdate, rbac.ActionDelete}, Resource: rbac.ResourceFile.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ - true: {admin, memberMe, orgMemberMe}, - false: {orgAdmin, otherOrgAdmin, otherOrgMember}, + true: {admin, memberMe, orgMemberMe, templateAdmin}, + false: {orgAdmin, otherOrgAdmin, otherOrgMember, userAdmin}, }, }, { @@ -211,7 +225,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganization, AuthorizeMap: map[bool][]authSubject{ true: {admin}, - false: {orgAdmin, otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe}, + false: {orgAdmin, otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe, templateAdmin, userAdmin}, }, }, { @@ -220,7 +234,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganization.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin}, - false: {otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe}, + false: {otherOrgAdmin, otherOrgMember, memberMe, orgMemberMe, templateAdmin, userAdmin}, }, }, { @@ -229,7 +243,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganization.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin, orgMemberMe}, - false: {otherOrgAdmin, otherOrgMember, memberMe}, + false: {otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, }, }, { @@ -238,7 +252,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceRoleAssignment, AuthorizeMap: map[bool][]authSubject{ true: {admin}, - false: {orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe}, + false: {orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, }, }, { @@ -246,7 +260,7 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead}, Resource: rbac.ResourceRoleAssignment, AuthorizeMap: map[bool][]authSubject{ - true: {admin, orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe}, + true: {admin, orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, false: {}, }, }, @@ -256,7 +270,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrgRoleAssignment.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin}, - false: {orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe}, + false: {orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, }, }, { @@ -265,7 +279,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrgRoleAssignment.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin, orgMemberMe}, - false: {otherOrgAdmin, otherOrgMember, memberMe}, + false: {otherOrgAdmin, otherOrgMember, memberMe, templateAdmin, userAdmin}, }, }, { @@ -274,7 +288,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceAPIKey.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgMemberMe, memberMe}, - false: {orgAdmin, otherOrgAdmin, otherOrgMember}, + false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, { @@ -283,7 +297,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceUserData.WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgMemberMe, memberMe}, - false: {orgAdmin, otherOrgAdmin, otherOrgMember}, + false: {orgAdmin, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, { @@ -292,7 +306,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganizationMember.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin}, - false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember}, + false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, { @@ -301,7 +315,7 @@ func TestRolePermissions(t *testing.T) { Resource: rbac.ResourceOrganizationMember.InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ true: {admin, orgAdmin, orgMemberMe}, - false: {memberMe, otherOrgAdmin, otherOrgMember}, + false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin, userAdmin}, }, }, } @@ -402,7 +416,8 @@ func TestListRoles(t *testing.T) { "admin", "member", "auditor", - "template-manager", + "template-admin", + "user-admin", }, siteRoleNames) diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 88f342d286e41..0dff5218748da 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -22,6 +22,15 @@ var ( Type: "workspace", } + // ResourceWorkspaceExecution CRUD. Org + User owner + // create = workspace remote execution + // read = ? + // update = ? + // delete = ? + ResourceWorkspaceExecution = Object{ + Type: "workspace_execution", + } + // ResourceAuditLog // read = access audit log ResourceAuditLog = Object{ diff --git a/coderd/roles.go b/coderd/roles.go index 623ab7ad0768f..c8f3d95e112af 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -32,8 +32,6 @@ func (api *API) assignableSiteRoles(rw http.ResponseWriter, r *http.Request) { // assignableSiteRoles returns all site wide roles that can be assigned. func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { - // TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the - // role of the user. organization := httpmw.OrganizationParam(r) actorRoles := httpmw.AuthorizationUserRoles(r) diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index f08e9d77c01e4..fe1d1760c940b 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -43,7 +43,9 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) }) return } - if !api.Authorize(r, rbac.ActionRead, workspace) { + + if !api.Authorize(r, rbac.ActionCreate, + rbac.ResourceWorkspaceExecution.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String())) { httpapi.ResourceNotFound(rw) return } From 30338b41a87430932f8c80609d83cc6f17303665 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 14:21:19 -0500 Subject: [PATCH 04/11] Protect workspace execution --- coderd/database/modelmethods.go | 4 ++++ coderd/roles_test.go | 2 +- coderd/workspaceagents.go | 12 +++++++++--- coderd/workspaceapps.go | 3 +-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index d83180247ac83..1002ef4b4b523 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -17,6 +17,10 @@ func (w Workspace) RBACObject() rbac.Object { return rbac.ResourceWorkspace.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) } +func (w Workspace) ExecutionRBAC() rbac.Object { + return rbac.ResourceWorkspaceExecution.InOrg(w.OrganizationID).WithOwner(w.OwnerID.String()) +} + func (m OrganizationMember) RBACObject() rbac.Object { return rbac.ResourceOrganizationMember.InOrg(m.OrganizationID) } diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 5e023d552032c..d2b83c83cc618 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -120,7 +120,7 @@ func TestListRoles(t *testing.T) { require.NoError(t, err, "create org") const forbidden = "Forbidden" - siteRoles := convertRoles(rbac.RoleAdmin(), "auditor", "template-manager") + siteRoles := convertRoles(rbac.RoleAdmin(), "auditor", "template-admin", "user-admin") orgRoles := convertRoles(rbac.RoleOrgAdmin(admin.OrganizationID)) testCases := []struct { diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 798b75c88d07e..cc239f682a809 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -70,7 +70,7 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) { workspaceAgent := httpmw.WorkspaceAgentParam(r) workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(r, rbac.ActionUpdate, workspace) { + if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -304,6 +304,12 @@ func (api *API) workspaceAgentICEServers(rw http.ResponseWriter, _ *http.Request // workspaceAgentTurn proxies a WebSocket connection to the TURN server. func (api *API) workspaceAgentTurn(rw http.ResponseWriter, r *http.Request) { + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { + httpapi.ResourceNotFound(rw) + return + } + api.websocketWaitMutex.Lock() api.websocketWaitGroup.Add(1) api.websocketWaitMutex.Unlock() @@ -364,7 +370,7 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { workspaceAgent := httpmw.WorkspaceAgentParam(r) workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(r, rbac.ActionUpdate, workspace) { + if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -478,7 +484,7 @@ func (api *API) postWorkspaceAgentWireguardPeer(rw http.ResponseWriter, r *http. workspace = httpmw.WorkspaceParam(r) ) - if !api.Authorize(r, rbac.ActionUpdate, workspace) { + if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index fe1d1760c940b..99c30eec2b7a7 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -44,8 +44,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } - if !api.Authorize(r, rbac.ActionCreate, - rbac.ResourceWorkspaceExecution.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String())) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } From 99d6f50c36c656d15f3ac9980b0a0c46d857bbaa Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 14:26:10 -0500 Subject: [PATCH 05/11] Remove unused perm --- coderd/rbac/builtin.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index fe3c5645105c3..32ab1583e5b0b 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -82,8 +82,7 @@ var ( ResourceProvisionerDaemon: {ActionRead}, }), User: permissions(map[Object][]Action{ - ResourceWildcard: {WildcardSymbol}, - ResourceWorkspaceExecution: {WildcardSymbol}, + ResourceWildcard: {WildcardSymbol}, }), } }, From b91903ee59d03139df28fa528c26d64cf7cc39ac Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 14:29:50 -0500 Subject: [PATCH 06/11] Fix unit test --- coderd/coderd_test.go | 14 +++++++++----- coderd/workspaceagents.go | 8 ++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index f3c4baaa227ab..89fa81b275db7 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -220,6 +220,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { // Some quick reused objects workspaceRBACObj := rbac.ResourceWorkspace.InOrg(organization.ID).WithOwner(workspace.OwnerID.String()) + workspaceExecObj := rbac.ResourceWorkspaceExecution.InOrg(organization.ID).WithOwner(workspace.OwnerID.String()) // skipRoutes allows skipping routes from being checked. skipRoutes := map[string]string{ @@ -268,7 +269,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/workspaceagents/me/wireguardlisten": {NoAuthorize: true}, "POST:/api/v2/workspaceagents/me/keys": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true}, - "GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/derp": {NoAuthorize: true}, // These endpoints have more assertions. This is good, add more endpoints to assert if you can! @@ -331,12 +331,16 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertObject: workspaceRBACObj, }, "GET:/api/v2/workspaceagents/{workspaceagent}/dial": { - AssertAction: rbac.ActionUpdate, - AssertObject: workspaceRBACObj, + AssertAction: rbac.ActionCreate, + AssertObject: workspaceExecObj, + }, + "GET:/api/v2/workspaceagents/{workspaceagent}/turn": { + AssertAction: rbac.ActionCreate, + AssertObject: workspaceExecObj, }, "GET:/api/v2/workspaceagents/{workspaceagent}/pty": { - AssertAction: rbac.ActionUpdate, - AssertObject: workspaceRBACObj, + AssertAction: rbac.ActionCreate, + AssertObject: workspaceExecObj, }, "GET:/api/v2/workspaces/": { StatusCode: http.StatusOK, diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index cc239f682a809..cd5b7c5bd9173 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -70,7 +70,7 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) { workspaceAgent := httpmw.WorkspaceAgentParam(r) workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -305,7 +305,7 @@ func (api *API) workspaceAgentICEServers(rw http.ResponseWriter, _ *http.Request // workspaceAgentTurn proxies a WebSocket connection to the TURN server. func (api *API) workspaceAgentTurn(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -370,7 +370,7 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { workspaceAgent := httpmw.WorkspaceAgentParam(r) workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } @@ -484,7 +484,7 @@ func (api *API) postWorkspaceAgentWireguardPeer(rw http.ResponseWriter, r *http. workspace = httpmw.WorkspaceParam(r) ) - if !api.Authorize(r, rbac.ActionUpdate, workspace.ExecutionRBAC()) { + if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } From 21aed8ab5342b9ee3bd1aef01f21e5d32d4e8914 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 14:39:05 -0500 Subject: [PATCH 07/11] Add provisioner daemon crud to template admin --- coderd/rbac/builtin.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 32ab1583e5b0b..1f3f34fd9ffb7 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -112,6 +112,8 @@ var ( // CRUD all files, even those they did not upload. ResourceFile: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, ResourceWorkspace: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + // CRUD to provisioner daemons for now. + ResourceProvisionerDaemon: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, }), } }, From 6791247829a1fe51d62fa18648a79ce8a08c5078 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 15:57:36 -0500 Subject: [PATCH 08/11] Add tooltip to user roles --- .../Tooltips/UserRoleHelpTooltip.tsx | 30 +++++++++++++++++++ site/src/components/Tooltips/index.ts | 1 + .../components/UsersTable/UsersTable.test.tsx | 22 ++++++++++++++ site/src/components/UsersTable/UsersTable.tsx | 9 +++++- 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 site/src/components/Tooltips/UserRoleHelpTooltip.tsx create mode 100644 site/src/components/UsersTable/UsersTable.test.tsx diff --git a/site/src/components/Tooltips/UserRoleHelpTooltip.tsx b/site/src/components/Tooltips/UserRoleHelpTooltip.tsx new file mode 100644 index 0000000000000..47a3722c938ba --- /dev/null +++ b/site/src/components/Tooltips/UserRoleHelpTooltip.tsx @@ -0,0 +1,30 @@ +import { FC } from "react" +import { + HelpTooltip, + HelpTooltipLink, + HelpTooltipLinksGroup, + HelpTooltipText, + HelpTooltipTitle, +} from "./HelpTooltip" + +export const Language = { + title: "What is a role?", + text: + "Coder role-based access control (RBAC) provides fine-grained access management. " + + "View our docs on how to use the available roles.", + link: "User Roles", +} + +export const UserRoleHelpTooltip: FC = () => { + return ( + + {Language.title} + {Language.text} + + + {Language.link} + + + + ) +} diff --git a/site/src/components/Tooltips/index.ts b/site/src/components/Tooltips/index.ts index 4f7eabac682c7..5f6bb5cf087b0 100644 --- a/site/src/components/Tooltips/index.ts +++ b/site/src/components/Tooltips/index.ts @@ -3,3 +3,4 @@ export { AuditHelpTooltip } from "./AuditHelpTooltip" export { OutdatedHelpTooltip } from "./OutdatedHelpTooltip" export { ResourcesHelpTooltip } from "./ResourcesHelpTooltip" export { WorkspaceHelpTooltip } from "./WorkspaceHelpTooltip" +export { UserRoleHelpTooltip } from "./UserRoleHelpTooltip" diff --git a/site/src/components/UsersTable/UsersTable.test.tsx b/site/src/components/UsersTable/UsersTable.test.tsx new file mode 100644 index 0000000000000..25e3cff6830ac --- /dev/null +++ b/site/src/components/UsersTable/UsersTable.test.tsx @@ -0,0 +1,22 @@ +import { fireEvent, screen } from "@testing-library/react" +import { Language as UserRoleLanguage } from "components/Tooltips/UserRoleHelpTooltip" +import { Language as TooltipLanguage } from "components/Tooltips/HelpTooltip/HelpTooltip" +import { render } from "testHelpers/renderHelpers" +import { UsersTable } from "./UsersTable" + +describe("AuditPage", () => { + it("renders a page with a title and subtitle", async () => { + // When + render( jest.fn()} + onActivateUser={() => jest.fn()} + onResetUserPassword={() => jest.fn()} + onUpdateUserRoles={() => jest.fn()} + />) + + // Then + const tooltipIcon = await screen.findByRole("button", { name: TooltipLanguage.ariaLabel }) + fireEvent.mouseOver(tooltipIcon) + expect(await screen.findByText(UserRoleLanguage.title)).toBeInTheDocument() + }) +}) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 0275ea08cfc91..e4694ef96d0a2 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -7,6 +7,8 @@ import TableRow from "@material-ui/core/TableRow" import { FC } from "react" import * as TypesGen from "../../api/typesGenerated" import { UsersTableBody } from "./UsersTableBody" +import {UserRoleHelpTooltip} from "../Tooltips" +import {Stack} from "../Stack/Stack"; export const Language = { usernameLabel: "User", @@ -44,7 +46,12 @@ export const UsersTable: FC = ({ {Language.usernameLabel} {Language.statusLabel} - {Language.rolesLabel} + + + {Language.rolesLabel} + + + {/* 1% is a trick to make the table cell width fit the content */} {canEditUsers && } From 61df3610924213ea8fd1bc1879bdcf5d7577e07e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 16:02:09 -0500 Subject: [PATCH 09/11] Make fmt --- site/src/components/Tooltips/index.ts | 2 +- .../components/UsersTable/UsersTable.test.tsx | 16 +++++++++------- site/src/components/UsersTable/UsersTable.tsx | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/site/src/components/Tooltips/index.ts b/site/src/components/Tooltips/index.ts index 5f6bb5cf087b0..e7b4f1bc5a2ff 100644 --- a/site/src/components/Tooltips/index.ts +++ b/site/src/components/Tooltips/index.ts @@ -2,5 +2,5 @@ export { AgentHelpTooltip } from "./AgentHelpTooltip" export { AuditHelpTooltip } from "./AuditHelpTooltip" export { OutdatedHelpTooltip } from "./OutdatedHelpTooltip" export { ResourcesHelpTooltip } from "./ResourcesHelpTooltip" -export { WorkspaceHelpTooltip } from "./WorkspaceHelpTooltip" export { UserRoleHelpTooltip } from "./UserRoleHelpTooltip" +export { WorkspaceHelpTooltip } from "./WorkspaceHelpTooltip" diff --git a/site/src/components/UsersTable/UsersTable.test.tsx b/site/src/components/UsersTable/UsersTable.test.tsx index 25e3cff6830ac..d9ca51fae5cc5 100644 --- a/site/src/components/UsersTable/UsersTable.test.tsx +++ b/site/src/components/UsersTable/UsersTable.test.tsx @@ -1,18 +1,20 @@ import { fireEvent, screen } from "@testing-library/react" -import { Language as UserRoleLanguage } from "components/Tooltips/UserRoleHelpTooltip" import { Language as TooltipLanguage } from "components/Tooltips/HelpTooltip/HelpTooltip" +import { Language as UserRoleLanguage } from "components/Tooltips/UserRoleHelpTooltip" import { render } from "testHelpers/renderHelpers" import { UsersTable } from "./UsersTable" describe("AuditPage", () => { it("renders a page with a title and subtitle", async () => { // When - render( jest.fn()} - onActivateUser={() => jest.fn()} - onResetUserPassword={() => jest.fn()} - onUpdateUserRoles={() => jest.fn()} - />) + render( + jest.fn()} + onActivateUser={() => jest.fn()} + onResetUserPassword={() => jest.fn()} + onUpdateUserRoles={() => jest.fn()} + />, + ) // Then const tooltipIcon = await screen.findByRole("button", { name: TooltipLanguage.ariaLabel }) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index e4694ef96d0a2..fc49dca3b1b34 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -6,9 +6,9 @@ import TableHead from "@material-ui/core/TableHead" import TableRow from "@material-ui/core/TableRow" import { FC } from "react" import * as TypesGen from "../../api/typesGenerated" +import { Stack } from "../Stack/Stack" +import { UserRoleHelpTooltip } from "../Tooltips" import { UsersTableBody } from "./UsersTableBody" -import {UserRoleHelpTooltip} from "../Tooltips" -import {Stack} from "../Stack/Stack"; export const Language = { usernameLabel: "User", From beea2e0b49eca2bed0569d94e7d7c4fcc1a528d7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 16:05:24 -0500 Subject: [PATCH 10/11] Update coderd/rbac/builtin_test.go Co-authored-by: Mathias Fredriksson --- coderd/rbac/builtin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/rbac/builtin_test.go b/coderd/rbac/builtin_test.go index 780658a5c2808..de0bb076d82a9 100644 --- a/coderd/rbac/builtin_test.go +++ b/coderd/rbac/builtin_test.go @@ -126,7 +126,7 @@ func TestRolePermissions(t *testing.T) { otherOrgAdmin := authSubject{Name: "org_admin_other", UserID: uuid.NewString(), Roles: []string{rbac.RoleMember(), rbac.RoleOrgMember(otherOrg), rbac.RoleOrgAdmin(otherOrg)}} templateAdmin := authSubject{Name: "template-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleTemplateAdmin()}} - userAdmin := authSubject{Name: "template-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleUserAdmin()}} + userAdmin := authSubject{Name: "user-admin", UserID: templateAdminID.String(), Roles: []string{rbac.RoleMember(), rbac.RoleUserAdmin()}} // requiredSubjects are required to be asserted in each test case. This is // to make sure one is not forgotten. From eb0e98675f4f7dd75e4ff5ff91458bf41840ea41 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Aug 2022 16:48:44 -0500 Subject: [PATCH 11/11] Add authorization check in /turn for users --- coderd/coderd.go | 2 +- coderd/workspaceagents.go | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 9437a9219b045..9c5633cb03033 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -340,7 +340,7 @@ func New(options *Options) *API { r.Get("/", api.workspaceAgent) r.Post("/peer", api.postWorkspaceAgentWireguardPeer) r.Get("/dial", api.workspaceAgentDial) - r.Get("/turn", api.workspaceAgentTurn) + r.Get("/turn", api.userWorkspaceAgentTurn) r.Get("/pty", api.workspaceAgentPTY) r.Get("/iceservers", api.workspaceAgentICEServers) r.Get("/derp", api.derpMap) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index cd5b7c5bd9173..0178026f4d8b6 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -302,14 +302,21 @@ func (api *API) workspaceAgentICEServers(rw http.ResponseWriter, _ *http.Request httpapi.Write(rw, http.StatusOK, api.ICEServers) } -// workspaceAgentTurn proxies a WebSocket connection to the TURN server. -func (api *API) workspaceAgentTurn(rw http.ResponseWriter, r *http.Request) { +// userWorkspaceAgentTurn is a user connecting to a remote workspace agent +// through turn. +func (api *API) userWorkspaceAgentTurn(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) if !api.Authorize(r, rbac.ActionCreate, workspace.ExecutionRBAC()) { httpapi.ResourceNotFound(rw) return } + // Passed authorization + api.workspaceAgentTurn(rw, r) +} + +// workspaceAgentTurn proxies a WebSocket connection to the TURN server. +func (api *API) workspaceAgentTurn(rw http.ResponseWriter, r *http.Request) { api.websocketWaitMutex.Lock() api.websocketWaitGroup.Add(1) api.websocketWaitMutex.Unlock()