1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-09-10 17:53:36 +02:00

fix: only delete SSO-synced group membership where membership was added by SSO sync (#4929)

Fixes an issue where SSO group sync would delete a syncable group that a
user was manually added to

## Discussion points

Is this the longterm fix for this? Or would we want another column in
the mapping table for future-proofing this?
This commit is contained in:
David Leek 2023-10-05 13:22:46 +02:00
parent fc03621791
commit 1c5d8fd74c
No known key found for this signature in database
GPG Key ID: 515EE0F1BB6D0BE1
2 changed files with 11 additions and 7 deletions

View File

@ -330,7 +330,7 @@ export default class GroupStore implements IGroupStore {
}) })
.orWhereRaw('jsonb_array_length(mappings_sso) = 0'), .orWhereRaw('jsonb_array_length(mappings_sso) = 0'),
) )
.where('gu.user_id', userId); .where({ 'gu.user_id': userId, 'gu.created_by': 'SSO' });
return rows.map(rowToGroupUser); return rows.map(rowToGroupUser);
} }

View File

@ -54,34 +54,38 @@ test('should have three group', async () => {
}); });
test('should add person to 2 groups', async () => { test('should add person to 2 groups', async () => {
await groupService.syncExternalGroups(user.id, ['dev', 'maintainer']); await groupService.syncExternalGroups(
user.id,
['dev', 'maintainer'],
'SSO',
);
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(2); expect(groups.length).toBe(2);
}); });
test('should remove person from one group', async () => { test('should remove person from one group', async () => {
await groupService.syncExternalGroups(user.id, ['maintainer']); await groupService.syncExternalGroups(user.id, ['maintainer'], 'SSO');
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(1); expect(groups.length).toBe(1);
expect(groups[0].name).toEqual('maintainer_group'); expect(groups[0].name).toEqual('maintainer_group');
}); });
test('should add person to completely new group with new name', async () => { test('should add person to completely new group with new name', async () => {
await groupService.syncExternalGroups(user.id, ['dev']); await groupService.syncExternalGroups(user.id, ['dev'], 'SSO');
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(1); expect(groups.length).toBe(1);
expect(groups[0].name).toEqual('dev_group'); expect(groups[0].name).toEqual('dev_group');
}); });
test('should not update groups when not string array ', async () => { test('should not update groups when not string array ', async () => {
await groupService.syncExternalGroups(user.id, 'Everyone' as any); await groupService.syncExternalGroups(user.id, 'Everyone' as any, 'SSO');
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(1); expect(groups.length).toBe(1);
expect(groups[0].name).toEqual('dev_group'); expect(groups[0].name).toEqual('dev_group');
}); });
test('should clear groups when empty array ', async () => { test('should clear groups when empty array ', async () => {
await groupService.syncExternalGroups(user.id, []); await groupService.syncExternalGroups(user.id, [], 'SSO');
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(0); expect(groups.length).toBe(0);
}); });
@ -92,7 +96,7 @@ test('should not remove user from no SSO definition group', async () => {
description: 'no_mapping_group', description: 'no_mapping_group',
}); });
await groupStore.addUserToGroups(user.id, [group.id]); await groupStore.addUserToGroups(user.id, [group.id]);
await groupService.syncExternalGroups(user.id, []); await groupService.syncExternalGroups(user.id, [], 'SSO');
const groups = await groupService.getGroupsForUser(user.id); const groups = await groupService.getGroupsForUser(user.id);
expect(groups.length).toBe(1); expect(groups.length).toBe(1);
expect(groups[0].name).toEqual('no_mapping_group'); expect(groups[0].name).toEqual('no_mapping_group');