fix: do not require clientAuth extension (#2595)

* remove duplicate certificate validation step Signed-off-by: achmelo <a.chmelo@gmail.com> * revert Signed-off-by: achmelo <a.chmelo@gmail.com> * remove import Signed-off-by: achmelo <a.chmelo@gmail.com> * not relevant test Signed-off-by: achmelo <a.chmelo@gmail.com> * imports Signed-off-by: achmelo <a.chmelo@gmail.com> Signed-off-by: achmelo <a.chmelo@gmail.com> Co-authored-by: Petr Weinfurt <petr.weinfurt@broadcom.com>
zowe · Sep 27, 2022 · e9e8092 · e9e8092
1 parent 864099d
commit e9e8092
Show file tree

Hide file tree

Showing 13 changed files with 87 additions and 254 deletions.
diff --git a/cloud-gateway-service/src/test/resources/application.yml b/cloud-gateway-service/src/test/resources/application.yml
@@ -1,7 +1,7 @@
 eureka:
     client:
         serviceUrl:
-            defaultZone: https://localhost:10011/eureka/
+            defaultZone: https://localhost:999/eureka/
 
 apiml:
     service:

diff --git a/...service/src/main/java/org/zowe/apiml/gateway/security/config/ComponentsConfiguration.java b/...service/src/main/java/org/zowe/apiml/gateway/security/config/ComponentsConfiguration.java
@@ -17,7 +17,7 @@
 import org.springframework.context.annotation.Lazy;
 import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
 import org.zowe.apiml.gateway.security.login.Providers;
-import org.zowe.apiml.gateway.security.login.x509.X509AbstractMapper;
+import org.zowe.apiml.gateway.security.login.x509.X509AuthenticationMapper;
 import org.zowe.apiml.gateway.security.login.x509.X509CommonNameUserMapper;
 import org.zowe.apiml.gateway.security.service.AuthenticationService;
 import org.zowe.apiml.gateway.security.service.TokenCreationService;
@@ -71,7 +71,7 @@ public Providers loginProviders(
      */
     @Bean
     @Qualifier("x509MFAuthSourceService")
-    public X509AuthSourceService getX509MFAuthSourceService(X509AbstractMapper mapper, TokenCreationService tokenCreationService, AuthenticationService authenticationService) {
+    public X509AuthSourceService getX509MFAuthSourceService(X509AuthenticationMapper mapper, TokenCreationService tokenCreationService, AuthenticationService authenticationService) {
         return new X509AuthSourceService(mapper, tokenCreationService, authenticationService);
     }
 

diff --git a/...-service/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509AbstractMapper.java b/...-service/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509AbstractMapper.java
diff --git a/...ce/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509CommonNameUserMapper.java b/...ce/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509CommonNameUserMapper.java
@@ -28,7 +28,7 @@
 @Component
 @ConditionalOnExpression("T(org.springframework.util.StringUtils).isEmpty('${apiml.security.x509.externalMapperUrl}')"
 )
-public class X509CommonNameUserMapper extends X509AbstractMapper {
+public class X509CommonNameUserMapper implements X509AuthenticationMapper {
 
 
     /**
@@ -38,13 +38,11 @@ public class X509CommonNameUserMapper extends X509AbstractMapper {
      * @return the user
      */
     public String mapCertificateToMainframeUserId(X509Certificate certificate) {
-        if (isClientAuthCertificate(certificate)) {
-            String dn = certificate.getSubjectX500Principal().getName();
-            LdapName ldapDN = getLdapName(dn);
-            for (Rdn rdn : ldapDN.getRdns()) {
-                if ("cn".equalsIgnoreCase(rdn.getType())) {
-                    return String.valueOf(rdn.getValue());
-                }
+        String dn = certificate.getSubjectX500Principal().getName();
+        LdapName ldapDN = getLdapName(dn);
+        for (Rdn rdn : ldapDN.getRdns()) {
+            if ("cn".equalsIgnoreCase(rdn.getType())) {
+                return String.valueOf(rdn.getValue());
             }
         }
         return null;

diff --git a/...-service/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509ExternalMapper.java b/...-service/src/main/java/org/zowe/apiml/gateway/security/login/x509/X509ExternalMapper.java
@@ -43,7 +43,7 @@
 @RequiredArgsConstructor
 @ConditionalOnExpression("!T(org.springframework.util.StringUtils).isEmpty('${apiml.security.x509.externalMapperUrl}')"
 )
-public class X509ExternalMapper extends X509AbstractMapper {
+public class X509ExternalMapper implements X509AuthenticationMapper {
 
     private final CloseableHttpClient httpClientProxy;
     private final TokenCreationService tokenCreationService;
@@ -64,36 +64,33 @@ public class X509ExternalMapper extends X509AbstractMapper {
      */
     @Override
     public String mapCertificateToMainframeUserId(X509Certificate certificate) {
-        if (isClientAuthCertificate(certificate)) {
-            try {
-                String jwtToken = tokenCreationService.createJwtTokenWithoutCredentials(externalMapperUser);
-
-                HttpPost httpPost = new HttpPost(new URI(externalMapperUrl));
-                HttpEntity httpEntity = new ByteArrayEntity(certificate.getEncoded());
-                httpPost.setEntity(httpEntity);
-
-                httpPost.setHeader(new BasicHeader("Cookie", "apimlAuthenticationToken=" + jwtToken));
-                log.debug("Executing request against external mapper API: {}", httpPost.toString());
-
-                HttpResponse httpResponse = httpClientProxy.execute(httpPost);
-                String response = EntityUtils.toString(httpResponse.getEntity(), StandardCharsets.UTF_8);
-                log.debug("External mapper API returned: {}", response);
-
-                if (response == null || response.isEmpty()) {
-                    return null;
-                }
-
-                ObjectMapper objectMapper = new ObjectMapper();
-                CertMapperResponse certMapperResponse = objectMapper.readValue(response, CertMapperResponse.class);
-                return certMapperResponse.getUserId().trim();
-            } catch (URISyntaxException e) {
-                log.error("Wrong service URI provided", e);
-            } catch (CertificateEncodingException e) {
-                log.error("Can`t get encoded data from certificate", e);
-            } catch (IOException e) {
-                log.error("Not able to send certificate to mapper", e);
+        try {
+            String jwtToken = tokenCreationService.createJwtTokenWithoutCredentials(externalMapperUser);
+
+            HttpPost httpPost = new HttpPost(new URI(externalMapperUrl));
+            HttpEntity httpEntity = new ByteArrayEntity(certificate.getEncoded());
+            httpPost.setEntity(httpEntity);
+
+            httpPost.setHeader(new BasicHeader("Cookie", "apimlAuthenticationToken=" + jwtToken));
+            log.debug("Executing request against external mapper API: {}", httpPost.toString());
+
+            HttpResponse httpResponse = httpClientProxy.execute(httpPost);
+            String response = EntityUtils.toString(httpResponse.getEntity(), StandardCharsets.UTF_8);
+            log.debug("External mapper API returned: {}", response);
+
+            if (response == null || response.isEmpty()) {
+                return null;
             }
-            return null;
+
+            ObjectMapper objectMapper = new ObjectMapper();
+            CertMapperResponse certMapperResponse = objectMapper.readValue(response, CertMapperResponse.class);
+            return certMapperResponse.getUserId().trim();
+        } catch (URISyntaxException e) {
+            log.error("Wrong service URI provided", e);
+        } catch (CertificateEncodingException e) {
+            log.error("Can`t get encoded data from certificate", e);
+        } catch (IOException e) {
+            log.error("Not able to send certificate to mapper", e);
         }
         return null;
     }

diff --git a/...ain/java/org/zowe/apiml/gateway/security/service/schema/source/X509AuthSourceService.java b/...ain/java/org/zowe/apiml/gateway/security/service/schema/source/X509AuthSourceService.java
@@ -12,7 +12,7 @@
 
 import com.netflix.zuul.context.RequestContext;
 import lombok.RequiredArgsConstructor;
-import org.zowe.apiml.gateway.security.login.x509.X509AbstractMapper;
+import org.zowe.apiml.gateway.security.login.x509.X509AuthenticationMapper;
 import org.zowe.apiml.gateway.security.service.AuthenticationService;
 import org.zowe.apiml.gateway.security.service.TokenCreationService;
 import org.zowe.apiml.gateway.security.service.schema.source.AuthSource.Origin;
@@ -30,15 +30,15 @@
 
 /**
  * Basic implementation of AuthSourceService interface which uses client certificate as an authentication source.
- * This implementation relies on concrete implementation of {@link X509AbstractMapper} for validation and parsing of
+ * This implementation relies on concrete implementation of {@link X509AuthenticationMapper} for validation and parsing of
  * the client certificate.
  */
 @RequiredArgsConstructor
 public class X509AuthSourceService implements AuthSourceService {
     @InjectApimlLogger
     protected final ApimlLogger logger = ApimlLogger.empty();
 
-    private final X509AbstractMapper mapper;
+    private final X509AuthenticationMapper mapper;
     private final TokenCreationService tokenService;
     private final AuthenticationService authenticationService;
 
@@ -83,15 +83,7 @@ public boolean isValid(AuthSource authSource) {
      */
     protected boolean isValid(X509Certificate clientCert) {
         logger.log(MessageType.DEBUG, "Validating X509 client certificate.");
-        if (clientCert == null) {
-            return false;
-        }
-
-        if (mapper.isClientAuthCertificate(clientCert)) {
-            return true;
-        } else {
-            throw new AuthSchemeException("org.zowe.apiml.gateway.security.scheme.x509ExtendedKeyUsageError");
-        }
+        return !(clientCert == null);
     }
 
     /**
@@ -131,10 +123,10 @@ protected X509Certificate getOne(X509Certificate[] certs) {
      * Parse client certificate: get common name, distinguished name and encoded certificate value.
      *
      * @param clientCert {@link X509Certificate} client certificate to parse.
-     * @param mapper     instance of {@link X509AbstractMapper} to use for parsing.
+     * @param mapper     instance of {@link X509AuthenticationMapper} to use for parsing.
      * @return parsed authentication source or null in case of CertificateEncodingException.
      */
-    private Parsed parseClientCert(X509Certificate clientCert, X509AbstractMapper mapper) {
+    private Parsed parseClientCert(X509Certificate clientCert, X509AuthenticationMapper mapper) {
         try {
             String commonName = mapper.mapCertificateToMainframeUserId(clientCert);
             String encodedCert = Base64.getEncoder().encodeToString(clientCert.getEncoded());

diff --git a/gateway-service/src/test/java/org/zowe/apiml/acceptance/SafIdtSchemeTest.java b/gateway-service/src/test/java/org/zowe/apiml/acceptance/SafIdtSchemeTest.java
@@ -201,7 +201,7 @@ void setUp() throws Exception {
         }
 
         @Nested
-        class WhenClientAuthInExtendedKeyUsage {
+        class WhenClientAuthCertificate {
             @Test
             void thenValidSafIdTokenProvided() throws IOException {
                 given()
@@ -223,12 +223,11 @@ void thenValidSafIdTokenProvided() throws IOException {
          * client authentication then request will continue with X-Zowe-Auth-Failure header only.
          */
         @Nested
-        class WhenNoClientAuthInExtendedKeyUsage {
+        class WhenNoClientAuthCertificate {
             @Test
             void thenNoSafIdTokenProvided() throws IOException {
 
                 given()
-                    .config(SslContext.apimlRootCert)
                     .when()
                     .get(basePath + serviceWithDefaultConfiguration.getPath())
                     .then()
@@ -237,7 +236,7 @@ void thenNoSafIdTokenProvided() throws IOException {
                 ArgumentCaptor<HttpUriRequest> captor = ArgumentCaptor.forClass(HttpUriRequest.class);
                 verify(mockClient, times(1)).execute(captor.capture());
                 assertThat(captor.getValue().getHeaders("X-SAF-Token").length, is(0));
-                assertHeaderWithValue(captor.getValue(), AUTH_FAIL_HEADER, "ZWEAG165E X509 certificate is missing the client certificate extended usage definition");
+                assertHeaderWithValue(captor.getValue(), AUTH_FAIL_HEADER, "ZWEAG160E No authentication provided in the request");
             }
         }
     }

diff --git a/gateway-service/src/test/java/org/zowe/apiml/acceptance/X509SchemeTest.java b/gateway-service/src/test/java/org/zowe/apiml/acceptance/X509SchemeTest.java
@@ -126,26 +126,6 @@ private void assertHeaders(HttpUriRequest toVerify, String expectedCnHeader, Str
     @Nested
     class GivenInvalidCertificate {
 
-        @Test
-        void whenServerCertificate_thenNoCertDetailsInRequestHeaders() throws IOException {
-            String errorHeaderValue = "ZWEAG165E X509 certificate is missing the client certificate extended usage definition";
-
-            applicationRegistry.setCurrentApplication(serviceWithCustomConfiguration.getId());
-            mockValid200HttpResponse();
-
-            given()
-                .config(SslContext.apimlRootCert)
-                .when()
-                .get(basePath + serviceWithCustomConfiguration.getPath())
-                .then()
-                .statusCode(is(HttpStatus.SC_OK));
-
-            ArgumentCaptor<HttpUriRequest> captor = ArgumentCaptor.forClass(HttpUriRequest.class);
-            verify(mockClient, times(1)).execute(captor.capture());
-
-            assertHeaders(captor.getValue(), errorHeaderValue);
-        }
-
         @Test
         void whenNoCertificate_thenNoCertDetailsInRequestHeaders() throws IOException {
             String errorHeaderValue = "ZWEAG167E No client certificate provided in the request";

diff --git a/...rc/test/java/org/zowe/apiml/gateway/security/login/x509/X509CommonNameUserMapperTest.java b/...rc/test/java/org/zowe/apiml/gateway/security/login/x509/X509CommonNameUserMapperTest.java
@@ -19,7 +19,6 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
 
 class X509CommonNameUserMapperTest {
 
@@ -50,19 +49,6 @@ void whenWrongDN_exceptionIsThrown() {
         assertEquals("Not able to create ldap name from certificate. Cause: Invalid name: wrong DN", exception.getMessage());
     }
 
-    @Test
-    void whenWrongExtension_throwException() {
-        X509Certificate x509Certificate =
-            X509Utils.getCertificate(X509Utils.correctBase64("zowe"), "CN=user,OU=CA CZ,O=Broadcom,L=Prague,ST=Czechia,C=CZ");
-        try {
-            doThrow(new CertificateParsingException()).when(x509Certificate).getExtendedKeyUsage();
-        } catch (CertificateParsingException e) {
-            throw new RuntimeException("Error mocking exception");
-        }
-        Exception exception = assertThrows(AuthenticationServiceException.class, () -> x509CommonNameUserMapper.isClientAuthCertificate(x509Certificate));
-        assertEquals("Can't get extensions from certificate", exception.getMessage());
-    }
-
     @Test
     void whenNullExtension_thenReturnFalse() {
         X509Certificate x509Certificate =
@@ -72,7 +58,7 @@ void whenNullExtension_thenReturnFalse() {
         } catch (CertificateParsingException e) {
             throw new RuntimeException("Error mocking exception");
         }
-        assertFalse(x509CommonNameUserMapper.isClientAuthCertificate(x509Certificate));
+
     }
 
 }