Closed (won't fix)
Project:
LDAP integration
Version:
5.x-1.5
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
1 Oct 2007 at 10:53 UTC
Updated:
11 Jul 2011 at 09:52 UTC
Jump to comment: Most recent file
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
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | ldapauth.module.diff | 601 bytes | bovine |
| #1 | ldap_integration_no_nullify.patch | 566 bytes | AmrMostafa |
Comments
Comment #1
AmrMostafa commentedConfirmed, I have the same issue, but let me clarify more on the bug.
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!
Comment #2
sebastien_aubry commentedHello,
I had this bug too, and this patch solved it for me too.
Regards
Comment #3
stormtrek commentedHi,
Same problem here. The patch does work.
Thanks!
Comment #4
Arbelo commentedHow 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.
Comment #5
ogharib commentedThe patch worked fine with me.
Comment #6
wmostrey commentedWorks as advertised. RTBC.
Comment #7
scafmac commentedSo 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.
Comment #8
AmrMostafa commentedI think "By Design" isn't optimal here. Because:
Back to the problem itself:
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?
Comment #9
cpugeniusmv commentedI 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.
Comment #10
cpugeniusmv commentedI'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().Comment #11
scafmac commentedAgreed, 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:
Comment #12
AmrMostafa commentedUnfortunately 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
Comment #13
scafmac commentedNever 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.
Comment #14
bovine commentedI 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.
Comment #15
stefanw commentedThank 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.
Comment #16
dbennett commentedI 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.
Comment #17
wmostrey commentedThat means #14 is RTBC.
Comment #18
wmostrey commentedMarking 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).