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

Commit1e5438e

Browse files
authored
feat: remove user from groups on org membership delete (#14701)
* feat: remove user from groups on org membership deleteGroups inherently provide authz access to certain resources. If auser is removed from an organization, they should be removedfrom all their groups in said organization.
1 parentc145f11 commit1e5438e

File tree

6 files changed

+196
-1
lines changed

6 files changed

+196
-1
lines changed

‎coderd/database/dbmem/dbmem.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1944,7 +1944,7 @@ func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error
19441944
returnsql.ErrNoRows
19451945
}
19461946

1947-
func (q*FakeQuerier)DeleteOrganizationMember(_ context.Context,arg database.DeleteOrganizationMemberParams)error {
1947+
func (q*FakeQuerier)DeleteOrganizationMember(ctx context.Context,arg database.DeleteOrganizationMemberParams)error {
19481948
err:=validateDatabaseType(arg)
19491949
iferr!=nil {
19501950
returnerr
@@ -1959,6 +1959,16 @@ func (q *FakeQuerier) DeleteOrganizationMember(_ context.Context, arg database.D
19591959
iflen(deleted)==0 {
19601960
returnsql.ErrNoRows
19611961
}
1962+
1963+
// Delete group member trigger
1964+
q.groupMembers=slices.DeleteFunc(q.groupMembers,func(member database.GroupMemberTable)bool {
1965+
ifmember.UserID!=arg.UserID {
1966+
returnfalse
1967+
}
1968+
g,_:=q.getGroupByIDNoLock(ctx,member.GroupID)
1969+
returng.OrganizationID==arg.OrganizationID
1970+
})
1971+
19621972
returnnil
19631973
}
19641974

‎coderd/database/dump.sql

Lines changed: 21 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
DROPTRIGGER IF EXISTS trigger_delete_group_members_on_org_member_deleteON organization_members;
2+
DROPFUNCTION IF EXISTS delete_group_members_on_org_member_delete;
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
CREATEFUNCTIONdelete_group_members_on_org_member_delete() RETURNS TRIGGER
2+
LANGUAGE plpgsql
3+
AS $$
4+
DECLARE
5+
BEGIN
6+
-- Remove the user from all groups associated with the same
7+
-- organization as the organization_member being deleted.
8+
DELETEFROM group_members
9+
WHERE
10+
user_id=OLD.user_id
11+
AND group_idIN (
12+
SELECT id
13+
FROM groups
14+
WHERE organization_id=OLD.organization_id
15+
);
16+
RETURN OLD;
17+
END;
18+
$$;
19+
20+
CREATETRIGGERtrigger_delete_group_members_on_org_member_delete
21+
BEFOREDELETEON organization_members
22+
FOR EACH ROW
23+
EXECUTE PROCEDURE delete_group_members_on_org_member_delete();

‎coderd/database/querier_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,103 @@ func TestExpectOne(t *testing.T) {
12161216
})
12171217
}
12181218

1219+
funcTestGroupRemovalTrigger(t*testing.T) {
1220+
t.Parallel()
1221+
1222+
db,_:=dbtestutil.NewDB(t)
1223+
1224+
orgA:=dbgen.Organization(t,db, database.Organization{})
1225+
_,err:=db.InsertAllUsersGroup(context.Background(),orgA.ID)
1226+
require.NoError(t,err)
1227+
1228+
orgB:=dbgen.Organization(t,db, database.Organization{})
1229+
_,err=db.InsertAllUsersGroup(context.Background(),orgB.ID)
1230+
require.NoError(t,err)
1231+
1232+
orgs:= []database.Organization{orgA,orgB}
1233+
1234+
user:=dbgen.User(t,db, database.User{})
1235+
extra:=dbgen.User(t,db, database.User{})
1236+
users:= []database.User{user,extra}
1237+
1238+
groupA1:=dbgen.Group(t,db, database.Group{
1239+
OrganizationID:orgA.ID,
1240+
})
1241+
groupA2:=dbgen.Group(t,db, database.Group{
1242+
OrganizationID:orgA.ID,
1243+
})
1244+
1245+
groupB1:=dbgen.Group(t,db, database.Group{
1246+
OrganizationID:orgB.ID,
1247+
})
1248+
groupB2:=dbgen.Group(t,db, database.Group{
1249+
OrganizationID:orgB.ID,
1250+
})
1251+
1252+
groups:= []database.Group{groupA1,groupA2,groupB1,groupB2}
1253+
1254+
// Add users to all organizations
1255+
for_,u:=rangeusers {
1256+
for_,o:=rangeorgs {
1257+
dbgen.OrganizationMember(t,db, database.OrganizationMember{
1258+
OrganizationID:o.ID,
1259+
UserID:u.ID,
1260+
})
1261+
}
1262+
}
1263+
1264+
// Add users to all groups
1265+
for_,u:=rangeusers {
1266+
for_,g:=rangegroups {
1267+
dbgen.GroupMember(t,db, database.GroupMemberTable{
1268+
GroupID:g.ID,
1269+
UserID:u.ID,
1270+
})
1271+
}
1272+
}
1273+
1274+
// Verify user is in all groups
1275+
ctx:=testutil.Context(t,testutil.WaitLong)
1276+
onlyGroupIDs:=func(row database.GetGroupsRow) uuid.UUID {
1277+
returnrow.Group.ID
1278+
}
1279+
userGroups,err:=db.GetGroups(ctx, database.GetGroupsParams{
1280+
HasMemberID:user.ID,
1281+
})
1282+
require.NoError(t,err)
1283+
require.ElementsMatch(t, []uuid.UUID{
1284+
orgA.ID,orgB.ID,// Everyone groups
1285+
groupA1.ID,groupA2.ID,groupB1.ID,groupB2.ID,// Org groups
1286+
},db2sdk.List(userGroups,onlyGroupIDs))
1287+
1288+
// Remove the user from org A
1289+
err=db.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{
1290+
OrganizationID:orgA.ID,
1291+
UserID:user.ID,
1292+
})
1293+
require.NoError(t,err)
1294+
1295+
// Verify user is no longer in org A groups
1296+
userGroups,err=db.GetGroups(ctx, database.GetGroupsParams{
1297+
HasMemberID:user.ID,
1298+
})
1299+
require.NoError(t,err)
1300+
require.ElementsMatch(t, []uuid.UUID{
1301+
orgB.ID,// Everyone group
1302+
groupB1.ID,groupB2.ID,// Org groups
1303+
},db2sdk.List(userGroups,onlyGroupIDs))
1304+
1305+
// Verify extra user is unchanged
1306+
extraUserGroups,err:=db.GetGroups(ctx, database.GetGroupsParams{
1307+
HasMemberID:extra.ID,
1308+
})
1309+
require.NoError(t,err)
1310+
require.ElementsMatch(t, []uuid.UUID{
1311+
orgA.ID,orgB.ID,// Everyone groups
1312+
groupA1.ID,groupA2.ID,groupB1.ID,groupB2.ID,// Org groups
1313+
},db2sdk.List(extraUserGroups,onlyGroupIDs))
1314+
}
1315+
12191316
funcrequireUsersMatch(t testing.TB,expected []database.User,found []database.GetUsersRow,msgstring) {
12201317
t.Helper()
12211318
require.ElementsMatch(t,expected,database.ConvertUserRows(found),msg)

‎enterprise/members_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ func TestEnterpriseMembers(t *testing.T) {
2929
LicenseOptions:&coderdenttest.LicenseOptions{
3030
Features: license.Features{
3131
codersdk.FeatureMultipleOrganizations:1,
32+
codersdk.FeatureTemplateRBAC:1,
3233
},
3334
},
3435
})
@@ -39,6 +40,21 @@ func TestEnterpriseMembers(t *testing.T) {
3940
_,user:=coderdtest.CreateAnotherUser(t,owner,secondOrg.ID)
4041

4142
ctx:=testutil.Context(t,testutil.WaitMedium)
43+
44+
// Groups exist to ensure a user removed from the org loses their
45+
// group access.
46+
g1,err:=orgAdminClient.CreateGroup(ctx,secondOrg.ID, codersdk.CreateGroupRequest{
47+
Name:"foo",
48+
DisplayName:"Foo",
49+
})
50+
require.NoError(t,err)
51+
52+
g2,err:=orgAdminClient.CreateGroup(ctx,secondOrg.ID, codersdk.CreateGroupRequest{
53+
Name:"bar",
54+
DisplayName:"Bar",
55+
})
56+
require.NoError(t,err)
57+
4258
// Verify the org of 3 members
4359
members,err:=orgAdminClient.OrganizationMembers(ctx,secondOrg.ID)
4460
require.NoError(t,err)
@@ -47,6 +63,25 @@ func TestEnterpriseMembers(t *testing.T) {
4763
[]uuid.UUID{first.UserID,user.ID,orgAdmin.ID},
4864
db2sdk.List(members,onlyIDs))
4965

66+
// Add the member to some groups
67+
_,err=orgAdminClient.PatchGroup(ctx,g1.ID, codersdk.PatchGroupRequest{
68+
AddUsers: []string{user.ID.String()},
69+
})
70+
require.NoError(t,err)
71+
72+
_,err=orgAdminClient.PatchGroup(ctx,g2.ID, codersdk.PatchGroupRequest{
73+
AddUsers: []string{user.ID.String()},
74+
})
75+
require.NoError(t,err)
76+
77+
// Verify group membership
78+
userGroups,err:=orgAdminClient.Groups(ctx, codersdk.GroupArguments{
79+
HasMember:user.ID.String(),
80+
})
81+
require.NoError(t,err)
82+
// Everyone group + 2 groups
83+
require.Len(t,userGroups,3)
84+
5085
// Delete a member
5186
err=orgAdminClient.DeleteOrganizationMember(ctx,secondOrg.ID,user.Username)
5287
require.NoError(t,err)
@@ -57,6 +92,13 @@ func TestEnterpriseMembers(t *testing.T) {
5792
require.ElementsMatch(t,
5893
[]uuid.UUID{first.UserID,orgAdmin.ID},
5994
db2sdk.List(members,onlyIDs))
95+
96+
// User should now belong to 0 groups
97+
userGroups,err=orgAdminClient.Groups(ctx, codersdk.GroupArguments{
98+
HasMember:user.ID.String(),
99+
})
100+
require.NoError(t,err)
101+
require.Len(t,userGroups,0)
60102
})
61103

62104
t.Run("PostUser",func(t*testing.T) {

0 commit comments

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