mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01: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:
		
							parent
							
								
									8d0e947f31
								
							
						
					
					
						commit
						40ebb7ef95
					
				@ -330,7 +330,7 @@ export default class GroupStore implements IGroupStore {
 | 
			
		||||
                    })
 | 
			
		||||
                    .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);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
@ -57,34 +57,38 @@ test('should have three group', 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);
 | 
			
		||||
    expect(groups.length).toBe(2);
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
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);
 | 
			
		||||
    expect(groups.length).toBe(1);
 | 
			
		||||
    expect(groups[0].name).toEqual('maintainer_group');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
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);
 | 
			
		||||
    expect(groups.length).toBe(1);
 | 
			
		||||
    expect(groups[0].name).toEqual('dev_group');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
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);
 | 
			
		||||
    expect(groups.length).toBe(1);
 | 
			
		||||
    expect(groups[0].name).toEqual('dev_group');
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
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);
 | 
			
		||||
    expect(groups.length).toBe(0);
 | 
			
		||||
});
 | 
			
		||||
@ -95,7 +99,7 @@ test('should not remove user from no SSO definition group', async () => {
 | 
			
		||||
        description: 'no_mapping_group',
 | 
			
		||||
    });
 | 
			
		||||
    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);
 | 
			
		||||
    expect(groups.length).toBe(1);
 | 
			
		||||
    expect(groups[0].name).toEqual('no_mapping_group');
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user