Merge pull request #359 from bci-oss/bugfix/access-management-control…

…-visible-multiple-key BugFix: Fixed behaviour of access rule management
eclipse-tractusx · Apr 4, 2024 · 2dc4609 · 2dc4609
2 parents cf253c4 + 077da86
commit 2dc4609
Show file tree

Hide file tree

Showing 11 changed files with 379 additions and 145 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added lookup api test in aas-registry-e2e-test action
 ## 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/DEPENDENCIES b/DEPENDENCIES
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,12 @@
 
 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 Set<String> visibleSpecificAssetIdNamesRegardlessOfValues,
+      @NonNull Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues, @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;
@@ -35,32 +36,38 @@ class ShellVisibilityCriteriaTest {
 
    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, null, Set.of(), false ) )
+            .add( Arguments.of( null, null, null, null, true ) )
+            .add( Arguments.of( UUID.randomUUID().toString(), null, null, null, true ) )
+            .add( Arguments.of( null, Set.of(), null, null, false ) )
+            .add( Arguments.of( null, null, Map.of(), null, false ) )
+            .add( Arguments.of( null, null, null, Set.of(), false ) )
             .build();
    }
 
    @ParameterizedTest
    @MethodSource( "nullProvider" )
    void testConstructorCalledWithNullExpectException(
-         String aasId, Set<String> visibleSpecificAssetIdNames, Set<String> visibleSemanticIds, boolean publicOnly ) {
-      assertThatThrownBy( () -> new ShellVisibilityCriteria( aasId, visibleSpecificAssetIdNames, visibleSemanticIds, publicOnly ) )
+         String aasId, Set<String> visibleSpecificAssetIdNamesRegardlessOfValues, Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues,
+         Set<String> visibleSemanticIds, boolean publicOnly ) {
+      assertThatThrownBy( () -> new ShellVisibilityCriteria( aasId, visibleSpecificAssetIdNamesRegardlessOfValues, visibleSpecificAssetIdWhenMatchingValues,
+            visibleSemanticIds, publicOnly ) )
             .isExactlyInstanceOf( NullPointerException.class );
    }
 
    @Test
    void testConstructorCalledWithValidDataExpectSuccess() {
       final String aasId = UUID.randomUUID().toString();
-      final Set<String> visibleSpecificAssetIdNames = Set.of( "name1" );
+
+      final Set<String> visibleSpecificAssetIdNamesRegardlessOfValues = Set.of();
+      final Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues = Map.of( "name1", Set.of( "" ) );
       final Set<String> visibleSemanticIds = Set.of( "name2" );
       final boolean publicOnly = true;
 
-      ShellVisibilityCriteria actual = new ShellVisibilityCriteria( aasId, visibleSpecificAssetIdNames, visibleSemanticIds, publicOnly );
+      ShellVisibilityCriteria actual = new ShellVisibilityCriteria( aasId, visibleSpecificAssetIdNamesRegardlessOfValues,
+            visibleSpecificAssetIdWhenMatchingValues, visibleSemanticIds, publicOnly );
 
       assertThat( actual.aasId() ).isEqualTo( aasId );
-      assertThat( actual.visibleSpecificAssetIdNames() ).isEqualTo( visibleSpecificAssetIdNames );
+      assertThat( actual.visibleSpecificAssetIdWhenMatchingValues() ).isEqualTo( visibleSpecificAssetIdWhenMatchingValues );
       assertThat( actual.visibleSemanticIds() ).isEqualTo( visibleSemanticIds );
       assertThat( actual.publicOnly() ).isEqualTo( publicOnly );
    }

diff --git a/...lipse/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleService.java b/...lipse/tractusx/semantics/accesscontrol/sql/service/SqlBackedAccessControlRuleService.java
@@ -61,13 +61,22 @@ public List<String> filterValidSpecificAssetIdsForLookup(
       Set<AccessRulePolicy> allAccessControlRulesForBpn = findPotentiallyMatchingAccessControlRules( bpn ).collect( Collectors.toSet() );
       return shellContext.stream()
             .filter( aShellContext -> {
-               Set<String> visibleSpecificAssetIdNames = allAccessControlRulesForBpn.stream()
-                     .filter( accessControlRule -> aShellContext.specificAssetIds().containsAll( accessControlRule.getMandatorySpecificAssetIds() ) )
-                     .flatMap( accessControlRule -> accessControlRule.getVisibleSpecificAssetIdNames().stream() )
+               Set<String> visibleSpecificAssetIdNamesRegardlessOfValues = allAccessControlRulesForBpn.stream()
+                     .filter( accessControlRule -> aShellContext.specificAssetIds().containsAll(
+                           accessControlRule.getMandatorySpecificAssetIds() ) )
+                     .flatMap( this::keepVisibleSpecificAssetIdNamesWithoutValueRestrictions )
                      .collect( Collectors.toSet() );
+
+               Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues = allAccessControlRulesForBpn.stream()
+                     .filter( accessControlRule -> aShellContext.specificAssetIds().containsAll(
+                           accessControlRule.getMandatorySpecificAssetIds() ) )
+                     .flatMap( this::keepVisibleSpecificAssetIdNamesWithValueRestrictions )
+                     .collect( Collectors.groupingBy( SpecificAssetId::name, Collectors.mapping( SpecificAssetId::value, Collectors.toSet() ) ) );
+
                return aShellContext.specificAssetIds().stream()
-                     .filter( id -> visibleSpecificAssetIdNames.contains( id.name() ) )
-                     .collect( Collectors.toSet() ).containsAll( userQuery );
+                     .filter( id -> isSpecificAssetIdVisible( id, visibleSpecificAssetIdNamesRegardlessOfValues, visibleSpecificAssetIdWhenMatchingValues ) )
+                     .collect( Collectors.toSet() )
+                     .containsAll( userQuery );
             } )
             .map( ShellVisibilityContext::aasId )
             .toList();
@@ -76,15 +85,21 @@ 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() )
+      Set<String> visibleSpecificAssetIdNamesRegardlessOfValues = matchingAccessControlRules.stream()
+            .flatMap( this::keepVisibleSpecificAssetIdNamesWithoutValueRestrictions )
             .collect( Collectors.toSet() );
+
+      Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues = matchingAccessControlRules.stream()
+            .flatMap( this::keepVisibleSpecificAssetIdNamesWithValueRestrictions )
+            .collect( Collectors.groupingBy( SpecificAssetId::name, Collectors.mapping( SpecificAssetId::value, Collectors.toSet() ) ) );
+
       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(), visibleSpecificAssetIdNamesRegardlessOfValues, visibleSpecificAssetIdWhenMatchingValues,
+            visibleSemanticIds, publicOnly );
    }
 
    @Override
@@ -123,4 +138,20 @@ private Set<AccessRulePolicy> findMatchingAccessControlRules( ShellVisibilityCon
       }
       return matching;
    }
+
+   private boolean isSpecificAssetIdVisible( SpecificAssetId specificAssetId, Set<String> visibleSpecificAssetIdNamesRegardlessOfValues,
+         Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues ) {
+      return visibleSpecificAssetIdNamesRegardlessOfValues.contains( specificAssetId.name() )
+            || visibleSpecificAssetIdWhenMatchingValues.getOrDefault( specificAssetId.name(), Set.of() ).contains( specificAssetId.value() );
+   }
+
+   private Stream<String> keepVisibleSpecificAssetIdNamesWithoutValueRestrictions( AccessRulePolicy accessRulePolicy ) {
+      return accessRulePolicy.getVisibleSpecificAssetIdNames().stream()
+            .filter( name -> accessRulePolicy.getMandatorySpecificAssetIds().stream().map( SpecificAssetId::name ).noneMatch( name::equals ) );
+   }
+
+   private Stream<SpecificAssetId> keepVisibleSpecificAssetIdNamesWithValueRestrictions( AccessRulePolicy accessRulePolicy ) {
+      return accessRulePolicy.getMandatorySpecificAssetIds().stream()
+            .filter( mandatory -> accessRulePolicy.getVisibleSpecificAssetIdNames().contains( mandatory.name() ) );
+   }
 }
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,36 @@ 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 ),
+                  Set.of( VERSION_NUMBER ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(), Set.of( MANUFACTURER_PART_ID_99991.value() ),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of( CUSTOMER_PART_ID_ACME001.value() ),
+                        PART_INSTANCE_ID_00001.name(), Set.of( PART_INSTANCE_ID_00001.value() )
+                  ),
                   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 ),
+                  Set.of( PART_INSTANCE_ID ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(), Set.of( MANUFACTURER_PART_ID_99991.value() ),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of( CUSTOMER_PART_ID_ACME001.value() ),
+                        REVISION_NUMBER_01.name(), Set.of( REVISION_NUMBER_01.value() )
+                  ),
                   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 ),
+                  Set.of( PART_INSTANCE_ID, VERSION_NUMBER ),
+                  Map.of(
+                        MANUFACTURER_PART_ID_99991.name(),
+                        Set.of( MANUFACTURER_PART_ID_99991.value() ),
+                        CUSTOMER_PART_ID_ACME001.name(), Set.of( CUSTOMER_PART_ID_ACME001.value() ),
+                        PART_INSTANCE_ID_00001.name(), Set.of( PART_INSTANCE_ID_00001.value() ),
+                        REVISION_NUMBER_01.name(), Set.of( REVISION_NUMBER_01.value() )
+                  ),
                   Set.of( PRODUCT_CARBON_FOOTPRINTV_1_1_0, TRACEABILITYV_1_1_0 ) ) )
             .build();
    }
@@ -164,13 +182,14 @@ void testFetchVisibilityCriteriaForShellWhenNoMatchingBpnExpectException() {
    @ParameterizedTest
    @MethodSource( "matchingSpecificAssetIdVisibilityProvider" )
    void testFetchVisibilityCriteriaForShellWhenMatchingSpecificAssetIdsProvidedExpectFilteringList(
-         Set<SpecificAssetId> specificAssetIds, String bpn,
-         Set<String> expectedSpecificAssetIdNames, Set<String> expectedSemanticIds ) throws DenyAccessException {
+         Set<SpecificAssetId> specificAssetIds, String bpn, Set<String> expectedSpecificAssetIdNamesRegardlessOfValues,
+         Map<String, Set<String>> expectedSpecificAssetIdWhenMatchingValues, Set<String> expectedSemanticIds ) throws DenyAccessException {
       ShellVisibilityContext shellContext = new ShellVisibilityContext( UUID.randomUUID().toString(), specificAssetIds );
 
       final var actual = underTest.fetchVisibilityCriteriaForShell( shellContext, bpn );
 
       assertThat( actual.visibleSemanticIds() ).isEqualTo( expectedSemanticIds );
-      assertThat( actual.visibleSpecificAssetIdNames() ).isEqualTo( expectedSpecificAssetIdNames );
+      assertThat( actual.visibleSpecificAssetIdNamesRegardlessOfValues() ).isEqualTo( expectedSpecificAssetIdNamesRegardlessOfValues );
+      assertThat( actual.visibleSpecificAssetIdWhenMatchingValues() ).isEqualTo( expectedSpecificAssetIdWhenMatchingValues );
    }
 }
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,21 @@ 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 -> isSisSpecificAssetIdVisible( identifier, visibilityCriteria ) )
             //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 isSisSpecificAssetIdVisible( ShellIdentifier identifier, ShellVisibilityCriteria shellVisibilityCriteria ) {
+      Set<String> visibleSpecificAssetIdNamesRegardlessOfValues = shellVisibilityCriteria.visibleSpecificAssetIdNamesRegardlessOfValues();
+      Map<String, Set<String>> visibleSpecificAssetIdWhenMatchingValues = shellVisibilityCriteria.visibleSpecificAssetIdWhenMatchingValues();
+      return identifier.getKey().equals( ShellIdentifier.GLOBAL_ASSET_ID_KEY )
+            || visibleSpecificAssetIdNamesRegardlessOfValues.contains( identifier.getKey() )
+            || visibleSpecificAssetIdWhenMatchingValues.getOrDefault( identifier.getKey(), Set.of() ).contains( identifier.getValue() );
    }
 }