Item14469: properly recode values coming from LDAP

also: - return more telling error message coming from LDAP - added TLS to ldaptest
foswiki · Aug 30, 2017 · 21d89a8 · 21d89a8
1 parent c87f509
commit 21d89a8
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 34 deletions.
diff --git a/data/System/LdapContrib.txt b/data/System/LdapContrib.txt
@@ -154,7 +154,7 @@ setting the parameters
       a couple of extra operations are performed to generate a proper <nop>WikiName, i.e.
       removing illegal characters.
    * =WikiNameAliases=: a comma separated key=value list of <nop>WikiNames to be mapped
-     to another <nop>WikiName (DEPCRETATED: use =RewriteWikiNames= instead).
+     to another <nop>WikiName (DEPRECATED: use =RewriteWikiNames= instead).
    * =RewriteWikiNames=: a list of rewrite rules; see below
    * =UserMappingTopic=: a topic that holds a pre-defined login-to-wikiname mapping; this
      comes in handy when migrating from a <nop>TopicUserMapping to an LDAP setup where
@@ -500,6 +500,7 @@ This work was partly sponsored by
 ---++ Change History
 
 %TABLE{columnwidths="7em" tablewidth="100%"}%
+|  30 Aug 2017: | fixed checking local groups; fixed recoding values coming from the LDAP directory |
 |  16 Jan 2017: | fixed logging in via email using an ldap-apache login manager |
 |  11 Jan 2017: | added config parameter to switch off mapping of login names to cUIDs |
 |  02 Sep 2016: | added ability to login using an email address |

diff --git a/lib/Foswiki/Contrib/LdapContrib.pm b/lib/Foswiki/Contrib/LdapContrib.pm
@@ -63,8 +63,8 @@ my $count = $result->count();
 my @entries = $result->sorted('sn');
 my $entry = $result->entry(0);
 
-my $value = $entry->get_value('cn');
-my @emails = $entry->get_value('mail');
+my $commonName = $this->getValue($entry, 'cn');
+my $email = $this->getValue($entry, 'mail');
 </verbatim>
 
 ---++ Cache storage format
@@ -498,7 +498,7 @@ sub checkError {
   if ($code == LDAP_SUCCESS || $code == LDAP_REFERRAL) {
     $this->{error} = undef;
   } else {
-    $this->{error} = $code . ': ' . $msg->error();
+    $this->{error} = $msg->error_name() . "($code): " . $msg->error();
     #writeDebug($this->{error});
   }
 
@@ -744,7 +744,7 @@ sub cacheBlob {
 
   if ($refresh || !-f $fileName) {
     writeDebug("caching blob for $attr to $fileName");
-    my $value = $entry->get_value($attr);
+    my $value = $entry->get_value($attr); # not using getValue() as this is a blob
     return unless $value;
     mkdir($dir, 0775) unless -e $dir;
 
@@ -1315,14 +1315,12 @@ sub cacheUserFromEntry {
   my $dn = $entry->dn();
 
   # 1. get it
-  my $loginName = $entry->get_value($this->{loginAttribute});
-  $loginName =~ s/^\s+//o;
-  $loginName =~ s/\s+$//o;
+  my $loginName = $this->getValue($entry, $this->{loginAttribute});
+  $loginName =~ s/^\s+|\s+$//g;
   unless ($loginName) {
     writeDebug("no loginName for $dn ... skipping");
     return 0;
   }
-  $loginName = $this->fromLdapCharSet($loginName);
 
   # 2. normalize
   $loginName = $this->rewriteName($loginName, $this->{rewriteLoginNames});
@@ -1354,11 +1352,9 @@ sub cacheUserFromEntry {
       # 1. compute a new wikiName
       my @wikiName = ();
       foreach my $attr (@{$this->{wikiNameAttributes}}) {
-        my $value = $entry->get_value($attr);
+        my $value = $this->getValue($entry, $attr);
         next unless $value;
-        $value =~ s/^\s+//o;
-        $value =~ s/\s+$//o;
-        $value = $this->fromLdapCharSet($value);
+        $value =~ s/^\s+|\s+$//g;
         #writeDebug("$attr=$value");
         push @wikiName, $value;
       }
@@ -1425,14 +1421,14 @@ sub cacheUserFromEntry {
 
   # get email addrs
   my $emails;
-  @{$emails} = $entry->get_value($this->{mailAttribute});
+  @{$emails} = $this->getValue($entry, $this->{mailAttribute});
 
   # lower case all emails
   @{$emails} = map {lc $_} @$emails if defined $emails;
 
   # get primary group
   if ($this->{primaryGroupAttribute}) {
-    my $groupId = $entry->get_value($this->{primaryGroupAttribute});
+    my $groupId = $this->getValue($entry, $this->{primaryGroupAttribute});
     $this->{_primaryGroup}{$groupId}{$loginName} = 1 if $groupId;    # delayed
   }
 
@@ -1500,14 +1496,12 @@ sub cacheGroupFromEntry {
   my $dn = $entry->dn();
   writeDebug("caching group for $dn");
 
-  my $groupName = $entry->get_value($this->{groupAttribute});
+  my $groupName = $this->getValue($entry, $this->{groupAttribute});
   unless ($groupName) {
     writeDebug("no groupName for $dn ... skipping");
     return 0;
   }
-  $groupName =~ s/^\s+//o;
-  $groupName =~ s/\s+$//o;
-  $groupName = $this->fromLdapCharSet($groupName);
+  $groupName =~ s/^\s+|\s+$//g;
 
   if ($this->{normalizeGroupName}) {
     $groupName = $this->normalizeWikiName($groupName);
@@ -1543,13 +1537,13 @@ sub cacheGroupFromEntry {
   $data->{"U2DN::$groupName"} = $dn;
 
   # cache groupIds
-  my $groupId = $entry->get_value($this->{primaryGroupAttribute});
+  my $groupId = $this->getValue($entry, $this->{primaryGroupAttribute});
   if ($groupId) {
     $this->{_groupId}{$groupId} = $groupName;
   }
 
   # fetch all members of this group
-  my $memberVals = $entry->get_value($this->{memberAttribute}, alloptions => 1);
+  my $memberVals = $this->getValueMap($entry, $this->{memberAttribute});
   my @members = (defined($memberVals) && exists($memberVals->{''})) ? @{$memberVals->{''}} : ();
 
   my $addMember = sub {
@@ -1595,15 +1589,14 @@ sub cacheGroupFromEntry {
         last;
       }
 
-      $memberVals = $newEntry->get_value($this->{memberAttribute}, alloptions => 1);
+      $memberVals = $this->getValueMap($newEntry, $this->{memberAttribute});
     }
   }
 
   # fetch all inner groups of this group
-  foreach my $innerGroup ($entry->get_value($this->{innerGroupAttribute})) {
+  foreach my $innerGroup ($this->getValues($entry, $this->{innerGroupAttribute})) {
     next unless $innerGroup;
-    $innerGroup =~ s/^\s+//o;
-    $innerGroup =~ s/\s+$//o;
+    $innerGroup =~ s/^\s+|\s+$//g;
     # TODO: range=X-Y syntax not supported yet for innerGroups
     $this->{_groups}{$groupName}{$innerGroup} = 1;    # delay til all groups have been fetched
   }
@@ -2645,11 +2638,71 @@ sub fromLdapCharSet {
   my ($this, $string) = @_;
 
   my $ldapCharSet = $Foswiki::cfg{Ldap}{CharSet} || 'utf-8';
-  return $string if $Foswiki::cfg{Site}{CharSet} eq $ldapCharSet;
-
   return Encode::decode($ldapCharSet, $string);
 }
 
+=pod
+
+---++ getValue($entry, $key) -> $value
+
+returns a decoded string from an Net::LDAP::Entry object
+
+=cut
+
+sub getValue {
+  my ($this, $entry, $key) = @_;
+
+  my $val = $entry->get_value($key);
+  return unless defined $val;
+
+  return $this->fromLdapCharSet($val);
+}
+
+=pod
+
+---++ getValues($entry, $key) -> @values
+
+returns a decoded an array of strings from an Net::LDAP::Entry object
+
+=cut
+
+sub getValues {
+  my ($this, $entry, $key) = @_;
+
+  my @vals = $entry->get_value($key);
+  return unless @vals;
+
+  @vals = map {$this->fromLdapCharSet($_)} @vals;
+
+  return @vals;
+}
+
+=pod
+
+---++ getValueMap($entry, $key) -> \%result
+
+returns a decoded an array of strings from an Net::LDAP::Entry object
+
+=cut
+
+sub getValueMap {
+  my ($this, $entry, $key) = @_;
+
+  my $map = $entry->get_value($key, alloptions => 1);
+  return unless $map;
+
+  while (my ($key, $val) = each %$map) {
+    if (ref($val)) {
+      $map->{$key} = [map {$this->fromLdapCharSet($_)} @$val];
+    } else {
+      $map->{$key} = $this->fromLdapCharSet($val);
+    }
+  }
+
+  return $map;
+}
+
+
 =begin text
 
 ---++ toLdapCharSet($string) -> $string
@@ -2662,7 +2715,6 @@ sub toSiteCharSet {
   my ($this, $string) = @_;
 
   my $ldapCharSet = $Foswiki::cfg{Ldap}{CharSet} || 'utf-8';
-
   return Encode::encode($ldapCharSet, $string);
 }
 
@@ -2682,10 +2734,8 @@ sub loadSession {
   if (defined $authUser) {
 
     my $origAuthUser = $authUser;
-
-    $authUser =~ s/^\s+//o;
-    $authUser =~ s/\s+$//o;
     $authUser = $this->fromLdapCharSet($authUser);
+    $authUser =~ s/^\s+|\s+$//g;
 
     $authUser = lc($authUser) unless $this->{caseSensitiveLogin};
     $authUser = $this->normalizeLoginName($authUser) if $this->{normalizeLoginName};

diff --git a/lib/Foswiki/Contrib/LdapContrib/Config.spec b/lib/Foswiki/Contrib/LdapContrib/Config.spec
@@ -88,8 +88,8 @@ $Foswiki::cfg{Ldap}{UseTLS} = 0;
 # 'sslv2', 'sslv3',  'sslv2/3' or 'tlsv1'
 $Foswiki::cfg{Ldap}{TLSSSLVersion} = 'tlsv1';
 
-# **STRING DISPLAY_IF="{Ldap}{UseTLS}" CHECK="undefok emptyok"**
-# Specify how to verify the servers certificate. Possible values are: 'require', 'optional'
+# **SELECT required, optional, none DISPLAY_IF="{Ldap}{UseTLS}" CHECK="undefok emptyok"**
+# Specify how to verify the servers certificate.
 # or 'require'.
 $Foswiki::cfg{Ldap}{TLSVerify} = 'require';
 

diff --git a/lib/Foswiki/Contrib/LdapContrib/MANIFEST b/lib/Foswiki/Contrib/LdapContrib/MANIFEST
@@ -1,3 +1,4 @@
+!noci
 data/System/LdapContrib.txt 0644
 lib/Foswiki/Contrib/LdapContrib/Config.spec 0644
 lib/Foswiki/Contrib/LdapContrib/DEPENDENCIES 0644

diff --git a/lib/Foswiki/LoginManager/KerberosLogin.pm b/lib/Foswiki/LoginManager/KerberosLogin.pm
@@ -197,7 +197,7 @@ sub getUser {
   unless (defined $inToken) {
     writeDebug("initial redirect using WWW_Authenticate for ".$request->url(-full => 1, -path => 1, -query => 1));
     $response->header(
-      #-status => 401, # don't send a 401 if not required; a 401 is decided based on ACLs.
+      -status => 401, 
       -WWW_Authenticate => 'Negotiate'
     );
     return;

diff --git a/lib/Foswiki/Users/LdapPasswdUser.pm b/lib/Foswiki/Users/LdapPasswdUser.pm
@@ -225,7 +225,10 @@ sub readOnly {
 
   if ($this->{ldap}->{allowChangePassword}) {
     $this->{session}->enterContext('passwords_modifyable');
+    return 0;
   }
+
+  return 1;
 }
 
 =pod

diff --git a/tools/ldaptest b/tools/ldaptest
@@ -29,6 +29,7 @@ our $baseDN = 'dc=nodomain';
 our $bindUser = '';
 our $bindPassword = '';
 our $useSASL = 0;
+our $useTLS = 0;
 our $saslMechanism = 'GSSAPI';
 our $sizeLimit = 10;
 our $deref = "always";
@@ -46,6 +47,14 @@ sub ldapSearch {
   my $ldap = Net::LDAP->new($server) || die "$@";
   my $mesg;
 
+  if ($useTLS) {
+    $mesg = $ldap->start_tls({
+       sslversion => 'tlsv1',
+       verify => 'none',
+    });
+    die $mesg->{errorMessage} if $mesg->{errorMessage};
+  }
+
   if ($useSASL) {
     my $sasl = Authen::SASL->new(mechanism => $saslMechanism) || die "$@";
     $mesg = $ldap->bind(sasl => $sasl);

diff --git a/tools/ldaptest.conf.example b/tools/ldaptest.conf.example
@@ -14,6 +14,7 @@ $bindUser = '';
 $bindPassword = '';
 
 # security settings
+$useTLS = 0;
 $useSASL = 0;
 $saslMechanism = 'GSSAPI';