Test suite improvements (#28)

* Fix occasional problem shutting down the containers when running `docker compose down` at the end of integration tests. * Added Jacoco for test coverage. * Added new test cases to improve the coverage. Signed-off-by: Tero Saarni <[email protected]>
Nordix · Nov 2, 2024 · 9d57520 · 9d57520
1 parent c71f729
commit 9d57520
Show file tree

Hide file tree

Showing 11 changed files with 268 additions and 32 deletions.
diff --git a/README.md b/README.md
@@ -106,6 +106,8 @@ To run unit tests, use:
 ./mvnw clean test
 ```
 
+The coverage report is generated in `target/site/jacoco/index.html`.
+
 Integration tests require Docker Compose to start Keycloak and Envoy.
 Ensure Docker is installed.
 To run integration tests, use:

diff --git a/docker-compose.yaml b/docker-compose.yaml
@@ -49,8 +49,8 @@ services:
           --log-level=INFO,io.github.nordix.keycloak.services.x509:debug
 
     environment:
-      - KEYCLOAK_ADMIN=admin
-      - KEYCLOAK_ADMIN_PASSWORD=admin
+      - KC_BOOTSTRAP_ADMIN_USERNAME=admin
+      - KC_BOOTSTRAP_ADMIN_PASSWORD=admin
       - KC_HOSTNAME=https://keycloak.127.0.0.1.nip.io:8443
       - KC_HTTP_ENABLED=true
 

diff --git a/pom.xml b/pom.xml
@@ -34,6 +34,7 @@
     <maven.project-info-reports-plugin.version>3.8.0</maven.project-info-reports-plugin.version>
     <maven.failsafe.plugin.version>3.5.1</maven.failsafe.plugin.version>
     <maven.checkstyle.plugin.version>3.6.0</maven.checkstyle.plugin.version>
+    <maven.jacoco.plugin.version>0.8.12</maven.jacoco.plugin.version>
 
   </properties>
 
@@ -190,6 +191,27 @@
         </executions>
       </plugin>
 
+      <!-- Jacoco coverage report -->
+      <plugin>
+        <groupId>org.jacoco</groupId>
+        <artifactId>jacoco-maven-plugin</artifactId>
+        <version>${maven.jacoco.plugin.version}</version>
+        <executions>
+          <execution>
+            <goals>
+              <goal>prepare-agent</goal>
+            </goals>
+          </execution>
+          <execution>
+            <id>report</id>
+            <phase>test</phase>
+            <goals>
+              <goal>report</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+
       <!-- Checkstyle -->
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>

diff --git a/src/test/java/io/github/nordix/keycloak/services/x509/ClientCertificateLookupIT.java b/src/test/java/io/github/nordix/keycloak/services/x509/ClientCertificateLookupIT.java
@@ -118,7 +118,7 @@ public static void waitForKeycloak() throws Exception {
                 if (response.getStatusInfo().getFamily() != Response.Status.Family.SERVER_ERROR) {
                     break;
                 }
-                logger.infov("Response={0}", response.getStatus());
+                logger.infov("Response={0} {1}", response.getStatus(), response.getStatusInfo().getReasonPhrase());
 
             } catch (Exception e) {
                 logger.infov("Cannot connect: {0}", e.getMessage());
@@ -150,11 +150,13 @@ void testEnvoyAuthorizedClientCert() throws Exception {
 
         Response response = target.request().post(Entity.form(form));
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(200, response.getStatus(), "Failed to fetch token. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.OK, response.getStatusInfo(),
+                "Was expecting 200 OK. Response=" + responseBody);
 
         ObjectMapper mapper = new ObjectMapper();
         JsonNode obj = mapper.readTree(responseBody);
-        Assertions.assertTrue(obj.has("access_token"), "Response does not contain access_token");
+        Assertions.assertTrue(obj.has("access_token"),
+                "Was expecting access_token in response but got: " + responseBody);
     }
 
     /**
@@ -175,7 +177,8 @@ void testEnvoyUnauthorizedClientCert() throws Exception {
 
         Response response = target.request().post(Entity.form(form));
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(401, response.getStatus(), "Was expecting 401 Unauthorized. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.UNAUTHORIZED, response.getStatusInfo(),
+                "Was expecting 401 Unauthorized. Response=" + responseBody);
     }
 
     /**
@@ -193,7 +196,8 @@ void testEnvoyWithoutClientCert() throws Exception {
 
         Response response = target.request().post(Entity.form(form));
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(401, response.getStatus(), "Was expecting 401 Unauthorized. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.UNAUTHORIZED, response.getStatusInfo(),
+                "Was expecting 401 Unauthorized. Response=" + responseBody);
     }
 
     /**
@@ -213,7 +217,8 @@ void testInternalClientHttpUnauthorizedXfcc() throws Exception {
         Response response = target.request().header("x-forwarded-client-cert", Helpers.getXfccWithCert(xfccCred))
                 .post(Entity.form(form));
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(401, response.getStatus(), "Was expecting 401 Unauthorized. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.UNAUTHORIZED, response.getStatusInfo(),
+                "Was expecting 401 Unauthorized. Response=" + responseBody);
     }
 
     /**
@@ -235,7 +240,8 @@ void testInternalClientHttpsUnauthorizedXfcc() throws Exception {
                 .post(Entity.form(form));
 
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(401, response.getStatus(), "Was expecting 401 Unauthorized. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.UNAUTHORIZED, response.getStatusInfo(),
+                "Was expecting 401 Unauthorized. Response=" + responseBody);
     }
 
     /**
@@ -246,19 +252,21 @@ void testInternalClientHttpsUnauthorizedXfcc() throws Exception {
      * 5. Keycloak X509 Authenticator accepts request (CN=authorized-client) and returns the token.
      */
     @Test
-    void TestInternalClientHttpsAuthorized() throws Exception {
+    void testInternalClientHttpsAuthorized() throws Exception {
         Credential tlsCred = new Credential().subject("CN=authorized-client").issuer(clientCa);
 
         WebTarget target = newTargetWithClientAuth(
                 KEYCLOAK_DIRECT_HTTPS_BASE_URL + "/realms/xfcc/protocol/openid-connect/token", tlsCred);
 
         Response response = target.request().post(Entity.form(form));
         String responseBody = response.readEntity(String.class);
-        Assertions.assertEquals(200, response.getStatus(), "Failed to fetch token. Response=" + responseBody);
+        Assertions.assertEquals(Response.Status.OK, response.getStatusInfo(),
+                "Was expecting 200 OK. Response=" + responseBody);
 
         ObjectMapper mapper = new ObjectMapper();
         JsonNode obj = mapper.readTree(responseBody);
-        Assertions.assertTrue(obj.has("access_token"), "Response does not contain access_token");
+        Assertions.assertTrue(obj.has("access_token"),
+                "Was expecting access_token in response but got: " + responseBody);
     }
 
     // Helper methods

diff --git a/src/test/java/io/github/nordix/keycloak/services/x509/ClientCertificateLookupTest.java b/src/test/java/io/github/nordix/keycloak/services/x509/ClientCertificateLookupTest.java
@@ -124,15 +124,43 @@ void testMultipleXfccElements() throws Exception {
     }
 
     /**
-     * Corrupted certificate in XFCC header.
+     * Empty XFCC header.
      */
     @Test
-    void testCorruptedCertificate() throws Exception {
+    void testEmptyCertificate() throws Exception {
         HttpRequest request = new HttpRequestImpl(
+                MockHttpRequest.create("GET", "http://foo/bar").header("x-forwarded-client-cert", ""));
+        Assertions.assertNull(envoyLookup.getCertificateChain(request));
+    }
+
+    /**
+     * Corrupted XFCC header.
+     */
+    @Test
+    void testCorruptedXfcc() throws Exception {
+        HttpRequest request1 = new HttpRequestImpl(
                 MockHttpRequest.create("GET", "http://foo/bar").header("x-forwarded-client-cert", "Cert=\"foobar\""));
 
         Assertions.assertThrows(SecurityException.class, () -> {
-            envoyLookup.getCertificateChain(request);
+            envoyLookup.getCertificateChain(request1);
+        });
+
+        HttpRequest request2 = new HttpRequestImpl(MockHttpRequest.create("GET", "http://foo/bar").header("x-forwarded-client-cert",
+                "Hash=1234;Chain=\"foobar\""));
+        Assertions.assertThrows(SecurityException.class, () -> {
+            envoyLookup.getCertificateChain(request2);
+        });
+
+        HttpRequest request3 = new HttpRequestImpl(MockHttpRequest.create("GET", "http://foo/bar").header("x-forwarded-client-cert",
+                "Hash=1234;Cert=\"no end quote"));
+        Assertions.assertThrows(SecurityException.class, () -> {
+            envoyLookup.getCertificateChain(request3);
+        });
+
+        HttpRequest request4 = new HttpRequestImpl(MockHttpRequest.create("GET", "http://foo/bar").header("x-forwarded-client-cert",
+                "Hash=1234;Cert=no start quote\""));
+        Assertions.assertThrows(SecurityException.class, () -> {
+            envoyLookup.getCertificateChain(request4);
         });
     }
 

diff --git a/src/test/java/io/github/nordix/keycloak/services/x509/DockerComposeExtension.java b/src/test/java/io/github/nordix/keycloak/services/x509/DockerComposeExtension.java
@@ -25,7 +25,7 @@
 public class DockerComposeExtension implements BeforeAllCallback, AfterAllCallback {
 
     private static final String DOCKER_COMPOSE_UP = "docker compose up --force-recreate --no-color --abort-on-container-exit";
-    private static final String DOCKER_COMPOSE_DOWN = "docker compose down";
+    private static final String DOCKER_COMPOSE_DOWN = "docker compose rm --force --stop";
 
     private static Logger logger = Logger.getLogger(DockerComposeExtension.class);
 

diff --git a/src/test/java/io/github/nordix/keycloak/services/x509/Helpers.java b/src/test/java/io/github/nordix/keycloak/services/x509/Helpers.java
@@ -26,6 +26,9 @@
 
 public class Helpers {
 
+    /**
+     * Dummy password for the KeyStore for unit tests.
+     */
     static final String STORE_PASSWORD = "password";
 
     /**
@@ -38,31 +41,50 @@ static X509ClientCertificateLookup createLookupWithConfig(String configJson) {
         return factory.create(null);
     }
 
+    /**
+     * Get the certificate chain from the Credential in X509Certificate array.
+     */
     static X509Certificate[] getCertificateChain(Credential cred)
             throws CertificateException, NoSuchAlgorithmException {
         return Arrays.stream(cred.getCertificates()).map(cert -> (X509Certificate) cert)
                 .toArray(X509Certificate[]::new);
     }
 
+    /**
+     * Get XFCC element with the leaf certificate in "Cert" key.
+     * Hash is a dummy value.
+     */
     static String getXfccWithCert(Credential cred)
             throws CertificateException, NoSuchAlgorithmException, IOException {
         return String.format("Hash=1234;Cert=\"%s\"",
                 URLEncoder.encode(cred.getCertificateAsPem(), StandardCharsets.UTF_8));
     }
 
+    /**
+     * Get XFCC element with the certificate chain in "Chain" key.
+     * Hash is a dummy value.
+     */
     static String getXfccWithChain(Credential cred)
             throws CertificateException, NoSuchAlgorithmException, IOException {
         return String.format("Hash=1234;Chain=\"%s\"",
                 URLEncoder.encode(cred.getCertificatesAsPem(), StandardCharsets.UTF_8));
     }
 
+    /**
+     * Get XFCC element with both the leaf certificate in "Cert" and chain in "Chain" keys.
+     * Hash is a dummy value.
+     */
     static String getXfccWithCertAndChain(Credential cred)
             throws CertificateException, NoSuchAlgorithmException, IOException {
         return String.format("Hash=1234;Cert=\"%s\";Chain=\"%s\"",
                 URLEncoder.encode(cred.getCertificateAsPem(), StandardCharsets.UTF_8),
                 URLEncoder.encode(cred.getCertificatesAsPem(), StandardCharsets.UTF_8));
     }
 
+    /**
+     * Create new KeyStore with the Credential.
+     * The password is set to {@link #STORE_PASSWORD}.
+     */
     static KeyStore newKeyStore(Credential cred)
             throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException {
         KeyStore ks = KeyStore.getInstance("PKCS12");
@@ -72,6 +94,9 @@ static KeyStore newKeyStore(Credential cred)
         return ks;
     }
 
+    /**
+     * Create new TrustStore with the Credential.
+     */
     static KeyStore newTrustStore(Credential cred)
             throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException {
         KeyStore ts = KeyStore.getInstance("PKCS12");