From 68918206f56fc7a65ce9b84a9cf6a30edf8ce7c2 Mon Sep 17 00:00:00 2001
From: Jason Gerlowski <gerlowskija@apache.org>
Date: Fri, 29 Mar 2019 12:10:28 -0400
Subject: [PATCH] SOLR-13355: Small refactors to RuleBasedAuthorizationPlugin

---
 .../RuleBasedAuthorizationPlugin.java         | 120 +++++++++++-------
 1 file changed, 74 insertions(+), 46 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
index 6bf2822d624..7f97c880a59 100644
--- a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
@@ -112,55 +112,83 @@ public class RuleBasedAuthorizationPlugin implements AuthorizationPlugin, Config
   private MatchStatus checkPathPerm(List<Permission> permissions, AuthorizationContext context) {
     if (permissions == null || permissions.isEmpty()) return MatchStatus.NO_PERMISSIONS_FOUND;
     Principal principal = context.getUserPrincipal();
-    loopPermissions:
+
+    final Permission governingPermission = findFirstGoverningPermission(permissions, context);
+    if (governingPermission == null) {
+      log.debug("No permissions configured for the resource {} . So allowed to access", context.getResource());
+      return MatchStatus.NO_PERMISSIONS_FOUND;
+    }
+
+    return determineIfPermissionPermitsPrincipal(principal, governingPermission);
+  }
+
+  private Permission findFirstGoverningPermission(List<Permission> permissions, AuthorizationContext context) {
     for (int i = 0; i < permissions.size(); i++) {
       Permission permission = permissions.get(i);
-      if (PermissionNameProvider.values.containsKey(permission.name)) {
-        if (context.getHandler() instanceof PermissionNameProvider) {
-          PermissionNameProvider handler = (PermissionNameProvider) context.getHandler();
-          PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
-          if (permissionName == null || !permission.name.equals(permissionName.name)) {
-            continue;
-          }
-        } else {
-          //all is special. it can match any
-          if(permission.wellknownName != PermissionNameProvider.Name.ALL) continue;
-        }
-      } else {
-        if (permission.method != null && !permission.method.contains(context.getHttpMethod())) {
-          //this permissions HTTP method does not match this rule. try other rules
-          continue;
-        }
-        if (permission.params != null) {
-          for (Map.Entry<String, Function<String[], Boolean>> e : permission.params.entrySet()) {
-            String[] paramVal = context.getParams().getParams(e.getKey());
-            if(!e.getValue().apply(paramVal)) continue loopPermissions;
-          }
-        }
-      }
-
-      if (permission.role == null) {
-        //no role is assigned permission.That means everybody is allowed to access
-        return MatchStatus.PERMITTED;
-      }
-      if (principal == null) {
-        log.info("request has come without principal. failed permission {} ",permission);
-        //this resource needs a principal but the request has come without
-        //any credential.
-        return MatchStatus.USER_REQUIRED;
-      } else if (permission.role.contains("*")) {
-        return MatchStatus.PERMITTED;
-      }
-
-      for (String role : permission.role) {
-        Set<String> userRoles = usersVsRoles.get(principal.getName());
-        if (userRoles != null && userRoles.contains(role)) return MatchStatus.PERMITTED;
-      }
-      log.info("This resource is configured to have a permission {}, The principal {} does not have the right role ", permission, principal);
-      return MatchStatus.FORBIDDEN;
+      if (permissionAppliesToRequest(permission, context)) return permission;
     }
-    log.debug("No permissions configured for the resource {} . So allowed to access", context.getResource());
-    return MatchStatus.NO_PERMISSIONS_FOUND;
+
+    return null;
+  }
+
+  private boolean permissionAppliesToRequest(Permission permission, AuthorizationContext context) {
+    if (PermissionNameProvider.values.containsKey(permission.name)) {
+      return predefinedPermissionAppliesToRequest(permission, context);
+    } else {
+      return customPermissionAppliesToRequest(permission, context);
+    }
+  }
+
+  private boolean predefinedPermissionAppliesToRequest(Permission predefinedPermission, AuthorizationContext context) {
+    if (context.getHandler() instanceof PermissionNameProvider) {
+      PermissionNameProvider handler = (PermissionNameProvider) context.getHandler();
+      PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
+      if (permissionName == null || !predefinedPermission.name.equals(permissionName.name)) {
+        return false;
+      }
+    } else {
+      //all is special. it can match any
+      if(predefinedPermission.wellknownName != PermissionNameProvider.Name.ALL) return false;
+    }
+
+    return true;
+  }
+
+  private boolean customPermissionAppliesToRequest(Permission customPermission, AuthorizationContext context) {
+    if (customPermission.method != null && !customPermission.method.contains(context.getHttpMethod())) {
+      //this permissions HTTP method does not match this rule. try other rules
+      return false;
+    }
+    if (customPermission.params != null) {
+      for (Map.Entry<String, Function<String[], Boolean>> e : customPermission.params.entrySet()) {
+        String[] paramVal = context.getParams().getParams(e.getKey());
+        if(!e.getValue().apply(paramVal)) return false;
+      }
+    }
+
+    return true;
+  }
+
+  private MatchStatus determineIfPermissionPermitsPrincipal(Principal principal, Permission governingPermission) {
+    if (governingPermission.role == null) {
+      //no role is assigned permission.That means everybody is allowed to access
+      return MatchStatus.PERMITTED;
+    }
+    if (principal == null) {
+      log.info("request has come without principal. failed permission {} ", governingPermission);
+      //this resource needs a principal but the request has come without
+      //any credential.
+      return MatchStatus.USER_REQUIRED;
+    } else if (governingPermission.role.contains("*")) {
+      return MatchStatus.PERMITTED;
+    }
+
+    for (String role : governingPermission.role) {
+      Set<String> userRoles = usersVsRoles.get(principal.getName());
+      if (userRoles != null && userRoles.contains(role)) return MatchStatus.PERMITTED;
+    }
+    log.info("This resource is configured to have a permission {}, The principal {} does not have the right role ", governingPermission, principal);
+    return MatchStatus.FORBIDDEN;
   }
 
   @Override