From fab38abef98e2e38d1b8affd2aed8a5ed2977562 Mon Sep 17 00:00:00 2001 From: decfox Date: Sat, 11 May 2024 23:35:20 +0800 Subject: [PATCH] extend router tests for oonifindings --- .../src/oonifindings/routers/v1.py | 51 +++-- .../services/oonifindings/tests/conftest.py | 9 +- .../oonifindings/tests/test_oonifindings.py | 215 ++++++++++++++++-- 3 files changed, 240 insertions(+), 35 deletions(-) diff --git a/ooniapi/services/oonifindings/src/oonifindings/routers/v1.py b/ooniapi/services/oonifindings/src/oonifindings/routers/v1.py index 1ac111fd..7a923875 100644 --- a/ooniapi/services/oonifindings/src/oonifindings/routers/v1.py +++ b/ooniapi/services/oonifindings/src/oonifindings/routers/v1.py @@ -99,8 +99,21 @@ class OONIFinding(OONIFindingWithMail): ) +class OONIFindingWithText(OONIFinding): + text: str = Field( + title="content of the oonifinding report" + ) + + @field_validator("title", "text") + @classmethod + def check_empty(cls, v: str): + if not v: + raise ValueError("field cannot be empty") + return v + + class OONIFindingIncident(BaseModel): - incident: OONIFinding + incident: OONIFindingWithText class OONIFindingIncidents(BaseModel): @@ -143,7 +156,8 @@ def list_oonifindings( query = f"""SELECT id, update_time, start_time, end_time, reported_by, title, event_type, published, CCs, ASNs, domains, tags, test_names, - links, short_description, email_address, create_time, creator_account_id + links, short_description, email_address, create_time, + creator_account_id = %(account_id)s as mine FROM incidents FINAL {where} ORDER BY title @@ -159,8 +173,8 @@ def list_oonifindings( setnocacheresponse(response) incident_models = [] - # TODO(decfox): try using OONIFindings.validate_model to populate model - for incident in incidents: + for i in range(len(incidents)): + incident = incidents[i] incident_model = OONIFinding.model_validate(incident) incident_models.append(incident_model) return OONIFindingIncidents(incidents=incident_models) @@ -195,7 +209,8 @@ def get_oonifinding_by_id( query = f"""SELECT id, update_time, start_time, end_time, reported_by, title, text, event_type, published, CCs, ASNs, domains, tags, test_names, - links, short_description, email_address, create_time, creator_account_id + links, short_description, email_address, create_time, + creator_account_id = %(account_id)s AS mine FROM incidents FINAL {where} LIMIT 1 @@ -212,7 +227,7 @@ def get_oonifinding_by_id( # TODO: cache if possible setnocacheresponse(response) - incident_model = OONIFinding.model_validate(incident) + incident_model = OONIFindingWithText.model_validate(incident) return OONIFindingIncident(incident=incident_model) @@ -269,17 +284,8 @@ def verify_user( raise HTTPException(status_code=400, detail="Invalid email address for owner account") -class OONIFindingCreateUpdate(OONIFinding): - text: str = Field( - title="content of the oonifinding report" - ) - - @field_validator("title", "text") - @classmethod - def check_empty(cls, v: str): - if not v: - raise ValueError("field cannot be empty") - return v +class OONIFindingCreateUpdate(OONIFindingWithText): + pass class OONIFindingsUpdateResponse(OONIFindingId): @@ -370,10 +376,13 @@ def update_oonifinding( ) except: raise - - if update_request.published: - raise HTTPException(status_code=400, details="Not enough permissions to publish") + if update_request.published is True: + raise HTTPException(status_code=400, detail="Not enough permissions to publish") + + update_request.creator_account_id = get_account_id_or_raise( + authorization, jwt_encryption_key=settings.jwt_encryption_key + ) incident_dict = prepare_incident_dict(update_request) log.info(f"Updating incident {incident_id}") @@ -438,7 +447,7 @@ def delete_oonifinding( ) def update_oonifinding_publish_status( action: str, - publish_request: OONIFindingId, + publish_request: OONIFindingCreateUpdate, response: Response, db=Depends(get_clickhouse_session), ): diff --git a/ooniapi/services/oonifindings/tests/conftest.py b/ooniapi/services/oonifindings/tests/conftest.py index 23851954..1a82d119 100644 --- a/ooniapi/services/oonifindings/tests/conftest.py +++ b/ooniapi/services/oonifindings/tests/conftest.py @@ -110,7 +110,6 @@ def create_session_token(account_id: str, role: str) -> str: } return create_jwt(payload) - @pytest.fixture def client_with_user_role(client): client = TestClient(app) @@ -138,3 +137,11 @@ def _hashed_email(email: str, role: str): return client return _hashed_email + + +@pytest.fixture +def client_with_null_account(client): + client = TestClient(app) + jwt_token = create_session_token(None, "user") + client.headers = {"Authorization": f"Bearer {jwt_token}"} + yield client diff --git a/ooniapi/services/oonifindings/tests/test_oonifindings.py b/ooniapi/services/oonifindings/tests/test_oonifindings.py index bd7e039b..0a9d67e2 100644 --- a/ooniapi/services/oonifindings/tests/test_oonifindings.py +++ b/ooniapi/services/oonifindings/tests/test_oonifindings.py @@ -2,6 +2,7 @@ Integration test for OONIFindings API """ +from typing import Dict, List from copy import deepcopy from datetime import timedelta @@ -45,10 +46,11 @@ "end_time", "create_time", "update_time", - "mine", + "creator_account_id", "reported_by", "email_address", "text", + "mine", "published", "event_type", "ASNs", @@ -120,26 +122,40 @@ def test_oonifinding_publish(client, client_with_hashed_email): incident_id = r.json()["id"] assert incident_id - r = client_with_admin_role.post("api/v1/incidents/random", json=z) + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident_payload = r.json()["incident"] + + r = client_with_admin_role.post("api/v1/incidents/random", json=incident_payload) assert r.status_code == 400, "only publish and unpublish are valid supported actions" - r = client_with_user_role.post("api/v1/incidents/publish", json=z) + r = client_with_user_role.post("api/v1/incidents/publish", json=incident_payload) assert r.status_code == 401, "only admins can publish incidents" - r = client_with_admin_role.post("api/v1/incidents/publish", json=z) + incident_payload["id"] = "sample id" + r = client_with_admin_role.post("api/v1/incidents/publish", json=incident_payload) assert r.status_code == 404, "valid incident id should be passed" - z["id"] = incident_id - r = client_with_admin_role.post("api/v1/incidents/publish", json=z) + incident_payload["id"] = incident_id + r = client_with_admin_role.post("api/v1/incidents/publish", json=incident_payload) assert r.status_code == 200 assert r.json()["r"] == 1 assert r.json()["id"] == incident_id - r = client_with_admin_role.post("api/v1/incidents/unpublish", json=z) + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["published"] is True + + r = client_with_admin_role.post("api/v1/incidents/unpublish", json=incident_payload) assert r.status_code == 200 assert r.json()["r"] == 1 assert r.json()["id"] == incident_id + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["published"] is False + def test_oonifinding_delete(client, client_with_hashed_email): client_with_admin_role = client_with_hashed_email(SAMPLE_EMAIL, "admin") @@ -178,6 +194,9 @@ def test_oonifinding_delete(client, client_with_hashed_email): r = client_with_user_role.post("api/v1/incidents/delete", json=z) assert r.status_code == 200 + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + assert r.status_code == 404 + def test_oonifinding_update(client, client_with_hashed_email): client_with_admin_role = client_with_hashed_email(SAMPLE_EMAIL, "admin") @@ -195,21 +214,191 @@ def test_oonifinding_update(client, client_with_hashed_email): r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") incident_payload = r.json()["incident"] - incident_payload["text"] = "sample replacement text for update" + sample_replacement_text = "sample replacement text for update" + incident_payload["text"] = sample_replacement_text r = client_with_admin_role.post("api/v1/incidents/update", json=incident_payload) assert r.json()["r"] == 1 assert r.json()["id"] == incident_id + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident_payload = r.json()["incident"] + assert incident_payload + assert incident_payload["text"] == sample_replacement_text - incident_payload["short_description"] = "sample replacement discription for update" + incident_payload["text"] = "" + r = client_with_admin_role.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 422, "cannot update with empty text" + + incident_payload["text"] = sample_replacement_text + incident_payload["title"] = "" + r = client_with_admin_role.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 422, "cannot update with empty title" + + incident_payload["title"] = z["title"] + sample_replacement_description = "sample replacement discription for update" + incident_payload["short_description"] = sample_replacement_description incident_payload["email_address"] = "" r = client_with_user_role.post("api/v1/incidents/update", json=incident_payload) - assert r.status_code == 400 + assert r.status_code == 400, "cannot update with invalid email" incident_payload["email_address"] = SAMPLE_EMAIL mismatched_client = client_with_hashed_email("user@ooni.org", "user") - r = mismatched_client.post("api/v1/incidents/delete", json=incident_payload) - assert r.status_code == 400 + r = mismatched_client.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 400, "email should match account id" + + r = client_with_user_role.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 200 + assert r.json()["r"] == 1 + assert r.json()["id"] == incident_id + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident_payload = r.json()["incident"] + assert incident_payload + assert incident_payload["short_description"] == sample_replacement_description + + sample_tag = "sample_tag" + incident_payload["tags"].append(sample_tag) + r = client_with_user_role.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 200 + assert r.json()["r"] == 1 + assert r.json()["id"] == incident_id + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident_payload = r.json()["incident"] + assert incident_payload + assert len(incident_payload["tags"]) == 1 + assert incident_payload["tags"][0] == sample_tag + incident_payload["published"] = True r = client_with_user_role.post("api/v1/incidents/update", json=incident_payload) - assert r.status_code == 200 + assert r.status_code == 400, "user role cannot publish incident" + + r = client_with_admin_role.post("api/v1/incidents/update", json=incident_payload) + assert r.status_code == 200 + assert r.json()["r"] == 1 + assert r.json()["id"] == incident_id + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident_payload = r.json()["incident"] + assert incident_payload + assert incident_payload["published"] == True + + +# TODO(decfox): add checks for fetched incident fields +def test_oonifinding_workflow( + client, + client_with_hashed_email, + client_with_user_role, + client_with_null_account + ): + client_with_admin_role = client_with_hashed_email(SAMPLE_EMAIL, "admin") + + z = deepcopy(SAMPLE_OONIFINDING) + + r = client_with_admin_role.post("api/v1/incidents/create", json=z) + assert r.status_code == 200 + assert r.json()["r"] == 1 + + incident_id = r.json()["id"] + assert incident_id + + r = client_with_null_account.get(f"api/v1/incidents/show/{incident_id}") + assert r.status_code == 404, "unpublished events cannot be seen with invalid account id" + + r = client_with_user_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["mine"] is False + assert incident["email_address"] == "" + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["mine"] is True + assert incident["email_address"] == z["email_address"] + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + # publish incident and test + r = client_with_admin_role.post("api/v1/incidents/publish", json=incident) + assert r.json()["r"] == 1 + + r = client_with_null_account.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["mine"] is False + assert incident["email_address"] == "" + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_user_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["mine"] is False + assert incident["email_address"] == "" + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_admin_role.get(f"api/v1/incidents/show/{incident_id}") + incident = r.json()["incident"] + assert incident + assert incident["mine"] is True + assert incident["email_address"] == z["email_address"] + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + EXPECTED_OONIFINDING_PUBLIC_KEYS.remove("text") + + r = client_with_null_account.get("api/v1/incidents/search?only_mine=false") + assert r.status_code == 200 + incidents = r.json()["incidents"] + assert len(incidents) == 2 + for incident in incidents: + assert incident["email_address"] == "" + assert incident["mine"] is False + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + + r = client_with_user_role.get("api/v1/incidents/search?only_mine=false") + incidents = r.json()["incidents"] + assert len(incidents) == 4 + for incident in incidents: + assert incident["email_address"] == "" + assert incident["mine"] is False + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_admin_role.get("api/v1/incidents/search?only_mine=false") + incidents = r.json()["incidents"] + assert len(incidents) == 4 + for incident in incidents: + assert incident["email_address"] == SAMPLE_EMAIL + assert incident["mine"] is True + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_null_account.get("api/v1/incidents/search?only_mine=true") + assert r.status_code == 200 + incidents = r.json()["incidents"] + assert len(incidents) == 0 + + r = client_with_user_role.get("api/v1/incidents/search?only_mine=true") + assert r.status_code == 200 + incidents = r.json()["incidents"] + assert len(incidents) == 0 + + client_account_with_user_role = client_with_hashed_email(SAMPLE_EMAIL, "user") + + r = client_account_with_user_role.get("api/v1/incidents/search?only_mine=true") + assert r.status_code == 200 + incidents = r.json()["incidents"] + assert len(incidents) == 4 + for incident in incidents: + assert incident["email_address"] == "" + assert incident["mine"] is True + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS) + + r = client_with_admin_role.get("api/v1/incidents/search?only_mine=true") + assert r.status_code == 200 + incidents = r.json()["incidents"] + assert len(incidents) == 4 + for incident in incidents: + assert incident["email_address"] == SAMPLE_EMAIL + assert incident["mine"] is True + assert sorted(incident.keys()) == sorted(EXPECTED_OONIFINDING_PUBLIC_KEYS)