Merge pull request eclipse-tractusx#51 from catenax-ng/user-input-ver…

…ification

Validate attributes

CHANGELOG.md

@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Changed 
 - make DapsManager methods synchronized
 
+### Added
+- add attributes validation
+
 ## [2.0.6] - 2023-05-08
 
 ### Changed

src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java

@@ -29,14 +29,12 @@
 import lombok.extern.slf4j.Slf4j;
 import org.eclipse.tractusx.dapsreg.util.JsonUtil;
 import org.springframework.beans.factory.annotation.Value;
-import org.springframework.http.HttpHeaders;
-import org.springframework.http.HttpStatus;
-import org.springframework.http.MediaType;
-import org.springframework.http.ResponseEntity;
+import org.springframework.http.*;
 import org.springframework.stereotype.Service;
 import org.springframework.web.reactive.function.BodyInserters;
 import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.util.UriBuilder;
+import reactor.core.publisher.Mono;
 
 import java.io.IOException;
 import java.security.cert.X509Certificate;
@@ -53,7 +51,7 @@
 public class DapsClient {
 
     private static final long REFRESH_GAP = 100L;
-    private static final String PATH = "config/clients";
+    private static final String[] PATH = "config/clients".split("/");
 
     @Value("${app.daps.apiUri}")
     @Setter
@@ -113,7 +111,7 @@ public Optional<ResponseEntity<Void>> createClient(JsonNode json) {
 
     public HttpStatus updateClient(JsonNode json, String clientId) {
         return (HttpStatus) WebClient.create(dapsApiUri).put()
-                .uri(uriBuilder -> uriBuilder.pathSegment(PATH, clientId).build())
+                .uri(uriBuilder -> uriBuilder.pathSegment(PATH).pathSegment(clientId).build())
                 .headers(this::headersSetter)
                 .contentType(MediaType.APPLICATION_JSON)
                 .bodyValue(json)
@@ -122,22 +120,23 @@ public HttpStatus updateClient(JsonNode json, String clientId) {
                 .blockOptional().orElseThrow().getStatusCode();
     }
 
-    public JsonNode getClient(String clientId) {
+    public Optional<JsonNode> getClient(String clientId) {
         return WebClient.create(dapsApiUri).get()
-                .uri(uriBuilder -> uriBuilder.pathSegment(PATH, clientId).build())
+                .uri(uriBuilder -> uriBuilder.pathSegment(PATH).pathSegment(clientId).build())
                 .headers(this::headersSetter)
                 .accept(MediaType.APPLICATION_JSON)
                 .retrieve()
+                .onRawStatus(code -> code == 404, clientResponse -> Mono.empty())
                 .bodyToMono(JsonNode.class)
-                .blockOptional().orElseThrow();
+                .blockOptional();
     }
 
     public HttpStatus deleteClient(String clientId) {
-        return deleteSomething(uriBuilder -> uriBuilder.pathSegment(PATH, clientId));
+        return deleteSomething(uriBuilder -> uriBuilder.pathSegment(PATH).pathSegment(clientId));
     }
 
     public HttpStatus deleteCert(String clientId) {
-        return deleteSomething(uriBuilder -> uriBuilder.pathSegment(PATH, clientId, "keys"));
+        return deleteSomething(uriBuilder -> uriBuilder.pathSegment(PATH).pathSegment(clientId, "keys"));
     }
 
     private HttpStatus deleteSomething(UnaryOperator<UriBuilder> pathBuilder) {
@@ -152,12 +151,12 @@ private HttpStatus deleteSomething(UnaryOperator<UriBuilder> pathBuilder) {
     public HttpStatus uploadCert(X509Certificate certificate, String clientId) throws IOException {
         var body = jsonUtil.getCertificateJson(certificate);
         return (HttpStatus) WebClient.create(dapsApiUri).post()
-                .uri(uriBuilder -> uriBuilder.pathSegment(PATH, "{client_id}", "keys").build(clientId))
+                .uri(uriBuilder -> uriBuilder.pathSegment(PATH).pathSegment( clientId, "keys").build())
                 .headers(this::headersSetter)
                 .contentType(MediaType.APPLICATION_JSON)
                 .bodyValue(body)
                 .retrieve()
                 .toBodilessEntity()
                 .blockOptional().orElseThrow().getStatusCode();
     }
-}
+}

src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java

@@ -27,6 +27,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.eclipse.tractusx.dapsreg.api.DapsApiDelegate;
 import org.eclipse.tractusx.dapsreg.config.StaticJsonConfigurer.StaticJson;
+import org.eclipse.tractusx.dapsreg.util.AttributeValidator;
 import org.eclipse.tractusx.dapsreg.util.Certutil;
 import org.eclipse.tractusx.dapsreg.util.JsonUtil;
 import org.springframework.http.HttpStatus;
@@ -55,6 +56,8 @@ public class DapsManager implements DapsApiDelegate {
     private final ObjectMapper mapper;
     private final JsonUtil jsonUtil;
     private final StaticJson staticJson;
+    private final AttributeValidator attributeValidator;
+
 
     @SneakyThrows
     @Override
@@ -89,7 +92,10 @@ public synchronized ResponseEntity<Map<String, Object>> getClientGet(String clie
     @Override
     @PreAuthorize("hasAuthority(@securityRoles.updateRole)")
     public synchronized ResponseEntity<Void> updateClientPut(String clientId, Map<String, String> newAttr) {
-    var clientAttr = dapsClient.getClient(clientId).get("attributes");
+        newAttr.entrySet().stream()
+                .flatMap(entry -> Stream.of(entry.getKey(), entry.getValue()))
+                .forEach(attributeValidator::validate);
+        var clientAttr = dapsClient.getClient(clientId).map(jsn-> jsn.get("attributes")).orElseThrow();
         var keys = new HashSet<>();
         var attr = Stream.concat(
                         newAttr.entrySet().stream(),

src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java

@@ -0,0 +1,41 @@
+/********************************************************************************
+ * Copyright (c) 2021,2022 T-Systems International GmbH
+ * Copyright (c) 2021,2022 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Apache License, Version 2.0 which is available at
+ * https://www.apache.org/licenses/LICENSE-2.0.
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ ********************************************************************************/
+
+package org.eclipse.tractusx.dapsreg.util;
+
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpStatus;
+import org.springframework.stereotype.Component;
+import org.springframework.web.server.ResponseStatusException;
+
+@Component
+public class AttributeValidator {
+
+    @Value("${app.maxAttrLen:512}")
+    private int maxAttrLen;
+
+    public static final String regex = "^[a-zA-Z0-9@\"*&+:;,()/\s_.-]+$";
+    public void validate(String testString) {
+        if (testString != null && (testString.length() > maxAttrLen || !testString.matches(regex))) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "does not match the pattern");
+        }
+    }
+
+}

src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java

@@ -27,6 +27,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
 import org.springframework.web.server.ResponseStatusException;
+import org.w3c.dom.Attr;
 
 import java.io.IOException;
 import java.security.cert.X509Certificate;
@@ -40,13 +41,17 @@ public class JsonUtil {
     private final ObjectMapper mapper;
     private static final String KEY = "key";
     private static final String VALUE = "value";
+    private final AttributeValidator attributeValidator;
 
     public JsonNode getCertificateJson(X509Certificate x509Certificate) throws IOException {
         return mapper.createObjectNode().put("certificate", Certutil.getCertificate(x509Certificate));
     }
 
     public JsonNode getClientJson(String clientId, String clientName,
-                                    String securityProfile, String referringConnector) {
+                                  String securityProfile, String referringConnector) {
+        attributeValidator.validate(clientId);
+        attributeValidator.validate(clientName);
+        attributeValidator.validate(securityProfile);
         ObjectNode objectNode = mapper.createObjectNode();
         objectNode.put("client_id",
                         Optional.ofNullable(clientId)

src/main/resources/application.yml

@@ -9,6 +9,7 @@ springdoc:
 app:
   build:
     version: ^project.version^
+  maxAttrLen: 512
   daps:
     #apiUri:
     #tokenUri:
@@ -35,4 +36,4 @@ logging:
       springframework:
         security:
           web:
-            csrf: INFO
+            csrf: INFO

src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java

@@ -23,8 +23,15 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.io.Resources;
+import org.apache.commons.lang3.StringUtils;
 import org.eclipse.tractusx.dapsreg.util.Certutil;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.ArgumentsProvider;
+import org.junit.jupiter.params.provider.ArgumentsSource;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
 import org.springframework.boot.test.context.SpringBootTest;
@@ -36,6 +43,7 @@
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
 
 import java.util.Objects;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -55,11 +63,76 @@ class DapsregE2eTest {
     private ObjectMapper mapper;
 
     private JsonNode getClient(String client_id) throws Exception {
-        var contentAsString = mockMvc.perform(get("/api/v1/daps/".concat(client_id))).andDo(print()).andExpect(status().isOk())
+        var contentAsString = mockMvc.perform(get("/api/v1/daps/".concat(client_id)))
+                .andDo(print())
+                .andExpect(status().isOk())
                 .andReturn().getResponse().getContentAsString();
-        var response = mapper.readValue(contentAsString, JsonNode.class);
-        System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(response));
-        return response;
+        return mapper.readValue(contentAsString, JsonNode.class);
+    }
+
+
+    @WithMockUser(username = "fulladmin", authorities={"create_daps_client", "update_daps_client", "delete_daps_client", "retrieve_daps_client"})
+    @ParameterizedTest
+    @ValueSource(strings = {"</>", "hello\t", "hello\n", "?test", "#test"})
+    void createClientBadSymbolsInClientNameTest(String attrValue) throws Exception {
+        try (var pemStream = Resources.getResource("test.crt").openStream()) {
+            var pem = new String(pemStream.readAllBytes());
+            MockMultipartFile pemFile = new MockMultipartFile("file", "test.crt", "text/plain", pem.getBytes());
+            mockMvc.perform(MockMvcRequestBuilders.multipart("/api/v1/daps")
+                            .file(pemFile)
+                            .param("clientName", attrValue)
+                            .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN1234567890"))
+                    .andDo(print())
+                    .andExpect(status().is4xxClientError());
+        }
+    }
+
+    static class MyArgumentsProvider implements ArgumentsProvider {
+        @Override
+        public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+            return Stream.of(
+                    Arguments.of("test\n" ,"TEST#"),
+                    Arguments.of("</>", "www"),
+                    Arguments.of("#aaa", "bbb"),
+                    Arguments.of("longAttr", StringUtils.repeat('A', 1024))
+            );
+        }
+    }
+
+    @WithMockUser(username = "fulladmin", authorities={"create_daps_client", "update_daps_client", "delete_daps_client", "retrieve_daps_client"})
+    @ParameterizedTest
+    @ArgumentsSource(MyArgumentsProvider.class)
+    void updateClientAttrBadSymbolsTest(String attrName, String attrValue) throws Exception {
+        String clientId = null;
+        try (var pemStream = Resources.getResource("test.crt").openStream()) {
+            var pem = new String(pemStream.readAllBytes());
+            var cert = Certutil.loadCertificate(pem);
+            clientId = Certutil.getClientId(cert);
+            MockMultipartFile pemFile = new MockMultipartFile("file", "test.crt", "text/plain", pem.getBytes());
+            var createResultString = mockMvc.perform(MockMvcRequestBuilders.multipart("/api/v1/daps")
+                            .file(pemFile)
+                            .param("clientName", "bmw preprod")
+                            .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN1234567890"))
+                    .andDo(print())
+                    .andExpect(status().isCreated())
+                    .andExpect(MockMvcResultMatchers.jsonPath("$.clientId").value(clientId))
+                    .andExpect(MockMvcResultMatchers.jsonPath("$.daps_jwks").value("https://daps1.int.demo.catena-x.net/jwks.json"))
+                    .andReturn().getResponse().getContentAsString();
+            var createResultJson = mapper.readTree(createResultString);
+            assertThat(createResultJson.get("clientId").asText()).isEqualTo(clientId);
+            var orig = getClient(clientId);
+            assertThat(orig.get("name").asText()).isEqualTo("bmw preprod");
+            mockMvc.perform(put("/api/v1/daps/".concat(clientId))
+                            .param(attrName, attrValue))
+                    .andDo(print())
+                    .andExpect(status().is4xxClientError());
+        } finally {
+            if (!Objects.isNull(clientId)) {
+                mockMvc.perform(delete("/api/v1/daps/".concat(clientId)))
+                        .andDo(print())
+                        .andExpect(status().is2xxSuccessful());
+            }
+        }
     }
 
     @Test
@@ -72,9 +145,10 @@ void createRetrieveChangeDeleteTest() throws Exception {
             clientId = Certutil.getClientId(cert);
             MockMultipartFile pemFile = new MockMultipartFile("file", "test.crt", "text/plain", pem.getBytes());
             var createResultString = mockMvc.perform(MockMvcRequestBuilders.multipart("/api/v1/daps")
-                        .file(pemFile)
-                        .param("clientName", "bmw preprod")
-                        .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN1234567890"))
+                            .file(pemFile)
+                            .param("clientName", "bmw preprod")
+                            .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN1234567890"))
+                    .andDo(print())
                     .andExpect(status().isCreated())
                     .andExpect(MockMvcResultMatchers.jsonPath("$.clientId").value(clientId))
                     .andExpect(MockMvcResultMatchers.jsonPath("$.daps_jwks").value("https://daps1.int.demo.catena-x.net/jwks.json"))
@@ -84,9 +158,10 @@ void createRetrieveChangeDeleteTest() throws Exception {
             var orig = getClient(clientId);
             assertThat(orig.get("name").asText()).isEqualTo("bmw preprod");
             mockMvc.perform(put("/api/v1/daps/".concat(clientId))
-                    .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN0987654321")
-                    .param("email", "admin@test.com")
-            ).andExpect(status().isOk());
+                            .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN0987654321")
+                            .param("email", "admin@test.com"))
+                    .andDo(print())
+                    .andExpect(status().isOk());
             var changed = getClient(clientId);
             var referringConnector = StreamSupport.stream(changed.get("attributes").spliterator(), false)
                     .filter(jsonNode -> jsonNode.get("key").asText().equals("referringConnector")).findAny().orElseThrow();
@@ -96,8 +171,11 @@ void createRetrieveChangeDeleteTest() throws Exception {
             assertThat(email.get("value").asText()).isEqualTo("admin@test.com");
         } finally {
             if (!Objects.isNull(clientId)) {
-                mockMvc.perform(delete("/api/v1/daps/".concat(clientId))).andExpect(status().is2xxSuccessful());
+                mockMvc.perform(delete("/api/v1/daps/".concat(clientId)))
+                        .andDo(print())
+                        .andExpect(status().is2xxSuccessful());
             }
         }
     }
+
 }

src/test/resources/application-test.yml

@@ -29,6 +29,7 @@ springdoc:
 app:
   build:
     version: ^project.version^
+  maxAttrLen: 512
   daps:
     apiUri: http://localhost:4567/api/v1
     tokenUri: http://localhost:4567/token
@@ -57,4 +58,3 @@ logging:
         security:
           web:
             csrf: INFO
-