Multi-domain logins for Windows Desktop #51275
Conversation
| @@ -591,6 +594,7 @@ func (s *WindowsService) initializeLDAP() error { | |||
| } | |||
|
|
|||
| conn.SetTimeout(ldapRequestTimeout) | |||
| s.lc.SetConnectionCreator(createLDAPConnection) | |||
There was a problem hiding this comment.
Why do we need this connection creator thing? (Instead of just using ldap.Dial directly)
There was a problem hiding this comment.
We set up TLS here but we create new connection inside ldap.ReadWithFilter (when we get new referral to check in the response). This way we have factory for these connection with certificates and so on
There was a problem hiding this comment.
AFAICT it's called only in WindowsService.initializeLDAP, so is SetClient. Feels like s.lc could be created here instead and then we could avoid those setters and mutex. LDAPConfig is available in s.cfg.LDAPConfig anyway. Chances are I'm missing something here.
| @@ -244,6 +244,8 @@ require ( | |||
| software.sslmate.com/src/go-pkcs12 v0.5.0 | |||
| ) | |||
|
|
|||
| require github.com/go-asn1-ber/asn1-ber v1.5.7 | |||
There was a problem hiding this comment.
Perhaps should be included in the require block above.
| var ldapErr *ldap.Error | ||
| if !errors.As(err, &ldapErr) { | ||
| return nil, trace.Wrap(err, "fetching LDAP object %q with filter %q", dn, filter) | ||
| } |
There was a problem hiding this comment.
Maybe it would be cleaner to do this check inside extractReferrals, so we'd avoid writing the error string twice.
| @@ -327,7 +333,7 @@ var EnhancedKeyUsageExtension = pkix.Extension{ | |||
| } | |||
|
|
|||
| // SubjectAltNameExtension fills in the SAN for a Windows certificate | |||
| func SubjectAltNameExtension(user, domain string) (pkix.Extension, error) { | |||
| func SubjectAltNameExtension(userPrincipalName string) (pkix.Extension, error) { | |||
There was a problem hiding this comment.
Any reason this isn't private?
| @@ -591,6 +594,7 @@ func (s *WindowsService) initializeLDAP() error { | |||
| } | |||
|
|
|||
| conn.SetTimeout(ldapRequestTimeout) | |||
| s.lc.SetConnectionCreator(createLDAPConnection) | |||
There was a problem hiding this comment.
AFAICT it's called only in WindowsService.initializeLDAP, so is SetClient. Feels like s.lc could be created here instead and then we could avoid those setters and mutex. LDAPConfig is available in s.cfg.LDAPConfig anyway. Chances are I'm missing something here.
This change allows users to login to Windows desktop using multiple different domains as long as the trust is setup between them and LDAP returns proper referrals for the other domain.
If you have
domain1configured in Teleport anddomain2thatdomain1trusts then loggins asuser1will be treated asuser1@domain1, anduser1@domain1anduser2@domain2will use users from respective domains (and still log you in to the requested desktop in thedomain1).changelog: Add multi-domain logins for Windows Desktop