Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP Plugin - Call to a member function dn() on bool #270

Open
ZebboKlaufix opened this issue Apr 4, 2023 · 3 comments
Open

LDAP Plugin - Call to a member function dn() on bool #270

ZebboKlaufix opened this issue Apr 4, 2023 · 3 comments

Comments

@ZebboKlaufix
Copy link

Hi,
I,m using
osTicket v1.17.3 (ca95150)
Apache
MySQL 10.11.2
PHP 8.1.17
389ds on Port 636 (SSL)

I configured the LDAP-Plugin to use the 389ds as LDAP-Source.

Szene1: wrong username & password
[x] working like expected: Access denied

Szene2: correct username & wrong|correct password
[ ] error: stuck at loading spinner on the login screen

So I took a look. tcpdump shows package transfer on the ldap-server. Cool
Webserver Error Log shows:
"PHP Fatal error: Uncaught Error: Call to a member function dn() on bool in phar:///srv/www/htdocs/osTicket/include/plugins/auth-ldap.phar/authentication.php:246"

Ok. Downloaded the latest dev version of the ldap plugin. Build phar file and stored it into the plugin folder. Same result.
After some tests I assume the error the stick between $r = $c->search(...) and $bound = $c->bind($r->current()->dn(), $password);
The plugin fetches the user entry, but $r->current() is empty, Therefore the users dn is unknown ab the plugin is unable to check the password.

@JediKev already adresses a kinda similar error, but not exactly this in #260

How do I fix it, so the bind will work?
If you need any further log output, I'm hopefully able to generate and post it.

Best
Zebbo

@JediKev
Copy link
Contributor

JediKev commented Apr 4, 2023

@ZebboKlaufix

I would suggest the following:

diff --git a/auth-ldap/authentication.php b/auth-ldap/authentication.php
index 69715ec..20aa6b3 100644
--- a/auth-ldap/authentication.php
+++ b/auth-ldap/authentication.php
@@ -238,7 +238,7 @@ class LDAPAuthentication {
                 $schema['lookup']),
             array('sizelimit' => 1)
         );
-        if (PEAR::isError($r) || !$r->count())
+        if (PEAR::isError($r) || !$r->count() || !$r->current())
             return null;
 
         // Attempt to bind as the DN of the user looked up with the password

This is because $r->current() is required and if that is false then the subsequent methods will fail.

To test this go to the database, go to the plugin table, find the record for LDAP, set isphar to 0, set the install_path to plugins/auth-ldap, cd to the osTicket plugin directory on the server (eg. cd /path/to/osticket/include/plugins/), and run the following command to unpack the plugin: php -r '$phar = new Phar("auth-ldap.phar"); $phar->extractTo("./auth-ldap");'

Once you do this you can make the changes to the plugin files manually and test the changes. You may need to restart Apache and PHP-FPM (if you're running it) to clear any file cache.

Cheers.

@ZebboKlaufix
Copy link
Author

ZebboKlaufix commented Apr 5, 2023

Thanks for your reply, @JediKev .
But as far as I understand, this fix would deny the login for anybody, because $r->current() is always empty. Even if the credentials are correct. This, of course, would prevent the endles spinner on the client side and the error message in the logs.
But the main problem still exists:
The user can't login.

If $r->count() returns 1, the user entry was found in LDAP, right? But how can I now access its dn without using current()? I don't fully understand what's inside $r, so I can't 'extract' the needed information by myself.

Best
Zebbo

@JediKev
Copy link
Contributor

JediKev commented Apr 5, 2023

@ZebboKlaufix

If $r->count() returns 1, the user entry was found in LDAP, right?

Not necessarily, it uses a "_count_cache" so that count could be cached from previous attempts, etc.

Also, the current() method comment states If the search throwed an error, it returns false. False is also returned, if the end is reached. So it seems maybe the user search is failing or the iterator cache is not resetting/updating properly.

Regardless, my patch above still stands as like I said $r->current() is required for subsequent code so if this is truly a boolean (like in your case) the rest will fail anyways. So something is going on with your connection, either not able to bind, search, or something.

Also, I don't even get to that part in my auth attempts. My connection goes through the typical LDAPAuthentication::authenticate() method from the plugin and returns here successfully. So if you get past that to where it's checking for $r->current() then your initial bind failed for some reason. You should be able to log the error message with something like:

diff --git a/auth-ldap/authentication.php b/auth-ldap/authentication.php
index 69715ec..3f2aa08 100644
--- a/auth-ldap/authentication.php
+++ b/auth-ldap/authentication.php
@@ -223,6 +223,7 @@ class LDAPAuthentication {
         $r = $c->bind($dn, $password);
         if (!PEAR::isError($r))
             return $this->lookupAndSync($username, $dn);
+        error_log(print_r($r, true));
 
         // Another effort is to search for the user
         if (!$this->_bind($c))

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants