NetTalk Central

Author Topic: NetLDAP bug in SetUser procedure  (Read 2200 times)

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
NetLDAP bug in SetUser procedure
« on: May 05, 2023, 12:54:54 PM »
Bruce,

There's a bug in the NetLDAP.SetUser() procedure when the user name is of the format DOMAIN\Jane.Bean

SetUser is called multiple times by NetLdap.UserInGroup().

The first time through, it correctly strips off the domain and backslash and returns
sAMAccountName=Jane.Bean

After the first time, however, it hits the ELSE clause and adds a backslash.
This results in
sAMAccountName=\Jane.Bean

And the group lookup fails.

There's a comment on line 624 where the extraneous backslash is injected.
I don't know when the change was made to NetLDAP, but was testing something yesterday and discovered that this is broken.
That backslash needs to be not inserted.

NT 12.59.

jf

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #1 on: May 05, 2023, 11:28:20 PM »
I was working on that area and made some changes mid last year.
I have a vague memory of there being problems if the user didn't use domain\name  but just name
in which case the \ is needed for the 2nd pass

In my case it was because the samaccount and upn were different names

I'll check but I think I'm using that code in production and it works.  So we may need to dig a bit

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #2 on: May 05, 2023, 11:30:55 PM »
Hmm I'm wrong.   my code is
  self.User = 'sAMAccountName=' & clip(self.User) ! clip(self.LoginDomain) &

Somehow that didn't make it through to Bruce

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #3 on: May 06, 2023, 04:53:55 PM »
Thanks for confirming, Sean.

Up until now, all our UPNs have been default - matching the Active Directory domain name.

We're confronting needing to change our AD logins to jane@theEmailDomain.com rather than jane@theADdomain.local for some kind of integrated SSO security system our IT is looking at adopting.

I've changed my UPN to the proposed domain name (jane@theEmailDomain.com) and done some experimenting.
On an existing NetTalk app, I was not able to log in and get group membership when using
jane
theADdomain\jane
jane@theEmailDomain.com

Problems I found:
1.  The backslash issue.  Thanks for confirming that that's not intended.  That was making theADdomain\jane fail.
2.  Login default -
   NetLDAP.Start sets self.DomainType = NetLDAP:UserPrincipalName
   So for a login that I (and most of our users) typically do with just the sAMAccountName
      jane
   NetLDAP.SetUser uses self.DomainType , which means I have become jane@theADdomain.local (rather than the correct UPN that is set in AD).
   NetLDAP.SetAuthUser ignores selfDomainType and hard codes it to NetLDAP:UserPrincipalName (i.e., jane@theADdomain.local)
   
   In the 20+ years I've worked on AD networks I haven't been part of one where the UPN didn't match the AD domain name until now.  (Lucky me.)
   What I've discovered about UPNs is
      a. The UPN specified for the user is primary.  (In my case, jane@theEmailDomain.com )
      b. BUT the default UPN (jane@theADdomain.local)
         WILL also work for user login
         WILL NOT work to look up group memberships or attributes
      c. So having NetLDAP set the UPN to jane@theADdomain.local for a simple 'jane' login bolloxes things.
         Basic login works because of the implicit UPN
         Group lookups do not work.
         
   For the plain 'jane' login I can compensate by putting
       net.domainType = NetLDAP:sAMAccountName
   before calling net.UserInGroup()

   (I think this problem of defaulting to NetLDAP:UserPrincipalName is actually my doing.  In discussing with Bruce years ago the various login names users can enter I think I suggested defaulting to creating a UPN.  Shame on me... sigh...)
   
3. NetLDAP.Search is out to get me.  (Actually, UserInGroup when pActiveDirectory is set to TRUE)
   The ActiveDirectory_UserInGroup procedure in the LDAP demo calls net.SetDomain()
   So when I use jane@theEmailDomain.com, net.SetDomain helpfully splits that into dc=theEmailDomain,dc=com
   This domain does not exist in Active Directory.

   If pActiveDirectory is TRUE, then what is produced from NetLDAP.UserInGroup() is
   pFilter=(&(objectClass=group)(member:1.2.840.113556.1.4.1941:=CN=Jane Fleming,OU=SysAdmin Users,OU=CHC SysAdmin,DC=theAdDomain,DC=local))  -- which is correct.

   BUT the pBase parm fed into NetLDAP.Search is dc=theEmailDomain,dc=com
      WHICH IS WRONG because that domain does not exist in AD.

   If I manually look up the DistinguishedName and set that before calling net.UserInGroup(), then the recursive call (which sets the incorrect domain name) doesn't happen and I'm good.
   
So I think I can code around all my issues in my calling procedures (except for the extraneous backslash - which I edited in NetLDAP.clw.)  I made a dedicated proc to get the DN before calling UserInGroup.
Or I might try just overriding NetLDAP.SetDomain so that it always uses the domain being used by AuthUser.

And I'm guessing that not a lot of people will be using a different UPN suffix in Active Directory situations.  But perhaps good to be aware of the challenges in case anybody else goes that route.

Thanks for your input, Sean!

Cheers,

Jane

      

   
 


         

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #4 on: May 07, 2023, 03:45:47 AM »
Thanks the the amazing detail Jane,

Quote
I think this problem of defaulting to NetLDAP:UserPrincipalName is actually my doing.  In discussing with Bruce years ago the various login names users can enter I think I suggested defaulting to creating a UPN.  Shame on me... sigh...)

LOL Not to worry. I was one of the first users of the LDAP code.  In fact I made a number of changes and additions. If you search a few of my comment made it in. So I'm an accomplice here, and I think I also championed upn as a default, it just seemed logical.  Unfortunately our strategy didn't last the confrontation with the enemy (client)

Quote
In the 20+ years I've worked on AD networks I haven't been part of one where the UPN didn't match the AD domain name until now.  (Lucky me.)

Yep, I will confirm you're the lucky one.  Took me a good few weeks to figure out what the problem was and and find a solution. so, again my code got added. Not sure how that \ made it through though. 

I'll have to digest the rest tomorrow

 

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #5 on: May 07, 2023, 09:31:49 AM »
Quote
I'll have to digest the rest tomorrow
Sorry for the long rant, Sean.

TL;DR

I think your client situation sounds like
sAMAccountName = jane
windows2000 = myAdDomain\jane
UPN = jfleming@myAdDomain.local  (Same domain, different user name)

whereas my new situation is
sAMAccountName = jane
windows2000 = myAdDomain\jane
UPN = jane@myEmailDomain.com  (Different domain that does not match an actual AD domain, same user name)

Quote
Yep, I will confirm you're the lucky one.
Yes, I'm truly special  ;D

Cheers,

Jane

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #6 on: May 07, 2023, 10:38:59 PM »
Long rants are better than short blow up with no info!

Yes, your assessment of my client is correct.  As for plain jane login, I found this in my code:

Quote
IF NOT INSTRING('\',parms.pAuthUser) AND NOT INSTRING('@',parms.pAuthUser)
            parms.pAuthUser = CLIP(Config:Logon:Domain) & '\' & parms.pAuthUser
END

Now that's very old and may not be needed as such anymore, but it works, so I'm not touching it!
Not sure about your upn of theEmailDomain that isn't in AD , That just seems very odd


Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #7 on: May 08, 2023, 09:01:03 AM »
Quote
IF NOT INSTRING('\',parms.pAuthUser) AND NOT INSTRING('@',parms.pAuthUser)
            parms.pAuthUser = CLIP(Config:Logon:Domain) & '\' & parms.pAuthUser
END

Now that's very old and may not be needed as such anymore, but it works, so I'm not touching it!
Where are you finding that code, Sean?
That is fine and solid - assuming that the Active Directory forest only has one domain (Config:Logon:Domain).  Which is the case with my client.
That's similar to line 592 in NetLDAP.SetAuthUser. 

The code that has changed - removing self.LoginDomain but leaving the extraneous backslash, is in NetLDAP.SetUser (not SetAuthUser).

The LdapDemo app checks group membership for pUser, not pAuthUser.  NetLDAP.UserInGroup uses self.user, not selfAuthUser.

My new situation may seem strange, but may become more common. 
The AD domain uses a .LOCAL suffix.  The client needs to switch user logins to an actual owned domain for some kind of AD-integrated cloud-based single-sign-on service for compatibility with third-party cloud-based apps.

I have figured out how to make this domain mismatch work in my situation.  (Multiple possibilities, listed below.)

But I don't want to code it that way if it might be better to fix NetLDAP.clw.  (In case this client is not the only one for whom the mixed-domain-name situation will ever show up).

Illustration:
In the first pic, I've set up my domain to support two  UPNs in addition to the domain default (which is beach.test).

The second pic shows running an attributes query in the LdapDemo app for Suzi@janefleming.com who is actually a user in the beach.test domain.

Third pic shows the UserInGroup call from the LdapDemo app.  On the first pass through, it correctly has the beach.test AD domain name.
But in order to fetch the distinguished name it resets self.domain by fetching the (bogus/arbitrary) domain suffix from the user being checked.

In the second part of the recursion, it uses that incorrect dc=janefleming,dc=com in the call to Active Directory and throws an error.  (Third pic).

I have multiple ways to fix it that work for me.
1.  Override NetLDAP.SetDomain so it returns based on Config:Logon:Domain.
2.  Make my own call to search attributes for the distinguished name BEFORE doing the InGroup.  That avoids the recursion which avoids the wrong domain name being set in NetLdap.SetDomain.
       This second choice also has some slight speed improvements in one of my apps that checks 5 different group memberships as part of the login.  I can code so I only need to look up the distinguished name once and use that for all 5 domain-membership lookups. 

Again, don't know whether anyone wants to modify NetLDAP for this.  I'm fine coding my own workarounds now that I understand the issue.

But the extraneous backslash in NetLDAP.SetUser is definitely a bug that isn't valid for anybody's situation, as far as I can tell.

Cheers,

Jane
« Last Edit: May 08, 2023, 09:18:29 AM by Jane »

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #8 on: May 08, 2023, 03:11:57 PM »
Quote
Where are you finding that code, Sean?
Sorry I didn't make it clear.  That 's code from the login procedure of my nettalk app.  I have a configuration item the client fills out for Config:Logon:Domain
So I do some pre-fiddling of the name before sending out for authorization.

Thanks for the additional info.  I can totally see the problem.  I think it could certainly be fixed in NetLdap, but not easily. the problem, as you say, is the multiple recursion that effectively rebuilds the upn.  There is an implied rule that the domain on the upn is always an AD domain. 
we'd have to save the first dn in a property perhaps to reuse?

I agree that the \ is a bug.

either way I'm happy to help if you need any

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #9 on: May 09, 2023, 01:59:11 PM »
Pre-fiddling is good, Sean.  Sometimes very good ;)

How much of the NetLdap code is you, Bruce, someone else?

Fortunately, all the methods are virtual so it's easy to override them in procedures.  I would find it useful to have one additional string added to LDAPParametersGroupType ( a string(500) for distinguished name) but can live without it.

But is it you or Bruce who vets this code?

I've got code now that so far seems to deal with the various permutations of the different-domain-UPN without breaking same-domain-UPN or plain name or domain\name formats.
My overrides are 
1.  NetLDAP.SetDomain  (still need to work on making this more generic)
2.  NetLDAP.SetUser (ditto)
3.  NetLDAP.SetAuthUser - I think this is now a generic replacement for the shipping procedure.

I'll post my stuff when I've massaged it a bit more - probably in a couple of days.

Again, thanks for the input.

Cheers,

Jane



seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #10 on: May 09, 2023, 04:50:29 PM »
Quote
How much of the NetLdap code is you, Bruce, someone else?

Mostly Bruce.  I added a number of methods, the get attributes one for instance, and I've fixed a few others.  Because I was one of the first I found a few missing bits and "would be nice to have"s, so I added them.   Most recently was the problem where samaccount and upn names differed. 

Quote
But is it you or Bruce who vets this code?

Absolutely Bruce, it all gets filtered through him. But since my dirty fingerprints are on some it I know it better than most.

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #11 on: May 11, 2023, 12:30:20 PM »
Thanks again, Sean.

I've been going through several possibilities:
1.  Just create my own derived methods in all apps
2.  Edit NetLDAP for my own use since it rarely changes
3.  See whether I can persuade Bruce to incorporate my NetLDAP changes, assuming they don't break anyone else's code. 

To explore the latter possibility, could you email me an address I could use to send my revisions for testing... to see whether they break your stuff?  I don't know whether Bruce would be OK with my posting his entire NetLDAP.clw file in a zip file on a public site like this.

Thanks,

Jane

seanh

  • Jr. Member
  • **
  • Posts: 87
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #12 on: May 11, 2023, 08:45:27 PM »
Done

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #13 on: May 11, 2023, 09:17:43 PM »
Didn't receive anything, Sean.  Just sent you a PM through this forum.

Jane

  • Sr. Member
  • ****
  • Posts: 350
  • Expert on nothing with opinions on everything.
    • View Profile
    • Email
Re: NetLDAP bug in SetUser procedure
« Reply #14 on: June 19, 2023, 12:23:17 PM »
Extraneous backslash error fixed in 12.62.

Alternate UPN suffix for login working in 12.62 LdapDemo app.  (Nice cleanup using equates for states in ValidateUser.)

Thanks, Bruce!  (And Sean)

Jane