From 6b6247c180be9f489da7dba683c239a893f25c01 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Fri, 12 Jul 2013 12:12:39 +0000
Subject: [PATCH] AciEvalContext.java, AciContainer.java: Removed instance member isDenyEval, setDecidingAci() and setEvalReason(), replaced with setEvaluationResult() since both are always set at the same time. Changed the impl of isDenyEval() to use the evalReason.

---
 opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java       |   38 +++---------
 opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java         |   72 ++++++++---------------
 opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java     |   29 +++------
 opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEffectiveRights.java |    7 +-
 4 files changed, 46 insertions(+), 100 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
index 3e0b68d..786a948 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
@@ -82,11 +82,6 @@
     private boolean isEntryTestRule = false;
 
     /**
-     * True if the evaluation of an ACI is from the deny list.
-     */
-    private boolean isDenyEval;
-
-    /**
      * True if the evaluation is a result of an LDAP add operation.
      */
     private boolean isAddOp=false;
@@ -477,27 +472,19 @@
     * {@inheritDoc}
     */
     @Override
-    public void setDecidingAci(Aci aci) {
-      this.decidingAci=aci;
-    }
-
-   /**
-    * {@inheritDoc}
-    */
-    @Override
     public String getDecidingAciName() {
       if(this.decidingAci != null)
          return this.decidingAci.getName();
       else return null;
     }
 
-   /**
-    * {@inheritDoc}
-    */
-    @Override
-    public void setEvalReason(EnumEvalReason reason) {
-      this.evalReason=reason;
-    }
+  /** {@inheritDoc} */
+  @Override
+  public void setEvaluationResult(EnumEvalReason reason, Aci decidingAci)
+  {
+    this.evalReason = reason;
+    this.decidingAci = decidingAci;
+  }
 
    /**
     * {@inheritDoc}
@@ -665,7 +652,8 @@
     */
     @Override
     public boolean isDenyEval() {
-        return isDenyEval;
+        return EnumEvalReason.NO_ALLOW_ACIS.equals(evalReason)
+            || EnumEvalReason.EVALUATED_DENY_ACI.equals(evalReason);
     }
 
    /**
@@ -680,14 +668,6 @@
     * {@inheritDoc}
     */
     @Override
-    public void setDenyEval(boolean val) {
-        isDenyEval = val;
-    }
-
-   /**
-    * {@inheritDoc}
-    */
-    @Override
     public DN getClientDN() {
       if(this.useAuthzid)
         return this.authzid;
diff --git a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEffectiveRights.java b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEffectiveRights.java
index 616641b..6a6188c 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEffectiveRights.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEffectiveRights.java
@@ -29,6 +29,7 @@
 package org.opends.server.authorization.dseecompat;
 
 import static org.opends.server.authorization.dseecompat.Aci.*;
+
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.types.*;
 
@@ -433,8 +434,7 @@
     //another authzid or they don't have privs  -- fall through.
     if(skipCheck && container.isAuthzidAuthorizationDN()) {
       resString.append("write").append(":1");
-      container.setEvalReason(EnumEvalReason.SKIP_ACI);
-      container.setDecidingAci(null);
+      container.setEvaluationResult(EnumEvalReason.SKIP_ACI, null);
       createSummary(container, true, "main");
     } else {
      //Reset everything.
@@ -559,8 +559,7 @@
     //another authzid or they don't have privs  -- fall through.
     if(skipCheck && container.isAuthzidAuthorizationDN()) {
       resString.append(rightStr).append(":1");
-      container.setEvalReason(EnumEvalReason.SKIP_ACI);
-      container.setDecidingAci(null);
+      container.setEvaluationResult(EnumEvalReason.SKIP_ACI, null);
       createSummary(container, true, "main");
     } else {
       boolean ret;
diff --git a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
index 751aba7..6901fd6 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
@@ -74,12 +74,6 @@
     public List<Aci> getAllowList();
 
     /**
-     * Set when the deny list is being evaluated.
-     * @param v True if deny's are being evaluated.
-     */
-    public void setDenyEval(boolean v);
-
-    /**
      * Returns true if the deny list is being evaluated.
      * @return True if the deny list is being evaluated.
      */
@@ -217,14 +211,17 @@
     public void setTargAttrFiltersMatchOp(int flag);
 
   /**
-   * Set the reason the last access evaluation was evaluated the way it
-   * was. Used by geteffectiverights control evaluation to eventually build the
-   * summary string.
+   * Set the reason and the ACI that decided why the last access evaluation was
+   * evaluated the way it was. Used by geteffectiverights control evaluation to
+   * eventually build the summary string.
    *
-   * @param reason  The enumeration representing the reason of the last access
-   * evaluation.
+   * @param reason
+   *          The enumeration representing the reason of the last access
+   *          evaluation.
+   * @param decidingAci
+   *          The ACI that decided the last access evaluation.
    */
-    public void setEvalReason(EnumEvalReason reason);
+  void setEvaluationResult(EnumEvalReason reason, Aci decidingAci);
 
   /**
    * Return the reason the last access evaluation was evaluated the way it
@@ -237,14 +234,6 @@
     public EnumEvalReason getEvalReason();
 
   /**
-   * Set the ACI that decided that last access evaluation. Used by
-   * geteffectiverights control evaluation to the build summary string.
-   *
-   * @param aci The ACI that decided the last access evaluation.
-   */
-    public void setDecidingAci(Aci aci);
-
-  /**
    * Check if an evaluation context contains a set of access rights.
    *
    * @param rights The rights mask to check.
diff --git a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
index d097801..a0c13e2 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
@@ -30,6 +30,7 @@
 
 import static org.opends.messages.AccessControlMessages.*;
 import static org.opends.server.authorization.dseecompat.Aci.*;
+import static org.opends.server.authorization.dseecompat.EnumEvalReason.*;
 import static org.opends.server.config.ConfigConstants.*;
 import static org.opends.server.loggers.ErrorLogger.*;
 import static org.opends.server.loggers.debug.DebugLogger.*;
@@ -1389,17 +1390,15 @@
    */
   private boolean testApplicableLists(AciEvalContext evalCtx)
   {
-    evalCtx.setEvalReason(EnumEvalReason.NO_REASON);
-    // If allows list is empty and not doing geteffectiverights return
-    // false.
-    evalCtx.setDenyEval(true);
+    evalCtx.setEvaluationResult(NO_REASON, null);
+
     if (evalCtx.getAllowList().isEmpty()
-        && !(evalCtx.isGetEffectiveRightsEval()
-            && !evalCtx.hasRights(ACI_SELF) && evalCtx
-            .isTargAttrFilterMatchAciEmpty()))
+        && (!evalCtx.isGetEffectiveRightsEval()
+            || evalCtx.hasRights(ACI_SELF)
+            || !evalCtx.isTargAttrFilterMatchAciEmpty()))
     {
-      evalCtx.setEvalReason(EnumEvalReason.NO_ALLOW_ACIS);
-      evalCtx.setDecidingAci(null);
+      // If allows list is empty and not doing geteffectiverights return false.
+      evalCtx.setEvaluationResult(NO_ALLOW_ACIS, null);
       return false;
     }
 
@@ -1410,36 +1409,25 @@
       // search fails
       if (res.equals(EnumEvalResult.FAIL))
       {
-        evalCtx.setEvalReason(EnumEvalReason.EVALUATED_DENY_ACI);
-        evalCtx.setDecidingAci(denyAci);
+        evalCtx.setEvaluationResult(EVALUATED_DENY_ACI, denyAci);
         return false;
       }
       else if (res.equals(EnumEvalResult.TRUE))
       {
         if (evalCtx.isGetEffectiveRightsEval()
             && !evalCtx.hasRights(ACI_SELF)
-            && !evalCtx.isTargAttrFilterMatchAciEmpty())
+            && !evalCtx.isTargAttrFilterMatchAciEmpty()
+            // Iterate to next only if deny ACI contains a targattrfilters
+            // keyword.
+            && AciEffectiveRights.setTargAttrAci(evalCtx, denyAci, true))
         {
-          // Iterate to next only if deny ACI contains a targattrfilters
-          // keyword.
-          if (AciEffectiveRights.setTargAttrAci(evalCtx, denyAci, true))
-          {
-            continue;
-          }
-          evalCtx.setEvalReason(EnumEvalReason.EVALUATED_DENY_ACI);
-          evalCtx.setDecidingAci(denyAci);
-          return false;
+          continue;
         }
-        else
-        {
-          evalCtx.setEvalReason(EnumEvalReason.EVALUATED_DENY_ACI);
-          evalCtx.setDecidingAci(denyAci);
-          return false;
-        }
+        evalCtx.setEvaluationResult(EVALUATED_DENY_ACI, denyAci);
+        return false;
       }
     }
-    // Now check the allows -- flip the deny flag to false first.
-    evalCtx.setDenyEval(false);
+
     for (Aci allowAci : evalCtx.getAllowList())
     {
       final EnumEvalResult res = Aci.evaluate(evalCtx, allowAci);
@@ -1447,29 +1435,19 @@
       {
         if (evalCtx.isGetEffectiveRightsEval()
             && !evalCtx.hasRights(ACI_SELF)
-            && !evalCtx.isTargAttrFilterMatchAciEmpty())
+            && !evalCtx.isTargAttrFilterMatchAciEmpty()
+            // Iterate to next only if deny ACI contains a targattrfilters
+            // keyword.
+            && AciEffectiveRights.setTargAttrAci(evalCtx, allowAci, false))
         {
-          // Iterate to next only if deny ACI contains a targattrfilters
-          // keyword.
-          if (AciEffectiveRights.setTargAttrAci(evalCtx, allowAci, false))
-          {
-            continue;
-          }
-          evalCtx.setEvalReason(EnumEvalReason.EVALUATED_ALLOW_ACI);
-          evalCtx.setDecidingAci(allowAci);
-          return true;
+          continue;
         }
-        else
-        {
-          evalCtx.setEvalReason(EnumEvalReason.EVALUATED_ALLOW_ACI);
-          evalCtx.setDecidingAci(allowAci);
-          return true;
-        }
+        evalCtx.setEvaluationResult(EVALUATED_ALLOW_ACI, allowAci);
+        return true;
       }
     }
     // Nothing matched fall through.
-    evalCtx.setEvalReason(EnumEvalReason.NO_MATCHED_ALLOWS_ACIS);
-    evalCtx.setDecidingAci(null);
+    evalCtx.setEvaluationResult(NO_MATCHED_ALLOWS_ACIS, null);
     return false;
   }
 

--
Gitblit v1.10.0