fix: replace default x509 with categorize certs filter in zaas

Signed-off-by: Richard Salac <richard.salac@broadcom.com>
zowe · Jan 30, 2025 · 26ecb0e · 26ecb0e
1 parent 7663485
commit 26ecb0e
Show file tree

Hide file tree

Showing 16 changed files with 211 additions and 123 deletions.
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,7 +130,7 @@ private UserDetailsService x509UserDetailsService() {
         }
 
         private CategorizeCertsFilter reversedCategorizeCertFilter() {
-            CategorizeCertsFilter out = new CategorizeCertsFilter(publicKeyCertificatesBase64, certificateValidator);
+            CategorizeCertsFilter out = new CategorizeCertsFilter(publicKeyCertificatesBase64, certificateValidator, handlerInitializer.getUnAuthorizedHandler().getHandler(), false);
             out.setCertificateForClientAuth(crt -> out.getPublicKeyCertificatesBase64().contains(CategorizeCertsFilter.base64EncodePublicKey(crt)));
             out.setApimlCertificate(crt -> !out.getPublicKeyCertificatesBase64().contains(CategorizeCertsFilter.base64EncodePublicKey(crt)));
             return out;

diff --git a/...ty-common/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessSaf.java b/...ty-common/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessSaf.java
@@ -97,7 +97,8 @@ private boolean checkPermission(String userId, String resourceType, String resou
     @Override
     public boolean hasSafResourceAccess(Authentication authentication, String resourceClass, String resourceName, String accessLevel) {
         String userid = authentication.getName();
-        if (StringUtils.isEmpty(userid)) {
+        if (StringUtils.isEmpty(userid) || userid.length() > 8) {
+            log.debug("UserId {} is not valid for SAF permissions check", userid);
             return false;
         }
         AccessLevel level = AccessLevel.valueOf(accessLevel);

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
@@ -22,9 +22,11 @@
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
+import org.springframework.security.authentication.InsufficientAuthenticationException;
 import org.springframework.web.filter.OncePerRequestFilter;
 import org.zowe.apiml.message.log.ApimlLogger;
 import org.zowe.apiml.product.logging.annotations.InjectApimlLogger;
+import org.zowe.apiml.security.common.error.AuthExceptionHandler;
 import org.zowe.apiml.security.common.verify.CertificateValidator;
 
 import java.io.ByteArrayInputStream;
@@ -55,6 +57,14 @@ public class CategorizeCertsFilter extends OncePerRequestFilter {
 
     private final CertificateValidator certificateValidator;
 
+    private final AuthExceptionHandler authExceptionHandler;
+
+    private final boolean rejectIfZoweCertificateMissing;
+
+    public CategorizeCertsFilter(Set<String> publicKeyCertificatesBase64, CertificateValidator certificateValidator, AuthExceptionHandler authExceptionHandler) {
+        this(publicKeyCertificatesBase64, certificateValidator, authExceptionHandler, true);
+    }
+
     /**
      * Get certificates from request (if exists), separate them (to use only APIML certificate to request sign and
      * other for authentication) and store again into request.
@@ -64,10 +74,11 @@ public class CategorizeCertsFilter extends OncePerRequestFilter {
      * in the default attribute.
      *
      * @param request Request to filter certificates
+     * @throws InsufficientAuthenticationException Optionally, if the Zowe server certificate is missing and rejectIfZoweCertificateMissing is set to true
      */
     private void categorizeCerts(ServletRequest request) {
         X509Certificate[] certs = (X509Certificate[]) request.getAttribute(ATTRNAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE);
-        if (certs != null) {
+        if (certs != null && certs.length > 0) {
             Optional<Certificate> clientCert = getClientCertFromHeader((HttpServletRequest) request);
             if (certificateValidator.isForwardingEnabled() && certificateValidator.isTrusted(certs) && clientCert.isPresent()) {
                 certificateValidator.updateAPIMLPublicKeyCertificates(certs);
@@ -81,7 +92,12 @@ private void categorizeCerts(ServletRequest request) {
             }
 
             log.debug(LOG_FORMAT_FILTERING_CERTIFICATES, ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE, request.getAttribute(ATTRNAME_CLIENT_AUTH_X509_CERTIFICATE));
+
+        } else if (rejectIfZoweCertificateMissing) {
+            log.debug("No X509 certificate found in request attribute {}", ATTRNAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE);
+            throw new InsufficientAuthenticationException("No Zowe Server X509 certificate found in request");
         }
+
     }
 
     private Optional<Certificate> getClientCertFromHeader(HttpServletRequest request) {
@@ -138,7 +154,11 @@ public Enumeration<String> getHeaders(String name) {
      **/
     @Override
     protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response, @NonNull FilterChain filterChain) throws ServletException, IOException {
-        categorizeCerts(request);
+        try {
+            categorizeCerts(request);
+        } catch (InsufficientAuthenticationException ex) {
+            authExceptionHandler.handleException(request, response, ex);
+        }
         filterChain.doFilter(mutate(request), response);
     }
 

diff --git a/...curity-common/src/main/java/org/zowe/apiml/security/common/login/X509AuthAwareFilter.java b/...curity-common/src/main/java/org/zowe/apiml/security/common/login/X509AuthAwareFilter.java
@@ -10,22 +10,21 @@
 
 package org.zowe.apiml.security.common.login;
 
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.web.authentication.AuthenticationFailureHandler;
-
-import jakarta.servlet.FilterChain;
-import jakarta.servlet.ServletException;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
 
 import java.io.IOException;
 
-public class X509AuthAwareFilter extends X509AuthenticationFilter {
+public class X509AuthAwareFilter extends X509ForwardingAwareAuthenticationFilter {
     private final AuthenticationFailureHandler failureHandler;
 
     public X509AuthAwareFilter(String endpoint, AuthenticationFailureHandler failureHandler, AuthenticationProvider authenticationProvider) {

diff --git a/...ommon/login/X509AuthenticationFilter.java → ...9ForwardingAwareAuthenticationFilter.java b/...ommon/login/X509AuthenticationFilter.java → ...9ForwardingAwareAuthenticationFilter.java
@@ -10,28 +10,28 @@
 
 package org.zowe.apiml.security.common.login;
 
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
 import org.zowe.apiml.security.common.token.X509AuthenticationToken;
 
-import jakarta.servlet.FilterChain;
-import jakarta.servlet.ServletException;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.security.cert.X509Certificate;
 
 @Slf4j
-public class X509AuthenticationFilter extends NonCompulsoryAuthenticationProcessingFilter {
+public class X509ForwardingAwareAuthenticationFilter extends NonCompulsoryAuthenticationProcessingFilter {
 
     private final AuthenticationProvider authenticationProvider;
     private final AuthenticationSuccessHandler successHandler;
 
-    public X509AuthenticationFilter(String endpoint,
-                                    AuthenticationSuccessHandler successHandler,
-                                    AuthenticationProvider authenticationProvider) {
+    public X509ForwardingAwareAuthenticationFilter(String endpoint,
+                                                   AuthenticationSuccessHandler successHandler,
+                                                   AuthenticationProvider authenticationProvider) {
         super(endpoint);
         this.authenticationProvider = authenticationProvider;
         this.successHandler = successHandler;

diff --git a/...ommon/src/test/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessSafTest.java b/...ommon/src/test/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessSafTest.java
@@ -16,14 +16,15 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 
 import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.*;
 
 @ExtendWith(MockitoExtension.class)
 class SafResourceAccessSafTest {
@@ -88,6 +89,15 @@ void testHasSafResourceAccess_whenNoResponse_thenTrue() {
         assertTrue(safResourceAccessVerifying.hasSafResourceAccess(authentication, CLASS, RESOURCE, LEVEL.name()));
     }
 
+    @ParameterizedTest
+    @NullSource
+    @ValueSource(strings = {"", "tooLongUserId"})
+    void testInvalidUserIds_thenSkipped(String userId) {
+        var auth = new UsernamePasswordAuthenticationToken(userId, "");
+        assertDoesNotThrow(() -> safResourceAccessVerifying.hasSafResourceAccess(auth, CLASS, RESOURCE, LEVEL.name()));
+        verify(checkPermissionMock, never()).checkPermission(any(), any(), any(), anyInt());
+    }
+
     @Builder
     public static class TestPlatformReturned {
 

diff --git a/...common/src/test/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilterTest.java b/...common/src/test/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilterTest.java
@@ -10,18 +10,21 @@
 
 package org.zowe.apiml.security.common.filter;
 
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.authentication.InsufficientAuthenticationException;
+import org.zowe.apiml.security.common.error.AuthExceptionHandler;
 import org.zowe.apiml.security.common.utils.X509Utils;
 import org.zowe.apiml.security.common.verify.CertificateValidator;
 
-import jakarta.servlet.ServletException;
-import jakarta.servlet.http.HttpServletRequest;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -71,6 +74,7 @@ class CategorizeCertsFilterTest {
     private X509Certificate[] clientCerts;
 
     private CertificateValidator certificateValidator;
+    private AuthExceptionHandler authExceptionHandler;
 
     @BeforeAll
     public static void init() throws CertificateException {
@@ -80,21 +84,23 @@ public static void init() throws CertificateException {
     }
 
     @BeforeEach
-    public void setUp() {
+    public void setUp() throws ServletException {
         request = new MockHttpServletRequest();
         response = new MockHttpServletResponse();
         chain = new MockFilterChain();
         certificateValidator = mock(CertificateValidator.class);
+        authExceptionHandler = mock(AuthExceptionHandler.class);
         when(certificateValidator.isForwardingEnabled()).thenReturn(false);
         when(certificateValidator.isTrusted(any())).thenReturn(false);
+        doNothing().when(authExceptionHandler).handleException(any(), any(), any());
     }
 
     @Nested
     class GivenNoPublicKeysInFilter {
 
         @BeforeEach
         void setUp() {
-            filter = new CategorizeCertsFilter(new HashSet<>(), certificateValidator);
+            filter = new CategorizeCertsFilter(new HashSet<>(), certificateValidator, authExceptionHandler);
         }
 
         @Nested
@@ -309,6 +315,7 @@ class WhenOtherHeadersInRequest {
 
             private static final String COMMON_HEADER = "User-Agent";
             private static final String COMMON_HEADER_VALUE = "dummy";
+
             @BeforeEach
             void setUp() {
                 request.addHeader(COMMON_HEADER, COMMON_HEADER_VALUE);
@@ -336,7 +343,7 @@ void setUp() {
                 X509Utils.correctBase64("apimlCert1"),
                 X509Utils.correctBase64("apimlCertCA")
             ));
-            filter = new CategorizeCertsFilter(serverCertChain, certificateValidator);
+            filter = new CategorizeCertsFilter(serverCertChain, certificateValidator, authExceptionHandler);
         }
 
         @Nested
@@ -486,4 +493,31 @@ void thenClientCertHeaderIgnored() throws ServletException, IOException {
             }
         }
     }
+
+    @Nested
+    class WhenNoZoweServerCertificateProvided {
+
+        private MockHttpServletRequest requestWithNoCertificates;
+
+        @BeforeEach
+        void setup() {
+            requestWithNoCertificates = new MockHttpServletRequest();
+        }
+
+        @Test
+        void givenRejectOnMissingZoweServerCertificateEnabled_thenThrowException() throws ServletException, IOException {
+            var rejectOnFilter = new CategorizeCertsFilter(new HashSet<>(), certificateValidator, authExceptionHandler);
+            ArgumentCaptor<RuntimeException> argument = ArgumentCaptor.forClass(RuntimeException.class);
+            rejectOnFilter.doFilter(requestWithNoCertificates, response, chain);
+            verify(authExceptionHandler, times(1)).handleException(any(), any(), argument.capture());
+            assertInstanceOf(InsufficientAuthenticationException.class, argument.getValue());
+        }
+
+        @Test
+        void givenRejectOnMissingZoweServerCertificateDisabled_thenDoNotThrowException() throws ServletException, IOException {
+            var rejectOffFilter = new CategorizeCertsFilter(new HashSet<>(), certificateValidator, authExceptionHandler, false);
+            rejectOffFilter.doFilter(request, response, chain);
+            verify(authExceptionHandler, never()).handleException(any(), any(), any());
+        }
+    }
 }
diff --git a/...s/src/test/java/org/zowe/apiml/integration/authentication/pat/AccessTokenServiceTest.java b/...s/src/test/java/org/zowe/apiml/integration/authentication/pat/AccessTokenServiceTest.java
@@ -218,7 +218,7 @@ void givenNotAuthorizedCall_thenDontAllowToRevokeTokensForUser() {
             requestBody.put("userId", SecurityUtils.USERNAME);
             given().contentType(ContentType.JSON).config(SslContext.clientCertApiml).body(requestBody)
                 .when().delete(REVOKE_FOR_USER_ENDPOINT)
-                .then().statusCode(403);
+                .then().statusCode(401);
 //            validate after revocation rule
             given().contentType(ContentType.JSON).body(bodyContent).when()
                 .post(VALIDATE_ENDPOINT)

diff --git a/...ml/integration/zaas/ZaasNegativeTest.java → ...egration/zaas/ZaasSchemeNegativeTest.java b/...ml/integration/zaas/ZaasNegativeTest.java → ...egration/zaas/ZaasSchemeNegativeTest.java
@@ -40,7 +40,7 @@
 import static org.zowe.apiml.util.SecurityUtils.*;
 
 @ZaasTest
-public class ZaasNegativeTest {
+public class ZaasSchemeNegativeTest {
 
     private final static String APPLICATION_NAME = ConfigReader.environmentConfiguration().getDiscoverableClientConfiguration().getApplId();
     private final static SafIdtConfiguration SAFIDT_CONF = ConfigReader.environmentConfiguration().getSafIdtConfiguration();
@@ -115,7 +115,7 @@ void setUpCertificateAndToken() {
         }
 
         @ParameterizedTest
-        @MethodSource("org.zowe.apiml.integration.zaas.ZaasNegativeTest#provideZaasEndpoints")
+        @MethodSource("org.zowe.apiml.integration.zaas.ZaasSchemeNegativeTest#provideZaasEndpoints")
         void givenNoToken(URI uri, RequestSpecification requestSpecification) {
             //@formatter:off
             requestSpecification
@@ -127,7 +127,7 @@ void givenNoToken(URI uri, RequestSpecification requestSpecification) {
         }
 
         @ParameterizedTest
-        @MethodSource("org.zowe.apiml.integration.zaas.ZaasNegativeTest#provideZaasEndpointsWithAllTokens")
+        @MethodSource("org.zowe.apiml.integration.zaas.ZaasSchemeNegativeTest#provideZaasEndpointsWithAllTokens")
         void givenInvalidToken(URI uri, RequestSpecification requestSpecification, String token) {
             //@formatter:off
             requestSpecification
@@ -140,7 +140,7 @@ void givenInvalidToken(URI uri, RequestSpecification requestSpecification, Strin
         }
 
         @ParameterizedTest
-        @MethodSource("org.zowe.apiml.integration.zaas.ZaasNegativeTest#provideZaasEndpoints")
+        @MethodSource("org.zowe.apiml.integration.zaas.ZaasSchemeNegativeTest#provideZaasEndpoints")
         void givenOKTATokenWithNoMapping(URI uri, RequestSpecification requestSpecification) {
             String oktaTokenNoMapping = SecurityUtils.validOktaAccessToken(false);
             //@formatter:off
@@ -165,7 +165,7 @@ class GivenBadCertificate {
         String keystore;
 
         @ParameterizedTest
-        @MethodSource("org.zowe.apiml.integration.zaas.ZaasNegativeTest#provideZaasEndpoints")
+        @MethodSource("org.zowe.apiml.integration.zaas.ZaasSchemeNegativeTest#provideZaasEndpoints")
         void givenNoCertificate_thenReturnUnauthorized(URI uri, RequestSpecification requestSpecification) {
             //@formatter:off
             requestSpecification
@@ -179,7 +179,7 @@ void givenNoCertificate_thenReturnUnauthorized(URI uri, RequestSpecification req
         }
 
         @ParameterizedTest
-        @MethodSource("org.zowe.apiml.integration.zaas.ZaasNegativeTest#provideZaasTokenEndpoints")
+        @MethodSource("org.zowe.apiml.integration.zaas.ZaasSchemeNegativeTest#provideZaasTokenEndpoints")
         void givenClientAndHeaderCertificates_thenReturnTokenFromClientCert(URI uri, RequestSpecification requestSpecification) throws Exception {
             TlsConfiguration tlsCfg = ConfigReader.environmentConfiguration().getTlsConfiguration();
             SslContextConfigurer sslContextConfigurer = new SslContextConfigurer(tlsCfg.getKeyStorePassword(), tlsCfg.getClientKeystore(), tlsCfg.getKeyStore());