Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Please implement a hook to authenticate local users - similar to what hook_auth does for remote users. Alternatively, extend hook_auth to authenticate both local and remote users.
Current behaviour:
- If a user logs in with "user@domain", hook_auth gets activated;
- If a user logs in with "user", hook_auth is bypassed.
More details: http://drupal.org/node/27959
Comment | File | Size | Author |
---|---|---|---|
#59 | patch_47 | 7.48 KB | moshe weitzman |
#56 | patch_46 | 7.98 KB | moshe weitzman |
#54 | patch_45 | 7.64 KB | moshe weitzman |
#53 | user_new.patch_0.txt | 6.95 KB | naudefj |
#52 | user_new.patch.txt | 6.43 KB | naudefj |
Comments
Comment #1
killes@www.drop.org CreditAttribution: killes@www.drop.org commenteduse hook_user(login)
Comment #2
naudefj CreditAttribution: naudefj commentedI just tried hook_user(login), but it only fires when a valid drupal userid/password is entered.
Comment #3
naudefj CreditAttribution: naudefj commentedI found the reason why hook_auth only works for remote users. Look at this code from users.module:
Is this something that should be fixed, or should I just comment out the the "&& $server" part myself?
Comment #4
naudefj CreditAttribution: naudefj commentedHere is a patch that will allow HOOK_AUTH to work for both local and remote users. If would be great if it can be reviewed and applied to HEAD.
Comment #5
naudefj CreditAttribution: naudefj commentedComment #6
moshe weitzman CreditAttribution: moshe weitzman commentedhook_auth deliberately is ineffective for local logins. if you enable it for all logins, you will slow down the experience for many people who guess their passwords wrong on login page. in that case, code would call all remote auth modules an dthey in turn would contact their home base. thats too slow. we want to return a 'sorry try again' message very quickly.
what are you trying to accomplish here?
Comment #7
naudefj CreditAttribution: naudefj commentedHi,
> hook_auth deliberately is ineffective for local logins.
Not true. HOOK_AUTH cannot be used for local logins. Look at this line in user.module:
else if (!$user->uid && $server) {
> what are you trying to accomplish here?
I would like my users to use their existing forum userids and passwords to login to Drupal. However, I quickly discovered that external authentication only works if you append a domain/servername to your userid. For example, if a username like "user@domain" is entered, HOOK_AUTH gets activated. Problem is that if a userid like "user" is used, HOOK_AUTH will be bypassed. This fix allows local users (i.e. just "user") to be authenticated against an external source.
Any other solutions/ suggestions would be greatly appreciated!
Best regards.
Frank
Comment #8
nedjoWhy? I assume authentication should be either local or external--not both. Therefore, we need a way to distinguish between the two. Currently, the username@domain convention is used. Removing the @domain part of the DrupalID would remove this disctinction. Is the issue just that this convention is confusing for new users? In this case, I'd suggest pulling the domain part out into a different field. So, when drupal.module is enabled, there is a third user login field, "site" or "domain", that would include "local" and any registered associates.
Comment #9
naudefj CreditAttribution: naudefj commented> Why?
Without it, it would be impossible to have a single userid/password for your entire site. It would be silly to ask users to log in to Drupal using their forumid@local. Surely there must be a way to authenticate them without having to type the "@local" part?
> So, when drupal.module is enabled, there is a third user login field, "site" or "domain", that would include "local" and any registered associates.
Not sure I understand - I'm not using drupal.module, but are trying to implement my own authentication module without the need to hack into user.module (or any other module for that sake).
Comment #10
bslade CreditAttribution: bslade commentedNedjo said:
What exactly do you mean by "local" and "external"? Are you assuming:
I think this issue is talking about implementing some sort of authentication hook/module so that item (1) can access some other local table besides the core Drupal users table without having to hack the user.module.
It sounds like, right now, the answer is, "can't do that".
Ben in DC
Comment #11
naudefj CreditAttribution: naudefj commentedExactly, it is currently not possible. However, you can imagine how important it is to anyone that wants to integrate their site with some other "local" authentication provider.
Comment #12
pablobm CreditAttribution: pablobm commentedWell, this is one of the things I've been struggling with since I took over ldap_integration module maintenance. So I'm all for it.
For my module, I solved the issue without having to patch Drupal's core, but yet having to perform some hack stunt: I replaced a callback set in user.module by another one placed in an auxiliary module I called zcallbacks.module. (The name is due to its need to be alphabetically after user.module for the hack to work). Have a look at the code for the gory details.
Not to mention that I'd rather have a nicer way to do this that didn't cause any trouble in the long run. Sadly, a look to uncoming Drupal 4.7's code reveals that this is not going to change soon, but better late than never.
Comment #13
Geary CreditAttribution: Geary commentedThis is a critical need for me as well. I am implementing a corporate site and all user logins must be authenticated against a RADIUS server. Users will not be logging in with an @server, just a username, and I need to intercept all logins.
I will use the z trick like the LDAP module, but that is a a real hack.
Comment #14
moshe weitzman CreditAttribution: moshe weitzman commentedreplacing the login callback is a good solution IMO. nice one.
Comment #15
naudefj CreditAttribution: naudefj commentedDo you mean use hook_user(login)?
If so, it doesn't work - see comments above: "it only fires when a valid drupal userid/password is entered".
That still leaves us with no solution to this critical problem.
Comment #16
pablobm CreditAttribution: pablobm commentedThat's not what Moshe and I mean. We speak about a hack I implement in my ldap_integration module which makes use of undocumented stuff in order to achieve the desired effect. Have a look at the module code. The relevant parts are the short file zcallbacks.module and the last section of ldap_integration.module .
The problem is: it's still a hack and may cause problems in the future, so I'm still all for changing it.
Comment #17
naudefj CreditAttribution: naudefj commentedHi Pablo,
Thanks for clarifying your solution.
Looks like it will only require a couple of lines to be changed in users.module. Why don't you roll a patch, then we all have a clear API to develop against.
Best regards.
Frank
Comment #18
pablobm CreditAttribution: pablobm commentedFurther to this topic, Drupal 4.7 has brought another way to work this issue around.
The login process can be changed by altering the login forms through the new forms API. A quick example would be like this:
It is not yet the proper thing we are looking for, yet it is nicer than the last method I proposed (i.e.: taking over the
user_login
callback).And yes, I (or somebody else) should study the problem thoroughly and provide a proper solution to propose to the Drupal team so it makes it to the official core.
Comment #19
Bèr Kessels CreditAttribution: Bèr Kessels commentedFor my module I am really in need of this too.
I have been pondering a lot about how to get by this. I see on option, and that is an extra argument to hook_auth. If the flag $server is set to false, then the hook needs to run for failed logins too. Else it will contain the server part.
some pseudo code
Comment #20
moshe weitzman CreditAttribution: moshe weitzman commented@ber - that sounds reasonable to me.
Comment #21
markus_petrux CreditAttribution: markus_petrux commentedI was also thinking about this (I'm working on an alternative phpBB module, where I need to use the phpBB user database).
I was about to propose a similar thing, but in the else part of the code:
Comment #22
Bèr Kessels CreditAttribution: Bèr Kessels commentedwow. that is a hell of a lot of nested ifs there :)
you probably want to look at module_invoke_all, or else do something in the lines of
It will break out as soon as there is a hit. while your code will still call all the rest of the modules.
PS, markus, the mysqlauth can do the atuthentication for you, you could decide to use that instead of copy-pasting it. After all the Drupal Way is to NRY (never repeat yourself) wherever possible.
Comment #23
markus_petrux CreditAttribution: markus_petrux commentedlol, that code is from user.module user_authenticate function, which is something that could probably be simplified as this:
Note that $server is optional in the else part of the big if.
I'll be sure to check the mysqlauth module, thanks :-)
Comment #24
markus_petrux CreditAttribution: markus_petrux commentedSo is there any chance to allow a contrib module to authenticate local users in 4.7?
I mean an authentification method that doesn't need to append @server to the user name. This is what the code I posted above would allow.
Comment #25
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedIf you'd post a patch it could be reviewed. :p
Comment #26
markus_petrux CreditAttribution: markus_petrux commentedok, here we go. First attempt :-)
Please, review attached patch. It is not exactly equal to the code I posted above. It has the following, IMO, benefits:
1) Simplied code, returning $user is user_load succeeds. No need to check for $user-uid later on.
2) Opimized foreach loop when checking for modules that implement hook_auth().
3) Check for variable_get('user_register', 1) has been removed, because IMO this option is for 'normal' user registration, authentication modules may still allow this kind of registrations.
4) When authmap is not used, the server part of the user name is optional. So the implementation of hook_auth can decide what to do.
Comment #27
markus_petrux CreditAttribution: markus_petrux commentedComment #28
naudefj CreditAttribution: naudefj commented+1
Hope it's not too late for 4.7.
Comment #29
markus_petrux CreditAttribution: markus_petrux commentedCan someone please confirm? This is hardly need to connect with local systems, such as phpBB (me works on it) and any other local "user provider".
Comment #30
naudefj CreditAttribution: naudefj commentedIt is absolutely required. Without it you need to either hack users.module or accept usernames like "user@localhost".
Comment #31
moshe weitzman CreditAttribution: moshe weitzman commentedi tested the patch and it works well. Also cleans up some verbose code.
However, when one has a DA module enabled and fails local login, then you can get errors from your DA module when it tries to connnect without any server being pased. note the url that is being called in the call below.
"_xmlrpc('http:///xmlrpc.php', 'drupal.login', 'moshe weitzman', '
')"
So I think we need some more defensiveness in all our DA moduls to handle being called without a server?
Also, please remove the quotes from the line below. I know that they were there before but they shouldn't be
$account = user_load(array('name' => "$name"));
Comment #32
markus_petrux CreditAttribution: markus_petrux commentedFixed.
The attached patch also modifies drupal.module::hook_auth() with a possible way to deal with this. It makes the $server argument optional, defaulting to FALSE (the return value of strrchr(), when $server is set in user_authenticate, if the server part is not present).
Comment #33
markus_petrux CreditAttribution: markus_petrux commentedSorry, forgot to change the issue status.
Comment #34
moshe weitzman CreditAttribution: moshe weitzman commentedthanks for quickly addressing those details. it is RTBC now, but you gave me an idea. instead of $server = FALSE, maybe drupal.module should add a setting such that logins would automatically go to another drupal site for authentication. in other words, let drupal.module take advantage of the feature we are enabling here. users shouldn't have to type in a server for drupal.module either.
sorry for the feature creep. makes sense though. will help implement networks of sites within an enterprise, for example.
Comment #35
markus_petrux CreditAttribution: markus_petrux commentedDo you mean a new textfield in the drupal settings panel, so admins can specify the _default_ server where they wish to accept DA?
That makes me think that the options under the "Receive data from other Drupal sites" fieldset may need a bit more love. I mean, there is actually no way to restrict from which sites one wishes to "receive data". This is a blank or white situation.
What about commiting the patch as it is and open a different issue to think about that? That would deal with drupal.module's ability to specify a _default_ server and/or options to possibly restrict those features to a white list of servers?
Comment #36
moshe weitzman CreditAttribution: moshe weitzman commentedOK, lets agree that this default drupal DA server will be a future feature. RTBC
Comment #37
moshe weitzman CreditAttribution: moshe weitzman commentedi have a use case for the 'default drupal DA server'. the upcoming http://groups.drupal.org site wants a login box that automatically sends login info to drupal.org site without user having to type in @drupal.org in username.
Comment #38
Bèr Kessels CreditAttribution: Bèr Kessels commentedI tested the patch in #32. And it works as advertised. It breaks no current implmentations, so far as I can see and can test (drupal dist auth works).
It will also make SQL auth module a lot easier. http://drupal.org/node/50946
A +1 for the patch
Comment #39
markus_petrux CreditAttribution: markus_petrux commentedChanging status, I'll try to implement moshe's suggestion (a simple textfield in the settings panel to propose a default server seems easily doable :-)... unless someone else feels otherwise...
Comment #40
markus_petrux CreditAttribution: markus_petrux commentedHere we go. The only difference in this patch is in changes made to drupal.module. A new option to specify the default authentication server (is the description correct?) and then, how it used in hook_auth.
Is it good enough?
Comment #41
markus_petrux CreditAttribution: markus_petrux commentedOptimized logic in hook_auth. better now?
Comment #42
markus_petrux CreditAttribution: markus_petrux commentedmoshe, does the patch cover what you meant with groups.drupal.org ?
Comment #43
markus_petrux CreditAttribution: markus_petrux commented1) I have just noticed something that needs attention. It is this line:
Nice, isn't it? However, a) $error is not used! b) If the password fails, the user is already warned and a record inserted to watchdog. So...
Could we remove that error message?
2) moshe: Now that we have a default server... does it worth to add an option so admins can say "allow DA, but only with this server", or something of that sort.
3) Is this something to include in 4.7? If so, please mark this as critical.
Comment #44
moshe weitzman CreditAttribution: moshe weitzman commented1: sure, remove the message
2: yes, good idea. doesn't have to hold up the patch though.
3: i emailed killes to see if he will take this for 4.7.
Comment #45
markus_petrux CreditAttribution: markus_petrux commentedSweet! Here we go... though, please review the latest changes.
Comment #46
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI talked to Steven about it and he thinks it's a 4.8 feature.
Comment #47
markus_petrux CreditAttribution: markus_petrux commentedDo you mean those changes in drupal.module or the whole thing?
PS: It would have been nicer to speak up before, so one do not spend their time trying to maintain a patch that is not going to be applied soon, it will be broken again and again ;)
Comment #48
moshe weitzman CreditAttribution: moshe weitzman commentedmarkus - drumm will apply this patch as soon as HEAD opens back up for 4.8. it should not "keep on breaking". i think killes and steven are calling current behavior "not a bug" which is a reasonable conclusion. also, there are OK workarounds posted in this thread. thanks much for your persistence here. this will get in soon, and we'll all rejoice.
Comment #49
naudefj CreditAttribution: naudefj commentedSince CVS is now open, can this patch please be committed ASAP?
Comment #50
drummThe settings page needs some work. The "Only default server" option is hard to understand. I think adding these settings justifies putting all three settings related to distributed authentication in their own group.
Comment #51
moshe weitzman CreditAttribution: moshe weitzman commented2 out of 3 hunks failed. needs refresh
Comment #52
naudefj CreditAttribution: naudefj commentedHere is the updated patch. Please review and commit ASAP.
Comment #53
naudefj CreditAttribution: naudefj commentedFix small bug in patch code + added if-statement to allow hook_auth to set a user's creation time.
Comment #54
moshe weitzman CreditAttribution: moshe weitzman commentedthanks for the persistance. i put this patch through some vigorous testing and it looks good to me.
reroll so it applies cleanly and two minor text improvements
Comment #55
Dries CreditAttribution: Dries commentedI don't understand the help text.
t('Only accept remote logins from the above specified default authentication server and not from any other server.')
I think we should be more verbose. Why would you want to set that setting? What if I want to accept authentication from a number of trusted partners?
In the documentation, the fully-qualified domain name examples are not fully-qualified. They are missing 'http://'.
Comment #56
moshe weitzman CreditAttribution: moshe weitzman commentedimproved the help texts as suggested. i am leaving the "multiple trusted partners" as a feature request. in my experience, the main use case is that the site owner has some external database/directory that is authoritative. this patch let's drupal site act as slave to that server and disallow all other authentication sources.
Comment #57
naudefj CreditAttribution: naudefj commentedLooks like some debugging code accidentally slipped in:
Can we remove it?
Comment #58
drummThe chunk with
+ if (!isset($array['created'])) { // Allow 'created' to be set by hook_auth
also scares me.
Comment #59
moshe weitzman CreditAttribution: moshe weitzman commentednew patch without debug code. sorry.
@drumm - no need for fear. the form api protects us from tampering like that. if you look back, user_save() had a lot of checks to prevent tampering like that but they were all removed with fapi. the calling function can already set status and password, for example. and we fear not because fapi will strip those fields if user attempts to post them.
Comment #60
naudefj CreditAttribution: naudefj commentedCan this patch please be committed to the 4.8 branch ASAP?
I'm already using it on my production site - the code is rock solid.
Comment #61
drummCommitted to HEAD.
Remember- asking for commit ASAP doesn't help, but more reviews do.
Comment #62
Bèr Kessels CreditAttribution: Bèr Kessels commentedmore review? there aer 62 followups, how much reviews should something get?
Comment #63
(not verified) CreditAttribution: commentedComment #64
tones CreditAttribution: tones commentedit appears this patch causes a small bug (unless i applied it wrong):
http://drupal.org/node/73760
Comment #65
tones CreditAttribution: tones commentedComment #66
darkblue CreditAttribution: darkblue commentedI'm trying to patch the user module on Windows. I downloaded the Unix utils from here:
http://unxutils.sourceforge.net/
then copied patch.exe, user.module and patch_47 in the sh.exe folder. Then I run this into sh.exe:
patch -p0
But I get this:
Assertion failed: hunk, file patch.c, line 321
Some problem with patch.exe probably? Any suggestions what to do? My Drupal version is 4.7.2
Comment #67
darkblue CreditAttribution: darkblue commentedThen I run this into sh.exe:
patch -p0 <patch_47 user.module
But I get this:
Assertion failed: hunk, file patch.c, line 321
Some problem with patch.exe probably? Any suggestions what to do? My Drupal version is 4.7.2.
Comment #68
asimmonds CreditAttribution: asimmonds commentedPlease don't change the issue title to something that is unrelated.
@darkblue, EOL encoding, look at http://drupal.org/node/60116
Comment #69
zoinks CreditAttribution: zoinks commentedI was directed to this node by the README install instructions for the FUDforum module. They say to "Apply patch at http://drupal.org/node/29147 (look for #59 patch_47)".
Two questions...
1) I'm using the latest Drupal (v 4.7.3). Has this patch already been worked into it?
2) If not, how does one implement the patch? Is it a matter of replacing all the code in "/modules/drupal.module" with the patch code? Or just certain parts?
Thanks in advance for any help!
Comment #70
naudefj CreditAttribution: naudefj commentedI'm using the latest Drupal (v 4.7.3). Has this patch already been worked into it?
No, it will be part of Drupal 5.0 when it is released.
If not, how does one implement the patch?
Best would be to wait or the new Drupal release. The patch is meant for developers that know how to do this kind of stuff.
Comment #71
zoinks CreditAttribution: zoinks commentedThanks for the response, Moshe. I'm sure you didn't mean it that way, but saying "The patch is meant for developers that know how to do this kind of stuff" seems like a bit of a brush-off. Despite how it may seem, I am very proficient with PHP code and with installing Drupal modules. As you can see I have the FUDforum installed already ( http://www.wesleycrushers.com/forum ) and would like to work it into my Drupal 4.7.3 site. Not asking for hand-holding; I'm just a bit unclear on the particulars of how to make the changes that your module entails, and a bit more documentation would be very helpful. Please?
Comment #72
moshe weitzman CreditAttribution: moshe weitzman commentedI didn't mean it any which way because I didn't write that. naudefj wrote the last followup. see the usernames next to each post.
The feature request called for here has been delivered. If anyone has support needs, please ask in the forum. do not reopen this issue for support questions.
Comment #73
zoinks CreditAttribution: zoinks commentedSorry Moshe, I misunderstood this to mean you and naudefj were one and the same...
Thanks for the feedback, I'll check the forums. In the meantime, maybe you could post any good threads that address my "newbie" question? I'm sure I'm not the only one wondering about this, and could be a good idea in the spirit of open-source and sharing and all... Thanks again!
Comment #74
brownas98 CreditAttribution: brownas98 commentedDid This actually get included in drupal 5?
I am trying to use the hook_auth function but it seems that is is not being called when a user tries to login....
Thanks
Comment #75
brownas98 CreditAttribution: brownas98 commentedBut the hook_auth function is called when I try to login using a username ending in "@domain.com"
Comment #76
kodiat CreditAttribution: kodiat commentedHi. I'm a newbie in Drupal. I use Drupal 5.1 and I checked both drupal.module and user.module, it seems patch mentioned in #59 have been implemented, however when I use hook_auth, it seems it is invoked only when I entered user@domain, otherwise if I entered user without domain, it is never being called. Did I miss something here? Thanks in advanced for the help.
Comment #77
kodiat CreditAttribution: kodiat commentedI found an interesting posting about behavior of hook_auth in Drupal 5.x
Simply said, hook_auth will be called when you do not use @domain in username as long as user IDs are not in the Drupal database already
Read this comment http://drupal.org/node/125626#comment-210855
Comment #78
moshe weitzman CreditAttribution: moshe weitzman commentedthere is a pref in drupal.module which is a 'default authentication domain' or somesuch. when that is set, you can skip the @example.com part.