Skip to content

Commit fa079d7

Browse files
Emmanuel BarriosCChemin
authored andcommitted
[BUG] 🐛 Fixed filter users by group (#418)
Signed-off-by: Emmanuel Barrios <emmanuel.barrios@insee.fr>
1 parent 12fb6fe commit fa079d7

16 files changed

Lines changed: 82 additions & 77 deletions

File tree

sugoi-api-file-store-provider/src/main/java/fr/insee/sugoi/store/file/FileWriterStore.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public ProviderResponse addUserToGroup(
308308
() -> new UserNotFoundException(config.get(GlobalKeysConfig.REALM), userId));
309309
if (user != null) {
310310
try {
311-
user.addGroup(new Group(appName, groupName));
311+
user.addGroup(new Group(groupName, appName));
312312
updateUser(user, new ProviderRequest(null, false, null));
313313
if (group.getUsers() == null) {
314314
group.setUsers(new ArrayList<>());

sugoi-api-file-store-provider/src/test/java/fr/insee/sugoi/store/file/FileWriterStoreTest.java

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ public void setup() {
7777
testoOrgAdress.setLines(
7878
new String[] {"Insee", null, null, "88 AVE VERDIER", null, null, null});
7979

80-
Group utilisateursApplitest = new Group("Applitest", "Utilisateurs_Applitest");
81-
Group adminApplitest = new Group("Applitest", "Administrateurs_Applitest");
80+
Group utilisateursApplitest = new Group("Utilisateurs_Applitest", "Applitest");
81+
Group adminApplitest = new Group("Administrateurs_Applitest", "Applitest");
8282
adminApplitest.setDescription("toto");
8383
User simpleTestcUser = new User();
8484
simpleTestcUser.setUsername("testc");
@@ -254,33 +254,28 @@ public void testDeleteUser() {
254254
fileReaderStore.getGroup("Applitest", "Utilisateurs_Applitest").get().getUsers().stream()
255255
.anyMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
256256
assertThat(
257-
"byebye is in Utilisateurs_Applitest",
257+
"byebye is in Utilisateurs",
258258
fileReaderStore.getUsersInGroup("Applitest", "Utilisateurs_Applitest").getResults().stream()
259259
.anyMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
260260
fileWriterStore.deleteUser("byebye", null);
261261
assertThat("byebye should have been deleted", fileReaderStore.getUser("byebye").isEmpty());
262262
assertThat(
263-
"byebye should no more be in Utilisateurs_Applitest",
264-
!fileReaderStore.getGroup("Applitest", "Utilisateurs_Applitest").get().getUsers().stream()
265-
.anyMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
263+
"byebye should no more be in Utilisateurs",
264+
fileReaderStore.getGroup("Applitest", "Utilisateurs_Applitest").get().getUsers().stream()
265+
.noneMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
266266
assertThat(
267-
"byebye is in Utilisateurs_Applitest",
268-
!fileReaderStore
269-
.getUsersInGroup("Applitest", "Utilisateurs_Applitest")
270-
.getResults()
271-
.stream()
272-
.anyMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
267+
"byebye is in Utilisateurs",
268+
fileReaderStore.getUsersInGroup("Applitest", "Utilisateurs_Applitest").getResults().stream()
269+
.noneMatch(user -> user.getUsername().equalsIgnoreCase("byebye")));
273270
}
274271

275272
@Test
276273
public void testCreateApplication() {
277274
Application application = new Application();
278275
application.setName("MyApplication");
279276
List<Group> groups = new ArrayList<>();
280-
Group group1 = new Group();
281-
group1.setName("Group1_MyApplication");
282-
Group group2 = new Group();
283-
group2.setName("Group2_MyApplication");
277+
Group group1 = new Group("Group1_MyApplication", "MyApplication");
278+
Group group2 = new Group("Group2_MyApplication", "MyApplication");
284279
group2.setUsers(List.of(new User("toto")));
285280
groups.add(group1);
286281
groups.add(group2);
@@ -300,8 +295,7 @@ public void testCreateApplication() {
300295
@Test
301296
public void testUpdateApplication() {
302297
Application application = fileReaderStore.getApplication("Applitest").get();
303-
Group group1 = new Group();
304-
group1.setName("Group1_Applitest");
298+
Group group1 = new Group("Group1_Applitest", "Applitest");
305299
application.getGroups().add(group1);
306300
application.getGroups().get(0).setDescription("new description");
307301
application.getGroups().stream()
@@ -322,8 +316,8 @@ public void testUpdateApplication() {
322316
.anyMatch(group -> group.getDescription().equals("new description")));
323317
assertThat(
324318
"Users should not have been modified",
325-
!retrievedApplication.getGroups().stream()
326-
.anyMatch(
319+
retrievedApplication.getGroups().stream()
320+
.noneMatch(
327321
group ->
328322
group.getName().equals("Utilisateurs_Applitest")
329323
&& group.getUsers().size() != 3));
@@ -334,8 +328,8 @@ public void testDeleteApplication() {
334328
Application application = new Application();
335329
application.setName("NotEmptyApplication");
336330
List<Group> groups = new ArrayList<>();
337-
Group group1 = new Group();
338-
group1.setName("Group1_NotEmptyApplication");
331+
Group group1 = new Group("Group1_NotEmptyApplication", "NotEmptyApplication");
332+
groups.add(group1);
339333
application.setGroups(groups);
340334
fileWriterStore.createApplication(application, null);
341335
assertThat(
@@ -350,8 +344,7 @@ public void testDeleteApplication() {
350344

351345
@Test
352346
public void testCreateGroup() {
353-
Group group = new Group();
354-
group.setName("Groupy_Applitest");
347+
Group group = new Group("Groupy_Applitest", "Applitest");
355348
group.setDescription("Super groupy de test");
356349
User user = new User();
357350
user.setUsername("usery");
@@ -367,13 +360,12 @@ public void testCreateGroup() {
367360

368361
@Test
369362
public void testDeleteGroup() {
370-
Group group = new Group();
371-
group.setName("Asupprimer_WebServicesLdap");
363+
Group group = new Group("Asupprimer_WebServicesLdap", "WebServicesLdap");
372364
group.setDescription("supprime ce groupe");
373365
fileWriterStore.createGroup("WebServicesLdap", group, null);
374366
fileWriterStore.deleteGroup("WebServicesLdap", "Asupprimer_WebServicesLdap", null);
375367
assertThat(
376-
"Should have been deleted",
368+
"Asupprimer Should have been deleted",
377369
fileReaderStore.getGroup("WebServicesLdap", "Asupprimer_WebServicesLdap").isEmpty());
378370
}
379371

sugoi-api-ldap-store-provider/src/main/java/fr/insee/sugoi/store/ldap/LdapReaderStore.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,16 +263,13 @@ private <MapperType> Filter getFilterFromObject(
263263
return LdapFilter.and(
264264
mapper.createAttributesForFilter(object).stream()
265265
.filter(attribute -> !attribute.getValue().equals(""))
266-
.map(
267-
attribute -> {
268-
return LdapFilter.contains(attribute.getName(), attribute.getValues());
269-
})
266+
.map(attribute -> LdapFilter.createFilter(attribute.getName(), attribute.getValues()))
270267
.collect(Collectors.toList()));
271268
} else if (searchType.equalsIgnoreCase("OR")) {
272269
List<Filter> objectClassListFilter =
273270
mapper.createAttributesForFilter(object).stream()
274271
.filter(attribute -> attribute.getName().equals("objectClass"))
275-
.map(attribute -> LdapFilter.contains(attribute.getName(), attribute.getValues()))
272+
.map(attribute -> LdapFilter.createFilter(attribute.getName(), attribute.getValues()))
276273
.collect(Collectors.toList());
277274

278275
List<Filter> attributeListFilter =
@@ -281,7 +278,7 @@ private <MapperType> Filter getFilterFromObject(
281278
attribute ->
282279
!attribute.getName().equals("objectClass")
283280
&& !attribute.getValue().equals(""))
284-
.map(attribute -> LdapFilter.contains(attribute.getName(), attribute.getValues()))
281+
.map(attribute -> LdapFilter.createFilter(attribute.getName(), attribute.getValues()))
285282
.collect(Collectors.toList());
286283

287284
if (!objectClassListFilter.isEmpty() && attributeListFilter.isEmpty()) {

sugoi-api-ldap-store-provider/src/test/java/fr/insee/sugoi/ldap/LdapReaderStoreTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,9 @@ public void testSearchUserWithMatchingMail() {
208208
}
209209

210210
@Test
211-
public void testSearchUsersWithMatchingGroup() {
211+
void testSearchUsersWithMatchingGroup() {
212212
User testUser = new User();
213-
testUser.setGroups(List.of(new Group("Utilisateurs_Applitest")));
213+
testUser.setGroups(List.of(new Group("Utilisateurs_Applitest", "Applitest")));
214214
List<User> users =
215215
ldapReaderStore.searchUsers(testUser, new PageableResult(), "AND").getResults();
216216
assertThat(

sugoi-api-ldap-store-provider/src/test/java/fr/insee/sugoi/ldap/LdapWriterStoreTest.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,10 +267,8 @@ public void testCreateApplication() {
267267
Application application = new Application();
268268
application.setName("MyApplication");
269269
List<Group> groups = new ArrayList<>();
270-
Group group1 = new Group();
271-
group1.setName("Group1_MyApplication");
272-
Group group2 = new Group();
273-
group2.setName("Group2_MyApplication");
270+
Group group1 = new Group("Group1_MyApplication", "MyApplication");
271+
Group group2 = new Group("Group2_MyApplication", "MyApplication");
274272
groups.add(group1);
275273
groups.add(group2);
276274
application.setGroups(groups);
@@ -286,8 +284,7 @@ public void testCreateApplication() {
286284
@Test
287285
public void testUpdateApplicationWithGroupAdding() {
288286
Application application = ldapReaderStore.getApplication("Applitest").get();
289-
Group group1 = new Group();
290-
group1.setName("Group1_Applitest");
287+
Group group1 = new Group("Group1_Applitest", "Applitest");
291288
application.getGroups().add(group1);
292289
ldapWriterStore.updateApplication(application, null);
293290
Application retrievedApplication = ldapReaderStore.getApplication("Applitest").get();
@@ -346,8 +343,7 @@ public void testDeleteApplication() {
346343
Application application = new Application();
347344
application.setName("NotEmptyApplication");
348345
List<Group> groups = new ArrayList<>();
349-
Group group1 = new Group();
350-
group1.setName("Group1_NotEmptyApplication");
346+
Group group1 = new Group("Group1_NotEmptyApplication", "NotEmptyApplication");
351347
application.setGroups(groups);
352348
ldapWriterStore.createApplication(application, null);
353349
ldapWriterStore.deleteApplication("NotEmptyApplication", null);
@@ -361,8 +357,7 @@ public void testDeleteApplication() {
361357

362358
@Test
363359
public void testCreateGroup() {
364-
Group group = new Group();
365-
group.setName("Groupy_Applitest");
360+
Group group = new Group("Groupy_Applitest", "Applitest");
366361
group.setDescription("Super groupy de test");
367362
User user = new User();
368363
user.setUsername("usery");
@@ -389,8 +384,7 @@ void testCreateGroupApplicationNotFound() {
389384

390385
@Test
391386
public void testDeleteGroup() {
392-
Group group = new Group();
393-
group.setName("Asupprimer_WebServicesLdap");
387+
Group group = new Group("Asupprimer_WebServicesLdap", "WebServicesLdap");
394388
group.setDescription("supprime ce groupe");
395389
ldapWriterStore.createGroup("WebServicesLdap", group, null);
396390
ldapWriterStore.deleteGroup("WebServicesLdap", "Asupprimer_WebServicesLdap", null);

sugoi-api-ldap-store-provider/src/test/resources/ldap.ldif

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ objectClass: inseeAttributsAuthentification
264264
objectClass: inseeAttributsHabilitation
265265
objectClass: inseeAttributsCommunication
266266
inseeOrganisationDN: uid=testo,ou=organisations,ou=clients_domaine1,o=insee,c=fr
267-
memberOf: cn=Administrateur_Applitest,ou=Applitest_Objets,ou=Applitest,ou=Applications,o=insee,c=fr
267+
memberOf: cn=Administrateurs_Applitest,ou=Applitest_Objets,ou=Applitest,ou=Applications,o=insee,c=fr
268268
uid: nogroup
269269

270270
dn: uid=byebye,ou=contacts,ou=clients_domaine1,o=insee,c=fr

sugoi-api-ldap-utils/src/main/java/fr/insee/sugoi/ldap/utils/LdapFilter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,14 @@ public static Filter exists(String property) {
2525
return Filter.createPresenceFilter(property);
2626
}
2727

28-
public static Filter contains(String property, String[] values) {
28+
public static Filter createFilter(String property, String[] values) {
2929
return LdapFilter.and(
30-
Arrays.asList(values).stream()
30+
Arrays.stream(values)
3131
.map(
3232
value ->
3333
property.equalsIgnoreCase("objectclass")
34-
? Filter.createEqualityFilter("objectclass", value)
34+
|| property.equalsIgnoreCase("memberOf")
35+
? Filter.createEqualityFilter(property, value)
3536
: Filter.createSubAnyFilter(property, value))
3637
.collect(Collectors.toList()));
3738
}

sugoi-api-ldap-utils/src/main/java/fr/insee/sugoi/ldap/utils/mapper/GenericLdapMapper.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ private static Object transformLdapAttributeToSugoiAttribute(
207207
pattern.matcher(attributeValue.substring(attributeValue.indexOf(",") + 1));
208208
if (matcher.matches()) {
209209
return new Group(
210-
matcher.group(1), LdapUtils.getNodeValueFromDN(attributeValue));
210+
LdapUtils.getNodeValueFromDN(attributeValue), matcher.group(1));
211211
} else {
212212
return null;
213213
}
@@ -272,17 +272,17 @@ private static Attribute transformSugoiToAttribute(
272272
// Assume that if list is empty it's really to set an empty list (delete all
273273
// values)
274274
// Groups list can be set null if not needed
275+
String genericGroup = "cn={group}" + "," + config.get(LdapConfigKeys.GROUP_SOURCE_PATTERN);
276+
275277
return new Attribute(
276278
ldapAttributeName,
277279
((List<Group>) sugoiValue)
278280
.stream()
279281
.map(
280282
group ->
281-
String.format(
282-
// TODO should be a param
283-
"cn=%s,",
284-
//
285-
group.getName()))
283+
genericGroup
284+
.replace("{appliname}", group.getAppName())
285+
.replace("{group}", group.getName()))
286286
.collect(Collectors.toList()));
287287
case LIST_STRING:
288288
// Assume that if list is empty it's really to set an empty list (delete all

sugoi-api-model/src/main/java/fr/insee/sugoi/model/Group.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public Group(String name) {
2828
this.name = name;
2929
}
3030

31-
public Group(String appName, String name) {
32-
this.appName = appName;
31+
public Group(String name, String appName) {
3332
this.name = name;
33+
this.appName = appName;
3434
}
3535

3636
public String getName() {

sugoi-api-model/src/main/java/fr/insee/sugoi/model/User.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ public class User implements Serializable {
3333
private List<Group> groups;
3434
// Don't set a default empty list to distinguish case null (not provided) and empty
3535
private List<Habilitation> habilitations;
36-
3736
private PostalAddress address;
3837
private Map<String, Object> metadatas = new HashMap<>();
3938
private Map<String, Object> attributes = new HashMap<>();

0 commit comments

Comments
 (0)