Skip to content

Add unit tests to oauth2-core #4298

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
jgrandja opened this issue Apr 21, 2017 · 4 comments
Closed

Add unit tests to oauth2-core #4298

jgrandja opened this issue Apr 21, 2017 · 4 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Milestone

Comments

@jgrandja
Copy link
Contributor

Summary

Unit tests need to be added to the oauth2-core module.

@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Apr 21, 2017
@jgrandja jgrandja added this to the 5.0.0.M1 milestone Apr 21, 2017
@jgrandja jgrandja self-assigned this Apr 21, 2017
@jgrandja jgrandja modified the milestones: 5.0.0.M2, 5.0.0.M1 May 1, 2017
@rwinch rwinch modified the milestones: 5.0.0.M2, 5.0.0.M3 Jun 15, 2017
@jgrandja jgrandja modified the milestones: 5.0.0.M3, 5.0.0.M4 Jul 24, 2017
@luander
Copy link
Contributor

luander commented Jul 31, 2017

What is the expected behavior when the following code is called:

AuthorizationCodeTokenRequestAttributes
	.withCode("code")
//	.clientId(null)
	.redirectUri("http://redirect.uri/")
	.build();

Does it let the object be created without the clientId or should it fail with IllegalArgumentException?

@jgrandja
Copy link
Contributor Author

@luander The build() method should throw IllegalArgumentException given that clientId is a required attribute as per spec.

@luander
Copy link
Contributor

luander commented Aug 14, 2017

Thanks for the answer Joe. I was writing the tests for this and other classes and noticed the current behavior is not correct.

@jgrandja
Copy link
Contributor Author

@Launder Yes, the current behaviour is not correct hence the need for unit tests. Thanks for taking this on!

FYI, our current naming scheme for test methods is as follows:

methodNameWhen<Condition>Then<Expectation>

You can see examples of this in the following test classes:

AuthorizationCodeAuthenticationProcessingFilterTests
AuthorizationCodeRequestRedirectFilterTests

@jgrandja jgrandja mentioned this issue Aug 15, 2017
28 tasks
@rwinch rwinch modified the milestones: 5.0.0.M4, 5.0.0.M5 Sep 13, 2017
@rwinch rwinch modified the milestones: 5.0.0.M5, 5.0.0.RC1 Oct 3, 2017
@rwinch rwinch modified the milestones: 5.0.0.RC1, 5.0.0 Oct 30, 2017
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this issue Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Projects
None yet
Development

No branches or pull requests

3 participants