[BUG] 🐛 $(userStorage) should not be accepted when us is null

docs/configuration.md

@@ -112,7 +112,7 @@ Sugoi-api implements spring security with full customization allowed
 | fr.insee.sugoi.api.regexp.role.admin                       |             |               |         |
 | fr.insee.sugoi.api.regexp.role.app.manager                 |             |               |         |
 | fr.insee.sugoi.api.regexp.role.password.manager            |             |               |         |
-| fr.insee.sugoi.api.enable.preauthorize                     |             |               |         |
+| fr.insee.sugoi.api.enable.preauthorize                     | Set to false to disable role validation on sugoi. Should not false on production. | true |         |
 
 #### Password configuration
 

sugoi-api-core/src/main/java/fr/insee/sugoi/core/service/impl/PermissionServiceImpl.java

@@ -191,6 +191,7 @@ private List<String> getSearchRoleList(
     }
     return regexpList.stream()
         .map(regexp -> StrSubstitutor.replace(regexp, valueMap, "$(", ")"))
+        .filter(role -> !role.matches(".*\\$\\(.*\\).*"))
         .collect(Collectors.toList());
   }
 

sugoi-api-core/src/test/java/fr/insee/sugoi/core/service/PermissionServiceTests.java

@@ -85,6 +85,19 @@ public void testUserStorageNull() {
     }
   }
 
+  @Test
+  public void testPatternAreRemoved() {
+    SugoiUser sugoiUser =
+        new SugoiUser(
+            "toto",
+            List.of("role_$reader_realm2_sugoi", "role_reader_realm1_$(userstorage)_sugoi"));
+    assertThat(
+        "User cannot read realm1 with fake pattern role",
+        !permissions.isReader(sugoiUser, "realm1", null));
+    assertThat(
+        "User can read realm2 even with $ symbol", permissions.isReader(sugoiUser, "realm2", null));
+  }
+
   @Test
   public void testAppManager() {
     try {
@@ -157,4 +170,25 @@ public void testgetAllowedAttributePattern() {
       fail();
     }
   }
+
+  @Test
+  public void testIsValidAttribute() {
+    SugoiUser sugoiUser =
+        new SugoiUser("toto", List.of("role_Asi_appli1", "role_reader_realm1_sugoi"));
+    String attributeValue = "toto_appli1";
+    String pattern_of_attribute = "(.*)_$(APPLICATION)";
+
+    assertThat(
+        "Should be able to write toto_appli1",
+        permissions.isValidAttributeAccordingAttributePattern(
+            sugoiUser, "domaine1", "storage", pattern_of_attribute, attributeValue));
+    assertThat(
+        "Should not be able to write toto_appli2",
+        !permissions.isValidAttributeAccordingAttributePattern(
+            sugoiUser, "domaine1", "storage", pattern_of_attribute, "toto_appli2"));
+    assertThat(
+        "Should not be able to write appli1_toto",
+        !permissions.isValidAttributeAccordingAttributePattern(
+            sugoiUser, "domaine1", "storage", pattern_of_attribute, "appli1_toto"));
+  }
 }

sugoi-api-core/src/test/resources/permissions/test-regexp-permissions.properties

@@ -1,5 +1,5 @@
 fr.insee.sugoi.api.regexp.role.writer=ROLE_WRITER_$(realm)_SUGOI
-fr.insee.sugoi.api.regexp.role.reader=ROLE_READER_$(realm)_SUGOI
+fr.insee.sugoi.api.regexp.role.reader=ROLE_READER_$(realm)_SUGOI,ROLE_$READER_$(realm)_SUGOI,ROLE_READER_$(realm)_$(userstorage)_SUGOI
 fr.insee.sugoi.api.regexp.role.admin=ROLE_ADMIN_SUGOI, ROLE_*_Admin
 
 

sugoi-api-rest-services/src/main/java/fr/insee/sugoi/services/decider/AuthorizeMethodDecider.java

@@ -31,7 +31,7 @@ public class AuthorizeMethodDecider {
 
   private static final Logger logger = LoggerFactory.getLogger(AuthorizeMethodDecider.class);
 
-  @Value("${fr.insee.sugoi.api.enable.preauthorize}")
+  @Value("${fr.insee.sugoi.api.enable.preauthorize:true}")
   private boolean enable;
 
   @Autowired PermissionService permissionService;

sugoi-api-rest-services/src/test/java/fr/insee/sugoi/services/controller/AppManagedUserAttributeControllerTest.java

@@ -15,20 +15,19 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import fr.insee.sugoi.commons.services.controller.technics.SugoiAdviceController;
 import fr.insee.sugoi.core.configuration.GlobalKeysConfig;
+import fr.insee.sugoi.core.model.ProviderRequest;
 import fr.insee.sugoi.core.model.ProviderResponse;
 import fr.insee.sugoi.core.model.ProviderResponse.ProviderResponseStatus;
 import fr.insee.sugoi.core.service.ConfigService;
-import fr.insee.sugoi.core.service.PermissionService;
 import fr.insee.sugoi.core.service.UserService;
+import fr.insee.sugoi.core.service.impl.PermissionServiceImpl;
 import fr.insee.sugoi.model.Realm;
 import fr.insee.sugoi.model.User;
-import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -45,7 +44,11 @@
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 
 @SpringBootTest(
-    classes = {AppManagedUserAttributeController.class, SugoiAdviceController.class},
+    classes = {
+      AppManagedUserAttributeController.class,
+      SugoiAdviceController.class,
+      PermissionServiceImpl.class
+    },
     properties = "spring.config.location=classpath:/permissions/test-regexp-permissions.properties")
 @AutoConfigureMockMvc
 @EnableWebMvc
@@ -54,8 +57,6 @@ public class AppManagedUserAttributeControllerTest {
 
   @MockBean private UserService userService;
 
-  @MockBean private PermissionService permissionService;
-
   @MockBean private ConfigService configService;
 
   ObjectMapper objectMapper = new ObjectMapper();
@@ -72,171 +73,101 @@ public void setup() {
 
   @Test
   @WithMockUser(username = "reader_realm1", roles = "ADMIN_SUGOI")
-  public void get200WhenAddCorrectAttributes() {
-    try {
-
-      Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
-      Mockito.when(permissionService.getUserRealmAppManager(Mockito.any()))
-          .thenReturn(List.of("*\\appA", "*\\appB"));
-      Mockito.doReturn(true)
-          .when(permissionService)
-          .isWriter(Mockito.any(), Mockito.anyString(), Mockito.anyString());
-      Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
-          .when(userService)
-          .addAppManagedAttribute(
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any());
-      RequestBuilder requestBuilder =
-          MockMvcRequestBuilders.patch(
-                  "/realms/domaine1/storages/test/users/Toto/my-attribute-key/prop_role_appA")
-              .contentType(MediaType.APPLICATION_JSON)
-              .accept(MediaType.APPLICATION_JSON)
-              .with(csrf());
-      MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
-
-      assertThat("Response must be 200 OK", response.getStatus(), is(204));
-
-    } catch (Exception e) {
-      e.printStackTrace();
-      fail();
-    }
-  }
-
-  @Test
-  @WithMockUser
-  public void get200WhenAddCorrectAttributesWhenAdminOrWriter() {
-    try {
-
-      Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
-      Mockito.doReturn(true)
-          .when(permissionService)
-          .isWriter(Mockito.any(), Mockito.anyString(), Mockito.anyString());
-      Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
-          .when(userService)
-          .addAppManagedAttribute(
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any());
-      RequestBuilder requestBuilder =
-          MockMvcRequestBuilders.patch(
-                  "/realms/domaine1/storages/test/users/Toto/my-attribute-key/prop_role_appA")
-              .contentType(MediaType.APPLICATION_JSON)
-              .accept(MediaType.APPLICATION_JSON)
-              .with(csrf());
-      MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
-
-      assertThat("Response must be 200 OK", response.getStatus(), is(204));
-
-    } catch (Exception e) {
-      e.printStackTrace();
-      fail();
-    }
+  public void get200WhenAddCorrectAttributes() throws Exception {
+
+    Mockito.when(configService.getRealm("test")).thenReturn(realm);
+    Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
+        .when(userService)
+        .addAppManagedAttribute(
+            Mockito.eq("test"),
+            Mockito.eq("test"),
+            Mockito.eq("Toto"),
+            Mockito.eq("my-attribute-key"),
+            Mockito.eq("prop_role_appA"),
+            Mockito.any(ProviderRequest.class));
+    RequestBuilder requestBuilder =
+        MockMvcRequestBuilders.patch(
+                "/realms/test/storages/test/users/Toto/my-attribute-key/prop_role_appA")
+            .contentType(MediaType.APPLICATION_JSON)
+            .accept(MediaType.APPLICATION_JSON)
+            .with(csrf());
+    MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
+
+    assertThat("Response must be 200 OK", response.getStatus(), is(204));
   }
 
   @Test
-  @WithMockUser(username = "reader_realm1", roles = "ASI_SUGOI")
-  public void get200WhenAddCorrectAttributesWhenAppManager() {
-    try {
-
-      Mockito.when(
-              permissionService.isWriter(Mockito.any(), Mockito.anyString(), Mockito.anyString()))
-          .thenReturn(false);
-      Mockito.doReturn(true)
-          .when(permissionService)
-          .isValidAttributeAccordingAttributePattern(
-              Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
-      Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
-          .when(userService)
-          .addAppManagedAttribute(
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any(),
-              Mockito.any());
-      Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
-      Mockito.when(
-              permissionService.getAllowedAttributePattern(
-                  Mockito.any(), Mockito.anyString(), Mockito.any(), Mockito.anyString()))
-          .thenReturn(List.of("(.*)_appA", "(.*)_appB"));
-      RequestBuilder requestBuilder =
-          MockMvcRequestBuilders.patch(
-                  "/realms/domaine1/storages/test/users/Toto/my-attribute-key/prop_role_appA")
-              .contentType(MediaType.APPLICATION_JSON)
-              .accept(MediaType.APPLICATION_JSON)
-              .with(csrf());
-      MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
-
-      assertThat("Response must be 200 OK", response.getStatus(), is(204));
-
-    } catch (Exception e) {
-      e.printStackTrace();
-      fail();
-    }
+  @WithMockUser(username = "reader_realm1", roles = "ASI_APPA")
+  public void get200WhenAddCorrectAttributesWhenAppManager() throws Exception {
+
+    Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
+        .when(userService)
+        .addAppManagedAttribute(
+            Mockito.eq("test"),
+            Mockito.eq("test"),
+            Mockito.eq("Toto"),
+            Mockito.eq("my-attribute-key"),
+            Mockito.eq("prop_role_appA"),
+            Mockito.any(ProviderRequest.class));
+    Mockito.when(configService.getRealm("test")).thenReturn(realm);
+    RequestBuilder requestBuilder =
+        MockMvcRequestBuilders.patch(
+                "/realms/test/storages/test/users/Toto/my-attribute-key/prop_role_appA")
+            .contentType(MediaType.APPLICATION_JSON)
+            .accept(MediaType.APPLICATION_JSON)
+            .with(csrf());
+    MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
+
+    assertThat("Response must be 200 OK", response.getStatus(), is(204));
   }
 
   @Test
-  @WithMockUser(username = "reader_realm1", roles = "ASI_SUGOI")
-  public void get403WhenAddIncorrectAttributes() {
-    try {
-
-      Mockito.when(
-              permissionService.isWriter(Mockito.any(), Mockito.anyString(), Mockito.anyString()))
-          .thenReturn(false);
-      Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
-      Mockito.when(
-              permissionService.getAllowedAttributePattern(
-                  Mockito.any(), Mockito.anyString(), Mockito.any(), Mockito.anyString()))
-          .thenReturn(List.of("(.*)_appA", "(.*)_appB"));
-      RequestBuilder requestBuilder =
-          MockMvcRequestBuilders.patch(
-                  "/realms/domaine1/storages/test/users/Toto/my-attribute-key2/prop_role_appA")
-              .contentType(MediaType.APPLICATION_JSON)
-              .accept(MediaType.APPLICATION_JSON)
-              .with(csrf());
-      MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
-
-      assertThat("Response must be 403", response.getStatus(), is(403));
-
-    } catch (Exception e) {
-      e.printStackTrace();
-      fail();
-    }
+  @WithMockUser(username = "reader_realm1", roles = "ASI_APPA")
+  public void get403WhenAddIncorrectAttributes() throws Exception {
+
+    Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
+        .when(userService)
+        .addAppManagedAttribute(
+            Mockito.eq("test"),
+            Mockito.eq("test"),
+            Mockito.eq("Toto"),
+            Mockito.eq("my-attribute-key2"),
+            Mockito.eq("prop_role_appA"),
+            Mockito.any(ProviderRequest.class));
+    Mockito.when(configService.getRealm("test")).thenReturn(realm);
+    RequestBuilder requestBuilder =
+        MockMvcRequestBuilders.patch(
+                "/realms/test/storages/test/users/Toto/my-attribute-key2/prop_role_appA")
+            .contentType(MediaType.APPLICATION_JSON)
+            .accept(MediaType.APPLICATION_JSON)
+            .with(csrf());
+    MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
+
+    assertThat("Response must be 403", response.getStatus(), is(403));
   }
 
   @Test
   @WithMockUser(username = "reader_realm1", roles = "ASI_SUGOI")
-  public void get403WhenNoRightIncorrectAttributes() {
-    try {
-
-      Mockito.when(
-              permissionService.isWriter(Mockito.any(), Mockito.anyString(), Mockito.anyString()))
-          .thenReturn(false);
-      Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
-      Mockito.when(
-              permissionService.getAllowedAttributePattern(
-                  Mockito.any(), Mockito.anyString(), Mockito.any(), Mockito.anyString()))
-          .thenReturn(List.of("(.*)_sugoi"));
-      RequestBuilder requestBuilder =
-          MockMvcRequestBuilders.patch(
-                  "/realms/domaine1/storages/test/users/Toto/my-attribute-key/prop_role_appA")
-              .contentType(MediaType.APPLICATION_JSON)
-              .accept(MediaType.APPLICATION_JSON)
-              .with(csrf());
-      MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
-
-      assertThat("Response must be 403", response.getStatus(), is(403));
-
-    } catch (Exception e) {
-      e.printStackTrace();
-      fail();
-    }
+  public void get403WhenNoRightIncorrectAttributes() throws Exception {
+
+    Mockito.doReturn(new ProviderResponse("", "requestId", ProviderResponseStatus.OK, null, null))
+        .when(userService)
+        .addAppManagedAttribute(
+            Mockito.eq("test"),
+            Mockito.eq("test"),
+            Mockito.eq("Toto"),
+            Mockito.eq("my-attribute-key"),
+            Mockito.eq("prop_role_appA"),
+            Mockito.any(ProviderRequest.class));
+    Mockito.when(configService.getRealm(Mockito.anyString())).thenReturn(realm);
+    RequestBuilder requestBuilder =
+        MockMvcRequestBuilders.patch(
+                "/realms/test/storages/test/users/Toto/my-attribute-key/prop_role_appA")
+            .contentType(MediaType.APPLICATION_JSON)
+            .accept(MediaType.APPLICATION_JSON)
+            .with(csrf());
+    MockHttpServletResponse response = mockMvc.perform(requestBuilder).andReturn().getResponse();
+
+    assertThat("Response must be 403", response.getStatus(), is(403));
   }
 }