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

Commitb1ece73

Browse files
committed
chore: remove resetting user's roles on misconfigured
1 parentca8e9b9 commitb1ece73

File tree

3 files changed

+88
-35
lines changed

3 files changed

+88
-35
lines changed

‎coderd/idpsync/group_test.go‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,7 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
756756
OrganizationID:org.ID,
757757
})
758758
}
759+
759760
iflen(def.OrganizationRoles)>0 {
760761
_,err:=db.UpdateMemberRoles(context.Background(), database.UpdateMemberRolesParams{
761762
GrantedRoles:def.OrganizationRoles,
@@ -764,6 +765,24 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
764765
})
765766
require.NoError(t,err)
766767
}
768+
769+
iflen(def.CustomRoles)>0 {
770+
for_,cr:=rangedef.CustomRoles {
771+
_,err:=db.InsertCustomRole(context.Background(), database.InsertCustomRoleParams{
772+
Name:cr,
773+
DisplayName:cr,
774+
OrganizationID: uuid.NullUUID{
775+
UUID:org.ID,
776+
Valid:true,
777+
},
778+
SitePermissions:nil,
779+
OrgPermissions:nil,
780+
UserPermissions:nil,
781+
})
782+
require.NoError(t,err)
783+
}
784+
}
785+
767786
forgroupID,in:=rangedef.Groups {
768787
dbgen.Group(t,db, database.Group{
769788
ID:groupID,
@@ -793,10 +812,10 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
793812
typeorgSetupDefinitionstruct {
794813
Namestring
795814
// True if the user is a member of the group
796-
Groupsmap[uuid.UUID]bool
797-
GroupNamesmap[string]bool
798-
815+
Groupsmap[uuid.UUID]bool
816+
GroupNamesmap[string]bool
799817
OrganizationRoles []string
818+
CustomRoles []string
800819
// NotMember if true will ensure the user is not a member of the organization.
801820
NotMemberbool
802821

@@ -839,7 +858,8 @@ func (o orgSetupDefinition) Assert(t *testing.T, orgID uuid.UUID, db database.St
839858
o.assertRoles.Assert(t,orgID,db,o.NotMember,user)
840859
}
841860

842-
ifo.assertGroups==nil&&o.assertRoles==nil {
861+
// If the user is not a member, there is nothing to really assert in the org
862+
ifo.assertGroups==nil&&o.assertRoles==nil&&!o.NotMember {
843863
t.Errorf("no group or role asserts present, must have at least one")
844864
t.FailNow()
845865
}

‎coderd/idpsync/role.go‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,16 @@ func (s AGPLIDPSync) SyncRoles(ctx context.Context, db database.Store, user data
9898
slog.Error(err),
9999
)
100100

101-
// Failing role sync should reset a user's roles.
102-
expectedRoles[orgID]= []rbac.RoleIdentifier{}
101+
// TODO: If rolesync fails, we might want to reset a user's
102+
// roles to prevent stale roles from existing.
103+
// Eg: `expectedRoles[orgID] = []rbac.RoleIdentifier{}`
104+
// However, implementing this could lock an org admin out
105+
// of fixing their configuration.
106+
// There is also no current method to notify an org admin of
107+
// a configuration issue.
108+
// So until org admins can be notified of configuration issues,
109+
// and they will not be locked out, this code will do nothing to
110+
// the user's roles.
103111

104112
// Do not return an error, because that would prevent a user
105113
// from logging in. A misconfigured organization should not
@@ -172,28 +180,6 @@ func (s AGPLIDPSync) SyncRoles(ctx context.Context, db database.Store, user data
172180
returnnil
173181
}
174182

175-
// resetUserOrgRoles will reset the user's roles for a specific organization.
176-
// It does not remove them as a member from the organization.
177-
func (sAGPLIDPSync)resetUserOrgRoles(ctx context.Context,tx database.Store,member database.OrganizationMembersRow,orgID uuid.UUID)error {
178-
withoutMember:=slices.DeleteFunc(member.OrganizationMember.Roles,func(sstring)bool {
179-
returns==rbac.RoleOrgMember()
180-
})
181-
// If the user has no roles, then skip doing any database request.
182-
iflen(withoutMember)==0 {
183-
returnnil
184-
}
185-
186-
_,err:=tx.UpdateMemberRoles(ctx, database.UpdateMemberRolesParams{
187-
GrantedRoles: []string{},
188-
UserID:member.OrganizationMember.UserID,
189-
OrgID:orgID,
190-
})
191-
iferr!=nil {
192-
returnxerrors.Errorf("zero out member roles(%s): %w",member.OrganizationMember.UserID.String(),err)
193-
}
194-
returnnil
195-
}
196-
197183
func (sAGPLIDPSync)syncSiteWideRoles(ctx context.Context,tx database.Store,user database.User,paramsRoleParams)error {
198184
// Apply site wide roles to a user.
199185
// ignored is the list of roles that are not valid Coder roles and will

‎coderd/idpsync/role_test.go‎

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ func TestRoleSyncTable(t *testing.T) {
3131
"create-bar","create-baz",
3232
"legacy-bar",rbac.RoleOrgAuditor(),
3333
},
34+
// bad-claim is a number, and will fail any role sync
35+
"bad-claim":100,
3436
}
3537

3638
//ids := coderdtest.NewDeterministicUUIDGenerator()
@@ -43,41 +45,83 @@ func TestRoleSyncTable(t *testing.T) {
4345
},
4446
},
4547
{
46-
Name:"NoSyncNoChange",
48+
Name:"SyncDisabled",
4749
OrganizationRoles: []string{
4850
rbac.RoleOrgAdmin(),
4951
},
52+
RoleSettings:&idpsync.RoleSyncSettings{},
5053
assertRoles:&orgRoleAssert{
5154
ExpectedOrgRoles: []string{
5255
rbac.RoleOrgAdmin(),
5356
},
5457
},
5558
},
5659
{
57-
Name:"NoChange",
60+
// Audit role from claim
61+
Name:"RawAudit",
5862
OrganizationRoles: []string{
5963
rbac.RoleOrgAdmin(),
6064
},
61-
RoleSettings:&idpsync.RoleSyncSettings{},
65+
RoleSettings:&idpsync.RoleSyncSettings{
66+
Field:"roles",
67+
Mapping:map[string][]string{},
68+
},
6269
assertRoles:&orgRoleAssert{
6370
ExpectedOrgRoles: []string{
64-
rbac.RoleOrgAdmin(),
71+
rbac.RoleOrgAuditor(),
6572
},
6673
},
6774
},
6875
{
69-
// Audit role from claim
70-
Name:"RawAudit",
76+
Name:"CustomRole",
7177
OrganizationRoles: []string{
7278
rbac.RoleOrgAdmin(),
7379
},
80+
CustomRoles: []string{"foo"},
7481
RoleSettings:&idpsync.RoleSyncSettings{
7582
Field:"roles",
7683
Mapping:map[string][]string{},
7784
},
7885
assertRoles:&orgRoleAssert{
7986
ExpectedOrgRoles: []string{
8087
rbac.RoleOrgAuditor(),
88+
"foo",
89+
},
90+
},
91+
},
92+
{
93+
Name:"RoleMapping",
94+
OrganizationRoles: []string{
95+
rbac.RoleOrgAdmin(),
96+
"invalid",// Throw in an extra invalid role that will be removed
97+
},
98+
CustomRoles: []string{"custom"},
99+
RoleSettings:&idpsync.RoleSyncSettings{
100+
Field:"roles",
101+
Mapping:map[string][]string{
102+
"foo": {"custom",rbac.RoleOrgTemplateAdmin()},
103+
},
104+
},
105+
assertRoles:&orgRoleAssert{
106+
ExpectedOrgRoles: []string{
107+
rbac.RoleOrgAuditor(),
108+
rbac.RoleOrgTemplateAdmin(),
109+
"custom",
110+
},
111+
},
112+
},
113+
{
114+
// InvalidClaims will log an error, but do not block authentication.
115+
// This is to prevent a misconfigured organization from blocking
116+
// a user from authenticating.
117+
Name:"InvalidClaim",
118+
OrganizationRoles: []string{rbac.RoleOrgAdmin()},
119+
RoleSettings:&idpsync.RoleSyncSettings{
120+
Field:"bad-claim",
121+
},
122+
assertRoles:&orgRoleAssert{
123+
ExpectedOrgRoles: []string{
124+
rbac.RoleOrgAdmin(),
81125
},
82126
},
83127
},
@@ -90,7 +134,9 @@ func TestRoleSyncTable(t *testing.T) {
90134

91135
db,_:=dbtestutil.NewDB(t)
92136
manager:=runtimeconfig.NewManager()
93-
s:=idpsync.NewAGPLSync(slogtest.Make(t,&slogtest.Options{}),
137+
s:=idpsync.NewAGPLSync(slogtest.Make(t,&slogtest.Options{
138+
IgnoreErrors:true,
139+
}),
94140
manager,
95141
idpsync.DeploymentSyncSettings{
96142
SiteRoleField:"roles",
@@ -105,6 +151,7 @@ func TestRoleSyncTable(t *testing.T) {
105151
// Do the group sync!
106152
err:=s.SyncRoles(ctx,db,user, idpsync.RoleParams{
107153
SyncEnabled:true,
154+
SyncSiteWide:false,
108155
MergedClaims:userClaims,
109156
})
110157
require.NoError(t,err)

0 commit comments

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