Merge pull request #41174 from michalvavrik/feature/fix-test-security…

…-http-credentials-comb

Fix mixing of the @testsecurity annotation with HTTP request credentials inside one test method

docs/src/main/asciidoc/security-testing.adoc

@@ -139,6 +139,43 @@ If it becomes necessary to test security features using both `@TestSecurity` and
 mechanism when none is defined), then Basic Auth needs to be enabled explicitly,
 for example by setting `quarkus.http.auth.basic=true` or `%test.quarkus.http.auth.basic=true`.
 
+Similarly, if it becomes necessary to test security features using both `@TestSecurity` and Bearer token authentication,
+you can leverage both like in the example below:
+
+[source, java]
+----
+@Test
+@TestSecurity(user = "Bob")
+public void testSecurityMetaAnnotation {
+    RestAssured.given()
+            .auth().oauth2(getTokenForUser("Alice")) <1>
+            .get("hello")
+            .then()
+            .statusCode(200)
+            .body(Matchers.is("Hello Alice"));
+    RestAssured.given()
+            .get("hello")
+            .then()
+            .statusCode(200)
+            .body(Matchers.is("Hello Bob")); <2>
+}
+
+@Path("hello")
+public static class HelloResource {
+
+  @Inject
+  SecurityIdentity identity;
+
+  @Authenticated
+  @GET
+  public String sayHello() {
+       return "Hello " + identity.getPrincipal().getName();
+  }
+}
+----
+<1> Bearer token authentication mechanism is used, because a Bearer access token is sent with the HTTP request.
+<2> No authorization header is present, therefore the Test Security Extension creates user `Bob`.
+
 === Path-based authentication
 
 `@TestSecurity` can also be used when xref:security-authentication-mechanisms.adoc#combining-authentication-mechanisms[authentication mechanisms must be combined].

integration-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/TenantResource.java

@@ -197,6 +197,15 @@ public String scopePermissionsWebApp2(@PathParam("tenant") String tenant) {
                 .collect(Collectors.joining(" "));
     }
 
+    @AuthorizationCodeFlow
+    @GET
+    @Path("webapp-local-logout")
+    @RolesAllowed("user")
+    public String localLogout() {
+        oidcSession.logout().await().indefinitely();
+        return securityIdentity.getPrincipal().getName();
+    }
+
     private String getNameWebAppType(String name,
             String idTokenNameClaim,
             String idTokenNameClaimNotExpected) {

integration-tests/oidc-tenancy/src/main/resources/application.properties

@@ -54,7 +54,7 @@ quarkus.oidc.tenant-web-app.application-type=web-app
 quarkus.oidc.tenant-web-app.roles.source=userinfo
 quarkus.oidc.tenant-web-app.allow-user-info-cache=false
 # Adding this property should not affect the flow if no expected request header
-# "HX-Request" identifiying it as a JavaScript request is found
+# "HX-Request" identifying it as a JavaScript request is found
 quarkus.oidc.tenant-web-app.authentication.java-script-auto-redirect=false
 
 # Tenant Web App Java Script

integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/BearerTokenAuthorizationTest.java

@@ -900,7 +900,7 @@ private String getOpaqueAccessTokenFromSimpleOidc() {
         return object.getString("access_token");
     }
 
-    private WebClient createWebClient() {
+    static WebClient createWebClient() {
         WebClient webClient = new WebClient();
         webClient.setCssErrorHandler(new SilentCssErrorHandler());
         return webClient;
@@ -916,7 +916,7 @@ private Cookie getStateCookie(WebClient webClient, String tenantId) {
         return null;
     }
 
-    private Cookie getSessionCookie(WebClient webClient, String tenantId) {
+    static Cookie getSessionCookie(WebClient webClient, String tenantId) {
         return webClient.getCookieManager().getCookie("q_session" + (tenantId == null ? "" : "_" + tenantId));
     }
 

integration-tests/oidc-tenancy/src/test/java/io/quarkus/it/keycloak/TestSecurityCombiningAuthMechTest.java

@@ -1,17 +1,31 @@
 package io.quarkus.it.keycloak;
 
+import static io.quarkus.it.keycloak.AnnotationBasedTenantTest.getTokenWithRole;
+import static io.quarkus.it.keycloak.BearerTokenAuthorizationTest.createWebClient;
+import static io.quarkus.it.keycloak.BearerTokenAuthorizationTest.getSessionCookie;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.io.IOException;
+
+import org.hamcrest.Matchers;
+import org.htmlunit.WebClient;
+import org.htmlunit.html.HtmlForm;
+import org.htmlunit.html.HtmlPage;
 import org.junit.jupiter.api.Test;
 
+import io.quarkus.test.common.QuarkusTestResource;
 import io.quarkus.test.common.http.TestHTTPEndpoint;
 import io.quarkus.test.junit.QuarkusTest;
 import io.quarkus.test.security.TestSecurity;
 import io.restassured.RestAssured;
 import io.restassured.http.ContentType;
 
-@TestHTTPEndpoint(MultipleAuthMechResource.class)
 @QuarkusTest
+@QuarkusTestResource(KeycloakRealmResourceManager.class)
 public class TestSecurityCombiningAuthMechTest {
 
+    @TestHTTPEndpoint(MultipleAuthMechResource.class)
     @TestSecurity(user = "testUser", authMechanism = "basic")
     @Test
     public void testBasicAuthentication() {
@@ -43,6 +57,7 @@ public void testBasicAuthentication() {
                 .statusCode(401);
     }
 
+    @TestHTTPEndpoint(MultipleAuthMechResource.class)
     @TestSecurity(user = "testUser", authMechanism = "Bearer")
     @Test
     public void testBearerBasedAuthentication() {
@@ -72,6 +87,7 @@ public void testBearerBasedAuthentication() {
                 .statusCode(200);
     }
 
+    @TestHTTPEndpoint(MultipleAuthMechResource.class)
     @TestSecurity(user = "testUser", authMechanism = "custom")
     @Test
     public void testCustomAuthentication() {
@@ -102,4 +118,47 @@ public void testCustomAuthentication() {
                 .then()
                 .statusCode(401);
     }
+
+    @TestHTTPEndpoint(TenantEchoResource.class)
+    @TestSecurity(user = "testUser", authMechanism = "Bearer", roles = "role1")
+    @Test
+    public void testHttpCredentialsHasPriorityOverTestSecurity() {
+        // token has priority over @TestSecurity
+        RestAssured.given().auth().oauth2(getTokenWithRole("role1"))
+                .when().get("jax-rs-perm-check")
+                .then().statusCode(200)
+                .body(Matchers.equalTo(("tenant-id=tenant-public-key, static.tenant.id=tenant-public-key, name=alice")));
+        // no token -> use @TestSecurity
+        RestAssured.given()
+                .when().get("jax-rs-perm-check")
+                .then().statusCode(200)
+                .body(Matchers.equalTo(("tenant-id=tenant-public-key, static.tenant.id=null, name=testUser")));
+    }
+
+    @TestSecurity(user = "testUser", authMechanism = "Bearer", roles = "role1")
+    @Test
+    public void testSessionCookieHasPriorityOverTestSecurity() throws IOException {
+        // @TestSecurity still use Bearer authentication as we didn't specify credentials
+        RestAssured.given()
+                .redirects().follow(false)
+                .when().get("/tenant/tenant-web-app/api/user/webapp-local-logout")
+                .then().statusCode(302);
+        RestAssured.given()
+                .when().get("/api/tenant-echo/jax-rs-perm-check")
+                .then().statusCode(200)
+                .body(Matchers.equalTo(("tenant-id=tenant-public-key, static.tenant.id=null, name=testUser")));
+
+        // path specific authentication is still possible, the @TestSecurity is not used as it uses Bearer, not code
+        try (final WebClient webClient = createWebClient()) {
+            HtmlPage page = webClient.getPage("http://localhost:8081/tenant/tenant-web-app/api/user/webapp-local-logout");
+            assertNull(getSessionCookie(webClient, "tenant-web-app"));
+            assertEquals("Sign in to quarkus-webapp", 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", page.getBody().asNormalizedText());
+            assertNull(getSessionCookie(webClient, "tenant-web-app"));
+        }
+    }
 }

test-framework/security/src/main/java/io/quarkus/test/security/TestHttpAuthenticationMechanism.java → test-framework/security/src/main/java/io/quarkus/test/security/AbstractTestHttpAuthenticationMechanism.java

@@ -4,7 +4,6 @@
 import java.util.Set;
 
 import jakarta.annotation.PostConstruct;
-import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 
 import io.quarkus.runtime.LaunchMode;
@@ -17,13 +16,12 @@
 import io.smallrye.mutiny.Uni;
 import io.vertx.ext.web.RoutingContext;
 
-@ApplicationScoped
-public class TestHttpAuthenticationMechanism implements HttpAuthenticationMechanism {
+abstract class AbstractTestHttpAuthenticationMechanism implements HttpAuthenticationMechanism {
 
     @Inject
     TestIdentityAssociation testIdentityAssociation;
 
-    volatile String authMechanism = null;
+    protected volatile String authMechanism = null;
 
     @PostConstruct
     public void check() {
@@ -54,11 +52,6 @@ public Uni<HttpCredentialTransport> getCredentialTransport(RoutingContext contex
                 : Uni.createFrom().item(new HttpCredentialTransport(HttpCredentialTransport.Type.TEST_SECURITY, authMechanism));
     }
 
-    @Override
-    public int getPriority() {
-        return 3000;
-    }
-
     void setAuthMechanism(String authMechanism) {
         this.authMechanism = authMechanism;
     }

test-framework/security/src/main/java/io/quarkus/test/security/FallbackTestHttpAuthenticationMechanism.java

@@ -0,0 +1,13 @@
+package io.quarkus.test.security;
+
+import jakarta.enterprise.context.ApplicationScoped;
+
+/**
+ * This test mechanism is fallback when no other mechanism manages to authenticate.
+ * When the test method is annotated with the {@link TestSecurity} annotation,
+ * users can still send credentials inside HTTP request and the credentials will have priority.
+ */
+@ApplicationScoped
+public class FallbackTestHttpAuthenticationMechanism extends AbstractTestHttpAuthenticationMechanism {
+
+}

test-framework/security/src/main/java/io/quarkus/test/security/PathBasedTestHttpAuthenticationMechanism.java

@@ -0,0 +1,60 @@
+package io.quarkus.test.security;
+
+import static io.netty.handler.codec.http.HttpHeaderNames.AUTHORIZATION;
+
+import jakarta.enterprise.context.ApplicationScoped;
+
+import io.quarkus.security.identity.IdentityProviderManager;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import io.vertx.core.http.Cookie;
+import io.vertx.ext.web.RoutingContext;
+
+/**
+ * When authentication mechanism is selected with the {@link TestSecurity#authMechanism()} annotation attribute,
+ * we must be sure that the test mechanism is primary identity provider for that authentication type.
+ * <p>
+ * For example when a test method is annotated with `@TestSecurity(authMechanism = "basic")`,
+ * we want to be the ones providing basic authentication when no authorization headers are present,
+ * and not the {@link io.quarkus.vertx.http.runtime.security.BasicAuthenticationMechanism} mechanism.
+ * This test mechanism must exist because when a path-specific authentication mechanism is selected,
+ * for example via {@link io.quarkus.vertx.http.runtime.security.annotation.BasicAuthentication},
+ * it is also required and therefore exactly one mechanism is enforced.
+ */
+@ApplicationScoped
+public class PathBasedTestHttpAuthenticationMechanism extends AbstractTestHttpAuthenticationMechanism {
+
+    @Override
+    public Uni<SecurityIdentity> authenticate(RoutingContext context, IdentityProviderManager identityProviderManager) {
+        if (authMechanism != null && requestNotAuthenticated(context)) {
+            // return the SecurityIdentity defined via @TestSecurity
+            return super.authenticate(context, identityProviderManager);
+        }
+        // do not authenticate - give a change to other mechanisms
+        return Uni.createFrom().nullItem();
+    }
+
+    @Override
+    public int getPriority() {
+        return 3000;
+    }
+
+    private static boolean requestNotAuthenticated(RoutingContext context) {
+        // on a best-effort basis try to guess whether incoming request is authorized
+        return context.request().getHeader(AUTHORIZATION) == null
+                && !hasOidcSessionCookieCandidate(context);
+    }
+
+    private static boolean hasOidcSessionCookieCandidate(RoutingContext context) {
+        if (context.request().cookies() == null) {
+            return false;
+        }
+        for (Cookie cookie : context.request().cookies()) {
+            if (cookie.getName() != null && cookie.getName().startsWith("q_session")) {
+                // there is a possibility this is an OIDC session cookie
+                return true;
+            }
+        }
+        return false;
+    }
+}

test-framework/security/src/main/java/io/quarkus/test/security/QuarkusSecurityTestExtension.java

@@ -26,7 +26,9 @@ public void afterEach(QuarkusTestMethodContext context) {
         try {
             if (getAnnotationContainer(context).isPresent()) {
                 CDI.current().select(TestAuthController.class).get().setEnabled(true);
-                CDI.current().select(TestHttpAuthenticationMechanism.class).get().setAuthMechanism(null);
+                for (var testMechanism : CDI.current().select(AbstractTestHttpAuthenticationMechanism.class)) {
+                    testMechanism.setAuthMechanism(null);
+                }
                 var testIdentity = CDI.current().select(TestIdentityAssociation.class).get();
                 testIdentity.setTestIdentity(null);
                 testIdentity.setPathBasedIdentity(false);
@@ -66,8 +68,9 @@ public void beforeEach(QuarkusTestMethodContext context) {
                 SecurityIdentity userIdentity = augment(user.build(), allAnnotations);
                 CDI.current().select(TestIdentityAssociation.class).get().setTestIdentity(userIdentity);
                 if (!testSecurity.authMechanism().isEmpty()) {
-                    CDI.current().select(TestHttpAuthenticationMechanism.class).get()
-                            .setAuthMechanism(testSecurity.authMechanism());
+                    for (var testMechanism : CDI.current().select(AbstractTestHttpAuthenticationMechanism.class)) {
+                        testMechanism.setAuthMechanism(testSecurity.authMechanism());
+                    }
                     CDI.current().select(TestIdentityAssociation.class).get().setPathBasedIdentity(true);
                 }
             }