From b8bdad1004716ab473f377f06fb01a58349fccc9 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Fri, 16 Feb 2024 19:08:37 +0000 Subject: [PATCH] Check the code flow access token after ID token --- .../runtime/AbstractJsonObjectResponse.java | 2 +- .../oidc/runtime/OidcIdentityProvider.java | 86 ++++++++++--------- .../runtime/OidcTokenCredentialProducer.java | 38 +++++++- .../io/quarkus/oidc/runtime/OidcUtils.java | 5 ++ .../CodeFlowTokenIntrospectionResource.java | 6 +- .../keycloak/CodeFlowAuthorizationTest.java | 4 +- 6 files changed, 94 insertions(+), 47 deletions(-) diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/AbstractJsonObjectResponse.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/AbstractJsonObjectResponse.java index 9dc01cc51c156..416848f07fcf2 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/AbstractJsonObjectResponse.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/AbstractJsonObjectResponse.java @@ -58,7 +58,7 @@ public Object get(String name) { } public boolean contains(String propertyName) { - return json.containsKey(propertyName) && !json.isNull(propertyName); + return json != null && json.containsKey(propertyName) && !json.isNull(propertyName); } public Set getPropertyNames() { 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 711baa25e5435..5ecd2c072296a 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 @@ -46,7 +46,6 @@ public class OidcIdentityProvider implements IdentityProvider NULL_CODE_ACCESS_TOKEN_UNI = Uni.createFrom().nullItem(); - private static final String CODE_ACCESS_TOKEN_RESULT = "code_flow_access_token_result"; protected final DefaultTenantConfigResolver tenantResolver; private final BlockingTaskRunner uniVoidOidcContext; @@ -149,30 +148,7 @@ public Uni apply(UserInfo userInfo, Throwable t) { isIdToken(request), null); } - // Verify Code Flow access token first if it is available and has to be verified. - // It may be refreshed if it has or has nearly expired - Uni codeAccessTokenUni = verifyCodeFlowAccessTokenUni(requestData, request, - resolvedContext, - null); - return codeAccessTokenUni.onItemOrFailure().transformToUni( - new BiFunction>() { - @Override - public Uni apply(TokenVerificationResult codeAccessTokenResult, Throwable t) { - if (t != null) { - return Uni.createFrom().failure(t instanceof AuthenticationFailedException ? t - : new AuthenticationFailedException(t)); - } - if (codeAccessTokenResult != null) { - if (tokenAutoRefreshPrepared(codeAccessTokenResult, requestData, - resolvedContext.oidcConfig)) { - return Uni.createFrom().failure(new TokenAutoRefreshException(null)); - } - requestData.put(CODE_ACCESS_TOKEN_RESULT, codeAccessTokenResult); - } - return getUserInfoAndCreateIdentity(primaryTokenUni, requestData, request, resolvedContext); - } - }); - + return getUserInfoAndCreateIdentity(primaryTokenUni, requestData, request, resolvedContext); } } @@ -191,7 +167,7 @@ public Uni apply(TokenVerificationResult codeAccessToken, Thro } if (codeAccessToken != null) { - requestData.put(CODE_ACCESS_TOKEN_RESULT, codeAccessToken); + requestData.put(OidcUtils.CODE_ACCESS_TOKEN_RESULT, codeAccessToken); } Uni tokenUni = verifyTokenUni(requestData, resolvedContext, @@ -217,7 +193,8 @@ public Uni apply(TokenVerificationResult result, Throwable t) } private Uni getUserInfoAndCreateIdentity(Uni tokenUni, - Map requestData, TokenAuthenticationRequest request, + Map requestData, + TokenAuthenticationRequest request, TenantConfigContext resolvedContext) { return tokenUni.onItemOrFailure() @@ -227,21 +204,49 @@ public Uni apply(TokenVerificationResult result, Throwable t) if (t != null) { return Uni.createFrom().failure(new AuthenticationFailedException(t)); } - if (resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false)) { - return getUserInfoUni(requestData, request, resolvedContext).onItemOrFailure().transformToUni( - new BiFunction>() { - @Override - public Uni apply(UserInfo userInfo, Throwable t) { - if (t != null) { - return Uni.createFrom().failure(new AuthenticationFailedException(t)); + + Uni codeAccessTokenUni = verifyCodeFlowAccessTokenUni(requestData, request, + resolvedContext, + null); + return codeAccessTokenUni.onItemOrFailure().transformToUni( + new BiFunction>() { + @Override + public Uni apply(TokenVerificationResult codeAccessTokenResult, + Throwable t) { + if (t != null) { + return Uni.createFrom().failure(t instanceof AuthenticationFailedException ? t + : new AuthenticationFailedException(t)); + } + if (codeAccessTokenResult != null) { + if (tokenAutoRefreshPrepared(codeAccessTokenResult, requestData, + resolvedContext.oidcConfig)) { + return Uni.createFrom().failure(new TokenAutoRefreshException(null)); } + requestData.put(OidcUtils.CODE_ACCESS_TOKEN_RESULT, codeAccessTokenResult); + } + + if (resolvedContext.oidcConfig.authentication.isUserInfoRequired().orElse(false)) { + return getUserInfoUni(requestData, 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, + requestData, request, + resolvedContext, userInfo); + } + }); + } else { return createSecurityIdentityWithOidcServer(result, requestData, request, - resolvedContext, userInfo); + resolvedContext, null); } - }); - } else { - return createSecurityIdentityWithOidcServer(result, requestData, request, resolvedContext, null); - } + } + }); } }); @@ -405,7 +410,8 @@ private static JsonObject getRolesJson(Map requestData, TenantCo rolesJson = new JsonObject(userInfo.getJsonObject().toString()); } else if (tokenCred instanceof IdTokenCredential && resolvedContext.oidcConfig.roles.source.get() == Source.accesstoken) { - rolesJson = ((TokenVerificationResult) requestData.get(CODE_ACCESS_TOKEN_RESULT)).localVerificationResult; + rolesJson = ((TokenVerificationResult) requestData + .get(OidcUtils.CODE_ACCESS_TOKEN_RESULT)).localVerificationResult; if (rolesJson == 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 diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTokenCredentialProducer.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTokenCredentialProducer.java index 2c5513e10aed8..fff629c7bffb5 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTokenCredentialProducer.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcTokenCredentialProducer.java @@ -9,6 +9,7 @@ import org.jboss.logging.Logger; import io.quarkus.oidc.AccessTokenCredential; +import io.quarkus.oidc.IdToken; import io.quarkus.oidc.IdTokenCredential; import io.quarkus.oidc.RefreshToken; import io.quarkus.oidc.TokenIntrospection; @@ -78,13 +79,43 @@ UserInfo currentUserInfo() { } /** - * The producer method for the current UserInfo + * The producer method for the ID token TokenIntrospection only. * - * @return the user info + * @return the ID token introspection */ @Produces @RequestScoped - TokenIntrospection currentTokenIntrospection() { + @IdToken + TokenIntrospection idTokenIntrospection() { + return tokenIntrospectionFromIdentityAttribute(); + } + + /** + * The producer method for the current TokenIntrospection. + *

+ * This TokenIntrospection always represents the bearer access token introspection when the bearer access tokens + * are used. + *

+ * In case of the authorization code flow, it represents a code flow access token introspection + * if it has been enabled by setting the `quarkus.oidc.authentication.verify-access-token` property to `true` + * and an ID token introspection otherwise. Use the `@IdToken` qualifier if both ID and code flow access tokens + * must be introspected. + * + * @return the token introspection + */ + @Produces + @RequestScoped + TokenIntrospection tokenIntrospection() { + TokenVerificationResult codeFlowAccessTokenResult = (TokenVerificationResult) identity + .getAttribute(OidcUtils.CODE_ACCESS_TOKEN_RESULT); + if (codeFlowAccessTokenResult == null) { + return tokenIntrospectionFromIdentityAttribute(); + } else { + return codeFlowAccessTokenResult.introspectionResult; + } + } + + TokenIntrospection tokenIntrospectionFromIdentityAttribute() { TokenIntrospection introspection = (TokenIntrospection) identity.getAttribute(OidcUtils.INTROSPECTION_ATTRIBUTE); if (introspection == null) { LOG.trace("TokenIntrospection is null"); @@ -92,4 +123,5 @@ TokenIntrospection currentTokenIntrospection() { } return introspection; } + } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java index 763a991177bfc..a436e2456ca7f 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java @@ -83,6 +83,7 @@ public final class OidcUtils { public static final Integer MAX_COOKIE_VALUE_LENGTH = 4096; public static final String POST_LOGOUT_COOKIE_NAME = "q_post_logout"; static final String UNDERSCORE = "_"; + static final String CODE_ACCESS_TOKEN_RESULT = "code_flow_access_token_result"; static final String COMMA = ","; static final Uni VOID_UNI = Uni.createFrom().voidItem(); static final BlockingTaskRunner deleteTokensRequestContext = new BlockingTaskRunner(); @@ -350,6 +351,10 @@ static QuarkusSecurityIdentity validateAndCreateIdentity(Map req setSecurityIdentityConfigMetadata(builder, resolvedContext); setBlockingApiAttribute(builder, vertxContext); setTenantIdAttribute(builder, config); + TokenVerificationResult codeFlowAccessTokenResult = (TokenVerificationResult) requestData.get(CODE_ACCESS_TOKEN_RESULT); + if (codeFlowAccessTokenResult != null) { + builder.addAttribute(CODE_ACCESS_TOKEN_RESULT, codeFlowAccessTokenResult); + } return builder.build(); } diff --git a/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowTokenIntrospectionResource.java b/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowTokenIntrospectionResource.java index 4bfd58e5de927..86825d36ddaac 100644 --- a/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowTokenIntrospectionResource.java +++ b/integration-tests/oidc-wiremock/src/main/java/io/quarkus/it/keycloak/CodeFlowTokenIntrospectionResource.java @@ -4,6 +4,7 @@ import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; +import io.quarkus.oidc.TokenIntrospection; import io.quarkus.security.Authenticated; import io.quarkus.security.identity.SecurityIdentity; @@ -14,8 +15,11 @@ public class CodeFlowTokenIntrospectionResource { @Inject SecurityIdentity identity; + @Inject + TokenIntrospection tokenIntrospection; + @GET public String access() { - return identity.getPrincipal().getName(); + return identity.getPrincipal().getName() + ":" + tokenIntrospection.getUsername(); } } diff --git a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java index a21b2f14fc117..56e7bae3d7d58 100644 --- a/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java +++ b/integration-tests/oidc-wiremock/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java @@ -287,12 +287,12 @@ public void testCodeFlowTokenIntrospection() throws Exception { TextPage textPage = form.getInputByValue("login").click(); - assertEquals("alice", textPage.getContent()); + assertEquals("alice:alice", textPage.getContent()); // refresh Thread.sleep(3000); textPage = webClient.getPage("http://localhost:8081/code-flow-token-introspection"); - assertEquals("admin", textPage.getContent()); + assertEquals("admin:admin", textPage.getContent()); webClient.getCookieManager().clearCookies(); }