Skip to content

Commit 096525e

Browse files
EmyrkEmyrk
committed
Continue to fix all compile issues
1 parent 95bdf96 commit 096525e

File tree

21 files changed

+205
-108
lines changed

21 files changed

+205
-108
lines changed

‎coderd/audit.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
199199
Roles: []codersdk.SlimRole{},
200200
}
201201

202-
for _, roleName := range dblog.UserRoles {
202+
for _, input := range dblog.UserRoles {
203+
roleName, _ := rbac.RoleNameFromString(input)
203204
rbacRole, _ := rbac.RoleByName(roleName)
204205
user.Roles = append(user.Roles, db2sdk.SlimRole(rbacRole))
205206
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,11 @@ func CreateFirstUser(t testing.TB, client *codersdk.Client) codersdk.CreateFirst
664664

665665
// CreateAnotherUser creates and authenticates a new user.
666666
// Roles can include org scoped roles with 'roleName:<organization_id>'
667-
func CreateAnotherUser(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles ...string) (*codersdk.Client, codersdk.User) {
667+
func CreateAnotherUser(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles ...rbac.RoleName) (*codersdk.Client, codersdk.User) {
668668
return createAnotherUserRetry(t, client, organizationID, 5, roles)
669669
}
670670

671-
func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
671+
func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles []rbac.RoleName, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
672672
return createAnotherUserRetry(t, client, organizationID, 5, roles, mutators...)
673673
}
674674

@@ -691,7 +691,7 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
691691
}
692692
}
693693

694-
func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
694+
func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []rbac.RoleName, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
695695
req := codersdk.CreateUserRequest{
696696
Email: namesgenerator.GetRandomName(10) + "@coder.com",
697697
Username: RandomUsername(t),

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,12 @@ func User(user database.User, organizationIDs []uuid.UUID) codersdk.User {
170170
}
171171

172172
for _, roleName := range user.RBACRoles {
173-
rbacRole, err := rbac.RoleByName(roleName)
173+
// TODO: Currently the api only returns site wide roles.
174+
// Should it return organization roles?
175+
rbacRole, err := rbac.RoleByName(rbac.RoleName{
176+
Name: roleName,
177+
OrganizationID: uuid.Nil,
178+
})
174179
if err == nil {
175180
convertedUser.Roles = append(convertedUser.Roles, SlimRole(rbacRole))
176181
} else {
@@ -519,29 +524,26 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
519524
}
520525

521526
func SlimRole(role rbac.Role) codersdk.SlimRole {
522-
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
523-
if err != nil {
524-
roleName = role.Name
527+
orgID := ""
528+
if role.Name.OrganizationID != uuid.Nil {
529+
orgID = role.Name.OrganizationID.String()
525530
}
526531

527532
return codersdk.SlimRole{
528533
DisplayName: role.DisplayName,
529-
Name: roleName,
530-
OrganizationID: orgIDStr,
534+
Name: role.Name.Name,
535+
OrganizationID: orgID,
531536
}
532537
}
533538

534539
func RBACRole(role rbac.Role) codersdk.Role {
535-
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
536-
if err != nil {
537-
roleName = role.Name
538-
}
539-
orgPerms := role.Org[orgIDStr]
540+
slim := SlimRole(role)
540541

542+
orgPerms := role.Org[slim.OrganizationID]
541543
return codersdk.Role{
542-
Name: roleName,
543-
OrganizationID: orgIDStr,
544-
DisplayName: role.DisplayName,
544+
Name: slim.Name,
545+
OrganizationID: slim.OrganizationID,
546+
DisplayName: slim.DisplayName,
545547
SitePermissions: List(role.Site, RBACPermission),
546548
OrganizationPermissions: List(orgPerms, RBACPermission),
547549
UserPermissions: List(role.User, RBACPermission),

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ var (
162162
ID: uuid.Nil.String(),
163163
Roles: rbac.Roles([]rbac.Role{
164164
{
165-
Name: "provisionerd",
165+
Name: rbac.RoleName{Name: "provisionerd"},
166166
DisplayName: "Provisioner Daemon",
167167
Site: rbac.Permissions(map[string][]policy.Action{
168168
// TODO: Add ProvisionerJob resource type.
@@ -191,7 +191,7 @@ var (
191191
ID: uuid.Nil.String(),
192192
Roles: rbac.Roles([]rbac.Role{
193193
{
194-
Name: "autostart",
194+
Name: rbac.RoleName{Name: "autostart"},
195195
DisplayName: "Autostart Daemon",
196196
Site: rbac.Permissions(map[string][]policy.Action{
197197
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
@@ -213,7 +213,7 @@ var (
213213
ID: uuid.Nil.String(),
214214
Roles: rbac.Roles([]rbac.Role{
215215
{
216-
Name: "hangdetector",
216+
Name: rbac.RoleName{Name: "hangdetector"},
217217
DisplayName: "Hang Detector Daemon",
218218
Site: rbac.Permissions(map[string][]policy.Action{
219219
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
@@ -232,7 +232,7 @@ var (
232232
ID: uuid.Nil.String(),
233233
Roles: rbac.Roles([]rbac.Role{
234234
{
235-
Name: "system",
235+
Name: rbac.RoleName{Name: "system"},
236236
DisplayName: "Coder",
237237
Site: rbac.Permissions(map[string][]policy.Action{
238238
rbac.ResourceWildcard.Type: {policy.ActionRead},
@@ -582,24 +582,33 @@ func (q *querier) authorizeUpdateFileTemplate(ctx context.Context, file database
582582
}
583583
}
584584

585-
// uniqueOrganizationRoles converts a set of scoped role names to their unique
585+
// convertToOrganizationRoles converts a set of scoped role names to their unique
586586
// scoped names.
587-
func (q *querier) uniqueOrganizationRoles(organizationID uuid.UUID, names []string) ([]rbac.RoleName, error) {
587+
func (q *querier) convertToOrganizationRoles(organizationID uuid.UUID, names []string) ([]rbac.RoleName, error) {
588588
uniques := make([]rbac.RoleName, 0, len(names))
589589
for _, name := range names {
590590
// This check is a developer safety check. Old code might try to invoke this code path with
591591
// organization id suffixes. Catch this and return a nice error so it can be fixed.
592-
_, foundOrg, _ := rbac.RoleSplit(rbac.RoleName(name))
593-
if foundOrg != "" {
592+
if strings.Contains(name, ":") {
594593
return nil, xerrors.Errorf("attempt to assign a role %q, remove the ':<organization_id> suffix", name)
595594
}
596595

597-
uniques = append(uniques, rbac.RoleName(name, organizationID.String()))
596+
uniques = append(uniques, rbac.RoleName{Name: name, OrganizationID: organizationID})
598597
}
599598

600599
return uniques, nil
601600
}
602601

602+
// convertToDeploymentRoles converts string role names into deployment wide roles.
603+
func (q *querier) convertToDeploymentRoles(names []string) []rbac.RoleName {
604+
uniques := make([]rbac.RoleName, 0, len(names))
605+
for _, name := range names {
606+
uniques = append(uniques, rbac.RoleName{Name: name})
607+
}
608+
609+
return uniques
610+
}
611+
603612
// canAssignRoles handles assigning built in and custom roles.
604613
func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, removed []rbac.RoleName) error {
605614
actor, ok := ActorFromContext(ctx)
@@ -618,25 +627,21 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
618627
customRoles := make([]rbac.RoleName, 0)
619628
// Validate that the roles being assigned are valid.
620629
for _, r := range grantedRoles {
621-
roleOrgIDStr, isOrgRole := rbac.IsOrgRole(r)
630+
isOrgRole := r.OrganizationID != uuid.Nil
622631
if shouldBeOrgRoles && !isOrgRole {
623632
return xerrors.Errorf("Must only update org roles")
624633
}
634+
625635
if !shouldBeOrgRoles && isOrgRole {
626636
return xerrors.Errorf("Must only update site wide roles")
627637
}
628638

629639
if shouldBeOrgRoles {
630-
roleOrgID, err := uuid.Parse(roleOrgIDStr)
631-
if err != nil {
632-
return xerrors.Errorf("role %q has invalid uuid for org: %w", r, err)
633-
}
634-
635640
if orgID == nil {
636641
return xerrors.Errorf("should never happen, orgID is nil, but trying to assign an organization role")
637642
}
638643

639-
if roleOrgID != *orgID {
644+
if r.OrganizationID != *orgID {
640645
return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, orgID.String())
641646
}
642647
}
@@ -667,7 +672,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
667672
// returns them all, but then someone could pass in a large list to make us do
668673
// a lot of loop iterations.
669674
if !slices.ContainsFunc(expandedCustomRoles, func(customRole rbac.Role) bool {
670-
return strings.EqualFold(customRole.Name, role)
675+
return strings.EqualFold(customRole.Name.Name, role.Name) && customRole.Name.OrganizationID == role.OrganizationID
671676
}) {
672677
return xerrors.Errorf("%q is not a supported role", role)
673678
}
@@ -2489,9 +2494,14 @@ func (q *querier) InsertOrganization(ctx context.Context, arg database.InsertOrg
24892494
}
24902495

24912496
func (q *querier) InsertOrganizationMember(ctx context.Context, arg database.InsertOrganizationMemberParams) (database.OrganizationMember, error) {
2497+
orgRoles, err := q.convertToOrganizationRoles(arg.OrganizationID, arg.Roles)
2498+
if err != nil {
2499+
return database.OrganizationMember{}, xerrors.Errorf("converting to organization roles: %w", err)
2500+
}
2501+
24922502
// All roles are added roles. Org member is always implied.
2493-
addedRoles := append(arg.Roles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
2494-
err := q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []string{})
2503+
addedRoles := append(orgRoles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
2504+
err = q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []rbac.RoleName{})
24952505
if err != nil {
24962506
return database.OrganizationMember{}, err
24972507
}
@@ -2577,8 +2587,8 @@ func (q *querier) InsertTemplateVersionWorkspaceTag(ctx context.Context, arg dat
25772587

25782588
func (q *querier) InsertUser(ctx context.Context, arg database.InsertUserParams) (database.User, error) {
25792589
// Always check if the assigned roles can actually be assigned by this actor.
2580-
impliedRoles := append([]string{rbac.RoleMember()}, arg.RBACRoles...)
2581-
err := q.canAssignRoles(ctx, nil, impliedRoles, []string{})
2590+
impliedRoles := append([]rbac.RoleName{rbac.RoleMember()}, q.convertToDeploymentRoles(arg.RBACRoles)...)
2591+
err := q.canAssignRoles(ctx, nil, impliedRoles, []rbac.RoleName{})
25822592
if err != nil {
25832593
return database.User{}, err
25842594
}
@@ -2865,23 +2875,22 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
28652875
return database.OrganizationMember{}, err
28662876
}
28672877

2878+
originalRoles, err := q.convertToOrganizationRoles(member.OrganizationID, member.Roles)
2879+
if err != nil {
2880+
return database.OrganizationMember{}, xerrors.Errorf("convert original roles: %w", err)
2881+
}
2882+
28682883
// The 'rbac' package expects role names to be scoped.
28692884
// Convert the argument roles for validation.
2870-
scopedGranted := make([]rbac.RoleName, 0, len(arg.GrantedRoles))
2871-
for _, grantedRole := range arg.GrantedRoles {
2872-
// This check is a developer safety check. Old code might try to invoke this code path with
2873-
// organization id suffixes. Catch this and return a nice error so it can be fixed.
2874-
_, foundOrg, _ := rbac.RoleSplit(grantedRole)
2875-
if foundOrg != "" {
2876-
return database.OrganizationMember{}, xerrors.Errorf("attempt to assign a role %q, remove the ':<organization_id> suffix", grantedRole)
2877-
}
2878-
2879-
scopedGranted = append(scopedGranted, rbac.RoleName(grantedRole, arg.OrgID.String()))
2885+
scopedGranted, err := q.convertToOrganizationRoles(arg.OrgID, arg.GrantedRoles)
2886+
if err != nil {
2887+
return database.OrganizationMember{}, err
28802888
}
28812889

28822890
// The org member role is always implied.
28832891
impliedTypes := append(scopedGranted, rbac.ScopedRoleOrgMember(arg.OrgID))
2884-
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
2892+
2893+
added, removed := rbac.ChangeRoleSet(originalRoles, impliedTypes)
28852894
err = q.canAssignRoles(ctx, &arg.OrgID, added, removed)
28862895
if err != nil {
28872896
return database.OrganizationMember{}, err
@@ -3222,9 +3231,9 @@ func (q *querier) UpdateUserRoles(ctx context.Context, arg database.UpdateUserRo
32223231
}
32233232

32243233
// The member role is always implied.
3225-
impliedTypes := append(arg.GrantedRoles, rbac.RoleMember())
3234+
impliedTypes := append(q.convertToDeploymentRoles(arg.GrantedRoles), rbac.RoleMember())
32263235
// If the changeset is nothing, less rbac checks need to be done.
3227-
added, removed := rbac.ChangeRoleSet(user.RBACRoles, impliedTypes)
3236+
added, removed := rbac.ChangeRoleSet(q.convertToDeploymentRoles(user.RBACRoles), impliedTypes)
32283237
err = q.canAssignRoles(ctx, nil, added, removed)
32293238
if err != nil {
32303239
return database.User{}, err

‎coderd/database/dbmem/dbmem.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4808,7 +4808,7 @@ func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
48084808
users = usersFilteredByStatus
48094809
}
48104810

4811-
if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember()) {
4811+
if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember().String()) {
48124812
usersFilteredByRole := make([]database.User, 0, len(users))
48134813
for i, user := range users {
48144814
if slice.OverlapCompare(params.RbacRole, user.RBACRoles, strings.EqualFold) {

‎coderd/database/modelmethods.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import (
55
"strconv"
66
"time"
77

8-
"github.com/google/uuid"
98
"golang.org/x/exp/maps"
109
"golang.org/x/oauth2"
10+
"golang.org/x/xerrors"
1111

1212
"github.com/coder/coder/v2/coderd/database/dbtime"
1313
"github.com/coder/coder/v2/coderd/rbac"
@@ -375,9 +375,21 @@ func (p ProvisionerJob) FinishedAt() time.Time {
375375
return time.Time{}
376376
}
377377

378-
func (r CustomRole) UniqueName() rbac.RoleName {
379-
if r.OrganizationID.UUID == uuid.Nil {
380-
return rbac.RoleName(r.Name, "")
378+
func (r CustomRole) RoleName() rbac.RoleName {
379+
return rbac.RoleName{
380+
Name: r.Name,
381+
OrganizationID: r.OrganizationID.UUID,
381382
}
382-
return rbac.RoleName(r.Name, r.OrganizationID.UUID.String())
383+
}
384+
385+
func (r GetAuthorizationUserRolesRow) RoleNames() ([]rbac.RoleName, error) {
386+
names := make([]rbac.RoleName, 0, len(r.Roles))
387+
for _, role := range r.Roles {
388+
value, err := rbac.RoleNameFromString(role)
389+
if err != nil {
390+
return nil, xerrors.Errorf("convert role %q: %w", role, err)
391+
}
392+
names = append(names, value)
393+
}
394+
return names, nil
383395
}

‎coderd/httpmw/apikey.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,16 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
438438
})
439439
}
440440

441+
roleNames, err := roles.RoleNames()
442+
if err != nil {
443+
return write(http.StatusInternalServerError, codersdk.Response{
444+
Message: "Internal Server Error",
445+
Detail: err.Error(),
446+
})
447+
}
448+
441449
//nolint:gocritic // Permission to lookup custom roles the user has assigned.
442-
rbacRoles, err := rolestore.Expand(dbauthz.AsSystemRestricted(ctx), cfg.DB, roles.Roles)
450+
rbacRoles, err := rolestore.Expand(dbauthz.AsSystemRestricted(ctx), cfg.DB, roleNames)
443451
if err != nil {
444452
return write(http.StatusInternalServerError, codersdk.Response{
445453
Message: "Failed to expand authenticated user roles",

‎coderd/httpmw/workspaceagent.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,18 @@ func ExtractWorkspaceAgentAndLatestBuild(opts ExtractWorkspaceAgentAndLatestBuil
119119
return
120120
}
121121

122+
roleNames, err := roles.RoleNames()
123+
if err != nil {
124+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
125+
Message: "Internal server error",
126+
Detail: err.Error(),
127+
})
128+
return
129+
}
130+
122131
subject := rbac.Subject{
123132
ID: row.Workspace.OwnerID.String(),
124-
Roles: rbac.RoleNames(roles.Roles),
133+
Roles: rbac.RoleNames(roleNames),
125134
Groups: roles.Groups,
126135
Scope: rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
127136
WorkspaceID: row.Workspace.ID,

‎coderd/identityprovider/tokens.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,15 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
214214
if err != nil {
215215
return oauth2.Token{}, err
216216
}
217+
218+
roleNames, err := roles.RoleNames()
219+
if err != nil {
220+
return oauth2.Token{}, xerrors.Errorf("role names: %w", err)
221+
}
222+
217223
userSubj := rbac.Subject{
218224
ID: dbCode.UserID.String(),
219-
Roles: rbac.RoleNames(roles.Roles),
225+
Roles: rbac.RoleNames(roleNames),
220226
Groups: roles.Groups,
221227
Scope: rbac.ScopeAll,
222228
}
@@ -310,9 +316,15 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
310316
if err != nil {
311317
return oauth2.Token{}, err
312318
}
319+
320+
roleNames, err := roles.RoleNames()
321+
if err != nil {
322+
return oauth2.Token{}, xerrors.Errorf("role names: %w", err)
323+
}
324+
313325
userSubj := rbac.Subject{
314326
ID: prevKey.UserID.String(),
315-
Roles: rbac.RoleNames(roles.Roles),
327+
Roles: rbac.RoleNames(roleNames),
316328
Groups: roles.Groups,
317329
Scope: rbac.ScopeAll,
318330
}

0 commit comments

Comments
 (0)