From c4bb45a6bf39689e81542e31ee10d36111652258 Mon Sep 17 00:00:00 2001
From: James Rouzier <jrouzier@inverse.ca>
Date: Mon, 28 Jun 2021 19:18:54 -0400
Subject: [PATCH] Use the value of the basedn attribute

---
 lib/pf/Authentication/Source/LDAPSource.pm    |  5 ++--
 t/data/authentication.conf                    | 21 ++++++++++++++
 t/unittest/Authentication/Source/LDAPSource.t | 28 +++++++++++++++++--
 3 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/lib/pf/Authentication/Source/LDAPSource.pm b/lib/pf/Authentication/Source/LDAPSource.pm
index 5400638f4752..d07759c0730c 100644
--- a/lib/pf/Authentication/Source/LDAPSource.pm
+++ b/lib/pf/Authentication/Source/LDAPSource.pm
@@ -610,12 +610,13 @@ sub ldap_filter_for_conditions {
       foreach my $condition (@{$conditions}) {
           my $str;
           my $operator = $condition->{'operator'};
-          my $value = escape_filter_value($condition->{'value'});
           my $attribute = $condition->{'attribute'};
           if ($attribute eq "basedn") {
-              $basedn = $attribute;
+              $basedn = $condition->{'value'};
               next;
           }
+
+          my $value = escape_filter_value($condition->{'value'});
           if ($operator eq $Conditions::EQUALS) {
               $str = "${attribute}=${value}";
           } elsif ($operator eq $Conditions::NOT_EQUALS) {
diff --git a/t/data/authentication.conf b/t/data/authentication.conf
index 459b638b1d00..0ed0293c11bb 100644
--- a/t/data/authentication.conf
+++ b/t/data/authentication.conf
@@ -317,6 +317,27 @@ condition0=ldapfilter,match filter,(&(|(cn=${username})(samaccountname=${radius.
 action0=set_role=default
 action1=set_access_duration=1D
 
+[LDAPBASEDNSOURCE]
+description=pf-test
+password=
+scope=sub
+binddn=CN=test,DC=inverse,DC=ca
+basedn=DC=ldap,DC=inverse,DC=ca
+usernameattribute=user
+connection_timeout=5
+encryption=none
+port=33389
+type=AD
+host=127.0.0.1
+cache_match=1
+
+[LDAPBASEDNSOURCE rule advanced]
+description=apply default role
+match=all
+condition0=basedn,is,CN=IS_Assurance,DC=ldap,DC=inverse,DC=ca
+action0=set_role=default
+action1=set_access_duration=1D
+
 [tls_all]
 description=tls
 type=EAPTLS
diff --git a/t/unittest/Authentication/Source/LDAPSource.t b/t/unittest/Authentication/Source/LDAPSource.t
index 13500d957f32..59089647a682 100755
--- a/t/unittest/Authentication/Source/LDAPSource.t
+++ b/t/unittest/Authentication/Source/LDAPSource.t
@@ -114,7 +114,7 @@ BEGIN {
 
 }
 
-use Test::More tests => 9 + 2 * ( scalar @CACHEABLE_RULES + scalar @NON_CACHEABLE_RULES);
+use Test::More tests => 14 + 2 * ( scalar @CACHEABLE_RULES + scalar @NON_CACHEABLE_RULES);
 
 #This test will running last
 use Test::NoWarnings;
@@ -176,12 +176,36 @@ ok(!$source->is_rule_cacheable(undef), "undef is always uncacheable");
     my $rule = $source->rules->[0];
 
     ok($rule, "Got rule for $source_id");
-    my $filter = $source->ldap_filter_for_conditions($rule->conditions, $rule->match, $source->{usernameattribute}, { username => 'bob', 'radius.username' => "bobette" });
+    my ($filter, $basedn) = $source->ldap_filter_for_conditions($rule->conditions, $rule->match, $source->{usernameattribute}, { username => 'bob', 'radius.username' => "bobette" });
     is(
         $filter,
         '(&(|(cn=bob)(samaccountname=bobette))(|(memberof=student)(memberof=staff)))',
         "Use the advanced filter"
     );
+
+    is ($basedn, undef, "undef basedn");
+}
+
+{
+    my $source_id = 'LDAPBASEDNSOURCE';
+
+    my $source = getAuthenticationSource($source_id);
+
+    ok($source, "Got source id $source_id");
+
+    BAIL_OUT("Cannot get $source_id") unless $source;
+
+    my $rule = $source->rules->[0];
+
+    ok($rule, "Got rule for $source_id");
+    my ($filter, $basedn) = $source->ldap_filter_for_conditions($rule->conditions, $rule->match, $source->{usernameattribute}, { username => 'bob', 'radius.username' => "bobette" });
+    is(
+        $filter,
+        '(user=bob)',
+        "basic filter"
+    );
+
+    is ($basedn, "CN=IS_Assurance,DC=ldap,DC=inverse,DC=ca", "Condition basedn");
 }
 
 =head1 AUTHOR