Skip to content

Security cleanup #2475

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

Closed
wants to merge 2 commits into from
Closed

Conversation

rwinch
Copy link
Member

@rwinch rwinch commented Feb 10, 2015

My one concern is this seems like the changes I made may impact #1556 but I did not see any tests for that issue, so I'm not sure if I broke anything there.

cc @dsyer

@rwinch
Copy link
Member Author

rwinch commented Feb 18, 2015

@dsyer @philwebb I'd really like this to make the next milestone if possible. Any chance you can take a look?

PS: The build passed for me locally. It looks like Travis said it was taking too long and the build might be hung.

@dsyer dsyer modified the milestone: 1.2.2 Feb 18, 2015
@philwebb philwebb added type: blocker An issue that is blocking us from releasing and removed type: blocker An issue that is blocking us from releasing labels Feb 23, 2015
@philwebb philwebb removed this from the 1.2.2 milestone Feb 25, 2015
@philwebb
Copy link
Member

Looks like both commits in this PR have been merged already.

@mikemosseri
Copy link

This actually broke the ability to not use the default in memory auth. The change on this line from configure to init (e42fa79#diff-9fddf220548c41d170820f76c509875aR137).

This change causes auth.isConfigured to always be false even when there is another configuration. This is due to the AuthenticationManagerBuilder.performBuild not being called until init is complete. When the method was configure, the user defined security would be used and configured prior.

@dsyer
Copy link
Member

dsyer commented Mar 2, 2015

I'm still using custom user details in a bunch of projects that seem to work. Can you open a new issue, please, anyway, and attach a sample project if possible? The spring-boit-issues project is a good place to send sample projects as pull requests.

@mikemosseri
Copy link

Yes, will try to. Look at your startup in your projects. You will see that it now shows:

org.springframework.boot.autoconfigure.security.AuthenticationManagerConfiguration#147 - Using default security password: ad3ece3e-f4bd-4f3f-b71c-403bf510260d

@rwinch
Copy link
Member Author

rwinch commented Mar 2, 2015

@mikemosseri Thanks for the report. This sounds like a duplicate of #2567

EDIT I was able to reproduce it so that the logging occurred, but it appears that both the custom authentication and the generated credentials work. Can you please follow up on that ticket?

@mikemosseri
Copy link

@rwinch Yes, it is thanks. I couldn't find something similar earlier.

@rwinch
Copy link
Member Author

rwinch commented Mar 2, 2015

@michaelbrand Did you see my EDIT? Specifically can you follow up on that ticket as to if both custom and generated credentials work? If so, I can push my fixes and perhaps you can verify against the latest snapshot. Thanks!

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.

4 participants