Merge pull request #34105 from sberyozkin/oidc_userinfo_cache_problem

Avoid calling OIDC UserInfo endpoint if UserInfo is cached
quarkusio · Jun 20, 2023 · 51653b4 · 51653b4
2 parents e6cfd3f + e2cb17c
commit 51653b4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 11 deletions.
diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcIdentityProvider.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcIdentityProvider.java
@@ -447,7 +447,7 @@ private Uni<TokenVerificationResult> refreshJwksAndVerifyTokenUni(TenantConfigCo
                 .recoverWithUni(f -> introspectTokenUni(resolvedContext, token));
     }
 
-    private Uni<TokenVerificationResult> introspectTokenUni(TenantConfigContext resolvedContext, String token) {
+    private Uni<TokenVerificationResult> introspectTokenUni(TenantConfigContext resolvedContext, final String token) {
         TokenIntrospectionCache tokenIntrospectionCache = tenantResolver.getTokenIntrospectionCache();
         Uni<TokenIntrospection> tokenIntrospectionUni = tokenIntrospectionCache == null ? null
                 : tokenIntrospectionCache
@@ -456,7 +456,12 @@ private Uni<TokenVerificationResult> introspectTokenUni(TenantConfigContext reso
             tokenIntrospectionUni = newTokenIntrospectionUni(resolvedContext, token);
         } else {
             tokenIntrospectionUni = tokenIntrospectionUni.onItem().ifNull()
-                    .switchTo(newTokenIntrospectionUni(resolvedContext, token));
+                    .switchTo(new Supplier<Uni<? extends TokenIntrospection>>() {
+                        @Override
+                        public Uni<TokenIntrospection> get() {
+                            return newTokenIntrospectionUni(resolvedContext, token);
+                        }
+                    });
         }
         return tokenIntrospectionUni.onItem().transform(t -> new TokenVerificationResult(null, t));
     }
@@ -501,10 +506,8 @@ private Uni<UserInfo> getUserInfoUni(RoutingContext vertxContext, TokenAuthentic
         }
 
         LOG.debug("Requesting UserInfo");
-        String accessToken = vertxContext.get(OidcConstants.ACCESS_TOKEN_VALUE);
-        if (accessToken == null) {
-            accessToken = request.getToken().getToken();
-        }
+        String contextAccessToken = vertxContext.get(OidcConstants.ACCESS_TOKEN_VALUE);
+        final String accessToken = contextAccessToken != null ? contextAccessToken : request.getToken().getToken();
 
         UserInfoCache userInfoCache = tenantResolver.getUserInfoCache();
         Uni<UserInfo> userInfoUni = userInfoCache == null ? null
@@ -513,7 +516,12 @@ private Uni<UserInfo> getUserInfoUni(RoutingContext vertxContext, TokenAuthentic
             userInfoUni = newUserInfoUni(resolvedContext, accessToken);
         } else {
             userInfoUni = userInfoUni.onItem().ifNull()
-                    .switchTo(newUserInfoUni(resolvedContext, accessToken));
+                    .switchTo(new Supplier<Uni<? extends UserInfo>>() {
+                        @Override
+                        public Uni<UserInfo> get() {
+                            return newUserInfoUni(resolvedContext, accessToken);
+                        }
+                    });
         }
         return userInfoUni;
     }

diff --git a/...on-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowUserInfoResource.java b/...on-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowUserInfoResource.java
@@ -31,11 +31,9 @@ public class CodeFlowUserInfoResource {
     @GET
     @Path("/code-flow-user-info-only")
     public String access() {
-        int cacheSize = tokenCache.getCacheSize();
-        tokenCache.clearCache();
         return identity.getPrincipal().getName() + ":" + userInfo.getPreferredUserName() + ":" + accessToken.getName()
                 + ", cache size: "
-                + cacheSize;
+                + tokenCache.getCacheSize();
     }
 
     @GET

diff --git a/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java b/...n-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java
@@ -3,8 +3,10 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
 import static com.github.tomakehurst.wiremock.client.WireMock.containing;
 import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.matching;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -214,9 +216,11 @@ public void testCodeFlowUserInfo() throws Exception {
         defineCodeFlowAuthorizationOauth2TokenStub();
 
         doTestCodeFlowUserInfo("code-flow-user-info-only", 300);
+        clearCache();
         doTestCodeFlowUserInfo("code-flow-user-info-github", 360);
+        clearCache();
         doTestCodeFlowUserInfo("code-flow-user-info-dynamic-github", 301);
-
+        clearCache();
         doTestCodeFlowUserInfoCashedInIdToken();
     }
 
@@ -256,6 +260,13 @@ private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetim
             page = form.getInputByValue("login").click();
 
             assertEquals("alice:alice:alice, cache size: 1", page.getBody().asNormalizedText());
+            page = webClient.getPage("http://localhost:8081/" + tenantId);
+            assertEquals("alice:alice:alice, cache size: 1", page.getBody().asNormalizedText());
+            page = webClient.getPage("http://localhost:8081/" + tenantId);
+            assertEquals("alice:alice:alice, cache size: 1", page.getBody().asNormalizedText());
+
+            wireMockServer.verify(1, getRequestedFor(urlPathMatching("/auth/realms/quarkus/protocol/openid-connect/userinfo")));
+            wireMockServer.resetRequests();
 
             JsonObject idTokenClaims = decryptIdToken(webClient, tenantId);
             assertNull(idTokenClaims.getJsonObject(OidcUtils.USER_INFO_ATTRIBUTE));