From 1e2181c878f4553938e0c4d5396a220e611198d6 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Tue, 13 Sep 2022 11:10:32 -0500 Subject: [PATCH 01/14] adds email for privacy request review --- data/config/fidesops.toml | 1 + .../docs/guides/configuration_reference.md | 2 + .../docs/guides/email_communications.md | 1 + docs/fidesops/docs/guides/privacy_requests.md | 4 ++ fidesops.toml | 1 + .../v1/endpoints/privacy_request_endpoints.py | 43 +++++++++++++++++-- src/fidesops/ops/core/config.py | 1 + .../ops/email_templates/get_email_template.py | 7 ++- .../ops/email_templates/template_names.py | 2 + .../privacy_request_review_approve.html | 14 ++++++ .../privacy_request_review_deny.html | 20 +++++++++ src/fidesops/ops/schemas/email/email.py | 8 ++++ .../service/email/email_dispatch_service.py | 16 +++++++ .../test_privacy_request_endpoints.py | 26 ++++++++++- tests/ops/conftest.py | 9 ++++ 15 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 src/fidesops/ops/email_templates/templates/privacy_request_review_approve.html create mode 100644 src/fidesops/ops/email_templates/templates/privacy_request_review_deny.html diff --git a/data/config/fidesops.toml b/data/config/fidesops.toml index 6dd94aee2..54561cdaa 100644 --- a/data/config/fidesops.toml +++ b/data/config/fidesops.toml @@ -41,3 +41,4 @@ enabled = true [notifications] send_request_completion_notification = true +send_request_review_notification = true diff --git a/docs/fidesops/docs/guides/configuration_reference.md b/docs/fidesops/docs/guides/configuration_reference.md index d2b0fae60..64847e2b7 100644 --- a/docs/fidesops/docs/guides/configuration_reference.md +++ b/docs/fidesops/docs/guides/configuration_reference.md @@ -64,6 +64,7 @@ The `fidesops.toml` file should specify the following variables: |`enabled` | `FIDESOPS__ADMIN_UI__ENABLED` | bool | False | True | Toggle whether the Admin UI is served from `/` | Fidesops Notification Variables|---|---|---|---|---| |`send_request_completion_notification` | `FIDESOPS__NOTIFICATIONS__SEND_REQUEST_COMPLETION_NOTIFICATION` | bool | True | True | Whether a notification will be sent to data subjects upon privacy request completion +|`send_request_review_notification` | `FIDESOPS__NOTIFICATIONS__SEND_REQUEST_REVIEW_NOTIFICATION` | bool | True | True | Whether a notification will be sent to data subjects upon privacy request review ### An example `fidesops.toml` configuration file @@ -118,6 +119,7 @@ enabled = true [notifications] send_request_completion_notification = true +send_request_review_notification = true ``` Note: The configuration is case-sensitive, so the variables must be specified in `lowercase`. diff --git a/docs/fidesops/docs/guides/email_communications.md b/docs/fidesops/docs/guides/email_communications.md index 2a05da4e3..213e0769a 100644 --- a/docs/fidesops/docs/guides/email_communications.md +++ b/docs/fidesops/docs/guides/email_communications.md @@ -7,6 +7,7 @@ Supported modes of use: - Subject Identity Verification - sends a verification code to the user's email address prior to processing a subject request. For more information on identity verification, see the [Privacy Requests](privacy_requests.md#subject-identity-verification) guide. - Erasure Request Email Fulfillment - sends an email to configured third parties to process erasures for a given data subject. See [creating email Connectors](#email-third-party-services) for more information. +- Privacy Request Review Notification - sends an email to user's email address upon privacy request review, including rejection reason if applicable. - Privacy Request Completion Notification - sends an email to user's email address with privacy request completion notification, including a download link to data package, for access requests. For more information on request completion notification, see the [Privacy Requests](privacy_requests.md#request-completion-notification) guide. ## Prerequisites diff --git a/docs/fidesops/docs/guides/privacy_requests.md b/docs/fidesops/docs/guides/privacy_requests.md index 6f5fe10de..147117c32 100644 --- a/docs/fidesops/docs/guides/privacy_requests.md +++ b/docs/fidesops/docs/guides/privacy_requests.md @@ -64,6 +64,10 @@ to your users. If using a custom privacy center, ensure that you intake an email For security purposes, the data package download link is a one-time link and expires in 24 hrs by default. To change TTL, update the `subject_request_download_link_ttl_seconds` variable in your `fidesops.toml`. +### Request Review + +An email will be sent to users to notify them when their privacy request has been reviewed. If the privacy request was rejected, the email will include rejection reason. + ## Approve and deny Privacy Requests diff --git a/fidesops.toml b/fidesops.toml index e81c12d72..3ba3b29bb 100644 --- a/fidesops.toml +++ b/fidesops.toml @@ -48,3 +48,4 @@ enabled = true [notifications] send_request_completion_notification = false +send_request_review_notification = false diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index e61709522..f456230bf 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -63,7 +63,7 @@ IdentityVerificationException, NoCachedManualWebhookEntry, TraversalError, - ValidationError, + ValidationError, IdentityNotFoundException, ) from fidesops.ops.core.config import config from fidesops.ops.graph.config import CollectionAddress @@ -77,7 +77,7 @@ ExecutionLog, PrivacyRequest, PrivacyRequestStatus, - ProvidedIdentity, + ProvidedIdentity, ProvidedIdentityType, ) from fidesops.ops.schemas.dataset import ( CollectionAddressResponse, @@ -85,7 +85,7 @@ ) from fidesops.ops.schemas.email.email import ( EmailActionType, - SubjectIdentityVerificationBodyParams, + SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, ) from fidesops.ops.schemas.external_https import PrivacyRequestResumeFormat from fidesops.ops.schemas.privacy_request import ( @@ -240,6 +240,7 @@ async def create_privacy_request( "message": "", }, ) + # do we want approval email sent for automatic approvals? queue_privacy_request(privacy_request.id) except EmailDispatchException as exc: kwargs["privacy_request_id"] = privacy_request.id @@ -1062,6 +1063,28 @@ def review_privacy_request( ) +def _send_privacy_request_review_email_to_user( + db: Session, action_type: EmailActionType, email: Optional[str], rejection_reason: Optional[str] +) -> None: + """Helper method to send review notification email to user, shared between approve and deny""" + if not email: + logger.error( + IdentityNotFoundException( + "Identity email was not found, so request review email could not be sent." + ) + ) + try: + dispatch_email( + db=db, + action_type=action_type, + to_email=email, + email_body_params=RequestReviewDenyBodyParams(rejection_reason=rejection_reason) if action_type is EmailActionType.PRIVACY_REQUEST_REVIEW_DENY else None, + ) + except EmailDispatchException as e: + # this failure isn't fatal to privacy request + logger.error(e) + + @router.post( PRIVACY_REQUEST_VERIFY_IDENTITY, status_code=HTTP_200_OK, @@ -1130,6 +1153,13 @@ def _approve_request(privacy_request: PrivacyRequest) -> None: privacy_request.reviewed_at = datetime.utcnow() privacy_request.reviewed_by = user_id privacy_request.save(db=db) + if config.notifications.send_request_review_notification: + _send_privacy_request_review_email_to_user( + db=db, + action_type=EmailActionType.PRIVACY_REQUEST_REVIEW_APPROVE, + email=privacy_request.get_cached_identity_data().get(ProvidedIdentityType.email.value), + rejection_reason=None + ) AuditLog.create( db=db, @@ -1180,6 +1210,13 @@ def _deny_request( "message": privacy_requests.reason, }, ) + if config.notifications.send_request_review_notification: + _send_privacy_request_review_email_to_user( + db=db, + action_type=EmailActionType.PRIVACY_REQUEST_REVIEW_DENY, + email=privacy_request.get_cached_identity_data().get(ProvidedIdentityType.email.value), + rejection_reason=privacy_requests.reason + ) privacy_request.status = PrivacyRequestStatus.denied privacy_request.reviewed_at = datetime.utcnow() privacy_request.reviewed_by = user_id diff --git a/src/fidesops/ops/core/config.py b/src/fidesops/ops/core/config.py index 706b5d060..a3eae3d3f 100644 --- a/src/fidesops/ops/core/config.py +++ b/src/fidesops/ops/core/config.py @@ -154,6 +154,7 @@ class FidesopsNotificationSettings(FidesSettings): """Configuration settings for data subject and/or data processor notifications""" send_request_completion_notification: Optional[bool] = True + send_request_review_notification: Optional[bool] = True class Config: env_prefix = "FIDESOPS__NOTIFICATIONS__" diff --git a/src/fidesops/ops/email_templates/get_email_template.py b/src/fidesops/ops/email_templates/get_email_template.py index 2ecf05af2..eb3e39bca 100644 --- a/src/fidesops/ops/email_templates/get_email_template.py +++ b/src/fidesops/ops/email_templates/get_email_template.py @@ -8,7 +8,8 @@ EMAIL_ERASURE_REQUEST_FULFILLMENT, PRIVACY_REQUEST_COMPLETE_ACCESS_TEMPLATE, PRIVACY_REQUEST_COMPLETE_DELETION_TEMPLATE, - SUBJECT_IDENTITY_VERIFICATION_TEMPLATE, + SUBJECT_IDENTITY_VERIFICATION_TEMPLATE, PRIVACY_REQUEST_REVIEW_DENY_TEMPLATE, + PRIVACY_REQUEST_REVIEW_APPROVE_TEMPLATE, ) from fidesops.ops.schemas.email.email import EmailActionType @@ -31,6 +32,10 @@ def get_email_template(action_type: EmailActionType) -> Template: return template_env.get_template(PRIVACY_REQUEST_COMPLETE_ACCESS_TEMPLATE) if action_type == EmailActionType.PRIVACY_REQUEST_COMPLETE_DELETION: return template_env.get_template(PRIVACY_REQUEST_COMPLETE_DELETION_TEMPLATE) + if action_type == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY: + return template_env.get_template(PRIVACY_REQUEST_REVIEW_DENY_TEMPLATE) + if action_type == EmailActionType.PRIVACY_REQUEST_REVIEW_APPROVE: + return template_env.get_template(PRIVACY_REQUEST_REVIEW_APPROVE_TEMPLATE) logger.error("No corresponding template linked to the %s", action_type) raise EmailTemplateUnhandledActionType( diff --git a/src/fidesops/ops/email_templates/template_names.py b/src/fidesops/ops/email_templates/template_names.py index afcf3cda6..4ff602ac7 100644 --- a/src/fidesops/ops/email_templates/template_names.py +++ b/src/fidesops/ops/email_templates/template_names.py @@ -2,3 +2,5 @@ EMAIL_ERASURE_REQUEST_FULFILLMENT = "erasure_request_email_fulfillment.html" PRIVACY_REQUEST_COMPLETE_DELETION_TEMPLATE = "privacy_request_complete_deletion.html" PRIVACY_REQUEST_COMPLETE_ACCESS_TEMPLATE = "privacy_request_complete_access.html" +PRIVACY_REQUEST_REVIEW_DENY_TEMPLATE = "privacy_request_review_deny.html" +PRIVACY_REQUEST_REVIEW_APPROVE_TEMPLATE = "privacy_request_review_approve.html" diff --git a/src/fidesops/ops/email_templates/templates/privacy_request_review_approve.html b/src/fidesops/ops/email_templates/templates/privacy_request_review_approve.html new file mode 100644 index 000000000..7b56434a1 --- /dev/null +++ b/src/fidesops/ops/email_templates/templates/privacy_request_review_approve.html @@ -0,0 +1,14 @@ + + + + + Privacy Request Approved + + +
+

+ Your privacy request has been approved and is currently processing. +

+
+ + \ No newline at end of file diff --git a/src/fidesops/ops/email_templates/templates/privacy_request_review_deny.html b/src/fidesops/ops/email_templates/templates/privacy_request_review_deny.html new file mode 100644 index 000000000..f4838059b --- /dev/null +++ b/src/fidesops/ops/email_templates/templates/privacy_request_review_deny.html @@ -0,0 +1,20 @@ + + + + + Privacy Request Denied + + +
+ {% if rejection_reason %} +

+ Your privacy request has been denied for the following reason: {{rejection_reason}} +

+ {% else %} +

+ Your privacy request has been denied. +

+ {% endif %} +
+ + \ No newline at end of file diff --git a/src/fidesops/ops/schemas/email/email.py b/src/fidesops/ops/schemas/email/email.py index 05ad08e09..da07172dc 100644 --- a/src/fidesops/ops/schemas/email/email.py +++ b/src/fidesops/ops/schemas/email/email.py @@ -22,6 +22,8 @@ class EmailActionType(Enum): EMAIL_ERASURE_REQUEST_FULFILLMENT = "email_erasure_fulfillment" PRIVACY_REQUEST_COMPLETE_ACCESS = "privacy_request_complete_access" PRIVACY_REQUEST_COMPLETE_DELETION = "privacy_request_complete_deletion" + PRIVACY_REQUEST_REVIEW_DENY = "privacy_request_review_deny" + PRIVACY_REQUEST_REVIEW_APPROVE = "privacy_request_review_approve" class EmailTemplateBodyParams(Enum): @@ -49,6 +51,12 @@ class AccessRequestCompleteBodyParams(BaseModel): download_links: List[str] +class RequestReviewDenyBodyParams(BaseModel): + """Body params required for privacy request review deny email template""" + + rejection_reason: Optional[str] + + class EmailForActionType(BaseModel): """Email details that depend on action type""" diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index 2784897b1..5b9032e4f 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -93,6 +93,22 @@ def _build_email( subject="Your data has been deleted", body=base_template.render(), ) + if action_type == EmailActionType.PRIVACY_REQUEST_REVIEW_APPROVE: + base_template = get_email_template(action_type) + return EmailForActionType( + subject="Your request has been approved", + body=base_template.render(), + ) + if action_type == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY: + base_template = get_email_template(action_type) + return EmailForActionType( + subject="Your request has been denied", + body=base_template.render( + { + "rejection_reason": body_params.rejection_reason + } + ), + ) logger.error("Email action type %s is not implemented", action_type) raise EmailDispatchException(f"Email action type {action_type} is not implemented") diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index 5aace5cfd..3a10fface 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -65,7 +65,7 @@ from fidesops.ops.schemas.dataset import DryRunDatasetResponse from fidesops.ops.schemas.email.email import ( EmailActionType, - SubjectIdentityVerificationBodyParams, + SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, ) from fidesops.ops.schemas.masking.masking_secrets import SecretType from fidesops.ops.schemas.policy import PolicyResponse @@ -1868,6 +1868,14 @@ class TestDenyPrivacyRequest: def url(self, db, privacy_request): return V1_URL_PREFIX + PRIVACY_REQUEST_DENY + @pytest.fixture(autouse=True, scope="function") + def privacy_request_review_email_notification_enabled(self): + """Enable request review email""" + original_value = config.notifications.send_request_review_notification + config.notifications.send_request_review_notification = True + yield + config.notifications.send_request_review_notification = original_value + def test_deny_privacy_request_not_authenticated(self, url, api_client): response = api_client.patch(url) assert response.status_code == 401 @@ -1924,8 +1932,12 @@ def test_deny_completed_privacy_request( @mock.patch( "fidesops.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" ) + @mock.patch( + "fidesops.ops.api.v1.endpoints.privacy_request_endpoints.dispatch_email" + ) def test_deny_privacy_request_without_denial_reason( self, + mock_dispatch_email, submit_mock, db, url, @@ -1965,6 +1977,10 @@ def test_deny_privacy_request_without_denial_reason( & (AuditLog.user_id == user.id) ), ).first() + call_args = mock_dispatch_email.call_args[1] + assert call_args["action_type"] == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY + assert call_args["to_email"] == "test@example.com" + assert call_args["email_body_params"] == RequestReviewDenyBodyParams(rejection_reason=None) assert denial_audit_log.message is None @@ -1975,8 +1991,12 @@ def test_deny_privacy_request_without_denial_reason( @mock.patch( "fidesops.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" ) + @mock.patch( + "fidesops.ops.api.v1.endpoints.privacy_request_endpoints.dispatch_email" + ) def test_deny_privacy_request_with_denial_reason( self, + mock_dispatch_email, submit_mock, db, url, @@ -2016,6 +2036,10 @@ def test_deny_privacy_request_with_denial_reason( & (AuditLog.user_id == user.id) ), ).first() + call_args = mock_dispatch_email.call_args[1] + assert call_args["action_type"] == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY + assert call_args["to_email"] == "test@example.com" + assert call_args["email_body_params"] == RequestReviewDenyBodyParams(rejection_reason=denial_reason) assert denial_audit_log.message == denial_reason diff --git a/tests/ops/conftest.py b/tests/ops/conftest.py index 439396cee..0f9464f63 100644 --- a/tests/ops/conftest.py +++ b/tests/ops/conftest.py @@ -264,6 +264,15 @@ def privacy_request_complete_email_notification_disabled(): config.notifications.send_request_completion_notification = original_value +@pytest.fixture(autouse=True, scope="function") +def privacy_request_review_email_notification_disabled(): + """Disable request review email for most tests unless overridden""" + original_value = config.notifications.send_request_review_notification + config.notifications.send_request_review_notification = False + yield + config.notifications.send_request_review_notification = original_value + + @pytest.fixture(scope="session", autouse=True) def event_loop(): try: From 8e56200d3141dde5b3de771429d22066c2373aa6 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Tue, 13 Sep 2022 12:25:14 -0500 Subject: [PATCH 02/14] formatting --- .../v1/endpoints/privacy_request_endpoints.py | 32 +++++++++++++------ .../ops/email_templates/get_email_template.py | 3 +- .../service/email/email_dispatch_service.py | 4 +-- .../test_privacy_request_endpoints.py | 11 +++++-- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index f456230bf..cab082ba4 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -60,10 +60,11 @@ from fidesops.ops.common_exceptions import ( EmailDispatchException, FunctionalityNotConfigured, + IdentityNotFoundException, IdentityVerificationException, NoCachedManualWebhookEntry, TraversalError, - ValidationError, IdentityNotFoundException, + ValidationError, ) from fidesops.ops.core.config import config from fidesops.ops.graph.config import CollectionAddress @@ -77,7 +78,8 @@ ExecutionLog, PrivacyRequest, PrivacyRequestStatus, - ProvidedIdentity, ProvidedIdentityType, + ProvidedIdentity, + ProvidedIdentityType, ) from fidesops.ops.schemas.dataset import ( CollectionAddressResponse, @@ -85,7 +87,8 @@ ) from fidesops.ops.schemas.email.email import ( EmailActionType, - SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, + RequestReviewDenyBodyParams, + SubjectIdentityVerificationBodyParams, ) from fidesops.ops.schemas.external_https import PrivacyRequestResumeFormat from fidesops.ops.schemas.privacy_request import ( @@ -1064,7 +1067,10 @@ def review_privacy_request( def _send_privacy_request_review_email_to_user( - db: Session, action_type: EmailActionType, email: Optional[str], rejection_reason: Optional[str] + db: Session, + action_type: EmailActionType, + email: Optional[str], + rejection_reason: Optional[str], ) -> None: """Helper method to send review notification email to user, shared between approve and deny""" if not email: @@ -1078,7 +1084,11 @@ def _send_privacy_request_review_email_to_user( db=db, action_type=action_type, to_email=email, - email_body_params=RequestReviewDenyBodyParams(rejection_reason=rejection_reason) if action_type is EmailActionType.PRIVACY_REQUEST_REVIEW_DENY else None, + email_body_params=RequestReviewDenyBodyParams( + rejection_reason=rejection_reason + ) + if action_type is EmailActionType.PRIVACY_REQUEST_REVIEW_DENY + else None, ) except EmailDispatchException as e: # this failure isn't fatal to privacy request @@ -1157,8 +1167,10 @@ def _approve_request(privacy_request: PrivacyRequest) -> None: _send_privacy_request_review_email_to_user( db=db, action_type=EmailActionType.PRIVACY_REQUEST_REVIEW_APPROVE, - email=privacy_request.get_cached_identity_data().get(ProvidedIdentityType.email.value), - rejection_reason=None + email=privacy_request.get_cached_identity_data().get( + ProvidedIdentityType.email.value + ), + rejection_reason=None, ) AuditLog.create( @@ -1214,8 +1226,10 @@ def _deny_request( _send_privacy_request_review_email_to_user( db=db, action_type=EmailActionType.PRIVACY_REQUEST_REVIEW_DENY, - email=privacy_request.get_cached_identity_data().get(ProvidedIdentityType.email.value), - rejection_reason=privacy_requests.reason + email=privacy_request.get_cached_identity_data().get( + ProvidedIdentityType.email.value + ), + rejection_reason=privacy_requests.reason, ) privacy_request.status = PrivacyRequestStatus.denied privacy_request.reviewed_at = datetime.utcnow() diff --git a/src/fidesops/ops/email_templates/get_email_template.py b/src/fidesops/ops/email_templates/get_email_template.py index eb3e39bca..3ff50420a 100644 --- a/src/fidesops/ops/email_templates/get_email_template.py +++ b/src/fidesops/ops/email_templates/get_email_template.py @@ -8,8 +8,9 @@ EMAIL_ERASURE_REQUEST_FULFILLMENT, PRIVACY_REQUEST_COMPLETE_ACCESS_TEMPLATE, PRIVACY_REQUEST_COMPLETE_DELETION_TEMPLATE, - SUBJECT_IDENTITY_VERIFICATION_TEMPLATE, PRIVACY_REQUEST_REVIEW_DENY_TEMPLATE, PRIVACY_REQUEST_REVIEW_APPROVE_TEMPLATE, + PRIVACY_REQUEST_REVIEW_DENY_TEMPLATE, + SUBJECT_IDENTITY_VERIFICATION_TEMPLATE, ) from fidesops.ops.schemas.email.email import EmailActionType diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index 5b9032e4f..0e77da966 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -104,9 +104,7 @@ def _build_email( return EmailForActionType( subject="Your request has been denied", body=base_template.render( - { - "rejection_reason": body_params.rejection_reason - } + {"rejection_reason": body_params.rejection_reason} ), ) logger.error("Email action type %s is not implemented", action_type) diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index 3a10fface..bb908cedf 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -65,7 +65,8 @@ from fidesops.ops.schemas.dataset import DryRunDatasetResponse from fidesops.ops.schemas.email.email import ( EmailActionType, - SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, + RequestReviewDenyBodyParams, + SubjectIdentityVerificationBodyParams, ) from fidesops.ops.schemas.masking.masking_secrets import SecretType from fidesops.ops.schemas.policy import PolicyResponse @@ -1980,7 +1981,9 @@ def test_deny_privacy_request_without_denial_reason( call_args = mock_dispatch_email.call_args[1] assert call_args["action_type"] == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY assert call_args["to_email"] == "test@example.com" - assert call_args["email_body_params"] == RequestReviewDenyBodyParams(rejection_reason=None) + assert call_args["email_body_params"] == RequestReviewDenyBodyParams( + rejection_reason=None + ) assert denial_audit_log.message is None @@ -2039,7 +2042,9 @@ def test_deny_privacy_request_with_denial_reason( call_args = mock_dispatch_email.call_args[1] assert call_args["action_type"] == EmailActionType.PRIVACY_REQUEST_REVIEW_DENY assert call_args["to_email"] == "test@example.com" - assert call_args["email_body_params"] == RequestReviewDenyBodyParams(rejection_reason=denial_reason) + assert call_args["email_body_params"] == RequestReviewDenyBodyParams( + rejection_reason=denial_reason + ) assert denial_audit_log.message == denial_reason From 93d876285730a428c55a7dc6f5f5a9d684e89ea3 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Tue, 13 Sep 2022 12:27:31 -0500 Subject: [PATCH 03/14] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0bfa3699..bdece3cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ The types of changes are: * Added erasure endpoints for Shopify connector [#1289](https://github.com/ethyca/fidesops/pull/1289) * Adds ability to send email notification upon privacy request completion [#1282](https://github.com/ethyca/fidesops/pull/1282) * Enable new manual webhooks in privacy request execution [#1285](https://github.com/ethyca/fidesops/pull/1285) +* * Adds ability to send email notification upon privacy request review [#1306](https://github.com/ethyca/fidesops/pull/1306) * Added human readable label to ConnectionType endpoint [#1297](https://github.com/ethyca/fidesops/pull/1297) From 9f93cb929914ef4923eac768c1e13da9e18b9f3d Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 12:24:11 -0500 Subject: [PATCH 04/14] mask potential pii in emaildispatchexceptions --- .../ops/api/v1/endpoints/privacy_request_endpoints.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index cab082ba4..555d89ff5 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -1090,9 +1090,9 @@ def _send_privacy_request_review_email_to_user( if action_type is EmailActionType.PRIVACY_REQUEST_REVIEW_DENY else None, ) - except EmailDispatchException as e: + except EmailDispatchException as exc: # this failure isn't fatal to privacy request - logger.error(e) + logger.info("Email dispatch failed with exception %s", Pii(exc)) @router.post( From 3962654ce8419485439d4e451090716d0c917483 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 12:35:30 -0500 Subject: [PATCH 05/14] missed import in conflict resolution --- src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index 8eb53193e..cbeefda79 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -106,7 +106,7 @@ RowCountRequest, VerificationCode, ) -from fidesops.ops.service.email.email_dispatch_service import dispatch_email_task +from fidesops.ops.service.email.email_dispatch_service import dispatch_email_task, dispatch_email from fidesops.ops.service.privacy_request.request_runner_service import ( generate_id_verification_code, queue_privacy_request, From 7a0967acd0fbf75e08cef7a09f8c9af35dd8aaa0 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 12:38:26 -0500 Subject: [PATCH 06/14] isort --- .../ops/api/v1/endpoints/privacy_request_endpoints.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index cbeefda79..e4846dac6 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -106,7 +106,10 @@ RowCountRequest, VerificationCode, ) -from fidesops.ops.service.email.email_dispatch_service import dispatch_email_task, dispatch_email +from fidesops.ops.service.email.email_dispatch_service import ( + dispatch_email, + dispatch_email_task, +) from fidesops.ops.service.privacy_request.request_runner_service import ( generate_id_verification_code, queue_privacy_request, From 0242cbc59a885763d54bcac5f82eff006ac8f856 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 14:17:35 -0500 Subject: [PATCH 07/14] add type to dispatch _email --- src/fidesops/ops/service/email/email_dispatch_service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index cd44fe70d..4e613048c 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -16,7 +16,7 @@ EmailServiceSecrets, EmailServiceType, FidesopsEmail, - SubjectIdentityVerificationBodyParams, + SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, ) from fidesops.ops.tasks import DatabaseTask, celery_app from fidesops.ops.util.logger import Pii @@ -51,6 +51,7 @@ def dispatch_email( Union[ AccessRequestCompleteBodyParams, SubjectIdentityVerificationBodyParams, + RequestReviewDenyBodyParams, List[CheckpointActionRequired], ] ] = None, From f86b35514a8eb4f046f7f774c0869c0f0ddaa7da Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 14:21:49 -0500 Subject: [PATCH 08/14] format --- src/fidesops/ops/service/email/email_dispatch_service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index 4e613048c..ef2ef1c79 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -16,7 +16,8 @@ EmailServiceSecrets, EmailServiceType, FidesopsEmail, - SubjectIdentityVerificationBodyParams, RequestReviewDenyBodyParams, + RequestReviewDenyBodyParams, + SubjectIdentityVerificationBodyParams, ) from fidesops.ops.tasks import DatabaseTask, celery_app from fidesops.ops.util.logger import Pii From 0dc2525eae2c6586c895ccf1a6ab3aa493740c02 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 16:34:40 -0500 Subject: [PATCH 09/14] adds more test cases for approval flow, refactor order of events for approve/deny --- .../v1/endpoints/privacy_request_endpoints.py | 28 +++++++++---------- .../test_privacy_request_endpoints.py | 27 +++++++++++++++++- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index e4846dac6..a1466ff2a 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -249,7 +249,6 @@ async def create_privacy_request( "message": "", }, ) - # do we want approval email sent for automatic approvals? queue_privacy_request(privacy_request.id) except EmailDispatchException as exc: kwargs["privacy_request_id"] = privacy_request.id @@ -1177,6 +1176,15 @@ def _approve_request(privacy_request: PrivacyRequest) -> None: privacy_request.reviewed_at = datetime.utcnow() privacy_request.reviewed_by = user_id privacy_request.save(db=db) + AuditLog.create( + db=db, + data={ + "user_id": user_id, + "privacy_request_id": privacy_request.id, + "action": AuditLogAction.approved, + "message": "", + }, + ) if config.notifications.send_request_review_notification: _send_privacy_request_review_email_to_user( db=db, @@ -1187,15 +1195,6 @@ def _approve_request(privacy_request: PrivacyRequest) -> None: rejection_reason=None, ) - AuditLog.create( - db=db, - data={ - "user_id": user_id, - "privacy_request_id": privacy_request.id, - "action": AuditLogAction.approved, - "message": "", - }, - ) queue_privacy_request(privacy_request_id=privacy_request.id) return review_privacy_request( @@ -1226,7 +1225,10 @@ def _deny_request( privacy_request: PrivacyRequest, ) -> None: """Method for how to process requests - denied""" - + privacy_request.status = PrivacyRequestStatus.denied + privacy_request.reviewed_at = datetime.utcnow() + privacy_request.reviewed_by = user_id + privacy_request.save(db=db) AuditLog.create( db=db, data={ @@ -1245,10 +1247,6 @@ def _deny_request( ), rejection_reason=privacy_requests.reason, ) - privacy_request.status = PrivacyRequestStatus.denied - privacy_request.reviewed_at = datetime.utcnow() - privacy_request.reviewed_by = user_id - privacy_request.save(db=db) return review_privacy_request( db=db, diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index e32b0c492..05f4e1b9c 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -1684,6 +1684,14 @@ class TestApprovePrivacyRequest: def url(self, db, privacy_request): return V1_URL_PREFIX + PRIVACY_REQUEST_APPROVE + @pytest.fixture(autouse=True, scope="function") + def privacy_request_review_email_notification_enabled(self): + """Enable request review email""" + original_value = config.notifications.send_request_review_notification + config.notifications.send_request_review_notification = True + yield + config.notifications.send_request_review_notification = original_value + def test_approve_privacy_request_not_authenticated(self, url, api_client): response = api_client.patch(url) assert response.status_code == 401 @@ -1784,8 +1792,12 @@ def test_approve_privacy_request_no_user_on_client( @mock.patch( "fidesops.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" ) + @mock.patch( + "fidesops.ops.api.v1.endpoints.privacy_request_endpoints.dispatch_email" + ) def test_approve_privacy_request( self, + mock_dispatch_email, submit_mock, db, url, @@ -1821,14 +1833,19 @@ def test_approve_privacy_request( assert response_body["succeeded"][0]["reviewed_by"] == user.id assert submit_mock.called + assert not mock_dispatch_email.called privacy_request.delete(db) @mock.patch( "fidesops.ops.service.privacy_request.request_runner_service.run_privacy_request.delay" ) - def test_approve_privacy_request_creates_audit_log( + @mock.patch( + "fidesops.ops.api.v1.endpoints.privacy_request_endpoints.dispatch_email" + ) + def test_approve_privacy_request_creates_audit_log_and_sends_email( self, + mock_dispatch_email, submit_mock, db, url, @@ -1836,6 +1853,7 @@ def test_approve_privacy_request_creates_audit_log( generate_auth_header, user, privacy_request_status_pending, + privacy_request_review_email_notification_enabled, ): payload = { JWE_PAYLOAD_SCOPES: user.client.scopes, @@ -1863,6 +1881,13 @@ def test_approve_privacy_request_creates_audit_log( approval_audit_log.delete(db) + call_args = mock_dispatch_email.call_args[1] + assert ( + call_args["action_type"] == EmailActionType.PRIVACY_REQUEST_REVIEW_APPROVE + ) + assert call_args["to_email"] == "test@example.com" + assert call_args["email_body_params"] is None + class TestDenyPrivacyRequest: @pytest.fixture(scope="function") From ed3e77dd5cf0efb060e7d0ac5e5c6261f383edaa Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Wed, 14 Sep 2022 16:41:44 -0500 Subject: [PATCH 10/14] let email dispatch service mask pii --- src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py index a1466ff2a..e2e8a03c0 100644 --- a/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fidesops/ops/api/v1/endpoints/privacy_request_endpoints.py @@ -1105,7 +1105,7 @@ def _send_privacy_request_review_email_to_user( ) except EmailDispatchException as exc: # this failure isn't fatal to privacy request - logger.info("Email dispatch failed with exception %s", Pii(exc)) + logger.info("Email dispatch failed with exception %s", exc) @router.post( From aecbdd8153a7b44db9dd0eefe4295c9f0e21de49 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 15 Sep 2022 12:25:20 -0500 Subject: [PATCH 11/14] remove autouse from fixture --- tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index 05f4e1b9c..cfc38470a 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -1684,7 +1684,7 @@ class TestApprovePrivacyRequest: def url(self, db, privacy_request): return V1_URL_PREFIX + PRIVACY_REQUEST_APPROVE - @pytest.fixture(autouse=True, scope="function") + @pytest.fixture(scope="function") def privacy_request_review_email_notification_enabled(self): """Enable request review email""" original_value = config.notifications.send_request_review_notification From dfa64c9da02657fc27f4633e43cf8ae55eabd74b Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 15 Sep 2022 12:40:36 -0500 Subject: [PATCH 12/14] pylint disable --- src/fidesops/ops/email_templates/get_email_template.py | 2 +- src/fidesops/ops/service/email/email_dispatch_service.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fidesops/ops/email_templates/get_email_template.py b/src/fidesops/ops/email_templates/get_email_template.py index 7ecf1959f..93039515d 100644 --- a/src/fidesops/ops/email_templates/get_email_template.py +++ b/src/fidesops/ops/email_templates/get_email_template.py @@ -25,7 +25,7 @@ ) -def get_email_template(action_type: EmailActionType) -> Template: +def get_email_template(action_type: EmailActionType) -> Template: # pylint: disable=too-many-return-statements if action_type == EmailActionType.SUBJECT_IDENTITY_VERIFICATION: return template_env.get_template(SUBJECT_IDENTITY_VERIFICATION_TEMPLATE) if action_type == EmailActionType.EMAIL_ERASURE_REQUEST_FULFILLMENT: diff --git a/src/fidesops/ops/service/email/email_dispatch_service.py b/src/fidesops/ops/service/email/email_dispatch_service.py index 768dc7e17..2c014e133 100644 --- a/src/fidesops/ops/service/email/email_dispatch_service.py +++ b/src/fidesops/ops/service/email/email_dispatch_service.py @@ -87,7 +87,7 @@ def dispatch_email( ) -def _build_email( +def _build_email( # pylint: disable=too-many-return-statements action_type: EmailActionType, body_params: Any, ) -> EmailForActionType: From d5603d6692a107881c5cd234cb2ba9e496f8a1d0 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 15 Sep 2022 12:42:53 -0500 Subject: [PATCH 13/14] format --- src/fidesops/ops/email_templates/get_email_template.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fidesops/ops/email_templates/get_email_template.py b/src/fidesops/ops/email_templates/get_email_template.py index 93039515d..15bc3e2f5 100644 --- a/src/fidesops/ops/email_templates/get_email_template.py +++ b/src/fidesops/ops/email_templates/get_email_template.py @@ -25,7 +25,9 @@ ) -def get_email_template(action_type: EmailActionType) -> Template: # pylint: disable=too-many-return-statements +def get_email_template( + action_type: EmailActionType, +) -> Template: # pylint: disable=too-many-return-statements if action_type == EmailActionType.SUBJECT_IDENTITY_VERIFICATION: return template_env.get_template(SUBJECT_IDENTITY_VERIFICATION_TEMPLATE) if action_type == EmailActionType.EMAIL_ERASURE_REQUEST_FULFILLMENT: From 5521065e20fa5f54bb2114064952afe25dadf42e Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 15 Sep 2022 12:49:26 -0500 Subject: [PATCH 14/14] move pylint ignore to appropriate line --- src/fidesops/ops/email_templates/get_email_template.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fidesops/ops/email_templates/get_email_template.py b/src/fidesops/ops/email_templates/get_email_template.py index 15bc3e2f5..c5e967d5a 100644 --- a/src/fidesops/ops/email_templates/get_email_template.py +++ b/src/fidesops/ops/email_templates/get_email_template.py @@ -25,9 +25,9 @@ ) -def get_email_template( +def get_email_template( # pylint: disable=too-many-return-statements action_type: EmailActionType, -) -> Template: # pylint: disable=too-many-return-statements +) -> Template: if action_type == EmailActionType.SUBJECT_IDENTITY_VERIFICATION: return template_env.get_template(SUBJECT_IDENTITY_VERIFICATION_TEMPLATE) if action_type == EmailActionType.EMAIL_ERASURE_REQUEST_FULFILLMENT: