From f030a3364dc4188943d7988fc21352ab85d41df2 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Wed, 8 May 2024 17:59:11 +0100 Subject: [PATCH] Update docs to make it easy to see that the code flow access token fails, update tests --- ...rity-oidc-bearer-token-authentication.adoc | 6 +++ ...ecurity-oidc-code-flow-authentication.adoc | 11 ++++ ...VerifyInjectedAccessTokenDisabledTest.java | 54 +++++++++++++++++++ .../ProtectedResourceWithJwtAccessToken.java | 31 +++++++++++ .../test/UserInfoRequiredDetectionTest.java | 19 +++++++ ...-injected-access-token-disabled.properties | 5 ++ .../io/quarkus/oidc/OidcTenantConfig.java | 28 ++++++---- .../runtime/CodeAuthenticationMechanism.java | 15 +++++- 8 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowVerifyInjectedAccessTokenDisabledTest.java create mode 100644 extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResourceWithJwtAccessToken.java create mode 100644 extensions/oidc/deployment/src/test/resources/application-verify-injected-access-token-disabled.properties diff --git a/docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc b/docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc index 4d764c915a9e9..b1c4bd9180b8f 100644 --- a/docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc +++ b/docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc @@ -95,6 +95,12 @@ If you must request a UserInfo JSON object from the OIDC `UserInfo` endpoint, se A request is sent to the OIDC provider `UserInfo` endpoint, and an `io.quarkus.oidc.UserInfo` (a simple `javax.json.JsonObject` wrapper) object is created. `io.quarkus.oidc.UserInfo` can be injected or accessed as a `SecurityIdentity` `userinfo` attribute. +`quarkus.oidc.authentication.user-info-required` is automatically enabled if one of these conditions is met: + +- if `quarkus.oidc.roles.source` is set to `userinfo` or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true` or `quarkus.oidc.authentication.id-token-required` is set to `false`, the current OIDC tenant must support a UserInfo endpoint in these cases. + +- if `io.quarkus.oidc.UserInfo` injection point is detected but only if the current OIDC tenant supports a UserInfo endpoint. + [[config-metadata]] === Configuration metadata diff --git a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc index dde80eaf849d9..95e773e227277 100644 --- a/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc +++ b/docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc @@ -492,6 +492,11 @@ public class ProtectedResource { } ---- +[NOTE] +==== +When an authorization code flow access token is injected as `JsonWebToken`, its verification is automatically enabled, in addition to the mandatory ID token verification. If really needed, you can disable this code flow access token verification with `quarkus.oidc.authentication.verify-access-token=false`. +==== + [NOTE] ==== `AccessTokenCredential` is used if the access token issued to the Quarkus `web-app` application is opaque (binary) and cannot be parsed to a `JsonWebToken` or if the inner content is necessary for the application. @@ -510,6 +515,12 @@ Set the `quarkus.oidc.authentication.user-info-required=true` property to reques A request is sent to the OIDC provider `UserInfo` endpoint by using the access token returned with the authorization code grant response, and an `io.quarkus.oidc.UserInfo` (a simple `jakarta.json.JsonObject` wrapper) object is created. `io.quarkus.oidc.UserInfo` can be injected or accessed as a SecurityIdentity `userinfo` attribute. +`quarkus.oidc.authentication.user-info-required` is automatically enabled if one of these conditions is met: + +- if `quarkus.oidc.roles.source` is set to `userinfo` or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true` or `quarkus.oidc.authentication.id-token-required` is set to `false`, the current OIDC tenant must support a UserInfo endpoint in these cases. + +- if `io.quarkus.oidc.UserInfo` injection point is detected but only if the current OIDC tenant supports a UserInfo endpoint. + [[config-metadata]] ==== Accessing the OIDC configuration information diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowVerifyInjectedAccessTokenDisabledTest.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowVerifyInjectedAccessTokenDisabledTest.java new file mode 100644 index 0000000000000..6d1aaf041f503 --- /dev/null +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowVerifyInjectedAccessTokenDisabledTest.java @@ -0,0 +1,54 @@ +package io.quarkus.oidc.test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.gargoylesoftware.htmlunit.SilentCssErrorHandler; +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; + +import io.quarkus.test.QuarkusUnitTest; +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager; + +@QuarkusTestResource(KeycloakTestResourceLifecycleManager.class) +public class CodeFlowVerifyInjectedAccessTokenDisabledTest { + + @RegisterExtension + static final QuarkusUnitTest test = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(ProtectedResourceWithJwtAccessToken.class) + .addAsResource("application-verify-injected-access-token-disabled.properties", "application.properties")); + + @Test + public void testVerifyAccessTokenDisabled() throws IOException, InterruptedException { + try (final WebClient webClient = createWebClient()) { + + HtmlPage page = webClient.getPage("http://localhost:8081/protected"); + + assertEquals("Sign in to quarkus", page.getTitleText()); + + HtmlForm loginForm = page.getForms().get(0); + + loginForm.getInputByName("username").setValueAttribute("alice"); + loginForm.getInputByName("password").setValueAttribute("alice"); + + page = loginForm.getInputByName("login").click(); + + assertEquals("alice:false", page.getBody().asNormalizedText()); + + webClient.getCookieManager().clearCookies(); + } + } + + private WebClient createWebClient() { + WebClient webClient = new WebClient(); + webClient.setCssErrorHandler(new SilentCssErrorHandler()); + return webClient; + } +} diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResourceWithJwtAccessToken.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResourceWithJwtAccessToken.java new file mode 100644 index 0000000000000..dc82b0c8ce510 --- /dev/null +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/ProtectedResourceWithJwtAccessToken.java @@ -0,0 +1,31 @@ +package io.quarkus.oidc.test; + +import jakarta.inject.Inject; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +import org.eclipse.microprofile.jwt.JsonWebToken; + +import io.quarkus.oidc.IdToken; +import io.quarkus.oidc.runtime.OidcConfig; +import io.quarkus.security.Authenticated; + +@Path("/protected") +@Authenticated +public class ProtectedResourceWithJwtAccessToken { + + @Inject + @IdToken + JsonWebToken idToken; + + @Inject + JsonWebToken accessToken; + + @Inject + OidcConfig config; + + @GET + public String getName() { + return idToken.getName() + ":" + config.defaultTenant.authentication.verifyAccessToken; + } +} diff --git a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java index 02a81c23f5610..73633f3bbb4a1 100644 --- a/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java +++ b/extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/UserInfoRequiredDetectionTest.java @@ -41,6 +41,12 @@ public class UserInfoRequiredDetectionTest { quarkus.oidc.named-2.tenant-paths=/user-info/named-tenant-2 quarkus.oidc.named-2.discovery-enabled=false quarkus.oidc.named-2.jwks-path=protocol/openid-connect/certs + quarkus.oidc.named-3.auth-server-url=${quarkus.oidc.auth-server-url} + quarkus.oidc.named-3.tenant-paths=/user-info/named-tenant-3 + quarkus.oidc.named-3.discovery-enabled=false + quarkus.oidc.named-3.jwks-path=protocol/openid-connect/certs + quarkus.oidc.named-3.user-info-path=http://${quarkus.http.host}:${quarkus.http.port}/user-info-endpoint + quarkus.oidc.named-3.authentication.user-info-required=false quarkus.http.auth.proactive=false """), "application.properties")); @@ -63,6 +69,12 @@ public void testUserInfoNotRequiredWhenMissingUserInfoEndpoint() { .body(Matchers.is("false")); } + @Test + public void testUserInfoNotRequiredIfDisabledWhenUserInfoEndpointIsPresent() { + RestAssured.given().auth().oauth2(getAccessToken()).get("/user-info/named-tenant-3").then().statusCode(200) + .body(Matchers.is("false")); + } + private static String getAccessToken() { return new KeycloakTestClient().getAccessToken("alice", "alice", "quarkus-service-app", "secret", List.of("openid")); } @@ -111,6 +123,13 @@ public String getNamedTenantName() { public boolean getNamed2TenantUserInfoRequired() { return config.namedTenants.get("named-2").authentication.userInfoRequired.orElse(false); } + + @PermissionsAllowed("openid") + @Path("named-tenant-3") + @GET + public boolean getNamed3TenantUserInfoRequired() { + return config.namedTenants.get("named-3").authentication.userInfoRequired.orElse(false); + } } } diff --git a/extensions/oidc/deployment/src/test/resources/application-verify-injected-access-token-disabled.properties b/extensions/oidc/deployment/src/test/resources/application-verify-injected-access-token-disabled.properties new file mode 100644 index 0000000000000..5f262bf4b7779 --- /dev/null +++ b/extensions/oidc/deployment/src/test/resources/application-verify-injected-access-token-disabled.properties @@ -0,0 +1,5 @@ +quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus +quarkus.oidc.client-id=quarkus-web-app +quarkus.oidc.credentials.secret=secret +quarkus.oidc.application-type=web-app +quarkus.oidc.authentication.verify-access-token=false diff --git a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java index f1a60bea14516..9fb3df20d5b58 100644 --- a/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java +++ b/extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java @@ -982,16 +982,18 @@ public enum ResponseMode { /** * Both ID and access tokens are fetched from the OIDC provider as part of the authorization code flow. + *

* ID token is always verified on every user request as the primary token which is used * to represent the principal and extract the roles. - * Access token is not verified by default since it is meant to be propagated to the downstream services. - * The verification of the access token should be enabled if it is injected as a JWT token. - * - * Access tokens obtained as part of the code flow are always verified if `quarkus.oidc.roles.source` - * property is set to `accesstoken` which means the authorization decision is based on the roles extracted from the - * access token. - * - * Bearer access tokens are always verified. + *

+ * Authorization code flow access token is meant to be propagated to downstream services + * and is not verified by default unless `quarkus.oidc.roles.source` property is set to `accesstoken` + * which means the authorization decision is based on the roles extracted from the access token. + *

+ * Authorization code flow access token verification is also enabled if this token is injected as JsonWebToken. + * Set this property to `false` if it is not required. + *

+ * Bearer access token is always verified. */ @ConfigItem(defaultValueDocumentation = "true when access token is injected as the JsonWebToken bean, false otherwise") public boolean verifyAccessToken; @@ -1129,10 +1131,14 @@ public enum ResponseMode { /** * If this property is set to `true`, an OIDC UserInfo endpoint is called. - * This property is enabled if `quarkus.oidc.roles.source` is `userinfo`. - * or `quarkus.oidc.token.verify-access-token-with-user-info` is `true` + *

+ * This property is enabled automatically if `quarkus.oidc.roles.source` is set to `userinfo` + * or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true` * or `quarkus.oidc.authentication.id-token-required` is set to `false`, - * you do not need to enable this property manually in these cases. + * the current OIDC tenant must support a UserInfo endpoint in these cases. + *

+ * It is also enabled automatically if `io.quarkus.oidc.UserInfo` injection point is detected but only + * if the current OIDC tenant supports a UserInfo endpoint. */ @ConfigItem(defaultValueDocumentation = "true when UserInfo bean is injected, false otherwise") public Optional userInfoRequired = Optional.empty(); 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 253079dfb1bfd..1c6f3276bc856 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 @@ -338,7 +338,7 @@ public Uni apply(Throwable t) { .hasErrorCode(ErrorCodes.EXPIRED); if (!expired) { - LOG.errorf("ID token verification failure: %s", errorMessage(t)); + logAuthenticationError(context, t); return removeSessionCookie(context, configContext.oidcConfig) .replaceWith(Uni.createFrom() .failure(t @@ -837,7 +837,7 @@ public Throwable apply(Throwable tInner) { return tInner; } - LOG.errorf("ID token verification has failed: %s", errorMessage(tInner)); + logAuthenticationError(context, tInner); return new AuthenticationCompletionException(tInner); } }); @@ -846,6 +846,17 @@ public Throwable apply(Throwable tInner) { }); } + private static void logAuthenticationError(RoutingContext context, Throwable t) { + final String errorMessage = errorMessage(t); + final boolean accessTokenFailure = context.get(OidcConstants.ACCESS_TOKEN_VALUE) != null + && context.get(OidcUtils.CODE_ACCESS_TOKEN_RESULT) == null; + if (accessTokenFailure) { + LOG.errorf("Access token verification has failed: %s. ID token has not been verified yet", errorMessage); + } else { + LOG.errorf("ID token verification has failed: %s", errorMessage); + } + } + private static boolean prepareNonceForVerification(RoutingContext context, OidcTenantConfig oidcConfig, CodeAuthenticationStateBean stateBean, String idToken) { if (oidcConfig.authentication.nonceRequired) {