From 345b56912dd006ea587aa75a63ec315f33d162fc Mon Sep 17 00:00:00 2001 From: Ernst-Christoph Schrewe Date: Fri, 14 Jun 2024 08:45:41 +0200 Subject: [PATCH] fix: review issues --- .../ErpAdapterSecurityConfiguration.java | 44 +++++++++++++++++ .../backend/common/util/VariablesService.java | 27 ---------- .../service/ErpAdapterRequestClient.java | 10 ++-- .../src/main/resources/application.properties | 1 + .../service/ErpAdapterRequestClientTest.java | 49 ++++--------------- .../src/test/resources/application.properties | 4 +- charts/puris/README.md | 1 + .../puris/templates/backend-deployment.yaml | 7 ++- charts/puris/templates/backend-secrets.yaml | 2 - charts/puris/values.yaml | 2 + 10 files changed, 69 insertions(+), 78 deletions(-) create mode 100644 backend/src/main/java/org/eclipse/tractusx/puris/backend/common/security/ErpAdapterSecurityConfiguration.java diff --git a/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/security/ErpAdapterSecurityConfiguration.java b/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/security/ErpAdapterSecurityConfiguration.java new file mode 100644 index 00000000..93ab5ee5 --- /dev/null +++ b/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/security/ErpAdapterSecurityConfiguration.java @@ -0,0 +1,44 @@ +package org.eclipse.tractusx.puris.backend.common.security; + +import lombok.Getter; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Configuration; + +@Configuration +@Getter +public class ErpAdapterSecurityConfiguration { + + + /** + * Toggles usage of the ERP adapter + */ + @Value("${puris.erpadapter.enabled}") + private boolean erpAdapterEnabled; + + /** + * The URL of the ERP adapter + */ + @Value("${puris.erpadapter.url}") + private String erpAdapterUrl; + + /** + * The URL under which we expect responses from + * the ERP adapter + */ + @Value("${puris.baseurl}" + "catena/erp-adapter") + private String erpResponseUrl; + + /** + * The auth-key used when accessing the ERP adapter's + * request interface + */ + @Value("${puris.erpadapter.authkey}") + private String erpAdapterAuthKey; + + /** + * The auth-secret used when accessing the ERP adapter's + * request interface + */ + @Value("${puris.erpadapter.authsecret}") + private String erpAdapterAuthSecret; +} diff --git a/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/util/VariablesService.java b/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/util/VariablesService.java index c5f3c7b8..9f51420e 100644 --- a/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/util/VariablesService.java +++ b/backend/src/main/java/org/eclipse/tractusx/puris/backend/common/util/VariablesService.java @@ -50,33 +50,6 @@ public class VariablesService { */ private String demoRole; - @Value("${puris.erpadapter.url}") - /** - * The URL of the ERP adapter - */ - private String erpAdapterUrl; - - /** - * The URL under which we expect responses from - * the ERP adapter - */ - @Value("${puris.baseurl}" + "catena/erp-adapter") - private String erpResponseUrl; - - /** - * The auth-key used when accessing the ERP adapter's - * request interface - */ - @Value("${puris.erpadapter.authkey}") - private String erpAdapterAuthKey; - - /** - * The auth-secret used when accessing the ERP adapter's - * request interface - */ - @Value("${puris.erpadapter.authsecret}") - private String erpAdapterAuthSecret; - @Value("${puris.baseurl}" + "catena/item-stock/request") /** * The url under which this application's request endpoint can diff --git a/backend/src/main/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClient.java b/backend/src/main/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClient.java index cd25a5dd..229e45c2 100644 --- a/backend/src/main/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClient.java +++ b/backend/src/main/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClient.java @@ -25,7 +25,7 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import okhttp3.*; -import org.eclipse.tractusx.puris.backend.common.util.VariablesService; +import org.eclipse.tractusx.puris.backend.common.security.ErpAdapterSecurityConfiguration; import org.eclipse.tractusx.puris.backend.erpadapter.domain.model.ErpAdapterRequest; import org.springframework.stereotype.Service; @@ -40,10 +40,10 @@ public class ErpAdapterRequestClient { private final ObjectMapper mapper = new ObjectMapper(); - private final VariablesService variablesService; + private final ErpAdapterSecurityConfiguration erpAdapterSecurityConfiguration; public Integer sendRequest(ErpAdapterRequest erpAdapterRequest){ - HttpUrl.Builder urlBuilder = HttpUrl.parse(variablesService.getErpAdapterUrl()).newBuilder(); + HttpUrl.Builder urlBuilder = HttpUrl.parse(erpAdapterSecurityConfiguration.getErpAdapterUrl()).newBuilder(); urlBuilder.addQueryParameter("bpnl", erpAdapterRequest.getPartnerBpnl()); urlBuilder.addQueryParameter("request-type", erpAdapterRequest.getRequestType()); urlBuilder.addQueryParameter("request-id", erpAdapterRequest.getId().toString()); @@ -54,14 +54,14 @@ public Integer sendRequest(ErpAdapterRequest erpAdapterRequest){ requestBody.put("material", erpAdapterRequest.getOwnMaterialNumber()); requestBody.put("direction", erpAdapterRequest.getDirectionCharacteristic().toString()); - requestBody.put("responseUrl", variablesService.getErpResponseUrl()); + requestBody.put("responseUrl", erpAdapterSecurityConfiguration.getErpResponseUrl()); RequestBody body = RequestBody.create(requestBody.toString(), MediaType.parse("application/json")); Request request = new Request.Builder() .post(body) .url(urlBuilder.build()) - .header(variablesService.getErpAdapterAuthKey(), variablesService.getErpAdapterAuthSecret()) + .header(erpAdapterSecurityConfiguration.getErpAdapterAuthKey(), erpAdapterSecurityConfiguration.getErpAdapterAuthSecret()) .header("Content-Type", "application/json") .build(); try (var response = client.newCall(request).execute()) { diff --git a/backend/src/main/resources/application.properties b/backend/src/main/resources/application.properties index 8d7976f2..970f0432 100755 --- a/backend/src/main/resources/application.properties +++ b/backend/src/main/resources/application.properties @@ -21,6 +21,7 @@ puris.dtr.idp.edc-client.id=${PURIS_DTR_IDP_EDC-CLIENT_ID:FOSS-DTR-CLIENT} puris.dtr.idp.edc-client.secret.alias=${PURIS_DTR_IDP_EDC-CLIENT_SECRET_ALIAS} puris.dtr.idp.puris-client.id=${PURIS_DTR_IDP_PURIS-CLIENT_ID:FOSS-DTR-CLIENT} puris.dtr.idp.puris-client.secret=${PURIS_DTR_IDP_PURIS-CLIENT_SECRET} +puris.erpadapter.enabled=${PURIS_ERPADAPTER_ENABLED:false} puris.erpadapter.url=${PURIS_ERPADAPTER_URL:http://my-erpadapter:8080} puris.erpadapter.authkey=${PURIS_ERPADAPTER_AUTHKEY:x-api-key} puris.erpadapter.authsecret=${PURIS_ERPADAPTER_AUTHSECRET:erp-password} diff --git a/backend/src/test/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClientTest.java b/backend/src/test/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClientTest.java index 58df1ab3..ef9ed9b6 100644 --- a/backend/src/test/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClientTest.java +++ b/backend/src/test/java/org/eclipse/tractusx/puris/backend/erpadapter/logic/service/ErpAdapterRequestClientTest.java @@ -25,7 +25,7 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; import org.assertj.core.api.Assertions; -import org.eclipse.tractusx.puris.backend.common.util.VariablesService; +import org.eclipse.tractusx.puris.backend.common.security.ErpAdapterSecurityConfiguration; import org.eclipse.tractusx.puris.backend.erpadapter.domain.model.ErpAdapterRequest; import org.eclipse.tractusx.puris.backend.stock.logic.dto.itemstocksamm.DirectionCharacteristic; import org.junit.jupiter.api.AfterEach; @@ -50,8 +50,9 @@ public class ErpAdapterRequestClientTest { private MockWebServer mockWebServer; + @Mock - private VariablesService variablesService; + private ErpAdapterSecurityConfiguration erpAdapterSecurityConfiguration; @InjectMocks private ErpAdapterRequestClient erpAdapterRequestClient; @@ -76,7 +77,7 @@ public class ErpAdapterRequestClientTest { public void setUp() throws Exception { mockWebServer = new MockWebServer(); mockWebServer.start(); - erpAdapterRequestClient = new ErpAdapterRequestClient(variablesService); + erpAdapterRequestClient = new ErpAdapterRequestClient(erpAdapterSecurityConfiguration); } @@ -101,10 +102,10 @@ public void test_should_success() throws Exception { .build(); // when - Mockito.when(variablesService.getErpAdapterUrl()).thenReturn(mockWebServer.url("/").toString()); - Mockito.when(variablesService.getErpAdapterAuthKey()).thenReturn(apiKey); - Mockito.when(variablesService.getErpAdapterAuthSecret()).thenReturn(apiSecret); - Mockito.when(variablesService.getErpResponseUrl()).thenReturn(erpResponseUrl); + Mockito.when(erpAdapterSecurityConfiguration.getErpAdapterUrl()).thenReturn(mockWebServer.url("/").toString()); + Mockito.when(erpAdapterSecurityConfiguration.getErpAdapterAuthKey()).thenReturn(apiKey); + Mockito.when(erpAdapterSecurityConfiguration.getErpAdapterAuthSecret()).thenReturn(apiSecret); + Mockito.when(erpAdapterSecurityConfiguration.getErpResponseUrl()).thenReturn(erpResponseUrl); erpAdapterRequestClient.sendRequest(erpAdapterRequest); RecordedRequest request = mockWebServer.takeRequest(2, TimeUnit.SECONDS); @@ -116,10 +117,8 @@ public void test_should_success() throws Exception { var pairs = request.getPath().substring(2).split("&"); Map parameters = Stream.of(pairs) - .map(string -> { - var keyValue = string.split("="); - return Map.entry(keyValue[0], keyValue[1]); - }).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + .map(string -> string.split("=")) + .collect(Collectors.toMap(pair -> pair[0], pair -> pair[1])); Assertions.assertThat(parameters.size()).isEqualTo(5); Assertions.assertThat(parameters.get("bpnl")).isEqualTo(supplierPartnerBpnl); @@ -136,32 +135,4 @@ public void test_should_success() throws Exception { Assertions.assertThat(requestBodyNode.get("responseUrl").asText()).isEqualTo(erpResponseUrl); } } - - @Test - public void test_with_faulty_credentials_should_fail() throws Exception { - // given - UUID uuid = UUID.randomUUID(); - ErpAdapterRequest erpAdapterRequest = ErpAdapterRequest.builder() - .requestDate(new Date()) - .partnerBpnl(supplierPartnerBpnl) - .id(uuid) - .directionCharacteristic(DirectionCharacteristic.INBOUND) - .ownMaterialNumber(matNbrCustomer) - .requestType(requestType) - .sammVersion(sammVersion) - .build(); - - // when - Mockito.when(variablesService.getErpAdapterUrl()).thenReturn(mockWebServer.url("/").toString()); - Mockito.when(variablesService.getErpAdapterAuthKey()).thenReturn("wrong-key"); - Mockito.when(variablesService.getErpAdapterAuthSecret()).thenReturn(apiSecret); - Mockito.when(variablesService.getErpResponseUrl()).thenReturn(erpResponseUrl); - erpAdapterRequestClient.sendRequest(erpAdapterRequest); - RecordedRequest request = mockWebServer.takeRequest(2, TimeUnit.SECONDS); - - // then - Assertions.assertThat(request.getMethod()).isEqualTo("POST"); - Assertions.assertThat(request.getHeader("Content-type")).contains("application/json"); - Assertions.assertThat(request.getHeader(apiKey)).isNotEqualTo(apiSecret); - } } diff --git a/backend/src/test/resources/application.properties b/backend/src/test/resources/application.properties index 747f5b56..bf266c71 100755 --- a/backend/src/test/resources/application.properties +++ b/backend/src/test/resources/application.properties @@ -20,8 +20,10 @@ puris.dtr.idp.edc-client.secret.alias=${PURIS_DTR_IDP_EDC-CLIENT_SECRET_ALIAS:te puris.dtr.idp.puris-client.id=${PURIS_DTR_IDP_PURIS-CLIENT_ID:FOSS-DTR-CLIENT} puris.dtr.idp.puris-client.secret=${PURIS_DTR_IDP_PURIS-CLIENT_SECRET:test} +puris.erpadapter.enabled=${PURIS_ERPADAPTER_ENABLED:false} puris.erpadapter.url=${PURIS_ERPADAPTER_URL:http://my-erpadapter:8080} - +puris.erpadapter.authkey=${PURIS_ERPADAPTER_AUTHKEY:x-api-key} +puris.erpadapter.authsecret=${PURIS_ERPADAPTER_AUTHSECRET:erp-password} puris.generatematerialcatenaxid=${PURIS_GENERATEMATERIALCATENAXID:true} # DB Configuration diff --git a/charts/puris/README.md b/charts/puris/README.md index 2e6c056f..6c7f73bf 100644 --- a/charts/puris/README.md +++ b/charts/puris/README.md @@ -98,6 +98,7 @@ dependencies: | backend.puris.edc.dataplane.public.url | string | `"https://your-data-plane:8285/api/public/"` | Url of one of your data plane's public api | | backend.puris.erpadapter.authkey | string | `"x-api-key"` | | | backend.puris.erpadapter.authsecret | string | `"erp-password"` | | +| backend.puris.erpadapter.enabled | bool | `false` | | | backend.puris.erpadapter.url | string | `"http://my-erpadapter:8080"` | | | backend.puris.existingSecret | string | `"secret-puris-backend"` | Secret for backend passwords. For more information look into 'backend-secrets.yaml' file. | | backend.puris.frameworkagreement.credential | string | `"Puris"` | The name of the framework agreement. Starting with Uppercase and using CamelCase. | diff --git a/charts/puris/templates/backend-deployment.yaml b/charts/puris/templates/backend-deployment.yaml index 481138b8..e252155a 100644 --- a/charts/puris/templates/backend-deployment.yaml +++ b/charts/puris/templates/backend-deployment.yaml @@ -168,13 +168,12 @@ spec: key: "puris-dtr-idp-puris-client-secret" - name: PURIS_GENERATEMATERIALCATENAXID value: "{{ .Values.backend.puris.generatematerialcatenaxid | default true}}" + - name: PURIS_ERPADAPTER_ENABLED + value: "{{ .Values.backend.puris.erpadapter.enabled}}" - name: PURIS_ERPADAPTER_URL value: "{{ .Values.backend.puris.erpadapter.url}}" - name: PURIS_ERPADAPTER_AUTHKEY - valueFrom: - secretKeyRef: - name: "{{ .Values.backend.puris.existingSecret }}" - key: "puris-erpadapter-authkey" + value: {{ .Values.backend.puris.erpadapter.authkey }} - name: PURIS_ERPADAPTER_AUTHSECRET valueFrom: secretKeyRef: diff --git a/charts/puris/templates/backend-secrets.yaml b/charts/puris/templates/backend-secrets.yaml index 22d024de..35fdcf40 100644 --- a/charts/puris/templates/backend-secrets.yaml +++ b/charts/puris/templates/backend-secrets.yaml @@ -17,7 +17,6 @@ data: puris-datasource-password: {{ (.Values.backend.puris.datasource.password | b64enc) | default (index $secret.data "puris-datasource-password") | quote }} puris-edc-controlplane-key: {{ (.Values.backend.puris.edc.controlplane.key | b64enc) | default (index $secret.data "puris-edc-controlplane-key") | quote }} puris-dtr-idp-puris-client-secret: {{ (.Values.backend.puris.dtr.idp.clients.puris.secret | b64enc) | default (index $secret.data "puris-dtr-idp-puris-client-secret") | quote }} - puris-erpadapter-authkey: {{ (.Values.backend.puris.erpadapter.authkey | b64enc) default (index $secret.data "puris-erpadapter-authkey") | quote }} puris-erpadapter-authsecret: {{ (.Values.backend.puris.erpadapter.authsecret | b64enc) default (index $secret.data "puris-erpadapter-authsecret") | quote }} {{ else -}} stringData: @@ -27,6 +26,5 @@ stringData: puris-edc-controlplane-key: {{ .Values.backend.puris.edc.controlplane.key | default ( randAlphaNum 32 ) | quote }} # don't generate a random one as this is set in identity provider puris-dtr-idp-puris-client-secret: {{ .Values.backend.puris.dtr.idp.clients.puris.secret | quote }} - puris-erpadapter-authkey: {{ .Values.backend.puris.erpadapter.authkey | default ("x-api-key") | quote }} puris-erpadapter-authsecret: {{ .Values.backend.puris.erpadapter.authkey | default ( randAlphaNum 32 ) | quote }} {{ end }} diff --git a/charts/puris/values.yaml b/charts/puris/values.yaml index ee684248..aff45ad8 100644 --- a/charts/puris/values.yaml +++ b/charts/puris/values.yaml @@ -472,6 +472,8 @@ backend: # Material entity. generatematerialcatenaxid: true erpadapter: + # Toggles usage of the ERP adapter + enabled: false # The url of your ERP adapter's request api url: http://my-erpadapter:8080 # The auth key to be used on your ERP adapter's request api