Mainly cosmetic changes

zmldapsync/README.md

@@ -14,7 +14,7 @@ The goals are :
 
 ## Configuration
 
-The configuration is stored in a single file in YAML format. The script will look for a config at /opt/zimbra/conf/ldap_sync.yml or trhe one specified in the --config argument.
+The configuration is stored in a single file in YAML format. The script will look for a config at /opt/zimbra/conf/zmldapsync.yml or trhe one specified in the --config argument.
 
 The config has two main section :
 
@@ -70,23 +70,36 @@ domains:
     groups:
       base: ou=groups,dc=corp2,dc=com
 
-  # A more complete example, which shows all the available settings
+  # A more complete example, which shows all the available settings, with their meaning
   corp3.net:
     ldap:
+      # List of LDAP servers to try (in order)
       servers:
         - ldap://ldap1.corp3.net:389
         - ldap://ldap3.corp3.net:389
+      # Use starttls/ Do not set this if using ldaps:// URI
       start_tls: True
+      # Optional bind DN and bind password for searches
       bind_dn: CN=Zimbra,OU=Apps,DC=corp3,DC=net
       bind_pass: 'p@ssw0rd'
+      # the schema used. Can be ad, rfc2307, rfc2307bis or simply ldap.
+      # ad, rfc2307 and rfc2307bis provides default values for attribute mapping. ldap is when you want
+      # a complete control, and you'll have to configure the mapping yourself
       schema: ad
-      type: ad 
     users:
+      # Base DN where to look for users
       base: OU=People,DC=corp3,DC=net
+      # Filter to look for users
       filter: '(&(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=Role_Mail,OU=Roles,DC=corp3,DC=net)(mail=*))'
+      # The attribute which uniquely identify a user. Usually either uid or sAMAccountName
+      # This attribute will be used as the user name in Zimbra (with the domain appended)
       key: sAMAccountName
+      # The attribute for the main email address
       mail_attr: mail
+      # The attribute for email aliases
       alias_attr: otherMailbox
+      # A dict of attribute to map from external LDAP to Zimbra.
+      # The format is ext_attr: zimbra_attr
       attr_map:
         displayName: displayName
         description: description
@@ -103,18 +116,31 @@ domains:
         title: title
         company: company
     groups:
+      # The base DN where to look for groups
       base: OU=Groups,DC=corp3,DC=net
+      # An optional filter to apply to group searches
       filter: (objectClass=group)
+      # The atribute which uniquely identify a group. Usually cn
+      # This attribute will be used as the distribution list name in Zimbra (with the domain appended)
       key: cn
+      # The attribute which lists the group members
       members_attr: member
+      # Are the members listed as full DN, or simply usernames (like memberUid with posixGroups)
       members_as_dn: True
+      # The attribute for the main email address
       mail_attr: mail
+      # The attribute for email aliases
       alias_attr: null
+      # A dict of attribute to map from external LDAP to Zimbra.
+      # The format is ext_attr: zimbra_attr
       attr_map:
         displayName: displayName
         description: description
     zimbra:
+      # Should zmldapsync create the domain if missing ?
       create_if_missing: False
+      # If the domain in Zimbra exists but is not configured
+      # for external auth (either LDAP or AD), should this script configure it ?
       setup_ldap_auth: True
 ```
 
@@ -122,6 +148,6 @@ domains:
 
 Once a configuration file is ready, the script can be called with the following command line arguments :
 
-  * --config : path to the config file (defaults to /opt/zimbra/conf/ldap_sync.yml)
+  * --config : path to the config file (defaults to /opt/zimbra/conf/zmldapsync.yml)
   * --quiet : will not print anything except errors
   * --verbose : prints aditional info during the sync

zmldapsync/zmldapsync.pl

@@ -92,7 +92,11 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
     ]
   );
   if ( $zim_domain_search->code ) {
-    handle_error( $domain, 'Zimbra domain lookup', $zim_domain_search->error );
+    handle_error(
+      $domain,
+      'Zimbra domain lookup',
+      $zim_domain_search->error
+    );
     next DOMAIN
   }
 
@@ -100,13 +104,23 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
   if ( scalar $zim_domain_search->entries == 0 ) {
     if ( yaml_bool($conf->{domains}->{$domain}->{zimbra}->{create_if_missing}) ) {
       log_info( "Creating domain $domain" );
-      ZmClient::sendZmprovRequest( "createDomain $domain " . build_domain_attrs($conf->{domains}->{$domain}) );
+      ZmClient::sendZmprovRequest( "createDomain $domain " .
+	                           build_domain_attrs($conf->{domains}->{$domain})
+      );
     } else {
-      handle_error( $domain, 'Zimbra domain lookup', "Domain $domain doesn't exist in Zimbra");
+      handle_error(
+        $domain,
+	'Zimbra domain lookup',
+	"Domain $domain doesn't exist in Zimbra"
+      );
       next DOMAIN;
     }
   } elsif ( scalar $zim_domain_search->entries gt 1 ) {
-    handle_error( $domain, 'Zimbra domain lookup', "Found several matches for domain $domain" );
+    handle_error(
+      $domain,
+      'Zimbra domain lookup',
+      "Found several matches for domain $domain"
+    );
     next DOMAIN;
   }
 
@@ -114,11 +128,18 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
   my $domain_entry = ldap2hashref( $zim_domain_search, 'zimbraDomainName' )->{$domain};
 
   # Check if auth is set to ad or ldap
-  if ( not defined $domain_entry->{zimbraAuthMech} or $domain_entry->{zimbraAuthMech} !~ m/^ad|ldap$/i) {
+  if (
+      not defined $domain_entry->{zimbraAuthMech} or
+      $domain_entry->{zimbraAuthMech} !~ m/^ad|ldap$/i
+     ) {
     if ( yaml_bool( $conf->{domains}->{$domain}->{zimbra}->{setup_ldap_auth} ) ) {
       send_zmprov_cmd( "modifyDomain $domain " . build_domain_attrs( $conf->{domains}->{$domain} ) );
     } else {
-      handle_error( $domain, 'Domain external auth check', "domain $domain must be configured for LDAP or AD authentication first" );
+      handle_error(
+        $domain,
+	'Domain external auth check',
+	"domain $domain must be configured for LDAP or AD authentication first"
+      );
       next DOMAIN;
     }
   }
@@ -128,16 +149,22 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
     filter => "(&(objectClass=zimbraDomain)(zimbraDomainAliasTargetId=" . $domain_entry->{zimbraId} . "))"
   );
   if ( $zim_domain_alias_search->code ) {
-    handle_error( $domain, 'Zimbra domain alias lookup', $zim_domain_alias_search->error );
+    handle_error(
+      $domain,
+      'Zimbra domain alias lookup',
+      $zim_domain_alias_search->error
+    );
     next DOMAIN;
   }
 
   $domain_entry->{zimbraDomainAliases} = [];
   foreach my $alias ( $zim_domain_alias_search->entries ) {
-    push @{ $domain_entry->{zimbraDomainAliases} }, $alias->get_value('zimbraDomainName');
+    push @{ $domain_entry->{zimbraDomainAliases} },
+        $alias->get_value('zimbraDomainName');
   }
 
-  log_verbose( "Trying to connect to " . join( ' or ', @{ $conf->{domains}->{$domain}->{ldap}->{servers} } ) );
+  log_verbose( "Trying to connect to " .
+	         join( ' or ', @{ $conf->{domains}->{$domain}->{ldap}->{servers} } ) );
 
   my $ext_ldap = Net::LDAP->new( [ @{ $conf->{domains}->{$domain}->{ldap}->{servers} } ] );
   if ( not $ext_ldap ) {
@@ -185,14 +212,21 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
       ]
     );
     if ( $zim_aliases_search->code ) {
-      handle_error( $domain, 'Zimbra user and distribution lists alias lookup', $zim_aliases_search->error );
+      handle_error(
+        $domain,
+	'Zimbra user and distribution lists alias lookup',
+	$zim_aliases_search->error
+      );
       next DOMAIN;
     }
 
     $zim_aliases->{$domain_alias} = ldap2hashref( $zim_aliases_search, 'uid' );
   }
 
-  log_verbose( "Searching for potential users in " . $conf->{domains}->{$domain}->{users}->{base} . " matching filter " . $conf->{domains}->{$domain}->{users}->{filter} );
+  log_verbose( "Searching for potential users in " .
+	       $conf->{domains}->{$domain}->{users}->{base} .
+	       " matching filter " .
+	       $conf->{domains}->{$domain}->{users}->{filter} );
 
   # List of attributes to fetch from LDAP
   # First, we want all the attributes which are mapped to Zimbra fields
@@ -207,6 +241,7 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
     push $fetch_attrs, $conf->{domains}->{$domain}->{users}->{$_};
   }
 
+  # Now we can run the lookup
   my $ext_user_search = $ext_ldap->search(
      base   => $conf->{domains}->{$domain}->{users}->{base},
      filter => $conf->{domains}->{$domain}->{users}->{filter},
@@ -214,11 +249,16 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
   );
 
   if ( $ext_user_search->code ) {
-    handle_error( $domain, 'External LDAP user lookup', $ext_user_search->error );
+    handle_error(
+      $domain,
+      'External LDAP user lookup',
+      $ext_user_search->error
+    );
     next DOMAIN;
   }
 
-  log_verbose( "Found " . scalar $ext_user_search->entries . " users in external LDAP" );
+  log_verbose( "Found " . scalar $ext_user_search->entries .
+	       " users in external LDAP" );
 
   log_verbose( "Searching for users in Zimbra" );
 
@@ -231,23 +271,37 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
                 '(mail=' . $zim_ldap->global->get_value('zimbraAmavisQuarantineAccount') . ')' .
                 '(uid=galsync*)(uid=admin))))',
     attrs => [
-      ( map { $conf->{domains}->{$domain}->{users}->{attr_map}->{$_} } keys $conf->{domains}->{$domain}->{users}->{attr_map} ),
+      ( map { $conf->{domains}->{$domain}->{users}->{attr_map}->{$_} }
-      ( 'uid', 'zimbraAccountStatus', 'zimbraAuthLdapExternalDn', 'zimbraMailAlias', 'mail', 'zimbraNotes' )
+	       keys $conf->{domains}->{$domain}->{users}->{attr_map} ),
+      ( 'uid',
+	'zimbraAccountStatus',
+	'zimbraAuthLdapExternalDn',
+	'zimbraMailAlias',
+	'mail',
+	'zimbraNotes' )
     ]
   );
   if ( $zim_user_search->code ) {
-    handle_error( $domain, 'Zimbra users lookup', $zim_user_search->error );
+    handle_error(
+      $domain,
+      'Zimbra users lookup',
+      $zim_user_search->error
+    );
     next DOMAIN;
   }
 
-  log_verbose( "Found " . scalar $zim_user_search->entries . " users in Zimbra" );
+  log_verbose( "Found " . scalar $zim_user_search->entries .
+	       " users in Zimbra" );
 
   log_verbose( "Comparing the accounts" );
 
   my $ext_users = ldap2hashref(
     $ext_user_search,
     $conf->{domains}->{$domain}->{users}->{key},
-    ( $conf->{domains}->{$domain}->{users}->{mail_attr}, $conf->{domains}->{$domain}->{users}->{alias_attr} )
+    (
+      $conf->{domains}->{$domain}->{users}->{mail_attr},
+      $conf->{domains}->{$domain}->{users}->{alias_attr}
+    )
   );
   my $zim_users = ldap2hashref(
     $zim_user_search,
@@ -264,14 +318,17 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
       # User exists in Zimbra, lets check its attribute are up to date
       foreach my $attr ( keys $conf->{domains}->{$domain}->{users}->{attr_map} ) {
 
-        if ( not defined $ext_users->{$user}->{$attr} and not defined $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} ) {
+        if ( not defined $ext_users->{$user}->{$attr} and
+            not defined $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} ) {
           # Attr does not exist in external LDAP and in Zimbra, no need to continue comparing them
           next;
         }
-        if ( $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} ne 'sn' and not defined $ext_users->{$user}->{$attr} ) {
+        if ( $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} ne 'sn' and
+             not defined $ext_users->{$user}->{$attr} ) {
           # If the attribute doesn't exist in external LDAP, we must remove it from Zimbra.
           # Except for sn which is mandatory in Zimbra
-          $attrs .= '-' . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " '" . $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} . "' ";
+          $attrs .= '-' . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " " .
+	            zim_attr_value( $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} );
         } elsif (
                   ( $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} ne 'sn' and
                     $ext_users->{$user}->{$attr} ne ( $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} || '' )
@@ -280,12 +337,12 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
                     defined $ext_users->{$user}->{$attr} and
                     $ext_users->{$user}->{$attr} ne ( $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} || '' )
                 ) {
-            $attrs .= " " . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " " . zim_attr_value( $ext_users->{$user}->{$attr} );
+            $attrs .= " " . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " " .
+	              zim_attr_value( $ext_users->{$user}->{$attr} );
             log_verbose( "Attribute $attr for user $user changed from " .
                        $zim_users->{$user}->{$conf->{domains}->{$domain}->{users}->{attr_map}->{$attr}} .
                        " to " .
-                       $ext_users->{$user}->{$attr}
+                       $ext_users->{$user}->{$attr} );
-            );
         }
       }
 
@@ -308,7 +365,8 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
 
       foreach my $attr ( keys $conf->{domains}->{$domain}->{users}->{attr_map} ) {
         next if (not defined $ext_users->{$user}->{$attr} or $ext_users->{$user}->{$attr} eq '');
-        $attrs .= ' ' . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " " . zim_attr_value( $ext_users->{$user}->{$attr} );
+        $attrs .= ' ' . $conf->{domains}->{$domain}->{users}->{attr_map}->{$attr} . " " .
+                  zim_attr_value( $ext_users->{$user}->{$attr} );
       }
       # The password won't be used because Zimbra is set to use external LDAP/AD auth
       # But better to set it to a random value
@@ -318,11 +376,10 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
 
     my @ext_aliases = ();
     foreach my $mail_attr ( qw( mail_attr alias_attr ) ) {
-      next if (
+      next if ( not defined $conf->{domains}->{$domain}->{users}->{$mail_attr} or
-        not defined $conf->{domains}->{$domain}->{users}->{$mail_attr} or
+                not defined $ext_users->{$user}->{$conf->{domains}->{$domain}->{users}->{$mail_attr}} );
-        not defined $ext_users->{$user}->{$conf->{domains}->{$domain}->{users}->{$mail_attr}}
+      push @ext_aliases,
-      );
+             @{ $ext_users->{$user}->{$conf->{domains}->{$domain}->{users}->{$mail_attr}} };
-      push @ext_aliases, @{ $ext_users->{$user}->{$conf->{domains}->{$domain}->{users}->{$mail_attr}} };
     }
 
     @ext_aliases = sort @ext_aliases;
@@ -335,23 +392,27 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
     }
 
     # On each sync, we register the list of LDAP aliases into Zimbra's LDAP in the zimbraNotes attribute
-    # We can compare if it has changed, and and/remove the aliases accordingly
+    # We can compare if it has changed, and add/remove the aliases accordingly
     # This is not very clean, but at least allows the script to be "stateless"
-    # and only relies on LDAP content on both sides
+    # and only relies on LDAP content on both sides. If only zimbraAlias objectClass allowed zimbraNotes attribute
+    # it'd be easier
     my $ext_prev_aliases = parse_zimbra_notes( $zim_users->{$user}->{zimbraNotes} || '' )->{LDAP_Aliases};
     my @ext_prev_aliases = ( defined $ext_prev_aliases ) ? sort @{ $ext_prev_aliases } : ();
     my $alias_diff       = Array::Diff->diff( \@ext_prev_aliases, \@ext_aliases );
 
     foreach my $alias ( @{ $alias_diff->deleted } ) {
       my ( $al, $dom ) = split /\@/, $alias;
-      next if ( not defined $zim_aliases->{$dom} or not defined $zim_aliases->{$dom}->{$al} );
+      next if ( not defined $zim_aliases->{$dom} or
-      log_verbose( "Removing LDAP alias $alias from user $user as it doesn't exist in LDAP anymore" );
+	        not defined $zim_aliases->{$dom}->{$al} );
+      log_verbose( "Removing LDAP alias $alias from user $user " .
+	           "as it doesn't exist in LDAP anymore" );
       send_zmprov_cmd( "removeAccountAlias $user\@$domain $alias" );
     }
 
     my $note = $sync_from_ldap . "|LDAP_Aliases=" . join(',', @ext_aliases);
     if ( $note ne ($zim_users->{$user}->{zimbraNotes} || '') ) {
-      send_zmprov_cmd( "modifyAccount $user\@$domain zimbraNotes " . zim_attr_value( $note ) );
+      send_zmprov_cmd( "modifyAccount $user\@$domain zimbraNotes " .
+	               zim_attr_value( $note ) );
     }
   }
 
@@ -364,12 +425,13 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
          $zim_users->{$user}->{zimbraNotes} =~ m/^$sync_from_ldap/ and
          defined $zim_users->{$user}->{zimbraAccountStatus} and
          $zim_users->{$user}->{zimbraAccountStatus} =~ m/^active|lockout$/ ) {
-      log_verbose( "User $user doesn't exist in external LDAP anymore, locking it in Zimbra" );
+      log_verbose( "User $user doesn't exist in external LDAP anymore, " .
+                   "locking it in Zimbra" );
       send_zmprov_cmd( "modifyAccount $user\@$domain zimbraAccountStatus locked" );
     }
   }
 
-  # Now, we try to sync groups in external LDAP into distribution list in Zimbra
+  # Now, we try to sync groups in external LDAP into distribution lists in Zimbra
   if ( defined $conf->{domains}->{$domain}->{groups} ) {
 
     log_verbose( "Searching for potential groups in " .
@@ -397,7 +459,8 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
       next DOMAIN;
     }
 
-    log_verbose( "Found " . scalar $ext_group_search->entries . " groups in external LDAP" );
+    log_verbose( "Found " . scalar $ext_group_search->entries .
+                 " groups in external LDAP" );
 
     log_verbose( "Searching for distribution lists in Zimbra" );
 
@@ -406,23 +469,37 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
       base   => 'ou=people,' . $domain_entry->{dn},
       filter => "(objectClass=zimbraDistributionList)",
       attrs  => [
-        ( map { $conf->{domains}->{$domain}->{groups}->{attr_map}->{$_} } keys $conf->{domains}->{$domain}->{groups}->{attr_map} ),
+        ( map { $conf->{domains}->{$domain}->{groups}->{attr_map}->{$_} }
-        ( 'uid', 'zimbraDistributionListSubscriptionPolicy', 'zimbraDistributionListUnsubscriptionPolicy',
+            keys $conf->{domains}->{$domain}->{groups}->{attr_map} ),
-          'zimbraMailForwardingAddress', 'zimbraNotes', 'zimbraMailStatus', 'mail' )
+        (
+          'uid',
+	  'zimbraDistributionListSubscriptionPolicy',
+	  'zimbraDistributionListUnsubscriptionPolicy',
+          'zimbraMailForwardingAddress',
+	  'zimbraNotes',
+	  'zimbraMailStatus',
+	  'mail'
+        )
       ]
     );
     if ( $zim_dl_search->code ) {
-      handle_error( $domain, 'Zimbra distribution lists lookup', $zim_dl_search->error );
+      handle_error(
+        $domain,
+	'Zimbra distribution lists lookup',
+	$zim_dl_search->error
+      );
       next DOMAIN;
     }
 
-    log_verbose( "Found " . scalar $zim_dl_search->entries . " distribution list(s) in Zimbra" );
+    log_verbose( "Found " . scalar $zim_dl_search->entries .
+	         " distribution list(s) in Zimbra" );
     log_verbose( "Comparing groups with distribution lists" );
 
     my $ext_groups = ldap2hashref(
       $ext_group_search,
       $conf->{domains}->{$domain}->{groups}->{key},
-      ( $conf->{domains}->{$domain}->{groups}->{members_attr},
+      (
+        $conf->{domains}->{$domain}->{groups}->{members_attr},
         $conf->{domains}->{$domain}->{groups}->{mail_attr},
         $conf->{domains}->{$domain}->{groups}->{alias_attr}
       )
@@ -434,53 +511,49 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
     );
 
     # Build a dn2id hashref to lookup users or groups by their DN
-    my $dn2id      = {};
+    my $dn2id = {};
-    $dn2id->{$ext_users->{$_}->{dn}} = $_ foreach ( keys $ext_users );
+    $dn2id->{$ext_users->{$_}->{dn}}  = $_ foreach ( keys $ext_users );
     $dn2id->{$ext_groups->{$_}->{dn}} = $_ foreach ( keys $ext_groups );
 
-    # First loop, check if every group in LDAP exist as a DL in Zimbra
+    # First loop, check if every group in LDAP exists as a DL in Zimbra
     foreach my $group ( keys $ext_groups ) {
       if ( defined $zim_dl->{$group} ) {
         # A group match an existing DL, we must check its attributes
 
         my $attrs = '';
         foreach my $attr ( keys $conf->{domains}->{$domain}->{groups}->{attr_map} ) {
-          if (
+          if ( not defined $ext_groups->{$group}->{$attr} and
-               not defined $ext_groups->{$group}->{$attr} and
+               not defined $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} ) {
-               not defined $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}}
-             ) {
             # Attr does not exist in external LDAP and in Zimbra, not need to continue
             next;
           } elsif ( not defined $ext_groups->{$group}->{$attr} ) {
             # Attr doesn't exist in external LDAP, but exists in Zimbra. We must remove it
-            $attrs = ' -' . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " . zim_attr_value( $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} );
+            $attrs = ' -' . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " .
+                     zim_attr_value( $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} );
           } elsif ( $ext_groups->{$group}->{$attr} ne $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} ) {
             # Attr exists in both but doesn't match
-            $attrs .= " " . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " . zim_attr_value( $ext_groups->{$group}->{$attr} );
+            $attrs .= " " . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " .
-            log_verbose( $ext_groups->{$group}->{$attr} . " vs " . $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} );
+                      zim_attr_value( $ext_groups->{$group}->{$attr} );
+            log_verbose( $ext_groups->{$group}->{$attr} . " vs " .
+		         $zim_dl->{$group}->{$conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr}} );
           }
         }
 
         # Users cannot subscribe or unsubscribe from LDAP group
-        if (
+        if ( not defined $zim_dl->{$group}->{zimbraDistributionListSubscriptionPolicy} or
-             not defined $zim_dl->{$group}->{zimbraDistributionListSubscriptionPolicy} or
+             $zim_dl->{$group}->{zimbraDistributionListSubscriptionPolicy} ne 'REJECT' ) {
-             $zim_dl->{$group}->{zimbraDistributionListSubscriptionPolicy} ne 'REJECT'
-           ) {
           $attrs .= " zimbraDistributionListSubscriptionPolicy REJECT";
         }
-        if (
+        if ( not defined $zim_dl->{$group}->{zimbraDistributionListUnsubscriptionPolicy} or
-             not defined $zim_dl->{$group}->{zimbraDistributionListUnsubscriptionPolicy} or
+             $zim_dl->{$group}->{zimbraDistributionListUnsubscriptionPolicy} ne 'REJECT' ) {
-             $zim_dl->{$group}->{zimbraDistributionListUnsubscriptionPolicy} ne 'REJECT'
-           ) {
           $attrs .= " zimbraDistributionListUnsubscriptionPolicy REJECT";
         }
 
         # If the group in LDAP has a mail defined, enable mail delivery in Zimbra. Else, disable it
-        my $mail_status = ( defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{mail_attr}} ) ? 'enabled' : 'disabled';
+        my $mail_status = ( defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{mail_attr}} ) ?
-        if (
+	                    'enabled' : 'disabled';
-             not defined $zim_dl->{$group}->{zimbraMailStatus} or
+        if ( not defined $zim_dl->{$group}->{zimbraMailStatus} or
-             $zim_dl->{$group}->{zimbraMailStatus} ne $mail_status
+             $zim_dl->{$group}->{zimbraMailStatus} ne $mail_status ) {
-           ) {
           $attrs .= " zimbraMailStatus $mail_status";
         }
 
@@ -495,11 +568,15 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
         log_verbose( "Found a new group : $group. Creating it in Zimbra" );
         my $attrs = '';
         foreach my $attr ( keys $conf->{domains}->{$domain}->{groups}->{attr_map} ) {
-          next if (not defined $ext_groups->{$group}->{$attr} or $ext_groups->{$group}->{$attr} eq '');
+          next if ( not defined $ext_groups->{$group}->{$attr} or
-          $attrs .= ' ' . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " . zim_attr_value( $ext_groups->{$group}->{$attr} );
+                    $ext_groups->{$group}->{$attr} eq '');
+          $attrs .= ' ' . $conf->{domains}->{$domain}->{groups}->{attr_map}->{$attr} . " " .
+                    zim_attr_value( $ext_groups->{$group}->{$attr} );
         }
-        $attrs .= " zimbraDistributionListUnsubscriptionPolicy REJECT zimbraDistributionListSubscriptionPolicy REJECT";
+        $attrs .= " zimbraDistributionListUnsubscriptionPolicy REJECT"
-        my $mail_status = ( defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{mail_attr}} ) ? 'enabled' : 'disabled';
+        $attrs .= " zimbraDistributionListSubscriptionPolicy REJECT";
+        my $mail_status = ( defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{mail_attr}} ) ?
+                            'enabled' : 'disabled';
         $attrs .= " zimbraMailStatus $mail_status";
         send_zmprov_cmd( "createDistributionList $group\@$domain $attrs" );
       }
@@ -509,30 +586,40 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
       my @ext_members = ();
 
       if ( not yaml_bool( $conf->{domains}->{$domain}->{groups}->{members_as_dn} ) ) {
+        # If members are not listed as full DN, but by uid, simply concat it with the domain
         foreach my $member ( $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{members_attr}} ) {
-          next if ( not defined $ext_users->{$member} and not defined $ext_groups->{$member} );
+          next if ( not defined $ext_users->{$member} and
+                    not defined $ext_groups->{$member} );
           push @ext_members, $member . '@' . $domain;
         }
       } else {
+        # If members are listed as full DN, we need to lookup in the dn2id we prepared earlier
         foreach my $member ( @{ $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{members_attr}} } ) {
-          next if not defined $dn2id->{$member};
+          next if ( not defined $dn2id->{$member} );
           push @ext_members, $dn2id->{$member} . '@' . $domain;
         }
       }
       @ext_members = sort @ext_members;
-      my @zim_members = (defined $zim_dl->{$group}->{zimbraMailForwardingAddress} ) ? sort @{$zim_dl->{$group}->{zimbraMailForwardingAddress}} : ();
+      my @zim_members = ( defined $zim_dl->{$group}->{zimbraMailForwardingAddress} ) ?
+                          sort @{$zim_dl->{$group}->{zimbraMailForwardingAddress}} : ();
+
+      # Now we can compare members for this group in external LDAP and Zimbra
       my $diff = Array::Diff->diff( \@ext_members, \@zim_members );
 
       if ( scalar @{ $diff->deleted } gt 0 ){
-        send_zmprov_cmd( "addDistributionListMember $group\@$domain " . join (' ', @{ $diff->deleted } ) );
+        send_zmprov_cmd( "addDistributionListMember $group\@$domain " .
+          join (' ', @{ $diff->deleted } ) );
       }
       if ( scalar @{ $diff->added } gt 0 ) {
-        send_zmprov_cmd( "removeDistributionListMember $group\@$domain $_ ") foreach ( @{ $diff->added } );
+        send_zmprov_cmd( "removeDistributionListMember $group\@$domain $_")
+          foreach ( @{ $diff->added } );
       }
       my @ext_aliases = ();
       foreach my $mail_attr ( qw( mail_attr alias_attr ) ) {
-        next if (not defined $conf->{domains}->{$domain}->{groups}->{$mail_attr} or not defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{$mail_attr}} );
+        next if ( not defined $conf->{domains}->{$domain}->{groups}->{$mail_attr} or
-        push @ext_aliases, @{ $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{$mail_attr}} };
+                  not defined $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{$mail_attr}} );
+        push @ext_aliases,
+               @{ $ext_groups->{$group}->{$conf->{domains}->{$domain}->{groups}->{$mail_attr}} };
       }
       foreach my $alias ( @ext_aliases ) {
         next if ( not alias_matches_domain( $alias, $domain_entry ) );
@@ -548,22 +635,27 @@ DOMAIN: foreach my $domain ( keys $conf->{domains} ) {
 
       foreach my $alias ( @{ $alias_diff->deleted } ) {
         my ( $al, $dom ) = split /\@/, $alias;
-        next if ( not defined $zim_aliases->{$dom} or not defined $zim_aliases->{$dom}->{$al} );
+        next if ( not defined $zim_aliases->{$dom} or
-        log_verbose( "Removing LDAP alias $alias from distribution list $group as it doesn't exist in LDAP anymore" );
+                  not defined $zim_aliases->{$dom}->{$al} );
+        log_verbose( "Removing LDAP alias $alias from distribution list $group " .
+                     "as it doesn't exist in LDAP anymore" );
         send_zmprov_cmd( "removeDistributionListAlias $group\@$domain $alias" );
       }
 
       my $note = $sync_from_ldap . "|LDAP_Aliases=" . join(',', @ext_aliases);
       if ( $note ne ($zim_dl->{$group}->{zimbraNotes} || '') ) {
-        send_zmprov_cmd( "modifyDistributionList $group\@$domain zimbraNotes " . zim_attr_value( $note ) );
+        send_zmprov_cmd( "modifyDistributionList $group\@$domain zimbraNotes " .
+                         zim_attr_value( $note ) );
       }
     }
 
     # Now, look at all the distribution list which were created from LDAP but doesn't exist anymore in LDAP
     foreach my $dl ( keys $zim_dl ) {
-      next if ( not defined $zim_dl->{$dl}->{zimbraNotes} or $zim_dl->{$dl}->{zimbraNotes} !~ m/^$sync_from_ldap/ );
+      next if ( not defined $zim_dl->{$dl}->{zimbraNotes} or
+                $zim_dl->{$dl}->{zimbraNotes} !~ m/^$sync_from_ldap/ );
       next if ( defined $ext_groups->{$dl} );
-      log_verbose( "Group $dl doesn't exist in LDAP anymore, removing the corresponding distribution list" );
+      log_verbose( "Group $dl doesn't exist in LDAP anymore, " .
+                   "removing the corresponding distribution list" );
       send_zmprov_cmd( "deleteDistributionList $dl\@$domain" );
     }
   }
@@ -633,7 +725,9 @@ sub handle_error {
       },
       body_str => "LDAP synchronisation for domain $domain failed at step '$step'. The error was\n$err\n",
     );
-    my $transport = Email::Sender::Transport::Sendmail->new({ sendmail => '/opt/zimbra/common/sbin/sendmail' });
+    my $transport = Email::Sender::Transport::Sendmail->new({
+      sendmail => '/opt/zimbra/common/sbin/sendmail'
+    });
     sendmail( $mail, { transport => $transport } );
   }
   $exit = 255;
@@ -642,7 +736,7 @@ sub handle_error {
 # ldap2hashref takes three args
 #  * An LDAP search result
 #  * The attribute used as the key of objects
-#  * an optional array of attributes we want as an array, even if there's a single value
+#  * An optional array of attributes we want as an array, even if there's a single value
 # It'll return a hashref. The key will be unaccentuated and lower cased.
 
 sub ldap2hashref {
@@ -675,17 +769,21 @@ sub yaml_bool {
 # Takes the domain conf hashref as only arg
 sub build_domain_attrs {
   my $domain_conf = shift;
-  my $attrs = "zimbraAuthMech " . zim_attr_value( $domain_conf->{ldap}->{type} );
+  my $type = ( $domain_conf->{ldap}->{schema} eq =~ m/^ad/i ) ? 'ad' : 'ldap';
-  $attrs .= " zimbraAuthMechAdmin " . zim_attr_value( $domain_conf->{ldap}->{type} );
+  my $attrs = "zimbraAuthMech " . zim_attr_value( $type );
-  if ( defined $domain_conf->{ldap}->{bind_dn} and defined $domain_conf->{ldap}->{bind_pass} ) {
+  $attrs .= " zimbraAuthMechAdmin " . zim_attr_value( $type );
+  if ( defined $domain_conf->{ldap}->{bind_dn} and
+       defined $domain_conf->{ldap}->{bind_pass} ) {
      $attrs .= " zimbraAuthLdapSearchBindDn " . zim_attr_value( $domain_conf->{ldap}->{bind_dn} );
      $attrs .= " zimbraAuthLdapSearchBindPassword " . zim_attr_value( $domain_conf->{ldap}->{bind_pass} );
   }
 #  if ( defined $domain_conf->{users}->{filter} ) {
 #    $attrs = " zimbraAuthLdapSearchFilter " . zim_attr_value( "(&(|(" . $domain_conf->{users}->{key} . "=%u)(" . $domain_conf->{users}->{key} . "=%n))(" . $domain_conf->{users}->{filter} . ")" );
 #  }
-  $attrs .= " zimbraAuthLdapURL " . join( ' +zimbraAuthLdapURL', zim_attr_value( $domain_conf->{ldap}->{servers} ) );
+  $attrs .= " zimbraAuthLdapURL " .
-  if ( defined $domain_conf->{ldap}->{start_tls} and yaml_bool( $domain_conf->{ldap}->{start_tls} ) ) {
+            join( ' +zimbraAuthLdapURL', zim_attr_value( $domain_conf->{ldap}->{servers} ) );
+  if ( defined $domain_conf->{ldap}->{start_tls} and
+       yaml_bool( $domain_conf->{ldap}->{start_tls} ) ) {
     $attrs .= " zimbraAuthLdapStartTlsEnabled TRUE";
   } else {
     $attrs .= " zimbraAuthLdapStartTlsEnabled FALSE";