perf: speed-up policy evaluation

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java

@@ -14,28 +14,27 @@
 import com.aws.greengrass.util.Utils;
 import lombok.Builder;
 import lombok.Value;
+import org.apache.commons.lang3.StringUtils;
 
 import java.util.Map;
 import java.util.Set;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.inject.Inject;
 
 public final class PermissionEvaluationUtils {
     private static final Logger logger = LogManager.getLogger(PermissionEvaluationUtils.class);
     private static final String ANY_REGEX = "*";
-    private static final String SERVICE_PATTERN_STRING = "([a-zA-Z]+)";
-    private static final String SERVICE_OPERATION_PATTERN_STRING = "([a-zA-Z0-9-_]+)";
-    private static final String SERVICE_RESOURCE_TYPE_PATTERN_STRING = "([a-zA-Z]+)";
-    // Characters, digits, special chars, space (Allowed in MQTT topics)
-    private static final String SERVICE_RESOURCE_NAME_PATTERN_STRING = "([\\w -\\/:-@\\[-\\`{-~]+)";
     private static final String SERVICE_OPERATION_FORMAT = "%s:%s";
     private static final String SERVICE_RESOURCE_FORMAT = "%s:%s:%s";
-    private static final Pattern SERVICE_OPERATION_PATTERN = Pattern.compile(
-            String.format(SERVICE_OPERATION_FORMAT, SERVICE_PATTERN_STRING, SERVICE_OPERATION_PATTERN_STRING));
-    private static final Pattern SERVICE_RESOURCE_PATTERN = Pattern.compile(
-            String.format(SERVICE_RESOURCE_FORMAT, SERVICE_PATTERN_STRING, SERVICE_RESOURCE_TYPE_PATTERN_STRING,
-                    SERVICE_RESOURCE_NAME_PATTERN_STRING), Pattern.UNICODE_CHARACTER_CLASS);
+    private static final Pattern RESOURCE_NAME_PATTERN =
+            Pattern.compile("([\\w -\\/:-@\\[-\\`{-~]+)", Pattern.UNICODE_CHARACTER_CLASS);
+    private static final String EXCEPTION_MALFORMED_OPERATION =
+            "Operation is malformed, must be of the form: "
+            + "([a-zA-Z]+):([a-zA-Z0-9-_]+)";
+    private static final String EXCEPTION_MALFORMED_RESOURCE =
+            "Resource is malformed, must be of the form: "
+            + "([a-zA-Z]+):([a-zA-Z]+):(" + RESOURCE_NAME_PATTERN.pattern() + "+)";
+
     private final GroupManager groupManager;
 
     /**
@@ -56,54 +55,66 @@ public PermissionEvaluationUtils(GroupManager groupManager) {
      *
      * @return boolean indicating if the operation requested is authorized
      */
+    @SuppressWarnings("PMD.AvoidBranchingStatementAsLastInLoop")
     public boolean isAuthorized(AuthorizationRequest request, Session session) {
-        Operation op = parseOperation(request.getOperation());
-        Resource rsc = parseResource(request.getResource());
+        Operation op;
+        try {
+            op = parseOperation(request.getOperation());
+        } catch (PolicyException e) {
+            logger.atError().setCause(e).log();
+            return false;
+        }
+
+        Resource rsc;
+        try {
+            rsc = parseResource(request.getResource());
+        } catch (PolicyException e) {
+            logger.atError().setCause(e).log();
+            return false;
+        }
 
         if (!rsc.getService().equals(op.getService())) {
             throw new IllegalArgumentException(
                     String.format("Operation %s service is not same as resource %s service", op, rsc));
-
         }
 
-        Map<String, Set<Permission>> groupToPermissionsMap = groupManager.getApplicablePolicyPermissions(session);
+        Map<String, Set<Permission>> groupPermissions = groupManager.getApplicablePolicyPermissions(session);
 
-        if (groupToPermissionsMap == null || groupToPermissionsMap.isEmpty()) {
+        if (groupPermissions == null || groupPermissions.isEmpty()) {
             logger.atDebug().kv("operation", request.getOperation()).kv("resource", request.getResource())
                     .log("No authorization group matches, " + "deny the request");
             return false;
         }
 
-        for (Map.Entry<String, Set<Permission>> entry : groupToPermissionsMap.entrySet()) {
+        for (Map.Entry<String, Set<Permission>> entry : groupPermissions.entrySet()) {
             String principal = entry.getKey();
-            Set<Permission> permissions = entry.getValue();
-            if (Utils.isEmpty(permissions)) {
-                continue;
-            }
 
             // Find the first matching permission since we don't support 'deny' operation yet.
             //TODO add support of 'deny' operation
-            Permission permission = permissions.stream().filter(e -> {
-                if (!comparePrincipal(principal, e.getPrincipal())) {
-                    return false;
+            for (Permission permission : entry.getValue()) {
+                if (!comparePrincipal(principal, permission.getPrincipal())) {
+                    continue;
                 }
-                if (!compareOperation(op, e.getOperation())) {
-                    return false;
+                if (!compareOperation(op, permission.getOperation())) {
+                    continue;
                 }
+
+                String resource;
                 try {
-                    return compareResource(rsc, e.getResource(session));
+                    resource = permission.getResource(session);
                 } catch (PolicyException er) {
                     logger.atError().setCause(er).log();
-                    return false;
+                    continue;
+                }
+
+                if (!compareResource(rsc, resource)) {
+                    continue;
                 }
-            }).findFirst().orElse(null);
 
-            if (permission != null) {
                 logger.atDebug().log("Hit policy with permission {}", permission);
                 return true;
             }
         }
-
         return false;
     }
 
@@ -119,6 +130,7 @@ private boolean compareOperation(Operation requestOperation, String policyOperat
         if (requestOperation.getOperationStr().equals(policyOperation)) {
             return true;
         }
+        // TODO
         if (String.format(SERVICE_OPERATION_FORMAT, requestOperation.getService(), ANY_REGEX).equals(policyOperation)) {
             return true;
         }
@@ -138,36 +150,89 @@ private boolean compareResource(Resource requestResource, String policyResource)
         return ANY_REGEX.equals(policyResource);
     }
 
-    private Operation parseOperation(String operationStr) {
+    private Operation parseOperation(String operationStr) throws PolicyException {
         if (Utils.isEmpty(operationStr)) {
-            throw new IllegalArgumentException("Operation can't be empty");
+            throw new PolicyException("Operation can't be empty");
+        }
+
+        int split = operationStr.indexOf(':');
+        if (split == -1) {
+            throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
+        }
+
+        String service = operationStr.substring(0, split);
+        if (service.isEmpty() || !StringUtils.isAlpha(service)) {
+            throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
         }
 
-        Matcher matcher = SERVICE_OPERATION_PATTERN.matcher(operationStr);
-        if (matcher.matches()) {
-            return Operation.builder().operationStr(operationStr).service(matcher.group(1)).action(matcher.group(2))
-                    .build();
+        String action = operationStr.substring(split + 1);
+        if (action.isEmpty() || !isAlphanumericWithExtraChars(action, "-_")) {
+            throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
         }
-        throw new IllegalArgumentException(String.format("Operation %s is not in the form of %s", operationStr,
-                SERVICE_OPERATION_PATTERN.pattern()));
+
+        return Operation.builder()
+                .operationStr(operationStr)
+                .service(service)
+                .action(action)
+                .build();
     }
 
-    private Resource parseResource(String resourceStr) {
+    private Resource parseResource(String resourceStr) throws PolicyException  {
         if (Utils.isEmpty(resourceStr)) {
-            throw new IllegalArgumentException("Resource can't be empty");
+            throw new PolicyException("Resource can't be empty");
         }
 
-        Matcher matcher = SERVICE_RESOURCE_PATTERN.matcher(resourceStr);
-        if (matcher.matches()) {
-            return Resource.builder().resourceStr(resourceStr)
-                    .service(matcher.group(1))
-                    .resourceType(matcher.group(2))
-                    .resourceName(matcher.group(3))
-                    .build();
+        int split = resourceStr.indexOf(':');
+        if (split == -1) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
 
-        throw new IllegalArgumentException(
-                String.format("Resource %s is not in the form of %s", resourceStr, SERVICE_RESOURCE_PATTERN.pattern()));
+        String service = resourceStr.substring(0, split);
+        if (service.isEmpty() || !StringUtils.isAlpha(service)) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
+        }
+
+        String typeAndName = resourceStr.substring(split + 1);
+        if (typeAndName.isEmpty()) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
+        }
+
+        split = typeAndName.indexOf(':');
+        if (split == -1) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
+        }
+
+        String resourceType = typeAndName.substring(0, split);
+        if (resourceType.isEmpty() || !StringUtils.isAlpha(resourceType)) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
+        }
+
+        String resourceName = typeAndName.substring(split + 1);
+
+        // still using regex because Pattern.UNICODE_CHARACTER_CLASS is complicated
+        if (!RESOURCE_NAME_PATTERN.matcher(resourceName).matches()) {
+            throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
+        }
+
+        return Resource.builder()
+                .resourceStr(resourceStr)
+                .service(service)
+                .resourceType(resourceType)
+                .resourceName(resourceName)
+                .build();
+    }
+
+    private static boolean isAlphanumericWithExtraChars(CharSequence cs, String extra) {
+        if (Utils.isEmpty(cs)) {
+            return false;
+        }
+        for (int i = 0; i < cs.length(); i++) {
+            char curr = cs.charAt(i);
+            if (!Character.isLetterOrDigit(curr) && extra.indexOf(curr) == -1) {
+                return false;
+            }
+        }
+        return true;
     }
 
     @Value

src/main/java/com/aws/greengrass/clientdevices/auth/configuration/GroupManager.java

@@ -8,11 +8,10 @@
 import com.aws.greengrass.clientdevices.auth.session.Session;
 
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 
 /**
  * Singleton manager class for managing device group roles and retrieving permissions associated with Sessions. To
@@ -37,21 +36,15 @@ public Map<String, Set<Permission>> getApplicablePolicyPermissions(Session sessi
         if (config == null) {
             return Collections.emptyMap();
         }
-        Set<String> matchingGroups = findMatchingGroups(config.getDefinitions(), session);
-        return matchingGroups.stream()
-                .collect(Collectors.toMap(group -> group, group -> config.getGroupToPermissionsMap().get(group)));
-    }
-
-    private Set<String> findMatchingGroups(Map<String, GroupDefinition> groupDefinitionMap, Session session) {
-        Set<String> matchingGroups = new HashSet<>();
-
-        for (String groupName : groupDefinitionMap.keySet()) {
-            GroupDefinition group = groupDefinitionMap.get(groupName);
-            if (group.containsClientDevice(session)) {
-                matchingGroups.add(groupName);
+        Map<String, Set<Permission>> groupPermissions = new HashMap<>();
+        for (Map.Entry<String, GroupDefinition> entry : config.getDefinitions().entrySet()) {
+            if (entry.getValue().containsClientDevice(session)) {
+                Set<Permission> permissions = config.getGroupToPermissionsMap().get(entry.getKey());
+                if (permissions != null) {
+                    groupPermissions.put(entry.getKey(), permissions);
+                }
             }
         }
-
-        return matchingGroups;
+        return groupPermissions;
     }
 }