From 12de4de1e650e2bb98ecfbdae723db0fd7537b7b Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Thu, 01 May 2014 00:25:45 +0000
Subject: [PATCH] Fix various bugs in access log filtering:

---
 opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisherTest.java |  363 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 opendj3-server-dev/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisher.java                             |   50 +++---
 2 files changed, 379 insertions(+), 34 deletions(-)

diff --git a/opendj3-server-dev/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisher.java b/opendj3-server-dev/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisher.java
index ef02824..2aa4c46 100644
--- a/opendj3-server-dev/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisher.java
+++ b/opendj3-server-dev/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisher.java
@@ -21,7 +21,7 @@
  * CDDL HEADER END
  *
  *
- *      Portions Copyright 2011-2014 ForgeRock AS
+ *      Copyright 2011-2014 ForgeRock AS
  */
 package org.opends.server.loggers;
 
@@ -68,7 +68,7 @@
   /**
    * Criteria based filter.
    */
-  private static final class CriteriaFilter implements Filter
+  static final class CriteriaFilter implements Filter
   {
     private final AccessLogFilteringCriteriaCfg cfg;
     private final boolean logConnectRecords;
@@ -96,7 +96,7 @@
      * @throws ConfigException
      *           If the configuration cannot be parsed.
      */
-    private CriteriaFilter(final AccessLogFilteringCriteriaCfg cfg)
+    CriteriaFilter(final AccessLogFilteringCriteriaCfg cfg)
         throws ConfigException
     {
       this.cfg = cfg;
@@ -470,24 +470,30 @@
     private boolean filterDN(final DN dn, PatternDN[] notEqualTo,
         PatternDN[] equalTo)
     {
-      for (final PatternDN pattern : notEqualTo)
+      if (notEqualTo.length > 0)
       {
-        if (pattern.matchesDN(dn))
+        for (final PatternDN pattern : notEqualTo)
         {
-          return false;
+          if (pattern.matchesDN(dn))
+          {
+            return false;
+          }
         }
       }
 
-      for (final PatternDN pattern : equalTo)
+      if (equalTo.length > 0)
       {
-        if (pattern.matchesDN(dn))
+        for (final PatternDN pattern : equalTo)
         {
-          return true;
+          if (pattern.matchesDN(dn))
+          {
+            return true;
+          }
         }
+        return false;
       }
 
-      // The DN did not match.
-      return false;
+      return true;
     }
 
 
@@ -497,26 +503,22 @@
       // Check response code.
       final int resultCode = operation.getResultCode().intValue();
 
-      if (!cfg.getResponseResultCodeNotEqualTo().isEmpty())
+      if (!cfg.getResponseResultCodeNotEqualTo().isEmpty()
+          && cfg.getResponseResultCodeNotEqualTo().contains(resultCode))
       {
-        if (cfg.getResponseResultCodeNotEqualTo().contains(resultCode))
-        {
-          return false;
-        }
+        return false;
       }
 
-      if (!cfg.getResponseResultCodeEqualTo().isEmpty())
+      if (!cfg.getResponseResultCodeEqualTo().isEmpty()
+          && !cfg.getResponseResultCodeEqualTo().contains(resultCode))
       {
-        if (!cfg.getResponseResultCodeNotEqualTo().contains(resultCode))
-        {
-          return false;
-        }
+        return false;
       }
 
       // Check etime.
       final long etime = operation.getProcessingTime();
 
-      final Integer etimeGT = cfg.getResponseEtimeLessThan();
+      final Integer etimeGT = cfg.getResponseEtimeGreaterThan();
       if (etimeGT != null)
       {
         if (etime <= ((long) etimeGT))
@@ -682,10 +684,10 @@
             logger.traceException(e);
           }
         }
+        return false;
       }
 
-      // The user entry did not match.
-      return false;
+      return true;
     }
 
   }
diff --git a/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisherTest.java b/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisherTest.java
index 5832def..9b727dd 100644
--- a/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisherTest.java
+++ b/opendj3-server-dev/tests/unit-tests-testng/src/server/org/opends/server/loggers/AbstractTextAccessLogPublisherTest.java
@@ -21,25 +21,109 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2013 ForgeRock AS
+ *      Copyright 2013-2014 ForgeRock AS
  */
 package org.opends.server.loggers;
 
-import static org.mockito.Mockito.*;
-import static org.testng.Assert.*;
 
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+import org.forgerock.opendj.ldap.AddressMask;
+import org.forgerock.opendj.ldap.ResultCode;
 import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.admin.std.meta.AccessLogFilteringCriteriaCfgDefn.LogRecordType;
+import org.opends.server.admin.std.server.AccessLogFilteringCriteriaCfg;
+import org.opends.server.api.ClientConnection;
+import org.opends.server.core.DirectoryServer;
+import org.opends.server.core.SearchOperation;
+import org.opends.server.loggers.AbstractTextAccessLogPublisher.CriteriaFilter;
 import org.opends.server.loggers.AbstractTextAccessLogPublisher.RootFilter;
+import org.opends.server.types.AuthenticationInfo;
+import org.opends.server.types.DN;
+import org.opends.server.types.DirectoryException;
 import org.opends.server.types.Operation;
+import org.opends.server.types.OperationType;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+
+
 @SuppressWarnings("javadoc")
 public class AbstractTextAccessLogPublisherTest extends DirectoryServerTestCase
 {
 
+  @AfterClass
+  public void afterClass() throws Exception
+  {
+    // Make sure group is removed from group manager.
+    TestCaseUtils.deleteEntry(dn("cn=group 1,ou=Groups,o=test"));
+    TestCaseUtils.clearDataBackends();
+  }
+
+
+
+  @BeforeClass
+  public void beforeClass() throws Exception
+  {
+    TestCaseUtils.startServer();
+    TestCaseUtils.clearDataBackends();
+    TestCaseUtils.initializeTestBackend(true);
+    TestCaseUtils.addEntries(
+        // @formatter:off
+        "dn: ou=People,o=test",
+        "objectClass: top",
+        "objectClass: organizationalUnit",
+        "ou: People",
+        "",
+        "dn: ou=Groups,o=test",
+        "objectClass: top",
+        "objectClass: organizationalUnit",
+        "ou: Groups",
+        "",
+        "dn: cn=group 1,ou=Groups,o=test",
+        "objectClass: top",
+        "objectClass: groupOfNames",
+        "cn: group 1",
+        "member: uid=user.1,ou=People,o=test",
+        "",
+        "dn: uid=user.1,ou=People,o=test",
+        "objectClass: top",
+        "objectClass: person",
+        "objectClass: organizationalPerson",
+        "objectClass: inetOrgPerson",
+        "uid: user.1",
+        "givenName: User",
+        "sn: 1",
+        "cn: User 1",
+        "userPassword: password",
+        "",
+        "dn: uid=user.2,ou=People,o=test",
+        "objectClass: top",
+        "objectClass: person",
+        "objectClass: organizationalPerson",
+        "objectClass: inetOrgPerson",
+        "uid: user.2",
+        "givenName: User",
+        "sn: 2",
+        "cn: User 2",
+        "userPassword: password");
+        // @formatter:on
+  }
+
+
+
   @DataProvider(name = "isLoggableData")
-  public Object[][] getIsLoggableData()
+  private Object[][] getIsLoggableData()
   {
     // When suppress is set to true and the corresponding operation is set to
     // true too, then the operation is not loggable.
@@ -49,6 +133,7 @@
     // There is just one exception: when the operation is a synchronization
     // operation and we do not suppress synchronization operation, then we
     // return true regardless of whether this is an internal operation
+    // @formatter:off
     return new Object[][] {
       { true, true, true, true, false },
       { true, true, true, false, false },
@@ -66,21 +151,279 @@
       { false, false, true, false, true },
       { false, false, false, true, true },
       { false, false, false, false, true }, };
+    // @formatter:on
   }
 
+
+
   @Test(dataProvider = "isLoggableData")
-  public void rootFilterIsLoggable(boolean suppressSynchronization,
-      boolean isSynchronizationOp, boolean suppressInternal,
-      boolean isInternalOp, boolean expectedTestResult)
+  public void rootFilterIsLoggable(final boolean suppressSynchronization,
+      final boolean isSynchronizationOp, final boolean suppressInternal,
+      final boolean isInternalOp, final boolean expectedTestResult)
   {
     final Operation operation = mock(Operation.class);
     when(operation.isSynchronizationOperation())
         .thenReturn(isSynchronizationOp);
     when(operation.isInnerOperation()).thenReturn(isInternalOp);
 
-    final RootFilter filter =
-        new RootFilter(suppressInternal, suppressSynchronization, null, null);
-    assertEquals(filter.isLoggable(operation), expectedTestResult);
+    final RootFilter filter = new RootFilter(suppressInternal,
+        suppressSynchronization, null, null);
+    assertThat(filter.isLoggable(operation)).isEqualTo(expectedTestResult);
   }
 
+
+
+  @Test
+  public void testCriteriaFilterDefault() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final Operation operation = mockAnonymousSearchOperation();
+    assertThat(filter.isRequestLoggable(operation)).isTrue();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterRequestTargetDNEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getRequestTargetDNEqualTo()).thenReturn(setOf("dc=com"));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getBaseDN()).thenReturn(dn("dc=com"), dn("dc=org"));
+    assertThat(filter.isRequestLoggable(operation)).isTrue();
+    assertThat(filter.isRequestLoggable(operation)).isFalse();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterRequestTargetDNNotEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getRequestTargetDNNotEqualTo()).thenReturn(setOf("dc=com"));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getBaseDN()).thenReturn(dn("dc=com"), dn("dc=org"));
+    assertThat(filter.isRequestLoggable(operation)).isFalse();
+    assertThat(filter.isRequestLoggable(operation)).isTrue();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterResponseEtimeGreaterThan() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getResponseEtimeGreaterThan()).thenReturn(100);
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getProcessingTime()).thenReturn(50L, 150L);
+    assertThat(filter.isResponseLoggable(operation)).isFalse();
+    assertThat(filter.isResponseLoggable(operation)).isTrue();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterResponseEtimeLessThan() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getResponseEtimeLessThan()).thenReturn(100);
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getProcessingTime()).thenReturn(50L, 150L);
+    assertThat(filter.isResponseLoggable(operation)).isTrue();
+    assertThat(filter.isResponseLoggable(operation)).isFalse();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterResponseResultCodeEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getResponseResultCodeEqualTo()).thenReturn(setOf(32));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getResultCode()).thenReturn(ResultCode.NO_SUCH_OBJECT,
+        ResultCode.SUCCESS);
+    assertThat(filter.isResponseLoggable(operation)).isTrue();
+    assertThat(filter.isResponseLoggable(operation)).isFalse();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterResponseResultCodeNotEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getResponseResultCodeNotEqualTo()).thenReturn(setOf(32));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation = mockAnonymousSearchOperation();
+    when(operation.getResultCode()).thenReturn(ResultCode.NO_SUCH_OBJECT,
+        ResultCode.SUCCESS);
+    assertThat(filter.isResponseLoggable(operation)).isFalse();
+    assertThat(filter.isResponseLoggable(operation)).isTrue();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterUserDNEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getUserDNEqualTo())
+        .thenReturn(setOf(dnOfUserInGroup().toString()));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation1 = mockAuthenticatedSearchOperation(dnOfUserInGroup());
+    assertThat(filter.isRequestLoggable(operation1)).isTrue();
+    final SearchOperation operation2 = mockAuthenticatedSearchOperation(dnOfUserNotInGroup());
+    assertThat(filter.isRequestLoggable(operation2)).isFalse();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterUserDNNotEqualTo() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getUserDNNotEqualTo()).thenReturn(
+        setOf(dnOfUserInGroup().toString()));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation1 = mockAuthenticatedSearchOperation(dnOfUserInGroup());
+    assertThat(filter.isRequestLoggable(operation1)).isFalse();
+    final SearchOperation operation2 = mockAuthenticatedSearchOperation(dnOfUserNotInGroup());
+    assertThat(filter.isRequestLoggable(operation2)).isTrue();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterUserIsMemberOf() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getUserIsMemberOf()).thenReturn(setOf(dnOfGroup()));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation1 = mockAuthenticatedSearchOperation(dnOfUserInGroup());
+    assertThat(filter.isRequestLoggable(operation1)).isTrue();
+    final SearchOperation operation2 = mockAuthenticatedSearchOperation(dnOfUserNotInGroup());
+    assertThat(filter.isRequestLoggable(operation2)).isFalse();
+  }
+
+
+
+  @Test
+  public void testCriteriaFilterUserIsNotMemberOf() throws Exception
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mockCriteriaFilterCfg();
+    when(cfg.getUserIsNotMemberOf()).thenReturn(setOf(dnOfGroup()));
+    final CriteriaFilter filter = new CriteriaFilter(cfg);
+    final SearchOperation operation1 = mockAuthenticatedSearchOperation(dnOfUserInGroup());
+    assertThat(filter.isRequestLoggable(operation1)).isFalse();
+    final SearchOperation operation2 = mockAuthenticatedSearchOperation(dnOfUserNotInGroup());
+    assertThat(filter.isRequestLoggable(operation2)).isTrue();
+  }
+
+
+
+  private DN dn(final String dn)
+  {
+    try
+    {
+      return DN.valueOf(dn);
+    }
+    catch (final DirectoryException e)
+    {
+      throw new RuntimeException(e);
+    }
+  }
+
+
+
+  private DN dnOfGroup()
+  {
+    return dn("cn=group 1,ou=Groups,o=test");
+  }
+
+
+
+  private DN dnOfUserInGroup()
+  {
+    return dn("uid=user.1,ou=People,o=test");
+  }
+
+
+
+  private DN dnOfUserNotInGroup()
+  {
+    return dn("uid=user.2,ou=People,o=test");
+  }
+
+
+
+  private SearchOperation mockAnonymousSearchOperation() throws Exception
+  {
+    return mockSearchOperation(new AuthenticationInfo());
+  }
+
+
+
+  private SearchOperation mockAuthenticatedSearchOperation(final DN user)
+      throws Exception
+  {
+    return mockSearchOperation(new AuthenticationInfo(
+        DirectoryServer.getEntry(user), false));
+  }
+
+
+
+  private AccessLogFilteringCriteriaCfg mockCriteriaFilterCfg()
+  {
+    final AccessLogFilteringCriteriaCfg cfg = mock(AccessLogFilteringCriteriaCfg.class);
+    when(cfg.getConnectionClientAddressEqualTo()).thenReturn(
+        new TreeSet<AddressMask>());
+    when(cfg.getConnectionClientAddressNotEqualTo()).thenReturn(
+        new TreeSet<AddressMask>());
+    when(cfg.getConnectionPortEqualTo()).thenReturn(new TreeSet<Integer>());
+    when(cfg.getConnectionProtocolEqualTo()).thenReturn(new TreeSet<String>());
+    when(cfg.getLogRecordType()).thenReturn(new TreeSet<LogRecordType>());
+    when(cfg.getRequestTargetDNEqualTo()).thenReturn(new TreeSet<String>());
+    when(cfg.getRequestTargetDNNotEqualTo()).thenReturn(new TreeSet<String>());
+    when(cfg.getResponseEtimeGreaterThan()).thenReturn(null);
+    when(cfg.getResponseEtimeLessThan()).thenReturn(null);
+    when(cfg.getResponseResultCodeEqualTo()).thenReturn(new TreeSet<Integer>());
+    when(cfg.getResponseResultCodeNotEqualTo()).thenReturn(
+        new TreeSet<Integer>());
+    when(cfg.isSearchResponseIsIndexed()).thenReturn(null);
+    when(cfg.getSearchResponseNentriesGreaterThan()).thenReturn(null);
+    when(cfg.getSearchResponseNentriesLessThan()).thenReturn(null);
+    when(cfg.getUserDNEqualTo()).thenReturn(new TreeSet<String>());
+    when(cfg.getUserDNNotEqualTo()).thenReturn(new TreeSet<String>());
+    when(cfg.getUserIsMemberOf()).thenReturn(new TreeSet<DN>());
+    when(cfg.getUserIsNotMemberOf()).thenReturn(new TreeSet<DN>());
+    return cfg;
+  }
+
+
+
+  private SearchOperation mockSearchOperation(final AuthenticationInfo authInfo)
+      throws Exception
+  {
+    final SearchOperation operation = mock(SearchOperation.class);
+    final ClientConnection connection = mock(ClientConnection.class);
+    when(operation.getOperationType()).thenReturn(OperationType.SEARCH);
+    when(operation.getClientConnection()).thenReturn(connection);
+    when(operation.getResultCode()).thenReturn(ResultCode.SUCCESS);
+    when(connection.getAuthenticationInfo()).thenReturn(authInfo);
+    return operation;
+  }
+
+
+
+  private <T> SortedSet<T> setOf(final T... values)
+  {
+    return new TreeSet<T>(Arrays.asList(values));
+  }
 }

--
Gitblit v1.10.0