From a34aa0ee90f3f987f9f09f768c040ff17cf90ed4 Mon Sep 17 00:00:00 2001 From: dvasunin Date: Tue, 28 Mar 2023 15:11:36 +0300 Subject: [PATCH 01/13] make DRS methods synchronized; check ClientId on Create --- .../tractusx/dapsreg/service/DapsClient.java | 11 +++---- .../tractusx/dapsreg/service/DapsManager.java | 15 +++++---- .../tractusx/dapsreg/DapsregE2eTest.java | 33 +++++++++++++++++++ 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java index 410ceaf..6086a0c 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java +++ b/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; @@ -122,14 +120,15 @@ public HttpStatus updateClient(JsonNode json, String clientId) { .blockOptional().orElseThrow().getStatusCode(); } - public JsonNode getClient(String clientId) { + public Optional getClient(String clientId) { return WebClient.create(dapsApiUri).get() .uri(uriBuilder -> uriBuilder.pathSegment(PATH, 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) { diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java index 01c6e21..d6ee0a9 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java +++ b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java @@ -59,12 +59,15 @@ public class DapsManager implements DapsApiDelegate { @SneakyThrows @Override @PreAuthorize("hasAuthority(@securityRoles.createRole)") - public ResponseEntity> createClientPost(String clientName, + public synchronized ResponseEntity> createClientPost(String clientName, URI referringConnector, MultipartFile file, String securityProfile) { var cert = Certutil.loadCertificate(new String(file.getBytes())); var clientId = Certutil.getClientId(cert); + if (dapsClient.getClient(clientId).isPresent()) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Client exists"); + } var clientJson = jsonUtil.getClientJson(clientId, clientName, securityProfile, referringConnector.toString()); dapsClient.createClient(clientJson) .map(ResponseEntity::getStatusCode) @@ -80,16 +83,16 @@ public ResponseEntity> createClientPost(String clientName, @Override @PreAuthorize("hasAuthority(@securityRoles.retrieveRole)") - public ResponseEntity> getClientGet(String clientId) { - var jsonNode = dapsClient.getClient(clientId); + public synchronized ResponseEntity> getClientGet(String clientId) { + var jsonNode = dapsClient.getClient(clientId).orElseThrow(); Map result = mapper.convertValue(jsonNode, new TypeReference<>() {}); return new ResponseEntity<>(result, HttpStatus.OK); } @Override @PreAuthorize("hasAuthority(@securityRoles.updateRole)") - public ResponseEntity updateClientPut(String clientId, Map newAttr) { - var clientAttr = dapsClient.getClient(clientId).get("attributes"); + public synchronized ResponseEntity updateClientPut(String clientId, Map newAttr) { + var clientAttr = dapsClient.getClient(clientId).map(jsn-> jsn.get("attributes")).orElseThrow(); var keys = new HashSet<>(); var attr = Stream.concat( newAttr.entrySet().stream(), @@ -106,7 +109,7 @@ public ResponseEntity updateClientPut(String clientId, Map @Override @PreAuthorize("hasAuthority(@securityRoles.deleteRole)") - public ResponseEntity deleteClientDelete(String clientId) { + public synchronized ResponseEntity deleteClientDelete(String clientId) { dapsClient.deleteCert(clientId); dapsClient.deleteClient(clientId); return new ResponseEntity<>(HttpStatus.NO_CONTENT); diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java index dedfbd7..ef4f89c 100644 --- a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java +++ b/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java @@ -100,4 +100,37 @@ void createRetrieveChangeDeleteTest() 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")) + .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")) + .andExpect(status().is(400)); + } finally { + if (!Objects.isNull(clientId)) { + mockMvc.perform(delete("/api/v1/daps/".concat(clientId))).andExpect(status().is2xxSuccessful()); + } + } + } } From 8a0a459caea598d0e5bcb613f573c99408cfc77a Mon Sep 17 00:00:00 2001 From: A112488628 Date: Tue, 28 Mar 2023 15:25:34 +0300 Subject: [PATCH 02/13] added changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e61fcd8..534040c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Added +- Added check to see if client exists before creating +### Changed +- endpoints are now synchronized to prevent race condition and corruption of clients.yml file ## [2.0.1] - 2023-03-20 From f1a1a151ecef684b409b394a449e03bc7ffa6d80 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Wed, 29 Mar 2023 15:42:01 +0530 Subject: [PATCH 03/13] bug --- charts/daps-reg-service/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/daps-reg-service/values.yaml b/charts/daps-reg-service/values.yaml index a50adee..457e003 100644 --- a/charts/daps-reg-service/values.yaml +++ b/charts/daps-reg-service/values.yaml @@ -31,7 +31,7 @@ image: # -- Set the Image Pull Policy pullPolicy: Always # -- Image tage is defined in chart appVersion. - tag: "" + tag: "2.0.2-SNAPSHOT" imagePullSecrets: [] From b522f3421a6080d765fe165bd0014fc66f5950b8 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Wed, 29 Mar 2023 16:53:30 +0530 Subject: [PATCH 04/13] Update values-dev.yaml --- charts/daps-reg-service/values-dev.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/charts/daps-reg-service/values-dev.yaml b/charts/daps-reg-service/values-dev.yaml index 96886bb..9098f22 100644 --- a/charts/daps-reg-service/values-dev.yaml +++ b/charts/daps-reg-service/values-dev.yaml @@ -50,7 +50,8 @@ drs: secret: clientId: "" clientSecret: "" - apiUri: "" - tokenUri: "" - daps_jwks: "" - jwkSetUri: "" + apiUri: "" + tokenUri: "" + daps_jwks: "" + jwkSetUri: "" + From 11b85ef9b31ea6831eb24c69ee01a47766467c35 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Fri, 31 Mar 2023 18:01:44 +0530 Subject: [PATCH 05/13] prevent registering duplicate clients --- CHANGELOG.md | 7 ++++++- README.md | 4 ++-- charts/daps-reg-service/Chart.yaml | 4 ++-- charts/daps-reg-service/README.md | 2 +- charts/daps-reg-service/values.yaml | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 534040c..ee72b17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +NA + +## [2.0.2] - 2023-03-31 + ### Added - Added check to see if client exists before creating + ### Changed -- endpoints are now synchronized to prevent race condition and corruption of clients.yml file +- Endpoints are now synchronized to prevent race condition and corruption of clients.yml file ## [2.0.1] - 2023-03-20 diff --git a/README.md b/README.md index 7ace09a..d7142b2 100644 --- 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.1 -Application version is v2.0.1 +Helm version is v2.0.2 +Application version is v2.0.2 ``` # Solution Strategy diff --git a/charts/daps-reg-service/Chart.yaml b/charts/daps-reg-service/Chart.yaml index 7cdee4a..c0e64bd 100644 --- a/charts/daps-reg-service/Chart.yaml +++ b/charts/daps-reg-service/Chart.yaml @@ -38,11 +38,11 @@ sources: # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 2.0.1 +version: 2.0.2 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: 2.0.1 +appVersion: 2.0.2 diff --git a/charts/daps-reg-service/README.md b/charts/daps-reg-service/README.md index 25406c9..7ac1e85 100644 --- a/charts/daps-reg-service/README.md +++ b/charts/daps-reg-service/README.md @@ -1,6 +1,6 @@ # daps-reg-service -![Version: 2.0.1](https://img.shields.io/badge/Version-2.0.1-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 2.0.1](https://img.shields.io/badge/AppVersion-2.0.1-informational?style=flat-square) +![Version: 2.0.2](https://img.shields.io/badge/Version-2.0.2-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 2.0.2](https://img.shields.io/badge/AppVersion-2.0.2-informational?style=flat-square) Daps regisgter service is used to register the EDC connector into DAPS diff --git a/charts/daps-reg-service/values.yaml b/charts/daps-reg-service/values.yaml index 457e003..a50adee 100644 --- a/charts/daps-reg-service/values.yaml +++ b/charts/daps-reg-service/values.yaml @@ -31,7 +31,7 @@ image: # -- Set the Image Pull Policy pullPolicy: Always # -- Image tage is defined in chart appVersion. - tag: "2.0.2-SNAPSHOT" + tag: "" imagePullSecrets: [] From 4562bd3aaf0bd01764a1761461937ecc14ff7725 Mon Sep 17 00:00:00 2001 From: dvasunin Date: Mon, 3 Apr 2023 17:18:58 +0300 Subject: [PATCH 06/13] add validator --- .../tractusx/dapsreg/service/DapsManager.java | 7 +- .../dapsreg/util/AttributeValidator.java | 41 +++++++++++ .../tractusx/dapsreg/util/JsonUtil.java | 5 ++ src/main/resources/application.yml | 1 + .../tractusx/dapsreg/DapsUtilTests.java | 17 +++++ .../tractusx/dapsreg/DapsregE2eTest.java | 68 +++++++++++++++++++ src/test/resources/application-test.yml | 1 + 7 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java index d6ee0a9..813658b 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java +++ b/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,7 @@ public class DapsManager implements DapsApiDelegate { private final ObjectMapper mapper; private final JsonUtil jsonUtil; private final StaticJson staticJson; + private final AttributeValidator attributeValidator; @SneakyThrows @Override @@ -92,7 +94,10 @@ public synchronized ResponseEntity> getClientGet(String clie @Override @PreAuthorize("hasAuthority(@securityRoles.updateRole)") public synchronized ResponseEntity updateClientPut(String clientId, Map newAttr) { - var clientAttr = dapsClient.getClient(clientId).map(jsn-> jsn.get("attributes")).orElseThrow(); + 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(), diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/AttributeValidator.java new file mode 100644 index 0000000..4c67ba0 --- /dev/null +++ b/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"); + } + } + +} diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java index e4f2adb..fd36c35 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/util/JsonUtil.java +++ b/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,6 +41,7 @@ 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)); @@ -47,6 +49,9 @@ public JsonNode getCertificateJson(X509Certificate x509Certificate) throws IOExc public JsonNode getClientJson(String clientId, String clientName, String securityProfile, String referringConnector) { + attributeValidator.validate(clientId); + attributeValidator.validate(clientName); + attributeValidator.validate(securityProfile); ObjectNode objectNode = mapper.createObjectNode(); objectNode.put("client_id", Optional.ofNullable(clientId) diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index b568326..79bd34a 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -9,6 +9,7 @@ springdoc: app: build: version: ^project.version^ + maxAttrLen: 512 daps: #apiUri: #tokenUri: diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java index 628a847..588f716 100644 --- 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.cert.CertificateException; @@ -47,6 +51,9 @@ class DapsUtilTests { @Autowired ObjectMapper mapper; + @Autowired + AttributeValidator attributeValidator; + @PostConstruct public void init() { mapper.enable(SerializationFeature.INDENT_OUTPUT); @@ -68,6 +75,16 @@ void utilTest() throws IOException, CertificateException { } } + @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 index ef4f89c..dd34fe3 100644 --- a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java +++ b/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; @@ -62,6 +70,66 @@ private JsonNode getClient(String client_id) throws Exception { return response; } + + @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")) + .andExpect(status().is4xxClientError()); + } + } + + static class MyArgumentsProvider implements ArgumentsProvider { + @Override + public Stream 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")) + .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) + ).andExpect(status().is4xxClientError()); + } finally { + if (!Objects.isNull(clientId)) { + mockMvc.perform(delete("/api/v1/daps/".concat(clientId))).andExpect(status().is2xxSuccessful()); + } + } + } + @Test @WithMockUser(username = "fulladmin", authorities={"create_daps_client", "update_daps_client", "delete_daps_client", "retrieve_daps_client"}) void createRetrieveChangeDeleteTest() throws Exception { diff --git a/src/test/resources/application-test.yml b/src/test/resources/application-test.yml index 3243e54..f869325 100644 --- a/src/test/resources/application-test.yml +++ b/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 From 35475acbde7e17dace0411dae88d1fbac01bc9b9 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Tue, 4 Apr 2023 14:09:58 +0530 Subject: [PATCH 07/13] changes --- charts/daps-reg-service/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/daps-reg-service/values.yaml b/charts/daps-reg-service/values.yaml index a50adee..959aeab 100644 --- a/charts/daps-reg-service/values.yaml +++ b/charts/daps-reg-service/values.yaml @@ -31,7 +31,7 @@ image: # -- Set the Image Pull Policy pullPolicy: Always # -- Image tage is defined in chart appVersion. - tag: "" + tag: "2.0.2-PEN" imagePullSecrets: [] From 4ba163b5504a9756b88825d927effc7e1d2c746a Mon Sep 17 00:00:00 2001 From: dvasunin Date: Wed, 19 Apr 2023 11:07:06 +0300 Subject: [PATCH 08/13] =?UTF-8?q?upgrade=20Spring=20Expression=20Language?= =?UTF-8?q?=20(SpEL)=20=C2=BB=206.0.8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 4 +++- pom.xml | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 548f64a..17400cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] -NA +### Changed +- Upgrade Spring Expression Language (SpEL) ยป 6.0.8 + ## [2.0.2] - 2023-03-31 diff --git a/pom.xml b/pom.xml index baf81ed..2fc2e5d 100644 --- a/pom.xml +++ b/pom.xml @@ -72,6 +72,11 @@ snakeyaml 2.0 + + org.springframework + spring-expression + 6.0.8 + org.springframework.boot From 439e929f9e79661a066372a2e35a850188d3ada6 Mon Sep 17 00:00:00 2001 From: dvasunin Date: Wed, 19 Apr 2023 23:42:16 +0300 Subject: [PATCH 09/13] fix in pathSegment --- .../tractusx/dapsreg/service/DapsClient.java | 12 +++---- .../tractusx/dapsreg/DapsregE2eTest.java | 35 +++++++++++++------ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java index 6086a0c..49fe0bc 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java +++ b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsClient.java @@ -51,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 @@ -111,7 +111,7 @@ public Optional> 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,7 +122,7 @@ public HttpStatus updateClient(JsonNode json, String clientId) { public Optional 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() @@ -132,11 +132,11 @@ public Optional getClient(String clientId) { } 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 pathBuilder) { @@ -151,7 +151,7 @@ private HttpStatus deleteSomething(UnaryOperator 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) diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java index dd34fe3..88fa26b 100644 --- a/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java +++ b/src/test/java/org/eclipse/tractusx/dapsreg/DapsregE2eTest.java @@ -63,10 +63,12 @@ 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)); + //System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(response)); return response; } @@ -82,6 +84,7 @@ void createClientBadSymbolsInClientNameTest(String attrValue) throws Exception { .file(pemFile) .param("clientName", attrValue) .param("referringConnector", "http://connector.cx-preprod.edc.aws.bmw.cloud/BPN1234567890")) + .andDo(print()) .andExpect(status().is4xxClientError()); } } @@ -112,6 +115,7 @@ void updateClientAttrBadSymbolsTest(String attrName, String attrValue) throws Ex .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")) @@ -121,11 +125,14 @@ void updateClientAttrBadSymbolsTest(String attrName, String attrValue) throws Ex var orig = getClient(clientId); assertThat(orig.get("name").asText()).isEqualTo("bmw preprod"); mockMvc.perform(put("/api/v1/daps/".concat(clientId)) - .param(attrName, attrValue) - ).andExpect(status().is4xxClientError()); + .param(attrName, attrValue)) + .andDo(print()) + .andExpect(status().is4xxClientError()); } 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()); } } } @@ -143,6 +150,7 @@ void createRetrieveChangeDeleteTest() throws Exception { .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")) @@ -152,9 +160,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(); @@ -164,7 +173,9 @@ 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()); } } } @@ -182,6 +193,7 @@ void createTwoSameExpectErrorTest() throws Exception { .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")) @@ -194,10 +206,13 @@ void createTwoSameExpectErrorTest() throws Exception { .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))).andExpect(status().is2xxSuccessful()); + mockMvc.perform(delete("/api/v1/daps/".concat(clientId))) + .andDo(print()) + .andExpect(status().is2xxSuccessful()); } } } From 140992bae8d1f2b52af60285265d5242dc7bab7e Mon Sep 17 00:00:00 2001 From: dvasunin Date: Thu, 11 May 2023 14:09:31 +0300 Subject: [PATCH 10/13] fix possible problem with SKI spoofing --- .../eclipse/tractusx/dapsreg/service/DapsManager.java | 3 +++ .../java/org/eclipse/tractusx/dapsreg/util/Certutil.java | 9 +++++++++ .../java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java | 5 ++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java index 813658b..4900afc 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java +++ b/src/main/java/org/eclipse/tractusx/dapsreg/service/DapsManager.java @@ -67,6 +67,9 @@ public synchronized ResponseEntity> createClientPost(String String securityProfile) { var cert = Certutil.loadCertificate(new String(file.getBytes())); var clientId = Certutil.getClientId(cert); + if (!Certutil.createSki(cert).equals(Certutil.getSki(cert))) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Certificate problem"); + } if (dapsClient.getClient(clientId).isPresent()) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Client exists"); } diff --git a/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java b/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java index 2cf2648..b12c3c5 100644 --- a/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java +++ b/src/main/java/org/eclipse/tractusx/dapsreg/util/Certutil.java @@ -25,11 +25,14 @@ import org.bouncycastle.asn1.ASN1OctetString; import org.bouncycastle.asn1.x509.AuthorityKeyIdentifier; import org.bouncycastle.asn1.x509.SubjectKeyIdentifier; +import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils; import org.bouncycastle.openssl.jcajce.JcaPEMWriter; import java.io.ByteArrayInputStream; 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; @@ -56,6 +59,12 @@ public static String getSki(X509Certificate cert) { return BaseEncoding.base16().upperCase().withSeparator(":", 2).encode(keyIdentifier); } + public static String createSki(X509Certificate cert) throws NoSuchAlgorithmException { + PublicKey publicKey = cert.getPublicKey(); + var r = new JcaX509ExtensionUtils().createSubjectKeyIdentifier(publicKey).getKeyIdentifier(); + return BaseEncoding.base16().upperCase().withSeparator(":", 2).encode(r); + } + public static X509Certificate loadCertificate(String pem) throws IOException, CertificateException { try(var ts = new ByteArrayInputStream(pem.getBytes(Charsets.UTF_8))) { CertificateFactory fac = CertificateFactory.getInstance("X509"); diff --git a/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java b/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java index 588f716..ef42287 100644 --- a/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java +++ b/src/test/java/org/eclipse/tractusx/dapsreg/DapsUtilTests.java @@ -37,6 +37,7 @@ import org.springframework.web.server.ResponseStatusException; import java.io.IOException; +import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import static org.assertj.core.api.Assertions.assertThat; @@ -60,12 +61,14 @@ public void init() { } @Test - void utilTest() throws IOException, CertificateException { + void utilTest() throws IOException, CertificateException, NoSuchAlgorithmException { try (var pemStream = Resources.getResource("test.crt").openStream()) { var pem = new String(pemStream.readAllBytes()); var cert = Certutil.loadCertificate(pem); var clientId = Certutil.getClientId(cert); + var ski = Certutil.createSki(cert); assertThat(clientId).isEqualTo("65:FA:DE:C2:6A:58:98:D8:EA:FC:70:27:76:A0:75:D5:A1:C4:89:F9:keyid:65:FA:DE:C2:6A:58:98:D8:EA:FC:70:27:76:A0:75:D5:A1:C4:89:F9"); + assertThat(ski).isEqualTo(Certutil.getSki(cert)); var certPem = Certutil.getCertificate(cert); System.out.println(certPem); var certJson = jsonUtil.getCertificateJson(cert); From 8fbdb726761f62b69fb81c04630ee7edee8b1078 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Fri, 12 May 2023 15:32:28 +0530 Subject: [PATCH 11/13] Removed temporary tag --- charts/daps-reg-service/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/daps-reg-service/values.yaml b/charts/daps-reg-service/values.yaml index b3e88e5..a61bde2 100644 --- a/charts/daps-reg-service/values.yaml +++ b/charts/daps-reg-service/values.yaml @@ -31,7 +31,7 @@ image: # -- Set the Image Pull Policy pullPolicy: Always # -- Image tage is defined in chart appVersion. - tag: "2.0.2-PEN" + tag: "" imagePullSecrets: [] From e949f472560f8574cff3e3e02c4240351d6fb508 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Mon, 15 May 2023 13:31:13 +0530 Subject: [PATCH 12/13] Updated drs secret token --- charts/daps-reg-service/values-beta.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/daps-reg-service/values-beta.yaml b/charts/daps-reg-service/values-beta.yaml index a5dc53f..ae1b93e 100644 --- a/charts/daps-reg-service/values-beta.yaml +++ b/charts/daps-reg-service/values-beta.yaml @@ -50,7 +50,7 @@ drs: secret: clientId: "" clientSecret: "" - apiUri: "" - tokenUri: "" + apiUri: "" + tokenUri: "" daps_jwks: "" jwkSetUri: "" From d0d11f2b9422a1cea017aaf897a26ffa8dfe9a14 Mon Sep 17 00:00:00 2001 From: adkumar1 Date: Mon, 15 May 2023 13:42:25 +0530 Subject: [PATCH 13/13] Fix for duplicate client Id --- CHANGELOG.md | 8 ++++++++ README.md | 4 ++-- charts/daps-reg-service/Chart.yaml | 4 ++-- charts/daps-reg-service/README.md | 2 +- pom.xml | 2 +- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cc9563..afea4eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] NA +## [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 index 16c07a5..b39f919 100644 --- 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/Chart.yaml b/charts/daps-reg-service/Chart.yaml index 06edf6e..1cf41b2 100644 --- a/charts/daps-reg-service/Chart.yaml +++ b/charts/daps-reg-service/Chart.yaml @@ -38,11 +38,11 @@ sources: # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 2.0.6 +version: 2.0.7 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: 2.0.6 +appVersion: 2.0.7 diff --git a/charts/daps-reg-service/README.md b/charts/daps-reg-service/README.md index ab75b25..204c3cc 100644 --- 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/pom.xml b/pom.xml index 92cf810..40defd4 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ org.eclipse.tractusx dapsreg - 2.0.6 + 2.0.7 dapsreg client registration to the DAPS