]> git.sur5r.net Git - openldap/commitdiff
ITS#6536
authorQuanah Gibson-Mount <quanah@openldap.org>
Fri, 17 Dec 2010 18:30:10 +0000 (18:30 +0000)
committerQuanah Gibson-Mount <quanah@openldap.org>
Fri, 17 Dec 2010 18:30:10 +0000 (18:30 +0000)
CHANGES
contrib/slapd-modules/autogroup/README
contrib/slapd-modules/autogroup/autogroup.c

diff --git a/CHANGES b/CHANGES
index c8777654917342392b2f0ca7672469882e34295e..c1d98f5b6d55840d991ae3885dc33b73a988e2ff 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -29,6 +29,7 @@ OpenLDAP 2.4.24 Engineering
        Fixed slapo-syncprov to send error if consumer is newer (ITS#6606)
        Fixed slapo-syncprov filter race condition (ITS#6708)
        Fixed slapo-syncprov active mod race (ITS#6709)
+       Fixed contrib/autogroup LDAP URI with attribute filter (ITS#6536)
        Fixed contrib/nssov to only close socket on shutdown (ITS#6676)
        Fixed contrib/nssov multi platform support (ITS#6604)
        Documentation
index f717b7661a2afdaf069e330eae65a836af471bda..5898218eb8c91f68f3c9a5ff215c054678f90938 100644 (file)
@@ -3,10 +3,12 @@ autogroup overlay Readme
 DESCRIPTION
     The autogroup overlay allows automated updates of group memberships which
     meet the requirements of any filter contained in the group definition.
-       The filters are built from LDAP URI-valued attributes. Any time an object
-       is added/deleted/updated, it is tested for compliance with the filters,
+    The filters are built from LDAP URI-valued attributes. Any time an object
+    is added/deleted/updated, it is tested for compliance with the filters,
     and its membership is accordingly updated. For searches and compares
     it behaves like a static group.
+    If the attribute part of the URI is filled, the group entry is populated
+    by the values of this attribute in the entries resulting from the search.
 
 BUILDING
     A Makefile is included.
@@ -67,6 +69,11 @@ EXAMPLE
 CAVEATS
     As with static groups, update operations on groups with a large number
     of members may be slow.
+    If the attribute part of the URI is specified, modify and delete operations
+    are more difficult to handle. In these cases the overlay will try to detect
+    if groups have been modified and then simply refresh them. This can cause
+    performance hits if the search specified by the URI deals with a significant
+    number of entries.
 
 ACKNOWLEDGEMENTS
     This module was originally written in 2007 by Michał Szulczyński.
index 6b38823f486d8601c141df38c72cbfccb5513dc6..05f5f3a02e0fae343155a4184f4187076362deed 100644 (file)
@@ -38,6 +38,7 @@ typedef struct autogroup_filter_t {
        struct berval                   agf_filterstr;
        Filter                          *agf_filter;
        int                             agf_scope;
+       AttributeName                   *agf_anlist;
        struct autogroup_filter_t       *agf_next;
 } autogroup_filter_t;
 
@@ -56,6 +57,8 @@ typedef struct autogroup_entry_t {
        autogroup_filter_t      *age_filter; /* List of filters made from memberURLs */
        autogroup_def_t         *age_def; /* Attribute definition */
        ldap_pvt_thread_mutex_t age_mutex;
+       int                     age_mustrefresh; /* Defined in request to refresh in response */
+       int                     age_modrdn_olddnmodified; /* Defined in request to refresh in response */
        struct autogroup_entry_t        *age_next;
 } autogroup_entry_t;
 
@@ -75,6 +78,7 @@ typedef struct autogroup_sc_t {
 /* Used for adding members, found when searching, to a group. */
 typedef struct autogroup_ga_t {
        autogroup_entry_t       *agg_group;     /* The group to which the members will be added. */
+       autogroup_filter_t      *agg_filter;    /* Current filter */
        Entry                   *agg_entry;     /* Used in autogroup_member_search_cb to modify 
                                                this entry with the search results. */
 
@@ -94,9 +98,9 @@ static int
 autogroup_add_member_to_group( Operation *op, BerValue *dn, BerValue *ndn, autogroup_entry_t *age )
 {
        slap_overinst   *on = (slap_overinst *)op->o_bd->bd_info;
-       Modifications   modlist;
+       Modifications   *modlist = (Modifications *)ch_calloc( 1, sizeof( Modifications ) );
        SlapReply       sreply = {REP_RESULT};
-       BerValue        vals[ 2 ], nvals[ 2 ];
+       BerValue        *vals, *nvals;
        slap_callback   cb = { NULL, slap_null_cb, NULL, NULL };
        Operation       o = *op;
 
@@ -106,17 +110,71 @@ autogroup_add_member_to_group( Operation *op, BerValue *dn, BerValue *ndn, autog
        assert( dn != NULL );
        assert( ndn != NULL );
 
-       vals[ 0 ] = *dn;
+       vals = (BerValue *)ch_calloc( 2, sizeof( BerValue ) );
+       nvals = (BerValue *)ch_calloc( 2, sizeof( BerValue ) );
+       ber_dupbv( vals, dn );
        BER_BVZERO( &vals[ 1 ] );
-       nvals[ 0 ] = *ndn;
+       ber_dupbv( nvals, ndn );
        BER_BVZERO( &nvals[ 1 ] );
 
+       modlist->sml_op = LDAP_MOD_ADD;
+       modlist->sml_desc = age->age_def->agd_member_ad;
+       modlist->sml_type = age->age_def->agd_member_ad->ad_cname;
+       modlist->sml_values = vals;
+       modlist->sml_nvalues = nvals;
+       modlist->sml_numvals = 1;
+       modlist->sml_flags = SLAP_MOD_INTERNAL;
+       modlist->sml_next = NULL;
+
+       o.o_tag = LDAP_REQ_MODIFY;
+       o.o_callback = &cb;
+       o.orm_modlist = modlist;
+       o.o_req_dn = age->age_dn;
+       o.o_req_ndn = age->age_ndn;
+       o.o_permissive_modify = 1;
+       o.o_managedsait = SLAP_CONTROL_CRITICAL;
+       o.o_relax = SLAP_CONTROL_CRITICAL;
+
+       o.o_bd->bd_info = (BackendInfo *)on->on_info;
+       (void)op->o_bd->be_modify( &o, &sreply );
+       o.o_bd->bd_info = (BackendInfo *)on;
+
+       slap_mods_free( modlist, 1 );
+
+       return sreply.sr_err;
+}
+
+/* 
+**     e       - the entry where to get the attribute values
+**     age     - the group to which the values will be added
+*/
+static int
+autogroup_add_member_values_to_group( Operation *op, Entry *e, autogroup_entry_t *age, AttributeDescription *attrdesc )
+{
+       slap_overinst   *on = (slap_overinst *)op->o_bd->bd_info;
+       Modifications   modlist;
+       SlapReply       sreply = {REP_RESULT};
+       Attribute       *attr;
+       slap_callback   cb = { NULL, slap_null_cb, NULL, NULL };
+       Operation       o = *op;
+
+       Debug(LDAP_DEBUG_TRACE, "==> autogroup_add_member_values_to_group adding <%s> to <%s>\n",
+               e->e_name.bv_val, age->age_dn.bv_val, 0);
+
+       assert( e != NULL );
+
+       attr = attrs_find( e->e_attrs, attrdesc );
+       if (!attr) {
+               // Nothing to add
+               return LDAP_SUCCESS;
+       }
+
        modlist.sml_op = LDAP_MOD_ADD;
        modlist.sml_desc = age->age_def->agd_member_ad;
        modlist.sml_type = age->age_def->agd_member_ad->ad_cname;
-       modlist.sml_values = vals;
-       modlist.sml_nvalues = nvals;
-       modlist.sml_numvals = 1;
+       modlist.sml_values = attr->a_vals;
+       modlist.sml_nvalues = attr->a_nvals;
+       modlist.sml_numvals = attr->a_numvals;
        modlist.sml_flags = SLAP_MOD_INTERNAL;
        modlist.sml_next = NULL;
 
@@ -145,9 +203,9 @@ static int
 autogroup_delete_member_from_group( Operation *op, BerValue *dn, BerValue *ndn, autogroup_entry_t *age )
 {
        slap_overinst   *on = (slap_overinst *)op->o_bd->bd_info;
-       Modifications   modlist;
+       Modifications   *modlist = (Modifications *)ch_calloc( 1, sizeof( Modifications ) );
        SlapReply       sreply = {REP_RESULT};
-       BerValue        vals[ 2 ], nvals[ 2 ];
+       BerValue        *vals, *nvals;
        slap_callback   cb = { NULL, slap_null_cb, NULL, NULL };
        Operation       o = *op;
 
@@ -155,33 +213,35 @@ autogroup_delete_member_from_group( Operation *op, BerValue *dn, BerValue *ndn,
                Debug(LDAP_DEBUG_TRACE, "==> autogroup_delete_member_from_group removing all members from <%s>\n",
                        age->age_dn.bv_val, 0 ,0);
 
-               modlist.sml_values = NULL;
-               modlist.sml_nvalues = NULL;
-               modlist.sml_numvals = 0;
+               modlist->sml_values = NULL;
+               modlist->sml_nvalues = NULL;
+               modlist->sml_numvals = 0;
        } else {
                Debug(LDAP_DEBUG_TRACE, "==> autogroup_delete_member_from_group removing <%s> from <%s>\n",
                        dn->bv_val, age->age_dn.bv_val, 0);
 
-               vals[ 0 ] = *dn;
+               vals = (BerValue *)ch_calloc( 2, sizeof( BerValue ) );
+               nvals = (BerValue *)ch_calloc( 2, sizeof( BerValue ) );
+               ber_dupbv( vals, dn );
                BER_BVZERO( &vals[ 1 ] );
-               nvals[ 0 ] = *ndn;
+               ber_dupbv( nvals, ndn );
                BER_BVZERO( &nvals[ 1 ] );
 
-               modlist.sml_values = vals;
-               modlist.sml_nvalues = nvals;
-               modlist.sml_numvals = 1;
+               modlist->sml_values = vals;
+               modlist->sml_nvalues = nvals;
+               modlist->sml_numvals = 1;
        }
 
 
-       modlist.sml_op = LDAP_MOD_DELETE;
-       modlist.sml_desc = age->age_def->agd_member_ad;
-       modlist.sml_type = age->age_def->agd_member_ad->ad_cname;
-       modlist.sml_flags = SLAP_MOD_INTERNAL;
-       modlist.sml_next = NULL;
+       modlist->sml_op = LDAP_MOD_DELETE;
+       modlist->sml_desc = age->age_def->agd_member_ad;
+       modlist->sml_type = age->age_def->agd_member_ad->ad_cname;
+       modlist->sml_flags = SLAP_MOD_INTERNAL;
+       modlist->sml_next = NULL;
 
        o.o_callback = &cb;
        o.o_tag = LDAP_REQ_MODIFY;
-       o.orm_modlist = &modlist;
+       o.orm_modlist = modlist;
        o.o_req_dn = age->age_dn;
        o.o_req_ndn = age->age_ndn;
        o.o_relax = SLAP_CONTROL_CRITICAL;
@@ -192,6 +252,8 @@ autogroup_delete_member_from_group( Operation *op, BerValue *dn, BerValue *ndn,
        (void)op->o_bd->be_modify( &o, &sreply );
        o.o_bd->bd_info = (BackendInfo *)on;
 
+       slap_mods_free( modlist, 1 );
+
        return sreply.sr_err;
 }
 
@@ -211,25 +273,43 @@ autogroup_member_search_cb( Operation *op, SlapReply *rs )
        if ( rs->sr_type == REP_SEARCH ) {
                autogroup_ga_t          *agg = (autogroup_ga_t *)op->o_callback->sc_private;
                autogroup_entry_t       *age = agg->agg_group;
+               autogroup_filter_t      *agf = agg->agg_filter;
                Modification            mod;
                const char              *text = NULL;
                char                    textbuf[1024];
-               struct berval           vals[ 2 ], nvals[ 2 ];
+               struct berval           *vals, *nvals;
+               int                     numvals;
 
                Debug(LDAP_DEBUG_TRACE, "==> autogroup_member_search_cb <%s>\n",
                        rs->sr_entry ? rs->sr_entry->e_name.bv_val : "UNKNOWN_DN", 0, 0);
 
-               vals[ 0 ] = rs->sr_entry->e_name;
-               BER_BVZERO( &vals[ 1 ] );
-               nvals[ 0 ] = rs->sr_entry->e_nname;
-               BER_BVZERO( &nvals[ 1 ] );
+               if ( agf->agf_anlist ) {
+                       Attribute *attr = attrs_find( rs->sr_entry->e_attrs, agf->agf_anlist[0].an_desc );
+                       if (attr) {
+                               vals = attr->a_vals;
+                               nvals = attr->a_nvals;
+                               numvals = attr->a_numvals;
+                       } else {
+                               // Nothing to add
+                               return 0;
+                       }
+               } else {
+                       struct berval           lvals[ 2 ], lnvals[ 2 ];
+                       lvals[ 0 ] = rs->sr_entry->e_name;
+                       BER_BVZERO( &lvals[ 1 ] );
+                       lnvals[ 0 ] = rs->sr_entry->e_nname;
+                       BER_BVZERO( &lnvals[ 1 ] );
+                       vals = lvals;
+                       nvals = lnvals;
+                       numvals = 1;
+               }
 
                mod.sm_op = LDAP_MOD_ADD;
                mod.sm_desc = age->age_def->agd_member_ad;
                mod.sm_type = age->age_def->agd_member_ad->ad_cname;
                mod.sm_values = vals;
                mod.sm_nvalues = nvals;
-               mod.sm_numvals = 1;
+               mod.sm_numvals = numvals;
 
                modify_add_values( agg->agg_entry, &mod, /* permissive */ 1, &text, textbuf, sizeof( textbuf ) );
        }
@@ -251,36 +331,56 @@ autogroup_member_search_modify_cb( Operation *op, SlapReply *rs )
        if ( rs->sr_type == REP_SEARCH ) {
                autogroup_ga_t          *agg = (autogroup_ga_t *)op->o_callback->sc_private;
                autogroup_entry_t       *age = agg->agg_group;
+               autogroup_filter_t      *agf = agg->agg_filter;
                Modifications           *modlist;
-               struct berval           vals[ 2 ], nvals[ 2 ];
+               struct berval           *vals, *nvals;
+               int                     numvals;
 
                Debug(LDAP_DEBUG_TRACE, "==> autogroup_member_search_modify_cb <%s>\n",
                        rs->sr_entry ? rs->sr_entry->e_name.bv_val : "UNKNOWN_DN", 0, 0);
 
-               vals[ 0 ] = rs->sr_entry->e_name;
-               BER_BVZERO( &vals[ 1 ] );
-               nvals[ 0 ] = rs->sr_entry->e_nname;
-               BER_BVZERO( &nvals[ 1 ] );
+               if ( agf->agf_anlist ) {
+                       Attribute *attr = attrs_find( rs->sr_entry->e_attrs, agf->agf_anlist[0].an_desc );
+                       if (attr) {
+                               vals = attr->a_vals;
+                               nvals = attr->a_nvals;
+                               numvals = attr->a_numvals;
+                       } else {
+                               // Nothing to add
+                               return 0;
+                       }
+               } else {
+                       struct berval           lvals[ 2 ], lnvals[ 2 ];
+                       lvals[ 0 ] = rs->sr_entry->e_name;
+                       BER_BVZERO( &lvals[ 1 ] );
+                       lnvals[ 0 ] = rs->sr_entry->e_nname;
+                       BER_BVZERO( &lnvals[ 1 ] );
+                       vals = lvals;
+                       nvals = lnvals;
+                       numvals = 1;
+               }
 
-               modlist = (Modifications *)ch_calloc( 1, sizeof( Modifications ) );
+               if ( numvals ) {
+                       modlist = (Modifications *)ch_calloc( 1, sizeof( Modifications ) );
 
-               modlist->sml_op = LDAP_MOD_ADD;
-               modlist->sml_desc = age->age_def->agd_member_ad;
-               modlist->sml_type = age->age_def->agd_member_ad->ad_cname;
+                       modlist->sml_op = LDAP_MOD_ADD;
+                       modlist->sml_desc = age->age_def->agd_member_ad;
+                       modlist->sml_type = age->age_def->agd_member_ad->ad_cname;
 
-               ber_bvarray_dup_x( &modlist->sml_values, vals, NULL );
-               ber_bvarray_dup_x( &modlist->sml_nvalues, nvals, NULL );
-               modlist->sml_numvals = 1;
+                       ber_bvarray_dup_x( &modlist->sml_values, vals, NULL );
+                       ber_bvarray_dup_x( &modlist->sml_nvalues, nvals, NULL );
+                       modlist->sml_numvals = numvals;
 
-               modlist->sml_flags = SLAP_MOD_INTERNAL;
-               modlist->sml_next = NULL;
+                       modlist->sml_flags = SLAP_MOD_INTERNAL;
+                       modlist->sml_next = NULL;
 
-               if ( agg->agg_mod == NULL ) {
-                       agg->agg_mod = modlist;
-                       agg->agg_mod_last = modlist;
-               } else {
-                       agg->agg_mod_last->sml_next = modlist;
-                       agg->agg_mod_last = modlist;
+                       if ( agg->agg_mod == NULL ) {
+                               agg->agg_mod = modlist;
+                               agg->agg_mod_last = modlist;
+                       } else {
+                               agg->agg_mod_last->sml_next = modlist;
+                               agg->agg_mod_last = modlist;
+                       }
                }
 
        }
@@ -325,9 +425,10 @@ autogroup_add_members_from_filter( Operation *op, Entry *e, autogroup_entry_t *a
        o.ors_limit = NULL;
        o.ors_tlimit = SLAP_NO_LIMIT;
        o.ors_slimit = SLAP_NO_LIMIT;
-       o.ors_attrs =  slap_anlist_no_attrs;
+       o.ors_attrs =  agf->agf_anlist ? agf->agf_anlist : slap_anlist_no_attrs;
 
        agg.agg_group = age;
+       agg.agg_filter = agf;
        agg.agg_mod = NULL;
        agg.agg_mod_last = NULL;
        agg.agg_entry = e;
@@ -348,7 +449,7 @@ autogroup_add_members_from_filter( Operation *op, Entry *e, autogroup_entry_t *a
        op->o_bd->be_search( &o, &rs );
        o.o_bd->bd_info = (BackendInfo *)on;    
 
-       if ( modify == 1 ) {
+       if ( modify == 1 && agg.agg_mod ) {
                o = *op;
                o.o_callback = &null_cb;
                o.o_tag = LDAP_REQ_MODIFY;
@@ -419,6 +520,8 @@ autogroup_add_group( Operation *op, autogroup_info_t *agi, autogroup_def_t *agd,
        ldap_pvt_thread_mutex_init( &(*agep)->age_mutex );
        (*agep)->age_def = agd;
        (*agep)->age_filter = NULL;
+       (*agep)->age_mustrefresh = 0;
+       (*agep)->age_modrdn_olddnmodified = 0;
 
        ber_dupbv( &(*agep)->age_dn, &e->e_name );
        ber_dupbv( &(*agep)->age_ndn, &e->e_nname );
@@ -464,6 +567,33 @@ autogroup_add_group( Operation *op, autogroup_info_t *agi, autogroup_def_t *agd,
                                agf->agf_filter = str2filter( lud->lud_filter );
                        }                       
 
+                       if ( lud->lud_attrs != NULL ) {
+                               int i;
+
+                               for ( i=0 ; lud->lud_attrs[i]!=NULL ; i++) {
+                                       /* Just counting */;
+                               }
+
+                               if ( i > 1 ) {
+                                       Debug( LDAP_DEBUG_ANY, "autogroup_add_group: to much attributes specified in url <%s>\n",
+                                               bv->bv_val, 0, 0);
+                                       /* FIXME: error? */
+                                       ldap_free_urldesc( lud );
+                                       ch_free( agf ); 
+                               }
+                                       
+                               agf->agf_anlist = str2anlist( NULL, lud->lud_attrs[0], "," );
+
+                               if ( agf->agf_anlist == NULL ) {
+                                       Debug( LDAP_DEBUG_ANY, "autogroup_add_group: unable to find AttributeDescription \"%s\".\n",
+                                               lud->lud_attrs[0], 0, 0 );              
+                                       /* FIXME: error? */
+                                       ldap_free_urldesc( lud );
+                                       ch_free( agf ); 
+                                       continue;
+                               }
+                       }
+
                        agf->agf_next = NULL;
 
 
@@ -575,7 +705,11 @@ autogroup_add_entry( Operation *op, SlapReply *rs)
                        if ( dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
                                rc = test_filter( op, op->ora_e, agf->agf_filter );
                                if ( rc == LDAP_COMPARE_TRUE ) {
-                               autogroup_add_member_to_group( op, &op->ora_e->e_name, &op->ora_e->e_nname, age );
+                                       if ( agf->agf_anlist ) {
+                                               autogroup_add_member_values_to_group( op, op->ora_e, age, agf->agf_anlist[0].an_desc );
+                                       } else {
+                                               autogroup_add_member_to_group( op, &op->ora_e->e_name, &op->ora_e->e_nname, age );
+                                       }
                                        break;
                                }
                        }
@@ -626,6 +760,8 @@ autogroup_delete_group( autogroup_info_t *agi, autogroup_entry_t *e )
                                ch_free( agf->agf_filterstr.bv_val );
                                ch_free( agf->agf_dn.bv_val );
                                ch_free( agf->agf_ndn.bv_val );
+                               anlist_free( agf->agf_anlist, 1, NULL );
+                               ch_free( agf );
                        }
 
                        ldap_pvt_thread_mutex_unlock( &age->age_mutex );                
@@ -703,7 +839,16 @@ autogroup_delete_entry( Operation *op, SlapReply *rs)
                        if ( dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
                                rc = test_filter( op, e, agf->agf_filter );
                                if ( rc == LDAP_COMPARE_TRUE ) {
-                               autogroup_delete_member_from_group( op, &e->e_name, &e->e_nname, age );
+                                       /* If the attribute is retrieved from the entry, we don't know what to delete
+                                       ** So the group must be entirely refreshed
+                                       ** But the refresh can't be done now because the entry is not deleted
+                                       ** So the group is marked as mustrefresh
+                                       */
+                                       if ( agf->agf_anlist ) {
+                                               age->age_mustrefresh = 1;
+                                       } else {
+                                               autogroup_delete_member_from_group( op, &e->e_name, &e->e_nname, age );
+                                       }
                                        break;
                                }
                        }
@@ -728,8 +873,33 @@ autogroup_response( Operation *op, SlapReply *rs )
        BerValue                new_dn, new_ndn, pdn;
        Entry                   *e, *group;
        Attribute               *a;
-       int                     is_olddn, is_newdn, dn_equal;
+       int                     is_olddn, is_newdn, is_value_refresh, dn_equal;
+
+       /* Handle all cases where a refresh of the group is needed */
+       if ( op->o_tag == LDAP_REQ_DELETE || op->o_tag == LDAP_REQ_MODIFY ) {
+               if ( rs->sr_type == REP_RESULT && rs->sr_err == LDAP_SUCCESS && !get_manageDSAit( op ) ) {
+
+                       ldap_pvt_thread_mutex_lock( &agi->agi_mutex );
+
+                       for ( age = agi->agi_entry ; age ; age = age->age_next ) {
+                               /* Request detected that the group must be refreshed */
+
+                               ldap_pvt_thread_mutex_lock( &age->age_mutex );
+
+                               if ( age->age_mustrefresh ) {
+                                       autogroup_delete_member_from_group( op, NULL, NULL, age) ;
+
+                                       for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
+                                               autogroup_add_members_from_filter( op, NULL, age, agf, 1 );
+                                       }
+                               }
 
+                               ldap_pvt_thread_mutex_unlock( &age->age_mutex );
+                       }
+
+                       ldap_pvt_thread_mutex_unlock( &agi->agi_mutex );
+               }
+       }
        if ( op->o_tag == LDAP_REQ_MODRDN ) {
                if ( rs->sr_type == REP_RESULT && rs->sr_err == LDAP_SUCCESS && !get_manageDSAit( op )) {
 
@@ -817,45 +987,68 @@ autogroup_response( Operation *op, SlapReply *rs )
                        for ( age = agi->agi_entry ; age ; age = age->age_next ) {
                                is_olddn = 0;
                                is_newdn = 0;
+                               is_value_refresh = 0;
 
 
                                ldap_pvt_thread_mutex_lock( &age->age_mutex );
 
-                               if ( overlay_entry_get_ov( op, &age->age_ndn, NULL, NULL, 0, &group, on ) !=
-                                       LDAP_SUCCESS || group == NULL ) {
-                                       Debug( LDAP_DEBUG_TRACE, "autogroup_response MODRDN cannot get group entry <%s>\n", age->age_dn.bv_val, 0, 0);
+                               if ( age->age_modrdn_olddnmodified ) {
+                                       /* Resquest already marked this group to be updated */
+                                       is_olddn = 1;
+                                       is_value_refresh = 1;
+                                       age->age_modrdn_olddnmodified = 0;
+                               } else {
 
-                                       op->o_tmpfree( new_dn.bv_val, op->o_tmpmemctx );
-                                       op->o_tmpfree( new_ndn.bv_val, op->o_tmpmemctx );
+                                       if ( overlay_entry_get_ov( op, &age->age_ndn, NULL, NULL, 0, &group, on ) !=
+                                               LDAP_SUCCESS || group == NULL ) {
+                                               Debug( LDAP_DEBUG_TRACE, "autogroup_response MODRDN cannot get group entry <%s>\n", age->age_dn.bv_val, 0, 0);
 
-                                       ldap_pvt_thread_mutex_unlock( &age->age_mutex );
-                                       ldap_pvt_thread_mutex_unlock( &agi->agi_mutex );
-                                       return SLAP_CB_CONTINUE;
-                               }
+                                               op->o_tmpfree( new_dn.bv_val, op->o_tmpmemctx );
+                                               op->o_tmpfree( new_ndn.bv_val, op->o_tmpmemctx );
 
-                               a = attrs_find( group->e_attrs, age->age_def->agd_member_ad );
+                                               ldap_pvt_thread_mutex_unlock( &age->age_mutex );
+                                               ldap_pvt_thread_mutex_unlock( &agi->agi_mutex );
+                                               return SLAP_CB_CONTINUE;
+                                       }
+
+                                       a = attrs_find( group->e_attrs, age->age_def->agd_member_ad );
+
+                                       if ( a != NULL ) {
+                                               if ( value_find_ex( age->age_def->agd_member_ad,
+                                                               SLAP_MR_ATTRIBUTE_VALUE_NORMALIZED_MATCH |
+                                                               SLAP_MR_ASSERTED_VALUE_NORMALIZED_MATCH,
+                                                               a->a_nvals, &op->o_req_ndn, op->o_tmpmemctx ) == 0 ) 
+                                               {
+                                                       is_olddn = 1;
+                                               }
 
-                               if ( a != NULL ) {
-                                       if ( value_find_ex( age->age_def->agd_member_ad,
-                                                       SLAP_MR_ATTRIBUTE_VALUE_NORMALIZED_MATCH |
-                                                       SLAP_MR_ASSERTED_VALUE_NORMALIZED_MATCH,
-                                                       a->a_nvals, &op->o_req_ndn, op->o_tmpmemctx ) == 0 ) 
-                                       {
-                                               is_olddn = 1;
                                        }
 
-                               }
+                                       overlay_entry_release_ov( op, group, 0, on );
 
-                               overlay_entry_release_ov( op, group, 0, on );
+                               }
 
                                for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
                                        if ( dnIsSuffix( &new_ndn, &agf->agf_ndn ) ) {
+                                               /* TODO: should retest filter as it could imply conditions on the dn */
                                                is_newdn = 1;
                                                break;
                                        }
                                }
 
 
+                               if ( is_value_refresh ) {
+                                       if ( is_olddn != is_newdn ) {
+                                               /* group refresh */
+                                               autogroup_delete_member_from_group( op, NULL, NULL, age) ;
+
+                                               for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
+                                                       autogroup_add_members_from_filter( op, NULL, age, agf, 1 );
+                                               }
+                                       }
+                                       ldap_pvt_thread_mutex_unlock( &age->age_mutex );
+                                       continue;
+                               }
                                if ( is_olddn == 1 && is_newdn == 0 ) {
                                        autogroup_delete_member_from_group( op, &op->o_req_dn, &op->o_req_ndn, age );
                                } else
@@ -921,7 +1114,7 @@ autogroup_response( Operation *op, SlapReply *rs )
 
                                        m = op->orm_modlist;
 
-                                       for ( ; age ; age = age->age_next ) {
+                                       for ( age = agi->agi_entry ; age ; age = age->age_next ) {
                                                ldap_pvt_thread_mutex_lock( &age->age_mutex );
 
                                                dnMatch( &match, 0, NULL, NULL, &op->o_req_ndn, &age->age_ndn );
@@ -1002,7 +1195,7 @@ autogroup_response( Operation *op, SlapReply *rs )
                                overlay_entry_release_ov( op, group, 0, on );
 
                                for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
-                                       if ( dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
+                                       if ( !agf->agf_anlist && dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
                                                if ( test_filter( op, e, agf->agf_filter ) == LDAP_COMPARE_TRUE ) {
                                                        is_newdn = 1;
                                                        break;
@@ -1055,6 +1248,26 @@ autogroup_modify_entry( Operation *op, SlapReply *rs)
                return SLAP_CB_CONTINUE;
        }
 
+       /* Must refresh groups if a matching member value is modified */
+       for ( ; age ; age = age->age_next ) {
+               autogroup_filter_t      *agf;
+               for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
+                       if ( agf->agf_anlist ) {
+                               Modifications   *m;
+                               for ( m = op->orm_modlist ; m ; m = m->sml_next ) {
+                                       if ( m->sml_desc == agf->agf_anlist[0].an_desc ) {
+                                               if ( dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
+                                                       int rc = test_filter( op, e, agf->agf_filter );
+                                                       if ( rc == LDAP_COMPARE_TRUE ) {
+                                                               age->age_mustrefresh = 1;
+                                                       }
+                                               }
+                                       }
+                               }
+                       }
+               }
+       }
+
        a = attrs_find( e->e_attrs, slap_schema.si_ad_objectClass );
 
        if ( a == NULL ) {
@@ -1077,7 +1290,7 @@ autogroup_modify_entry( Operation *op, SlapReply *rs)
 
                        m = op->orm_modlist;
 
-                       for ( ; age ; age = age->age_next ) {
+                       for ( age = agi->agi_entry ; age ; age = age->age_next ) {
                                dnMatch( &match, 0, NULL, NULL, &op->o_req_ndn, &age->age_ndn );
 
                                if ( match == 0 ) {
@@ -1105,6 +1318,51 @@ autogroup_modify_entry( Operation *op, SlapReply *rs)
        return SLAP_CB_CONTINUE;
 }
 
+/*
+** Detect if the olddn is part of a group and so if the group should be refreshed
+*/
+static int
+autogroup_modrdn_entry( Operation *op, SlapReply *rs)
+{
+       slap_overinst           *on = (slap_overinst *)op->o_bd->bd_info;
+       autogroup_info_t                *agi = (autogroup_info_t *)on->on_bi.bi_private;
+       autogroup_entry_t       *age = agi->agi_entry;
+       Entry                   *e;
+
+       if ( get_manageDSAit( op ) ) {
+               return SLAP_CB_CONTINUE;
+       }
+
+       Debug( LDAP_DEBUG_TRACE, "==> autogroup_modrdn_entry <%s>\n", op->o_req_dn.bv_val, 0, 0);
+       ldap_pvt_thread_mutex_lock( &agi->agi_mutex );                  
+
+       if ( overlay_entry_get_ov( op, &op->o_req_ndn, NULL, NULL, 0, &e, on ) !=
+               LDAP_SUCCESS || e == NULL ) {
+               Debug( LDAP_DEBUG_TRACE, "autogroup_modrdn_entry cannot get entry for <%s>\n", op->o_req_dn.bv_val, 0, 0);
+               ldap_pvt_thread_mutex_unlock( &agi->agi_mutex );
+               return SLAP_CB_CONTINUE;
+       }
+
+       /* Must check if a dn is modified */
+       for ( ; age ; age = age->age_next ) {
+               autogroup_filter_t      *agf;
+               for ( agf = age->age_filter ; agf ; agf = agf->agf_next ) {
+                       if ( agf->agf_anlist ) {
+                               if ( dnIsSuffix( &op->o_req_ndn, &agf->agf_ndn ) ) {
+                                       int rc = test_filter( op, e, agf->agf_filter );
+                                       if ( rc == LDAP_COMPARE_TRUE ) {
+                                               age->age_modrdn_olddnmodified = 1;
+                                       }
+                               }
+                       }
+               }
+       }
+
+       overlay_entry_release_ov( op, e, 0, on );
+       ldap_pvt_thread_mutex_unlock( &agi->agi_mutex );                        
+       return SLAP_CB_CONTINUE;
+}
+
 /* 
 ** Builds a filter for searching for the 
 ** group entries, according to the objectClass. 
@@ -1242,6 +1500,8 @@ ag_cfgen( ConfigArgs *c )
                                        ch_free( agf->agf_filterstr.bv_val );
                                        ch_free( agf->agf_dn.bv_val );
                                        ch_free( agf->agf_ndn.bv_val );
+                                       anlist_free( agf->agf_anlist, 1, NULL );
+                                       ch_free( agf );
                                }
 
                                ldap_pvt_thread_mutex_init( &age->age_mutex );
@@ -1289,6 +1549,8 @@ ag_cfgen( ConfigArgs *c )
                                                ch_free( agf->agf_filterstr.bv_val );
                                                ch_free( agf->agf_dn.bv_val );
                                                ch_free( agf->agf_ndn.bv_val );
+                                               anlist_free( agf->agf_anlist, 1, NULL );
+                                               ch_free( agf );
                                        }
 
                                        ldap_pvt_thread_mutex_destroy( &age->age_mutex );
@@ -1530,6 +1792,7 @@ autogroup_db_close(
                                ch_free( agf->agf_filterstr.bv_val );
                                ch_free( agf->agf_dn.bv_val );
                                ch_free( agf->agf_ndn.bv_val ); 
+                               anlist_free( agf->agf_anlist, 1, NULL );
                                ch_free( agf );
                        }
 
@@ -1584,6 +1847,7 @@ autogroup_initialize(void)
        autogroup.on_bi.bi_op_add = autogroup_add_entry;
        autogroup.on_bi.bi_op_delete = autogroup_delete_entry;
        autogroup.on_bi.bi_op_modify = autogroup_modify_entry;
+       autogroup.on_bi.bi_op_modrdn = autogroup_modrdn_entry;
 
        autogroup.on_response = autogroup_response;