From dac533fef045802b96ce09d817df74af8fa40725 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Tue, 25 Jul 2023 11:54:42 +0100 Subject: [PATCH] OIDC UserInfo request must not be made if the tioken verification fails --- .../oidc/runtime/OidcIdentityProvider.java | 258 ++++++++++-------- .../io/quarkus/it/keycloak/OidcResource.java | 6 +- .../quarkus/it/keycloak/TenantResource.java | 1 + .../BearerTokenAuthorizationTest.java | 24 +- 4 files changed, 164 insertions(+), 125 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 index 5d61792865d2c..92b31d9072835 100644 --- 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 @@ -45,7 +45,6 @@ public class OidcIdentityProvider implements IdentityProvider NULL_CODE_ACCESS_TOKEN_UNI = Uni.createFrom().nullItem(); - private static final Uni NULL_USER_INFO_UNI = Uni.createFrom().nullItem(); private static final String CODE_ACCESS_TOKEN_RESULT = "code_flow_access_token_result"; @Inject @@ -107,21 +106,21 @@ && isOpaqueAccessToken(vertxContext, request, resolvedContext)) { // Typically it will be done for bearer access tokens therefore even if the access token has expired // the client will be able to refresh if needed, no refresh token is available to Quarkus during the // bearer access token verification - - Uni userInfo = resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false) - ? getUserInfoUni(vertxContext, request, resolvedContext) - : NULL_USER_INFO_UNI; - - return userInfo.onItemOrFailure().transformToUni( - new BiFunction>() { - @Override - public Uni apply(UserInfo userInfo, Throwable t) { - if (t != null) { - return Uni.createFrom().failure(new AuthenticationFailedException(t)); + if (resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false)) { + return getUserInfoUni(vertxContext, request, resolvedContext).onItemOrFailure().transformToUni( + new BiFunction>() { + @Override + public Uni apply(UserInfo userInfo, Throwable t) { + if (t != null) { + return Uni.createFrom().failure(new AuthenticationFailedException(t)); + } + return validateTokenWithUserInfoAndCreateIdentity(vertxContext, request, resolvedContext, + userInfo); } - return validateTokenWithUserInfoAndCreateIdentity(vertxContext, request, resolvedContext, userInfo); - } - }); + }); + } else { + return validateTokenWithUserInfoAndCreateIdentity(vertxContext, request, resolvedContext, null); + } } else { final Uni primaryTokenUni; if (isInternalIdToken(request)) { @@ -184,7 +183,20 @@ public Uni apply(TokenVerificationResult codeAccessToken, Thro Uni tokenUni = verifyTokenUni(resolvedContext, request.getToken().getToken(), false, userInfo); - return createSecurityIdentityWithOidcServer(tokenUni, vertxContext, request, resolvedContext, userInfo); + return tokenUni.onItemOrFailure() + .transformToUni( + new BiFunction>() { + @Override + public Uni apply(TokenVerificationResult result, Throwable t) { + if (t != null) { + return Uni.createFrom().failure(new AuthenticationFailedException(t)); + } + + return createSecurityIdentityWithOidcServer(result, vertxContext, request, + resolvedContext, userInfo); + } + }); + } }); } @@ -193,20 +205,32 @@ private Uni getUserInfoAndCreateIdentity(Uni userInfo = resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false) - ? getUserInfoUni(vertxContext, request, resolvedContext) - : NULL_USER_INFO_UNI; - - return userInfo.onItemOrFailure().transformToUni( - new BiFunction>() { + return tokenUni.onItemOrFailure() + .transformToUni(new BiFunction>() { @Override - public Uni apply(UserInfo userInfo, Throwable t) { + public Uni apply(TokenVerificationResult result, Throwable t) { if (t != null) { return Uni.createFrom().failure(new AuthenticationFailedException(t)); } - return createSecurityIdentityWithOidcServer(tokenUni, vertxContext, request, resolvedContext, userInfo); + if (resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false)) { + return getUserInfoUni(vertxContext, request, resolvedContext).onItemOrFailure().transformToUni( + new BiFunction>() { + @Override + public Uni apply(UserInfo userInfo, Throwable t) { + if (t != null) { + return Uni.createFrom().failure(new AuthenticationFailedException(t)); + } + return createSecurityIdentityWithOidcServer(result, vertxContext, request, + resolvedContext, userInfo); + } + }); + } else { + return createSecurityIdentityWithOidcServer(result, vertxContext, request, resolvedContext, null); + } + } }); + } private boolean isOpaqueAccessToken(RoutingContext vertxContext, TokenAuthenticationRequest request, @@ -222,108 +246,100 @@ private boolean isOpaqueAccessToken(RoutingContext vertxContext, TokenAuthentica return false; } - private Uni createSecurityIdentityWithOidcServer(Uni tokenUni, + private Uni createSecurityIdentityWithOidcServer(TokenVerificationResult result, RoutingContext vertxContext, TokenAuthenticationRequest request, TenantConfigContext resolvedContext, final UserInfo userInfo) { - return tokenUni.onItemOrFailure() - .transformToUni(new BiFunction>() { - @Override - public Uni apply(TokenVerificationResult result, Throwable t) { - if (t != null) { - return Uni.createFrom().failure(new AuthenticationFailedException(t)); - } - // Token has been verified, as a JWT or an opaque token, possibly involving - // an introspection request. - final TokenCredential tokenCred = request.getToken(); - - JsonObject tokenJson = result.localVerificationResult; - if (tokenJson == null) { - // JSON token representation may be null not only if it is an opaque access token - // but also if it is JWT and no JWK with a matching kid is available, asynchronous - // JWK refresh has not finished yet, but the fallback introspection request has succeeded. - tokenJson = OidcUtils.decodeJwtContent(tokenCred.getToken()); - } - if (tokenJson != null) { - try { - OidcUtils.validatePrimaryJwtTokenType(resolvedContext.oidcConfig.token, tokenJson); - JsonObject rolesJson = getRolesJson(vertxContext, resolvedContext, tokenCred, tokenJson, - userInfo); - SecurityIdentity securityIdentity = validateAndCreateIdentity(vertxContext, tokenCred, - resolvedContext, tokenJson, rolesJson, userInfo, result.introspectionResult); - // If the primary token is a bearer access token then there's no point of checking if - // it should be refreshed as RT is only available for the code flow tokens - if (isIdToken(request) - && tokenAutoRefreshPrepared(result, vertxContext, resolvedContext.oidcConfig)) { - return Uni.createFrom().failure(new TokenAutoRefreshException(securityIdentity)); - } else { - return Uni.createFrom().item(securityIdentity); - } - } catch (Throwable ex) { - return Uni.createFrom().failure(new AuthenticationFailedException(ex)); - } - } else if (isIdToken(request) - || tokenCred instanceof AccessTokenCredential - && !((AccessTokenCredential) tokenCred).isOpaque()) { - return Uni.createFrom() - .failure(new AuthenticationFailedException("JWT token can not be converted to JSON")); - } else { - // ID Token or Bearer access token has been introspected or verified via Userinfo acquisition - QuarkusSecurityIdentity.Builder builder = QuarkusSecurityIdentity.builder(); - builder.addCredential(tokenCred); - OidcUtils.setSecurityIdentityUserInfo(builder, userInfo); - OidcUtils.setSecurityIdentityConfigMetadata(builder, resolvedContext); - final String userName; - if (result.introspectionResult == null) { - if (resolvedContext.oidcConfig.token.allowOpaqueTokenIntrospection && - resolvedContext.oidcConfig.token.verifyAccessTokenWithUserInfo.orElse(false)) { - if (resolvedContext.oidcConfig.token.principalClaim.isPresent() && userInfo != null) { - userName = userInfo.getString(resolvedContext.oidcConfig.token.principalClaim.get()); - } else { - userName = ""; - } - } else { - // we don't expect this to ever happen - LOG.debug("Illegal state - token introspection result is not available."); - return Uni.createFrom().failure(new AuthenticationFailedException()); - } - } else { - OidcUtils.setSecurityIdentityIntrospection(builder, result.introspectionResult); - String principalName = result.introspectionResult.getUsername(); - if (principalName == null) { - principalName = result.introspectionResult.getSubject(); - } - userName = principalName != null ? principalName : ""; + // Token has been verified, as a JWT or an opaque token, possibly involving + // an introspection request. + final TokenCredential tokenCred = request.getToken(); - Set scopes = result.introspectionResult.getScopes(); - if (scopes != null) { - builder.addRoles(scopes); - } - } - builder.setPrincipal(new Principal() { - @Override - public String getName() { - return userName != null ? userName : ""; - } - }); - if (userInfo != null) { - OidcUtils.setSecurityIdentityRoles(builder, resolvedContext.oidcConfig, - new JsonObject(userInfo.getJsonObject().toString())); - } - OidcUtils.setBlockingApiAttribute(builder, vertxContext); - OidcUtils.setTenantIdAttribute(builder, resolvedContext.oidcConfig); - OidcUtils.setRoutingContextAttribute(builder, vertxContext); - SecurityIdentity identity = builder.build(); - // If the primary token is a bearer access token then there's no point of checking if - // it should be refreshed as RT is only available for the code flow tokens - if (isIdToken(request) - && tokenAutoRefreshPrepared(result, vertxContext, resolvedContext.oidcConfig)) { - return Uni.createFrom().failure(new TokenAutoRefreshException(identity)); - } - return Uni.createFrom().item(identity); - } + JsonObject tokenJson = result.localVerificationResult; + if (tokenJson == null) { + // JSON token representation may be null not only if it is an opaque access token + // but also if it is JWT and no JWK with a matching kid is available, asynchronous + // JWK refresh has not finished yet, but the fallback introspection request has succeeded. + tokenJson = OidcUtils.decodeJwtContent(tokenCred.getToken()); + } + if (tokenJson != null) { + try { + OidcUtils.validatePrimaryJwtTokenType(resolvedContext.oidcConfig.token, tokenJson); + JsonObject rolesJson = getRolesJson(vertxContext, resolvedContext, tokenCred, tokenJson, + userInfo); + SecurityIdentity securityIdentity = validateAndCreateIdentity(vertxContext, tokenCred, + resolvedContext, tokenJson, rolesJson, userInfo, result.introspectionResult); + // If the primary token is a bearer access token then there's no point of checking if + // it should be refreshed as RT is only available for the code flow tokens + if (isIdToken(request) + && tokenAutoRefreshPrepared(result, vertxContext, resolvedContext.oidcConfig)) { + return Uni.createFrom().failure(new TokenAutoRefreshException(securityIdentity)); + } else { + return Uni.createFrom().item(securityIdentity); + } + } catch (Throwable ex) { + return Uni.createFrom().failure(new AuthenticationFailedException(ex)); + } + } else if (isIdToken(request) + || tokenCred instanceof AccessTokenCredential + && !((AccessTokenCredential) tokenCred).isOpaque()) { + return Uni.createFrom() + .failure(new AuthenticationFailedException("JWT token can not be converted to JSON")); + } else { + // ID Token or Bearer access token has been introspected or verified via Userinfo acquisition + QuarkusSecurityIdentity.Builder builder = QuarkusSecurityIdentity.builder(); + builder.addCredential(tokenCred); + OidcUtils.setSecurityIdentityUserInfo(builder, userInfo); + OidcUtils.setSecurityIdentityConfigMetadata(builder, resolvedContext); + final String userName; + if (result.introspectionResult == null) { + if (resolvedContext.oidcConfig.token.allowOpaqueTokenIntrospection && + resolvedContext.oidcConfig.token.verifyAccessTokenWithUserInfo.orElse(false)) { + if (resolvedContext.oidcConfig.token.principalClaim.isPresent() && userInfo != null) { + userName = userInfo.getString(resolvedContext.oidcConfig.token.principalClaim.get()); + } else { + userName = ""; } - }); + } else { + // we don't expect this to ever happen + LOG.debug("Illegal state - token introspection result is not available."); + return Uni.createFrom().failure(new AuthenticationFailedException()); + } + } else { + OidcUtils.setSecurityIdentityIntrospection(builder, result.introspectionResult); + String principalName = result.introspectionResult.getUsername(); + if (principalName == null) { + principalName = result.introspectionResult.getSubject(); + } + userName = principalName != null ? principalName : ""; + + Set scopes = result.introspectionResult.getScopes(); + if (scopes != null) { + builder.addRoles(scopes); + } + } + builder.setPrincipal(new Principal() { + @Override + public String getName() { + return userName != null ? userName : ""; + } + }); + if (userInfo != null) { + OidcUtils.setSecurityIdentityRoles(builder, resolvedContext.oidcConfig, + new JsonObject(userInfo.getJsonObject().toString())); + } + OidcUtils.setBlockingApiAttribute(builder, vertxContext); + OidcUtils.setTenantIdAttribute(builder, resolvedContext.oidcConfig); + OidcUtils.setRoutingContextAttribute(builder, vertxContext); + SecurityIdentity identity = builder.build(); + // If the primary token is a bearer access token then there's no point of checking if + // it should be refreshed as RT is only available for the code flow tokens + if (isIdToken(request) + && tokenAutoRefreshPrepared(result, vertxContext, resolvedContext.oidcConfig)) { + return Uni.createFrom().failure(new TokenAutoRefreshException(identity)); + } + return Uni.createFrom().item(identity); + } + } private static boolean isInternalIdToken(TokenAuthenticationRequest request) { diff --git a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java index 4dd17153b0e28..bc3e34a53b3da 100644 --- a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java +++ b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/OidcResource.java @@ -115,9 +115,11 @@ public int resetIntrospectionEndpointCallCount() { @Produces("application/json") @Path("introspect") public String introspect(@FormParam("client_id") String clientId, @FormParam("client_secret") String clientSecret, - @HeaderParam("Authorization") String authorization) throws Exception { + @HeaderParam("Authorization") String authorization, @FormParam("token") String token) throws Exception { introspectionEndpointCallCount++; + boolean activeStatus = introspection && !token.endsWith("-invalid"); + String introspectionClientId = "none"; String introspectionClientSecret = "none"; if (clientSecret != null) { @@ -139,7 +141,7 @@ public String introspect(@FormParam("client_id") String clientId, @FormParam("cl } return "{" + - " \"active\": " + introspection + "," + + " \"active\": " + activeStatus + "," + " \"scope\": \"user\"," + " \"email\": \"user@gmail.com\"," + " \"username\": \"alice\"," + diff --git a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java index 811604691668e..5c59e4bb2b6f5 100644 --- a/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java +++ b/integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java @@ -74,6 +74,7 @@ public String userNameService(@PathParam("tenant") String tenant, @QueryParam("r response += (",introspection_client_id:" + introspection.getString("introspection_client_id")); response += (",introspection_client_secret:" + introspection.getString("introspection_client_secret")); response += (",active:" + introspection.getBoolean("active")); + response += (",userinfo:" + getUserInfo().getPreferredUserName()); response += (",cache-size:" + tokenCache.getCacheSize()); } diff --git a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java index ada7f13607413..7926bdd89cee3 100644 --- a/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java +++ b/integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java @@ -491,7 +491,7 @@ public void testJwtTokenIntrospectionOnlyAndUserInfo() { .statusCode(200) .body(equalTo( "tenant-oidc-introspection-only:alice,client_id:client-introspection-only," - + "introspection_client_id:none,introspection_client_secret:none,active:true,cache-size:0")); + + "introspection_client_id:none,introspection_client_secret:none,active:true,userinfo:alice,cache-size:0")); } RestAssured.when().get("/oidc/jwk-endpoint-call-count").then().body(equalTo("0")); @@ -501,6 +501,26 @@ public void testJwtTokenIntrospectionOnlyAndUserInfo() { RestAssured.when().get("/cache/size").then().body(equalTo("0")); } + @Test + public void testNoUserInfoCallIfTokenIsInvalid() { + RestAssured.when().post("/oidc/userinfo-endpoint-call-count").then().body(equalTo("0")); + RestAssured.when().post("/oidc/enable-introspection").then().body(equalTo("true")); + RestAssured.when().post("/cache/clear").then().body(equalTo("0")); + + String jwt = getAccessTokenFromSimpleOidc("2"); + // Modify the signature + jwt += "-invalid"; + RestAssured.given().auth().oauth2(jwt) + .when().get("/tenant/tenant-oidc-introspection-only/api/user") + .then() + .statusCode(401); + + RestAssured.when().get("/oidc/introspection-endpoint-call-count").then().body(equalTo("1")); + RestAssured.when().post("/oidc/disable-introspection").then().body(equalTo("false")); + RestAssured.when().get("/oidc/userinfo-endpoint-call-count").then().body(equalTo("0")); + RestAssured.when().get("/cache/size").then().body(equalTo("0")); + } + @Test public void testJwtTokenIntrospectionOnlyAndUserInfoCache() { RestAssured.when().post("/oidc/jwk-endpoint-call-count").then().body(equalTo("0")); @@ -537,7 +557,7 @@ private void verifyTokenIntrospectionAndUserInfoAreCached(String token1, int exp .statusCode(200) .body(equalTo( "tenant-oidc-introspection-only-cache:alice,client_id:client-introspection-only-cache," - + "introspection_client_id:bob,introspection_client_secret:bob_secret,active:true,cache-size:" + + "introspection_client_id:bob,introspection_client_secret:bob_secret,active:true,userinfo:alice,cache-size:" + expectedCacheSize)); } RestAssured.when().get("/oidc/introspection-endpoint-call-count").then().body(equalTo("1"));