- Fixed behaviour of access rule management in case of multiple speci…

…ficAssetIds with same keys and different values.
eclipse-tractusx · Apr 2, 2024 · 32e7b5d · 32e7b5d
1 parent 3d83519
commit 32e7b5d
Show file tree

Hide file tree

Showing 10 changed files with 251 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 ## fixed
 - KICS findings fixed
-
+- Fixed behaviour of access rule management in case of multiple specificAssetIds with same keys and different values.
+- Update Springboot to version 3.2.4
 
 ## 0.4.1
 ### Added

diff --git a/.../java/org/eclipse/tractusx/semantics/accesscontrol/api/model/ShellVisibilityCriteria.java b/.../java/org/eclipse/tractusx/semantics/accesscontrol/api/model/ShellVisibilityCriteria.java
@@ -20,10 +20,11 @@
 
 package org.eclipse.tractusx.semantics.accesscontrol.api.model;
 
+import java.util.Map;
 import java.util.Set;
 
 import lombok.NonNull;
 
 public record ShellVisibilityCriteria(
-      @NonNull String aasId, @NonNull Set<String> visibleSpecificAssetIdNames, @NonNull Set<String> visibleSemanticIds, boolean publicOnly) {
+      @NonNull String aasId, @NonNull Map<String, Set<String>> visibleSpecificAssetIdNames, @NonNull Set<String> visibleSemanticIds, boolean publicOnly) {
 }
diff --git a/...a/org/eclipse/tractusx/semantics/accesscontrol/api/model/ShellVisibilityCriteriaTest.java b/...a/org/eclipse/tractusx/semantics/accesscontrol/api/model/ShellVisibilityCriteriaTest.java
@@ -22,6 +22,7 @@
 
 import static org.assertj.core.api.Assertions.*;
 
+import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.stream.Stream;
@@ -37,23 +38,24 @@ public static Stream<Arguments> nullProvider() {
       return Stream.<Arguments> builder()
             .add( Arguments.of( null, null, null, true ) )
             .add( Arguments.of( UUID.randomUUID().toString(), null, null, true ) )
-            .add( Arguments.of( null, Set.of(), null, false ) )
+            .add( Arguments.of( null, Map.of(), null, false ) )
             .add( Arguments.of( null, null, Set.of(), false ) )
             .build();
    }
 
    @ParameterizedTest
    @MethodSource( "nullProvider" )
    void testConstructorCalledWithNullExpectException(
-         String aasId, Set<String> visibleSpecificAssetIdNames, Set<String> visibleSemanticIds, boolean publicOnly ) {
+         String aasId, Map<String, Set<String>> visibleSpecificAssetIdNames, Set<String> visibleSemanticIds, boolean publicOnly ) {
       assertThatThrownBy( () -> new ShellVisibilityCriteria( aasId, visibleSpecificAssetIdNames, visibleSemanticIds, publicOnly ) )
             .isExactlyInstanceOf( NullPointerException.class );
    }
 
    @Test
    void testConstructorCalledWithValidDataExpectSuccess() {
       final String aasId = UUID.randomUUID().toString();
-      final Set<String> visibleSpecificAssetIdNames = Set.of( "name1" );
+
+      final Map<String, Set<String>> visibleSpecificAssetIdNames = Map.of( "name1", Set.of( "" ) );
       final Set<String> visibleSemanticIds = Set.of( "name2" );
       final boolean publicOnly = true;
 

diff --git a/...lipse/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleService.java b/...lipse/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleService.java
@@ -22,6 +22,8 @@
 
 import java.time.Instant;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -65,9 +67,11 @@ public List<String> filterValidSpecificAssetIdsForLookup(
                      .filter( accessControlRule -> aShellContext.specificAssetIds().containsAll( accessControlRule.getMandatorySpecificAssetIds() ) )
                      .flatMap( accessControlRule -> accessControlRule.getVisibleSpecificAssetIdNames().stream() )
                      .collect( Collectors.toSet() );
+               Map<String, Set<String>> duplicatedSpecificAssetIds = getDuplicateSpecificAssetIds( aShellContext );
                return aShellContext.specificAssetIds().stream()
-                     .filter( id -> visibleSpecificAssetIdNames.contains( id.name() ) )
-                     .collect( Collectors.toSet() ).containsAll( userQuery );
+                     .filter( id -> isSpecificAssetIdVisible( id, visibleSpecificAssetIdNames, duplicatedSpecificAssetIds ) )
+                     .collect( Collectors.toSet() )
+                     .containsAll( userQuery );
             } )
             .map( ShellVisibilityContext::aasId )
             .toList();
@@ -76,15 +80,28 @@ public List<String> filterValidSpecificAssetIdsForLookup(
    @Override
    public ShellVisibilityCriteria fetchVisibilityCriteriaForShell( ShellVisibilityContext shellContext, String bpn ) throws DenyAccessException {
       Set<AccessRulePolicy> matchingAccessControlRules = findMatchingAccessControlRules( shellContext, bpn );
-      Set<String> visibleSpecificAssetIdNames = matchingAccessControlRules.stream()
-            .flatMap( accessControlRule -> accessControlRule.getVisibleSpecificAssetIdNames().stream() )
-            .collect( Collectors.toSet() );
+
+      Map<String, Set<String>> duplicatedSpecificAssetIds = getDuplicateSpecificAssetIds( shellContext );
+
+      Map<String, Set<String>> visibleSpecificAssetIds = new HashMap<>();
+      matchingAccessControlRules.forEach( rule -> {
+         rule.getMandatorySpecificAssetIds().forEach( specificAssetId -> {
+            if ( duplicatedSpecificAssetIds.containsKey( specificAssetId.name() ) ) {
+               visibleSpecificAssetIds.computeIfAbsent( specificAssetId.name(), k -> new HashSet<>() ).add( specificAssetId.value() );
+            }
+         } );
+
+         rule.getVisibleSpecificAssetIdNames().forEach( specificAssetIdName -> {
+            visibleSpecificAssetIds.computeIfAbsent( specificAssetIdName, k -> new HashSet<>() );
+         } );
+      } );
+
       Set<String> visibleSemanticIds = matchingAccessControlRules.stream()
             .map( AccessRulePolicy::getVisibleSemanticIds )
             .flatMap( Collection::stream )
             .collect( Collectors.toSet() );
       boolean publicOnly = matchingAccessControlRules.stream().noneMatch( rule -> rule.getBpn().equals( bpn ) );
-      return new ShellVisibilityCriteria( shellContext.aasId(), visibleSpecificAssetIdNames, visibleSemanticIds, publicOnly );
+      return new ShellVisibilityCriteria( shellContext.aasId(), visibleSpecificAssetIds, visibleSemanticIds, publicOnly );
    }
 
    @Override
@@ -123,4 +140,20 @@ private Set<AccessRulePolicy> findMatchingAccessControlRules( ShellVisibilityCon
       }
       return matching;
    }
+
+   private Map<String, Set<String>> getDuplicateSpecificAssetIds( ShellVisibilityContext shellContext ) {
+      return shellContext.specificAssetIds().stream()
+            .collect( Collectors.groupingBy( SpecificAssetId::name,
+                  Collectors.mapping( SpecificAssetId::value, Collectors.toSet() ) ) )
+            .entrySet().stream()
+            .filter( specificAssetIds -> specificAssetIds.getValue().size() > 1 )
+            .collect( Collectors.toMap( Map.Entry::getKey, Map.Entry::getValue ) );
+   }
+
+   private boolean isSpecificAssetIdVisible( SpecificAssetId specificAssetId, Set<String> visibleSpecificAssetIds,
+         Map<String, Set<String>> duplicatedSpecificAssetIds ) {
+      Set<String> identifierValues = duplicatedSpecificAssetIds.getOrDefault( specificAssetId.name(), Set.of() );
+      return visibleSpecificAssetIds.contains( specificAssetId.name() )
+            && (identifierValues.isEmpty() || identifierValues.contains( specificAssetId.value() ));
+   }
 }
diff --git a/...e/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleServiceTest.java b/...e/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleServiceTest.java
@@ -29,6 +29,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
@@ -89,19 +90,37 @@ public static Stream<Arguments> matchingSpecificAssetIdFilterProvider() {
    public static Stream<Arguments> matchingSpecificAssetIdVisibilityProvider() {
       return Stream.<Arguments> builder()
             .add( Arguments.of(
-                  Set.of( MANUFACTURER_PART_ID_99991, CUSTOMER_PART_ID_ACME001, PART_INSTANCE_ID_00001, VERSION_NUMBER_01 ),
+                  Set.of( MANUFACTURER_PART_ID_99991, CUSTOMER_PART_ID_ACME001, PART_INSTANCE_ID_00001, PART_INSTANCE_ID_00002, VERSION_NUMBER_01 ),
                   BPNA,
-                  Set.of( MANUFACTURER_PART_ID, CUSTOMER_PART_ID, PART_INSTANCE_ID, VERSION_NUMBER ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(),
+                        Set.of(),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of(),
+                        PART_INSTANCE_ID_00001.name(), Set.of( PART_INSTANCE_ID_00001.value() ),
+                        VERSION_NUMBER_01.name(), Set.of()
+                  ),
                   Set.of( TRACEABILITYV_1_1_0 ) ) )
             .add( Arguments.of(
                   Set.of( MANUFACTURER_PART_ID_99991, CUSTOMER_PART_ID_ACME001, PART_INSTANCE_ID_00002, REVISION_NUMBER_01 ),
                   BPNA,
-                  Set.of( MANUFACTURER_PART_ID, CUSTOMER_PART_ID, PART_INSTANCE_ID, REVISION_NUMBER ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(), Set.of(),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of(),
+                        PART_INSTANCE_ID_00001.name(), Set.of(),
+                        REVISION_NUMBER_01.name(), Set.of()
+                  ),
                   Set.of( PRODUCT_CARBON_FOOTPRINTV_1_1_0 ) ) )
             .add( Arguments.of(
                   Set.of( MANUFACTURER_PART_ID_99991, CUSTOMER_PART_ID_ACME001, PART_INSTANCE_ID_00001, VERSION_NUMBER_01, REVISION_NUMBER_01 ),
                   BPNA,
-                  Set.of( MANUFACTURER_PART_ID, CUSTOMER_PART_ID, PART_INSTANCE_ID, REVISION_NUMBER, VERSION_NUMBER ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(),
+                        Set.of(),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of(),
+                        PART_INSTANCE_ID_00001.name(), Set.of(),
+                        VERSION_NUMBER_01.name(), Set.of(),
+                        REVISION_NUMBER_01.name(), Set.of()
+                  ),
                   Set.of( PRODUCT_CARBON_FOOTPRINTV_1_1_0, TRACEABILITYV_1_1_0 ) ) )
             .build();
    }
@@ -165,7 +184,7 @@ void testFetchVisibilityCriteriaForShellWhenNoMatchingBpnExpectException() {
    @MethodSource( "matchingSpecificAssetIdVisibilityProvider" )
    void testFetchVisibilityCriteriaForShellWhenMatchingSpecificAssetIdsProvidedExpectFilteringList(
          Set<SpecificAssetId> specificAssetIds, String bpn,
-         Set<String> expectedSpecificAssetIdNames, Set<String> expectedSemanticIds ) throws DenyAccessException {
+         Map<String, Set<String>> expectedSpecificAssetIdNames, Set<String> expectedSemanticIds ) throws DenyAccessException {
       ShellVisibilityContext shellContext = new ShellVisibilityContext( UUID.randomUUID().toString(), specificAssetIds );
 
       final var actual = underTest.fetchVisibilityCriteriaForShell( shellContext, bpn );

diff --git a/...main/java/org/eclipse/tractusx/semantics/registry/service/GranularShellAccessHandler.java b/...main/java/org/eclipse/tractusx/semantics/registry/service/GranularShellAccessHandler.java
@@ -23,6 +23,7 @@
 import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -151,7 +152,7 @@ private Shell filterShellContents( Shell shell, ShellVisibilityCriteria visibili
       Set<Submodel> filteredSubmodels = shell.getSubmodels().stream()
             .filter( submodel -> submodel.getSemanticId().getKeys().stream()
                   .anyMatch( key -> key.getType() == ReferenceKeyType.SUBMODEL
-                                    && visibilityCriteria.visibleSemanticIds().contains( key.getValue() ) ) )
+                        && visibilityCriteria.visibleSemanticIds().contains( key.getValue() ) ) )
             .collect( Collectors.toSet() );
       final Shell filtered;
       if ( visibilityCriteria.publicOnly() ) {
@@ -182,15 +183,19 @@ private ShellVisibilityContext toShellVisibilityContext( Shell shell ) {
    private Set<ShellIdentifier> filterSpecificAssetIdsByTenantId( Set<ShellIdentifier> shellIdentifiers, ShellVisibilityCriteria visibilityCriteria ) {
       //noinspection SimplifyStreamApiCallChains
       return shellIdentifiers.stream()
-            .filter( identifier -> identifier.getKey().equals( ShellIdentifier.GLOBAL_ASSET_ID_KEY )
-                                   || visibilityCriteria.visibleSpecificAssetIdNames().contains( identifier.getKey() ) )
+            .filter( identifier -> isSpecificAssetIdVisible( visibilityCriteria.visibleSpecificAssetIdNames(), identifier ) )
             //TODO: Do we need to clear the list of external subject Ids?
             .map( identifier -> {
                Optional.ofNullable( identifier.getExternalSubjectId() )
                      .ifPresent( extSubId -> extSubId.setKeys( Collections.emptySet() ) );
                return identifier;
             } )
             .collect( Collectors.toSet() );
+   }
 
+   private boolean isSpecificAssetIdVisible( Map<String, Set<String>> visibleSpecificAssetIdNames, ShellIdentifier identifier ) {
+      Set<String> identifierValues = visibleSpecificAssetIdNames.get( identifier.getKey() );
+      return identifier.getKey().equals( ShellIdentifier.GLOBAL_ASSET_ID_KEY ) ||
+            identifierValues != null && (identifierValues.isEmpty() || identifierValues.contains( identifier.getValue() ));
    }
 }
diff --git a/...java/org/eclipse/tractusx/semantics/registry/AssetAdministrationShellApiSecurityTest.java b/...java/org/eclipse/tractusx/semantics/registry/AssetAdministrationShellApiSecurityTest.java
@@ -24,6 +24,7 @@
 import static org.hamcrest.Matchers.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
 import java.util.UUID;
@@ -664,10 +665,6 @@ public void testGetAllShellsWithDefaultClosedFilteredSpecificAssetIdsByTenantId(
 
       @Test
       public void testGetShellWithFilteredSpecificAssetIdsByTenantId() throws Exception {
-         AssetAdministrationShellDescriptor shellPayload = TestUtil
-               .createCompleteAasDescriptor( keyPrefix + "semanticId", "http://example.com/" );
-         shellPayload.setId( keyPrefix );
-         shellPayload.setSpecificAssetIds( null );
          SpecificAssetId asset1 = TestUtil.createSpecificAssetId( keyPrefix + "CustomerPartId", "tenantTwoAssetIdValue",
                List.of( jwtTokenFactory.tenantTwo().getTenantId() ) );
          SpecificAssetId asset2 = TestUtil.createSpecificAssetId( keyPrefix + "CustomerPartId2", "tenantThreeAssetIdValue",
@@ -680,11 +677,22 @@ public void testGetShellWithFilteredSpecificAssetIdsByTenantId() throws Exceptio
          SpecificAssetId asset5 = TestUtil.createSpecificAssetId( "manufacturerPartId", keyPrefix + "wildcardAllowed",
                List.of( getExternalSubjectIdWildcardPrefix() ) );
 
-         shellPayload.setSpecificAssetIds( List.of( asset1, asset2, asset3, asset4, asset5 ) );
+         List<SpecificAssetId> specificAssetIds = List.of( asset1, asset2, asset3, asset4, asset5 );
+         List<SpecificAssetId> expectedSpecificAssetIdsTenantTwo = List.of( asset1, asset3, asset5 );
+         testGetShellWithFilteredSpecificAssetIdsByTenantId( specificAssetIds, expectedSpecificAssetIdsTenantTwo );
+      }
+
+      public void testGetShellWithFilteredSpecificAssetIdsByTenantId( List<SpecificAssetId> specificAssetIds, List<SpecificAssetId> expectedSpecificAssetIds )
+            throws Exception {
+         AssetAdministrationShellDescriptor shellPayload = TestUtil
+               .createCompleteAasDescriptor( keyPrefix + "semanticId", "http://example.com/" );
+         shellPayload.setId( keyPrefix );
+         shellPayload.setSpecificAssetIds( specificAssetIds );
          performShellCreateRequest( mapper.writeValueAsString( shellPayload ) );
 
          String shellId = shellPayload.getId();
          String encodedShellId = getEncodedValue( shellId );
+
          // Owner of tenant has access to all specificAssetIds
          mvc.perform(
                      MockMvcRequestBuilders
@@ -696,11 +704,13 @@ public void testGetShellWithFilteredSpecificAssetIdsByTenantId() throws Exceptio
                .andDo( MockMvcResultHandlers.print() )
                .andExpect( status().isOk() )
                .andExpect( jsonPath( "$.id", equalTo( shellId ) ) )
-               .andExpect( jsonPath( "$.specificAssetIds[*].value",
-                     containsInAnyOrder( "tenantTwoAssetIdValue", "tenantThreeAssetIdValue", "withoutTenantAssetIdValue", "ignoreWildcard",
-                           keyPrefix + "wildcardAllowed" ) ) );
+               .andExpect( jsonPath( "$.specificAssetIds[*].name", hasItems( specificAssetIds.stream().map( SpecificAssetId::getName ).toArray() ) ) )
+               .andExpect( jsonPath( "$.specificAssetIds[*].value", hasItems( specificAssetIds.stream().map( SpecificAssetId::getValue ).toArray() ) ) );
 
          // test with tenant two
+         ArrayList<SpecificAssetId> hiddenSpecificAssetIds = new ArrayList<>( specificAssetIds );
+         hiddenSpecificAssetIds.removeAll( expectedSpecificAssetIds );
+
          mvc.perform(
                      MockMvcRequestBuilders
                            .get( SINGLE_SHELL_BASE_PATH, encodedShellId )
@@ -711,10 +721,10 @@ public void testGetShellWithFilteredSpecificAssetIdsByTenantId() throws Exceptio
                .andDo( MockMvcResultHandlers.print() )
                .andExpect( status().isOk() )
                .andExpect( jsonPath( "$.id", equalTo( shellId ) ) )
-               .andExpect( jsonPath( "$.specificAssetIds[*].value",
-                     hasItems( "tenantTwoAssetIdValue", "withoutTenantAssetIdValue", keyPrefix + "wildcardAllowed" ) ) )
-               .andExpect( jsonPath( "$.specificAssetIds[*].value",
-                     not( hasItems( "tenantThreeAssetIdValue", "ignoreWildcard" ) ) ) );
+               .andExpect( jsonPath( "$.specificAssetIds[*].name", hasItems( expectedSpecificAssetIds.stream().map( SpecificAssetId::getName ).toArray() ) ) )
+               .andExpect( jsonPath( "$.specificAssetIds[*].value", hasItems( expectedSpecificAssetIds.stream().map( SpecificAssetId::getValue ).toArray() ) ) )
+               .andExpect(
+                     jsonPath( "$.specificAssetIds[*].value", not( hasItems( hiddenSpecificAssetIds.stream().map( SpecificAssetId::getValue ).toArray() ) ) ) );
       }
 
       @Test