Merge pull request #40523 from sberyozkin/improve_code_flow_at_failur…

…e_message

Update docs to make it easy to see that the code flow access token fails, update tests

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
 

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
 

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;
+    }
+}

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;
+    }
+}

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);
+        }
     }
 
 }

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

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.
+         * <p>
          * 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.
+         * <p>
+         * 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.
+         * <p>
+         * 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.
+         * <p>
+         * 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`
+         * <p>
+         * 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.
+         * <p>
+         * 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<Boolean> userInfoRequired = Optional.empty();

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java

@@ -338,7 +338,7 @@ public Uni<? extends SecurityIdentity> 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) {