Merge pull request #19 from catenax-ng/A1IDSES-1279-bugfix

A1 idses 1279 bugfix
catenax-ng · Jun 5, 2023 · 74a9632 · 74a9632
2 parents ae3b6f8 + 2cb1924
commit 74a9632
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - fix #33 - SKI and AKI validated as user input @dvasunin (#53)
 
 
+## [2.0.7] - 2023-05-12
+
+### Fixed
+- Fixed potential dataloss on concurrent database access
+- Possible multi-registration of clients
+- Fixed user input not validated
+- SKI and AKI validated as user input
+
 ## [2.0.6] - 2023-05-08
 
 ### Changed

diff --git a/README.md b/README.md
@@ -10,8 +10,8 @@ of the DAPS are not disclosed to the requester.
 
 ### Software Version
 ```shell
-Helm version is v2.0.6
-Application version is v2.0.6
+Helm version is v2.0.7
+Application version is v2.0.7
 ```
 
 # Solution Strategy 

diff --git a/charts/daps-reg-service/README.md b/charts/daps-reg-service/README.md
@@ -1,6 +1,6 @@
 # daps-reg-service
 
-![Version: 2.0.6](https://img.shields.io/badge/Version-2.0.6-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 2.0.6](https://img.shields.io/badge/AppVersion-2.0.6-informational?style=flat-square)
+![Version: 2.0.7](https://img.shields.io/badge/Version-2.0.7-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 2.0.7](https://img.shields.io/badge/AppVersion-2.0.7-informational?style=flat-square)
 
 Daps regisgter service is used to register the EDC connector into DAPS
 

diff --git a/charts/daps-reg-service/values-beta.yaml b/charts/daps-reg-service/values-beta.yaml
@@ -50,7 +50,7 @@ drs:
   secret:
     clientId: "<path:essential-services/data/daps-beta#clientId>"
     clientSecret: "<path:essential-services/data/daps-beta#clientSecret>"
-    apiUri: "<path:essential-services/data/daps-reg-svc-beta#apiUri>"
-    tokenUri: "<path:essential-services/data/daps-reg-svc-beta#tokenUri>"
+    apiUri: "<path:essential-services/data/daps-register-svc-beta#apiUri>"
+    tokenUri: "<path:essential-services/data/daps-register-svc-beta#tokenUri>"
     daps_jwks: "<path:essential-services/data/daps-register-svc-beta#daps_jwks>"
     jwkSetUri: "<path:essential-services/data/daps-register-svc-beta#jwk-set-uri>"
diff --git a/charts/daps-reg-service/values-dev.yaml b/charts/daps-reg-service/values-dev.yaml
@@ -50,7 +50,8 @@ drs:
   secret:
     clientId: "<path:essential-services/data/daps#clientId>"
     clientSecret: "<path:essential-services/data/daps#clientSecret>"
-    apiUri: "<path:essential-services/data/daps-register-svc#apiUri>"
-    tokenUri: "<path:essential-services/data/daps-register-svc#tokenUri>"
-    daps_jwks: "<path:essential-services/data/daps-register-svc#daps_jwks>"
-    jwkSetUri: "<path:essential-services/data/daps-register-svc#jwk-set-uri>"
+    apiUri: "<path:essential-services/data/daps-register-svc-dev#apiUri>"
+    tokenUri: "<path:essential-services/data/daps-register-svc-dev#tokenUri>"
+    daps_jwks: "<path:essential-services/data/daps-register-svc-dev#daps_jwks>"
+    jwkSetUri: "<path:essential-services/data/daps-register-svc-dev#jwk-set-uri>"
+
diff --git a/pom.xml b/pom.xml
@@ -10,7 +10,7 @@
 	</parent>
 	<groupId>org.eclipse.tractusx</groupId>
 	<artifactId>dapsreg</artifactId>
-	<version>2.0.6</version>
+	<version>2.0.7</version>
 	<name>dapsreg</name>
 	<description>client registration to the DAPS</description>
 	<properties>
@@ -72,6 +72,11 @@
 			<artifactId>snakeyaml</artifactId>
 			<version>2.0</version>
 		</dependency>
+		<dependency>
+			<groupId>org.springframework</groupId>
+			<artifactId>spring-expression</artifactId>
+			<version>6.0.8</version>
+		</dependency>
 
 		<dependency>
 			<groupId>org.springframework.boot</groupId>

diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java
@@ -58,7 +58,6 @@ public class DapsManager implements DapsApiDelegate {
     private final StaticJson staticJson;
     private final AttributeValidator attributeValidator;
 
-
     @SneakyThrows
     @Override
     @PreAuthorize("hasAuthority(@securityRoles.createRole)")
@@ -90,7 +89,7 @@ public synchronized ResponseEntity<Map<String, Object>> createClientPost(String
     @Override
     @PreAuthorize("hasAuthority(@securityRoles.retrieveRole)")
     public synchronized ResponseEntity<Map<String, Object>> getClientGet(String clientId) {
-        var jsonNode = dapsClient.getClient(clientId);
+        var jsonNode = dapsClient.getClient(clientId).orElseThrow();
         Map<String, Object> result = mapper.convertValue(jsonNode, new TypeReference<>() {});
         return new ResponseEntity<>(result, HttpStatus.OK);
     }

diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java
@@ -37,5 +37,4 @@ public void validate(String testString) {
             throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "does not match the pattern");
         }
     }
-
-}
+}
diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java
@@ -32,12 +32,12 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.security.NoSuchAlgorithmException;
+import java.security.PublicKey;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 
 public class Certutil {
-
     private Certutil() {
         throw new IllegalStateException("Utility class");
     }
@@ -59,7 +59,7 @@ public static String getSki(X509Certificate cert) {
     }
 
     public static String createSki(X509Certificate cert) throws NoSuchAlgorithmException {
-        var publicKey = cert.getPublicKey();
+        PublicKey publicKey = cert.getPublicKey();
         var r = new JcaX509ExtensionUtils().createSubjectKeyIdentifier(publicKey).getKeyIdentifier();
         return BaseEncoding.base16().upperCase().withSeparator(":", 2).encode(r);
     }

diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java
@@ -48,7 +48,7 @@ public JsonNode getCertificateJson(X509Certificate x509Certificate) throws IOExc
     }
 
     public JsonNode getClientJson(String clientId, String clientName,
-                                  String securityProfile, String referringConnector) {
+                                    String securityProfile, String referringConnector) {
         attributeValidator.validate(clientId);
         attributeValidator.validate(clientName);
         attributeValidator.validate(securityProfile);

diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java
@@ -25,13 +25,17 @@
 import com.fasterxml.jackson.databind.SerializationFeature;
 import com.google.common.io.Resources;
 import org.eclipse.tractusx.dapsreg.util.Certutil;
+import org.eclipse.tractusx.dapsreg.util.AttributeValidator;
 import org.eclipse.tractusx.dapsreg.util.JsonUtil;
+import org.junit.Assert;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.TestPropertySource;
 
 import jakarta.annotation.PostConstruct;
+import org.springframework.web.server.ResponseStatusException;
+
 import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
@@ -48,6 +52,9 @@ class DapsUtilTests {
 	@Autowired
 	ObjectMapper mapper;
 
+	@Autowired
+	AttributeValidator attributeValidator;
+
 	@PostConstruct
 	public void init() {
 		mapper.enable(SerializationFeature.INDENT_OUTPUT);
@@ -71,6 +78,16 @@ void utilTest() throws IOException, CertificateException, NoSuchAlgorithmExcepti
 		}
 	}
 
+	@Test
+	void testPatternPositive() {
+		attributeValidator.validate("https://asdfg.qwe.com/VVV@p_pp1234()+-;");
+	}
+
+	@Test
+	void testPatternNegative() {
+		Assert.assertThrows(ResponseStatusException.class, () -> attributeValidator.validate("</>\\aa"));
+	}
+
 	private String toString(JsonNode json) throws IOException {
 		return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(json);
 	}

diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java
@@ -67,7 +67,9 @@ private JsonNode getClient(String client_id) throws Exception {
                 .andDo(print())
                 .andExpect(status().isOk())
                 .andReturn().getResponse().getContentAsString();
-        return mapper.readValue(contentAsString, JsonNode.class);
+        var response = mapper.readValue(contentAsString, JsonNode.class);
+        //System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(response));
+        return response;
     }
 
 
@@ -145,9 +147,9 @@ 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))
@@ -158,8 +160,8 @@ 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"))
+                        .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);
@@ -215,4 +217,41 @@ void createTwoSameExpectErrorTest() throws Exception {
         }
     }
 
+    @Test
+    @WithMockUser(username = "fulladmin", authorities={"create_daps_client", "update_daps_client", "delete_daps_client", "retrieve_daps_client"})
+    void createTwoSameExpectErrorTest() 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(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().is(400));
+        } finally {
+            if (!Objects.isNull(clientId)) {
+                mockMvc.perform(delete("/api/v1/daps/".concat(clientId)))
+                        .andDo(print())
+                        .andExpect(status().is2xxSuccessful());
+            }
+        }
+    }
+
 }