Author: jsedding
Date: Tue Feb 23 12:59:07 2016
New Revision: 1731850
URL: http://svn.apache.org/viewvc?rev=1731850&view=rev
Log:
OAK-4037 - Include paths in AccessControlExceptions
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java?rev=1731850&r1=1731849&r2=1731850&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
Tue Feb 23 12:59:07 2016
@@ -172,7 +172,7 @@ class AccessControlValidator extends Def
private static void checkIsAccessControlEntry(Tree tree) throws
CommitFailedException {
if (!isAccessControlEntry(tree)) {
- throw accessViolation(2, "Access control entry node expected.");
+ throw accessViolation(2, "Access control entry node expected at "
+ tree.getPath());
}
}
@@ -188,18 +188,18 @@ class AccessControlValidator extends Def
POLICY_NODE_NAMES :
Collections.singleton(REP_POLICY);
if (!validPolicyNames.contains(policyTree.getName())) {
- throw accessViolation(3, "Invalid policy name " +
policyTree.getName());
+ throw accessViolation(3, "Invalid policy name " +
policyTree.getName() + " at " + parent.getPath());
}
if (!policyNode.hasProperty(TreeConstants.OAK_CHILD_ORDER)) {
- throw accessViolation(4, "Invalid policy node: Order of children
is not stable.");
+ throw accessViolation(4, "Invalid policy node at " +
policyTree.getPath() + ": Order of children is not stable.");
}
Set<Entry> aceSet = Sets.newHashSet();
for (Tree child : policyTree.getChildren()) {
if (isAccessControlEntry(child)) {
if (!aceSet.add(new Entry(parent.getPath(), child))) {
- throw accessViolation(13, "Duplicate ACE found in policy");
+ throw accessViolation(13, "Duplicate ACE '" +
child.getPath() + "' found in policy");
}
}
}
@@ -223,38 +223,40 @@ class AccessControlValidator extends Def
if (!parent.exists() ||
!NT_REP_ACL.equals(TreeUtil.getPrimaryTypeName(parent))) {
throw accessViolation(7, "Isolated access control entry at " +
aceNode.getPath());
}
- checkValidPrincipal(TreeUtil.getString(aceNode, REP_PRINCIPAL_NAME));
- checkValidPrivileges(TreeUtil.getStrings(aceNode, REP_PRIVILEGES));
+ checkValidPrincipal(aceNode);
+ checkValidPrivileges(aceNode);
checkValidRestrictions(aceNode);
}
- private void checkValidPrincipal(@CheckForNull String principalName)
throws CommitFailedException {
+ private void checkValidPrincipal(@Nonnull Tree aceNode) throws
CommitFailedException {
+ String principalName = TreeUtil.getString(aceNode, REP_PRINCIPAL_NAME);
if (principalName == null || principalName.isEmpty()) {
- throw accessViolation(8, "Missing principal name.");
+ throw accessViolation(8, "Missing principal name at " +
aceNode.getPath());
}
// validity of principal is only a JCR specific contract and will not
be
// enforced on the oak level.
}
- private void checkValidPrivileges(Iterable<String> privilegeNames) throws
CommitFailedException {
+ private void checkValidPrivileges(@Nonnull Tree aceNode) throws
CommitFailedException {
+ Iterable<String> privilegeNames = TreeUtil.getStrings(aceNode,
REP_PRIVILEGES);
if (privilegeNames == null || Iterables.isEmpty(privilegeNames)) {
- throw accessViolation(9, "Missing privileges.");
+ throw accessViolation(9, "Missing privileges at " +
aceNode.getPath());
}
for (String privilegeName : privilegeNames) {
try {
Privilege privilege =
privilegeManager.getPrivilege(privilegeName);
if (privilege.isAbstract()) {
- throw accessViolation(11, "Abstract privilege " +
privilegeName);
+ throw accessViolation(11, "Abstract privilege " +
privilegeName + " at " + aceNode.getPath());
}
} catch (AccessControlException e) {
- throw accessViolation(10, "Invalid privilege " +
privilegeName);
+ throw accessViolation(10, "Invalid privilege " + privilegeName
+ " at " + aceNode.getPath());
} catch (RepositoryException e) {
throw new IllegalStateException("Failed to read privileges",
e);
}
}
}
- private void checkValidRestrictions(Tree aceTree) throws
CommitFailedException {
+ private void checkValidRestrictions(@Nonnull Tree aceTree) throws
CommitFailedException {
String path;
Tree aclTree = checkNotNull(aceTree.getParent());
String aclPath = aclTree.getPath();
@@ -282,7 +284,7 @@ class AccessControlValidator extends Def
private static void checkValidRepoAccessControlled(Tree
accessControlledTree) throws CommitFailedException {
if (!accessControlledTree.isRoot()) {
- throw accessViolation(12, "Only root can store repository level
policies.");
+ throw accessViolation(12, "Only root can store repository level
policies (" + accessControlledTree.getPath() + ')');
}
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java?rev=1731850&r1=1731849&r2=1731850&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
Tue Feb 23 12:59:07 2016
@@ -38,9 +38,10 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -110,7 +111,8 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
- assertEquals("OakAccessControl0004: Invalid policy node: Order of
children is not stable.", e.getMessage());
+ assertThat(e.getMessage(),
containsString("OakAccessControl0004")); // Order of children is not stable
+ assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
}
}
@@ -125,6 +127,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot"));
}
}
@@ -139,6 +142,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot"));
} finally {
policy.getTree().remove();
}
@@ -158,6 +162,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isConstraintViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/rep:policy"));
} finally {
policy.getTree().remove();
}
@@ -178,6 +183,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isConstraintViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/rep:policy"));
} finally {
policy.getTree().remove();
}
@@ -198,6 +204,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isConstraintViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/rep:policy/validAce"));
} finally {
entry.getTree().remove();
}
@@ -218,6 +225,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isConstraintViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/rep:policy"));
} finally {
entry.getTree().remove();
}
@@ -237,6 +245,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot"));
} finally {
// revert pending changes that cannot be saved.
policy.getTree().remove();
@@ -258,6 +267,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/isolatedACE"));
} finally {
// revert pending changes that cannot be saved.
ace.getTree().remove();
@@ -275,6 +285,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot"));
} finally {
// revert pending changes that cannot be saved.
restriction.getTree().remove();
@@ -293,6 +304,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
}
}
@@ -309,6 +321,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
}
}
@@ -322,6 +335,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
}
}
@@ -335,6 +349,7 @@ public class AccessControlValidatorTest
} catch (CommitFailedException e) {
// success
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
}
}
@@ -356,6 +371,7 @@ public class AccessControlValidatorTest
fail("Creating duplicate ACE must be detected");
} catch (CommitFailedException e) {
assertTrue(e.isAccessControlViolation());
+ assertThat(e.getMessage(),
containsString("/testRoot/rep:policy/duplicateAce"));
}
}
