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.

Sign up for GitHub

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

Jump to bottom

Test method in PasswordOAuth2AuthorizedClientProviderTests has incorrect setup of token expiry #9506

Closed
H-LREB opened this issue Mar 16, 2021 · 3 comments
Closed

Test method in PasswordOAuth2AuthorizedClientProviderTests has incorrect setup of token expiry #9506

H-LREB opened this issue Mar 16, 2021 · 3 comments
Assignees
@eleftherias
Labels
in: build An issue in the build type: enhancement A general enhancement
Milestone
5.5.0-RC1

Comments

@H-LREB
Copy link
Contributor

H-LREB commented Mar 16, 2021

I believe the test authorizeWhenPasswordAndAuthorizedAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize in class PasswordOAuth2AuthorizedClientProviderTests aims at testing the case when a token is not expired but discarded because of Clock Skew. Based on the comment, it seems the test has been added to verify the fix of gh-7511.

However, in the test, the token is already expired (regardless of Clock Skew) :

Instant expiresAt = now.minus(Duration.ofMinutes(1)); --> token expired 1 minute ago

I think it should be :

Instant expiresAt = now.plus(Duration.ofMinutes(1)); --> token expires in 1 minute
@H-LREB H-LREB added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 16, 2021
@eleftherias eleftherias self-assigned this Mar 17, 2021
@eleftherias
Copy link
Contributor

Thanks for the report @H-LREB!
Are you interested in submitting a Pull Request to fix the test?

@eleftherias eleftherias added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 17, 2021
@H-LREB
Copy link
Contributor Author

H-LREB commented Mar 17, 2021

Sure!
I will submit the PR later today.

H-LREB added a commit to H-LREB/spring-security that referenced this issue Mar 17, 2021
@H-LREB
Fix spring-projectsgh-9506
c1dfb9e 
Change test setup so that token is not expired yet.
@H-LREB H-LREB mentioned this issue Mar 17, 2021
Closed
@H-LREB
Copy link
Contributor Author

H-LREB commented Mar 17, 2021

I submitted pull request.

@eleftherias eleftherias added this to the 5.5.0-RC1 milestone Mar 17, 2021
@eleftherias eleftherias added the in: build An issue in the build label Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eleftherias eleftherias

Labels
in: build An issue in the build type: enhancement A general enhancement
Projects
None yet
Milestone
5.5.0-RC1
Development

No branches or pull requests
2 participants
@H-LREB @eleftherias