From f3c08b363ae8c35304dc6bc4c5747ab9c654fb32 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Thu, 24 Aug 2023 17:18:18 +0100 Subject: [PATCH] Report 401 and remove OIDC session cookie if it is malformed --- .../runtime/CodeAuthenticationMechanism.java | 11 ++++- .../runtime/DefaultTokenStateManager.java | 46 +++++++++++-------- .../io/quarkus/oidc/runtime/OidcProvider.java | 4 +- .../io/quarkus/it/keycloak/CodeFlowTest.java | 17 +++++++ 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java index e61ae8449f502..bd36bbb81e98c 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java @@ -304,6 +304,15 @@ private Uni reAuthenticate(Cookie sessionCookie, context.put(TenantConfigContext.class.getName(), configContext); return resolver.getTokenStateManager().getTokens(context, configContext.oidcConfig, sessionCookie.getValue(), getTokenStateRequestContext) + .onFailure(AuthenticationCompletionException.class) + .recoverWithUni( + new Function>() { + @Override + public Uni apply(Throwable t) { + return removeSessionCookie(context, configContext.oidcConfig) + .replaceWith(Uni.createFrom().failure(t)); + } + }) .chain(new Function>() { @Override public Uni apply(AuthorizationCodeTokens session) { @@ -846,7 +855,7 @@ private static boolean prepareNonceForVerification(RoutingContext context, OidcT } } - private static Object errorMessage(Throwable t) { + private static String errorMessage(Throwable t) { return t.getCause() != null ? t.getCause().getMessage() : t.getMessage(); } diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java index bc66480aa59fb..fcc50bfe52a5f 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java @@ -6,6 +6,7 @@ import io.quarkus.oidc.OidcRequestContext; import io.quarkus.oidc.OidcTenantConfig; import io.quarkus.oidc.TokenStateManager; +import io.quarkus.security.AuthenticationCompletionException; import io.quarkus.security.AuthenticationFailedException; import io.smallrye.mutiny.Uni; import io.vertx.core.http.Cookie; @@ -77,33 +78,38 @@ public Uni getTokens(RoutingContext routingContext, Oid tokenState = decryptAll ? decryptToken(tokenState, routingContext, oidcConfig) : tokenState; String[] tokens = CodeAuthenticationMechanism.COOKIE_PATTERN.split(tokenState); + String idToken = decryptAll ? tokens[0] : decryptToken(tokens[0], routingContext, oidcConfig); String accessToken = null; String refreshToken = null; - if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.KEEP_ALL_TOKENS) { - if (!oidcConfig.tokenStateManager.splitTokens) { - accessToken = decryptAll ? tokens[1] : decryptToken(tokens[1], routingContext, oidcConfig); - refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig); - } else { - Cookie atCookie = getAccessTokenCookie(routingContext, oidcConfig); - if (atCookie != null) { - accessToken = decryptToken(atCookie.getValue(), routingContext, oidcConfig); - } - Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig); - if (rtCookie != null) { - refreshToken = decryptToken(rtCookie.getValue(), routingContext, oidcConfig); + try { + if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.KEEP_ALL_TOKENS) { + if (!oidcConfig.tokenStateManager.splitTokens) { + accessToken = decryptAll ? tokens[1] : decryptToken(tokens[1], routingContext, oidcConfig); + refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig); + } else { + Cookie atCookie = getAccessTokenCookie(routingContext, oidcConfig); + if (atCookie != null) { + accessToken = decryptToken(atCookie.getValue(), routingContext, oidcConfig); + } + Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig); + if (rtCookie != null) { + refreshToken = decryptToken(rtCookie.getValue(), routingContext, oidcConfig); + } } - } - } else if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.ID_REFRESH_TOKENS) { - if (!oidcConfig.tokenStateManager.splitTokens) { - refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig); - } else { - Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig); - if (rtCookie != null) { - refreshToken = decryptToken(rtCookie.getValue(), routingContext, oidcConfig); + } else if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.ID_REFRESH_TOKENS) { + if (!oidcConfig.tokenStateManager.splitTokens) { + refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig); + } else { + Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig); + if (rtCookie != null) { + refreshToken = decryptToken(rtCookie.getValue(), routingContext, oidcConfig); + } } } + } catch (ArrayIndexOutOfBoundsException ex) { + return Uni.createFrom().failure(new AuthenticationCompletionException("Session cookie is malformed")); } return Uni.createFrom().item(new AuthorizationCodeTokens(idToken, accessToken, refreshToken)); diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java index dc79678538eeb..16da83a0859d6 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java @@ -301,9 +301,9 @@ public TokenIntrospection apply(TokenIntrospection introspectionResult, Throwabl throw new AuthenticationFailedException(t); } if (!introspectionResult.isActive()) { - LOG.debugf("Token issued to client %s is not active", oidcConfig.clientId.get()); verifyTokenExpiry(introspectionResult.getLong(OidcConstants.INTROSPECTION_TOKEN_EXP)); - throw new AuthenticationFailedException(); + throw new AuthenticationFailedException( + String.format("Token issued to client %s is not active", oidcConfig.clientId.get())); } verifyTokenExpiry(introspectionResult.getLong(OidcConstants.INTROSPECTION_TOKEN_EXP)); try { diff --git a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java index 9ae62850ca88b..75e3afffc6d24 100644 --- a/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java +++ b/integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java @@ -733,6 +733,7 @@ public void testIdTokenInjection() throws IOException { Cookie sessionCookie = getSessionCookie(webClient, null); assertNotNull(sessionCookie); + // Replace the session cookie with the correctly formatted cookie but with invalid token values webClient.getCookieManager().clearCookies(); webClient.getCookieManager().addCookie(new Cookie(sessionCookie.getDomain(), sessionCookie.getName(), "1|2|3")); @@ -747,6 +748,22 @@ public void testIdTokenInjection() throws IOException { assertNull(getSessionCookie(webClient, null)); } webClient.getCookieManager().clearCookies(); + + // Replace the session cookie with malformed cookie + webClient.getCookieManager().clearCookies(); + webClient.getCookieManager().addCookie(new Cookie(sessionCookie.getDomain(), sessionCookie.getName(), + "1")); + sessionCookie = getSessionCookie(webClient, null); + assertEquals("1", sessionCookie.getValue()); + + try { + webClient.getPage("http://localhost:8081/web-app"); + fail("401 status error is expected"); + } catch (FailingHttpStatusCodeException ex) { + assertEquals(401, ex.getStatusCode()); + assertNull(getSessionCookie(webClient, null)); + } + webClient.getCookieManager().clearCookies(); } }