[MRG] Merge pull request #603 from dfir-iris/api_v2_regression_fixes

Api v2 regression fixes

.github/workflows/ci.yml

@@ -55,18 +55,21 @@ jobs:
           python -m venv venv
           source venv/bin/activate
           pip install -r requirements.txt
-          docker compose --file ../docker-compose.dev.yml --env-file data/basic.env up --detach
+          docker compose --file ../docker-compose.dev.yml --env-file data/basic.env up --detach --wait
           PYTHONUNBUFFERED=true python -m unittest --verbose
           docker compose down
       - name: Start development server
         run: |
           docker compose --file docker-compose.dev.yml up --detach
+      - name: Install e2e dependencies
+        working-directory: e2e
+        run: npm ci
+      - name: Install playwright dependencies
+        working-directory: e2e
+        run: npx playwright install chromium firefox
       - name: Run end to end tests
         working-directory: e2e
-        run: |
-          npm ci
-          npx playwright install --with-deps
-          npx playwright test
+        run: npx playwright test
       - name: Generate GraphQL documentation
         run: |
           npx spectaql@^3.0.2 source/spectaql/config.yml

docker-compose.dev.yml

@@ -45,6 +45,9 @@ services:
     volumes:
       - ./source/app:/iriswebapp/app
       - ./ui/dist:/iriswebapp/static
+    healthcheck:
+      test: curl --head --fail http://localhost:8000 || exit 1
+      start_period: 60s
 
   worker:
     extends:
@@ -79,4 +82,4 @@ networks:
   iris_backend:
     name: iris_backend
   iris_frontend:
-    name: iris_frontend
+    name: iris_frontend

e2e/tests/administrator/case/assets.spec.js

@@ -0,0 +1,25 @@
+import { test, expect } from '@playwright/test';
+import crypto from 'node:crypto';
+
+test.beforeEach(async({ page }) => {
+    await page.goto('/case/assets?cid=1');
+});
+
+test('should not be able to create an asset with the same type and value', async ({ page }) => {
+    const assetValue = `Asset value - ${crypto.randomUUID()}`;
+
+    await page.getByRole('button', { name: 'Add assets' }).click();
+    await page.getByRole('button', { name: 'None' }).click();
+    await page.getByRole('listbox').getByRole('option', { name: 'Account', exact: true }).click();
+    await page.getByPlaceholder('One asset per line').fill(assetValue);
+    await page.getByRole('button', { name: 'Save' }).click();
+
+    await page.getByRole('button', { name: 'Add assets' }).click();
+    await page.getByRole('button', { name: 'None' }).click();
+    await page.getByRole('listbox').getByRole('option', { name: 'Account', exact: true }).click();
+    await page.getByPlaceholder('One asset per line').fill(assetValue);
+    await page.getByRole('button', { name: 'Save' }).click();
+
+    await expect(page.getByText('Asset with same value and type already exists')).toBeVisible();
+    await expect(page.getByRole('button', { name: 'Save' })).toBeVisible();
+});

e2e/tests/administrator/case/ioc.spec.js

@@ -1,4 +1,5 @@
-import { test, expect } from '@playwright/test';
+import { test } from '../../restFixture.js';
+import { expect } from '@playwright/test';
 import crypto from 'node:crypto';
 
 test.beforeEach(async({ page }) => {
@@ -40,3 +41,32 @@ test('should not be able to create an IOC with the same type and value', async (
     await expect(page.getByText('IOC with same value and type already exists')).toBeVisible();
     await expect(page.getByRole('button', { name: 'Save' })).toBeVisible();
 });
+
+test('should paginate the IOCs', async ({ page, rest }) => {
+    const caseName = `Case - ${crypto.randomUUID()}`;
+
+    // TODO maybe should remove cases between each tests (like in the backend tests)
+    let response = await rest.post('/api/v2/cases', {
+        data: {
+            case_name: caseName,
+            case_description: 'Case description',
+            case_customer: 1,
+            case_soc_id: ''
+        }
+    });
+    const caseIdentifier = (await response.json()).case_id;
+    for (let i = 0; i < 11; i++) {
+        await rest.post(`/api/v2/cases/${caseIdentifier}/iocs`, {
+            data: {
+                ioc_type_id: 1,
+                ioc_value: `IOC value - ${crypto.randomUUID()}`,
+                ioc_tlp_id: 2,
+                ioc_description: 'rewrw',
+                ioc_tags: ''
+            }
+        })
+    }
+
+    await page.goto(`/case/ioc?cid=${caseIdentifier}`);
+    await expect(page.getByRole('link', { name: '2', exact: true })).toBeVisible();
+});

e2e/tests/auth.setup.js

@@ -1,29 +1,19 @@
-import { test as setup, expect } from '@playwright/test';
-import dotenv from 'dotenv';
+import { test as setup } from './restFixture.js';
+import { expect } from '@playwright/test';
 import fs from 'node:fs';
-
-const _API_URL = 'http://127.0.0.1:8000';
+import dotenv from 'dotenv';
 
 const _PERMISSION_CUSTOMERS_READ = 0x40;
 
 const _ADMINISTRATOR_USERNAME = 'administrator';
 
-let apiContext;
 let administrator_password;
 
 setup.beforeAll(async ({ playwright }) => {
     const envFile = fs.readFileSync('../.env');
     const env = dotenv.parse(envFile);
 
-    administrator_password = env.IRIS_ADM_PASSWORD
-
-    apiContext = await playwright.request.newContext({
-        baseURL: _API_URL,
-        extraHTTPHeaders: {
-            'Authorization': `Bearer ${env.IRIS_ADM_API_KEY}`,
-            'Content-Type': 'application/json'
-        },
-    });
+    administrator_password = env.IRIS_ADM_PASSWORD;
 });
 
 async function authenticate(page, login, password) {
@@ -42,14 +32,14 @@ setup('authenticate as administrator', async ({ page }) => {
     await authenticate(page, _ADMINISTRATOR_USERNAME, administrator_password);
 });
 
-setup('authenticate as user with customers read rights', async ({ page }) => {
+setup('authenticate as user with customers read rights', async ({ page, rest }) => {
     // TODO when this method is called a second time, all these request will fail
     //      think about a better ways of doing things, some possible strategies
     //      - find a way to create a new valid database before and empty the database after
     //      - find a way to remove elements from the database to roughly get back to the initial state
     //      - code so that these requests are robust (check the group exists, user exists, link between the two is set...)
     //      - global setup and teardown? https://playwright.dev/docs/test-global-setup-teardown
-    let response = await apiContext.post('/manage/groups/add', {
+    let response = await rest.post('/manage/groups/add', {
         data: {
             group_name: 'group_customers_r',
             group_description: 'Group with rights: customers_read',
@@ -59,7 +49,7 @@ setup('authenticate as user with customers read rights', async ({ page }) => {
     const groupIdentifier = (await response.json()).data.group_id;
     const login = 'user_customers_r';
     const password = 'aA.1234567890';
-    response = await apiContext.post('/manage/users/add', {
+    response = await rest.post('/manage/users/add', {
         data: {
             user_name: login,
             user_login: login,
@@ -68,7 +58,7 @@ setup('authenticate as user with customers read rights', async ({ page }) => {
         }
     });
     const userIdentifier = (await response.json()).data.id;
-    response = await apiContext.post(`/manage/users/${userIdentifier}/groups/update`, {
+    response = await rest.post(`/manage/users/${userIdentifier}/groups/update`, {
         data: {
             groups_membership: [groupIdentifier]
         }
@@ -77,8 +67,3 @@ setup('authenticate as user with customers read rights', async ({ page }) => {
     await authenticate(page, login, password);
 });
 
-setup.afterAll(async ({ }) => {
-  // Dispose all responses.
-  await apiContext.dispose();
-});
-

e2e/tests/restFixture.js

@@ -0,0 +1,27 @@
+import { test as base } from '@playwright/test';
+import fs from 'node:fs';
+import dotenv from 'dotenv';
+
+const _API_URL = 'http://127.0.0.1:8000';
+
+export const test = base.extend({
+  rest: async ({ playwright }, use) => {
+    // Set up the fixture.
+    const envFile = fs.readFileSync('../.env');
+    const env = dotenv.parse(envFile);
+
+    const apiContext = await playwright.request.newContext({
+        baseURL: _API_URL,
+        extraHTTPHeaders: {
+            'Authorization': `Bearer ${env.IRIS_ADM_API_KEY}`,
+            'Content-Type': 'application/json'
+        },
+    });
+
+    // Use the fixture value in the test.
+    await use(apiContext);
+
+    // Clean up the fixture.
+    await apiContext.dispose();
+  },
+});

source/app/blueprints/rest/case/case_assets_routes.py

@@ -35,6 +35,7 @@
 from app.business.assets import assets_get_detailed
 from app.business.assets import assets_get
 from app.business.errors import BusinessProcessingError
+from app.datamgmt.case.case_assets_db import case_assets_db_exists
 from app.datamgmt.case.case_assets_db import add_comment_to_asset
 from app.datamgmt.case.case_assets_db import create_asset
 from app.datamgmt.case.case_assets_db import delete_asset_comment
@@ -143,7 +144,6 @@ def add_asset(identifier):
 
     asset_schema = CaseAssetsSchema()
     try:
-        asset_schema.is_unique_for_cid(identifier, request.get_json())
         _, asset = assets_create(identifier, request.get_json())
         return response_api_created(asset_schema.dump(asset))
     except BusinessProcessingError as e:
@@ -216,10 +216,13 @@ def case_upload_ioc(caseid):
 
             row['analysis_status_id'] = analysis_status_id
 
+            # TODO from here
             request_data = call_modules_hook('on_preload_asset_create', data=row, caseid=caseid)
 
-            add_asset_schema.is_unique_for_cid(caseid, request_data)
             asset_sc = add_asset_schema.load(request_data)
+            asset_sc.case_id = caseid
+            if case_assets_db_exists(asset_sc):
+                return response_error('Data error', data='Asset with same value and type already exists')
             asset_sc.custom_attributes = get_default_custom_attributes('asset')
             asset = create_asset(asset=asset_sc,
                                  caseid=caseid,
@@ -229,12 +232,14 @@ def case_upload_ioc(caseid):
             asset = call_modules_hook('on_postload_asset_create', data=asset, caseid=caseid)
 
             if not asset:
-                errors.append("Unable to add asset for internal reason")
+                errors.append('Unable to add asset for internal reason')
                 index += 1
                 continue
 
+            # to here: should call assets_create from the business layer.
+            #          But should the custom_attributes always be called?
+            track_activity(f'added asset {asset.asset_name}', caseid=caseid)
             ret.append(request_data)
-            track_activity(f"added asset {asset.asset_name}", caseid=caseid)
 
             index += 1
 
@@ -246,7 +251,7 @@ def case_upload_ioc(caseid):
         return response_success(msg=msg, data=ret)
 
     except marshmallow.exceptions.ValidationError as e:
-        return response_error(msg="Data error", data=e.messages)
+        return response_error(msg='Data error', data=e.messages)
 
 
 @case_assets_rest_blueprint.route('/case/assets/<int:cur_id>', methods=['GET'])
@@ -292,8 +297,10 @@ def asset_update(cur_id, caseid):
 
         request_data['asset_id'] = cur_id
 
-        add_asset_schema.is_unique_for_cid(caseid, request_data)
         asset_schema = add_asset_schema.load(request_data, instance=asset)
+        asset_schema.case_id = caseid
+        if case_assets_db_exists(asset_schema):
+            return response_error('Data error', data='Asset with same value and type already exists')
 
         update_assets_state(caseid=caseid)
         db.session.commit()
@@ -313,7 +320,7 @@ def asset_update(cur_id, caseid):
         return response_error("Unable to update asset for internal reasons")
 
     except marshmallow.exceptions.ValidationError as e:
-        return response_error(msg="Data error", data=e.messages)
+        return response_error(msg='Data error', data=e.messages)
 
 
 @case_assets_rest_blueprint.route('/case/assets/delete/<int:cur_id>', methods=['POST'])

source/app/blueprints/rest/case/case_ioc_routes.py

@@ -154,7 +154,7 @@ def case_ioc_state(caseid):
 
 
 @case_ioc_rest_blueprint.route('/case/ioc/add', methods=['POST'])
-@endpoint_deprecated('POST', '/api/v2/cases/{identifier}/iocs')
+@endpoint_deprecated('POST', '/api/v2/cases/<int:identifier>/iocs')
 @ac_requires_case_identifier(CaseAccessLevel.full_access)
 @ac_api_requires()
 def deprecated_case_add_ioc(caseid):

source/app/blueprints/rest/case/case_tasks_routes.py

@@ -194,7 +194,7 @@ def deprecated_case_delete_task(cur_id, caseid):
 def case_delete_task(identifier):
     try:
         task = tasks_get(identifier)
-        if not ac_fast_check_current_user_has_case_access(identifier, [CaseAccessLevel.full_access]):
+        if not ac_fast_check_current_user_has_case_access(task.task_case_id, [CaseAccessLevel.full_access]):
             return ac_api_return_access_denied(caseid=identifier)
 
         tasks_delete(task)

source/app/business/assets.py

@@ -20,39 +20,46 @@
 from marshmallow.exceptions import ValidationError
 
 from app.business.errors import BusinessProcessingError
+from app.models import CaseAssets
 from app.datamgmt.case.case_assets_db import get_asset
+from app.datamgmt.case.case_assets_db import case_assets_db_exists
 from app.datamgmt.case.case_assets_db import create_asset
 from app.datamgmt.case.case_assets_db import set_ioc_links
 from app.datamgmt.case.case_assets_db import get_linked_iocs_finfo_from_asset
 from app.datamgmt.case.case_assets_db import delete_asset
 from app.iris_engine.module_handler.module_handler import call_modules_hook
 from app.iris_engine.utils.tracker import track_activity
-from app.models import CaseAssets
 from app.schema.marshables import CaseAssetsSchema
 
 
-def _load(request_data):
+def _load(case_identifier, request_data):
     try:
         add_assets_schema = CaseAssetsSchema()
-        return add_assets_schema.load(request_data)
+        asset = add_assets_schema.load(request_data)
+        asset.case_id = case_identifier
+        return asset
     except ValidationError as e:
         raise BusinessProcessingError('Data error', e.messages)
 
 
 def assets_create(case_identifier, request_json):
     request_data = call_modules_hook('on_preload_asset_create', data=request_json, caseid=case_identifier)
-    asset = _load(request_data)
+    asset = _load(case_identifier, request_data)
+
+    if case_assets_db_exists(asset):
+        raise BusinessProcessingError('Asset with same value and type already exists')
     asset = create_asset(asset=asset, caseid=case_identifier, user_id=current_user.id)
+    # TODO should the custom attributes be set?
     if request_data.get('ioc_links'):
         errors, _ = set_ioc_links(request_data.get('ioc_links'), asset.asset_id)
         if errors:
             raise BusinessProcessingError('Encountered errors while linking IOC. Asset has still been updated.')
     asset = call_modules_hook('on_postload_asset_create', data=asset, caseid=case_identifier)
     if asset:
-        track_activity(f"added asset \"{asset.asset_name}\"", caseid=case_identifier)
+        track_activity(f'added asset "{asset.asset_name}"', caseid=case_identifier)
         return 'Asset added', asset
 
-    raise BusinessProcessingError("Unable to create asset for internal reasons")
+    raise BusinessProcessingError('Unable to create asset for internal reasons')
 
 
 def assets_delete(asset: CaseAssets):

source/app/business/iocs.py

@@ -15,15 +15,14 @@
 #  You should have received a copy of the GNU Lesser General Public License
 #  along with this program; if not, write to the Free Software Foundation,
 #  Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-import logging
 
 from flask_login import current_user
 from marshmallow.exceptions import ValidationError
 
 from app import db
 from app.models import Ioc
 from app.datamgmt.case.case_iocs_db import add_ioc
-from app.datamgmt.case.case_iocs_db import case_iocs_db_find_duplicate
+from app.datamgmt.case.case_iocs_db import case_iocs_db_exists
 from app.datamgmt.case.case_iocs_db import check_ioc_type_id
 from app.datamgmt.case.case_iocs_db import get_iocs
 from app.datamgmt.case.case_iocs_db import delete_ioc
@@ -64,7 +63,7 @@ def iocs_create(request_json, case_identifier):
     if not check_ioc_type_id(type_id=ioc.ioc_type_id):
         raise BusinessProcessingError('Not a valid IOC type')
 
-    if case_iocs_db_find_duplicate(ioc):
+    if case_iocs_db_exists(ioc):
         raise BusinessProcessingError('IOC with same value and type already exists')
 
     add_ioc(ioc, current_user.id, case_identifier)