From 9822b597165b229a1472a0833951f0abeff56e72 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Fri, 19 Aug 2022 09:13:15 -0500 Subject: [PATCH 1/4] Adds tests for email endpoints and dispatch service --- .../ops/api/v1/endpoints/email_endpoints.py | 9 +- .../api/v1/endpoints/test_email_endpoints.py | 497 ++++++++++++++++++ tests/ops/fixtures/application_fixtures.py | 30 ++ .../email/email_dispatch_service_test.py | 89 ++++ 4 files changed, 621 insertions(+), 4 deletions(-) create mode 100644 tests/ops/api/v1/endpoints/test_email_endpoints.py create mode 100644 tests/ops/service/email/email_dispatch_service_test.py diff --git a/src/fidesops/ops/api/v1/endpoints/email_endpoints.py b/src/fidesops/ops/api/v1/endpoints/email_endpoints.py index 7e4450c4d..b13545fc1 100644 --- a/src/fidesops/ops/api/v1/endpoints/email_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/email_endpoints.py @@ -2,8 +2,9 @@ from typing import Optional from fastapi import Depends, Security -from fastapi_pagination import Page, Params, paginate +from fastapi_pagination import Page, Params from fastapi_pagination.bases import AbstractPage +from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy.orm import Session from starlette.exceptions import HTTPException from starlette.status import ( @@ -97,10 +98,10 @@ def patch_config_by_key( try: return update_email_config(db=db, key=config_key, config=email_config) except EmailConfigNotFoundException: - logger.warning(f"No email config found with key {email_config.key}") + logger.warning(f"No email config found with key {config_key}") raise HTTPException( - status_code=HTTP_400_BAD_REQUEST, - detail=f"No email config found with key {email_config.key}", + status_code=HTTP_404_NOT_FOUND, + detail=f"No email config found with key {config_key}", ) except Exception as exc: diff --git a/tests/ops/api/v1/endpoints/test_email_endpoints.py b/tests/ops/api/v1/endpoints/test_email_endpoints.py new file mode 100644 index 000000000..2d781e697 --- /dev/null +++ b/tests/ops/api/v1/endpoints/test_email_endpoints.py @@ -0,0 +1,497 @@ +import json + +import pytest +from fastapi_pagination import Params +from sqlalchemy.orm import Session +from starlette.testclient import TestClient + +from fidesops.ops.api.v1.scope_registry import ( + EMAIL_CREATE_OR_UPDATE, + EMAIL_DELETE, + EMAIL_READ, +) +from fidesops.ops.api.v1.urn_registry import ( + EMAIL_BY_KEY, + EMAIL_CONFIG, + EMAIL_SECRETS, + V1_URL_PREFIX, +) +from fidesops.ops.models.email import EmailConfig +from fidesops.ops.schemas.email.email import ( + EmailServiceDetails, + EmailServiceSecrets, + EmailServiceType, +) + +PAGE_SIZE = Params().size + + +class TestPostEmailConfig: + @pytest.fixture(scope="function") + def url(self) -> str: + return V1_URL_PREFIX + EMAIL_CONFIG + + @pytest.fixture(scope="function") + def payload(self): + return { + "name": "mailgun", + "service_type": EmailServiceType.MAILGUN.value, + "details": {EmailServiceDetails.DOMAIN.value: "my.mailgun.domain"}, + } + + def test_post_email_config_not_authenticated( + self, api_client: TestClient, payload, url + ): + response = api_client.post(url, headers={}, json=payload) + assert 401 == response.status_code + + def test_post_email_config_incorrect_scope( + self, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.post(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_post_email_config_with_invalid_mailgun_details( + self, + db: Session, + api_client: TestClient, + url, + payload, + generate_auth_header, + ): + payload["details"] = {"invalid": "invalid"} + + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post(url, headers=auth_header, json=payload) + assert 422 == response.status_code + assert ( + json.loads(response.text)["detail"][0]["msg"] + == "[\"field required ('domain',)\", \"extra fields not permitted ('invalid',)\"]" + ) + + def test_post_email_config_with_not_supported_service_type( + self, + db: Session, + api_client: TestClient, + url, + payload, + generate_auth_header, + ): + payload["service_type"] = "twilio" + + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post(url, headers=auth_header, json=payload) + assert 422 == response.status_code + assert ( + json.loads(response.text)["detail"][0]["msg"] + == "value is not a valid enumeration member; permitted: 'mailgun'" + ) + + def test_post_email_config_with_no_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post(url, headers=auth_header, json=payload) + + assert 200 == response.status_code + response_body = json.loads(response.text) + + assert response_body["key"] == "mailgun" + email_config = db.query(EmailConfig).filter_by(key="mailgun")[0] + email_config.delete(db) + + def test_post_email_config_with_invalid_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + payload["key"] = "*invalid-key" + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post(url, headers=auth_header, json=payload) + assert 422 == response.status_code + assert ( + json.loads(response.text)["detail"][0]["msg"] + == "FidesKey must only contain alphanumeric characters, '.', '_' or '-'." + ) + + def test_post_email_config_with_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + payload["key"] = "my_email_config" + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + + response = api_client.post(url, headers=auth_header, json=payload) + assert 200 == response.status_code + + response_body = json.loads(response.text) + email_config = db.query(EmailConfig).filter_by(key="my_email_config")[0] + + expected_response = { + "key": "my_email_config", + "name": "mailgun", + "service_type": EmailServiceType.MAILGUN.value, + "details": { + EmailServiceDetails.API_VERSION.value: "v3", + EmailServiceDetails.DOMAIN.value: "my.mailgun.domain", + EmailServiceDetails.IS_EU_DOMAIN.value: False, + }, + } + assert expected_response == response_body + email_config.delete(db) + + def test_post_email_config_missing_detail( + self, + api_client: TestClient, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post( + url, + headers=auth_header, + json={ + "key": "my_email_config", + "name": "mailgun", + "service_type": EmailServiceType.MAILGUN.value, + "details": { + # "domain": "my.mailgun.domain" + }, + }, + ) + assert response.status_code == 422 + errors = response.json()["detail"] + assert "details" in errors[0]["loc"] + assert errors[0]["msg"] == "[\"field required ('domain',)\"]" + + def test_post_email_config_already_exists( + self, + api_client: TestClient, + url, + email_config, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.post( + url, + headers=auth_header, + json={ + "key": "my_email_config", + "name": "mailgun", + "service_type": EmailServiceType.MAILGUN.value, + "details": {EmailServiceDetails.DOMAIN.value: "my.mailgun.domain"}, + }, + ) + assert response.status_code == 400 + assert response.json() == { + "detail": "Only one email config is supported at a time. Config with key my_email_config is already configured." + } + + +class TestPatchEmailConfig: + @pytest.fixture(scope="function") + def url(self, email_config) -> str: + return (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key=email_config.key) + + @pytest.fixture(scope="function") + def payload(self): + return { + "key": "my_email_config", + "name": "mailgun new name", + "service_type": EmailServiceType.MAILGUN.value, + "details": {EmailServiceDetails.DOMAIN.value: "my.mailgun.domain"}, + } + + def test_patch_email_config_not_authenticated( + self, api_client: TestClient, payload, url + ): + response = api_client.patch(url, headers={}, json=payload) + assert 401 == response.status_code + + def test_patch_email_config_incorrect_scope( + self, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.patch(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_patch_email_config_with_key_not_found( + self, + db: Session, + api_client: TestClient, + payload, + email_config, + generate_auth_header, + ): + url = (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key="nonexistent_key") + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + + response = api_client.patch(url, headers=auth_header, json=payload) + assert response.status_code == 404 + assert response.json() == { + "detail": "No email config found with key nonexistent_key" + } + + def test_patch_email_config_with_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + + response = api_client.patch(url, headers=auth_header, json=payload) + assert 200 == response.status_code + + response_body = json.loads(response.text) + email_config = db.query(EmailConfig).filter_by(key="my_email_config")[0] + + expected_response = { + "key": "my_email_config", + "name": "mailgun new name", + "service_type": EmailServiceType.MAILGUN.value, + "details": { + EmailServiceDetails.API_VERSION.value: "v3", + EmailServiceDetails.DOMAIN.value: "my.mailgun.domain", + EmailServiceDetails.IS_EU_DOMAIN.value: False, + }, + } + assert expected_response == response_body + email_config.delete(db) + + +class TestPutEmailConfigSecretsMailgun: + @pytest.fixture(scope="function") + def url(self, email_config) -> str: + return (V1_URL_PREFIX + EMAIL_SECRETS).format(config_key=email_config.key) + + @pytest.fixture(scope="function") + def payload(self): + return { + EmailServiceSecrets.MAILGUN_API_KEY.value: "1345234524", + } + + def test_put_config_secrets_unauthenticated( + self, api_client: TestClient, payload, url + ): + response = api_client.put(url, headers={}, json=payload) + assert 401 == response.status_code + + def test_put_config_secrets_wrong_scope( + self, api_client: TestClient, payload, url, generate_auth_header + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.put(url, headers=auth_header, json=payload) + assert 403 == response.status_code + + def test_put_config_secret_invalid_config( + self, api_client: TestClient, payload, generate_auth_header + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + url = (V1_URL_PREFIX + EMAIL_SECRETS).format(config_key="invalid_key") + response = api_client.put(url, headers=auth_header, json=payload) + assert 404 == response.status_code + + def test_update_with_invalid_secrets_key( + self, api_client: TestClient, generate_auth_header, url + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.put(url, headers=auth_header, json={"bad_key": "12345"}) + + assert response.status_code == 400 + assert response.json() == { + "detail": [ + "field required ('mailgun_api_key',)", + "extra fields not permitted ('bad_key',)", + ] + } + + def test_put_config_secrets( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + email_config, + ): + auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) + response = api_client.put(url, headers=auth_header, json=payload) + assert 200 == response.status_code + + db.refresh(email_config) + + assert json.loads(response.text) == { + "msg": "Secrets updated for EmailConfig with key: my_email_config.", + "test_status": None, + "failure_reason": None, + } + assert ( + email_config.secrets[EmailServiceSecrets.MAILGUN_API_KEY.value] + == "1345234524" + ) + + +class TestGetEmailConfigs: + @pytest.fixture(scope="function") + def url(self) -> str: + return V1_URL_PREFIX + EMAIL_CONFIG + + def test_get_configs_not_authenticated(self, api_client: TestClient, url) -> None: + response = api_client.get(url) + assert 401 == response.status_code + + def test_get_configs_wrong_scope( + self, api_client: TestClient, url, generate_auth_header + ) -> None: + auth_header = generate_auth_header([EMAIL_DELETE]) + response = api_client.get(url, headers=auth_header) + assert 403 == response.status_code + + def test_get_configs( + self, db, api_client: TestClient, url, generate_auth_header, email_config + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.get(url, headers=auth_header) + assert 200 == response.status_code + + expected_response = { + "items": [ + { + "key": "my_email_config", + "name": email_config.name, + "service_type": EmailServiceType.MAILGUN.value, + "details": { + EmailServiceDetails.API_VERSION.value: "v3", + EmailServiceDetails.DOMAIN.value: "some.domain", + EmailServiceDetails.IS_EU_DOMAIN.value: False, + }, + } + ], + "page": 1, + "size": PAGE_SIZE, + "total": 1, + } + response_body = json.loads(response.text) + assert expected_response == response_body + + +class TestGetEmailConfig: + @pytest.fixture(scope="function") + def url(self, email_config) -> str: + return (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key=email_config.key) + + def test_get_config_not_authenticated(self, url, api_client: TestClient): + response = api_client.get(url) + assert 401 == response.status_code + + def test_get_config_wrong_scope( + self, url, api_client: TestClient, generate_auth_header + ): + auth_header = generate_auth_header([EMAIL_DELETE]) + response = api_client.get(url, headers=auth_header) + assert 403 == response.status_code + + def test_get_config_invalid( + self, api_client: TestClient, generate_auth_header, email_config + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.get( + (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key="invalid"), + headers=auth_header, + ) + assert 404 == response.status_code + + def test_get_config( + self, url, api_client: TestClient, generate_auth_header, email_config + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.get(url, headers=auth_header) + assert response.status_code == 200 + + response_body = json.loads(response.text) + + assert response_body == { + "key": "my_email_config", + "name": email_config.name, + "service_type": EmailServiceType.MAILGUN.value, + "details": { + EmailServiceDetails.API_VERSION.value: "v3", + EmailServiceDetails.DOMAIN.value: "some.domain", + EmailServiceDetails.IS_EU_DOMAIN.value: False, + }, + } + + +class TestDeleteConfig: + @pytest.fixture(scope="function") + def url(self, email_config) -> str: + return (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key=email_config.key) + + def test_delete_config_not_authenticated(self, url, api_client: TestClient): + response = api_client.delete(url) + assert 401 == response.status_code + + def test_delete_config_wrong_scope( + self, url, api_client: TestClient, generate_auth_header + ): + auth_header = generate_auth_header([EMAIL_READ]) + response = api_client.delete(url, headers=auth_header) + assert 403 == response.status_code + + def test_delete_config_invalid(self, api_client: TestClient, generate_auth_header): + auth_header = generate_auth_header([EMAIL_DELETE]) + response = api_client.delete( + (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key="invalid"), + headers=auth_header, + ) + assert 404 == response.status_code + + def test_delete_config( + self, + db: Session, + url, + api_client: TestClient, + generate_auth_header, + ): + # Creating new config, so we don't run into issues trying to clean up a deleted fixture + email_config = EmailConfig.create( + db=db, + data={ + "key": "my_different_email_config", + "name": "mailgun", + "service_type": EmailServiceType.MAILGUN, + "details": {EmailServiceDetails.DOMAIN.value: "my.mailgun.domain"}, + }, + ) + url = (V1_URL_PREFIX + EMAIL_BY_KEY).format(config_key=email_config.key) + auth_header = generate_auth_header([EMAIL_DELETE]) + response = api_client.delete(url, headers=auth_header) + assert response.status_code == 204 + + db.expunge_all() + config = db.query(EmailConfig).filter_by(key=email_config.key).first() + assert config is None diff --git a/tests/ops/fixtures/application_fixtures.py b/tests/ops/fixtures/application_fixtures.py index 9524482c1..b31e82793 100644 --- a/tests/ops/fixtures/application_fixtures.py +++ b/tests/ops/fixtures/application_fixtures.py @@ -23,6 +23,7 @@ ConnectionType, ) from fidesops.ops.models.datasetconfig import DatasetConfig +from fidesops.ops.models.email import EmailConfig from fidesops.ops.models.policy import ( ActionType, Policy, @@ -33,6 +34,12 @@ ) from fidesops.ops.models.privacy_request import PrivacyRequest, PrivacyRequestStatus from fidesops.ops.models.storage import ResponseFormat, StorageConfig +from fidesops.ops.schemas.email.email import ( + EmailServiceDetails, + EmailServiceSecrets, + EmailServiceSecretsMailgun, + EmailServiceType, +) from fidesops.ops.schemas.redis_cache import PrivacyRequestIdentity from fidesops.ops.schemas.storage.storage import ( FileNaming, @@ -165,6 +172,29 @@ def storage_config_onetrust(db: Session) -> Generator: storage_config.delete(db) +@pytest.fixture(scope="function") +def email_config(db: Session) -> Generator: + name = str(uuid4()) + email_config = EmailConfig.create( + db=db, + data={ + "name": name, + "key": "my_email_config", + "service_type": EmailServiceType.MAILGUN, + "details": { + EmailServiceDetails.API_VERSION.value: "v3", + EmailServiceDetails.DOMAIN.value: "some.domain", + EmailServiceDetails.IS_EU_DOMAIN.value: False, + }, + }, + ) + email_config.set_secrets( + db=db, email_secrets={EmailServiceSecrets.MAILGUN_API_KEY.value: "12984r70298r"} + ) + yield email_config + email_config.delete(db) + + @pytest.fixture(scope="function") def https_connection_config(db: Session) -> Generator: name = str(uuid4()) diff --git a/tests/ops/service/email/email_dispatch_service_test.py b/tests/ops/service/email/email_dispatch_service_test.py new file mode 100644 index 000000000..b1d52c1ec --- /dev/null +++ b/tests/ops/service/email/email_dispatch_service_test.py @@ -0,0 +1,89 @@ +from unittest import mock +from unittest.mock import Mock + +import pytest +from sqlalchemy.orm import Session + +from fidesops.ops.common_exceptions import EmailDispatchException +from fidesops.ops.models.email import EmailConfig +from fidesops.ops.schemas.email.email import ( + EmailActionType, + EmailForActionType, + EmailServiceDetails, + EmailServiceType, + SubjectIdentityVerificationBodyParams, +) +from fidesops.ops.service.email.email_dispatch_service import dispatch_email + + +@mock.patch("fidesops.ops.service.email.email_dispatch_service._mailgun_dispatcher") +def test_email_dispatch_mailgun_success( + mock_mailgun_dispatcher: Mock, db: Session, email_config +) -> None: + + dispatch_email( + db=db, + action_type=EmailActionType.SUBJECT_IDENTITY_VERIFICATION, + to_email="test@email.com", + email_body_params=SubjectIdentityVerificationBodyParams(access_code="2348"), + ) + + mock_mailgun_dispatcher.assert_called_with( + email_config=email_config, + email=EmailForActionType( + subject="Your one-time code", + body=f"Your one-time code is 2348. Hurry! It expires in 10 minutes.", + ), + to_email="test@email.com", + ) + + +@mock.patch("fidesops.ops.service.email.email_dispatch_service._mailgun_dispatcher") +def test_email_dispatch_mailgun_config_not_found( + mock_mailgun_dispatcher: Mock, db: Session +) -> None: + + with pytest.raises(EmailDispatchException) as exc: + dispatch_email( + db=db, + action_type=EmailActionType.SUBJECT_IDENTITY_VERIFICATION, + to_email="test@email.com", + email_body_params=SubjectIdentityVerificationBodyParams(access_code="2348"), + ) + assert exc.value.args[0] == "No email config found." + + mock_mailgun_dispatcher.assert_not_called() + + +@mock.patch("fidesops.ops.service.email.email_dispatch_service._mailgun_dispatcher") +def test_email_dispatch_mailgun_config_no_secrets( + mock_mailgun_dispatcher: Mock, db: Session +) -> None: + + email_config = EmailConfig.create( + db=db, + data={ + "name": "mailgun config", + "key": "my_email_config", + "service_type": EmailServiceType.MAILGUN, + "details": { + EmailServiceDetails.DOMAIN.value: "some.domain", + }, + }, + ) + + with pytest.raises(EmailDispatchException) as exc: + dispatch_email( + db=db, + action_type=EmailActionType.SUBJECT_IDENTITY_VERIFICATION, + to_email="test@email.com", + email_body_params=SubjectIdentityVerificationBodyParams(access_code="2348"), + ) + assert ( + exc.value.args[0] + == "Email secrets not found for config with key: my_email_config" + ) + + mock_mailgun_dispatcher.assert_not_called() + + email_config.delete(db) From c6a3c4a8d3dac27cc03846a83ed6f48369a608e8 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Fri, 19 Aug 2022 09:14:49 -0500 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b15d5f6c4..6f2d6e046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The types of changes are: * Access and erasure support for Logi ID [#1074](https://github.com/ethyca/fidesops/pull/1074) * Adds infra for email config and dispatch [#1059](https://github.com/ethyca/fidesops/pull/1059) * Add an endpoint that allows you to create a Saas connector and all supporting resources with a single request [#1076](https://github.com/ethyca/fidesops/pull/1076) +* Adds tests for email endpoints and service [#1112](https://github.com/ethyca/fidesops/pull/1112) ### Developer Experience From 49108d2a6ded9d8ac569166153093a129e42b2c2 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Fri, 19 Aug 2022 15:19:32 -0500 Subject: [PATCH 3/4] rely on existing pydantic validation, update to 500 err code, better testing on email dispatch service --- .../ops/api/v1/endpoints/email_endpoints.py | 5 +-- src/fidesops/ops/schemas/email/email.py | 32 ------------------- .../service/email/email_dispatch_service.py | 4 +-- .../api/v1/endpoints/test_email_endpoints.py | 7 ++-- .../email/email_dispatch_service_test.py | 28 ++++++++++++++++ 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/fidesops/ops/api/v1/endpoints/email_endpoints.py b/src/fidesops/ops/api/v1/endpoints/email_endpoints.py index b13545fc1..c8c82839e 100644 --- a/src/fidesops/ops/api/v1/endpoints/email_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/email_endpoints.py @@ -13,6 +13,7 @@ HTTP_400_BAD_REQUEST, HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, + HTTP_500_INTERNAL_SERVER_ERROR, ) from fidesops.ops.api import deps @@ -76,7 +77,7 @@ def post_config( except Exception as exc: logger.warning(f"Create failed for email config {email_config.key}: {exc}") raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, + status_code=HTTP_500_INTERNAL_SERVER_ERROR, detail=f"Config with key {email_config.key} failed to be added", ) @@ -107,7 +108,7 @@ def patch_config_by_key( except Exception as exc: logger.warning(f"Patch failed for email config {email_config.key}: {exc}") raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, + status_code=HTTP_500_INTERNAL_SERVER_ERROR, detail=f"Config with key {email_config.key} failed to be added", ) diff --git a/src/fidesops/ops/schemas/email/email.py b/src/fidesops/ops/schemas/email/email.py index 0c5311e5e..a5413e3e7 100644 --- a/src/fidesops/ops/schemas/email/email.py +++ b/src/fidesops/ops/schemas/email/email.py @@ -94,38 +94,6 @@ class Config: use_enum_values = False orm_mode = True - @validator("details", pre=True, always=True) - def validate_details( - cls, - v: Dict[str, str], - values: Dict[str, Any], - ) -> Dict[str, str]: - """ - Custom validation logic for the `details` field. - """ - service_type = values.get("service_type") - if not service_type: - raise ValueError("A `service_type` field must be specified.") - - try: - schema = { - EmailServiceType.MAILGUN: EmailServiceDetailsMailgun, - }[service_type] - except KeyError: - raise ValueError( - f"`service type` {service_type} has no supported `details` validation." - ) - - try: - schema.parse_obj(v) # type: ignore - except ValidationError as exc: - # Pydantic requires validators raise either a ValueError, TypeError, or AssertionError - # so this exception is cast into a `ValueError`. - errors = [f"{err['msg']} {str(err['loc'])}" for err in exc.errors()] - raise ValueError(errors) - - return v - class EmailConfigResponse(BaseModel): """Email Config Response Schema""" diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index ba7144f15..8034cca36 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -103,5 +103,5 @@ def _mailgun_dispatcher( f"Email failed to send with status code {response.status_code}" ) except Exception as e: - logger.error(e) - raise EmailDispatchException(format("Email failed to send: %s", str(e))) + logger.error("Email failed to send: %s", str(e)) + raise EmailDispatchException(f"Email failed to send due to: {e}") diff --git a/tests/ops/api/v1/endpoints/test_email_endpoints.py b/tests/ops/api/v1/endpoints/test_email_endpoints.py index 2d781e697..173021df5 100644 --- a/tests/ops/api/v1/endpoints/test_email_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_email_endpoints.py @@ -69,9 +69,10 @@ def test_post_email_config_with_invalid_mailgun_details( auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE]) response = api_client.post(url, headers=auth_header, json=payload) assert 422 == response.status_code + assert json.loads(response.text)["detail"][0]["msg"] == "field required" assert ( - json.loads(response.text)["detail"][0]["msg"] - == "[\"field required ('domain',)\", \"extra fields not permitted ('invalid',)\"]" + json.loads(response.text)["detail"][1]["msg"] + == "extra fields not permitted" ) def test_post_email_config_with_not_supported_service_type( @@ -179,7 +180,7 @@ def test_post_email_config_missing_detail( assert response.status_code == 422 errors = response.json()["detail"] assert "details" in errors[0]["loc"] - assert errors[0]["msg"] == "[\"field required ('domain',)\"]" + assert errors[0]["msg"] == "field required" def test_post_email_config_already_exists( self, diff --git a/tests/ops/service/email/email_dispatch_service_test.py b/tests/ops/service/email/email_dispatch_service_test.py index b1d52c1ec..eae86ace1 100644 --- a/tests/ops/service/email/email_dispatch_service_test.py +++ b/tests/ops/service/email/email_dispatch_service_test.py @@ -2,6 +2,9 @@ from unittest.mock import Mock import pytest +import requests.exceptions +import requests_mock +from fastapi import HTTPException from sqlalchemy.orm import Session from fidesops.ops.common_exceptions import EmailDispatchException @@ -87,3 +90,28 @@ def test_email_dispatch_mailgun_config_no_secrets( mock_mailgun_dispatcher.assert_not_called() email_config.delete(db) + + +def test_email_dispatch_mailgun_failed_email(db: Session, email_config) -> None: + with requests_mock.Mocker() as mock_response: + mock_response.post( + f"https://api.mailgun.net/{email_config.details[EmailServiceDetails.API_VERSION.value]}/{email_config.details[EmailServiceDetails.DOMAIN.value]}/messages", + json={ + "message": "Rejected: IP can’t be used to send the message", + "id": "<20111114174239.25659.5817@samples.mailgun.org>", + }, + status_code=403, + ) + with pytest.raises(EmailDispatchException) as exc: + dispatch_email( + db=db, + action_type=EmailActionType.SUBJECT_IDENTITY_VERIFICATION, + to_email="test@email.com", + email_body_params=SubjectIdentityVerificationBodyParams( + access_code="2348" + ), + ) + assert ( + exc.value.args[0] + == "Email failed to send due to: Email failed to send with status code 403" + ) From 1a0c619b2becf4308b313058aacc979ec26e2db2 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Fri, 19 Aug 2022 15:23:57 -0500 Subject: [PATCH 4/4] pylint --- src/fidesops/ops/schemas/email/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fidesops/ops/schemas/email/email.py b/src/fidesops/ops/schemas/email/email.py index a5413e3e7..a13691ead 100644 --- a/src/fidesops/ops/schemas/email/email.py +++ b/src/fidesops/ops/schemas/email/email.py @@ -1,7 +1,7 @@ from enum import Enum from typing import Any, Dict, Optional, Union -from pydantic import BaseModel, Extra, ValidationError, validator +from pydantic import BaseModel, Extra from fidesops.ops.schemas import Msg from fidesops.ops.schemas.shared_schemas import FidesOpsKey