You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2010/02/21 19:37:45 UTC

svn commit: r912387 - in /sling/trunk: bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/

Author: enorman
Date: Sun Feb 21 18:37:45 2010
New Revision: 912387

URL: http://svn.apache.org/viewvc?rev=912387&view=rev
Log:
SLING-997 ModifyAceServlet replaces rather than merges privileges

Modified:
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java
    sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java

Modified: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java?rev=912387&r1=912386&r2=912387&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java (original)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java Sun Feb 21 18:37:45 2010
@@ -19,7 +19,9 @@
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import javax.jcr.Item;
 import javax.jcr.RepositoryException;
@@ -134,19 +136,23 @@
 
 		List<String> grantedPrivilegeNames = new ArrayList<String>();
 		List<String> deniedPrivilegeNames = new ArrayList<String>();
+		//SLING-997: keep track of the privilege names that were posted, so the others can be preserved
+		Set<String> postedPrivilegeNames = new HashSet<String>();
 		Enumeration parameterNames = request.getParameterNames();
 		while (parameterNames.hasMoreElements()) {
 			Object nextElement = parameterNames.nextElement();
 			if (nextElement instanceof String) {
 				String paramName = (String)nextElement;
 				if (paramName.startsWith("privilege@")) {
+					String privilegeName = paramName.substring(10);
+					//keep track of which privileges should be changed
+					postedPrivilegeNames.add(privilegeName); 
+					
 					String parameterValue = request.getParameter(paramName);
 					if (parameterValue != null && parameterValue.length() > 0) {
 						if ("granted".equals(parameterValue)) {
-							String privilegeName = paramName.substring(10);
 							grantedPrivilegeNames.add(privilegeName);
 						} else if ("denied".equals(parameterValue)) {
-							String privilegeName = paramName.substring(10);
 							deniedPrivilegeNames.add(privilegeName);
 						}
 					}
@@ -165,6 +171,9 @@
 				newPrivileges = new StringBuilder();
 			}
 
+			List<Privilege> preserveGrantedPrivileges = new ArrayList<Privilege>();
+			List<Privilege> preserveDeniedPrivileges = new ArrayList<Privilege>();
+			
 			//keep track of the existing Aces for the target principal
 			AccessControlEntry[] accessControlEntries = updatedAcl.getAccessControlEntries();
 			List<AccessControlEntry> oldAces = new ArrayList<AccessControlEntry>();
@@ -175,20 +184,25 @@
 					}
 					oldAces.add(ace);
 
-					if (log.isDebugEnabled()) {
-						//collect the information for debug logging
-						boolean isAllow = AccessControlUtil.isAllow(ace);
-						Privilege[] privileges = ace.getPrivileges();
-						for (Privilege privilege : privileges) {
-							if (oldPrivileges.length() > 0) {
-								oldPrivileges.append(", "); //separate entries by commas
-							}
+					boolean isAllow = AccessControlUtil.isAllow(ace);
+					Privilege[] privileges = ace.getPrivileges();
+					for (Privilege privilege : privileges) {
+						String privilegeName = privilege.getName();
+						if (!postedPrivilegeNames.contains(privilegeName)) {
+							//this privilege was not posted, so record the existing state to be 
+							// preserved when the ACE is re-created below
 							if (isAllow) {
-								oldPrivileges.append("granted=");
+								preserveGrantedPrivileges.add(privilege);
 							} else {
-								oldPrivileges.append("denied=");
+								preserveDeniedPrivileges.add(privilege);
+							}
+						}
+
+						if (log.isDebugEnabled()) {
+							//collect the information for debug logging
+							if (oldPrivileges.length() > 0) {
+								oldPrivileges.append(", "); //separate entries by commas
 							}
-							oldPrivileges.append(privilege.getName());
 						}
 					}
 				}
@@ -209,15 +223,20 @@
 				}
 				Privilege privilege = accessControlManager.privilegeFromName(name);
 				grantedPrivilegeList.add(privilege);
+			}
+			//add the privileges that should be preserved
+			grantedPrivilegeList.addAll(preserveGrantedPrivileges);
 
-				if (log.isDebugEnabled()) {
+			if (log.isDebugEnabled()) {
+				for (Privilege privilege : grantedPrivilegeList) {
 					if (newPrivileges.length() > 0) {
 						newPrivileges.append(", "); //separate entries by commas
 					}
 					newPrivileges.append("granted=");
 					newPrivileges.append(privilege.getName());
-				}
+				}				
 			}
+
 			if (grantedPrivilegeList.size() > 0) {
 				Principal principal = authorizable.getPrincipal();
 				updatedAcl.addAccessControlEntry(principal, grantedPrivilegeList.toArray(new Privilege[grantedPrivilegeList.size()]));
@@ -233,8 +252,12 @@
 					}
 					Privilege privilege = accessControlManager.privilegeFromName(name);
 					deniedPrivilegeList.add(privilege);
-
-					if (log.isDebugEnabled()) {
+				}
+				//add the privileges that should be preserved
+				deniedPrivilegeList.addAll(preserveDeniedPrivileges);
+				
+				if (log.isDebugEnabled()) {
+					for (Privilege privilege : deniedPrivilegeList) {
 						if (newPrivileges.length() > 0) {
 							newPrivileges.append(", "); //separate entries by commas
 						}
@@ -242,6 +265,7 @@
 						newPrivileges.append(privilege.getName());
 					}
 				}
+				
 				if (deniedPrivilegeList.size() > 0) {
 					Principal principal = authorizable.getPrincipal();
 					AccessControlUtil.addEntry(updatedAcl, principal, deniedPrivilegeList.toArray(new Privilege[deniedPrivilegeList.size()]), false);

Modified: sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java?rev=912387&r1=912386&r2=912387&view=diff
==============================================================================
--- sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java (original)
+++ sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java Sun Feb 21 18:37:45 2010
@@ -18,7 +18,9 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import javax.servlet.http.HttpServletResponse;
 
@@ -137,4 +139,109 @@
 		//denied rights are not applied for groups, so make sure it is not there
 		assertTrue(aceObject.isNull("denied"));
 	}
+	
+	/**
+	 * Test for SLING-997, preserve privileges that were not posted with the modifyAce 
+	 * request.
+	 */
+	public void testMergeAceForUser() throws IOException, JSONException {
+		testUserId = createTestUser();
+		testFolderUrl = createTestFolder();
+		
+        String postUrl = testFolderUrl + ".modifyAce.html";
+
+        //1. create an initial set of privileges
+		List<NameValuePair> postParams = new ArrayList<NameValuePair>();
+		postParams.add(new NameValuePair("principalId", testUserId));
+		postParams.add(new NameValuePair("privilege@jcr:read", "granted"));
+		postParams.add(new NameValuePair("privilege@jcr:readAccessControl", "granted"));
+		postParams.add(new NameValuePair("privilege@jcr:addChildNodes", "granted"));
+		postParams.add(new NameValuePair("privilege@jcr:modifyAccessControl", "denied"));
+		postParams.add(new NameValuePair("privilege@jcr:removeChildNodes", "denied"));
+		
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		assertAuthenticatedPostStatus(creds, postUrl, HttpServletResponse.SC_OK, postParams, null);
+		
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+		JSONObject jsonObj = new JSONObject(json);
+		String aceString = jsonObj.getString(testUserId);
+		assertNotNull(aceString);
+		
+		JSONObject aceObject = new JSONObject(aceString); 
+		assertNotNull(aceObject);
+		
+		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		assertNotNull(grantedArray);
+		assertEquals(3, grantedArray.length());
+		Set<String> grantedPrivilegeNames = new HashSet<String>();
+		for (int i=0; i < grantedArray.length(); i++) {
+			grantedPrivilegeNames.add(grantedArray.getString(i));
+		}
+		assertTrue(grantedPrivilegeNames.contains("jcr:read"));
+		assertTrue(grantedPrivilegeNames.contains("jcr:readAccessControl"));
+		assertTrue(grantedPrivilegeNames.contains("jcr:addChildNodes"));
+
+		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		assertNotNull(deniedArray);
+		assertEquals(2, deniedArray.length());
+		Set<String> deniedPrivilegeNames = new HashSet<String>();
+		for (int i=0; i < deniedArray.length(); i++) {
+			deniedPrivilegeNames.add(deniedArray.getString(i));
+		}
+		assertTrue(deniedPrivilegeNames.contains("jcr:modifyAccessControl"));
+		assertTrue(deniedPrivilegeNames.contains("jcr:removeChildNodes"));
+		
+		
+		
+        //2. post a new set of privileges to merge with the existing privileges
+		List<NameValuePair> postParams2 = new ArrayList<NameValuePair>();
+		postParams2.add(new NameValuePair("principalId", testUserId));
+		//jcr:read and jcr:addChildNodes are not posted, so they should remain in the granted ACE
+		postParams2.add(new NameValuePair("privilege@jcr:readAccessControl", "none")); //clear the existing privilege
+		postParams2.add(new NameValuePair("privilege@jcr:modifyProperties", "granted")); //add a new privilege
+		//jcr:modifyAccessControl is not posted, so it should remain in the denied ACE
+		postParams2.add(new NameValuePair("privilege@jcr:modifyAccessControl", "denied")); //deny the modifyAccessControl privilege
+		postParams2.add(new NameValuePair("privilege@jcr:removeChildNodes", "none")); //clear the existing privilege
+		postParams2.add(new NameValuePair("privilege@jcr:removeNode", "denied")); //deny a new privilege
+		
+		assertAuthenticatedPostStatus(creds, postUrl, HttpServletResponse.SC_OK, postParams2, null);
+		
+		
+		//fetch the JSON for the acl to verify the settings.
+		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		
+		assertNotNull(json2);
+		JSONObject jsonObj2 = new JSONObject(json2);
+		String aceString2 = jsonObj2.getString(testUserId);
+		assertNotNull(aceString2);
+		
+		JSONObject aceObject2 = new JSONObject(aceString2); 
+		assertNotNull(aceObject2);
+		
+		JSONArray grantedArray2 = aceObject2.getJSONArray("granted");
+		assertNotNull(grantedArray2);
+		assertEquals(3, grantedArray2.length());
+		Set<String> grantedPrivilegeNames2 = new HashSet<String>();
+		for (int i=0; i < grantedArray2.length(); i++) {
+			grantedPrivilegeNames2.add(grantedArray2.getString(i));
+		}
+		assertTrue(grantedPrivilegeNames2.contains("jcr:read"));
+		assertTrue(grantedPrivilegeNames2.contains("jcr:addChildNodes"));
+		assertTrue(grantedPrivilegeNames2.contains("jcr:modifyProperties"));
+
+		JSONArray deniedArray2 = aceObject2.getJSONArray("denied");
+		assertNotNull(deniedArray2);
+		assertEquals(2, deniedArray2.length());
+		Set<String> deniedPrivilegeNames2 = new HashSet<String>();
+		for (int i=0; i < deniedArray2.length(); i++) {
+			deniedPrivilegeNames2.add(deniedArray2.getString(i));
+		}
+		assertTrue(deniedPrivilegeNames2.contains("jcr:modifyAccessControl"));
+		assertTrue(deniedPrivilegeNames2.contains("jcr:removeNode"));
+	}
+	
 }