fix: handle exceptions that could arise in the passticket authenticat…

…ion schema (#3871) * draft of fixes (except evaluation of client certificate in the ZAAS) Signed-off-by: Pavel Jareš <[email protected]> * handle exceptions in zaas and unexpected status codes in gateway Signed-off-by: ac892247 <[email protected]> * align passticket tests with updated status codes, remove invalid test Signed-off-by: ac892247 <[email protected]> * handle safid exceptions in the same way as passticket Signed-off-by: ac892247 <[email protected]> * when content type is omitted Signed-off-by: ac892247 <[email protected]> * remove nested structure Signed-off-by: ac892247 <[email protected]> * remove error controller Signed-off-by: ac892247 <[email protected]> * return not_found handler Signed-off-by: ac892247 <[email protected]> * test endpoint not_found Signed-off-by: ac892247 <[email protected]> * add unit tests Signed-off-by: ac892247 <[email protected]> * test internal error and not found exception Signed-off-by: ac892247 <[email protected]> * remove duplicated exception handler for access denied Signed-off-by: ac892247 <[email protected]> * code review comments Signed-off-by: ac892247 <[email protected]> * include zaas IT in jacoco Signed-off-by: ac892247 <[email protected]> * revert unauth handler Signed-off-by: ac892247 <[email protected]> * zaas debug level Signed-off-by: ac892247 <[email protected]> * enabled debug logs Signed-off-by: ac892247 <[email protected]> * revert spring config location Signed-off-by: ac892247 <[email protected]> * move profiles to jvm flags Signed-off-by: ac892247 <[email protected]> * don't need specific for package Signed-off-by: ac892247 <[email protected]> * use the same method for cert PK encoding Signed-off-by: ac892247 <[email protected]> * test / code coverage Signed-off-by: Pavel Jareš <[email protected]> * test / code coverage - 403 Signed-off-by: Pavel Jareš <[email protected]> * tests / code coverage Signed-off-by: Pavel Jareš <[email protected]> * sonar Signed-off-by: Pavel Jareš <[email protected]> * remove unused field Signed-off-by: Pavel Jareš <[email protected]> * javax x jakarta + replace missing error with an internal error Signed-off-by: Pavel Jareš <[email protected]> * fix + test misconfigured service Signed-off-by: Pavel Jareš <[email protected]> * consider server cert only Signed-off-by: ac892247 <[email protected]> * check apiml cert in header too Signed-off-by: ac892247 <[email protected]> * return sooner, mode info in the logs Signed-off-by: ac892247 <[email protected]> * static definition for GitHub action Signed-off-by: Pavel Jareš <[email protected]> * styles and imports Signed-off-by: ac892247 <[email protected]> * update number of registered services in catalog Signed-off-by: ac892247 <[email protected]> --------- Signed-off-by: Pavel Jareš <[email protected]> Signed-off-by: ac892247 <[email protected]> Co-authored-by: ac892247 <[email protected]>
zowe · Nov 6, 2024 · defe1dc · defe1dc
1 parent bf1f2ed
commit defe1dc
Show file tree

Hide file tree

Showing 42 changed files with 830 additions and 634 deletions.
diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml
@@ -468,7 +468,7 @@ jobs:
 
             -   name: Dump DC jacoco data
                 run: >
-                    java -jar ./scripts/jacococli.jar dump --address gateway-service --port 6300 --destfile ./results/gateway-service.exec
+                    java -jar ./scripts/jacococli.jar dump --address zaas-service --port 6301 --destfile ./results/zaas-service.exec
 
             -   name: Store results
                 uses: actions/upload-artifact@v4
@@ -1485,6 +1485,10 @@ jobs:
                 with:
                     name: GatewayProxy-${{ env.JOB_ID }}
                     path: GatewayProxy
+            -   uses: actions/download-artifact@v4
+                with:
+                    name: ContainerCITestsZaas-${{ env.JOB_ID }}
+                    path: ContainerCITestsZaas
             -   uses: actions/download-artifact@v4
                 with:
                     name: CITestsWebSocketChaoticHA-${{ env.JOB_ID }}
@@ -1496,7 +1500,7 @@ jobs:
 
             -   name: Code coverage and publish results
                 run: >
-                    ./gradlew --info coverage sonar -Dresults="containercitests/results,citestswithinfinispan/results,GatewayProxy/results,citestswebsocketchaoticha/results,GatewayServiceRouting/results,containercitestszaas/results"
+                    ./gradlew --info coverage sonar -Dresults="containercitests/results,citestswithinfinispan/results,GatewayProxy/results,citestswebsocketchaoticha/results,GatewayServiceRouting/results,ContainerCITestsZaas/results"
                     -Psonar.host.url=$SONAR_HOST_URL -Dsonar.token=$SONAR_TOKEN -Partifactory_user=$ARTIFACTORY_USERNAME -Partifactory_password=$ARTIFACTORY_PASSWORD
                 env:
                     ARTIFACTORY_USERNAME: ${{ secrets.ARTIFACTORY_USERNAME }}

diff --git a/...src/main/java/org/zowe/apiml/apicatalog/controllers/handlers/NotFoundErrorController.java b/...src/main/java/org/zowe/apiml/apicatalog/controllers/handlers/NotFoundErrorController.java
@@ -15,7 +15,6 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.zowe.apiml.product.compatibility.ApimlErrorController;
 
 import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.http.HttpServletRequest;
@@ -26,11 +25,7 @@
  */
 @Controller
 @Order(Ordered.HIGHEST_PRECEDENCE)
-public class NotFoundErrorController implements ApimlErrorController {
-
-
-    private static final String PATH = "/not_found";    // NOSONAR
-
+public class NotFoundErrorController {
 
     @GetMapping(value = "/not_found")
     public String handleError(HttpServletRequest request) {
@@ -48,10 +43,6 @@ public String handleError(HttpServletRequest request) {
         return "error";
     }
 
-    @Override
-    public String getErrorPath() {
-        return PATH;
-    }
 }
 
 
diff --git a/...alog-services/src/main/java/org/zowe/apiml/apicatalog/security/SecurityConfiguration.java b/...alog-services/src/main/java/org/zowe/apiml/apicatalog/security/SecurityConfiguration.java
@@ -130,8 +130,8 @@ private UserDetailsService x509UserDetailsService() {
 
         private CategorizeCertsFilter reversedCategorizeCertFilter() {
             CategorizeCertsFilter out = new CategorizeCertsFilter(publicKeyCertificatesBase64, certificateValidator);
-            out.setCertificateForClientAuth(crt -> out.getPublicKeyCertificatesBase64().contains(out.base64EncodePublicKey(crt)));
-            out.setApimlCertificate(crt -> !out.getPublicKeyCertificatesBase64().contains(out.base64EncodePublicKey(crt)));
+            out.setCertificateForClientAuth(crt -> out.getPublicKeyCertificatesBase64().contains(CategorizeCertsFilter.base64EncodePublicKey(crt)));
+            out.setApimlCertificate(crt -> !out.getPublicKeyCertificatesBase64().contains(CategorizeCertsFilter.base64EncodePublicKey(crt)));
             return out;
         }
     }

diff --git a/api-catalog-ui/frontend/cypress/e2e/detail-page/service-version-compare.cy.js b/api-catalog-ui/frontend/cypress/e2e/detail-page/service-version-compare.cy.js
@@ -36,7 +36,7 @@ describe('>>> Service version compare Test', () => {
         );
         cy.get('div.MuiTabs-flexContainer.MuiTabs-flexContainerVertical') // Select the parent div
             .find('a.MuiTab-root') // Find all the anchor elements within the div
-            .should('have.length', 13); // Check if there are 13 anchor elements within the div -- changed from 12 to 13 after adding Discoverable client with GraphQL
+            .should('have.length', 15); // Check if there are 15 anchor elements within the div
         cy.get('.version-text').should('exist');
         cy.get('.version-text').should('contain.text', 'Compare');
     });

diff --git a/apiml-common/src/main/java/org/zowe/apiml/product/compatibility/ApimlErrorController.java b/apiml-common/src/main/java/org/zowe/apiml/product/compatibility/ApimlErrorController.java
diff --git a/...ndpointImproprietyConfigureException.java → ...EndpointImproperlyConfigureException.java b/...ndpointImproprietyConfigureException.java → ...EndpointImproperlyConfigureException.java
@@ -12,19 +12,19 @@
 
 import lombok.Getter;
 
-public class EndpointImproprietyConfigureException extends RuntimeException {
+public class EndpointImproperlyConfigureException extends RuntimeException {
 
     private static final long serialVersionUID = -4582785501782402751L;
 
     @Getter
     private final String endpoint;
 
-    public EndpointImproprietyConfigureException(String message, String endpoint) {
+    public EndpointImproperlyConfigureException(String message, String endpoint) {
         super(message);
         this.endpoint = endpoint;
     }
 
-    public EndpointImproprietyConfigureException(String message, String endpoint, Throwable cause) {
+    public EndpointImproperlyConfigureException(String message, String endpoint, Throwable cause) {
         super(message, cause);
         this.endpoint = endpoint;
     }

diff --git a/...mmon/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpoint.java b/...mmon/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpoint.java
@@ -61,13 +61,13 @@ public boolean hasSafResourceAccess(Authentication authentication, String resour
             );
             Response response = responseEntity.getBody();
             if (response != null && response.isError()) {
-                throw new EndpointImproprietyConfigureException("Endpoint " + endpointUrl + " is not properly configured: " + response.getMessage(), endpointUrl);
+                throw new EndpointImproperlyConfigureException("Endpoint " + endpointUrl + " is not properly configured: " + response.getMessage(), endpointUrl);
             }
             return response != null && !response.isError() && response.isAuthorized();
-        } catch (EndpointImproprietyConfigureException e) {
+        } catch (EndpointImproperlyConfigureException e) {
             throw e;
         } catch (Exception e) {
-            throw new EndpointImproprietyConfigureException("Endpoint " + endpointUrl + " is not properly configured.", endpointUrl, e);
+            throw new EndpointImproperlyConfigureException("Endpoint " + endpointUrl + " is not properly configured.", endpointUrl, e);
         }
     }
 

diff --git a/...y-common/src/main/java/org/zowe/apiml/security/common/error/AbstractExceptionHandler.java b/...y-common/src/main/java/org/zowe/apiml/security/common/error/AbstractExceptionHandler.java
@@ -79,7 +79,7 @@ protected void writeErrorResponse(ApiMessageView message, HttpStatus status, Htt
             mapper.writeValue(response.getWriter(), message);
         } catch (IOException e) {
             apimlLog.log("org.zowe.apiml.security.errorWrittingResponse", e.getMessage());
-            throw new ServletException("Error writting response", e);
+            throw new ServletException("Error writing response", e);
         }
     }
 }
diff --git a/...ity-common/src/main/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilter.java b/...ity-common/src/main/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilter.java
@@ -19,6 +19,7 @@
 import lombok.Getter;
 import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
+import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.web.filter.OncePerRequestFilter;
@@ -67,20 +68,18 @@ public class CategorizeCertsFilter extends OncePerRequestFilter {
     private void categorizeCerts(ServletRequest request) {
         X509Certificate[] certs = (X509Certificate[]) request.getAttribute(ATTRNAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE);
         if (certs != null) {
-            if (certificateValidator.isForwardingEnabled() && certificateValidator.isTrusted(certs)) {
+            Optional<Certificate> clientCert = getClientCertFromHeader((HttpServletRequest) request);
+            if (certificateValidator.isForwardingEnabled() && certificateValidator.isTrusted(certs) && clientCert.isPresent()) {
                 certificateValidator.updateAPIMLPublicKeyCertificates(certs);
-                Optional<Certificate> clientCert = getClientCertFromHeader((HttpServletRequest) request);
-                if (clientCert.isPresent()) {
-                    // add the client certificate to the certs array
-                    String subjectDN = ((X509Certificate) clientCert.get()).getSubjectX500Principal().getName();
-                    log.debug("Found client certificate in header, adding it to the request. Subject DN: {}", subjectDN);
-                    certs = Arrays.copyOf(certs, certs.length + 1);
-                    certs[certs.length - 1] = (X509Certificate) clientCert.get();
-                }
+                // add the client certificate to the certs array
+                String subjectDN = ((X509Certificate) clientCert.get()).getSubjectX500Principal().getName();
+                log.debug("Found client certificate in header, adding it to the request. Subject DN: {}", subjectDN);
+                request.setAttribute(ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE, selectCerts(new X509Certificate[]{(X509Certificate) clientCert.get()}, certificateForClientAuth));
+            } else {
+                request.setAttribute(ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE, selectCerts(certs, certificateForClientAuth));
+                request.setAttribute(ATTRNAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE, selectCerts(certs, apimlCertificate));
             }
 
-            request.setAttribute(ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE, selectCerts(certs, certificateForClientAuth));
-            request.setAttribute(ATTRNAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE, selectCerts(certs, apimlCertificate));
             log.debug(LOG_FORMAT_FILTERING_CERTIFICATES, ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE, request.getAttribute(ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE));
         }
     }
@@ -144,24 +143,19 @@ protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull Ht
     }
 
     private X509Certificate[] selectCerts(X509Certificate[] certs, Predicate<X509Certificate> test) {
-        return Arrays.stream(certs)
-            .filter(test)
-            .toList().toArray(new X509Certificate[0]);
+        if (test.test(certs[0])) {
+            return certs;
+        }
+        return new X509Certificate[0];
     }
 
-    public String base64EncodePublicKey(X509Certificate cert) {
+    public static String base64EncodePublicKey(X509Certificate cert) {
         return Base64.getEncoder().encodeToString(cert.getPublicKey().getEncoded());
     }
 
-    public void setCertificateForClientAuth(Predicate<X509Certificate> certificateForClientAuth) {
-        this.certificateForClientAuth = certificateForClientAuth;
-    }
-
-    public void setApimlCertificate(Predicate<X509Certificate> apimlCertificate) {
-        this.apimlCertificate = apimlCertificate;
-    }
-
+    @Setter
     Predicate<X509Certificate> certificateForClientAuth = crt -> !getPublicKeyCertificatesBase64().contains(base64EncodePublicKey(crt));
+    @Setter
     Predicate<X509Certificate> apimlCertificate = crt -> getPublicKeyCertificatesBase64().contains(base64EncodePublicKey(crt));
 
 }
diff --git a/...rity-common/src/main/java/org/zowe/apiml/security/common/verify/CertificateValidator.java b/...rity-common/src/main/java/org/zowe/apiml/security/common/verify/CertificateValidator.java
@@ -22,11 +22,10 @@
 
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
-import java.util.Base64;
 import java.util.List;
 import java.util.Set;
 
-import static java.util.Collections.emptyList;
+import static org.zowe.apiml.security.common.filter.CategorizeCertsFilter.base64EncodePublicKey;
 
 /**
  * Service to verify if given certificate chain can be trusted.
@@ -63,15 +62,19 @@ public CertificateValidator(TrustedCertificatesProvider trustedCertificatesProvi
      * @return true if all given certificates are known false otherwise
      */
     public boolean isTrusted(X509Certificate[] certs) {
-        List<Certificate> trustedCerts = StringUtils.isBlank(proxyCertificatesEndpoint) ? emptyList() : trustedCertificatesProvider.getTrustedCerts(proxyCertificatesEndpoint);
+        if (StringUtils.isBlank(proxyCertificatesEndpoint)) {
+            log.debug("No endpoint configured to retrieve trusted certificates. Provide URL via apiml.security.x509.certificatesUrl");
+            return false;
+        }
+        List<Certificate> trustedCerts = trustedCertificatesProvider.getTrustedCerts(proxyCertificatesEndpoint);
         for (X509Certificate cert : certs) {
             if (!trustedCerts.contains(cert)) {
                 apimlLog.log("org.zowe.apiml.security.common.verify.untrustedCert");
                 log.debug("Untrusted certificate is {}", cert);
                 return false;
             }
         }
-        log.debug("All certificates are trusted.");
+        log.debug("The whole certificate chain is trusted.");
         return true;
     }
 
@@ -82,8 +85,10 @@ public boolean isTrusted(X509Certificate[] certs) {
      */
     public void updateAPIMLPublicKeyCertificates(X509Certificate[] certs) {
         for (X509Certificate cert : certs) {
-            String publicKey = Base64.getEncoder().encodeToString(cert.getPublicKey().getEncoded());
+            String publicKey = base64EncodePublicKey(cert);
             publicKeyCertificatesBase64.add(publicKey);
         }
     }
+
+
 }
diff --git a/apiml-security-common/src/main/resources/security-common-log-messages.yml b/apiml-security-common/src/main/resources/security-common-log-messages.yml
@@ -147,7 +147,7 @@ messages:
       reason: "The parameter `apiml.security.authorization.provider` is set to `endpoint`"
       action: "Change the SAF provider to another one to use this endpoint"
 
-    - key: org.zowe.apiml.security.common.auth.saf.endpoint.endpointImproprietyConfigure
+    - key: org.zowe.apiml.security.common.auth.saf.endpoint.endpointImproperlyConfigure
       number: ZWEAT603
       type: ERROR
       text: "Endpoint `%s` is not properly configured"

diff --git a/.../src/test/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpointTest.java b/.../src/test/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpointTest.java
@@ -82,7 +82,7 @@ void testHasSafResourceAccess_whenErrorHappened_thenFalse(boolean authorized) {
         ).when(restTemplate).exchange(
                 eq(TEST_URI_ARGS), eq(HttpMethod.GET), any(), eq(SafResourceAccessEndpoint.Response.class), eq(RESOURCE), eq(LEVEL)
         );
-        assertThrows(EndpointImproprietyConfigureException.class, () -> safResourceAccessEndpoint.hasSafResourceAccess(authentication, SUPPORTED_CLASS, RESOURCE, LEVEL));
+        assertThrows(EndpointImproperlyConfigureException.class, () -> safResourceAccessEndpoint.hasSafResourceAccess(authentication, SUPPORTED_CLASS, RESOURCE, LEVEL));
     }
 
     @Test
@@ -96,18 +96,18 @@ void givenFaultyResponse_whenRestTemplateMethodReturnsNull_thenFalse() {
     }
 
     @Test
-    void givenUnsupportedResouceClass_whenVerify_thenEndpointImproprietyConfigureException() {
+    void givenUnsupportedResouceClass_whenVerify_thenendpointImproperlyConfigureException() {
         assertThrows(UnsupportedResourceClassException.class, () -> safResourceAccessEndpoint.hasSafResourceAccess(authentication, UNSUPPORTED_CLASS, RESOURCE, LEVEL));
     }
 
     @Test
-    void givenExceptionOnRestCall_whenVerifying_thenEndpointImproprietyConfigureException() {
+    void givenExceptionOnRestCall_whenVerifying_thenendpointImproperlyConfigureException() {
         doThrow(
             new RuntimeException()
         ).when(restTemplate).exchange(
             anyString(), any(), any(), eq(SafResourceAccessEndpoint.Response.class), anyString(), anyString()
         );
-        assertThrows(EndpointImproprietyConfigureException.class, () -> safResourceAccessEndpoint.hasSafResourceAccess(authentication, SUPPORTED_CLASS, RESOURCE, LEVEL));
+        assertThrows(EndpointImproperlyConfigureException.class, () -> safResourceAccessEndpoint.hasSafResourceAccess(authentication, SUPPORTED_CLASS, RESOURCE, LEVEL));
     }
 
 }