Add backend validation and migration for failOnContentCheck (#2844)

Fixes #2838 

- Ensure `failOnContentCheck` cannot be set when a browser profile is
not set in crawlconfig POST and PATCH endpoints, else return 400 with
detail `fail_on_content_check_requires_profile`
- Add migration to unset `failOnContentCheck` for any workflows in the
database that currently have that flag enabled without a profile
configured

Author: tw4l

backend/btrixcloud/crawlconfigs.py

@@ -259,6 +259,11 @@ async def add_crawl_config(
         # ensure profile is valid, if provided
         if profileid:
             await self.profiles.get_profile(profileid, org)
+        else:
+            if config_in.config and config_in.config.failOnContentCheck:
+                raise HTTPException(
+                    status_code=400, detail="fail_on_content_check_requires_profile"
+                )
 
         # ensure proxyId is valid and available for org
         if config_in.proxyId:
@@ -525,6 +530,29 @@ async def update_crawl_config(
                     status_code=400, detail="use_one_of_seeds_or_seedfile"
                 )
 
+        # If adding failOnContentCheck, ensure a profile is set
+        if update.config and update.config.failOnContentCheck:
+            if (orig_crawl_config.profileid and update.profileid == "") or (
+                not orig_crawl_config.profileid and not update.profileid
+            ):
+                raise HTTPException(
+                    status_code=400, detail="fail_on_content_check_requires_profile"
+                )
+
+        # If unsetting profile, ensure failOnContentCheck is also not set
+        if orig_crawl_config.profileid and update.profileid == "":
+            if update.config and update.config.failOnContentCheck:
+                raise HTTPException(
+                    status_code=400, detail="fail_on_content_check_requires_profile"
+                )
+            if orig_crawl_config.config.failOnContentCheck:
+                if not update.config or (
+                    update.config and update.config.failOnContentCheck is not False
+                ):
+                    raise HTTPException(
+                        status_code=400, detail="fail_on_content_check_requires_profile"
+                    )
+
         # indicates if any k8s crawl config settings changed
         changed = False
         changed = changed or (

backend/btrixcloud/db.py

@@ -34,7 +34,7 @@
     ) = PageOps = BackgroundJobOps = FileUploadOps = CrawlLogOps = object
 
 
-CURR_DB_VERSION = "0050"
+CURR_DB_VERSION = "0051"
 
 
 # ============================================================================

backend/btrixcloud/migrations/migration_0051_fail_on_content_check_profiles.py

@@ -0,0 +1,37 @@
+"""
+Migration 0051 - Ensure failOnContentCheck is not set for workflows without profiles
+"""
+
+from btrixcloud.migrations import BaseMigration
+
+
+MIGRATION_VERSION = "0051"
+
+
+# pylint: disable=duplicate-code
+class Migration(BaseMigration):
+    """Migration class."""
+
+    # pylint: disable=unused-argument
+    def __init__(self, mdb, **kwargs):
+        super().__init__(mdb, migration_version=MIGRATION_VERSION)
+
+    async def migrate_up(self) -> None:
+        """Perform migration up.
+
+        Unset failOnContentCheck for workflows that don't have a profile set
+        """
+        crawl_configs_mdb = self.mdb["crawl_configs"]
+
+        try:
+            await crawl_configs_mdb.update_many(
+                {"profileid": None, "config.failOnContentCheck": True},
+                {"$set": {"config.failOnContentCheck": False}},
+            )
+
+        # pylint: disable=broad-exception-caught
+        except Exception as err:
+            print(
+                f"Error unsetting failOnContentCheck for configs without profiles: {err}",
+                flush=True,
+            )

backend/test/conftest.py

@@ -332,7 +332,10 @@ def sample_crawl_data():
     return {
         "runNow": False,
         "name": "Test Crawl",
-        "config": {"seeds": [{"url": "https://example-com.webrecorder.net/"}], "extraHops": 1},
+        "config": {
+            "seeds": [{"url": "https://example-com.webrecorder.net/"}],
+            "extraHops": 1,
+        },
         "tags": ["tag1", "tag2"],
     }
 
@@ -822,6 +825,35 @@ def profile_2_id(admin_auth_headers, default_org_id, profile_browser_2_id):
             time.sleep(5)
 
 
+@pytest.fixture(scope="session")
+def profile_2_config_id(admin_auth_headers, default_org_id, profile_2_id):
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/profiles/{profile_2_id}",
+        headers=admin_auth_headers,
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["id"] == profile_2_id
+
+    # Use profile in a workflow
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/",
+        headers=admin_auth_headers,
+        json={
+            "runNow": False,
+            "name": "Profile 2 Test Crawl",
+            "description": "Crawl using browser profile",
+            "config": {
+                "seeds": [{"url": "https://webrecorder.net/"}],
+                "exclude": "community",
+            },
+            "profileid": profile_2_id,
+        },
+    )
+    data = r.json()
+    return data["id"]
+
+
 @pytest.fixture(scope="session")
 def seed_file_id(crawler_auth_headers, default_org_id):
     with open(os.path.join(curr_dir, "data", "seedfile.txt"), "rb") as fh:

backend/test/test_crawlconfigs.py

@@ -993,3 +993,137 @@ def test_delete_seed_file_in_use_crawlconfig(
     )
     assert r.status_code == 200
     assert r.json()["id"] == seed_file_id
+
+
+def test_add_crawl_config_fail_on_content_check_no_profile(
+    crawler_auth_headers, default_org_id, sample_crawl_data
+):
+    # Ensure we're not able to set failOnContentCheck on a new crawlconfig
+    # if a profile is not also set
+    sample_crawl_data["config"]["failOnContentCheck"] = True
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/",
+        headers=crawler_auth_headers,
+        json=sample_crawl_data,
+    )
+    assert r.status_code == 400
+    assert r.json()["detail"] == "fail_on_content_check_requires_profile"
+
+    # Ensure an empty string doesn't count as a profile being set
+    sample_crawl_data["profileid"] = ""
+    sample_crawl_data["config"]["failOnContentCheck"] = True
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/",
+        headers=crawler_auth_headers,
+        json=sample_crawl_data,
+    )
+    assert r.status_code == 400
+    assert r.json()["detail"] == "fail_on_content_check_requires_profile"
+
+
+def test_update_crawl_config_fail_on_content_check_no_profile(
+    crawler_auth_headers, default_org_id
+):
+    # Ensure we're not able to update an existing config with no profile to
+    # enable failOnContentCheck
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{_admin_crawl_cid}/",
+        headers=crawler_auth_headers,
+        json={
+            "config": {
+                "failOnContentCheck": True,
+            }
+        },
+    )
+    assert r.status_code == 400
+    assert r.json()["detail"] == "fail_on_content_check_requires_profile"
+
+
+def test_update_crawl_config_remove_profile_with_fail_on_content_check(
+    crawler_auth_headers, default_org_id, profile_2_config_id
+):
+    # Ensure removing a profile fails validation if failOnContentCheck is set
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={
+            "profileid": "",
+            "config": {
+                "failOnContentCheck": True,
+            },
+        },
+    )
+    assert r.status_code == 400
+    assert r.json()["detail"] == "fail_on_content_check_requires_profile"
+
+
+def test_update_crawl_config_fail_on_content_check_with_profile(
+    crawler_auth_headers, default_org_id, profile_2_config_id
+):
+    # Ensure we are able to update a config to enable failOnContentCheck
+    # if a profile is set for the config
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={
+            "config": {
+                "failOnContentCheck": True,
+            }
+        },
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["settings_changed"] == True
+    assert data["metadata_changed"] == False
+
+
+def test_update_crawl_config_remove_profile_no_fail_on_content_check(
+    crawler_auth_headers, default_org_id, profile_2_id, profile_2_config_id
+):
+    # First remove failOnContentCheck
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={
+            "config": {
+                "failOnContentCheck": False,
+            }
+        },
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["settings_changed"] == True
+    assert data["metadata_changed"] == False
+
+    # Now we should be able to remove the profile without getting an error
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={"profileid": ""},
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["settings_changed"] == True
+    assert data["metadata_changed"] == False
+
+    # Add the profile and failOnContentCheck back
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={"profileid": profile_2_id, "config": {"failOnContentCheck": True}},
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["settings_changed"] == True
+    assert data["metadata_changed"] == False
+
+    # Now check removing them both together
+    r = requests.patch(
+        f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs/{profile_2_config_id}/",
+        headers=crawler_auth_headers,
+        json={"profileid": "", "config": {"failOnContentCheck": False}},
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["settings_changed"] == True
+    assert data["metadata_changed"] == False

backend/test/test_filter_sort_results.py

@@ -23,8 +23,8 @@ def test_get_config_by_modified_by(
         f"{API_PREFIX}/orgs/{default_org_id}/crawlconfigs?modifiedBy={crawler_userid}",
         headers=crawler_auth_headers,
     )
-    assert len(r.json()["items"]) == 9
-    assert r.json()["total"] == 9
+    assert len(r.json()["items"]) == 10
+    assert r.json()["total"] == 10
 
 
 def test_get_configs_by_first_seed(
@@ -362,9 +362,9 @@ def test_sort_crawl_configs(
         headers=crawler_auth_headers,
     )
     data = r.json()
-    assert data["total"] == 15
+    assert data["total"] == 16
     items = data["items"]
-    assert len(items) == 15
+    assert len(items) == 16
 
     last_created = None
     for config in items: