opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: remove user from groups on org membership delete (#14701)

Browse Source 
* feat: remove user from groups on org membership delete

Groups inherently provide authz access to certain resources. If a
user is removed from an organization, they should be removed
from all their groups in said organization.
This commit is contained in:
Steven Masley
2024-09-17 19:41:34 -05:00
committed by GitHub
parent c145f113fe
commit 1e5438eadb
6 changed files with 196 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/dbmem/dbmem.go
+11 -1
View File
@@ -1944,7 +1944,7 @@ func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error
return sql.ErrNoRows
}
func (q *FakeQuerier) DeleteOrganizationMember(_ context.Context, arg database.DeleteOrganizationMemberParams) error {
func (q *FakeQuerier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
err := validateDatabaseType(arg)
if err != nil {
return err
@@ -1959,6 +1959,16 @@ func (q *FakeQuerier) DeleteOrganizationMember(_ context.Context, arg database.D
if len(deleted) == 0 {
return sql.ErrNoRows
}
// Delete group member trigger
q.groupMembers = slices.DeleteFunc(q.groupMembers, func(member database.GroupMemberTable) bool {
if member.UserID != arg.UserID {
return false
}
g, _ := q.getGroupByIDNoLock(ctx, member.GroupID)
return g.OrganizationID == arg.OrganizationID
})
return nil
}
coderd/database/dump.sql
Generated
+21
View File
@@ -286,6 +286,25 @@ BEGIN
END;
$$;
CREATE FUNCTION delete_group_members_on_org_member_delete() RETURNS trigger
LANGUAGE plpgsql
AS $$
DECLARE
BEGIN
-- Remove the user from all groups associated with the same
-- organization as the organization_member being deleted.
DELETE FROM group_members
WHERE
user_id = OLD.user_id
AND group_id IN (
SELECT id
FROM groups
WHERE organization_id = OLD.organization_id
);
RETURN OLD;
END;
$$;
CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger
LANGUAGE plpgsql
AS $$
@@ -2041,6 +2060,8 @@ CREATE TRIGGER tailnet_notify_peer_change AFTER INSERT OR DELETE OR UPDATE ON ta
CREATE TRIGGER tailnet_notify_tunnel_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_tunnels FOR EACH ROW EXECUTE FUNCTION tailnet_notify_tunnel_change();
CREATE TRIGGER trigger_delete_group_members_on_org_member_delete BEFORE DELETE ON organization_members FOR EACH ROW EXECUTE FUNCTION delete_group_members_on_org_member_delete();
CREATE TRIGGER trigger_delete_oauth2_provider_app_token AFTER DELETE ON oauth2_provider_app_tokens FOR EACH ROW EXECUTE FUNCTION delete_deleted_oauth2_provider_app_token_api_key();
CREATE TRIGGER trigger_insert_apikeys BEFORE INSERT ON api_keys FOR EACH ROW EXECUTE FUNCTION insert_apikey_fail_if_user_deleted();
coderd/database/migrations/000252_group_member_trigger.down.sql
+2
View File
@@ -0,0 +1,2 @@
DROP TRIGGER IF EXISTS trigger_delete_group_members_on_org_member_delete ON organization_members;
DROP FUNCTION IF EXISTS delete_group_members_on_org_member_delete;
coderd/database/migrations/000252_group_member_trigger.up.sql
+23
View File
@@ -0,0 +1,23 @@
CREATE FUNCTION delete_group_members_on_org_member_delete() RETURNS TRIGGER
LANGUAGE plpgsql
AS $$
DECLARE
BEGIN
-- Remove the user from all groups associated with the same
-- organization as the organization_member being deleted.
DELETE FROM group_members
WHERE
user_id = OLD.user_id
AND group_id IN (
SELECT id
FROM groups
WHERE organization_id = OLD.organization_id
);
RETURN OLD;
END;
$$;
CREATE TRIGGER trigger_delete_group_members_on_org_member_delete
BEFORE DELETE ON organization_members
FOR EACH ROW
EXECUTE PROCEDURE delete_group_members_on_org_member_delete();
coderd/database/querier_test.go
+97
View File
@@ -1216,6 +1216,103 @@ func TestExpectOne(t *testing.T) {
})
}
func TestGroupRemovalTrigger(t *testing.T) {
t.Parallel()
db, _ := dbtestutil.NewDB(t)
orgA := dbgen.Organization(t, db, database.Organization{})
_, err := db.InsertAllUsersGroup(context.Background(), orgA.ID)
require.NoError(t, err)
orgB := dbgen.Organization(t, db, database.Organization{})
_, err = db.InsertAllUsersGroup(context.Background(), orgB.ID)
require.NoError(t, err)
orgs := []database.Organization{orgA, orgB}
user := dbgen.User(t, db, database.User{})
extra := dbgen.User(t, db, database.User{})
users := []database.User{user, extra}
groupA1 := dbgen.Group(t, db, database.Group{
OrganizationID: orgA.ID,
})
groupA2 := dbgen.Group(t, db, database.Group{
OrganizationID: orgA.ID,
})
groupB1 := dbgen.Group(t, db, database.Group{
OrganizationID: orgB.ID,
})
groupB2 := dbgen.Group(t, db, database.Group{
OrganizationID: orgB.ID,
})
groups := []database.Group{groupA1, groupA2, groupB1, groupB2}
// Add users to all organizations
for _, u := range users {
for _, o := range orgs {
dbgen.OrganizationMember(t, db, database.OrganizationMember{
OrganizationID: o.ID,
UserID: u.ID,
})
}
}
// Add users to all groups
for _, u := range users {
for _, g := range groups {
dbgen.GroupMember(t, db, database.GroupMemberTable{
GroupID: g.ID,
UserID: u.ID,
})
}
}
// Verify user is in all groups
ctx := testutil.Context(t, testutil.WaitLong)
onlyGroupIDs := func(row database.GetGroupsRow) uuid.UUID {
return row.Group.ID
}
userGroups, err := db.GetGroups(ctx, database.GetGroupsParams{
HasMemberID: user.ID,
})
require.NoError(t, err)
require.ElementsMatch(t, []uuid.UUID{
orgA.ID, orgB.ID, // Everyone groups
groupA1.ID, groupA2.ID, groupB1.ID, groupB2.ID, // Org groups
}, db2sdk.List(userGroups, onlyGroupIDs))
// Remove the user from org A
err = db.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{
OrganizationID: orgA.ID,
UserID: user.ID,
})
require.NoError(t, err)
// Verify user is no longer in org A groups
userGroups, err = db.GetGroups(ctx, database.GetGroupsParams{
HasMemberID: user.ID,
})
require.NoError(t, err)
require.ElementsMatch(t, []uuid.UUID{
orgB.ID, // Everyone group
groupB1.ID, groupB2.ID, // Org groups
}, db2sdk.List(userGroups, onlyGroupIDs))
// Verify extra user is unchanged
extraUserGroups, err := db.GetGroups(ctx, database.GetGroupsParams{
HasMemberID: extra.ID,
})
require.NoError(t, err)
require.ElementsMatch(t, []uuid.UUID{
orgA.ID, orgB.ID, // Everyone groups
groupA1.ID, groupA2.ID, groupB1.ID, groupB2.ID, // Org groups
}, db2sdk.List(extraUserGroups, onlyGroupIDs))
}
func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) {
t.Helper()
require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg)
enterprise/members_test.go
+42
View File
@@ -29,6 +29,7 @@ func TestEnterpriseMembers(t *testing.T) {
LicenseOptions: &coderdenttest.LicenseOptions{
Features: license.Features{
codersdk.FeatureMultipleOrganizations: 1,
codersdk.FeatureTemplateRBAC: 1,
},
},
})
@@ -39,6 +40,21 @@ func TestEnterpriseMembers(t *testing.T) {
_, user := coderdtest.CreateAnotherUser(t, owner, secondOrg.ID)
ctx := testutil.Context(t, testutil.WaitMedium)
// Groups exist to ensure a user removed from the org loses their
// group access.
g1, err := orgAdminClient.CreateGroup(ctx, secondOrg.ID, codersdk.CreateGroupRequest{
Name: "foo",
DisplayName: "Foo",
})
require.NoError(t, err)
g2, err := orgAdminClient.CreateGroup(ctx, secondOrg.ID, codersdk.CreateGroupRequest{
Name: "bar",
DisplayName: "Bar",
})
require.NoError(t, err)
// Verify the org of 3 members
members, err := orgAdminClient.OrganizationMembers(ctx, secondOrg.ID)
require.NoError(t, err)
@@ -47,6 +63,25 @@ func TestEnterpriseMembers(t *testing.T) {
[]uuid.UUID{first.UserID, user.ID, orgAdmin.ID},
db2sdk.List(members, onlyIDs))
// Add the member to some groups
_, err = orgAdminClient.PatchGroup(ctx, g1.ID, codersdk.PatchGroupRequest{
AddUsers: []string{user.ID.String()},
})
require.NoError(t, err)
_, err = orgAdminClient.PatchGroup(ctx, g2.ID, codersdk.PatchGroupRequest{
AddUsers: []string{user.ID.String()},
})
require.NoError(t, err)
// Verify group membership
userGroups, err := orgAdminClient.Groups(ctx, codersdk.GroupArguments{
HasMember: user.ID.String(),
})
require.NoError(t, err)
// Everyone group + 2 groups
require.Len(t, userGroups, 3)
// Delete a member
err = orgAdminClient.DeleteOrganizationMember(ctx, secondOrg.ID, user.Username)
require.NoError(t, err)
@@ -57,6 +92,13 @@ func TestEnterpriseMembers(t *testing.T) {
require.ElementsMatch(t,
[]uuid.UUID{first.UserID, orgAdmin.ID},
db2sdk.List(members, onlyIDs))
// User should now belong to 0 groups
userGroups, err = orgAdminClient.Groups(ctx, codersdk.GroupArguments{
HasMember: user.ID.String(),
})
require.NoError(t, err)
require.Len(t, userGroups, 0)
})
t.Run("PostUser", func(t *testing.T) {