fix: Move refresh rotate check to refresh flow (#2437)

Add client_auth_method to access token always but allow refresh (without secret) only if token before was client_auth=none and client has rotate=true for refresh tokens
cloudfoundry · Aug 17, 2023 · 4021783 · 4021783
1 parent d6973c2
commit 4021783
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 9 deletions.
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java
@@ -318,18 +318,19 @@ private static String getAuthenticationMethod(OAuth2Request oAuth2Request) {
         return ofNullable(oAuth2Request.getExtensions().get(CLIENT_AUTH_METHOD)).map(String.class::cast).orElse(null);
     }
 
-    private static void addAuthenticationMethod(Claims claims, Map<String, Object> additionalRootClaims, UserAuthenticationData authenticationData) {
+    private void addAuthenticationMethod(Claims claims, Map<String, Object> additionalRootClaims, UserAuthenticationData authenticationData) {
         if (authenticationData.clientAuth != null && CLIENT_AUTH_NONE.equals(authenticationData.clientAuth)) {
-            // public refresh flow, allowed if access_token before was also without authentication (claim: client_auth_method=none)
-            if (!CLIENT_AUTH_NONE.equals(claims.getClientAuth())) {
+            // public refresh flow, allowed if access_token before was also without authentication (claim: client_auth_method=none) and refresh token is one time use (rotate it in refresh)
+            if (refreshTokenCreator.shouldRotateRefreshTokens() && CLIENT_AUTH_NONE.equals(claims.getClientAuth())) {
+                addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth);
+            } else {
                 throw new TokenRevokedException("Refresh without client authentication not allowed.");
             }
-            addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, authenticationData.clientAuth);
         }
     }
 
-    private Map<String, Object> addAuthenticationMethod(Map<String, Object> additionalRootClaims, String clientAuthentication) {
-        if (clientAuthentication != null && refreshTokenCreator.shouldRotateRefreshTokens()) {
+    private static Map<String, Object> addAuthenticationMethod(Map<String, Object> additionalRootClaims, String clientAuthentication) {
+        if (clientAuthentication != null) {
             additionalRootClaims = addRootClaimEntry(additionalRootClaims, CLIENT_AUTH_METHOD, clientAuthentication);
         }
         return additionalRootClaims;

diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/RefreshRotationTest.java
@@ -40,7 +40,6 @@
 import static org.hamcrest.Matchers.hasEntry;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -155,7 +154,7 @@ void testRefreshPublicClientWithoutRotation() {
     OAuth2Authentication authentication = new OAuth2Authentication(oAuth2Request, tokenSupport.defaultUserAuthentication);
     CompositeToken accessToken = (CompositeToken) tokenServices.createAccessToken(authentication);
 
-    assertNull(UaaTokenUtils.getClaims(accessToken.getValue()).get(CLIENT_AUTH_METHOD));
+    assertThat(UaaTokenUtils.getClaims(accessToken.getValue()), hasEntry(CLIENT_AUTH_METHOD, CLIENT_AUTH_NONE));
     String refreshTokenValue = accessToken.getRefreshToken().getValue();
     assertThat(refreshTokenValue, is(notNullValue()));
 

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java
@@ -255,7 +255,7 @@ void ensureJKUHeaderIsSetWhenBuildingARefreshToken() {
     @Nested
     @DisplayName("when performing the refresh grant type")
     @DefaultTestContext
-    @TestPropertySource(properties = {"uaa.url=https://uaa.some.test.domain.com:555/uaa"})
+    @TestPropertySource(properties = {"uaa.url=https://uaa.some.test.domain.com:555/uaa", "jwt.token.refresh.rotate=true"})
     @DirtiesContext
     class WhenRefreshGrant {
         @Autowired