Update for 6.x

cybertron1 - January 4, 2008 - 07:46
Project:Webserver authentication
Version:HEAD
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

Well, I am trying to make an update for 6.x, sofar not much has been changed actually, it works quite ok (mostly the info file that has been changed) although I am having trouble getting the admin interface working. I have the link working as it should (i belive) but the form itself doesn't show up.

I will try and fix this and if anyone knows what wrong, please help.

/CyberTron

AttachmentSize
webserver_auth.zip13.63 KB

#1

cybertron1 - January 8, 2008 - 05:40

Well, I have finally implemented the last parts of getting it to work. It is probably not the best work securitywise or like that, but it works and that is all I need. Configuration works also.

I have not tested this on a complete new system (only with my own backup site) so I take no responsibility what so ever!

have fun

AttachmentSize
webserver_auth.zip 13.68 KB

#2

moshe weitzman - March 10, 2008 - 21:19
Status:active» needs work

pleae attach individual files. lets find a patch that is better than "not best security wise".

#3

moshe weitzman - March 10, 2008 - 21:45

HEAD now has a version suitable for D5 so this needs to rolled from HEAD again. Note that we have a hook_menu in this module now.

#4

Paul Kishimoto - June 13, 2008 - 20:34

@Moshe: do you mean "D6" in #3?

The patch inside the .zip #1 contains a hook_menu, which as Moshe says is now in HEAD, so that can probably be removed. What is the effect of the other changes supposed to be?

#5

Paul Kishimoto - July 7, 2008 - 21:49

OK, posting a patch here because this issue seems to have been posted earliest. Also relevant are:

The attached patch:

  • Adds core = 6.x to the .info file
  • Adds webserver_auth.install which modifies the {system} table to ensure the webserver_auth hooks are always run after user.module hooks. This was found to be necessary for the bits below...
  • Modifies webserver_auth_menu() to (I hope) match 6.x conventions.
  • Modifies webserver_auth_init() extensively:
    • Plugs a 'security hole' in mod_auth_kerb or PHP (not sure which) that exposes plaintext passwords in $_SERVER[].
    • No longer calls drupal_load(), because the {system} modification guarantees user.module will be loaded when this function is called.
    • Calls the new D6 function user_external_login_register(), which I guess was introduced to help with OpenID.
  • Removes webserver_auth_webserver_auth(), and moves most of its functionality to webserver_auth_user(), which:
    • Cleans up NTLM/Negotiate usernames, as before, but only if the appropriate settings are set (they are set by default).
    • Autogenerates an e-mail address from the "post-strip" username if the appropriate variable is set.
    • Runs some custom code (see below, #1) to modify user accounts at the time of creation.
    • Sends user (see below, #2) to a non-SSL domain if logging out.
  • Modifies webserver_auth_settings():
    • Rename webserver_auth_domain to webserver_auth_email_domain.
    • Add webserver_auth_strip_prefix.
    • Add webserver_auth_strip_domain.
    • Add webserver_auth_insert.

Notes

  1. I have used this patch with some success in a production environment. I know that doesn't imply that the code is good. My environment is an Apache 2 server with PHP 5; we have access to a Kerberos server and an LDAP server. mod_auth_kerb is used to authenticate users to SSL services on Apache. This means a user visits our site, sees the usual HTTP Basic Auth dialog box, enters his/her credentials, which are referred to the Kerberos server. I have entered the following PHP code for the variable webserver_auth_insert:
    <?php
     
    // connect to LDAP
     
    $ds = ldap_connect('ldap.example.com');
     
    ldap_bind($ds, 'cn=myname,ou=users,dc=ldap,dc=example,dc=com', 'password');
     
    // look up user by USERid
     
    $search = ldap_search($ds, 'dc=ldap,dc=example,dc=com', 'userid='. $account->name);
     
    $entry = ldap_first_entry($ds, $search);
     
    $attr = ldap_get_attributes($ds, $entry);
     
    ldap_unbind($ds);
     
    // process canonical (full) name
     
    if (array_key_exists('cn', $attr) && array_key_exists(0, $attr['cn']) && !empty($attr['cn'][0])) {
       
    // update account object for further processing
       
    $account->name = str_replace('  ', ' ', $attr['cn'][0]);
       
    // save user name
       
    db_query("UPDATE {users} SET name = '%s' WHERE uid = %d", $account->name, $account->uid);
      }
    ?>

    The effect of this is that the user's canonical (full) name is retrieved from the LDAP directory and set as their Drupal username, which looks much better than their Kerberos identifier (even with the domain stripped).
  2. This is necessary because the Kerberos session isn't ended when the Drupal session is. If the user is redirected back to a HTTPS URL, Apache tells PHP/Drupal the secure session is still valid (it is!) and Drupal happily logs the user back in. The Drupal installation is available both inside and outside of SSL (i.e. at the same URLs with either the "http://" and "https://" protocol prefixes.), so by sending the just-logged-out user to the non-secure site, they are not automatically logged back in.

The only major difference the behaviour of this module is that it uses a completely un-mangled authname in the {authmap} table, and only changes the username in the {users} table. Prior to writing this patch I didn't know that {authmap} existed, hence my naive comments on some other issues. Also, the 'webserver_auth' hooks have disappeared in terms of a custom-PHP-from-the-user-interface-for-processing model, which is used elsewhere in Drupal and contributed modules (Blocks, Views, etc.)

Since this is more or less a total rewrite, I could submit separate patches but they would need to be applied simultaneously. I hope my explanations above serve just as well in making the code understandable.

Anyway, I would appreciate some eyes on the code, and maybe some testing (especially by NTLM users, since I don't have such a setup available for testing) and perhaps if it's alright even a 6.x development branch of the module which would be so so so wonderful (please?!).

AttachmentSize
webserver_auth-6.x.patch 10.16 KB

#6

moshe weitzman - July 17, 2008 - 20:47
Status:needs work» fixed

I have committed this patch to HEAD. I made some small changes. The only significant one is that I changed the username/email munging to happen during hook_user('submit') instead of 'insert'. That way, we don't have to UPDATE directly into users table. Also, all modules which listen on hook_user() get passed the correct username. Please test this. I actually coded this blind since I am not currently setup to test this module.

I made a DRUPAL-5 branch and finally a full D5 release. I also made a DRUPAL-6--1 branch and removed all the files from HEAD branch since thats going unused for a while.

Thanks for this nice patch.

#7

Anonymous (not verified) - July 31, 2008 - 20:55
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#8

Paul Kishimoto - August 6, 2008 - 17:57

@moshe: I just tried the 6.x-dev code, but the change to hook_user('submit') doesn't appear to be working. I had used 'insert' instead of 'submit' because user_module_invoke('insert', ...) is called during user_save(), right in the middle of the creation of the new account. The direct access to the database is messy, but I figured it was sanctioned when I read the following in the API docs for hook_user():

$op What kind of action is being performed. Possible values (in alphabetical order):

  • ...
  • "insert": The user account is being added. The module should save its custom additions to the user object into the database and set the saved fields to NULL in $edit.
  • ...

I can't find a user_module_invoke('submit', ...) call anywhere in core except in functions like user_edit_submit(), which appear only to be triggered on a form submission...not really what we want.

For now I continue to run the code in my patch above; if you need me to reroll a patch against 6.x-dev, please let me know.

#9

moshe weitzman - August 6, 2008 - 20:24
Status:closed» active

paul - you are correct. please do reroll as you had it (no submit hook) and i will commit.

#10

Paul Kishimoto - August 7, 2008 - 15:33
Status:active» reviewed & tested by the community

Reroll.

AttachmentSize
webserver_auth-6.x.patch 3.87 KB

#11

moshe weitzman - August 7, 2008 - 16:40
Status:reviewed & tested by the community» fixed

Committed. I'm issueing a release as well (finally). Thanks a ton.

#12

mecvo1984 - August 7, 2008 - 22:12

I'm trying to implement these patches but what are all the + and - and @ symbols? These cause errors in execution. Is there any way to get clean copies (cut & paste) versions? Thanks much.

#13

Paul Kishimoto - August 10, 2008 - 04:46

From http://drupal.org/patch: "patches describe the changes between a before and after state." If you only need the "after" state, you can download the 6.x-1.0 release.

#14

mecvo1984 - August 10, 2008 - 15:29

Thanks for this, can't believe I missed it. All is well now.

#15

Grantovich - August 12, 2008 - 19:36

Question on this new version: How hard would it be to backport to Drupal 5 and create a 5.x-1.0 release? I am interested in the new fixes and features here (especially the LDAP lookup scheme described in #5), but since some modules I'm using still aren't available for Drupal 6, upgrading is not yet a possibility.

#16

Anonymous (not verified) - August 26, 2008 - 19:42
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.