Config:
- Drupal DB first, then LDAP

LDAP users authentication: no problem (even with wrong passwords)

Drupal users:
When a drupal user logs in with a wrong password I get the following error:
Fatal error: Call to a member function setOption() on a non-object in /home/www/essent.intranet/http/sites/all/modules/ldap_integration/ldapauth.module on line 662

When I log try to login with a not existing user I get the no acces messages => OK

Comments

AmrMostafa’s picture

Status: Active » Needs review
StatusFileSize
new566 bytes

Confirmed, I have the same issue, but let me clarify more on the bug.

  • To reproduce, you must have an existing local user (e.g. John).
  • That user must exist in LDAP too, with the same name (e.g. John)
  • Try to login with the LDAP password (which in my case is different from the local one)
  • You will get the error: Fatal error: Call to a member function setOption() on a non-object in ldapauth.module.

A patch attached, not very well tested, but it worked for me. It simply removes a line of code, I don't understand why that line was there in the first place, it must be for a good reason so I hope someone with better knowledge of the module code step in to review! Thanks!

sebastien_aubry’s picture

Hello,
I had this bug too, and this patch solved it for me too.
Regards

stormtrek’s picture

Hi,

Same problem here. The patch does work.

Thanks!

Arbelo’s picture

How do you apply the patch? Do you cut and paste it in ldapauth.module? (Sorry, I'm a new Drupal admin)

NEVERMIND- I figured it out.

ogharib’s picture

The patch worked fine with me.

wmostrey’s picture

Status: Needs review » Reviewed & tested by the community

Works as advertised. RTBC.

scafmac’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

So this patch, which does appear to work, changes the functionality of the module, assuming you have it set to authenticate against Drupal then LDAP on the admin page (/admin/settings/ldapauth/options). If that assumption is wrong, then let me know cause it would be a bug.

The way the module is meant to work when set to authenticate against Drupal is if there is a local and LDAP user with the same id, authenticates against the local user. If there is no local user, then authenticate against LDAP.

This patch sabotages that functionality - now all authentications when the user name exists in both locations will be LDAP only. That is not as designed.

Please let me know if I'm mistaken about the admin setting. Otherwise, this is not a bug and the patch, which should be unnecessary if you set the "LDAP Only" option on the admin, will not be committed.

AmrMostafa’s picture

I think "By Design" isn't optimal here. Because:

  • If the patch is wrong, then it's a patch problem and a better patch would be needed.
  • The "bug/issue" causes a Fatal Error, regardless of configuration settings, I don't think it's ever acceptable that a Fatal Error is produced.

Back to the problem itself:

The way the module is meant to work when set to authenticate against Drupal is if there is a local and LDAP user with the same id, authenticates against the local user. If there is no local user, then authenticate against LDAP.

What if there is a local user with the same id, but not the same password? The local auth will fail. What should the module do in that case? Give a "wrong user/pass message" or try to authenticate against LDAP?

cpugeniusmv’s picture

I think it should give a wrong user/pass message. If it were to try to authenticate against LDAP after the failed password attempt on a local account and the given password was successful in authenticating via LDAP, the creation of an account in Drupal would be unsuccessful because it would try to use the same username of the account that already exists.

cpugeniusmv’s picture

Status: Postponed (maintainer needs more info) » Needs review

I've looked over this area of the code a few times now and I don't understand how it makes all authentications when the user name exists in both locations become LDAP only. It looks to me like the functionality of the _ldap_user_authenticate function, where this applies, doesn't change at all. If the username exists in Drupal and isn't ldap_authentified, it'll use user_authenticate from core.

If I'm missing something and the patch does alter the functionality, here's an alternative solution: uncomment line 661 (// $ldapauth_ldap = new LDAPInterface();) in function _ldapauth_init().

scafmac’s picture

Status: Closed (works as designed) » Postponed (maintainer needs more info)

Agreed, a fatal error should not occur. And I think the current paradigm is the right one - if set to authenticate against Drupal first, then LDAP and a Drupal user exists and the password isn't correct - it fails. No checking LDAP.

So just to make sure we're on the same page, please address each of these:

  • You've tried both the LDAP authentication options, Drupal first or LDAP only? And you experienced the same error message in both cases?
  • Are you sure of the version? You are using the 5.x-1.3 download? The 5.x-dev release is the most up-to-date version right now.
  • How did the duplicate username come to occur? Did the drupal account exist first and then you installed the ldap module?
AmrMostafa’s picture

Status: Needs review » Postponed (maintainer needs more info)

Unfortunately I don't remember exactly how to reproduce. I will try to get back to the production server and reproduce one locally, revert the patch and see if I can reproduce. If anybody can reproduce now please do it :P

scafmac’s picture

Assigned: Unassigned » scafmac
Status: Postponed (maintainer needs more info) » Active

Never mind, I can repeat it on both 5.x-1.3 & 5.x-dev.

It turns out this might be a problem with the module architecture. You're better off sticking with the patch for now.

bovine’s picture

StatusFileSize
new601 bytes

I was experiencing this problem and ended up coming up with precisely the change suggested by comment #10 (uncomment // $ldapauth_ldap = new LDAPInterface();) and it seems to have resolved that problem for me.

My patch adds a null check around that line, since I didn't want to unnecessarily reallocate it every time.

stefanw’s picture

Thank you. I can confirm that this patch works and resolves the problem.
(Drupal 5.10 here, with ldapauth 5.x-1.3)

I vote for merging the patch into the module.
Fatal errors on common actions like login should really be avoided if possible.

dbennett’s picture

Version: 5.x-1.3 » 5.x-1.5

I received the error with ldapauth version 5.x-15. bovine's patch worked great, and my site is checking for users in its Drupal DB before checking LDAP. Using Drupal version 5.21.

Thanks.

wmostrey’s picture

Status: Active » Reviewed & tested by the community

That means #14 is RTBC.

wmostrey’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

Marking this as fixed since Drupal 5 is no longer supported (and this module doesn't appear to be either, or at least not the D5 version).