Repository: OpenAEV-Platform/openaev (java)

Issue:
Can't create a new atomic test with an asset group automatically created from Crowdstrike
## Description

When I try to create a new atomic test with an asset group automatically created, the attribute asset_group_dynamic_filter is not filled by default so the red message "internal error" is shown and the atomic test is not created.

## Environment

1. OpenBAS version: 1.16.1

## Reproducible Steps

Steps to create the smallest reproducible scenario:
1. Enable the Crowdstrike Executor in your conf
2. Check if the host group from CS is created on OpenBAS asset groups screen
3. Create a new atomic test with a technical payload and the CS asset group
4. Error when we click on validate

## Expected Output

Atomic test is created.

## Actual Output

Atomic test is created.

## Additional information

Suggested fix:
Add a migration to set the attribute "asset_group_dynamic_filter" from the table asset_groups not null and with a default value (like when it's created manually with assets linked and no dynamic filter).



Discussion:
> Suggested fix:
Add a migration to set the attribute "asset_group_dynamic_filter" from the table asset_groups not null and with a default value (like when it's created manually with assets linked and no dynamic filter).

Note that this will impact future data model changes if we embed a specific data pattern as a db-side default, that is not aligned with how the FilterGroup class is shaped in the backend (whenever it evolves).

However this is an API facing type so we should catch that early and handle that properly within the contracts. Just make sure there's an automated test that would deserialise a newly created record with the default value set.

Patch:
diff --git a/openbas-api/src/main/java/io/openbas/migration/V3_88__Enforce_constraint_asset_group_dynamic_filter.java b/openbas-api/src/main/java/io/openbas/migration/V3_88__Enforce_constraint_asset_group_dynamic_filter.java
new file mode 100644
--- /dev/null
+++ b/openbas-api/src/main/java/io/openbas/migration/V3_88__Enforce_constraint_asset_group_dynamic_filter.java
@@ -0,0 +1,22 @@
+package io.openbas.migration;
+
+import java.sql.Statement;
+import org.flywaydb.core.api.migration.BaseJavaMigration;
+import org.flywaydb.core.api.migration.Context;
+import org.springframework.stereotype.Component;
+
+@Component
+public class V3_88__Enforce_constraint_asset_group_dynamic_filter extends BaseJavaMigration {
+
+  @Override
+  public void migrate(Context context) throws Exception {
+    try (Statement statement = context.getConnection().createStatement()) {
+      statement.execute(
+          """
+          ALTER TABLE asset_groups ALTER COLUMN asset_group_dynamic_filter SET DEFAULT '{"mode":"and","filters":[]}';
+          UPDATE asset_groups SET asset_group_dynamic_filter = '{"mode":"and","filters":[]}' where asset_group_dynamic_filter IS NULL;
+          ALTER TABLE asset_groups ALTER COLUMN asset_group_dynamic_filter SET NOT NULL;
+          """);
+    }
+  }
+}
diff --git a/openbas-api/src/main/java/io/openbas/rest/asset_group/form/AssetGroupInput.java b/openbas-api/src/main/java/io/openbas/rest/asset_group/form/AssetGroupInput.java
--- a/openbas-api/src/main/java/io/openbas/rest/asset_group/form/AssetGroupInput.java
+++ b/openbas-api/src/main/java/io/openbas/rest/asset_group/form/AssetGroupInput.java
@@ -25,5 +25,5 @@ public class AssetGroupInput {
   private List<String> tagIds = new ArrayList<>();
 
   @JsonProperty("asset_group_dynamic_filter")
-  private FilterGroup dynamicFilter;
+  private FilterGroup dynamicFilter = FilterGroup.defaultFilterGroup();
 }
diff --git a/openbas-model/src/main/java/io/openbas/database/model/AssetGroup.java b/openbas-model/src/main/java/io/openbas/database/model/AssetGroup.java
--- a/openbas-model/src/main/java/io/openbas/database/model/AssetGroup.java
+++ b/openbas-model/src/main/java/io/openbas/database/model/AssetGroup.java
@@ -62,7 +62,8 @@ public class AssetGroup implements Base {
   @Type(JsonType.class)
   @Column(name = "asset_group_dynamic_filter")
   @JsonProperty("asset_group_dynamic_filter")
-  private FilterGroup dynamicFilter;
+  @NotNull
+  private FilterGroup dynamicFilter = FilterGroup.defaultFilterGroup();
 
   @ArraySchema(schema = @Schema(type = "string"))
   @ManyToMany(fetch = FetchType.LAZY)
diff --git a/openbas-model/src/main/java/io/openbas/database/model/Filters.java b/openbas-model/src/main/java/io/openbas/database/model/Filters.java
--- a/openbas-model/src/main/java/io/openbas/database/model/Filters.java
+++ b/openbas-model/src/main/java/io/openbas/database/model/Filters.java
@@ -36,6 +36,19 @@ public static class FilterGroup {
     @NotNull private FilterMode mode; // Between filters
     private List<Filter> filters = new ArrayList<>();
 
+    /**
+     * asset_group_dynamic_filter in AssetGroup is now not null so we added a default not null value
+     * for the java object AssetGroup and the SQL table asset_groups Tests to protect this are
+     * implemented in AssetGroupApiTest Don't forget to update the default value for each case
+     * described above if needed !
+     */
+    public static FilterGroup defaultFilterGroup() {
+      FilterGroup filterGroup = new FilterGroup();
+      filterGroup.setMode(FilterMode.and);
+      filterGroup.setFilters(new ArrayList<>());
+      return filterGroup;
+    }
+
     // -- UTILS --
 
     public Optional<Filter> findByKey(@NotBlank final String filterKey) {


Test patch:
diff --git a/openbas-api/src/test/java/io/openbas/rest/asset_group/AssetGroupApiTest.java b/openbas-api/src/test/java/io/openbas/rest/asset_group/AssetGroupApiTest.java
--- a/openbas-api/src/test/java/io/openbas/rest/asset_group/AssetGroupApiTest.java
+++ b/openbas-api/src/test/java/io/openbas/rest/asset_group/AssetGroupApiTest.java
@@ -2,9 +2,9 @@
 
 import static io.openbas.rest.asset_group.AssetGroupApi.ASSET_GROUP_URI;
 import static io.openbas.utils.JsonUtils.asJsonString;
-import static io.openbas.utils.fixtures.AssetGroupFixture.createAssetGroupWithTags;
-import static io.openbas.utils.fixtures.AssetGroupFixture.createDefaultAssetGroup;
+import static io.openbas.utils.fixtures.AssetGroupFixture.*;
 import static io.openbas.utils.fixtures.InjectFixture.getDefaultInject;
+import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
@@ -21,6 +21,7 @@
 import io.openbas.utils.fixtures.ExerciseFixture;
 import io.openbas.utils.fixtures.TagFixture;
 import io.openbas.utils.mockUser.WithMockAdminUser;
+import jakarta.persistence.EntityManager;
 import jakarta.servlet.ServletException;
 import java.util.ArrayList;
 import java.util.List;
@@ -48,14 +49,128 @@ class AssetGroupApiTest extends IntegrationTest {
   @Autowired private TagRepository tagRepository;
   @Autowired private InjectRepository injectRepository;
   @Autowired private ExerciseService exerciseService;
+  @Autowired private EntityManager entityManager;
 
-  @DisplayName("Given valid AssetGroupInput, should create assetGroup successfully")
+  @DisplayName(
+      "Given valid AssetGroupInput, should create and get assetGroup without dynamic filter successfully")
+  @Test
+  @WithMockAdminUser
+  void given_validAssetGroupInput_should_createAndGetAssetGroupWithoutDynamicFilterSuccessfully()
+      throws Exception {
+    // -- PREPARE --
+    Tag tag = tagRepository.save(TagFixture.getTag());
+    AssetGroupInput assetGroupInput = createAssetGroupWithTags("Asset group", List.of(tag.getId()));
+    Filters.FilterGroup filterGroupExpected = Filters.FilterGroup.defaultFilterGroup();
+
+    // --EXECUTE--
+    String response =
+        mvc.perform(
+                post(ASSET_GROUP_URI)
+                    .content(asJsonString(assetGroupInput))
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .accept(MediaType.APPLICATION_JSON))
+            .andExpect(status().is2xxSuccessful())
+            .andReturn()
+            .getResponse()
+            .getContentAsString();
+
+    // --ASSERT--
+    assertThatJson(response).node("asset_group_name").isEqualTo(assetGroupInput.getName());
+    assertThatJson(response)
+        .node("asset_group_description")
+        .isEqualTo(assetGroupInput.getDescription());
+    // Check default because null in the input
+    assertThatJson(response).node("asset_group_dynamic_filter").isEqualTo(filterGroupExpected);
+    assertThatJson(response).node("asset_group_tags[0]").isEqualTo(tag.getId());
+
+    // --EXECUTE--
+    String response2 =
+        mvc.perform(
+                get(ASSET_GROUP_URI + "/" + JsonPath.read(response, "$.asset_group_id"))
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .accept(MediaType.APPLICATION_JSON))
+            .andExpect(status().is2xxSuccessful())
+            .andReturn()
+            .getResponse()
+            .getContentAsString();
+
+    // --ASSERT--
+    assertThatJson(response2).node("asset_group_name").isEqualTo(assetGroupInput.getName());
+    assertThatJson(response2)
+        .node("asset_group_description")
+        .isEqualTo(assetGroupInput.getDescription());
+    // Check default because null in the input
+    assertThatJson(response2).node("asset_group_dynamic_filter").isEqualTo(filterGroupExpected);
+    assertThatJson(response2).node("asset_group_tags[0]").isEqualTo(tag.getId());
+  }
+
+  @DisplayName(
+      "Given valid AssetGroupInput, should create and get assetGroup with dynamic filter successfully")
+  @Test
+  @WithMockAdminUser
+  void given_validAssetGroupInput_should_createAndGetAssetGroupWithDynamicFilterSuccessfully()
+      throws Exception {
+    // -- PREPARE --
+    Filters.FilterGroup dynamicFilter = new Filters.FilterGroup();
+    dynamicFilter.setMode(Filters.FilterMode.or);
+    Filters.Filter filter = new Filters.Filter();
+    filter.setKey("endpoint_platform");
+    filter.setMode(Filters.FilterMode.or);
+    filter.setOperator(Filters.FilterOperator.eq);
+    filter.setValues(List.of("Windows"));
+    dynamicFilter.setFilters(List.of(filter));
+    AssetGroupInput assetGroupInput =
+        createAssetGroupWithDynamicFilters("Asset group", dynamicFilter);
+
+    // --EXECUTE--
+    String response =
+        mvc.perform(
+                post(ASSET_GROUP_URI)
+                    .content(asJsonString(assetGroupInput))
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .accept(MediaType.APPLICATION_JSON))
+            .andExpect(status().is2xxSuccessful())
+            .andReturn()
+            .getResponse()
+            .getContentAsString();
+
+    // --ASSERT--
+    assertThatJson(response).node("asset_group_name").isEqualTo(assetGroupInput.getName());
+    assertThatJson(response)
+        .node("asset_group_description")
+        .isEqualTo(assetGroupInput.getDescription());
+    // Check default because null in the input
+    assertThatJson(response).node("asset_group_dynamic_filter").isEqualTo(dynamicFilter);
+
+    // --EXECUTE--
+    String response2 =
+        mvc.perform(
+                get(ASSET_GROUP_URI + "/" + JsonPath.read(response, "$.asset_group_id"))
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .accept(MediaType.APPLICATION_JSON))
+            .andExpect(status().is2xxSuccessful())
+            .andReturn()
+            .getResponse()
+            .getContentAsString();
+
+    // --ASSERT--
+    assertThatJson(response2).node("asset_group_name").isEqualTo(assetGroupInput.getName());
+    assertThatJson(response2)
+        .node("asset_group_description")
+        .isEqualTo(assetGroupInput.getDescription());
+    // Check default because null in the input
+    assertThatJson(response2).node("asset_group_dynamic_filter").isEqualTo(dynamicFilter);
+  }
+
+  @DisplayName(
+      "Create one asset group with Java and one with SQL, compare them to check the both asset_group_dynamic_filter are the same")
   @Test
   @WithMockAdminUser
-  void given_validAssetGroupInput_should_createAssetGroupSuccessfully() throws Exception {
+  void should_createOneAssetGroupWithJavaAndOneWithSQLAndCompareThem() throws Exception {
     // -- PREPARE --
     Tag tag = tagRepository.save(TagFixture.getTag());
     AssetGroupInput assetGroupInput = createAssetGroupWithTags("Asset group", List.of(tag.getId()));
+    Filters.FilterGroup filterGroupExpected = Filters.FilterGroup.defaultFilterGroup();
 
     // --EXECUTE--
     String response =
@@ -69,11 +184,21 @@ void given_validAssetGroupInput_should_createAssetGroupSuccessfully() throws Exc
             .getResponse()
             .getContentAsString();
 
+    entityManager
+        .createNativeQuery(
+            "INSERT INTO asset_groups (asset_group_id, asset_group_name) VALUES ('test_id', 'test_name')")
+        .executeUpdate();
+    Object resultSql =
+        entityManager
+            .createNativeQuery(
+                "SELECT asset_group_dynamic_filter FROM asset_groups WHERE asset_group_id = 'test_id'")
+            .getSingleResult();
+    entityManager.flush();
+    entityManager.clear();
+
     // --ASSERT--
-    assertEquals(assetGroupInput.getName(), JsonPath.read(response, "$.asset_group_name"));
-    assertEquals(
-        assetGroupInput.getDescription(), JsonPath.read(response, "$.asset_group_description"));
-    assertEquals(tag.getId(), JsonPath.read(response, "$.asset_group_tags[0]"));
+    // Check default because null in the input
+    assertThatJson(response).node("asset_group_dynamic_filter").isEqualTo(resultSql);
   }
 
   @DisplayName("Given valid AssetGroupInput, should update assetGroup successfully")
diff --git a/openbas-api/src/test/java/io/openbas/utils/fixtures/AssetGroupFixture.java b/openbas-api/src/test/java/io/openbas/utils/fixtures/AssetGroupFixture.java
--- a/openbas-api/src/test/java/io/openbas/utils/fixtures/AssetGroupFixture.java
+++ b/openbas-api/src/test/java/io/openbas/utils/fixtures/AssetGroupFixture.java
@@ -2,6 +2,7 @@
 
 import io.openbas.database.model.Asset;
 import io.openbas.database.model.AssetGroup;
+import io.openbas.database.model.Filters;
 import io.openbas.rest.asset_group.form.AssetGroupInput;
 import java.util.List;
 import org.jetbrains.annotations.NotNull;
@@ -24,6 +25,15 @@ public static AssetGroupInput createAssetGroupWithTags(
     return assetGroupInput;
   }
 
+  public static AssetGroupInput createAssetGroupWithDynamicFilters(
+      @NotNull final String name, @NotNull final Filters.FilterGroup dynamicFilter) {
+    AssetGroupInput assetGroupInput = new AssetGroupInput();
+    assetGroupInput.setName(name);
+    assetGroupInput.setDescription("An asset group");
+    assetGroupInput.setDynamicFilter(dynamicFilter);
+    return assetGroupInput;
+  }
+
   public static AssetGroup createAssetGroupWithAssets(
       @NotNull final String name, List<Asset> assets) {
     AssetGroup assetGroup = new AssetGroup();