From fe51cdabe75917e82195fcad47563fc169026625 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Wed, 3 Jun 2020 15:58:21 -0400
Subject: [PATCH] Issue 51132 - Winsync setting winSyncWindowsFilter not
 working as expected

Bug Description:  When processing updates from AD we search AD using a filter,
                  and this filter can be customized via the attribute setting:
                  winSyncWindowsFilter.  However, after setting a custom filter
                  replication appears to stop working as expected.  New entries
                  that match the filter are replicated to DS, but not updates
                  to these entries.  The problem is that when dirsync sends
                  updates, it is just a partial entry - only containing the
                  attributes that changed.  Then the server checks the filter
                  again on the returned entry, but if it's just a mod update then
                  the entry is missing most of its attributes, and the filter
                  check fails and the entry is not updated on DS.

Fix Description:  Do not check the filter on the returned entries when processing
                  incremental updates as the fitler test was already done when
                  gathering the candidates.

relates: https://pagure.io/389-ds-base/issue/51132

Reviewed by: tbordaz & firstyear (Thanks!)
---
 .../plugins/replication/windows_protocol_util.c   | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index e35437221..c394f82ce 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -48,7 +48,7 @@ static int windows_get_remote_entry(Private_Repl_Protocol *prp, const Slapi_DN *
 static int windows_get_remote_tombstone(Private_Repl_Protocol *prp, const Slapi_DN *remote_dn, Slapi_Entry **remote_entry);
 static int windows_reanimate_tombstone(Private_Repl_Protocol *prp, const Slapi_DN *tombstone_dn, const char *new_dn);
 static const char *op2string(int op);
-static int is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra);
+static int is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra, int test_filter);
 static int map_entry_dn_inbound(Slapi_Entry *e, Slapi_DN **dn, const Repl_Agmt *ra);
 static int map_entry_dn_inbound_ext(Slapi_Entry *e, Slapi_DN **dn, const Repl_Agmt *ra, int use_guid, int user_username);
 static int windows_update_remote_entry(Private_Repl_Protocol *prp, Slapi_Entry *remote_entry, Slapi_Entry *local_entry, int is_user);
@@ -57,6 +57,9 @@ static int map_windows_tombstone_dn(Slapi_Entry *e, Slapi_DN **dn, Private_Repl_
 static int windows_check_mods_for_rdn_change(Private_Repl_Protocol *prp, LDAPMod **original_mods, Slapi_Entry *local_entry, Slapi_DN *remote_dn, char **newrdn);
 static int windows_get_superior_change(Private_Repl_Protocol *prp, Slapi_DN *local_dn, Slapi_DN *remote_dn, char **newsuperior, int to_windows);
 
+#define SKIP_FILTER 0
+#define TEST_FILTER 1
+
 /* Controls the direction of flow for mapped attributes */
 typedef enum mapping_types {
     bidirectional,
@@ -442,7 +445,7 @@ map_dn_values(Private_Repl_Protocol *prp, Slapi_ValueSet *original_values, Slapi
             /* Try to get the remote entry */
             retval = windows_get_remote_entry(prp, original_dn, &remote_entry);
             if (remote_entry && 0 == retval) {
-                is_ours = is_subject_of_agreement_remote(remote_entry, prp->agmt);
+                is_ours = is_subject_of_agreement_remote(remote_entry, prp->agmt, TEST_FILTER);
                 if (is_ours) {
                     retval = map_entry_dn_inbound(remote_entry, &local_dn, prp->agmt);
                     if (0 == retval && local_dn) {
@@ -3708,7 +3711,7 @@ map_entry_dn_outbound(Slapi_Entry *e,
                       slapi_sdn_get_dn(new_dn),
                       remote_entry ? slapi_entry_get_dn_const(remote_entry) : "(null)");
         if (0 == rc && remote_entry) {
-            if (!is_subject_of_agreement_remote(remote_entry, prp->agmt)) {
+            if (!is_subject_of_agreement_remote(remote_entry, prp->agmt, TEST_FILTER)) {
                 /* The remote entry is out of scope of the agreement.
                  * Thus, we don't map the entry_dn.
                  * This occurs when the remote entry is moved out. */
@@ -4198,7 +4201,7 @@ is_dn_subject_of_agreement_local(const Slapi_DN *sdn, const Repl_Agmt *ra)
  *               0 -- out of scope
  */
 static int
-is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra)
+is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra, int test_filter)
 {
     int retval = 0;
     int is_in_subtree = 0;
@@ -4232,7 +4235,7 @@ is_subject_of_agreement_remote(Slapi_Entry *e, const Repl_Agmt *ra)
         Slapi_DN psdn = {0};
         Slapi_Entry *pentry = NULL;
 
-        if (windows_private_get_windows_filter(ra) &&
+        if (test_filter && windows_private_get_windows_filter(ra) &&
             slapi_filter_test_simple(e, windows_private_get_windows_filter(ra))) {
             /* type_winSyncWindowsFilter is set and the remote entry does not match the filter */
             goto error;
@@ -5627,7 +5630,7 @@ windows_process_dirsync_entry(Private_Repl_Protocol *prp, Slapi_Entry *e, int is
         }
     } else {
         /* Is this entry one we should be interested in ? */
-        if (is_subject_of_agreement_remote(e, prp->agmt)) {
+        if (is_subject_of_agreement_remote(e, prp->agmt, SKIP_FILTER)) {
             ConnResult cres = 0;
             const char *searchbase = slapi_entry_get_dn_const(e);
             char *filter = "(objectclass=*)";
-- 
2.26.2