Permission subset checks when creating roles (#64)

Expanded ability to check permissions for subset permissions; enforce role grants restrict to caller's permissions

quality/integration/src/test/java/test/integration/auth/RoleAdminTaskTest.java

@@ -57,7 +57,7 @@ public void setUp() {
 
         RoleIdentifier adminId = new RoleIdentifier(null, DefaultRoles.admin.toString());
         _roleManager.createRole(adminId, new RoleModification()
-                .withPermissionUpdate(new PermissionUpdateRequest().permit(ImmutableSet.of(Permissions.unlimitedRole()))));
+                .withPermissionUpdate(new PermissionUpdateRequest().permit(ImmutableSet.of(Permissions.unlimited()))));
         ApiKeySecurityManager securityManager = new ApiKeySecurityManager(
                 new ApiKeyRealm("test", new MemoryConstrainedCacheManager(), _authIdentityManager, permissionManager,
                         null));
@@ -84,6 +84,39 @@ public void testInvalidApiKey()
         assertEquals(out.toString(), "Not authorized\n");
     }
 
+    @Test
+    public void testInsufficientPermissionsToManageRoles()
+            throws Exception {
+        // Notably this role does not have permission to view roles
+        _roleManager.createRole(new RoleIdentifier(null, "insufficient-role"), new RoleModification()
+                .withPermissionUpdate(new PermissionUpdateRequest().permit(
+                        ImmutableSet.of("sor|read|*"))));
+
+        _authIdentityManager.createIdentity("insufficient-key", new ApiKeyModification().addRoles("insufficient-role"));
+
+        StringWriter out = new StringWriter();
+        _task.execute(ImmutableMultimap.of(
+                ApiKeyRequest.AUTHENTICATION_PARAM, "insufficient-key",
+                "action", "view",
+                "role", "unpermitted-role"),
+                new PrintWriter(out));
+
+        assertEquals(out.toString(), "Not authorized\n");
+
+        // Even without permission to view roles the API key should have permission to view it's own roles
+        out = new StringWriter();
+        _task.execute(ImmutableMultimap.of(
+                ApiKeyRequest.AUTHENTICATION_PARAM, "insufficient-key",
+                "action", "view",
+                "role", "insufficient-role"),
+                new PrintWriter(out));
+
+        // First line is a header.  Skip this and verify the single permission for "insufficient-role" is present
+        String[] lines = out.toString().split("\\n");
+        assertEquals(lines.length, 2);
+        assertEquals(lines[1], "- sor|read|*");
+    }
+
     @Test
     public void testViewRole()
             throws Exception {
@@ -226,4 +259,31 @@ public void testCheckRole()
             assertEquals(actual.subList(1, actual.size()), entry.getValue());
         }
     }
+
+    @Test
+    public void testCreateRoleWithUnboundPermissions() throws Exception  {
+        // First, give a new role all permissions within the "sor" context
+        _roleManager.createRole(new RoleIdentifier(null, "sor-only"), new RoleModification()
+                .withPermissionUpdate(new PermissionUpdateRequest().permit(
+                        ImmutableSet.of("sor|*"))));
+
+        _authIdentityManager.createIdentity("sor-only-key", new ApiKeyModification().addRoles("sor-only"));
+
+        StringWriter out = new StringWriter();
+
+        _task.execute(ImmutableMultimap.of(
+                ApiKeyRequest.AUTHENTICATION_PARAM, "sor-only-key",
+                "action", "update",
+                "role", "new-role",
+                "permit", "sor|update|if({..,\"foo\":\"bar\"})",
+                "permit", "queue|post|*"),
+                new PrintWriter(out));
+
+        List<String> actual = ImmutableList.copyOf(out.toString().split("\n"));
+
+        assertEquals(actual, ImmutableList.of(
+                "You do not has sufficient permissions to grant the following permission(s):",
+                "- queue|post|*",
+                "Please remove or rewrite the above permission(s) constrained within your permissions"));
+    }
 }

sor-api/src/main/java/com/bazaarvoice/emodb/sor/condition/Comparison.java

@@ -1,18 +1,34 @@
 package com.bazaarvoice.emodb.sor.condition;
 
+import com.bazaarvoice.emodb.sor.delta.Delta;
+
 public enum Comparison {
-    GT("gt"),
-    GE("ge"),
-    LT("lt"),
-    LE("le");
+    GT("gt", false),
+    GE("ge", true),
+    LT("lt", false),
+    LE("le", true);
 
     private final String _deltaFunction;
+    private final boolean _isClosed;
 
-    private Comparison(String deltaFunction) {
+    private Comparison(String deltaFunction, boolean isClosed) {
         _deltaFunction = deltaFunction;
+        _isClosed = isClosed;
     }
 
+    /**
+     * Returns the name of the this comparison as it appears as a function in the delta syntax.
+     * @see Delta#appendTo(Appendable)
+     */
     public String getDeltaFunction() {
         return _deltaFunction;
     }
+
+    /**
+     * A comparison is closed if the associated value is included in the defined range.  This method returns true for
+     * GE and LE, false for GT and LT.
+     */
+    public boolean isClosed() {
+        return _isClosed;
+    }
 }

sor-api/src/main/java/com/bazaarvoice/emodb/sor/condition/ComparisonCondition.java

@@ -5,4 +5,18 @@ public interface ComparisonCondition extends Condition {
     Comparison getComparison();
 
     Object getValue();
+
+    /**
+     * Returns true if there exists a value v which matches both this condition and the provided condition.
+     * For example, ge(5) overlaps lt(10) since all values 5 <= v < 10 match both, while ge(5) does not overlap lt(5)
+     * since they share no common values.
+     */
+    boolean overlaps(ComparisonCondition condition);
+
+    /**
+     * Returns true if for every value v which matches this condition v also matches the provided condition.
+     * For example, gt(10) is a subset of gt(5), while gt(10) is not a subset of lt(20) since all values
+     * v >= 20 match the former but not the latter.
+     */
+    boolean isSubsetOf(ComparisonCondition condition);
 }

sor-api/src/main/java/com/bazaarvoice/emodb/sor/condition/ContainsCondition.java

@@ -7,7 +7,7 @@
  */
 public interface ContainsCondition extends Condition {
 
-    public enum Containment {
+    enum Containment {
         ALL("All"),
         ANY("Any"),
         ONLY("Only");
@@ -27,4 +27,19 @@ public String getSuffix() {
     Set<Object> getValues();
 
     Containment getContainment();
+
+    /**
+     * Returns true if for every value v which matches this condition v also matches the provided condition.
+     * For example, containsAll("a","b","c") is a subset of containsAll("a","b"), while containsAll("a","b") is not
+     * a subset of containsAll("a","b","c") since v exists, ["a","b"], which matches the former but not the latter.
+     */
+    boolean isSubsetOf(ContainsCondition condition);
+
+    /**
+     * Returns true if there exists a value v which matches both this condition and the provided condition.
+     * For example, containsAny("a","b") overlaps containsAny("c","d") since there exists a value, ["a","c"], which
+     * matches both, while containsOnly("a","b") does not overlap containsOnly("c","d") since they share no common
+     * values.
+     */
+    boolean overlaps(ContainsCondition condition);
 }

sor-api/src/main/java/com/bazaarvoice/emodb/sor/condition/LikeCondition.java

@@ -1,6 +1,49 @@
 package com.bazaarvoice.emodb.sor.condition;
 
+import javax.annotation.Nullable;
+
 public interface LikeCondition extends Condition {
 
+    /**
+     * Returns the matching string used by this condition.
+     */
+    String getCondition();
+
+    /**
+     * Returns true if the provided String is a match for this condition.
+     */
     boolean matches(String input);
+
+    /**
+     * Returns true if there exists a value v which matches both this condition and the provided condition.
+     * For example, like("a*") overlaps like("*c") since there exists a value, "abc", which matches both,
+     * while like("a*") does not overlap like("b*") since they share no common values.
+     */
+    boolean overlaps(LikeCondition condition);
+
+    /**
+     * Returns true if for every value v which matches this condition v also matches the provided condition.
+     * For example, like("ab*") is a subset of like("a*"), while like("a*") is not a subset of like("*c") since
+     * there exists a value, "ab", which matches the former but not the latter.
+     */
+    boolean isSubsetOf(LikeCondition condition);
+
+    /**
+     * Returns the constant prefix shared by all results matching this condition, or null if no such prefix exists.
+     * For example:  "ab*cd" has prefix "ab" and "*cd" has prefix null.
+     */
+    @Nullable
+    String getPrefix();
+
+    /**
+     * Returns the constant suffix shared by all results matching this condition, or null if no such suffix exists.
+     * For example:  "ab*cd" has suffix "cd" and "ab*" has suffix null.
+     */
+    @Nullable
+    String getSuffix();
+
+    /**
+     * Returns true if the condition contains any wildcards, false if it is a constant.
+     */
+    boolean hasWildcards();
 }