From 3eedd4220bb9f9ac91be4f0d091312f5a2e1e325 Mon Sep 17 00:00:00 2001
From: Mathieu Baudier <mbaudier@argeo.org>
Date: Sat, 28 Nov 2020 12:52:22 +0100
Subject: [PATCH] Make aggregation authorization more robust.

---
 .../osgi/useradmin/AggregatingAuthorization.java     | 12 +++++++-----
 .../argeo/osgi/useradmin/AggregatingUserAdmin.java   |  7 ++++++-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingAuthorization.java b/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingAuthorization.java
index 758d3e355..ba9953416 100644
--- a/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingAuthorization.java
+++ b/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingAuthorization.java
@@ -2,27 +2,29 @@ package org.argeo.osgi.useradmin;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
 import org.osgi.service.useradmin.Authorization;
 
+/** An {@link Authorization} which combines roles form various auth sources. */
 class AggregatingAuthorization implements Authorization {
 	private final String name;
 	private final String displayName;
-	private final List<String> systemRoles;
-	private final List<String> roles;
+	private final Set<String> systemRoles;
+	private final Set<String> roles;
 
 	public AggregatingAuthorization(String name, String displayName, Set<String> systemRoles, String[] roles) {
 		this.name = name;
 		this.displayName = displayName;
-		this.systemRoles = Collections.unmodifiableList(new ArrayList<String>(systemRoles));
-		List<String> temp = new ArrayList<>();
+		this.systemRoles = Collections.unmodifiableSet(new HashSet<>(systemRoles));
+		Set<String> temp = new HashSet<>();
 		for (String role : roles) {
 			if (!temp.contains(role))
 				temp.add(role);
 		}
-		this.roles = Collections.unmodifiableList(temp);
+		this.roles = Collections.unmodifiableSet(temp);
 	}
 
 	@Override
diff --git a/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingUserAdmin.java b/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingUserAdmin.java
index 85a447082..f3e51804a 100644
--- a/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingUserAdmin.java
+++ b/org.argeo.enterprise/src/org/argeo/osgi/useradmin/AggregatingUserAdmin.java
@@ -110,7 +110,12 @@ public class AggregatingUserAdmin implements UserAdmin {
 		Set<String> sysRoles = new HashSet<String>();
 		for (String role : rawAuthorization.getRoles()) {
 			Authorization auth = systemRoles.getAuthorization((User) userAdmin.getRole(role));
-			sysRoles.addAll(Arrays.asList(auth.getRoles()));
+			systemRoles:for(String systemRole:auth.getRoles()) {
+				if(role.equals(systemRole))
+					continue systemRoles;
+				sysRoles.add(systemRole);
+			}
+//			sysRoles.addAll(Arrays.asList(auth.getRoles()));
 		}
 		addAbstractSystemRoles(rawAuthorization, sysRoles);
 		Authorization authorization = new AggregatingAuthorization(usernameToUse, displayNameToUse, sysRoles,
-- 
2.30.2