SAK-46236 hiding a tool removes all permissions associated with viewi…

…ng the tool (functions.require). (sakaiproject#9910)

But re-enabling the tool must only grant minimum permissions and not all permissions.

content/content-tool/tool/src/webapp/WEB-INF/tools/sakai.dropbox.xml

@@ -15,8 +15,8 @@
 
 		<!-- This is final because it's being added long after the tool has rolled out so making it final means
 		     all existing placements get this in their configuration without backfilling -->
-		<configuration name="functions.require" type="final"
-					   value="dropbox.own | dropbox.maintain | dropbox.maintain.own.groups | dropbox.delete.any | dropbox.delete.own | dropbox.write.any | dropbox.write.own" />
+		<!-- The order matters as functions.require is used for hiding/showing in Tool Order. First one is used to re-assign permissions. -->
+		<configuration name="functions.require" type="final" value="dropbox.own | dropbox.maintain | dropbox.maintain.own.groups" />
 	</tool>
 
 </registration>

kernel/kernel-impl/src/test/java/org/sakaiproject/tool/impl/ActiveToolComponentTest.java

@@ -21,10 +21,12 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Properties;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -216,6 +218,22 @@ public void testPerformance() {
 		}
 	}
 
+	@Test
+	public void testFlatmapOrdering() {
+		Properties props = new Properties();
+		props.put(ToolManager.TOOLCONFIG_REQUIRED_PERMISSIONS,
+				"dropbox.own | dropbox.maintain | dropbox.maintain.own.groups | dropbox.delete.any | dropbox.delete.own | dropbox.write.any | dropbox.write.own");
+
+		ToolConfiguration config = mock(ToolConfiguration.class);
+		when(config.getConfig()).thenReturn(props);
+
+		List<String> permissions = activeToolManager.getRequiredPermissions(config)
+				.stream().flatMap(Collection::stream).collect(Collectors.toList());
+
+		assertTrue("Order is maintained for item one", permissions.get(0).equals("dropbox.own"));
+		assertTrue("Order is maintained for item two", permissions.get(1).equals("dropbox.maintain"));
+	}
+
 	@Test
 	public void testFunctionsRequire() {
 

site-manage/pageorder/tool/src/java/org/sakaiproject/site/tool/helper/order/impl/SitePageEditHandler.java

@@ -614,8 +614,14 @@ private boolean setEnabled(String pageId, boolean enabled) throws SakaiException
                 // We never change SITE_UPD at all.
                 permissions.remove(SiteService.SECURE_UPDATE_SITE);
                 permissions.remove(SiteService.SITE_VISIT);
-                Set<Role> roles = getRolesWithout(authzGroup, SiteService.SECURE_UPDATE_SITE);
 
+                // When re-enabling permissions and there are multiple permissions,
+                // e.g., dropbox.own and dropbox.maintain, grab the first in list only.
+                if (enabled && permissions.size() > 1) {
+                    permissions = Collections.singletonList(permissions.get(0));
+                }
+
+                Set<Role> roles = getRolesWithout(authzGroup, SiteService.SECURE_UPDATE_SITE);
                 for (Role role : roles) {
                     if (enabled) {
                         role.allowFunctions(permissions);