Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kerberos support for LDAP connector #564

Merged
merged 9 commits into from Apr 28, 2020

Conversation

bhunut-adobe
Copy link
Collaborator

@bhunut-adobe bhunut-adobe commented Feb 7, 2020

Added Kerberos Authentication support for Windows #565
This enabled UST to no longer required username and password in configuration file.
UST will automatically use OS authenticated user.
LDAP Channel Binding is supported.

set the following in connector-ldap.yml
authentication_method: kerberos

Implemented Require_TLS_Cert. #566
This allow ldap (389) to have encrypted traffic over TLS.
This integrates with the system wide Certification Authorities. No need to specify CA cert path.

set the following in connector-ldap.yml
require_tls_cert: True

@adorton-adobe
Copy link
Collaborator

Before I review, can we get the build passing? I believe the dependency issue that Danimae fixed in #561 is causing the failure. I've merged that PR, so if you merge or rebase v2 to your branch the build should work.

@adorton-adobe
Copy link
Collaborator

@bhunut-adobe FYI - I just merged another build-related PR, so make sure you pull v2 again before you rebase/merge.

@adorton-adobe
Copy link
Collaborator

@bhunut-adobe is this ready for review?

@bajwa-adobe
Copy link
Contributor

bajwa-adobe commented Feb 20, 2020 via email

@bhunut-adobe
Copy link
Collaborator Author

@adorton-adobe Ok i fixed my rebase mess. It is ready now for review

@bhunut-adobe
Copy link
Collaborator Author

bhunut-adobe commented Feb 20, 2020

@bajwa-adobe It is not common to use 389 + TLS. I just added the support incase someone actually use it. For Kerberos support, the only requirement are Windows server joined to AD Domain and running UST with a domain user not local account.

Copy link
Collaborator

@adorton-adobe adorton-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. I'll need to test it before I can approve it. I have a couple of comments and one thing to change.

user_sync/connector/directory_ldap.py Show resolved Hide resolved
@johndoenulc
Copy link

Hi

any update on when this will be added to sync tool?

@adorton-adobe adorton-adobe added this to the v2.6.0 milestone Mar 27, 2020
@adorton-adobe
Copy link
Collaborator

adorton-adobe commented Apr 8, 2020

I think we need to generate an error when username and/or password are specified when kerberos is enabled. Right now when I specify both of them, I get no error and the connector uses Kerberos to connect. When I specify one but not the other, I get errors that could be confusing.

With username but no password: ldap configuration: must contain setting for "password" or "secure_password_key"

With password but no username: Found unused keys: ['password'] in: ldap configuration

Here's what we need to do - when kerberos is not enabled, then we report errors in the manner we currently report them. When kerberos is enabled, we should report an error along the lines of 'password' and 'username' are not allowed when 'authentication_method' is 'kerberos'.

@bhunut-adobe
Copy link
Collaborator Author

@adorton-adobe I made the change so it raise an exception when those two fields are specified

@adorton-adobe adorton-adobe merged commit 876d2de into adobe-apiplatform:v2 Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants