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

chore: remove organization_id suffix from org_member roles in database#13473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 7 commits intomainfromstevenmasley/org_roles_db_store
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletioncli/server_createadminuser.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -222,7 +222,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *serpent.Command {
UserID:newUser.ID,
CreatedAt:dbtime.Now(),
UpdatedAt:dbtime.Now(),
Roles: []string{rbac.RoleOrgAdmin(org.ID)},
Roles: []string{rbac.ScopedRoleOrgAdmin(org.ID)},
})
iferr!=nil {
returnxerrors.Errorf("insert organization member: %w",err)
Expand Down
2 changes: 1 addition & 1 deletioncli/server_createadminuser_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -71,7 +71,7 @@ func TestServerCreateAdminUser(t *testing.T) {
orgIDs2:=make(map[uuid.UUID]struct{},len(orgMemberships))
for_,membership:=rangeorgMemberships {
orgIDs2[membership.OrganizationID]=struct{}{}
assert.Equal(t, []string{rbac.RoleOrgAdmin(membership.OrganizationID)},membership.Roles,"user is not org admin")
assert.Equal(t, []string{rbac.ScopedRoleOrgAdmin(membership.OrganizationID)},membership.Roles,"user is not org admin")
}

require.Equal(t,orgIDs,orgIDs2,"user is not in all orgs")
Expand Down
3 changes: 3 additions & 0 deletionscoderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

3 changes: 3 additions & 0 deletionscoderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

2 changes: 1 addition & 1 deletioncoderd/authorize_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,7 +27,7 @@ func TestCheckPermissions(t *testing.T) {
memberClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID)
memberUser,err:=memberClient.User(ctx,codersdk.Me)
require.NoError(t,err)
orgAdminClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID,rbac.RoleOrgAdmin(adminUser.OrganizationID))
orgAdminClient,_:=coderdtest.CreateAnotherUser(t,adminClient,adminUser.OrganizationID,rbac.ScopedRoleOrgAdmin(adminUser.OrganizationID))
orgAdminUser,err:=orgAdminClient.User(ctx,codersdk.Me)
require.NoError(t,err)

Expand Down
2 changes: 1 addition & 1 deletioncoderd/batchstats/batcher_internal_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -177,7 +177,7 @@ func setupDeps(t *testing.T, store database.Store, ps pubsub.Pubsub) deps {
_,err:=store.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
OrganizationID:org.ID,
UserID:user.ID,
Roles: []string{rbac.RoleOrgMember(org.ID)},
Roles: []string{rbac.ScopedRoleOrgMember(org.ID)},
})
require.NoError(t,err)
tv:=dbgen.TemplateVersion(t,store, database.TemplateVersion{
Expand Down
5 changes: 4 additions & 1 deletioncoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -663,6 +663,7 @@ func CreateFirstUser(t testing.TB, client *codersdk.Client) codersdk.CreateFirst
}

// CreateAnotherUser creates and authenticates a new user.
// Roles can include org scoped roles with 'roleName:<organization_id>'
func CreateAnotherUser(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles ...string) (*codersdk.Client, codersdk.User) {
return createAnotherUserRetry(t, client, organizationID, 5, roles)
}
Expand All@@ -680,7 +681,7 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
roles = append(roles, r.Name)
}
// We assume only 1 org exists
roles = append(roles, rbac.RoleOrgMember(orgID))
roles = append(roles, rbac.ScopedRoleOrgMember(orgID))

return rbac.Subject{
ID: user.ID.String(),
Expand DownExpand Up@@ -754,6 +755,8 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI
for _, roleName := range roles {
roleName := roleName
orgID, ok := rbac.IsOrgRole(roleName)
roleName, _, err = rbac.RoleSplit(roleName)
require.NoError(t, err, "split org role name")
if ok {
orgRoles[orgID] = append(orgRoles[orgID], roleName)
} else {
Expand Down
20 changes: 13 additions & 7 deletionscoderd/database/db2sdk/db2sdk.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -204,13 +204,6 @@ func Group(group database.Group, members []database.User) codersdk.Group {
}
}

func SlimRole(role rbac.Role) codersdk.SlimRole {
return codersdk.SlimRole{
DisplayName: role.DisplayName,
Name: role.Name,
}
}

func TemplateInsightsParameters(parameterRows []database.GetTemplateParameterInsightsRow) ([]codersdk.TemplateParameterUsage, error) {
// Use a stable sort, similarly to how we would sort in the query, note that
// we don't sort in the query because order varies depending on the table
Expand DownExpand Up@@ -525,6 +518,19 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
return result
}

func SlimRole(role rbac.Role) codersdk.SlimRole {
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
if err != nil {
roleName = role.Name
}

return codersdk.SlimRole{
DisplayName: role.DisplayName,
Name: roleName,
OrganizationID: orgIDStr,
}
}

func RBACRole(role rbac.Role) codersdk.Role {
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletionscoderd/database/dbauthz/customroles_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -153,7 +153,7 @@ func TestUpsertCustomRoles(t *testing.T) {
UUID:uuid.New(),
Valid:true,
},
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
org:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
codersdk.ResourceWorkspace: {codersdk.ActionRead},
}),
Expand All@@ -162,7 +162,7 @@ func TestUpsertCustomRoles(t *testing.T) {
{
name:"user-escalation",
// These roles do not grant user perms
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
user:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
codersdk.ResourceWorkspace: {codersdk.ActionRead},
}),
Expand DownExpand Up@@ -190,7 +190,7 @@ func TestUpsertCustomRoles(t *testing.T) {
},
{
name:"read-workspace-in-org",
subject:merge(canAssignRole,rbac.RoleOrgAdmin(orgID.UUID)),
subject:merge(canAssignRole,rbac.ScopedRoleOrgAdmin(orgID.UUID)),
organizationID:orgID,
org:codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
codersdk.ResourceWorkspace: {codersdk.ActionRead},
Expand Down
18 changes: 16 additions & 2 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2472,7 +2472,7 @@ func (q *querier) InsertOrganization(ctx context.Context, arg database.InsertOrg

func (q *querier) InsertOrganizationMember(ctx context.Context, arg database.InsertOrganizationMemberParams) (database.OrganizationMember, error) {
// All roles are added roles. Org member is always implied.
addedRoles := append(arg.Roles, rbac.RoleOrgMember(arg.OrganizationID))
addedRoles := append(arg.Roles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
err := q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []string{})
if err != nil {
return database.OrganizationMember{}, err
Expand DownExpand Up@@ -2847,8 +2847,22 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
return database.OrganizationMember{}, err
}

// The 'rbac' package expects role names to be scoped.
// Convert the argument roles for validation.
scopedGranted := make([]string, 0, len(arg.GrantedRoles))
for _, grantedRole := range arg.GrantedRoles {
// This check is a developer safety check. Old code might try to invoke this code path with
// organization id suffixes. Catch this and return a nice error so it can be fixed.
_, foundOrg, _ := rbac.RoleSplit(grantedRole)
if foundOrg != "" {
return database.OrganizationMember{}, xerrors.Errorf("attempt to assign a role %q, remove the ':<organization_id> suffix", grantedRole)
}
Comment on lines +2854 to +2859
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is not required, but added as a safety check to catch dev errors from using the previousrolename:<org_id> syntax

johnstcn reacted with thumbs up emoji

scopedGranted = append(scopedGranted, rbac.RoleName(grantedRole, arg.OrgID.String()))
}

// The org member role is always implied.
impliedTypes := append(arg.GrantedRoles, rbac.RoleOrgMember(arg.OrgID))
impliedTypes := append(scopedGranted, rbac.ScopedRoleOrgMember(arg.OrgID))
added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
err = q.canAssignRoles(ctx, &arg.OrgID, added, removed)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletionscoderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ func (s *MethodTestSuite) TestOrganization() {
check.Args(database.InsertOrganizationMemberParams{
OrganizationID:o.ID,
UserID:u.ID,
Roles: []string{rbac.RoleOrgAdmin(o.ID)},
Roles: []string{rbac.ScopedRoleOrgAdmin(o.ID)},
}).Asserts(
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionAssign,
rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID),policy.ActionCreate)
Expand DownExpand Up@@ -664,7 +664,7 @@ func (s *MethodTestSuite) TestOrganization() {
mem:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{
OrganizationID:o.ID,
UserID:u.ID,
Roles: []string{rbac.RoleOrgAdmin(o.ID)},
Roles: []string{rbac.ScopedRoleOrgAdmin(o.ID)},
})
out:=mem
out.Roles= []string{}
Expand Down
4 changes: 3 additions & 1 deletioncoderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1997,7 +1997,9 @@ func (q *FakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U

for _, mem := range q.organizationMembers {
if mem.UserID == userID {
roles = append(roles, mem.Roles...)
for _, orgRole := range mem.Roles {
roles = append(roles, orgRole+":"+mem.OrganizationID.String())
}
roles = append(roles, "organization-member:"+mem.OrganizationID.String())
}
}
Expand Down
2 changes: 1 addition & 1 deletioncoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY organization_members ALTER COLUMN roles SET DEFAULT '{organization-member}';
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
-- The default was 'organization-member', but we imply that in the
-- 'GetAuthorizationUserRoles' query.
ALTER TABLE ONLY organization_members ALTER COLUMN roles SET DEFAULT '{}';

-- No one should be using organization roles yet. If they are, the names in the
-- database are now incorrect. Just remove them all.
UPDATE organization_members SET roles = '{}';
Comment on lines +1 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

another good thing to call out

Emyrk reacted with thumbs up emoji
8 changes: 5 additions & 3 deletionscoderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

8 changes: 5 additions & 3 deletionscoderd/database/queries/users.sql
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -227,12 +227,14 @@ SELECT
array_append(users.rbac_roles, 'member'),
(
SELECT
array_agg(org_roles)
-- The roles are returned as a flat array, org scoped and site side.
-- Concatenating the organization id scopes the organization roles.
array_agg(org_roles || ':' || organization_members.organization_id::text)
FROM
organization_members,
-- All org_members get theorg-member role for their orgs
-- All org_members get theorganization-member role for their orgs
unnest(
array_append(roles, 'organization-member:' || organization_members.organization_id::text)
array_append(roles, 'organization-member')
) AS org_roles
WHERE
user_id = users.id
Expand Down
8 changes: 4 additions & 4 deletionscoderd/httpmw/authorize_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,7 +68,7 @@ func TestExtractUserRoles(t *testing.T) {
Roles: orgRoles,
})
require.NoError(t, err)
return user, append(roles, append(orgRoles, rbac.RoleMember(), rbac.RoleOrgMember(org.ID))...), token
return user, append(roles, append(orgRoles, rbac.RoleMember(), rbac.ScopedRoleOrgMember(org.ID))...), token
},
},
{
Expand All@@ -89,7 +89,8 @@ func TestExtractUserRoles(t *testing.T) {

orgRoles := []string{}
if i%2 == 0 {
orgRoles = append(orgRoles, rbac.RoleOrgAdmin(organization.ID))
orgRoles = append(orgRoles, rbac.RoleOrgAdmin())
roles = append(roles, rbac.ScopedRoleOrgAdmin(organization.ID))
}
_, err = db.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
OrganizationID: organization.ID,
Expand All@@ -99,8 +100,7 @@ func TestExtractUserRoles(t *testing.T) {
Roles: orgRoles,
})
require.NoError(t, err)
roles = append(roles, orgRoles...)
roles = append(roles, rbac.RoleOrgMember(organization.ID))
roles = append(roles, rbac.ScopedRoleOrgMember(organization.ID))
}
return user, roles, token
},
Expand Down
2 changes: 1 addition & 1 deletioncoderd/httpmw/organizationparam_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -152,7 +152,7 @@ func TestOrganizationParam(t *testing.T) {
_=dbgen.OrganizationMember(t,db, database.OrganizationMember{
OrganizationID:organization.ID,
UserID:user.ID,
Roles: []string{rbac.RoleOrgMember(organization.ID)},
Roles: []string{rbac.ScopedRoleOrgMember(organization.ID)},
})
_,err:=db.UpdateUserRoles(ctx, database.UpdateUserRolesParams{
ID:user.ID,
Expand Down
2 changes: 1 addition & 1 deletioncoderd/members.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,7 +68,7 @@ func convertOrganizationMember(mem database.OrganizationMember) codersdk.Organiz
}

for _, roleName := range mem.Roles {
rbacRole, _ := rbac.RoleByName(roleName)
rbacRole, _ := rbac.RoleByName(rbac.RoleName(roleName, mem.OrganizationID.String()))
convertedMember.Roles = append(convertedMember.Roles, db2sdk.SlimRole(rbacRole))
}
return convertedMember
Expand Down
2 changes: 1 addition & 1 deletioncoderd/organizations.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -94,7 +94,7 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
// come back to determining the default role of the person who
// creates the org. Until that happens, all users in an organization
// should be just regular members.
rbac.RoleOrgMember(organization.ID),
rbac.ScopedRoleOrgMember(organization.ID),
},
})
iferr!=nil {
Expand Down
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp