Removed AuthorizationMap entirely, implemented SpEL authorization

pom.xml

@@ -6,7 +6,7 @@
   <groupId>edu.vt.middleware</groupId>
   <artifactId>edldap</artifactId>
   <packaging>jar</packaging>
-  <version>3.0-SNAPSHOT</version>
+  <version>3.0</version>
   <name>Enterprise Directory LDAP Libraries</name>
   <url>http://www.middleware.vt.edu/doku.php?id=middleware:ed:edldap</url>
   <licenses>

src/main/java/edu/vt/middleware/ldap/ed/AuthorizationMap.java

-/* See LICENSE for licensing and NOTICE for copyright. */
-package edu.vt.middleware.ldap.ed;
-
-import java.util.HashMap;
-
-/**
- * Represents authorization map for EdAuth.
- *
- * @author Middleware Services
- */
-public class AuthorizationMap extends HashMap<String, String[]>
-{
-
-  @Override
-  public String toString()
-  {
-    final StringBuilder sb = new StringBuilder();
-
-    for (String attribute : keySet()) {
-      sb.append(attribute);
-      sb.append(" = {");
-      final String[] values = get(attribute);
-      for (int i = 0; i < values.length; i++) {
-        final String value = values[i];
-        sb.append(value);
-        if (i + 1 < values.length) {
-          sb.append(", ");
-        }
-      }
-      sb.append("}\n");
-    }
-    return sb.toString();
-  }
-}

src/main/java/edu/vt/middleware/ldap/ed/AuthorizationMethod.java

-/* See LICENSE for licensing and NOTICE for copyright. */
-package edu.vt.middleware.ldap.ed;
-
-/**
- * Represents authorization methods for EdAuth.
- *
- * @author  Middleware Services
- */
-public enum AuthorizationMethod {
-
-  /** Authorized if any of the values match. */
-  ANY,
-
-  /** Authorized only if all of the values match. */
-  ALL,
-}

src/main/java/edu/vt/middleware/ldap/ed/EdAuth.java

@@ -2,9 +2,9 @@
 package edu.vt.middleware.ldap.ed;
 
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
-import java.util.Set;
+import java.util.Map;
 import edu.vt.middleware.ldap.ed.beans.VirginiaTechPerson;
 import org.ldaptive.ConnectionFactory;
 import org.ldaptive.Credential;
@@ -18,6 +18,11 @@ import org.ldaptive.auth.Authenticator;
 import org.ldaptive.auth.BindAuthenticationHandler;
 import org.ldaptive.auth.SearchDnResolver;
 import org.ldaptive.beans.reflect.DefaultLdapEntryMapper;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.Expression;
+import org.springframework.expression.ExpressionParser;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.StandardEvaluationContext;
 
 /**
  * EdAuth provides methods to perform common authentication and authorization
@@ -141,99 +146,46 @@ public final class EdAuth implements EdAuthService
   }
 
   @Override
-  public boolean authenticateAndAuthorize(
+  public void authenticateAndAuthorize(
     final String user,
     final Credential credential,
-    final AuthorizationMap authorizationMap,
-    final AuthorizationMethod method)
+    final String authorizationExpression)
     throws LdapException, EdAuthAuthorizationException
   {
 
-    boolean authorized = false;
+    final Map<String, String[]> attributesAndValues = new HashMap<>();
+    //Return all user attributes
+    final LdapEntry entry = authenticateAndGetAttributes(
+      user,
+      credential,
+      "*");
+    for (String userAttributeName : entry.getAttributeNames()) {
+      attributesAndValues.put(userAttributeName.trim().toLowerCase(),
+        entry.getAttribute(userAttributeName)
+                .getStringValues()
+                .toArray(new String[0])
+      );
+    }
 
-    if (authorizationMap.keySet().isEmpty()) {
+    if (authorizationExpression.trim().isEmpty())
+    {
       throw new EdAuthAuthorizationException(
         EdAuthAuthorizationException.EDAUTH_EXCEPTION_MSG_KEYSET_EMPTY);
     }
 
-    final String[] returnAttributes;
-    final Set<String> var = authorizationMap.keySet();
-    returnAttributes = var.toArray(new String[var.size()]);
+    final EdAuthorizationContext authorizationContext =
+            new EdAuthorizationContext(attributesAndValues);
 
-    final LdapEntry entry = authenticateAndGetAttributes(
-      user,
-      credential,
-      returnAttributes);
-    final String[] returnedAttributes = entry.getAttributeNames();
-
-    if (method == AuthorizationMethod.ALL) {
-      if (returnAttributes.length == returnedAttributes.length) {
-        // Make both arrays lowercase for comparison
-        for (int i = 0; i < returnAttributes.length; i++) {
-          returnAttributes[i] = returnAttributes[i].toLowerCase();
-          returnedAttributes[i] = returnedAttributes[i].toLowerCase();
-        }
-        if (
-          !(Arrays.asList(returnedAttributes).containsAll(
-                Arrays.asList(returnAttributes)))) {
-          throw new EdAuthAuthorizationException(
-            EdAuthAuthorizationException.EDAUTH_EXCEPTION_MSG_ATTR_NOT_RETURNED
-          );
-        }
-      } else {
-        throw new EdAuthAuthorizationException(
-          EdAuthAuthorizationException.EDAUTH_EXCEPTION_MSG_ATTR_NOT_RETURNED
-        );
-      }
-    }
+    final ExpressionParser parser = new SpelExpressionParser();
+    final Expression exp = parser.parseExpression(authorizationExpression);
+    final EvaluationContext context =
+            new StandardEvaluationContext(authorizationContext);
 
-    switch (method) {
-
-    case ANY:
-    mainloop:
-      for (String requestedAttribute : authorizationMap.keySet()) {
-        final LdapAttribute returnedAttribute = entry.getAttribute(
-          requestedAttribute);
-        final List<String> authorizationValues = Arrays.asList(
-          authorizationMap.get(requestedAttribute));
-        final List<String> attributeValues;
-        if (returnedAttribute != null) {
-          attributeValues = new ArrayList<>(
-            returnedAttribute.getStringValues());
-        } else {
-          attributeValues = new ArrayList<>();
-        }
-        for (String attributeValue : attributeValues) {
-          for (String authorizationValue : authorizationValues) {
-            if (authorizationValue.equals(attributeValue)) {
-              authorized = true;
-              break mainloop;
-            }
-          }
-        }
-      }
-      break;
-
-    case ALL:
-    default:
-
-      /**
-       * There will always be at least 1 comparison below with logical-and,
-       * therefore it's safe to set this to true.
-       */
-      authorized = true;
-      for (String requestedAttribute : authorizationMap.keySet()) {
-        final LdapAttribute returnedAttribute = entry.getAttribute(
-          requestedAttribute);
-        final List<String> authorizationValues = Arrays.asList(
-          authorizationMap.get(requestedAttribute));
-        final List<String> attributeValues = new ArrayList<>(
-          returnedAttribute.getStringValues());
-        authorized = authorized &&
-          attributeValues.containsAll(authorizationValues);
-      }
+    if (!exp.getValue(context, Boolean.class)) {
+      throw new EdAuthAuthorizationException(
+              EdAuthAuthorizationException.
+                      EDAUTH_EXCEPTION_MSG_ATTR_NOT_RETURNED);
     }
-    return authorized;
   }
 
   @Override
@@ -297,4 +249,75 @@ public final class EdAuth implements EdAuthService
 
     return memberships.toArray(new String[memberships.size()]);
   }
+
+  /**
+  * SpEL Authorization Context for this class.
+  *
+  * @author  Middleware Services
+  */
+  class EdAuthorizationContext
+  {
+
+    /** Map representing the attributes and their values **/
+    private final Map<String, String[]> entryAttributesAndValues;
+
+    /**
+     * Constructor must have the authorization attributes map
+     * @param entryAttributeValuesMap entryAttributeValuesMap
+     **/
+    public EdAuthorizationContext(
+          final Map<String, String[]> entryAttributeValuesMap
+    )
+    {
+      this.entryAttributesAndValues = entryAttributeValuesMap;
+    }
+
+    /**
+     * Whether the entry has an attribute specified
+     *
+     * @param attribute String value of the attribute name (Case insensitive)
+     * @return boolean
+     **/
+    public boolean hasAttribute(final String attribute)
+    {
+      boolean allow = false;
+      for (String attributeName : entryAttributesAndValues.keySet()) {
+        if (attribute.trim().toLowerCase().equals(attributeName)) {
+          allow = true;
+          break;
+        }
+      }
+      return allow;
+    }
+
+    /**
+     * Whether the entry has an attribute and the value specified
+     *
+     * @param attribute String value of the attribute name (Case insensitive)
+     * @param attributeValue String value of the attribute (Case sensitive)
+     * @return boolean
+     **/
+    public boolean hasAttributeValue(
+            final String attribute,
+            final String attributeValue)
+    {
+      boolean allow = false;
+    outerloop:
+      for (String attributeName : entryAttributesAndValues.keySet()) {
+        if (attribute.trim().toLowerCase().equals(attributeName)) {
+          for (String entryAttributeValue :
+                  entryAttributesAndValues.get(attributeName)
+          )
+          {
+            if (entryAttributeValue.equals(attributeValue)) {
+              allow = true;
+              break outerloop;
+            }
+          }
+          break;
+        }
+      }
+      return allow;
+    }
+  }
 }

src/main/java/edu/vt/middleware/ldap/ed/EdAuthAuthorizationException.java

@@ -19,11 +19,11 @@ public class EdAuthAuthorizationException extends Exception
    * doesn't have access to or has mistyped.
    */
   public static final String EDAUTH_EXCEPTION_MSG_ATTR_NOT_RETURNED =
-    "Could not retrieve all attributes requested for authorization.";
+    "Could not match all attributes requested for authorization.";
 
-  /** Error message for trying to authorize with an empty authorization map. */
+  /** Error message for trying to authorize with an empty authorization. */
   public static final String EDAUTH_EXCEPTION_MSG_KEYSET_EMPTY =
-    "Authorization map cannot be empty.";
+    "Authorization expression cannot be empty.";
 
 
   /** Creates a new EdAuthAuthorization exception. */

src/main/java/edu/vt/middleware/ldap/ed/EdAuthService.java

@@ -74,25 +74,19 @@ public interface EdAuthService extends EdOperation
    *
    * @param  user  username for bind
    * @param  credential  credential for bind
-   * @param  authorizationMap  to authorize with. Attribute names correspond to
-   * the map keys and attribute values correspond to an array of values. This
-   * method compares values only. Attribute names may be case-insensitive
-   * however attribute values are case-sensitive, compared using the {@link
-   * String#equals(Object)} method.
-   * @param  method  specifies the type of authorization performed on the
-   * authorizationMap.
-   *
-   * @return  ldap entry with the requested attributes
+   * @param  authorizationExpression  the SpEL expression to authorize with:
+   * Available methods are hasAttributeValue(String attribute, String value)
+   * and hasAttribute(String attribute).  The entire expression evaluates to
+   * false authorization will fail with EdAuthAuthorizationException
    *
    * @throws  LdapException  if the authentication fails for any reason
-   * @throws  EdAuthAuthorizationException  if the ldap entry doesn't return all
-   * the requested attributes
+   * @throws  EdAuthAuthorizationException  if the authorization fails for
+   * any reason
    */
-  boolean authenticateAndAuthorize(
+  void authenticateAndAuthorize(
     String user,
     Credential credential,
-    AuthorizationMap authorizationMap,
-    AuthorizationMethod method)
+    String authorizationExpression)
     throws LdapException, EdAuthAuthorizationException;
 
 

src/test/java/edu/vt/middleware/ldap/ed/EdAuthTest.java

@@ -4,6 +4,7 @@ package edu.vt.middleware.ldap.ed;
 import edu.vt.middleware.ldap.ed.beans.VirginiaTechPerson;
 import org.ldaptive.Credential;
 import org.ldaptive.LdapEntry;
+import org.ldaptive.LdapException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.AssertJUnit;
@@ -140,55 +141,79 @@ public class EdAuthTest
       }
     }
 
-    final AuthorizationMap authMap = new AuthorizationMap();
-    authMap.put(authorizationAttribute,
-            new String[]{authorizationAttributeMatch});
+    edauth.authenticateAndAuthorize(
+      authId,
+      password,
+      "hasAttribute('" + authorizationAttribute2 + "')"
+    );
 
-    AssertJUnit.assertTrue(
-      edauth.authenticateAndAuthorize(
-        authId,
-        password,
-        authMap,
-        AuthorizationMethod.ALL
-      )
+    edauth.authenticateAndAuthorize(
+      authId,
+      password,
+      "hasAttributeValue('" + authorizationAttribute + "', '"+
+              authorizationAttributeMatch +"')"
     );
 
-    //Adding an inexistent attribute value which should do nothing for ANY.
-    authMap.put(authorizationAttribute2, new String[] {"inexistentValue"});
+    edauth.authenticateAndAuthorize(
+      authId,
+      password,
+      "hasAttribute('" + authorizationAttribute2 + "') &&" +
+              " hasAttributeValue('" + authorizationAttribute + "', '"+
+              authorizationAttributeMatch +"')"
+    );
 
-    AssertJUnit.assertTrue(
+    edauth.authenticateAndAuthorize(
+      authId,
+      password,
+      "hasAttribute('this does not exist, but the other does') ||" +
+              " hasAttributeValue('" + authorizationAttribute + "', '"+
+              authorizationAttributeMatch +"')"
+    );
+
+    //Check for a failed authentication
+    LdapException requiredException1;
+    try {
       edauth.authenticateAndAuthorize(
         authId,
-        password,
-        authMap,
-        AuthorizationMethod.ANY
-      )
-    );
+        new Credential("THIS IS NOT THE PASSWORD"),
+        "hasAttributeValue('" + authorizationAttribute + "', 'FAIL!')"
+      );
+      requiredException1 = null;
+    } catch (LdapException ex) {
+      requiredException1 = ex;
+    }
+
+    AssertJUnit.assertNotNull(requiredException1);
 
-    //Check for empty authorization map.
-    EdAuthAuthorizationException requiredException;
+    //Check for a failed authorization
+    EdAuthAuthorizationException requiredException2;
     try {
       edauth.authenticateAndAuthorize(
         authId,
         password,
-        new AuthorizationMap(),
-        AuthorizationMethod.ANY
+        "hasAttributeValue('" + authorizationAttribute + "', 'FAIL!')"
       );
-      requiredException = null;
+      requiredException2 = null;
     } catch (EdAuthAuthorizationException ex) {
-      requiredException = ex;
+      requiredException2 = ex;
     }
 
-    AssertJUnit.assertNotNull(requiredException);
+    AssertJUnit.assertNotNull(requiredException2);
 
-    AssertJUnit.assertFalse(
+    //Check for empty authorization expression.
+    EdAuthAuthorizationException requiredException3;
+    try {
       edauth.authenticateAndAuthorize(
         authId,
         password,
-        authMap,
-        AuthorizationMethod.ALL
-      )
-    );
+        ""
+      );
+      requiredException3 = null;
+    } catch (EdAuthAuthorizationException ex) {
+      requiredException3 = ex;
+    }
+
+    AssertJUnit.assertNotNull(requiredException3);
 
     AssertJUnit.assertTrue(authorized);
   }

[Daniel Fisher]

Version change should be the last commit before a release. You don't want a case where someone can build something that says it's 3.0, but it's not. Personally I wish the git tag command had this built it.

[Daniel Fisher]

I don't see the spel artifact defined in the pom. That indicates it's a transitive dependency of spring web security? Since that is marked as a provided dependency we're going to run into problems with anyone using this in an environment that doesn't provide spel.

The current dependencies are: cryptacular, ldaptive, and slf4j. What will adding spel bring into the dependency graph?

[Daniel Fisher]

What's the use case for package level visibility here?

[Erdem A Memisyazici]

Absolutely none, changed to private. I also wrapped the EvaluationException I commented on earlier like so:

try {
  if (!exp.getValue(context, Boolean.class)) {
    throw new EdAuthAuthorizationException(
          EdAuthAuthorizationException.
                  EDAUTH_EXCEPTION_MSG_AUTHZ_FAILED);
  }
} catch (EvaluationException ee) {
  throw new EdAuthAuthorizationException(
          EdAuthAuthorizationException.
                  EDAUTH_EXCEPTION_MSG_AUTHZ_FAILED, ee);
}

[Daniel Fisher]

You can simplify this a little:

try {
  someClass.someMethod(parameters that cause exception);
  AssertJUnit.assertFail("Must throw exception");
} catch (Exception e) {
  AssertJUnit.assertEquals(SpecificException.class, e.getClass());
}