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

Force reconnect with credentials on DefaultTlsDirContextAuthenticationStrategy #432

Merged
merged 1 commit into from
Oct 6, 2017

Conversation

derTobsch
Copy link
Contributor

This could be a way to fix the problem of the issue #430. Another one would be to add the credentials before the TLS handshake or?

I am not completely sure if this is the right way, if it is I would provide a test. But the best would be that you think about it and let me know.

@rwinch
Copy link
Member

rwinch commented Dec 16, 2016

Thanks for the PR! I think before we do anything we probably need an example to see how to reproduce it. Any chance you could put a sample together?

@derTobsch
Copy link
Contributor Author

I hope I will get a example project by the end of the week.

@derTobsch
Copy link
Contributor Author

I made a example project and it shows the bug I reported before. I will upload it very soon. I will provide a readme so you can recreate it easily.

@derTobsch
Copy link
Contributor Author

The demo project is ready. Please have a look at https://github.com/derTobsch/tls-bug-demo

@derTobsch
Copy link
Contributor Author

good morning @rwinch have you seen my sample project? :-) Because the label is still on waiting-for-feedback

@Pitority
Copy link

When is this going to be merged?

@derTobsch
Copy link
Contributor Author

fyi: the ssl certificate in the ldap image does only live for 1 year :)

@rwinch
Copy link
Member

rwinch commented Sep 28, 2017

Thank you for the detailed report and sample.

The reason that any password works is because the LDAP sample is setup to allow Anonymous Bind. If you wish to use Anonymous Bind (not recommended since anyone can bind), you can switch from BindAuthenticator (currently being used) to PasswordComparisonAuthenticator by providing a PasswordEncoder in AuthenticationManagerConfiguration:

@Override
public void init(AuthenticationManagerBuilder auth) throws Exception {
    auth.ldapAuthentication()
        .passwordEncoder(NoOpPasswordEncoder.getInstance())
        .userSearchBase("ou=People")
        .userSearchFilter("cn={0}")
        .groupSearchBase("ou=Groups")
        .groupSearchFilter("member={0}")
        .contextSource(contextSource());
}

Alternatively, you need to disable Anonymous Bind within the LDAP server. At that point, authentication will fail with invalid credentials.

@rwinch rwinch closed this Sep 28, 2017
@rwinch rwinch reopened this Oct 6, 2017
@rwinch rwinch merged commit 08e8ae2 into spring-projects:master Oct 6, 2017
rwinch added a commit that referenced this pull request Oct 6, 2017
@rwinch
Copy link
Member

rwinch commented Oct 6, 2017

Thanks for the PR. This is now merged into master.

After additional evaluation it appears that some LDAP containers require additional interactions to ensure that bind authentication is performed. The reconnect ensures the bind is performed with the credentials and maintains the TLS connection.

I have merged this into master with an additional test. Thanks again for the PR!

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

3 participants