feat: make effective constraint on multiple signatures in SamlResponse (#879)

Author: giuseppe-gangemi

src/oneid/oneid-ecs-core/src/main/java/it/pagopa/oneid/service/utils/SAMLUtilsExtendedCore.java

@@ -209,8 +209,7 @@ private Response unmarshallResponse(byte[] decodedSamlResponse) throws OneIdenti
     } catch (SAXException | IOException | ParserConfigurationException |
              XPathExpressionException e) {
       Log.error("XML parsing exception " + e.getMessage());
-      // TODO uncomment once feature is active
-      // throw new OneIdentityException(e);
+      throw new OneIdentityException(e);
     }
 
     try {

src/oneid/oneid-ecs-core/src/main/java/it/pagopa/oneid/web/controller/SAMLController.java

@@ -95,8 +95,7 @@ public Response samlACS(@BeanParam @Valid SAMLResponseDTO samlResponseDTO) {
       cloudWatchConnectorImpl.sendIDPErrorMetricData(
           samlSession.getAuthorizationRequestDTOExtended().getIdp(),
           ErrorCode.IDP_ERROR_MULTIPLE_SAMLRESPONSE_SIGNATURES_PRESENT);
-      // TODO uncomment to activate the check for multiple signatures
-      // throw new GenericHTMLException(ErrorCode.IDP_ERROR_MULTIPLE_SAMLRESPONSE_SIGNATURES_PRESENT);
+      throw new GenericHTMLException(ErrorCode.IDP_ERROR_MULTIPLE_SAMLRESPONSE_SIGNATURES_PRESENT);
     }
 
     // 1d. Check status, will raise CustomException in case of error mapped to a custom html error page

src/oneid/oneid-ecs-core/src/test/java/it/pagopa/oneid/web/controller/SAMLControllerTest.java

@@ -10,6 +10,7 @@
 import com.nimbusds.oauth2.sdk.id.State;
 import io.quarkus.test.InjectMock;
 import io.quarkus.test.common.http.TestHTTPEndpoint;
+import io.quarkus.test.junit.QuarkusMock;
 import io.quarkus.test.junit.QuarkusTest;
 import io.quarkus.test.junit.TestProfile;
 import it.pagopa.oneid.common.model.exception.OneIdentityException;
@@ -23,6 +24,7 @@
 import it.pagopa.oneid.service.SessionServiceImpl;
 import it.pagopa.oneid.web.controller.interceptors.CurrentAuthDTO;
 import it.pagopa.oneid.web.controller.mock.SAMLControllerTestProfile;
+import it.pagopa.oneid.web.dto.AuthorizationRequestDTOExtended;
 import jakarta.inject.Inject;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
@@ -413,43 +415,46 @@ void samlACS_exceptionInCreatingCallbackURI() {
     Assertions.assertTrue(location.contains(headerLocation));
   }
 
-// TODO re-enable this test when the feature is back
-//  @Test
-//  @SneakyThrows
-//  void samlACS_SAMLResponseWithMultipleSignatures() {
-//    // given
-//    CurrentAuthDTO mockAuthDTO = Mockito.mock(CurrentAuthDTO.class);
-//    QuarkusMock.installMockForType(mockAuthDTO, CurrentAuthDTO.class);
-//
-//    Map<String, String> samlResponseDTO = new HashMap<>();
-//    samlResponseDTO.put("SAMLResponse", "dummySAMLResponse");
-//    samlResponseDTO.put("RelayState", "dummyRelayState");
-//
-//    // Mock CurrentAuthDTO to simulate multiple signatures scenario
-//    Mockito.when(mockAuthDTO.isResponseWithMultipleSignatures()).thenReturn(true);
-//
-//    // Setup mocks for response and samlSession as usual, but flow will stop at the multiple signatures check
-//    Response response = Mockito.mock(Response.class);
-//    Mockito.when(response.getInResponseTo()).thenReturn("Dummy");
-//    Mockito.when(samlServiceImpl.getSAMLResponseFromString(Mockito.any())).thenReturn(response);
-//    Mockito.when(mockAuthDTO.getResponse()).thenReturn(response);
-//
-//    AuthorizationRequestDTOExtended dto = Mockito.mock(AuthorizationRequestDTOExtended.class);
-//    Mockito.when(dto.getIdp()).thenReturn("dummy-idp"); // Stub idp for cloudwatch metrics
-//    SAMLSession samlSession = Mockito.mock(SAMLSession.class);
-//    Mockito.when(samlSession.getAuthorizationRequestDTOExtended()).thenReturn(dto);
-//    Mockito.when(mockAuthDTO.getSamlSession()).thenReturn(samlSession);
-//
-//    // HTTP 302
-//    given()
-//        .formParams(samlResponseDTO)
-//        .when()
-//        .post("/acs")
-//        .then()
-//        .statusCode(302)
-//        .header("Location", containsString(
-//            ErrorCode.IDP_ERROR_MULTIPLE_SAMLRESPONSE_SIGNATURES_PRESENT.getErrorCode()));
-//  }
+  @Test
+  @SneakyThrows
+  void samlACS_SAMLResponseWithMultipleSignatures() {
+    // given
+    CurrentAuthDTO mockAuthDTO = Mockito.mock(CurrentAuthDTO.class);
+    QuarkusMock.installMockForType(mockAuthDTO, CurrentAuthDTO.class);
+
+    Map<String, String> samlResponseDTO = new HashMap<>();
+    samlResponseDTO.put("SAMLResponse", "dummySAMLResponse");
+    samlResponseDTO.put("RelayState", "dummyRelayState");
+
+    // Mock CurrentAuthDTO to simulate multiple signatures scenario
+    Mockito.when(mockAuthDTO.isResponseWithMultipleSignatures()).thenReturn(true);
+
+    // Setup mocks for response and samlSession as usual, but flow will stop at the multiple signatures check
+    Response response = Mockito.mock(Response.class);
+    Mockito.when(response.getInResponseTo()).thenReturn("Dummy");
+    Mockito.when(samlServiceImpl.getSAMLResponseFromString(Mockito.any())).thenReturn(response);
+    Mockito.when(mockAuthDTO.getResponse()).thenReturn(response);
+
+    AuthorizationRequestDTOExtended dto = Mockito.mock(AuthorizationRequestDTOExtended.class);
+    Mockito.when(dto.getIdp()).thenReturn("dummy-idp"); // Stub idp for cloudwatch metrics
+    SAMLSession samlSession = Mockito.mock(SAMLSession.class);
+    Mockito.when(samlSession.getAuthorizationRequestDTOExtended()).thenReturn(dto);
+    Mockito.when(mockAuthDTO.getSamlSession()).thenReturn(samlSession);
+
+    // HTTP 302
+    String location = given()
+        .formParams(samlResponseDTO)
+        .when()
+        .post("/acs")
+        .then()
+        .statusCode(302)
+        .extract()
+        .header("location");
+    
+    Assertions.assertTrue(location.contains(
+        ErrorCode.IDP_ERROR_MULTIPLE_SAMLRESPONSE_SIGNATURES_PRESENT.getErrorCode()));
+
+  }
 
   @Test
   void assertion_ok() {