diff --git a/coderd/authorize.go b/coderd/authorize.go index b98ad5e20f83c..64d6af755413c 100644 --- a/coderd/authorize.go +++ b/coderd/authorize.go @@ -13,12 +13,12 @@ import ( ) func AuthorizeFilter[O rbac.Objecter](api *API, r *http.Request, action rbac.Action, objects []O) []O { - roles := httpmw.UserRoles(r) + roles := httpmw.AuthorizationUserRoles(r) return rbac.Filter(r.Context(), api.Authorizer, roles.ID.String(), roles.Roles, action, objects) } func (api *API) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.Action, object rbac.Objecter) bool { - roles := httpmw.UserRoles(r) + roles := httpmw.AuthorizationUserRoles(r) err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object.RBACObject()) if err != nil { httpapi.Write(rw, http.StatusForbidden, httpapi.Response{ diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 48fe3b81deb72..6fbe4256d692e 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -281,7 +281,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui organizationID, err := uuid.Parse(orgID) require.NoError(t, err, fmt.Sprintf("parse org id %q", orgID)) _, err = client.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(), - codersdk.UpdateRoles{Roles: append(roles, rbac.RoleOrgMember(organizationID))}) + codersdk.UpdateRoles{Roles: roles}) require.NoError(t, err, "update org membership roles") } } diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index aad1b333a5274..9e649db5b32e4 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -276,7 +276,7 @@ func (q *fakeQuerier) GetUsersByIDs(_ context.Context, ids []uuid.UUID) ([]datab return users, nil } -func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (database.GetAllUserRolesRow, error) { +func (q *fakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.UUID) (database.GetAuthorizationUserRolesRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -286,6 +286,7 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data if u.ID == userID { u := u roles = append(roles, u.RBACRoles...) + roles = append(roles, "member") user = &u break } @@ -294,14 +295,15 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data for _, mem := range q.organizationMembers { if mem.UserID == userID { roles = append(roles, mem.Roles...) + roles = append(roles, "organization-member:"+mem.OrganizationID.String()) } } if user == nil { - return database.GetAllUserRolesRow{}, sql.ErrNoRows + return database.GetAuthorizationUserRolesRow{}, sql.ErrNoRows } - return database.GetAllUserRolesRow{ + return database.GetAuthorizationUserRolesRow{ ID: userID, Username: user.Username, Status: user.Status, diff --git a/coderd/database/migrations/000016_drop_member_roles.down.sql b/coderd/database/migrations/000016_drop_member_roles.down.sql new file mode 100644 index 0000000000000..9268cd8db7eb8 --- /dev/null +++ b/coderd/database/migrations/000016_drop_member_roles.down.sql @@ -0,0 +1,11 @@ +--- Remove the now implied 'member' role. +UPDATE + users +SET + rbac_roles = array_append(rbac_roles, 'member'); + +--- Remove the now implied 'organization-member' role. +UPDATE + organization_members +SET + roles = array_append(roles, 'organization-member:'||organization_id::text); diff --git a/coderd/database/migrations/000016_drop_member_roles.up.sql b/coderd/database/migrations/000016_drop_member_roles.up.sql new file mode 100644 index 0000000000000..64b67c1a64d30 --- /dev/null +++ b/coderd/database/migrations/000016_drop_member_roles.up.sql @@ -0,0 +1,11 @@ +--- Remove the now implied 'member' role. +UPDATE + users +SET + rbac_roles = array_remove(rbac_roles, 'member'); + +--- Remove the now implied 'organization-member' role. +UPDATE + organization_members +SET + roles = array_remove(roles, 'organization-member:'||organization_id::text); diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 7d4c361cea05b..fc3274e94abc8 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -22,10 +22,12 @@ type querier interface { DeleteGitSSHKey(ctx context.Context, userID uuid.UUID) error DeleteParameterValueByID(ctx context.Context, id uuid.UUID) error GetAPIKeyByID(ctx context.Context, id string) (APIKey, error) - GetAllUserRoles(ctx context.Context, userID uuid.UUID) (GetAllUserRolesRow, error) // GetAuditLogsBefore retrieves `limit` number of audit logs before the provided // ID. GetAuditLogsBefore(ctx context.Context, arg GetAuditLogsBeforeParams) ([]AuditLog, error) + // This function returns roles for authorization purposes. Implied member roles + // are included. + GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) GetFileByHash(ctx context.Context, hash string) (File, error) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSHKey, error) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 29f3eb84f3272..be5f9216eff2c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2088,12 +2088,18 @@ func (q *sqlQuerier) UpdateTemplateVersionDescriptionByJobID(ctx context.Context return err } -const getAllUserRoles = `-- name: GetAllUserRoles :one +const getAuthorizationUserRoles = `-- name: GetAuthorizationUserRoles :one SELECT - -- username is returned just to help for logging purposes - -- status is used to enforce 'suspended' users, as all roles are ignored - -- when suspended. - id, username, status, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles + -- username is returned just to help for logging purposes + -- status is used to enforce 'suspended' users, as all roles are ignored + -- when suspended. + id, username, status, + array_cat( + -- All users are members + array_append(users.rbac_roles, 'member'), + -- All org_members get the org-member role for their orgs + array_append(organization_members.roles, 'organization-member:'||organization_members.organization_id::text)) :: text[] + AS roles FROM users LEFT JOIN organization_members @@ -2102,16 +2108,18 @@ WHERE id = $1 ` -type GetAllUserRolesRow struct { +type GetAuthorizationUserRolesRow struct { ID uuid.UUID `db:"id" json:"id"` Username string `db:"username" json:"username"` Status UserStatus `db:"status" json:"status"` Roles []string `db:"roles" json:"roles"` } -func (q *sqlQuerier) GetAllUserRoles(ctx context.Context, userID uuid.UUID) (GetAllUserRolesRow, error) { - row := q.db.QueryRowContext(ctx, getAllUserRoles, userID) - var i GetAllUserRolesRow +// This function returns roles for authorization purposes. Implied member roles +// are included. +func (q *sqlQuerier) GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) { + row := q.db.QueryRowContext(ctx, getAuthorizationUserRoles, userID) + var i GetAuthorizationUserRolesRow err := row.Scan( &i.ID, &i.Username, diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index a4a8bcfc7bec6..be4a42a1538b7 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -134,12 +134,20 @@ WHERE id = $1 RETURNING *; --- name: GetAllUserRoles :one +-- name: GetAuthorizationUserRoles :one +-- This function returns roles for authorization purposes. Implied member roles +-- are included. SELECT - -- username is returned just to help for logging purposes - -- status is used to enforce 'suspended' users, as all roles are ignored - -- when suspended. - id, username, status, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles + -- username is returned just to help for logging purposes + -- status is used to enforce 'suspended' users, as all roles are ignored + -- when suspended. + id, username, status, + array_cat( + -- All users are members + array_append(users.rbac_roles, 'member'), + -- All org_members get the org-member role for their orgs + array_append(organization_members.roles, 'organization-member:'||organization_members.organization_id::text)) :: text[] + AS roles FROM users LEFT JOIN organization_members diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index 1e4098c0be431..8c58606fd1027 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -31,6 +31,19 @@ func APIKey(r *http.Request) database.APIKey { return apiKey } +// User roles are the 'subject' field of Authorize() +type userRolesKey struct{} + +// AuthorizationUserRoles returns the roles used for authorization. +// Comes from the ExtractAPIKey handler. +func AuthorizationUserRoles(r *http.Request) database.GetAuthorizationUserRolesRow { + apiKey, ok := r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow) + if !ok { + panic("developer error: user roles middleware not provided") + } + return apiKey +} + // OAuth2Configs is a collection of configurations for OAuth-based authentication. // This should be extended to support other authentication types in the future. type OAuth2Configs struct { @@ -178,7 +191,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h // If the key is valid, we also fetch the user roles and status. // The roles are used for RBAC authorize checks, and the status // is to block 'suspended' users from accessing the platform. - roles, err := db.GetAllUserRoles(r.Context(), key.UserID) + roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID) if err != nil { httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ Message: "roles not found", diff --git a/coderd/httpmw/authorize.go b/coderd/httpmw/authorize.go deleted file mode 100644 index 84bf7cbfa04b4..0000000000000 --- a/coderd/httpmw/authorize.go +++ /dev/null @@ -1,40 +0,0 @@ -package httpmw - -import ( - "context" - "net/http" - - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/httpapi" -) - -// User roles are the 'subject' field of Authorize() -type userRolesKey struct{} - -// UserRoles returns the API key from the ExtractUserRoles handler. -func UserRoles(r *http.Request) database.GetAllUserRolesRow { - apiKey, ok := r.Context().Value(userRolesKey{}).(database.GetAllUserRolesRow) - if !ok { - panic("developer error: user roles middleware not provided") - } - return apiKey -} - -// ExtractUserRoles requires authentication using a valid API key. -func ExtractUserRoles(db database.Store) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - apiKey := APIKey(r) - role, err := db.GetAllUserRoles(r.Context(), apiKey.UserID) - if err != nil { - httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ - Message: "roles not found", - }) - return - } - - ctx := context.WithValue(r.Context(), userRolesKey{}, role) - next.ServeHTTP(rw, r.WithContext(ctx)) - }) - } -} diff --git a/coderd/httpmw/authorize_test.go b/coderd/httpmw/authorize_test.go index 279c037e2ae92..7ea363249b7e6 100644 --- a/coderd/httpmw/authorize_test.go +++ b/coderd/httpmw/authorize_test.go @@ -31,23 +31,23 @@ func TestExtractUserRoles(t *testing.T) { { Name: "Member", AddUser: func(db database.Store) (database.User, []string, string) { - roles := []string{rbac.RoleMember()} + roles := []string{} user, token := addUser(t, db, roles...) - return user, roles, token + return user, append(roles, rbac.RoleMember()), token }, }, { Name: "Admin", AddUser: func(db database.Store) (database.User, []string, string) { - roles := []string{rbac.RoleMember(), rbac.RoleAdmin()} + roles := []string{rbac.RoleAdmin()} user, token := addUser(t, db, roles...) - return user, roles, token + return user, append(roles, rbac.RoleMember()), token }, }, { Name: "OrgMember", AddUser: func(db database.Store) (database.User, []string, string) { - roles := []string{rbac.RoleMember()} + roles := []string{} user, token := addUser(t, db, roles...) org, err := db.InsertOrganization(context.Background(), database.InsertOrganizationParams{ ID: uuid.New(), @@ -58,7 +58,7 @@ func TestExtractUserRoles(t *testing.T) { }) require.NoError(t, err) - orgRoles := []string{rbac.RoleOrgMember(org.ID)} + orgRoles := []string{} _, err = db.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{ OrganizationID: org.ID, UserID: user.ID, @@ -67,7 +67,7 @@ func TestExtractUserRoles(t *testing.T) { Roles: orgRoles, }) require.NoError(t, err) - return user, append(roles, orgRoles...), token + return user, append(roles, append(orgRoles, rbac.RoleMember(), rbac.RoleOrgMember(org.ID))...), token }, }, } @@ -86,7 +86,7 @@ func TestExtractUserRoles(t *testing.T) { httpmw.ExtractAPIKey(db, &httpmw.OAuth2Configs{}), ) rtr.Get("/", func(_ http.ResponseWriter, r *http.Request) { - roles := httpmw.UserRoles(r) + roles := httpmw.AuthorizationUserRoles(r) require.ElementsMatch(t, user.ID, roles.ID) require.ElementsMatch(t, expRoles, roles.Roles) }) diff --git a/coderd/members.go b/coderd/members.go index 68767658079f4..7b16794c8584d 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -34,7 +34,9 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) { return } - added, removed := rbac.ChangeRoleSet(member.Roles, params.Roles) + // The org-member role is always implied. + impliedTypes := append(params.Roles, rbac.RoleOrgMember(organization.ID)) + added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes) for _, roleName := range added { // Assigning a role requires the create permission. if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) { diff --git a/coderd/organizations.go b/coderd/organizations.go index 9e15b8511852e..49dba0bb33324 100644 --- a/coderd/organizations.go +++ b/coderd/organizations.go @@ -73,13 +73,11 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { CreatedAt: database.Now(), UpdatedAt: database.Now(), Roles: []string{ - // Also assign member role incase they get demoted from admin - rbac.RoleOrgMember(organization.ID), rbac.RoleOrgAdmin(organization.ID), }, }) if err != nil { - return xerrors.Errorf("create organization member: %w", err) + return xerrors.Errorf("create organization admin: %w", err) } return nil }) diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go index 407d827448b54..f6691c50830e7 100644 --- a/coderd/rbac/builtin.go +++ b/coderd/rbac/builtin.go @@ -63,7 +63,7 @@ var ( member: func(_ string) Role { return Role{ Name: member, - DisplayName: "Member", + DisplayName: "", Site: permissions(map[Object][]Action{ // All users can read all other users and know they exist. ResourceUser: {ActionRead}, @@ -116,7 +116,7 @@ var ( orgMember: func(organizationID string) Role { return Role{ Name: roleName(orgMember, organizationID), - DisplayName: "Organization Member", + DisplayName: "", Org: map[string][]Permission{ organizationID: { { diff --git a/coderd/rbac/role.go b/coderd/rbac/role.go index c236bf4a3cf59..9913a1091a68c 100644 --- a/coderd/rbac/role.go +++ b/coderd/rbac/role.go @@ -17,7 +17,9 @@ type Permission struct { // Users of this package should instead **only** use the role names, and // this package will expand the role names into their json payloads. type Role struct { - Name string `json:"name"` + Name string `json:"name"` + // DisplayName is used for UI purposes. If the role has no display name, + // that means the UI should never display it. DisplayName string `json:"display_name"` Site []Permission `json:"site"` // Org is a map of orgid to permissions. We represent orgid as a string. diff --git a/coderd/roles.go b/coderd/roles.go index eff35eb016516..4443ec1e6dc5a 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -45,7 +45,7 @@ func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) { } // use the roles of the user specified, not the person making the request. - roles, err := api.Database.GetAllUserRoles(r.Context(), user.ID) + roles, err := api.Database.GetAuthorizationUserRoles(r.Context(), user.ID) if err != nil { httpapi.Forbidden(rw) return @@ -91,6 +91,10 @@ func convertRole(role rbac.Role) codersdk.Role { func convertRoles(roles []rbac.Role) []codersdk.Role { converted := make([]codersdk.Role, 0, len(roles)) for _, role := range roles { + // Roles without display names should never be shown to the ui. + if role.DisplayName == "" { + continue + } converted = append(converted, convertRole(role)) } return converted diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 4820b76567aa4..baa956863ab03 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -113,6 +113,8 @@ func TestListRoles(t *testing.T) { require.NoError(t, err, "create org") const forbidden = "forbidden" + siteRoles := convertRoles(rbac.RoleAdmin(), "auditor") + orgRoles := convertRoles(rbac.RoleOrgAdmin(admin.OrganizationID)) testCases := []struct { Name string @@ -127,14 +129,14 @@ func TestListRoles(t *testing.T) { x, err := member.ListSiteRoles(ctx) return x, err }, - ExpectedRoles: convertRoles(rbac.SiteRoles()), + ExpectedRoles: siteRoles, }, { Name: "OrgMemberListOrg", APICall: func() ([]codersdk.Role, error) { return member.ListOrganizationRoles(ctx, admin.OrganizationID) }, - ExpectedRoles: convertRoles(rbac.OrganizationRoles(admin.OrganizationID)), + ExpectedRoles: orgRoles, }, { Name: "NonOrgMemberListOrg", @@ -149,14 +151,14 @@ func TestListRoles(t *testing.T) { APICall: func() ([]codersdk.Role, error) { return orgAdmin.ListSiteRoles(ctx) }, - ExpectedRoles: convertRoles(rbac.SiteRoles()), + ExpectedRoles: siteRoles, }, { Name: "OrgAdminListOrg", APICall: func() ([]codersdk.Role, error) { return orgAdmin.ListOrganizationRoles(ctx, admin.OrganizationID) }, - ExpectedRoles: convertRoles(rbac.OrganizationRoles(admin.OrganizationID)), + ExpectedRoles: orgRoles, }, { Name: "OrgAdminListOtherOrg", @@ -171,14 +173,14 @@ func TestListRoles(t *testing.T) { APICall: func() ([]codersdk.Role, error) { return client.ListSiteRoles(ctx) }, - ExpectedRoles: convertRoles(rbac.SiteRoles()), + ExpectedRoles: siteRoles, }, { Name: "AdminListOrg", APICall: func() ([]codersdk.Role, error) { return client.ListOrganizationRoles(ctx, admin.OrganizationID) }, - ExpectedRoles: convertRoles(rbac.OrganizationRoles(admin.OrganizationID)), + ExpectedRoles: orgRoles, }, } @@ -200,17 +202,18 @@ func TestListRoles(t *testing.T) { } } -func convertRole(role rbac.Role) codersdk.Role { +func convertRole(roleName string) codersdk.Role { + role, _ := rbac.RoleByName(roleName) return codersdk.Role{ DisplayName: role.DisplayName, Name: role.Name, } } -func convertRoles(roles []rbac.Role) []codersdk.Role { - converted := make([]codersdk.Role, 0, len(roles)) - for _, role := range roles { - converted = append(converted, convertRole(role)) +func convertRoles(roleNames ...string) []codersdk.Role { + converted := make([]codersdk.Role, 0, len(roleNames)) + for _, roleName := range roleNames { + converted = append(converted, convertRole(roleName)) } return converted } diff --git a/coderd/users.go b/coderd/users.go index 7769a67d53fdc..d4e5d55cef80f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -88,7 +88,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { // and add some rbac bypass when calling api functions this way?? // Add the admin role to this first user. _, err = api.Database.UpdateUserRoles(r.Context(), database.UpdateUserRolesParams{ - GrantedRoles: []string{rbac.RoleAdmin(), rbac.RoleMember()}, + GrantedRoles: []string{rbac.RoleAdmin()}, ID: user.ID, }) if err != nil { @@ -473,7 +473,7 @@ func (api *API) userRoles(rw http.ResponseWriter, r *http.Request) { func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { // User is the user to modify. user := httpmw.UserParam(r) - roles := httpmw.UserRoles(r) + roles := httpmw.AuthorizationUserRoles(r) apiKey := httpmw.APIKey(r) if apiKey.UserID == user.ID { @@ -488,7 +488,9 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { return } - added, removed := rbac.ChangeRoleSet(roles.Roles, params.Roles) + // The member role is always implied. + impliedTypes := append(params.Roles, rbac.RoleMember()) + added, removed := rbac.ChangeRoleSet(roles.Roles, impliedTypes) for _, roleName := range added { // Assigning a role requires the create permission. if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceRoleAssignment.WithID(roleName)) { @@ -757,7 +759,7 @@ func (api *API) createAPIKey(rw http.ResponseWriter, r *http.Request, params dat func (api *API) createUser(ctx context.Context, req codersdk.CreateUserRequest) (database.User, uuid.UUID, error) { var user database.User return user, req.OrganizationID, api.Database.InTx(func(db database.Store) error { - var orgRoles []string + orgRoles := make([]string, 0) // If no organization is provided, create a new one for the user. if req.OrganizationID == uuid.Nil { organization, err := db.InsertOrganization(ctx, database.InsertOrganizationParams{ @@ -772,8 +774,6 @@ func (api *API) createUser(ctx context.Context, req codersdk.CreateUserRequest) req.OrganizationID = organization.ID orgRoles = append(orgRoles, rbac.RoleOrgAdmin(req.OrganizationID)) } - // Always also be a member. - orgRoles = append(orgRoles, rbac.RoleOrgMember(req.OrganizationID)) params := database.InsertUserParams{ ID: uuid.New(), @@ -782,7 +782,7 @@ func (api *API) createUser(ctx context.Context, req codersdk.CreateUserRequest) CreatedAt: database.Now(), UpdatedAt: database.Now(), // All new users are defaulted to members of the site. - RBACRoles: []string{rbac.RoleMember()}, + RBACRoles: []string{}, } // If a user signs up with OAuth, they can have no password! if req.Password != "" { diff --git a/coderd/users_test.go b/coderd/users_test.go index 66ddb7ce6b771..3b88b310f1101 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -416,43 +416,43 @@ func TestGrantRoles(t *testing.T) { member := coderdtest.CreateAnotherUser(t, admin, first.OrganizationID) _, err = admin.UpdateUserRoles(ctx, codersdk.Me, codersdk.UpdateRoles{ - Roles: []string{rbac.RoleOrgMember(first.OrganizationID)}, + Roles: []string{rbac.RoleOrgAdmin(first.OrganizationID)}, }) require.Error(t, err, "org role in site") requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateUserRoles(ctx, uuid.New().String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleOrgMember(first.OrganizationID)}, + Roles: []string{rbac.RoleOrgAdmin(first.OrganizationID)}, }) require.Error(t, err, "user does not exist") requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, codersdk.Me, codersdk.UpdateRoles{ - Roles: []string{rbac.RoleMember()}, + Roles: []string{rbac.RoleAdmin()}, }) require.Error(t, err, "site role in org") requireStatusCode(t, err, http.StatusBadRequest) _, err = admin.UpdateOrganizationMemberRoles(ctx, uuid.New(), codersdk.Me, codersdk.UpdateRoles{ - Roles: []string{rbac.RoleMember()}, + Roles: []string{}, }) require.Error(t, err, "role in org without membership") requireStatusCode(t, err, http.StatusNotFound) _, err = member.UpdateUserRoles(ctx, first.UserID.String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleMember()}, + Roles: []string{}, }) require.Error(t, err, "member cannot change other's roles") requireStatusCode(t, err, http.StatusForbidden) _, err = member.UpdateUserRoles(ctx, first.UserID.String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleMember()}, + Roles: []string{}, }) require.Error(t, err, "member cannot change any roles") requireStatusCode(t, err, http.StatusForbidden) _, err = member.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, first.UserID.String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleMember()}, + Roles: []string{}, }) require.Error(t, err, "member cannot change other's org roles") requireStatusCode(t, err, http.StatusForbidden) @@ -480,11 +480,9 @@ func TestGrantRoles(t *testing.T) { require.NoError(t, err) require.ElementsMatch(t, roles.Roles, []string{ rbac.RoleAdmin(), - rbac.RoleMember(), }, "should be a member and admin") require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], []string{ - rbac.RoleOrgMember(first.OrganizationID), rbac.RoleOrgAdmin(first.OrganizationID), }, "should be a member and admin") }) @@ -498,12 +496,10 @@ func TestGrantRoles(t *testing.T) { member := coderdtest.CreateAnotherUser(t, admin, first.OrganizationID) roles, err := member.GetUserRoles(ctx, codersdk.Me) require.NoError(t, err) - require.ElementsMatch(t, roles.Roles, []string{ - rbac.RoleMember(), - }, "should be a member and admin") + require.ElementsMatch(t, roles.Roles, []string{}, "should be a member") require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], - []string{rbac.RoleOrgMember(first.OrganizationID)}, + []string{}, ) memberUser, err := member.User(ctx, codersdk.Me) @@ -513,7 +509,6 @@ func TestGrantRoles(t *testing.T) { _, err = admin.UpdateUserRoles(ctx, memberUser.ID.String(), codersdk.UpdateRoles{ Roles: []string{ // Promote to site admin - rbac.RoleMember(), rbac.RoleAdmin(), }, }) @@ -523,7 +518,6 @@ func TestGrantRoles(t *testing.T) { _, err = admin.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, memberUser.ID.String(), codersdk.UpdateRoles{ Roles: []string{ // Promote to org admin - rbac.RoleOrgMember(first.OrganizationID), rbac.RoleOrgAdmin(first.OrganizationID), }, }) @@ -532,12 +526,10 @@ func TestGrantRoles(t *testing.T) { roles, err = member.GetUserRoles(ctx, codersdk.Me) require.NoError(t, err) require.ElementsMatch(t, roles.Roles, []string{ - rbac.RoleMember(), rbac.RoleAdmin(), }, "should be a member and admin") require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], []string{ - rbac.RoleOrgMember(first.OrganizationID), rbac.RoleOrgAdmin(first.OrganizationID), }, "should be a member and admin") }) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 3b0bfe79c728f..19f7f2683b73d 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -88,7 +88,7 @@ func TestAdminViewAllWorkspaces(t *testing.T) { // This other user is not in the first user's org. Since other is an admin, they can // still see the "first" user's workspace. - other := coderdtest.CreateAnotherUser(t, client, otherOrg.ID, rbac.RoleAdmin(), rbac.RoleMember()) + other := coderdtest.CreateAnotherUser(t, client, otherOrg.ID, rbac.RoleAdmin()) otherWorkspaces, err := other.Workspaces(context.Background(), codersdk.WorkspaceFilter{}) require.NoError(t, err, "(other) fetch workspaces") diff --git a/codersdk/users.go b/codersdk/users.go index 8d7ec54b196f2..8b3ac9ca87016 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -69,7 +69,7 @@ type UpdateUserPasswordRequest struct { } type UpdateRoles struct { - Roles []string `json:"roles" validate:"required"` + Roles []string `json:"roles" validate:""` } type UserRoles struct {