fix: always remove AUTHORIZATION before policyChain.doNext

Previously, in the case where the request was correctly cached, the handleSuccess was called first, which executed the chain without deleting the header. And at the end the header is removed. So it wasn't possible to overload the header with a transform-header for example. Now the header is removed before the chain is executed.
gravitee-io · Oct 6, 2023 · fb1c3d7 · fb1c3d7
1 parent aeeea08
commit fb1c3d7
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 4 deletions.
diff --git a/src/main/java/io/gravitee/policy/v3/oauth2/Oauth2PolicyV3.java b/src/main/java/io/gravitee/policy/v3/oauth2/Oauth2PolicyV3.java
@@ -142,10 +142,6 @@ public void onRequest(Request request, Response response, ExecutionContext execu
             // Validate access token
             oauth2.introspect(accessToken, handleResponse(policyChain, request, response, executionContext, null));
         }
-
-        if (!oAuth2PolicyConfiguration.isPropagateAuthHeader()) {
-            request.headers().remove(HttpHeaderNames.AUTHORIZATION);
-        }
     }
 
     Handler<OAuth2Response> handleResponse(
@@ -255,6 +251,10 @@ private void handleSuccess(
             cacheResource.getCache(executionContext).put(element);
         }
 
+        if (!oAuth2PolicyConfiguration.isPropagateAuthHeader()) {
+            request.headers().remove(HttpHeaderNames.AUTHORIZATION);
+        }
+
         // Continue chaining
         policyChain.doNext(request, response);
     }

diff --git a/src/test/java/io/gravitee/policy/v3/oauth2/Oauth2PolicyV3Test.java b/src/test/java/io/gravitee/policy/v3/oauth2/Oauth2PolicyV3Test.java
@@ -15,6 +15,7 @@
  */
 package io.gravitee.policy.v3.oauth2;
 
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 import static org.mockito.Mockito.argThat;
@@ -426,6 +427,7 @@ void shouldFail_goodIntrospection_noClientId() throws IOException {
         HttpHeaders httpHeaders = mock(HttpHeaders.class);
 
         Oauth2PolicyV3 policy = new Oauth2PolicyV3(oAuth2PolicyConfiguration);
+        when(oAuth2PolicyConfiguration.isPropagateAuthHeader()).thenReturn(true);
         when(oAuth2PolicyConfiguration.getOauthResource()).thenReturn("oauth2");
         when(mockExecutionContext.getComponent(ResourceManager.class)).thenReturn(resourceManager);
         when(resourceManager.getResource(oAuth2PolicyConfiguration.getOauthResource(), OAuth2Resource.class))
@@ -443,6 +445,7 @@ void shouldFail_goodIntrospection_noClientId() throws IOException {
 
     @Test
     void shouldValidate_goodIntrospection_withClientId() throws IOException {
+        when(oAuth2PolicyConfiguration.isPropagateAuthHeader()).thenReturn(true);
         when(oAuth2PolicyConfiguration.isExtractPayload()).thenReturn(true);
 
         Oauth2PolicyV3 policy = new Oauth2PolicyV3(oAuth2PolicyConfiguration);
@@ -475,6 +478,7 @@ public boolean matches(List<String> scopes) {
 
     @Test
     void shouldValidate_goodIntrospection_withClientId_validScopes() throws IOException {
+        when(oAuth2PolicyConfiguration.isPropagateAuthHeader()).thenReturn(true);
         when(oAuth2PolicyConfiguration.isCheckRequiredScopes()).thenReturn(true);
         when(mockExecutionContext.getComponent(ResourceManager.class)).thenReturn(resourceManager);
         when(resourceManager.getResource(oAuth2PolicyConfiguration.getOauthResource(), OAuth2Resource.class))
@@ -493,6 +497,7 @@ void shouldValidate_goodIntrospection_withClientId_validScopes() throws IOExcept
 
     @Test
     void shouldValidate_goodIntrospection_withCache() throws IOException {
+        when(oAuth2PolicyConfiguration.isPropagateAuthHeader()).thenReturn(true);
         when(oAuth2PolicyConfiguration.isCheckRequiredScopes()).thenReturn(true);
         when(mockExecutionContext.getComponent(ResourceManager.class)).thenReturn(resourceManager);
         when(resourceManager.getResource(oAuth2PolicyConfiguration.getOauthResource(), OAuth2Resource.class))
@@ -519,6 +524,38 @@ void shouldValidate_goodIntrospection_withCache() throws IOException {
         verify(cache).put(any(Element.class));
     }
 
+    @Test
+    void shouldValidate_goodIntrospection_withCache_without_auth_propagation() throws IOException {
+        when(oAuth2PolicyConfiguration.isPropagateAuthHeader()).thenReturn(false);
+        when(oAuth2PolicyConfiguration.isCheckRequiredScopes()).thenReturn(true);
+        when(mockExecutionContext.getComponent(ResourceManager.class)).thenReturn(resourceManager);
+        when(resourceManager.getResource(oAuth2PolicyConfiguration.getOauthResource(), OAuth2Resource.class))
+            .thenReturn(customOAuth2Resource);
+        when(customOAuth2Resource.getScopeSeparator()).thenReturn(DEFAULT_OAUTH_SCOPE_SEPARATOR);
+        when(mockRequest.headers()).thenReturn(HttpHeaders.create().set("Authorization", "Bearer " + UUID.randomUUID()));
+
+        Cache cache = mock(Cache.class);
+        when(customCacheResource.getCache(any(ExecutionContext.class))).thenReturn(cache);
+
+        Oauth2PolicyV3 policy = new Oauth2PolicyV3(oAuth2PolicyConfiguration);
+        Handler<OAuth2Response> handler = policy.handleResponse(
+            mockPolicychain,
+            mockRequest,
+            mockResponse,
+            mockExecutionContext,
+            customCacheResource
+        );
+
+        String payload = readResource("/io/gravitee/policy/oauth2/oauth2-response04.json");
+        handler.handle(new OAuth2Response(true, payload));
+
+        assertNull(mockRequest.headers().get("Authorization"));
+
+        verify(mockExecutionContext).setAttribute(Oauth2PolicyV3.CONTEXT_ATTRIBUTE_CLIENT_ID, "my-client-id");
+        verify(mockPolicychain).doNext(mockRequest, mockResponse);
+        verify(cache).put(any(Element.class));
+    }
+
     @Test
     void shouldValidate_goodIntrospection_withClientId_invalidScopes() throws IOException {
         HttpHeaders httpHeaders = mock(HttpHeaders.class);