Request

A modification to the functionality introduced in #1334138: LDAP Authentication: Strip REMOTE_USER domain name so it works with IIS, which populates $_SERVER['REMOTE_USER'] as "domain\username".

Proposed Solution

The attached patch adds an attempt to split the username on a "\". I did it after the check for the "@" because I don't think both characters would be present in the username.

CommentFileSizeAuthor
#3 ldap-sso-stripdomain-1491244-3.patch1.06 KBsmsearcy
strip_iis_domain_name.patch601 bytessmsearcy

Comments

johnbarclay’s picture

johnbarclay commented

thanks. this looks good to me. But would the following work and can you test it:

      if ($auth_conf->ssoRemoteUserStripDomainName) {
        $domain = NULL;
        $exploded = preg_split("/[\@\\]/", $remote_user);
        if (count($exploded) == 2) {
          $domain = $exploded[0][0];
          $remote_user = $exploded[1][0];
        }
     }

Its just slightly cleaner and a preg_replace might even be a one liner.

smsearcy’s picture

smsearcy commented

The issue I see is that the order of domain and username depends on which character $remote_user is split on, i.e. user@domain vs. domain\user.

So I think it would need to be something like this (haven't tested it yet, but I will):

      if ($auth_conf->ssoRemoteUserStripDomainName) {
        $domain = NULL;
        $exploded = preg_split("/[\@\\]/", $remote_user);
        if (count($exploded) == 2) {
          if (strpos($remote_user, '@') !== FALSE) {
            // user@domain
            $remote_user = $exploded[0][0];
            $domain = $exploded[1][0];
          else {
            // domain\user
            $domain = $exploded[0][0];
            $remote_user = $exploded[1][0];
          }
        }
     }

I'm open to better ways to write that, or maybe I'll think of something else while testing.

smsearcy’s picture

smsearcy commented
StatusFileSize
new ldap-sso-stripdomain-1491244-3.patch1.06 KB

Ok, here's some working code (patch attached). However, looking at the context of the code I have a new question, because in the prior "case 'mod_auth_kerb':" section, it strips out the domain/realm automatically. For consistency, maybe this code should go into the "case 'mod_auth_sspi':" section? Or the realm should always be stripped out and it was an oversight that it wasn't in SSPI authentication?

Either way, I think this functionally works (at least with "domain\user", I don't have the ability to test "user@domain").

    if ($remote_user) {
      if ($auth_conf->ssoRemoteUserStripDomainName) {
        $domain = NULL;
        $exploded = preg_split('/[\@\\\\]/', $remote_user);
        if (count($exploded) == 2) {
          if (strpos($remote_user, '@') !== FALSE) {
            $remote_user = $exploded[0];
            $domain = $exploded[1];
          } 
          else {
            $domain = $exploded[0];
            $remote_user = $exploded[1];
          }
        }
      }
johnbarclay’s picture

johnbarclay commented
Status: Active » Reviewed & tested by the community

looks good to me. I'll apply this as soon as I get back from drupalcon.

johnbarclay’s picture

johnbarclay commented
Component: Code » SimpleTests
Status: Reviewed & tested by the community » Fixed

Thanks. I committed this to 7.x-1.x-dev. Can you double check it when you get a chance as well as someone who uses the user@domain format. I'm changing this to be of component simpletest so I don't forget to add testing.

smsearcy’s picture

smsearcy commented

I verified that the current development snapshot works for me on IIS with usernames in the format of domain\user.

Thank you for your work on this project.

johnbarclay’s picture

johnbarclay commented

Thanks. appreciate the testing and follow through.

Status: Fixed » Closed (fixed)

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

jrsinclair’s picture

jrsinclair commented

This has been a big help. Many thanks.