Repository navigation
fix(authz): keep a rotated refresh token valid for a short grace window CU-17tkuw62gnb #2919
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
Merged
+686
−2
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
74649d6
chore(authz): add retired refresh token storage (no behaviour yet) CU…
aliaska-varieva 24f6017
test: reproduce bug CU-17tkuw62gnb (failing)
aliaska-varieva 787ecee
fix(authz): keep a rotated refresh token valid for a short grace wind…
aliaska-varieva 1f1595a
refactor(authz): address review findings on the refresh grace window …
aliaska-varieva ca2f5d2
Merge branch 'main' into fix/CU-17tkuw62gnb
aliaska-varieva File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
231 changes: 231 additions & 0 deletions
231
...test/java/com/openframe/authz/service/auth/MongoAuthorizationServiceRefreshGraceTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| package com.openframe.authz.service.auth; | ||
|
|
||
| import com.openframe.data.document.oauth.MongoOAuth2Authorization; | ||
| import com.openframe.data.document.oauth.RetiredRefreshToken; | ||
| import com.openframe.data.repository.oauth.MongoOAuth2AuthorizationRepository; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
| import org.mockito.junit.jupiter.MockitoSettings; | ||
| import org.mockito.quality.Strictness; | ||
| import org.springframework.security.oauth2.core.AuthorizationGrantType; | ||
| import org.springframework.security.oauth2.core.ClientAuthenticationMethod; | ||
| import org.springframework.security.oauth2.core.OAuth2RefreshToken; | ||
| import org.springframework.security.oauth2.server.authorization.OAuth2Authorization; | ||
| import org.springframework.security.oauth2.server.authorization.OAuth2TokenType; | ||
| import org.springframework.security.oauth2.server.authorization.client.RegisteredClient; | ||
| import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository; | ||
| import org.springframework.test.util.ReflectionTestUtils; | ||
|
|
||
| import java.time.Instant; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.ArgumentMatchers.anyString; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| @MockitoSettings(strictness = Strictness.LENIENT) | ||
| class MongoAuthorizationServiceRefreshGraceTest { | ||
|
|
||
| private static final String ID = "authorization-1"; | ||
| private static final String PRINCIPAL = "user@example.com"; | ||
| private static final long GRACE_SECONDS = 60; | ||
|
|
||
| @Mock | ||
| private MongoOAuth2AuthorizationRepository repository; | ||
| @Mock | ||
| private RegisteredClientRepository clients; | ||
|
|
||
| private final Map<String, MongoOAuth2Authorization> store = new HashMap<>(); | ||
| private MongoAuthorizationService service; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| when(clients.findById(anyString())).thenReturn(RegisteredClient.withId("client-1") | ||
| .clientId("client") | ||
| .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) | ||
| .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) | ||
| .build()); | ||
| when(repository.save(any())).thenAnswer(call -> { | ||
| MongoOAuth2Authorization entity = call.getArgument(0); | ||
| store.put(entity.getId(), entity); | ||
| return entity; | ||
| }); | ||
| when(repository.findById(anyString())) | ||
| .thenAnswer(call -> Optional.ofNullable(store.get(call.<String>getArgument(0)))); | ||
| when(repository.findByRefreshTokenValue(anyString())).thenAnswer(call -> store.values().stream() | ||
| .filter(e -> call.<String>getArgument(0).equals(e.getRefreshTokenValue())) | ||
| .findFirst()); | ||
| when(repository.findByRetiredRefreshTokensValue(anyString())).thenAnswer(call -> store.values().stream() | ||
| .filter(e -> retiredValues().contains(call.<String>getArgument(0))) | ||
| .findFirst()); | ||
| when(repository.findByAccessTokenValue(anyString())).thenReturn(Optional.empty()); | ||
| when(repository.findByAuthorizationCodeValue(anyString())).thenReturn(Optional.empty()); | ||
| when(repository.findByState(anyString())).thenReturn(Optional.empty()); | ||
| when(repository.findAllByPrincipalNameIgnoreCaseAndRefreshTokenValueNotNull(PRINCIPAL)) | ||
| .thenAnswer(call -> new ArrayList<>(store.values())); | ||
|
|
||
| service = new MongoAuthorizationService(repository, clients); | ||
| ReflectionTestUtils.setField(service, "refreshTokenGraceSeconds", GRACE_SECONDS); | ||
| store.put(ID, authorizationWithRefreshToken("token-0")); | ||
| } | ||
|
|
||
| private static MongoOAuth2Authorization authorizationWithRefreshToken(String token) { | ||
| Instant now = Instant.now(); | ||
| return MongoOAuth2Authorization.builder() | ||
| .id(ID) | ||
| .registeredClientId("client-1") | ||
| .principalName(PRINCIPAL) | ||
| .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) | ||
| .refreshTokenValue(token) | ||
| .refreshTokenIssuedAt(now) | ||
| .refreshTokenExpiresAt(now.plus(7, ChronoUnit.DAYS)) | ||
| .refreshTokenMetadata(new HashMap<>()) | ||
| .build(); | ||
| } | ||
|
|
||
| private MongoOAuth2Authorization stored() { | ||
| return store.get(ID); | ||
| } | ||
|
|
||
| private List<String> retiredValues() { | ||
| List<RetiredRefreshToken> retired = stored().getRetiredRefreshTokens(); | ||
| return retired == null ? List.of() : retired.stream().map(RetiredRefreshToken::value).toList(); | ||
| } | ||
|
|
||
| private OAuth2Authorization find(String token) { | ||
| return service.findByToken(token, OAuth2TokenType.REFRESH_TOKEN); | ||
| } | ||
|
|
||
| private void rotate(String presented, String replacement) { | ||
| Instant now = Instant.now(); | ||
| service.save(OAuth2Authorization.from(find(presented)) | ||
| .token(new OAuth2RefreshToken(replacement, now, now.plus(7, ChronoUnit.DAYS))) | ||
| .build()); | ||
| } | ||
|
|
||
| private void retire(String value, long secondsAgo) { | ||
| stored().setRetiredRefreshTokens(new ArrayList<>(List.of( | ||
| new RetiredRefreshToken(value, Instant.now().minusSeconds(secondsAgo))))); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsARetiredTokenOnceTheGraceWindowHasPassed() { | ||
| retire("expired", GRACE_SECONDS + 1); | ||
|
|
||
| assertThat(find("expired")).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void acceptsARetiredTokenJustInsideTheGraceWindow() { | ||
| retire("recent", GRACE_SECONDS - 5); | ||
|
|
||
| assertThat(find("recent")).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsATokenThatWasNeverIssued() { | ||
| assertThat(find("never-issued")).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void aRetryThatRotatesAgainKeepsEveryRetiredTokenUsable() { | ||
| rotate("token-0", "token-1"); | ||
| rotate("token-0", "token-2"); | ||
|
|
||
| assertThat(stored().getRefreshTokenValue()).isEqualTo("token-2"); | ||
| assertThat(retiredValues()).containsExactlyInAnyOrder("token-0", "token-1"); | ||
| assertThat(find("token-0").getRefreshToken().isActive()).isTrue(); | ||
| assertThat(find("token-1").getRefreshToken().isActive()).isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| void aRetiredTokenCarriesItsOwnValueSoSpringRevocationCanLookItUp() { | ||
| rotate("token-0", "token-1"); | ||
|
|
||
| assertThat(find("token-0").getToken("token-0")).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void aRetiredTokenKeepsTheCurrentExpiry() { | ||
| rotate("token-0", "token-1"); | ||
|
|
||
| assertThat(find("token-0").getRefreshToken().getToken().getExpiresAt()) | ||
| .isEqualTo(stored().getRefreshTokenExpiresAt()); | ||
| } | ||
|
|
||
| @Test | ||
| void revokingTheSessionAlsoEndsItsRetiredTokens() { | ||
| rotate("token-0", "token-1"); | ||
|
|
||
| service.revokeAllForPrincipal(PRINCIPAL); | ||
|
|
||
| assertThat(find("token-0").getRefreshToken().isActive()).isFalse(); | ||
| } | ||
|
|
||
| @Test | ||
| void revokingThroughARetiredTokenLeavesNoActiveTokenBehind() { | ||
| rotate("token-0", "token-1"); | ||
| OAuth2Authorization byRetired = find("token-0"); | ||
|
|
||
| service.save(OAuth2Authorization.from(byRetired) | ||
| .token(byRetired.getToken("token-0").getToken(), | ||
| metadata -> metadata.put(OAuth2Authorization.Token.INVALIDATED_METADATA_NAME, true)) | ||
| .build()); | ||
|
|
||
| assertThat(find("token-0").getRefreshToken().isActive()).isFalse(); | ||
| assertThat(find("token-1").getRefreshToken().isActive()).isFalse(); | ||
| assertThat(retiredValues()).containsExactly("token-1"); | ||
| } | ||
|
|
||
| @Test | ||
| void savingWithoutRotatingRetiresNothingAndKeepsWhatIsRetired() { | ||
| retire("recent", 1); | ||
|
|
||
| service.save(find("token-0")); | ||
|
|
||
| assertThat(retiredValues()).containsExactly("recent"); | ||
| } | ||
|
|
||
| @Test | ||
| void savingDropsRetiredTokensPastTheirWindow() { | ||
| retire("expired", GRACE_SECONDS + 1); | ||
|
|
||
| rotate("token-0", "token-1"); | ||
|
|
||
| assertThat(retiredValues()).containsExactly("token-0"); | ||
| } | ||
|
|
||
| @Test | ||
| void savingWithNothingRetiredStoresNoList() { | ||
| service.save(find("token-0")); | ||
|
|
||
| assertThat(stored().getRetiredRefreshTokens()).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void savingANewAuthorizationStoresNoList() { | ||
| OAuth2Authorization existing = find("token-0"); | ||
| store.clear(); | ||
|
|
||
| service.save(existing); | ||
|
|
||
| assertThat(stored().getRetiredRefreshTokens()).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void anUntypedLookupAlsoResolvesARetiredToken() { | ||
| rotate("token-0", "token-1"); | ||
|
|
||
| assertThat(service.findByToken("token-0", null)).isNotNull(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.