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

Added default setter for authentication #382

Merged
merged 3 commits into from
Sep 10, 2016
Merged

Conversation

unaor
Copy link
Contributor

@unaor unaor commented Aug 16, 2016

In case that user wants to just add handlers but not to provide a custom authentication ,the auth object is not saved
this will fix it

@hazendaz
Copy link
Member

@unaor Should spring 3 module have this same change? If so, can you add that and push it up as well?

@dblock
Copy link
Collaborator

dblock commented Aug 18, 2016

This definitely needs tests and a CHANGELOG entry, please.

@unaor
Copy link
Contributor Author

unaor commented Aug 18, 2016

@hazendaz yes i just looked, that module required the same change
@dblock ill update the CHANGELOG , about test do you currently have a similar test that i should update with this new scenario?

@dblock
Copy link
Collaborator

dblock commented Aug 19, 2016

We're missing a test for this Delegating* thing in https://github.com/dblock/waffle/tree/master/Source/JNA/waffle-spring-security3/src/test/java/waffle/spring, please add one.

@unaor
Copy link
Contributor Author

unaor commented Aug 19, 2016

@dblock I added a basic test that checks that the custom handler is setted correctly and another test to make sure that Authentication is not null (that was original bug).
I wanted to write test to show that the custom handler is actually being called (handler adds dummy header) but i might need some help with this as we need the mock request to be deauthorized

@dblock
Copy link
Collaborator

dblock commented Aug 19, 2016

That looks better, I will leave it to @hazendaz from here.

@hazendaz
Copy link
Member

@unaor Can we get the tests in spring3 too? Looks like you added it for only spring4. I'll merge right after. Also wondered if you were still planning on providing documentation for how you got this up and running in spring boot as well. I find that very useful info.

@hazendaz hazendaz merged commit 1dc668a into Waffle:master Sep 10, 2016
@hazendaz
Copy link
Member

I'll get the tests in spring 3 shortly.

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