diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d7e9408eb677f..125cf4faa5ba1 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -13699,6 +13699,7 @@ const docTemplate = `{ "read", "read_personal", "ssh", + "unassign", "update", "update_personal", "use", @@ -13714,6 +13715,7 @@ const docTemplate = `{ "ActionRead", "ActionReadPersonal", "ActionSSH", + "ActionUnassign", "ActionUpdate", "ActionUpdatePersonal", "ActionUse", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ff714e416c5ce..104d6fd70e077 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -12388,6 +12388,7 @@ "read", "read_personal", "ssh", + "unassign", "update", "update_personal", "use", @@ -12403,6 +12404,7 @@ "ActionRead", "ActionReadPersonal", "ActionSSH", + "ActionUnassign", "ActionUpdate", "ActionUpdatePersonal", "ActionUse", diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go index c5d40b0323185..815d6629f64f9 100644 --- a/coderd/database/dbauthz/customroles_test.go +++ b/coderd/database/dbauthz/customroles_test.go @@ -34,11 +34,12 @@ func TestInsertCustomRoles(t *testing.T) { } } - canAssignRole := rbac.Role{ + canCreateCustomRole := rbac.Role{ Identifier: rbac.RoleIdentifier{Name: "can-assign"}, DisplayName: "", Site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceAssignRole.Type: {policy.ActionRead, policy.ActionCreate}, + rbac.ResourceAssignRole.Type: {policy.ActionRead}, + rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate}, }), } @@ -61,17 +62,15 @@ func TestInsertCustomRoles(t *testing.T) { return all } - orgID := uuid.NullUUID{ - UUID: uuid.New(), - Valid: true, - } + orgID := uuid.New() + testCases := []struct { name string subject rbac.ExpandableRoles // Perms to create on new custom role - organizationID uuid.NullUUID + organizationID uuid.UUID site []codersdk.Permission org []codersdk.Permission user []codersdk.Permission @@ -79,19 +78,21 @@ func TestInsertCustomRoles(t *testing.T) { }{ { // No roles, so no assign role - name: "no-roles", - subject: rbac.RoleIdentifiers{}, - errorContains: "forbidden", + name: "no-roles", + organizationID: orgID, + subject: rbac.RoleIdentifiers{}, + errorContains: "forbidden", }, { // This works because the new role has 0 perms - name: "empty", - subject: merge(canAssignRole), + name: "empty", + organizationID: orgID, + subject: merge(canCreateCustomRole), }, { name: "mixed-scopes", - subject: merge(canAssignRole, rbac.RoleOwner()), organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleOwner()), site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), @@ -101,27 +102,30 @@ func TestInsertCustomRoles(t *testing.T) { errorContains: "organization roles specify site or user permissions", }, { - name: "invalid-action", - subject: merge(canAssignRole, rbac.RoleOwner()), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + name: "invalid-action", + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleOwner()), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ // Action does not go with resource codersdk.ResourceWorkspace: {codersdk.ActionViewInsights}, }), errorContains: "invalid action", }, { - name: "invalid-resource", - subject: merge(canAssignRole, rbac.RoleOwner()), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + name: "invalid-resource", + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleOwner()), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ "foobar": {codersdk.ActionViewInsights}, }), errorContains: "invalid resource", }, { // Not allowing these at this time. - name: "negative-permission", - subject: merge(canAssignRole, rbac.RoleOwner()), - site: []codersdk.Permission{ + name: "negative-permission", + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleOwner()), + org: []codersdk.Permission{ { Negate: true, ResourceType: codersdk.ResourceWorkspace, @@ -131,89 +135,69 @@ func TestInsertCustomRoles(t *testing.T) { errorContains: "no negative permissions", }, { - name: "wildcard", // not allowed - subject: merge(canAssignRole, rbac.RoleOwner()), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + name: "wildcard", // not allowed + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleOwner()), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {"*"}, }), errorContains: "no wildcard symbols", }, // escalation checks { - name: "read-workspace-escalation", - subject: merge(canAssignRole), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + name: "read-workspace-escalation", + organizationID: orgID, + subject: merge(canCreateCustomRole), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), errorContains: "not allowed to grant this permission", }, { - name: "read-workspace-outside-org", - organizationID: uuid.NullUUID{ - UUID: uuid.New(), - Valid: true, - }, - subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)), + name: "read-workspace-outside-org", + organizationID: uuid.New(), + subject: merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(orgID)), org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), - errorContains: "forbidden", + errorContains: "not allowed to grant this permission", }, { name: "user-escalation", // These roles do not grant user perms - subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)), + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(orgID)), user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), - errorContains: "not allowed to grant this permission", + errorContains: "organization roles specify site or user permissions", }, { - name: "template-admin-escalation", - subject: merge(canAssignRole, rbac.RoleTemplateAdmin()), + name: "site-escalation", + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleTemplateAdmin()), site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok! codersdk.ResourceDeploymentConfig: {codersdk.ActionUpdate}, // not ok! }), - user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok! - }), - errorContains: "deployment_config", + errorContains: "organization roles specify site or user permissions", }, // ok! { - name: "read-workspace-template-admin", - subject: merge(canAssignRole, rbac.RoleTemplateAdmin()), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + name: "read-workspace-template-admin", + organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.RoleTemplateAdmin()), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), }, { name: "read-workspace-in-org", - subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)), organizationID: orgID, + subject: merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(orgID)), org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), }, - { - name: "user-perms", - // This is weird, but is ok - subject: merge(canAssignRole, rbac.RoleMember()), - user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, - }), - }, - { - name: "site+user-perms", - subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()), - site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, - }), - user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, - }), - }, } for _, tc := range testCases { @@ -234,7 +218,7 @@ func TestInsertCustomRoles(t *testing.T) { _, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ Name: "test-role", DisplayName: "", - OrganizationID: tc.organizationID, + OrganizationID: uuid.NullUUID{UUID: tc.organizationID, Valid: true}, SitePermissions: db2sdk.List(tc.site, convertSDKPerm), OrgPermissions: db2sdk.List(tc.org, convertSDKPerm), UserPermissions: db2sdk.List(tc.user, convertSDKPerm), @@ -249,11 +233,11 @@ func TestInsertCustomRoles(t *testing.T) { LookupRoles: []database.NameOrganizationPair{ { Name: "test-role", - OrganizationID: tc.organizationID.UUID, + OrganizationID: tc.organizationID, }, }, ExcludeOrgRoles: false, - OrganizationID: uuid.UUID{}, + OrganizationID: uuid.Nil, }) require.NoError(t, err) require.Len(t, roles, 1) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index fdc9f6504d95d..877727069ab76 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -747,7 +747,7 @@ func (*querier) convertToDeploymentRoles(names []string) []rbac.RoleIdentifier { } // canAssignRoles handles assigning built in and custom roles. -func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, removed []rbac.RoleIdentifier) error { +func (q *querier) canAssignRoles(ctx context.Context, orgID uuid.UUID, added, removed []rbac.RoleIdentifier) error { actor, ok := ActorFromContext(ctx) if !ok { return NoActorError @@ -755,12 +755,14 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r roleAssign := rbac.ResourceAssignRole shouldBeOrgRoles := false - if orgID != nil { - roleAssign = rbac.ResourceAssignOrgRole.InOrg(*orgID) + if orgID != uuid.Nil { + roleAssign = rbac.ResourceAssignOrgRole.InOrg(orgID) shouldBeOrgRoles = true } - grantedRoles := append(added, removed...) + grantedRoles := make([]rbac.RoleIdentifier, 0, len(added)+len(removed)) + grantedRoles = append(grantedRoles, added...) + grantedRoles = append(grantedRoles, removed...) customRoles := make([]rbac.RoleIdentifier, 0) // Validate that the roles being assigned are valid. for _, r := range grantedRoles { @@ -774,11 +776,11 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r } if shouldBeOrgRoles { - if orgID == nil { + if orgID == uuid.Nil { return xerrors.Errorf("should never happen, orgID is nil, but trying to assign an organization role") } - if r.OrganizationID != *orgID { + if r.OrganizationID != orgID { return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, orgID.String()) } } @@ -824,7 +826,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r } if len(removed) > 0 { - if err := q.authorizeContext(ctx, policy.ActionDelete, roleAssign); err != nil { + if err := q.authorizeContext(ctx, policy.ActionUnassign, roleAssign); err != nil { return err } } @@ -1124,11 +1126,15 @@ func (q *querier) CleanTailnetTunnels(ctx context.Context) error { return q.db.CleanTailnetTunnels(ctx) } -// TODO: Handle org scoped lookups func (q *querier) CustomRoles(ctx context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceAssignRole); err != nil { + roleObject := rbac.ResourceAssignRole + if arg.OrganizationID != uuid.Nil { + roleObject = rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID) + } + if err := q.authorizeContext(ctx, policy.ActionRead, roleObject); err != nil { return nil, err } + return q.db.CustomRoles(ctx, arg) } @@ -1185,14 +1191,11 @@ func (q *querier) DeleteCryptoKey(ctx context.Context, arg database.DeleteCrypto } func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error { - if arg.OrganizationID.UUID != uuid.Nil { - if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { - return err - } - } else { - if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignRole); err != nil { - return err - } + if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil { + return NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")} + } + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return err } return q.db.DeleteCustomRole(ctx, arg) @@ -3009,14 +3012,11 @@ func (q *querier) InsertCryptoKey(ctx context.Context, arg database.InsertCrypto func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) { // Org and site role upsert share the same query. So switch the assertion based on the org uuid. - if arg.OrganizationID.UUID != uuid.Nil { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { - return database.CustomRole{}, err - } - } else { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil { - return database.CustomRole{}, err - } + if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil { + return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")} + } + if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return database.CustomRole{}, err } if err := q.customRoleCheck(ctx, database.CustomRole{ @@ -3146,7 +3146,7 @@ func (q *querier) InsertOrganizationMember(ctx context.Context, arg database.Ins // All roles are added roles. Org member is always implied. addedRoles := append(orgRoles, rbac.ScopedRoleOrgMember(arg.OrganizationID)) - err = q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []rbac.RoleIdentifier{}) + err = q.canAssignRoles(ctx, arg.OrganizationID, addedRoles, []rbac.RoleIdentifier{}) if err != nil { return database.OrganizationMember{}, err } @@ -3270,7 +3270,7 @@ func (q *querier) InsertTemplateVersionWorkspaceTag(ctx context.Context, arg dat func (q *querier) InsertUser(ctx context.Context, arg database.InsertUserParams) (database.User, error) { // Always check if the assigned roles can actually be assigned by this actor. impliedRoles := append([]rbac.RoleIdentifier{rbac.RoleMember()}, q.convertToDeploymentRoles(arg.RBACRoles)...) - err := q.canAssignRoles(ctx, nil, impliedRoles, []rbac.RoleIdentifier{}) + err := q.canAssignRoles(ctx, uuid.Nil, impliedRoles, []rbac.RoleIdentifier{}) if err != nil { return database.User{}, err } @@ -3608,14 +3608,11 @@ func (q *querier) UpdateCryptoKeyDeletesAt(ctx context.Context, arg database.Upd } func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) { - if arg.OrganizationID.UUID != uuid.Nil { - if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { - return database.CustomRole{}, err - } - } else { - if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignRole); err != nil { - return database.CustomRole{}, err - } + if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil { + return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")} + } + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return database.CustomRole{}, err } if err := q.customRoleCheck(ctx, database.CustomRole{ @@ -3695,7 +3692,7 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb impliedTypes := append(scopedGranted, rbac.ScopedRoleOrgMember(arg.OrgID)) added, removed := rbac.ChangeRoleSet(originalRoles, impliedTypes) - err = q.canAssignRoles(ctx, &arg.OrgID, added, removed) + err = q.canAssignRoles(ctx, arg.OrgID, added, removed) if err != nil { return database.OrganizationMember{}, err } @@ -4102,7 +4099,7 @@ func (q *querier) UpdateUserRoles(ctx context.Context, arg database.UpdateUserRo impliedTypes := append(q.convertToDeploymentRoles(arg.GrantedRoles), rbac.RoleMember()) // If the changeset is nothing, less rbac checks need to be done. added, removed := rbac.ChangeRoleSet(q.convertToDeploymentRoles(user.RBACRoles), impliedTypes) - err = q.canAssignRoles(ctx, nil, added, removed) + err = q.canAssignRoles(ctx, uuid.Nil, added, removed) if err != nil { return database.User{}, err } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 108a8166d19fb..1f2ae5eca62c4 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1011,7 +1011,7 @@ func (s *MethodTestSuite) TestOrganization() { Asserts( mem, policy.ActionRead, rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionAssign, // org-mem - rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionDelete, // org-admin + rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionUnassign, // org-admin ).Returns(out) })) } @@ -1619,7 +1619,7 @@ func (s *MethodTestSuite) TestUser() { }).Asserts( u, policy.ActionRead, rbac.ResourceAssignRole, policy.ActionAssign, - rbac.ResourceAssignRole, policy.ActionDelete, + rbac.ResourceAssignRole, policy.ActionUnassign, ).Returns(o) })) s.Run("AllUserIDs", s.Subtest(func(db database.Store, check *expects) { @@ -1653,30 +1653,28 @@ func (s *MethodTestSuite) TestUser() { check.Args(database.DeleteCustomRoleParams{ Name: customRole.Name, }).Asserts( - rbac.ResourceAssignRole, policy.ActionDelete) + // fails immediately, missing organization id + ).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}) })) s.Run("Blank/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) { dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) - customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{}) + customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{ + OrganizationID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + }) // Blank is no perms in the role check.Args(database.UpdateCustomRoleParams{ Name: customRole.Name, DisplayName: "Test Name", + OrganizationID: customRole.OrganizationID, SitePermissions: nil, OrgPermissions: nil, UserPermissions: nil, - }).Asserts(rbac.ResourceAssignRole, policy.ActionUpdate).ErrorsWithPG(sql.ErrNoRows) + }).Asserts(rbac.ResourceAssignOrgRole.InOrg(customRole.OrganizationID.UUID), policy.ActionUpdate) })) s.Run("SitePermissions/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) { - customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{ - OrganizationID: uuid.NullUUID{ - UUID: uuid.Nil, - Valid: false, - }, - }) check.Args(database.UpdateCustomRoleParams{ - Name: customRole.Name, - OrganizationID: customRole.OrganizationID, + Name: "", + OrganizationID: uuid.NullUUID{UUID: uuid.Nil, Valid: false}, DisplayName: "Test Name", SitePermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead, codersdk.ActionUpdate, codersdk.ActionDelete, codersdk.ActionViewInsights}, @@ -1686,17 +1684,8 @@ func (s *MethodTestSuite) TestUser() { codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), convertSDKPerm), }).Asserts( - // First check - rbac.ResourceAssignRole, policy.ActionUpdate, - // Escalation checks - rbac.ResourceTemplate, policy.ActionCreate, - rbac.ResourceTemplate, policy.ActionRead, - rbac.ResourceTemplate, policy.ActionUpdate, - rbac.ResourceTemplate, policy.ActionDelete, - rbac.ResourceTemplate, policy.ActionViewInsights, - - rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, - ).ErrorsWithPG(sql.ErrNoRows) + // fails immediately, missing organization id + ).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}) })) s.Run("OrgPermissions/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) { orgID := uuid.New() @@ -1726,13 +1715,15 @@ func (s *MethodTestSuite) TestUser() { })) s.Run("Blank/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { // Blank is no perms in the role + orgID := uuid.New() check.Args(database.InsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", + OrganizationID: uuid.NullUUID{UUID: orgID, Valid: true}, SitePermissions: nil, OrgPermissions: nil, UserPermissions: nil, - }).Asserts(rbac.ResourceAssignRole, policy.ActionCreate) + }).Asserts(rbac.ResourceAssignOrgRole.InOrg(orgID), policy.ActionCreate) })) s.Run("SitePermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertCustomRoleParams{ @@ -1746,17 +1737,8 @@ func (s *MethodTestSuite) TestUser() { codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), convertSDKPerm), }).Asserts( - // First check - rbac.ResourceAssignRole, policy.ActionCreate, - // Escalation checks - rbac.ResourceTemplate, policy.ActionCreate, - rbac.ResourceTemplate, policy.ActionRead, - rbac.ResourceTemplate, policy.ActionUpdate, - rbac.ResourceTemplate, policy.ActionDelete, - rbac.ResourceTemplate, policy.ActionViewInsights, - - rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, - ) + // fails immediately, missing organization id + ).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}) })) s.Run("OrgPermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { orgID := uuid.New() diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0e2bc0e37f375..25cadc470f9b0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7775,25 +7775,25 @@ SELECT FROM custom_roles WHERE - true - -- @lookup_roles will filter for exact (role_name, org_id) pairs - -- To do this manually in SQL, you can construct an array and cast it: - -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[]) - AND CASE WHEN array_length($1 :: name_organization_pair[], 1) > 0 THEN - -- Using 'coalesce' to avoid troubles with null literals being an empty string. - (name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY ($1::name_organization_pair[]) - ELSE true - END - -- This allows fetching all roles, or just site wide roles - AND CASE WHEN $2 :: boolean THEN - organization_id IS null + true + -- @lookup_roles will filter for exact (role_name, org_id) pairs + -- To do this manually in SQL, you can construct an array and cast it: + -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[]) + AND CASE WHEN array_length($1 :: name_organization_pair[], 1) > 0 THEN + -- Using 'coalesce' to avoid troubles with null literals being an empty string. + (name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY ($1::name_organization_pair[]) ELSE true - END - -- Allows fetching all roles to a particular organization - AND CASE WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - organization_id = $3 - ELSE true - END + END + -- This allows fetching all roles, or just site wide roles + AND CASE WHEN $2 :: boolean THEN + organization_id IS null + ELSE true + END + -- Allows fetching all roles to a particular organization + AND CASE WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + organization_id = $3 + ELSE true + END ` type CustomRolesParams struct { @@ -7866,16 +7866,16 @@ INSERT INTO updated_at ) VALUES ( - -- Always force lowercase names - lower($1), - $2, - $3, - $4, - $5, - $6, - now(), - now() - ) + -- Always force lowercase names + lower($1), + $2, + $3, + $4, + $5, + $6, + now(), + now() +) RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id ` diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql index 7246ddb6dee2d..ee5d35d91ab65 100644 --- a/coderd/database/queries/roles.sql +++ b/coderd/database/queries/roles.sql @@ -4,25 +4,25 @@ SELECT FROM custom_roles WHERE - true - -- @lookup_roles will filter for exact (role_name, org_id) pairs - -- To do this manually in SQL, you can construct an array and cast it: - -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[]) - AND CASE WHEN array_length(@lookup_roles :: name_organization_pair[], 1) > 0 THEN - -- Using 'coalesce' to avoid troubles with null literals being an empty string. - (name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY (@lookup_roles::name_organization_pair[]) - ELSE true - END - -- This allows fetching all roles, or just site wide roles - AND CASE WHEN @exclude_org_roles :: boolean THEN - organization_id IS null + true + -- @lookup_roles will filter for exact (role_name, org_id) pairs + -- To do this manually in SQL, you can construct an array and cast it: + -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[]) + AND CASE WHEN array_length(@lookup_roles :: name_organization_pair[], 1) > 0 THEN + -- Using 'coalesce' to avoid troubles with null literals being an empty string. + (name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY (@lookup_roles::name_organization_pair[]) ELSE true - END - -- Allows fetching all roles to a particular organization - AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - organization_id = @organization_id - ELSE true - END + END + -- This allows fetching all roles, or just site wide roles + AND CASE WHEN @exclude_org_roles :: boolean THEN + organization_id IS null + ELSE true + END + -- Allows fetching all roles to a particular organization + AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + organization_id = @organization_id + ELSE true + END ; -- name: DeleteCustomRole :exec @@ -46,16 +46,16 @@ INSERT INTO updated_at ) VALUES ( - -- Always force lowercase names - lower(@name), - @display_name, - @organization_id, - @site_permissions, - @org_permissions, - @user_permissions, - now(), - now() - ) + -- Always force lowercase names + lower(@name), + @display_name, + @organization_id, + @site_permissions, + @org_permissions, + @user_permissions, + now(), + now() +) RETURNING *; -- name: UpdateCustomRole :one diff --git a/coderd/members.go b/coderd/members.go index 97950b19e9137..c89b4c9c09c1a 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -323,7 +323,7 @@ func convertOrganizationMembers(ctx context.Context, db database.Store, mems []d customRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{ LookupRoles: roleLookup, ExcludeOrgRoles: false, - OrganizationID: uuid.UUID{}, + OrganizationID: uuid.Nil, }) if err != nil { // We are missing the display names, but that is not absolutely required. So just diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index e1fefada0f422..86faa5f9456dc 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -27,22 +27,21 @@ var ( // ResourceAssignOrgRole // Valid Actions - // - "ActionAssign" :: ability to assign org scoped roles - // - "ActionCreate" :: ability to create/delete custom roles within an organization - // - "ActionDelete" :: ability to delete org scoped roles - // - "ActionRead" :: view what roles are assignable - // - "ActionUpdate" :: ability to edit custom roles within an organization + // - "ActionAssign" :: assign org scoped roles + // - "ActionCreate" :: create/delete custom roles within an organization + // - "ActionDelete" :: delete roles within an organization + // - "ActionRead" :: view what roles are assignable within an organization + // - "ActionUnassign" :: unassign org scoped roles + // - "ActionUpdate" :: edit custom roles within an organization ResourceAssignOrgRole = Object{ Type: "assign_org_role", } // ResourceAssignRole // Valid Actions - // - "ActionAssign" :: ability to assign roles - // - "ActionCreate" :: ability to create/delete/edit custom roles - // - "ActionDelete" :: ability to unassign roles + // - "ActionAssign" :: assign user roles // - "ActionRead" :: view what roles are assignable - // - "ActionUpdate" :: ability to edit custom roles + // - "ActionUnassign" :: unassign user roles ResourceAssignRole = Object{ Type: "assign_role", } @@ -367,6 +366,7 @@ func AllActions() []policy.Action { policy.ActionRead, policy.ActionReadPersonal, policy.ActionSSH, + policy.ActionUnassign, policy.ActionUpdate, policy.ActionUpdatePersonal, policy.ActionUse, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 2aae17badfb95..0988401e3849c 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -19,7 +19,8 @@ const ( ActionWorkspaceStart Action = "start" ActionWorkspaceStop Action = "stop" - ActionAssign Action = "assign" + ActionAssign Action = "assign" + ActionUnassign Action = "unassign" ActionReadPersonal Action = "read_personal" ActionUpdatePersonal Action = "update_personal" @@ -221,20 +222,19 @@ var RBACPermissions = map[string]PermissionDefinition{ }, "assign_role": { Actions: map[Action]ActionDefinition{ - ActionAssign: actDef("ability to assign roles"), - ActionRead: actDef("view what roles are assignable"), - ActionDelete: actDef("ability to unassign roles"), - ActionCreate: actDef("ability to create/delete/edit custom roles"), - ActionUpdate: actDef("ability to edit custom roles"), + ActionAssign: actDef("assign user roles"), + ActionUnassign: actDef("unassign user roles"), + ActionRead: actDef("view what roles are assignable"), }, }, "assign_org_role": { Actions: map[Action]ActionDefinition{ - ActionAssign: actDef("ability to assign org scoped roles"), - ActionRead: actDef("view what roles are assignable"), - ActionDelete: actDef("ability to delete org scoped roles"), - ActionCreate: actDef("ability to create/delete custom roles within an organization"), - ActionUpdate: actDef("ability to edit custom roles within an organization"), + ActionAssign: actDef("assign org scoped roles"), + ActionUnassign: actDef("unassign org scoped roles"), + ActionCreate: actDef("create/delete custom roles within an organization"), + ActionRead: actDef("view what roles are assignable within an organization"), + ActionUpdate: actDef("edit custom roles within an organization"), + ActionDelete: actDef("delete roles within an organization"), }, }, "oauth2_app": { diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 7c733016430fe..d141a25eb8525 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -339,10 +339,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Identifier: RoleUserAdmin(), DisplayName: "User Admin", Site: Permissions(map[string][]policy.Action{ - ResourceAssignRole.Type: {policy.ActionAssign, policy.ActionDelete, policy.ActionRead}, + ResourceAssignRole.Type: {policy.ActionAssign, policy.ActionUnassign, policy.ActionRead}, // Need organization assign as well to create users. At present, creating a user // will always assign them to some organization. - ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionDelete, policy.ActionRead}, + ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionUnassign, policy.ActionRead}, ResourceUser.Type: { policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionUpdatePersonal, policy.ActionReadPersonal, @@ -459,7 +459,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Org: map[string][]Permission{ organizationID.String(): Permissions(map[string][]policy.Action{ // Assign, remove, and read roles in the organization. - ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionDelete, policy.ActionRead}, + ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionUnassign, policy.ActionRead}, ResourceOrganization.Type: {policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroup.Type: ResourceGroup.AvailableActions(), diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index b23849229e900..bb262223384ed 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -292,9 +292,9 @@ func TestRolePermissions(t *testing.T) { }, }, { - Name: "CreateCustomRole", - Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate}, - Resource: rbac.ResourceAssignRole, + Name: "CreateUpdateDeleteCustomRole", + Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, + Resource: rbac.ResourceAssignOrgRole, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, false: {setOtherOrg, setOrgNotMe, userAdmin, orgMemberMe, memberMe, templateAdmin}, @@ -302,7 +302,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "RoleAssignment", - Actions: []policy.Action{policy.ActionAssign, policy.ActionDelete}, + Actions: []policy.Action{policy.ActionAssign, policy.ActionUnassign}, Resource: rbac.ResourceAssignRole, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, userAdmin}, @@ -320,7 +320,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "OrgRoleAssignment", - Actions: []policy.Action{policy.ActionAssign, policy.ActionDelete}, + Actions: []policy.Action{policy.ActionAssign, policy.ActionUnassign}, Resource: rbac.ResourceAssignOrgRole.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, orgUserAdmin}, diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index f2751ac0334aa..68b765db3f8a6 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -49,6 +49,7 @@ const ( ActionRead RBACAction = "read" ActionReadPersonal RBACAction = "read_personal" ActionSSH RBACAction = "ssh" + ActionUnassign RBACAction = "unassign" ActionUpdate RBACAction = "update" ActionUpdatePersonal RBACAction = "update_personal" ActionUse RBACAction = "use" @@ -62,8 +63,8 @@ const ( var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceWildcard: {}, ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead, ActionUnassign, ActionUpdate}, + ResourceAssignRole: {ActionAssign, ActionRead, ActionUnassign}, ResourceAuditLog: {ActionCreate, ActionRead}, ResourceCryptoKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceDebugInfo: {ActionRead}, diff --git a/docs/reference/api/members.md b/docs/reference/api/members.md index 6daaaaeea736f..d29774663bc32 100644 --- a/docs/reference/api/members.md +++ b/docs/reference/api/members.md @@ -173,6 +173,7 @@ Status Code **200** | `action` | `read` | | `action` | `read_personal` | | `action` | `ssh` | +| `action` | `unassign` | | `action` | `update` | | `action` | `update_personal` | | `action` | `use` | @@ -335,6 +336,7 @@ Status Code **200** | `action` | `read` | | `action` | `read_personal` | | `action` | `ssh` | +| `action` | `unassign` | | `action` | `update` | | `action` | `update_personal` | | `action` | `use` | @@ -497,6 +499,7 @@ Status Code **200** | `action` | `read` | | `action` | `read_personal` | | `action` | `ssh` | +| `action` | `unassign` | | `action` | `update` | | `action` | `update_personal` | | `action` | `use` | @@ -628,6 +631,7 @@ Status Code **200** | `action` | `read` | | `action` | `read_personal` | | `action` | `ssh` | +| `action` | `unassign` | | `action` | `update` | | `action` | `update_personal` | | `action` | `use` | @@ -891,6 +895,7 @@ Status Code **200** | `action` | `read` | | `action` | `read_personal` | | `action` | `ssh` | +| `action` | `unassign` | | `action` | `update` | | `action` | `update_personal` | | `action` | `use` | diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 99f94e53992e8..b3e4821c2e39e 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -5104,6 +5104,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `read` | | `read_personal` | | `ssh` | +| `unassign` | | `update` | | `update_personal` | | `use` | diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index d5af54a35b03b..30432af76c7eb 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -127,8 +127,7 @@ func (api *API) putOrgRoles(rw http.ResponseWriter, r *http.Request) { }, }, ExcludeOrgRoles: false, - // Linter requires all fields to be set. This field is not actually required. - OrganizationID: organization.ID, + OrganizationID: organization.ID, }) // If it is a 404 (not found) error, ignore it. if err != nil && !httpapi.Is404Error(err) { diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index 483508bc11554..bfd1a46861090 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -15,18 +15,17 @@ export const RBACResourceActions: Partial< update: "update an api key, eg expires", }, assign_org_role: { - assign: "ability to assign org scoped roles", - create: "ability to create/delete custom roles within an organization", - delete: "ability to delete org scoped roles", - read: "view what roles are assignable", - update: "ability to edit custom roles within an organization", + assign: "assign org scoped roles", + create: "create/delete custom roles within an organization", + delete: "delete roles within an organization", + read: "view what roles are assignable within an organization", + unassign: "unassign org scoped roles", + update: "edit custom roles within an organization", }, assign_role: { - assign: "ability to assign roles", - create: "ability to create/delete/edit custom roles", - delete: "ability to unassign roles", + assign: "assign user roles", read: "view what roles are assignable", - update: "ability to edit custom roles", + unassign: "unassign user roles", }, audit_log: { create: "create new audit log entries", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index fdda12254052c..158237c50f5e3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1856,6 +1856,7 @@ export type RBACAction = | "read" | "read_personal" | "ssh" + | "unassign" | "update" | "update_personal" | "use" @@ -1871,6 +1872,7 @@ export const RBACActions: RBACAction[] = [ "read", "read_personal", "ssh", + "unassign", "update", "update_personal", "use",