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

Fixed issue #18413: Cannot authenticate with LDAP when using PHP 8.1 #2665

Merged
merged 1 commit into from Oct 13, 2022

Conversation

Dennis1993
Copy link
Contributor

@Dennis1993 Dennis1993 commented Oct 13, 2022

After upgrading to PHP 8.1 the login via LDAP isn't possible.

Error message: Cannot use object of type LDAP\Connection as array

This is a fix for it.

Fixed issue #18413 (https://bugs.limesurvey.org/view.php?id=18413)

After upgrading to PHP 8.1 the login via LDAP isn't possible.

Error message: Cannot use object of type LDAP\Connection as array

This is a fix for it.

BugTracker: https://bugs.limesurvey.org/view.php?id=18413
@c-schmitz c-schmitz changed the title LDAP PHP 8.1 fix Fixed issue #18413: Cannot authenticate with LDAP when using PHP 8.1 Oct 13, 2022
@c-schmitz c-schmitz merged commit a26b5a8 into LimeSurvey:master Oct 13, 2022
Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are issue here : $ldapconn is an array in case of error.

You just avoid the situation here ?

Maybe : if(is_array()) ?

I don't understand why is_resource didn't work here ?

Can you send the whole report on mantis please ?

@@ -480,7 +480,7 @@ public function newUserSession()

// Try to connect
$ldapconn = $this->createConnection();
Copy link
Collaborator

@Dennis1993
Copy link
Contributor Author

Dennis1993 commented Oct 14, 2022

I found this on Google: https://php.watch/versions/8.1/LDAP-resource @Shnoulle

@Shnoulle
Copy link
Collaborator

I found this on Google: https://php.watch/versions/8.1/LDAP-resource @Shnoulle

Yes, but LimeSurvey can return array before return an invalid ressource.

I currently check with invalid user in my test system to report and fix .

@Shnoulle
Copy link
Collaborator

@Shnoulle
Copy link
Collaborator

Shnoulle commented Oct 14, 2022

PS : see this simple code

        if ($ldapconn === false) {
            $this->setAuthFailure($ldapconn['errorCode'], gT($ldapconn['errorMessage']));

if $ldapconn === false then $ldapconn['errorCode'] is not set …

@andreas-bulling
Copy link

andreas-bulling commented Oct 21, 2022

I checked out the latest version from git but the problem isn't fixed for me with PHP 8.1. I still see the same error message "Cannot use object of type LDAP\Connection as array "

The error is in line 482:

if (isset($ldapconn['errorCode'])) {

EDIT: This error also occurs in other places. I think this needs a much more fundamental fix. The problem is that the type of $ldapconn is not unique, it is "misused" as an array - sometimes but not always. Part of the code expects an array, other parts don't.

For example, if I remove the first error by adding is_array($ldapconn), then I see another error in line 233

$oEvent->set('errorMessageBody', $ldapconn['errorMessage']);

I think this framework should be used instead:
https://www.php.net/manual/de/function.ldap-error.php

@andreas-bulling
Copy link

andreas-bulling commented Oct 22, 2022

I fixed those errors and created a pull request:
#2684

Now I see the following error message:
Credentials are valid but we failed to create a user

mail and full name properties have been set in the plugin settings

@Shnoulle
Copy link
Collaborator

EDIT: This error also occurs in other places. I think this needs a much more fundamental fix. The problem is that the type of $ldapconn is not unique, it is "misused" as an array - sometimes but not always. Part of the code expects an array, other parts don't.

$ldapconn is array when error. Let me fix it again

@andreas-bulling
Copy link

Fixed by #2684 and #2685

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