Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8f62311

Browse files
authored
chore: remove organization_id suffix from org_member roles in database (#13473)
Organization member's table is already scoped to an organization.Rolename should avoid having the org_id appended.Wipes all existing organization role assignments, which should not be used anyway.
1 parentfade8ba commit8f62311

File tree

38 files changed

+200
-118
lines changed

38 files changed

+200
-118
lines changed

‎cli/server_createadminuser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *serpent.Command {
222222
UserID:newUser.ID,
223223
CreatedAt:dbtime.Now(),
224224
UpdatedAt:dbtime.Now(),
225-
Roles: []string{rbac.RoleOrgAdmin(org.ID)},
225+
Roles: []string{rbac.ScopedRoleOrgAdmin(org.ID)},
226226
})
227227
iferr!=nil {
228228
returnxerrors.Errorf("insert organization member: %w",err)

‎cli/server_createadminuser_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestServerCreateAdminUser(t *testing.T) {
7171
orgIDs2:=make(map[uuid.UUID]struct{},len(orgMemberships))
7272
for_,membership:=rangeorgMemberships {
7373
orgIDs2[membership.OrganizationID]=struct{}{}
74-
assert.Equal(t, []string{rbac.RoleOrgAdmin(membership.OrganizationID)},membership.Roles,"user is not org admin")
74+
assert.Equal(t, []string{rbac.ScopedRoleOrgAdmin(membership.OrganizationID)},membership.Roles,"user is not org admin")
7575
}
7676

7777
require.Equal(t,orgIDs,orgIDs2,"user is not in all orgs")

‎coderd/apidoc/docs.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/authorize_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestCheckPermissions(t *testing.T) {
2727
memberClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID)
2828
memberUser,err:=memberClient.User(ctx,codersdk.Me)
2929
require.NoError(t,err)
30-
orgAdminClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID,rbac.RoleOrgAdmin(adminUser.OrganizationID))
30+
orgAdminClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID,rbac.ScopedRoleOrgAdmin(adminUser.OrganizationID))
3131
orgAdminUser,err:=orgAdminClient.User(ctx,codersdk.Me)
3232
require.NoError(t,err)
3333

‎coderd/batchstats/batcher_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func setupDeps(t *testing.T, store database.Store, ps pubsub.Pubsub) deps {
177177
_,err:=store.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
178178
OrganizationID:org.ID,
179179
UserID:user.ID,
180-
Roles: []string{rbac.RoleOrgMember(org.ID)},
180+
Roles: []string{rbac.ScopedRoleOrgMember(org.ID)},
181181
})
182182
require.NoError(t,err)
183183
tv:=dbgen.TemplateVersion(t,store, database.TemplateVersion{

‎coderd/coderdtest/coderdtest.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,7 @@ func CreateFirstUser(t testing.TB, client *codersdk.Client) codersdk.CreateFirst
663663
}
664664

665665
// CreateAnotherUser creates and authenticates a new user.
666+
// Roles can include org scoped roles with 'roleName:<organization_id>'
666667
funcCreateAnotherUser(t testing.TB,client*codersdk.Client,organizationID uuid.UUID,roles...string) (*codersdk.Client, codersdk.User) {
667668
returncreateAnotherUserRetry(t,client,organizationID,5,roles)
668669
}
@@ -680,7 +681,7 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
680681
roles=append(roles,r.Name)
681682
}
682683
// We assume only 1 org exists
683-
roles=append(roles,rbac.RoleOrgMember(orgID))
684+
roles=append(roles,rbac.ScopedRoleOrgMember(orgID))
684685

685686
return rbac.Subject{
686687
ID:user.ID.String(),
@@ -754,6 +755,8 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI
754755
for_,roleName:=rangeroles {
755756
roleName:=roleName
756757
orgID,ok:=rbac.IsOrgRole(roleName)
758+
roleName,_,err=rbac.RoleSplit(roleName)
759+
require.NoError(t,err,"split org role name")
757760
ifok {
758761
orgRoles[orgID]=append(orgRoles[orgID],roleName)
759762
}else {

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,6 @@ func Group(group database.Group, members []database.User) codersdk.Group {
204204
}
205205
}
206206

207-
funcSlimRole(role rbac.Role) codersdk.SlimRole {
208-
return codersdk.SlimRole{
209-
DisplayName:role.DisplayName,
210-
Name:role.Name,
211-
}
212-
}
213-
214207
funcTemplateInsightsParameters(parameterRows []database.GetTemplateParameterInsightsRow) ([]codersdk.TemplateParameterUsage,error) {
215208
// Use a stable sort, similarly to how we would sort in the query, note that
216209
// we don't sort in the query because order varies depending on the table
@@ -525,6 +518,19 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
525518
returnresult
526519
}
527520

521+
funcSlimRole(role rbac.Role) codersdk.SlimRole {
522+
roleName,orgIDStr,err:=rbac.RoleSplit(role.Name)
523+
iferr!=nil {
524+
roleName=role.Name
525+
}
526+
527+
return codersdk.SlimRole{
528+
DisplayName:role.DisplayName,
529+
Name:roleName,
530+
OrganizationID:orgIDStr,
531+
}
532+
}
533+
528534
funcRBACRole(role rbac.Role) codersdk.Role {
529535
roleName,orgIDStr,err:=rbac.RoleSplit(role.Name)
530536
iferr!=nil {

‎coderd/database/dbauthz/customroles_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func TestUpsertCustomRoles(t *testing.T) {
153153
UUID:uuid.New(),
154154
Valid:true,
155155
},
156-
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
156+
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
157157
org:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
158158
codersdk.ResourceWorkspace: {codersdk.ActionRead},
159159
}),
@@ -162,7 +162,7 @@ func TestUpsertCustomRoles(t *testing.T) {
162162
{
163163
name:"user-escalation",
164164
// These roles do not grant user perms
165-
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
165+
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
166166
user:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
167167
codersdk.ResourceWorkspace: {codersdk.ActionRead},
168168
}),
@@ -190,7 +190,7 @@ func TestUpsertCustomRoles(t *testing.T) {
190190
},
191191
{
192192
name:"read-workspace-in-org",
193-
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
193+
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
194194
organizationID:orgID,
195195
org:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
196196
codersdk.ResourceWorkspace: {codersdk.ActionRead},

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2472,7 +2472,7 @@ func (q *querier) InsertOrganization(ctx context.Context, arg database.InsertOrg
24722472

24732473
func (q*querier)InsertOrganizationMember(ctx context.Context,arg database.InsertOrganizationMemberParams) (database.OrganizationMember,error) {
24742474
// All roles are added roles. Org member is always implied.
2475-
addedRoles:=append(arg.Roles,rbac.RoleOrgMember(arg.OrganizationID))
2475+
addedRoles:=append(arg.Roles,rbac.ScopedRoleOrgMember(arg.OrganizationID))
24762476
err:=q.canAssignRoles(ctx,&arg.OrganizationID,addedRoles, []string{})
24772477
iferr!=nil {
24782478
return database.OrganizationMember{},err
@@ -2847,8 +2847,22 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
28472847
return database.OrganizationMember{},err
28482848
}
28492849

2850+
// The 'rbac' package expects role names to be scoped.
2851+
// Convert the argument roles for validation.
2852+
scopedGranted:=make([]string,0,len(arg.GrantedRoles))
2853+
for_,grantedRole:=rangearg.GrantedRoles {
2854+
// This check is a developer safety check. Old code might try to invoke this code path with
2855+
// organization id suffixes. Catch this and return a nice error so it can be fixed.
2856+
_,foundOrg,_:=rbac.RoleSplit(grantedRole)
2857+
iffoundOrg!="" {
2858+
return database.OrganizationMember{},xerrors.Errorf("attempt to assign a role %q, remove the ':<organization_id> suffix",grantedRole)
2859+
}
2860+
2861+
scopedGranted=append(scopedGranted,rbac.RoleName(grantedRole,arg.OrgID.String()))
2862+
}
2863+
28502864
// The org member role is always implied.
2851-
impliedTypes:=append(arg.GrantedRoles,rbac.RoleOrgMember(arg.OrgID))
2865+
impliedTypes:=append(scopedGranted,rbac.ScopedRoleOrgMember(arg.OrgID))
28522866
added,removed:=rbac.ChangeRoleSet(member.Roles,impliedTypes)
28532867
err=q.canAssignRoles(ctx,&arg.OrgID,added,removed)
28542868
iferr!=nil {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ func (s *MethodTestSuite) TestOrganization() {
636636
check.Args(database.InsertOrganizationMemberParams{
637637
OrganizationID:o.ID,
638638
UserID:u.ID,
639-
Roles: []string{rbac.RoleOrgAdmin(o.ID)},
639+
Roles: []string{rbac.ScopedRoleOrgAdmin(o.ID)},
640640
}).Asserts(
641641
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionAssign,
642642
rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID),policy.ActionCreate)
@@ -664,7 +664,7 @@ func (s *MethodTestSuite) TestOrganization() {
664664
mem:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{
665665
OrganizationID:o.ID,
666666
UserID:u.ID,
667-
Roles: []string{rbac.RoleOrgAdmin(o.ID)},
667+
Roles: []string{rbac.ScopedRoleOrgAdmin(o.ID)},
668668
})
669669
out:=mem
670670
out.Roles= []string{}

‎coderd/database/dbmem/dbmem.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1997,7 +1997,9 @@ func (q *FakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U
19971997

19981998
for_,mem:=rangeq.organizationMembers {
19991999
ifmem.UserID==userID {
2000-
roles=append(roles,mem.Roles...)
2000+
for_,orgRole:=rangemem.Roles {
2001+
roles=append(roles,orgRole+":"+mem.OrganizationID.String())
2002+
}
20012003
roles=append(roles,"organization-member:"+mem.OrganizationID.String())
20022004
}
20032005
}

‎coderd/database/dump.sql

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTERTABLE ONLY organization_members ALTER COLUMN rolesSET DEFAULT'{organization-member}';
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-- The default was 'organization-member', but we imply that in the
2+
-- 'GetAuthorizationUserRoles' query.
3+
ALTERTABLE ONLY organization_members ALTER COLUMN rolesSET DEFAULT'{}';
4+
5+
-- No one should be using organization roles yet. If they are, the names in the
6+
-- database are now incorrect. Just remove them all.
7+
UPDATE organization_membersSET roles='{}';

‎coderd/database/queries.sql.go

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/users.sql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,14 @@ SELECT
227227
array_append(users.rbac_roles,'member'),
228228
(
229229
SELECT
230-
array_agg(org_roles)
230+
-- The roles are returned as a flat array, org scoped and site side.
231+
-- Concatenating the organization id scopes the organization roles.
232+
array_agg(org_roles||':'||organization_members.organization_id::text)
231233
FROM
232234
organization_members,
233-
-- All org_members get theorg-member role for their orgs
235+
-- All org_members get theorganization-member role for their orgs
234236
unnest(
235-
array_append(roles,'organization-member:'||organization_members.organization_id::text)
237+
array_append(roles,'organization-member')
236238
)AS org_roles
237239
WHERE
238240
user_id=users.id

‎coderd/httpmw/authorize_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func TestExtractUserRoles(t *testing.T) {
6868
Roles:orgRoles,
6969
})
7070
require.NoError(t,err)
71-
returnuser,append(roles,append(orgRoles,rbac.RoleMember(),rbac.RoleOrgMember(org.ID))...),token
71+
returnuser,append(roles,append(orgRoles,rbac.RoleMember(),rbac.ScopedRoleOrgMember(org.ID))...),token
7272
},
7373
},
7474
{
@@ -89,7 +89,8 @@ func TestExtractUserRoles(t *testing.T) {
8989

9090
orgRoles:= []string{}
9191
ifi%2==0 {
92-
orgRoles=append(orgRoles,rbac.RoleOrgAdmin(organization.ID))
92+
orgRoles=append(orgRoles,rbac.RoleOrgAdmin())
93+
roles=append(roles,rbac.ScopedRoleOrgAdmin(organization.ID))
9394
}
9495
_,err=db.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
9596
OrganizationID:organization.ID,
@@ -99,8 +100,7 @@ func TestExtractUserRoles(t *testing.T) {
99100
Roles:orgRoles,
100101
})
101102
require.NoError(t,err)
102-
roles=append(roles,orgRoles...)
103-
roles=append(roles,rbac.RoleOrgMember(organization.ID))
103+
roles=append(roles,rbac.ScopedRoleOrgMember(organization.ID))
104104
}
105105
returnuser,roles,token
106106
},

‎coderd/httpmw/organizationparam_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func TestOrganizationParam(t *testing.T) {
152152
_=dbgen.OrganizationMember(t,db, database.OrganizationMember{
153153
OrganizationID:organization.ID,
154154
UserID:user.ID,
155-
Roles: []string{rbac.RoleOrgMember(organization.ID)},
155+
Roles: []string{rbac.ScopedRoleOrgMember(organization.ID)},
156156
})
157157
_,err:=db.UpdateUserRoles(ctx, database.UpdateUserRolesParams{
158158
ID:user.ID,

‎coderd/members.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func convertOrganizationMember(mem database.OrganizationMember) codersdk.Organiz
6868
}
6969

7070
for_,roleName:=rangemem.Roles {
71-
rbacRole,_:=rbac.RoleByName(roleName)
71+
rbacRole,_:=rbac.RoleByName(rbac.RoleName(roleName,mem.OrganizationID.String()))
7272
convertedMember.Roles=append(convertedMember.Roles,db2sdk.SlimRole(rbacRole))
7373
}
7474
returnconvertedMember

‎coderd/organizations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
9494
// come back to determining the default role of the person who
9595
// creates the org. Until that happens, all users in an organization
9696
// should be just regular members.
97-
rbac.RoleOrgMember(organization.ID),
97+
rbac.ScopedRoleOrgMember(organization.ID),
9898
},
9999
})
100100
iferr!=nil {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp