Merge pull request #623 from dfir-iris/deleted_asset_should_not_be_vi…

…sible

Deleted asset should not be accessible via REST

source/app/blueprints/rest/v2/case/api_v2_assets_routes.py

@@ -18,6 +18,7 @@
 
 from flask import Blueprint
 from flask import request
+from kombu.abstract import Object
 
 from app.blueprints.access_controls import ac_api_requires
 from app.blueprints.rest.endpoints import response_api_created
@@ -68,6 +69,8 @@ def asset_view(identifier):
             return ac_api_return_access_denied(caseid=asset.case_id)
 
         return response_api_success(asset_schema.dump(asset))
+    except ObjectNotFoundError:
+        return response_api_not_found()
     except BusinessProcessingError as e:
         return response_api_error(e.get_message())
 

source/app/business/assets.py

@@ -66,7 +66,7 @@ def assets_create(case_identifier, request_json):
 def assets_delete(asset: CaseAssets):
     call_modules_hook('on_preload_asset_delete', data=asset.asset_id)
     # Deletes an asset and the potential links with the IoCs from the database
-    delete_asset(asset.asset_id, asset.case_id)
+    delete_asset(asset)
     call_modules_hook('on_postload_asset_delete', data=asset.asset_id, caseid=asset.case_id)
     track_activity(f'removed asset ID {asset.asset_name}', caseid=asset.case_id)
 

source/app/datamgmt/case/case_assets_db.py

@@ -130,51 +130,33 @@ def update_asset(asset_name, asset_description, asset_ip, asset_info, asset_doma
     db.session.commit()
 
 
-def delete_asset(asset_id, caseid):
-    case_asset = get_asset(asset_id)
-    if case_asset is None:
-        return
+def delete_asset(asset: CaseAssets):
+    delete_ioc_asset_link(asset.asset_id)
 
-    if case_asset.case_id and case_asset.alerts is not None:
+    # Delete the relevant records from the CaseEventsAssets table
+    CaseEventsAssets.query.filter(
+        asset.asset_id == CaseEventsAssets.asset_id
+    ).delete()
+
+    # Delete the relevant records from the AssetComments table
+    com_ids = AssetComments.query.with_entities(
+        AssetComments.comment_id
+    ).filter(
+        AssetComments.comment_asset_id == asset.asset_id
+    ).all()
 
-        CaseEventsAssets.query.filter(
-            case_asset.asset_id == CaseEventsAssets.asset_id
-        ).delete()
+    com_ids = [c.comment_id for c in com_ids]
+    AssetComments.query.filter(AssetComments.comment_id.in_(com_ids)).delete()
 
-        case_asset.case_id = None
-        db.session.commit()
-        return
-
-    with db.session.begin_nested():
-        delete_ioc_asset_link(asset_id)
-
-        # Delete the relevant records from the CaseEventsAssets table
-        CaseEventsAssets.query.filter(
-            CaseEventsAssets.case_id == caseid,
-            CaseEventsAssets.asset_id == asset_id
-        ).delete()
-
-        # Delete the relevant records from the AssetComments table
-        com_ids = AssetComments.query.with_entities(
-            AssetComments.comment_id
-        ).filter(
-            AssetComments.comment_asset_id == asset_id,
-        ).all()
-
-        com_ids = [c.comment_id for c in com_ids]
-        AssetComments.query.filter(AssetComments.comment_id.in_(com_ids)).delete()
-
-        Comments.query.filter(
-            Comments.comment_id.in_(com_ids)
-        ).delete()
-
-        # Directly delete the relevant records from the CaseAssets table
-        CaseAssets.query.filter(
-            CaseAssets.asset_id == asset_id,
-            CaseAssets.case_id == caseid
-        ).delete()
-
-        update_assets_state(caseid=caseid)
+    Comments.query.filter(
+        Comments.comment_id.in_(com_ids)
+    ).delete()
+
+    db.session.delete(asset)
+
+    update_assets_state(asset.case_id)
+
+    db.session.commit()
 
 
 def get_assets_types():

source/app/datamgmt/case/case_iocs_db.py

@@ -70,9 +70,23 @@ def update_ioc(ioc_type, ioc_tags, ioc_value, ioc_description, ioc_tlp, userid,
 
 
 def delete_ioc(ioc: Ioc):
+    # Delete the relevant records from the AssetComments table
+    com_ids = IocComments.query.with_entities(
+        IocComments.comment_id
+    ).filter(
+        IocComments.comment_ioc_id == ioc.ioc_id,
+    ).all()
+
+    com_ids = [c.comment_id for c in com_ids]
+    IocComments.query.filter(IocComments.comment_id.in_(com_ids)).delete()
+
+    Comments.query.filter(
+        Comments.comment_id.in_(com_ids)
+    ).delete()
+
     db.session.delete(ioc)
 
-    update_ioc_state(caseid=ioc.case_id)
+    update_ioc_state(ioc.case_id)
 
 
 def get_detailed_iocs(caseid):

tests/tests_rest_assets.py

@@ -49,7 +49,7 @@ def test_create_asset_should_work(self):
         self.assertEqual(201, response.status_code)
 
     def test_get_asset_with_missing_asset_identifier_should_return_404(self):
-        response = self._subject.get('/api/v2/asset/None')
+        response = self._subject.get(f'/api/v2/asset/{_IDENTIFIER_FOR_NONEXISTENT_OBJECT}')
         self.assertEqual(404, response.status_code)
 
     def test_create_asset_with_missing_case_identifier_should_return_404(self):
@@ -78,3 +78,55 @@ def test_get_asset_should_return_200(self):
         asset_identifier = response['asset_id']
         response = self._subject.get(f'/api/v2/assets/{asset_identifier}')
         self.assertEqual(200, response.status_code)
+
+    def test_get_asset_should_return_404_after_it_was_deleted(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'asset_type_id': '1', 'asset_name': 'admin_laptop_test'}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/assets', body).json()
+        asset_identifier = response['asset_id']
+        self._subject.delete(f'/api/v2/assets/{asset_identifier}')
+        response = self._subject.get(f'/api/v2/assets/{asset_identifier}')
+        self.assertEqual(404, response.status_code)
+
+    def test_delete_asset_should_increment_asset_state(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'asset_type_id': '1', 'asset_name': 'admin_laptop_test'}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/assets', body).json()
+        asset_identifier = response['asset_id']
+        response = self._subject.get('/case/assets/state', {'cid': case_identifier}).json()
+        state = response['data']['object_state']
+        self._subject.delete(f'/api/v2/assets/{asset_identifier}')
+        response = self._subject.get('/case/assets/state', {'cid': case_identifier}).json()
+        self.assertEqual(state + 1, response['data']['object_state'])
+
+    def test_delele_asset_should_not_fail_when_it_is_linked_to_an_ioc(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'ioc_type_id': 1, 'ioc_tlp_id': 2, 'ioc_value': '8.8.8.8', 'ioc_description': 'rewrw', 'ioc_tags': ''}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/iocs', body).json()
+        ioc_identifier = response['ioc_id']
+        body = {'asset_type_id': '1', 'asset_name': 'admin_laptop_test', 'ioc_links': [ioc_identifier]}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/assets', body).json()
+        asset_identifier = response['asset_id']
+        response = self._subject.delete(f'/api/v2/assets/{asset_identifier}')
+        self.assertEqual(204, response.status_code)
+
+    def test_delete_asset_should_not_fail_when_it_has_associated_comments(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'asset_type_id': '1', 'asset_name': 'admin_laptop_test'}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/assets', body).json()
+        asset_identifier = response['asset_id']
+        self._subject.create(f'/case/assets/{asset_identifier}/comments/add', {'comment_text': 'comment text'})
+        response = self._subject.delete(f'/api/v2/assets/{asset_identifier}')
+        self.assertEqual(204, response.status_code)
+
+    def test_delete_asset_should_delete_associated_comments(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'asset_type_id': '1', 'asset_name': 'admin_laptop_test'}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/assets', body).json()
+        asset_identifier = response['asset_id']
+        response = self._subject.create(f'/case/assets/{asset_identifier}/comments/add', {'comment_text': 'comment text'}).json()
+        comment_identifier = response['data']['comment_id']
+        self._subject.delete(f'/api/v2/assets/{asset_identifier}')
+        response = self._subject.create(f'/case/assets/{case_identifier}/comments/{comment_identifier}/edit', {'comment_text': 'new comment text'})
+        # TODO should ideally rather be 404 here
+        self.assertEqual(400, response.status_code)

tests/tests_rest_iocs.py

@@ -157,6 +157,16 @@ def test_create_ioc_should_not_create_two_iocs_with_identical_type_and_value(sel
         response = self._subject.create(f'/api/v2/cases/{case_identifier}/iocs', body)
         self.assertEqual(400, response.status_code)
 
+    def test_delete_ioc_should_not_prevent_case_deletion(self):
+        case_identifier = self._subject.create_dummy_case()
+        body = {'ioc_type_id': 1, 'ioc_tlp_id': 2, 'ioc_value': '8.8.8.8', 'ioc_description': 'rewrw', 'ioc_tags': ''}
+        response = self._subject.create(f'/api/v2/cases/{case_identifier}/iocs', body).json()
+        ioc_identifier = response['ioc_id']
+        self._subject.create(f'/case/ioc/{ioc_identifier}/comments/add', {'comment_text': 'comment text'})
+        self._subject.delete(f'/api/v2/iocs/{ioc_identifier}')
+        response = self._subject.delete(f'/api/v2/cases/{case_identifier}')
+        self.assertEqual(204, response.status_code)
+
     def test_update_ioc_should_not_fail(self):
         case_identifier = self._subject.create_dummy_case()
         body = {'ioc_type_id': 1, 'ioc_tlp_id': 2, 'ioc_value': '8.8.8.8', 'ioc_description': 'rewrw', 'ioc_tags': ''}