From c6b88200156d329167686582dc276224f6c67772 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 09:17:11 +0000 Subject: [PATCH 01/20] allow template for sendgrid if exists --- .../messaging/message_dispatch_service.py | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index e6f7673f77..afd65c1fc9 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -6,7 +6,7 @@ import requests import sendgrid from loguru import logger -from sendgrid.helpers.mail import Content, Email, Mail, To +from sendgrid.helpers.mail import Content, Email, Mail, Personalization, TemplateId, To from sqlalchemy.orm import Session from twilio.base.exceptions import TwilioRestException from twilio.rest import Client @@ -45,6 +45,7 @@ from fides.core.config.config_proxy import ConfigProxy EMAIL_JOIN_STRING = ", " +EMAIL_TEMPLATE_NAME = "fides" def check_and_dispatch_error_notifications(db: Session) -> None: @@ -378,7 +379,7 @@ def _mailgun_dispatcher( try: # Check if a fides template exists template_test = requests.get( - f"{base_url}/{messaging_config.details[MessagingServiceDetails.API_VERSION.value]}/{domain}/templates/fides", + f"{base_url}/{messaging_config.details[MessagingServiceDetails.API_VERSION.value]}/{domain}/templates/{EMAIL_TEMPLATE_NAME}", auth=( "api", messaging_config.secrets[MessagingServiceSecrets.MAILGUN_API_KEY.value], @@ -392,7 +393,7 @@ def _mailgun_dispatcher( } if template_test.status_code == 200: - data["template"] = "fides" + data["template"] = EMAIL_TEMPLATE_NAME data["h:X-Mailgun-Variables"] = json.dumps( {"fides_email_body": message.body} ) @@ -456,18 +457,27 @@ def _twilio_email_dispatcher( ) try: + sg = sendgrid.SendGridAPIClient( api_key=messaging_config.secrets[ MessagingServiceSecrets.TWILIO_API_KEY.value ] ) + template_test = _get_template_id_if_exists(sg, EMAIL_TEMPLATE_NAME) + from_email = Email( messaging_config.details[MessagingServiceDetails.TWILIO_EMAIL_FROM.value] ) to_email = To(to.strip()) subject = message.subject - content = Content("text/html", message.body) - mail = Mail(from_email, to_email, subject, content) + if template_test: + personalization = Personalization() + personalization.dynamic_template_data = {"fides_email_body": message.body} + template_id = TemplateId(template_test) + mail = Mail(from_email, to_email, personalization, template_id) + else: + content = Content("text/html", message.body) + mail = Mail(from_email, to_email, subject, content) response = sg.client.mail.send.post(request_body=mail.get()) if response.status_code >= 400: logger.error( @@ -526,3 +536,22 @@ def _twilio_sms_dispatcher( except TwilioRestException as e: logger.error("Twilio SMS failed to send: {}", Pii(str(e))) raise MessageDispatchException(f"Twilio SMS failed to send due to: {Pii(e)}") + + +def _get_template_id_if_exists( + sg: sendgrid.SendGridAPIClient, template_name: str +) -> Optional[str]: + """ + Checks to see if a SendGrid template exists for Fides, returning the id if so + """ + + params = {"generations": "legacy,dynamic", "page_size": 18} + + response = sg.client.templates.get(query_params=params) + + response_body_dict = json.loads(response.body) + + for template in response_body_dict["result"]: + if template["name"].lower() == template_name.lower(): + return template["id"] + return None From ab4ddc7b39b01a364b0b61f659227e62381f7ab3 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 14:58:59 +0000 Subject: [PATCH 02/20] reconfigure sending from a template, dynamic only --- .../api/ops/service/messaging/message_dispatch_service.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index afd65c1fc9..ef143edcb9 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -471,10 +471,12 @@ def _twilio_email_dispatcher( to_email = To(to.strip()) subject = message.subject if template_test: + mail = Mail(from_email=from_email, subject=subject) + mail.template_id = TemplateId(template_test) personalization = Personalization() personalization.dynamic_template_data = {"fides_email_body": message.body} - template_id = TemplateId(template_test) - mail = Mail(from_email, to_email, personalization, template_id) + personalization.add_email(to_email) + mail.add_personalization(personalization) else: content = Content("text/html", message.body) mail = Mail(from_email, to_email, subject, content) @@ -545,7 +547,7 @@ def _get_template_id_if_exists( Checks to see if a SendGrid template exists for Fides, returning the id if so """ - params = {"generations": "legacy,dynamic", "page_size": 18} + params = {"generations": "dynamic", "page_size": 18} response = sg.client.templates.get(query_params=params) From c6df2577bd74414e894acff6bc1514b2c587d624 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 18:14:37 +0000 Subject: [PATCH 03/20] use max page limit due to broken client pagination --- .../api/ops/service/messaging/message_dispatch_service.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index ef143edcb9..97fb676578 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -547,10 +547,11 @@ def _get_template_id_if_exists( Checks to see if a SendGrid template exists for Fides, returning the id if so """ - params = {"generations": "dynamic", "page_size": 18} - + # the pagination via the client actually doesn't work + # in lieu of over-engineering this we can manually call + # the next page if/when we hit the limit here + params = {"generations": "dynamic", "page_size": 200} response = sg.client.templates.get(query_params=params) - response_body_dict = json.loads(response.body) for template in response_body_dict["result"]: From 532c5583a9afa43645f3c1820ab8b86a95de0f77 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 22:46:46 +0000 Subject: [PATCH 04/20] add tests for template function --- .../messaging/message_dispatch_service.py | 22 ++++++----- .../message_dispatch_service_test.py | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index 97fb676578..375a6a6b39 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -463,7 +463,16 @@ def _twilio_email_dispatcher( MessagingServiceSecrets.TWILIO_API_KEY.value ] ) - template_test = _get_template_id_if_exists(sg, EMAIL_TEMPLATE_NAME) + + # the pagination via the client actually doesn't work + # in lieu of over-engineering this we can manually call + # the next page if/when we hit the limit here + response = sg.client.templates.get( + query_params={"generations": "dynamic", "page_size": 200} + ) + template_test = _get_template_id_if_exists( + json.loads(response.body), EMAIL_TEMPLATE_NAME + ) from_email = Email( messaging_config.details[MessagingServiceDetails.TWILIO_EMAIL_FROM.value] @@ -541,20 +550,13 @@ def _twilio_sms_dispatcher( def _get_template_id_if_exists( - sg: sendgrid.SendGridAPIClient, template_name: str + templates_response: Dict[str, List], template_name: str ) -> Optional[str]: """ Checks to see if a SendGrid template exists for Fides, returning the id if so """ - # the pagination via the client actually doesn't work - # in lieu of over-engineering this we can manually call - # the next page if/when we hit the limit here - params = {"generations": "dynamic", "page_size": 200} - response = sg.client.templates.get(query_params=params) - response_body_dict = json.loads(response.body) - - for template in response_body_dict["result"]: + for template in templates_response["result"]: if template["name"].lower() == template_name.lower(): return template["id"] return None diff --git a/tests/ops/service/messaging/message_dispatch_service_test.py b/tests/ops/service/messaging/message_dispatch_service_test.py index d2506ff7c8..1a5338073d 100644 --- a/tests/ops/service/messaging/message_dispatch_service_test.py +++ b/tests/ops/service/messaging/message_dispatch_service_test.py @@ -25,6 +25,7 @@ from fides.api.ops.schemas.redis_cache import Identity from fides.api.ops.service.messaging.message_dispatch_service import ( _get_dispatcher_from_config_type, + _get_template_id_if_exists, _twilio_email_dispatcher, _twilio_sms_dispatcher, dispatch_message, @@ -32,6 +33,32 @@ from fides.core.config import CONFIG +@pytest.fixture +def test_template_response_body(): + yield { + "result": [ + { + "id": "d-0507bb6d47cb46f38761f541b9cb8507", + "name": "fides", + "generation": "dynamic", + "updated_at": "2023-02-28 00:43:28", + "versions": [ + { + "id": "2080ab46-ebd2-40fa-b595-01d3e94e2700", + "template_id": "d-0507bb6d47cb46f38761f541b9cb8507", + "active": 1, + "name": "fides", + "generate_plain_content": False, + "subject": "DSR Testing", + "updated_at": "2023-03-01 13:34:23", + "editor": "design", + } + ], + }, + ] + } + + @pytest.mark.unit class TestMessageDispatchService: @mock.patch( @@ -429,6 +456,16 @@ def test_dispatch_no_secrets(self, messaging_config_twilio_email): assert "No twilio email config details or secrets" in str(exc.value) + def test_template_found(self, test_template_response_body): + template_test = _get_template_id_if_exists(test_template_response_body, "fides") + assert template_test + + def test_no_template_found(self, test_template_response_body): + template_test = _get_template_id_if_exists( + test_template_response_body, "not_fides" + ) + assert template_test is None + class TestTwilioSmsDispatcher: def test_dispatch_no_to(self, messaging_config_twilio_sms): From 2d426556c78737e800e73da532b432a7443eb4ac Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Thu, 2 Mar 2023 13:01:41 +0000 Subject: [PATCH 05/20] refactor to improve test coverage --- .../messaging/message_dispatch_service.py | 38 ++++++++++++++----- .../message_dispatch_service_test.py | 34 ++++++++++++++++- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index 375a6a6b39..9ef4900d85 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -479,16 +479,10 @@ def _twilio_email_dispatcher( ) to_email = To(to.strip()) subject = message.subject - if template_test: - mail = Mail(from_email=from_email, subject=subject) - mail.template_id = TemplateId(template_test) - personalization = Personalization() - personalization.dynamic_template_data = {"fides_email_body": message.body} - personalization.add_email(to_email) - mail.add_personalization(personalization) - else: - content = Content("text/html", message.body) - mail = Mail(from_email, to_email, subject, content) + mail = _compose_twilio_mail( + from_email, to_email, subject, message.body, template_test + ) + response = sg.client.mail.send.post(request_body=mail.get()) if response.status_code >= 400: logger.error( @@ -560,3 +554,27 @@ def _get_template_id_if_exists( if template["name"].lower() == template_name.lower(): return template["id"] return None + + +def _compose_twilio_mail( + from_email: Email, + to_email: To, + subject: str, + message_body: str, + template_test: Optional[str] = None, +) -> Mail: + """ + Returns the Mail object to send, if a template is passed composes the Mail + appropriately with the template ID and paramaterized message body. + """ + if template_test: + mail = Mail(from_email=from_email, subject=subject) + mail.template_id = TemplateId(template_test) + personalization = Personalization() + personalization.dynamic_template_data = {"fides_email_body": message_body} + personalization.add_email(to_email) + mail.add_personalization(personalization) + else: + content = Content("text/html", message_body) + mail = Mail(from_email, to_email, subject, content) + return mail diff --git a/tests/ops/service/messaging/message_dispatch_service_test.py b/tests/ops/service/messaging/message_dispatch_service_test.py index 1a5338073d..4022e12979 100644 --- a/tests/ops/service/messaging/message_dispatch_service_test.py +++ b/tests/ops/service/messaging/message_dispatch_service_test.py @@ -3,6 +3,7 @@ import pytest import requests_mock +from sendgrid.helpers.mail import Email, To from sqlalchemy.orm import Session from fides.api.ops.common_exceptions import MessageDispatchException @@ -24,6 +25,8 @@ from fides.api.ops.schemas.privacy_request import Consent from fides.api.ops.schemas.redis_cache import Identity from fides.api.ops.service.messaging.message_dispatch_service import ( + EMAIL_TEMPLATE_NAME, + _compose_twilio_mail, _get_dispatcher_from_config_type, _get_template_id_if_exists, _twilio_email_dispatcher, @@ -59,6 +62,11 @@ def test_template_response_body(): } +@pytest.fixture +def test_message_body(): + yield "This is a test DSR message body" + + @pytest.mark.unit class TestMessageDispatchService: @mock.patch( @@ -457,15 +465,37 @@ def test_dispatch_no_secrets(self, messaging_config_twilio_email): assert "No twilio email config details or secrets" in str(exc.value) def test_template_found(self, test_template_response_body): - template_test = _get_template_id_if_exists(test_template_response_body, "fides") + template_test = _get_template_id_if_exists( + test_template_response_body, EMAIL_TEMPLATE_NAME + ) assert template_test def test_no_template_found(self, test_template_response_body): template_test = _get_template_id_if_exists( - test_template_response_body, "not_fides" + test_template_response_body, f"not_{EMAIL_TEMPLATE_NAME}" ) assert template_test is None + def test_templated_mail(self, test_message_body): + mail = _compose_twilio_mail( + Email("test@test.com"), + To("test@test.com"), + "Test DSR EMail", + test_message_body, + "test_template", + ) + assert "template_id" in mail.get() + + def test_non_templated_mail(self, test_message_body): + mail = _compose_twilio_mail( + Email("test@test.com"), + To("test@test.com"), + "Test DSR EMail", + test_message_body, + template_test=None, + ) + assert "template_id" not in mail.get() + class TestTwilioSmsDispatcher: def test_dispatch_no_to(self, messaging_config_twilio_sms): From 2afc8b64cf46e1ee67fb78afb3bc4549a96a99e3 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Fri, 3 Mar 2023 15:06:51 -0600 Subject: [PATCH 06/20] Make Scopes Easier to Use in the UI (#2741) Return the "total scopes" that the user has via their roles or that they were assigned directly. --- CHANGELOG.md | 1 + scripts/setup/user.py | 2 - .../ops/api/v1/endpoints/oauth_endpoints.py | 4 +- src/fides/api/ops/api/v1/scope_registry.py | 5 +- src/fides/api/ops/schemas/user_permission.py | 41 +++++++----- .../lib/models/fides_user_permissions.py | 13 ++++ src/fides/lib/oauth/roles.py | 2 +- tests/conftest.py | 8 ++- tests/fixtures/application_fixtures.py | 1 - .../test_user_permission_endpoints.py | 51 +++++++++++++- .../models/test_fidesopsuserpermissions.py | 67 ++++++++++++++++++- tests/ops/schemas/test_user_permission.py | 7 +- 12 files changed, 167 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b452e1b1b..aebac88cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ The types of changes are: * Redesigned the default/init config file to be auto-documented. Also updates the `fides init` logic and analytics consent logic [#2694](https://github.com/ethyca/fides/pull/2694) * Change how config creation/import is handled across the application [#2622](https://github.com/ethyca/fides/pull/2622) * Updates Roles->Scopes Mapping [#2744](https://github.com/ethyca/fides/pull/2744) +* Return user scopes as an enum, as well as total scopes [#2741](https://github.com/ethyca/fides/pull/2741) ### Developer Experience diff --git a/scripts/setup/user.py b/scripts/setup/user.py index 30de0e55e6..56ec171275 100644 --- a/scripts/setup/user.py +++ b/scripts/setup/user.py @@ -4,9 +4,7 @@ from loguru import logger from fides.api.ops.api.v1 import urn_registry as urls -from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY from fides.core.config import CONFIG -from fides.lib.oauth.roles import OWNER from . import constants diff --git a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py index 437a5f7987..d4409e9015 100644 --- a/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/oauth_endpoints.py @@ -21,7 +21,7 @@ CLIENT_UPDATE, SCOPE_READ, SCOPE_REGISTRY, - SCOPE_REGISTRY_ENUM, + ScopeRegistryEnum, ) from fides.api.ops.api.v1.urn_registry import ( CLIENT, @@ -193,7 +193,7 @@ def set_client_scopes( @router.get( SCOPE, dependencies=[Security(verify_oauth_client, scopes=[SCOPE_READ])], - response_model=List[SCOPE_REGISTRY_ENUM], + response_model=List[ScopeRegistryEnum], ) def read_scopes() -> List[str]: """Returns a list of all scopes available for assignment in the system""" diff --git a/src/fides/api/ops/api/v1/scope_registry.py b/src/fides/api/ops/api/v1/scope_registry.py index 22d65ec022..c75496f933 100644 --- a/src/fides/api/ops/api/v1/scope_registry.py +++ b/src/fides/api/ops/api/v1/scope_registry.py @@ -231,7 +231,8 @@ SCOPE_REGISTRY = list(SCOPE_DOCS.keys()) -_SCOPE_REGISTRY_DICT = {key: key for key in SCOPE_REGISTRY} # mypy doesn't like taking the dictionary to generate the enum # https://github.com/python/mypy/issues/5317 -SCOPE_REGISTRY_ENUM = Enum("ScopeRegistry", _SCOPE_REGISTRY_DICT) # type: ignore +ScopeRegistryEnum = Enum( # type: ignore[misc] + "ScopeRegistryEnum", {scope: scope for scope in SCOPE_REGISTRY} # type: ignore +) diff --git a/src/fides/api/ops/schemas/user_permission.py b/src/fides/api/ops/schemas/user_permission.py index d6126df15d..c9f30f12bb 100644 --- a/src/fides/api/ops/schemas/user_permission.py +++ b/src/fides/api/ops/schemas/user_permission.py @@ -1,12 +1,10 @@ from typing import List, Optional -from fastapi import HTTPException from pydantic import validator -from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY -from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY +from fides.api.ops.api.v1.scope_registry import SCOPE_REGISTRY, ScopeRegistryEnum from fides.api.ops.schemas.base_class import BaseSchema -from fides.lib.oauth.roles import RoleRegistry +from fides.lib.oauth.roles import RoleRegistryEnum class UserPermissionsCreate(BaseSchema): @@ -16,20 +14,8 @@ class UserPermissionsCreate(BaseSchema): but we also will continue to support the ability to be assigned specific individual scopes. """ - scopes: List[str] = [] - roles: List[RoleRegistry] = [] - - @validator("scopes") - @classmethod - def validate_scopes(cls, scopes: List[str]) -> List[str]: - """Validates that all incoming scopes are valid""" - diff = set(scopes).difference(set(SCOPE_REGISTRY)) - if len(diff) > 0: - raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Invalid Scope(s) {diff}. Scopes must be one of {SCOPE_REGISTRY}.", - ) - return scopes + scopes: List[ScopeRegistryEnum] = [] + roles: List[RoleRegistryEnum] = [] class Config: """So roles are strings when we add to the db""" @@ -50,3 +36,22 @@ class UserPermissionsResponse(UserPermissionsCreate): id: str user_id: str + scopes: List[ScopeRegistryEnum] + total_scopes: List[ScopeRegistryEnum] + + class Config: + use_enum_values = True + + @validator("scopes", pre=True) + def validate_obsolete_scopes( + cls, scopes: List[ScopeRegistryEnum] + ) -> List[ScopeRegistryEnum]: + """Filter out obsolete scopes if the scope registry has changed""" + return [scope for scope in scopes or [] if scope in SCOPE_REGISTRY] + + @validator("total_scopes", pre=True) + def validate_obsolete_total_scopes( + cls, total_scopes: List[ScopeRegistryEnum] + ) -> List[ScopeRegistryEnum]: + """Filter out obsolete total scopes if the scope registry has changed""" + return [scope for scope in total_scopes or [] if scope in SCOPE_REGISTRY] diff --git a/src/fides/lib/models/fides_user_permissions.py b/src/fides/lib/models/fides_user_permissions.py index 8c37bcc9db..ba07c60060 100644 --- a/src/fides/lib/models/fides_user_permissions.py +++ b/src/fides/lib/models/fides_user_permissions.py @@ -1,8 +1,11 @@ +from typing import List + from sqlalchemy import ARRAY, Column, ForeignKey, String from sqlalchemy.orm import backref, relationship from fides.lib.db.base_class import Base from fides.lib.models.fides_user import FidesUser +from fides.lib.oauth.roles import ROLES_TO_SCOPES_MAPPING class FidesUserPermissions(Base): @@ -15,3 +18,13 @@ class FidesUserPermissions(Base): FidesUser, backref=backref("permissions", cascade="all,delete", uselist=False), ) + + @property + def total_scopes(self) -> List[str]: + """Returns the total model-level scopes the user has, either 1) scopes they are assigned directly, + or 2) Roles they have via their scopes.""" + all_scopes = self.scopes or [] + for role in self.roles: + all_scopes += ROLES_TO_SCOPES_MAPPING.get(role, []) + + return sorted(list(set(all_scopes))) diff --git a/src/fides/lib/oauth/roles.py b/src/fides/lib/oauth/roles.py index 1d3371540f..4a1daeff8a 100644 --- a/src/fides/lib/oauth/roles.py +++ b/src/fides/lib/oauth/roles.py @@ -39,7 +39,7 @@ VIEWER_AND_APPROVER = "viewer_and_approver" -class RoleRegistry(Enum): +class RoleRegistryEnum(Enum): """Enum of available roles Owner - Full admin diff --git a/tests/conftest.py b/tests/conftest.py index ad5ed49dc2..c5e6ab258f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,7 +43,13 @@ from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions from fides.lib.oauth.jwt import generate_jwe -from fides.lib.oauth.roles import APPROVER, CONTRIBUTOR, VIEWER_AND_APPROVER +from fides.lib.oauth.roles import ( + APPROVER, + CONTRIBUTOR, + OWNER, + VIEWER, + VIEWER_AND_APPROVER, +) from tests.fixtures.application_fixtures import * from tests.fixtures.bigquery_fixtures import * from tests.fixtures.email_fixtures import * diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index 529dfb8127..083f5f5cab 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -71,7 +71,6 @@ from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions -from fides.lib.oauth.roles import OWNER, VIEWER logging.getLogger("faker").setLevel(logging.ERROR) # disable verbose faker logging diff --git a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py index 39ab61a222..6900301c53 100644 --- a/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_permission_endpoints.py @@ -23,7 +23,7 @@ from fides.lib.models.client import ClientDetail from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions -from fides.lib.oauth.roles import OWNER, VIEWER +from fides.lib.oauth.roles import OWNER, ROLES_TO_SCOPES_MAPPING, VIEWER from tests.conftest import generate_auth_header_for_user, generate_role_header_for_user @@ -61,7 +61,11 @@ def test_create_user_permissions_invalid_scope( response = api_client.post(url, headers=auth_header, json=body) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code - assert "Invalid Scope(s) {'not a real scope'}" in response.json()["detail"] + assert response.json()["detail"][0]["loc"] == ["body", "scopes", 0] + assert ( + "value is not a valid enumeration member" + in response.json()["detail"][0]["msg"] + ) user.delete(db) def test_create_user_permissions_invalid_user_id( @@ -248,7 +252,11 @@ def test_edit_user_permissions_invalid_scope( f"{V1_URL_PREFIX}/user/{user.id}/permission", headers=auth_header, json=body ) assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code - assert "Invalid Scope(s) {'not a real scope'}" in response.json()["detail"] + assert response.json()["detail"][0]["loc"] == ["body", "scopes", 0] + assert ( + "value is not a valid enumeration member" + in response.json()["detail"][0]["msg"] + ) user.delete(db) def test_edit_user_permissions_invalid_user_id( @@ -519,6 +527,37 @@ def test_get_user_permissions( assert response_body["user_id"] == user.id assert response_body["scopes"] == [PRIVACY_REQUEST_READ] assert response_body["roles"] == [] + assert response_body["total_scopes"] == [PRIVACY_REQUEST_READ] + + def test_get_user_permissions_outdated_scope( + self, db, api_client, user, auth_user, permissions + ) -> None: + user.permissions.scopes = [PRIVACY_REQUEST_READ, "OLD_DEPRECATED_SCOPE"] + user.permissions.save(db) + + scopes = [USER_PERMISSION_READ] + ClientDetail.create_client_and_secret( + db, + CONFIG.security.oauth_client_id_length_bytes, + CONFIG.security.oauth_client_secret_length_bytes, + scopes=scopes, + user_id=auth_user.id, + ) + auth_header = generate_auth_header_for_user(auth_user, scopes) + + response = api_client.get( + f"{V1_URL_PREFIX}/user/{user.id}/permission", + headers=auth_header, + ) + response_body = response.json() + assert HTTP_200_OK == response.status_code + assert response_body["id"] == permissions.id + assert response_body["user_id"] == user.id + assert response_body["scopes"] == [ + PRIVACY_REQUEST_READ + ] # Deprecated scope ignored + assert response_body["roles"] == [] + assert response_body["total_scopes"] == [PRIVACY_REQUEST_READ] def test_get_user_with_no_permissions_as_root( self, db, api_client, auth_user, root_auth_header @@ -534,6 +573,7 @@ def test_get_user_with_no_permissions_as_root( resp = response.json() assert resp["scopes"] == [] assert resp["roles"] == [] + assert resp["total_scopes"] == [] assert resp["user_id"] == auth_user.id def test_get_current_user_permissions(self, db, api_client, auth_user) -> None: @@ -560,6 +600,7 @@ def test_get_current_user_permissions(self, db, api_client, auth_user) -> None: assert response_body["id"] == permissions.id assert response_body["user_id"] == auth_user.id assert response_body["scopes"] == scopes + assert response_body["total_scopes"] == scopes assert response_body["roles"] == [] def test_get_current_root_user_permissions( @@ -575,6 +616,7 @@ def test_get_current_root_user_permissions( assert response_body["user_id"] == oauth_root_client.id assert response_body["scopes"] == SCOPE_REGISTRY assert response_body["roles"] == [OWNER] + assert response_body["total_scopes"] == sorted(SCOPE_REGISTRY) def test_get_root_user_permissions_by_non_root_user( self, db, api_client, oauth_root_client, auth_user @@ -617,6 +659,7 @@ def test_get_own_user_roles(self, db, api_client, auth_user): assert resp["scopes"] == [] assert resp["roles"] == [VIEWER] assert resp["user_id"] == auth_user.id + assert resp["total_scopes"] == sorted(ROLES_TO_SCOPES_MAPPING[VIEWER]) def test_get_other_user_roles_as_root( self, db, api_client, auth_user, root_auth_header @@ -634,6 +677,7 @@ def test_get_other_user_roles_as_root( assert resp["scopes"] == [] assert resp["roles"] == [VIEWER] assert resp["user_id"] == auth_user.id + assert resp["total_scopes"] == sorted(ROLES_TO_SCOPES_MAPPING[VIEWER]) def test_get_other_user_roles_as_viewer( self, db, api_client, auth_user, viewer_user @@ -666,3 +710,4 @@ def test_get_other_user_roles_as_owner(self, db, api_client, auth_user, owner_us assert resp["scopes"] == [] assert resp["roles"] == [VIEWER] assert resp["user_id"] == auth_user.id + assert resp["total_scopes"] == sorted(ROLES_TO_SCOPES_MAPPING[VIEWER]) diff --git a/tests/ops/models/test_fidesopsuserpermissions.py b/tests/ops/models/test_fidesopsuserpermissions.py index 5371035e7f..24969f167a 100644 --- a/tests/ops/models/test_fidesopsuserpermissions.py +++ b/tests/ops/models/test_fidesopsuserpermissions.py @@ -1,9 +1,13 @@ import pytest from sqlalchemy.orm import Session -from fides.api.ops.api.v1.scope_registry import PRIVACY_REQUEST_READ +from fides.api.ops.api.v1.scope_registry import ( + PRIVACY_REQUEST_CREATE, + PRIVACY_REQUEST_READ, +) from fides.lib.models.fides_user import FidesUser from fides.lib.models.fides_user_permissions import FidesUserPermissions +from fides.lib.oauth.roles import ROLES_TO_SCOPES_MAPPING, VIEWER class TestFidesUserPermissions: @@ -31,3 +35,64 @@ def test_create_user_permissions_bad_payload(self, db: Session) -> None: db=db, data={}, ) + + def test_total_scopes(self, db): + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions: FidesUserPermissions = FidesUserPermissions.create( + db=db, + data={ + "user_id": user.id, + "scopes": [PRIVACY_REQUEST_CREATE], + "roles": [VIEWER], + }, + ) + assert permissions.scopes == [PRIVACY_REQUEST_CREATE] + assert len(permissions.total_scopes) == 1 + len(ROLES_TO_SCOPES_MAPPING[VIEWER]) + assert PRIVACY_REQUEST_CREATE in permissions.total_scopes + user.delete(db) + + def test_total_scopes_no_roles(self, db): + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions: FidesUserPermissions = FidesUserPermissions.create( + db=db, + data={"user_id": user.id, "scopes": [PRIVACY_REQUEST_CREATE]}, + ) + assert permissions.scopes == [PRIVACY_REQUEST_CREATE] + assert permissions.total_scopes == [PRIVACY_REQUEST_CREATE] + user.delete(db) + + def test_total_scopes_no_direct_scopes(self, db): + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions: FidesUserPermissions = FidesUserPermissions.create( + db=db, + data={"user_id": user.id, "roles": [VIEWER]}, + ) + assert permissions.total_scopes == sorted(ROLES_TO_SCOPES_MAPPING[VIEWER]) + + user.delete(db) + + def test_total_scopes_no_roles_or_direct_scopes(self, db): + user = FidesUser.create( + db=db, + data={"username": "user_1", "password": "test_password"}, + ) + + permissions: FidesUserPermissions = FidesUserPermissions.create( + db=db, + data={"user_id": user.id}, + ) + assert permissions.total_scopes == [] + + user.delete(db) diff --git a/tests/ops/schemas/test_user_permission.py b/tests/ops/schemas/test_user_permission.py index a7ce4ae006..c497d81b9f 100644 --- a/tests/ops/schemas/test_user_permission.py +++ b/tests/ops/schemas/test_user_permission.py @@ -1,5 +1,6 @@ import pytest from fastapi import HTTPException +from pydantic import ValidationError from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY from fides.api.ops.api.v1.scope_registry import USER_DELETE, USER_PERMISSION_CREATE @@ -14,9 +15,7 @@ def test_scope_validation(self): def test_catch_invalid_scopes(self): invalid_scopes = ["not a real scope", "invalid scope here"] - with pytest.raises(HTTPException) as exc: + with pytest.raises(ValidationError) as exc: UserPermissionsCreate(scopes=invalid_scopes) - assert exc.value.status_code == HTTP_422_UNPROCESSABLE_ENTITY - assert invalid_scopes[0] in exc.value.detail - assert invalid_scopes[1] in exc.value.detail + assert len(exc.value.errors()) == 2 From 8b34c5346ce4f31c43d5781a16fc231ec2de63a4 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Fri, 3 Mar 2023 22:14:05 +0000 Subject: [PATCH 07/20] add changelog item --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aebac88cd8..56cfd6c1c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The types of changes are: * Access and erasure support for Delighted [#2244](https://github.com/ethyca/fides/pull/2244) * Improve "Upload a new dataset YAML" [#1531](https://github.com/ethyca/fides/pull/2258) * Access and erasure support for Yotpo [#2708](https://github.com/ethyca/fides/pull/2708) +* Allow SendGrid template usage [#2728](https://github.com/ethyca/fides/pull/2728) ### Changed From 29b68dfeb89210618573ae59f411ccb7640b3e33 Mon Sep 17 00:00:00 2001 From: LKCSmith <99051851+LKCSmith@users.noreply.github.com> Date: Sun, 5 Mar 2023 17:39:25 -0600 Subject: [PATCH 08/20] 2455/Configuration updates post-API (#2677) Co-authored-by: Adam Sachs Co-authored-by: Kelsey Thomas <101993653+Kelsey-Ethyca@users.noreply.github.com> --- .../cypress/e2e/privacy-requests.cy.ts | 45 ++++++-- .../messaging_configuration.json | 7 ++ clients/admin-ui/cypress/support/stubs.ts | 4 + .../admin-ui/src/features/common/Layout.tsx | 28 +++-- .../MailgunEmailConfiguration.tsx | 61 +++++----- .../configuration/MessagingConfiguration.tsx | 65 ++++++----- .../configuration/S3StorageConfiguration.tsx | 107 ++++++++++-------- .../configuration/StorageConfiguration.tsx | 28 +++-- .../TwilioEmailConfiguration.tsx | 63 ++++++----- .../configuration/TwilioSMS.tsx | 35 +++--- .../features/privacy-requests/constants.ts | 11 ++ .../privacy-requests.slice.ts | 8 +- .../src/features/privacy-requests/types.ts | 48 ++++---- .../src/pages/privacy-requests/configure.tsx | 11 +- 14 files changed, 319 insertions(+), 202 deletions(-) create mode 100644 clients/admin-ui/cypress/fixtures/privacy-requests/messaging_configuration.json diff --git a/clients/admin-ui/cypress/e2e/privacy-requests.cy.ts b/clients/admin-ui/cypress/e2e/privacy-requests.cy.ts index 6ea5c33d5f..8810b977d4 100644 --- a/clients/admin-ui/cypress/e2e/privacy-requests.cy.ts +++ b/clients/admin-ui/cypress/e2e/privacy-requests.cy.ts @@ -138,7 +138,7 @@ describe("Privacy Requests", () => { cy.getByTestId("option-local").click(); cy.wait("@createConfigurationSettings").then((interception) => { const { body } = interception.request; - expect(body.fides.storage.active_default_storage_type).to.eql("local"); + expect(body.storage.active_default_storage_type).to.eql("local"); }); cy.wait("@createStorage").then((interception) => { @@ -146,25 +146,52 @@ describe("Privacy Requests", () => { expect(body.type).to.eql("local"); expect(body.format).to.eql("json"); }); - - cy.contains("Configure active storage type saved successfully"); - cy.contains("Configure storage type details saved successfully"); }); it("Can configure S3 storage", () => { cy.getByTestId("option-s3").click(); cy.wait("@createConfigurationSettings").then((interception) => { const { body } = interception.request; - expect(body.fides.storage.active_default_storage_type).to.eql("s3"); + expect(body.storage.active_default_storage_type).to.eql("s3"); }); cy.wait("@getStorageDetails"); + }); + }); + + describe("Message Configuration", () => { + beforeEach(() => { + cy.visit("/privacy-requests/configure/messaging"); + }); + + it("Can configure Mailgun email", () => { + cy.getByTestId("option-mailgun").click(); + cy.getByTestId("input-domain").type("test-domain"); cy.getByTestId("save-btn").click(); - cy.wait("@createStorage").then((interception) => { + cy.wait("@createMessagingConfiguration").then((interception) => { const { body } = interception.request; - expect(body.type).to.eql("s3"); + expect(body.service_type).to.eql("MAILGUN"); + cy.contains( + "Mailgun email successfully updated. You can now enter your security key." + ); + }); + }); + + it("Can configure Twilio email", () => { + cy.getByTestId("option-twilio-email").click(); + cy.getByTestId("input-email").type("test-email"); + cy.getByTestId("save-btn").click(); + cy.wait("@createMessagingConfiguration").then(() => { + cy.contains( + "Twilio email successfully updated. You can now enter your security key." + ); + }); + }); + + it("Can configure Twilio SMS", () => { + cy.getByTestId("option-twilio-sms").click(); + cy.wait("@createMessagingConfiguration").then(() => { + cy.contains("Messaging provider saved successfully."); }); - cy.contains("S3 storage credentials successfully updated."); - cy.contains("Configure active storage type saved successfully."); }); }); }); diff --git a/clients/admin-ui/cypress/fixtures/privacy-requests/messaging_configuration.json b/clients/admin-ui/cypress/fixtures/privacy-requests/messaging_configuration.json new file mode 100644 index 0000000000..bc68c8d43d --- /dev/null +++ b/clients/admin-ui/cypress/fixtures/privacy-requests/messaging_configuration.json @@ -0,0 +1,7 @@ +{ + "details": { + "domain": "test-domain", + "is_eu_domain": "false" + }, + "service_type": "MAILGUN" + } diff --git a/clients/admin-ui/cypress/support/stubs.ts b/clients/admin-ui/cypress/support/stubs.ts index 4dc4164831..e9dd66b991 100644 --- a/clients/admin-ui/cypress/support/stubs.ts +++ b/clients/admin-ui/cypress/support/stubs.ts @@ -85,6 +85,10 @@ export const stubPrivacyRequestsConfigurationCrud = () => { cy.intercept("PUT", "/api/v1/storage/default/*/secret", { fixture: "/privacy-requests/settings_configuration.json", }).as("createStorageSecrets"); + + cy.intercept("PUT", "/api/v1/messaging/default", { + fixture: "/privacy-requests/messaging_configuration.json", + }).as("createMessagingConfiguration"); }; export const CONNECTION_STRING = diff --git a/clients/admin-ui/src/features/common/Layout.tsx b/clients/admin-ui/src/features/common/Layout.tsx index 3f1da20560..353a96af32 100644 --- a/clients/admin-ui/src/features/common/Layout.tsx +++ b/clients/admin-ui/src/features/common/Layout.tsx @@ -24,14 +24,24 @@ const Layout = ({ }) => { const features = useFeatures(); const router = useRouter(); - const { data: activeMessagingProvider } = - useGetActiveMessagingProviderQuery(); - const { data: activeStorage } = useGetActiveStorageQuery(); - const showConfigurationNotificationBanner = - (!activeStorage || !activeMessagingProvider) && - features.flags.privacyRequestsConfiguration && - (router.pathname === "/privacy-requests" || - router.pathname === "/datastore-connection"); + const isValidNotificationRoute = + router.pathname === "/privacy-requests" || + router.pathname === "/datastore-connection"; + const skip = !( + features.flags.privacyRequestsConfiguration && isValidNotificationRoute + ); + + const { data: activeMessagingProvider } = useGetActiveMessagingProviderQuery( + undefined, + { skip } + ); + + const { data: activeStorage } = useGetActiveStorageQuery(undefined, { + skip, + }); + + const showNotificationBanner = + (!activeMessagingProvider || !activeStorage) && isValidNotificationRoute; return (
@@ -50,7 +60,7 @@ const Layout = ({ - {showConfigurationNotificationBanner ? ( + {showNotificationBanner ? ( ) : null} {children} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx index 72e61d747d..dfa52c4bbd 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx @@ -5,6 +5,7 @@ import { useState } from "react"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; +import { messagingProviders } from "~/features/privacy-requests/constants"; import { useCreateMessagingConfigurationMutation, useCreateMessagingConfigurationSecretsMutation, @@ -19,7 +20,7 @@ const MailgunEmailConfiguration = () => { useState(""); const { handleError } = useAPIHelper(); const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ - type: "mailgun", + type: messagingProviders.mailgun, }); const [createMessagingConfiguration] = useCreateMessagingConfigurationMutation(); @@ -28,7 +29,7 @@ const MailgunEmailConfiguration = () => { const handleMailgunConfiguration = async (value: { domain: string }) => { const result = await createMessagingConfiguration({ - type: "mailgun", + service_type: messagingProviders.mailgun, details: { is_eu_domain: "false", domain: value.domain, @@ -49,7 +50,10 @@ const MailgunEmailConfiguration = () => { api_key: string; }) => { const result = await createMessagingConfigurationSecrets({ - mailgun_api_key: value.api_key, + details: { + mailgun_api_key: value.api_key, + }, + service_type: messagingProviders.mailgun, }); if (isErrorResult(result)) { @@ -64,7 +68,7 @@ const MailgunEmailConfiguration = () => { }; const initialAPIKeyValue = { - api_key: messagingDetails?.key ?? "", + api_key: "", }; return ( @@ -76,19 +80,22 @@ const MailgunEmailConfiguration = () => { - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
- + + + + )}
diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx index 1a50d8f717..8b9259e51b 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx @@ -10,14 +10,16 @@ import { Text, } from "@fidesui/react"; import NextLink from "next/link"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import Layout from "~/features/common/Layout"; +import { messagingProviders } from "~/features/privacy-requests/constants"; import { useCreateConfigurationSettingsMutation, useCreateMessagingConfigurationMutation, + useGetActiveMessagingProviderQuery, } from "~/features/privacy-requests/privacy-requests.slice"; import MailgunEmailConfiguration from "./MailgunEmailConfiguration"; @@ -31,34 +33,41 @@ const MessagingConfiguration = () => { const [createMessagingConfiguration] = useCreateMessagingConfigurationMutation(); const [saveActiveConfiguration] = useCreateConfigurationSettingsMutation(); + const { data: activeMessagingProvider } = + useGetActiveMessagingProviderQuery(); + + useEffect(() => { + if (activeMessagingProvider) { + setMessagingValue(activeMessagingProvider?.service_type); + } + }, [activeMessagingProvider]); const handleChange = async (value: string) => { const result = await saveActiveConfiguration({ - fides: { - notifications: { - notification_service_type: value, - send_request_completion_notification: true, - send_request_receipt_notification: true, - send_request_review_notification: true, - subject_identity_verification_required: true, - }, + notifications: { + notification_service_type: value, + send_request_completion_notification: true, + send_request_receipt_notification: true, + send_request_review_notification: true, + }, + execution: { + subject_identity_verification_required: true, }, }); if (isErrorResult(result)) { handleError(result.error); - } else if (value !== "twilio_text") { - successAlert(`Configured storage type successfully.`); + } else if (value !== messagingProviders.twilio_text) { setMessagingValue(value); } else { const twilioTextResult = await createMessagingConfiguration({ - type: "twilio_text", + service_type: messagingProviders.twilio_text, }); if (isErrorResult(twilioTextResult)) { handleError(twilioTextResult.error); } else { - successAlert(`Configure messaging provider saved successfully.`); + successAlert(`Messaging provider saved successfully.`); setMessagingValue(value); } } @@ -118,36 +127,38 @@ const MessagingConfiguration = () => { > - Mailgun email + Mailgun Email - Twilio email + Twilio Email - Twilio sms + Twilio SMS - {messagingValue === "mailgun-email" ? ( + {messagingValue === messagingProviders.mailgun ? ( ) : null} - {messagingValue === "twilio-email" ? ( + {messagingValue === messagingProviders.twilio_email ? ( ) : null} - {messagingValue === "twilio-text" ? : null} + {messagingValue === messagingProviders.twilio_text ? ( + + ) : null} ); diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx index 1aa2083c15..6f71480d34 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx @@ -1,31 +1,38 @@ -import { Button, Divider, Heading, Stack } from "@fidesui/react"; +import { Box, Button, Divider, Heading, Stack } from "@fidesui/react"; import { Form, Formik } from "formik"; import { useState } from "react"; import { CustomSelect, CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; +import { storageTypes } from "~/features/privacy-requests/constants"; import { useCreateStorageMutation, useCreateStorageSecretsMutation, } from "~/features/privacy-requests/privacy-requests.slice"; +interface SavedStorageDetails { + storageDetails: { + details: { + auth_method: string; + bucket: string; + }; + format: string; + }; +} interface StorageDetails { - type: string; - details: { + storageDetails: { auth_method: string; bucket: string; + format: string; }; - format: string; } interface SecretsStorageData { aws_access_key_id: string; aws_secret_access_key: string; } -const S3StorageConfiguration = ({ - storageDetails: { auth_method, bucket, format }, -}: any) => { +const S3StorageConfiguration = ({ storageDetails }: SavedStorageDetails) => { const [authMethod, setAuthMethod] = useState(""); const [saveStorageDetails] = useCreateStorageMutation(); const [setStorageSecrets] = useCreateStorageSecretsMutation(); @@ -34,12 +41,10 @@ const S3StorageConfiguration = ({ const { successAlert } = useAlert(); const initialValues = { - type: "s3", - details: { - auth_method: auth_method ?? "", - bucket: bucket ?? "", - }, - format: format ?? "", + type: storageTypes.s3, + auth_method: storageDetails?.details?.auth_method ?? "", + bucket: storageDetails?.details?.bucket ?? "", + format: storageDetails?.format ?? "", }; const initialSecretValues = { @@ -48,13 +53,13 @@ const S3StorageConfiguration = ({ }; const handleSubmitStorageConfiguration = async ( - newValues: StorageDetails + newValues: StorageDetails["storageDetails"] ) => { const result = await saveStorageDetails({ - type: "s3", + type: storageTypes.s3, details: { - auth_method: newValues.details.auth_method, - bucket: newValues.details.bucket, + auth_method: newValues.auth_method, + bucket: newValues.bucket, }, format: newValues.format, }); @@ -62,15 +67,18 @@ const S3StorageConfiguration = ({ if (isErrorResult(result)) { handleError(result.error); } else { - setAuthMethod(newValues.details.auth_method); + setAuthMethod(newValues.auth_method); successAlert(`S3 storage credentials successfully updated.`); } }; const handleSubmitStorageSecrets = async (newValues: SecretsStorageData) => { const result = await setStorageSecrets({ - aws_access_key_id: newValues.aws_access_key_id, - aws_secret_access_key: newValues.aws_secret_access_key, + details: { + aws_access_key_id: newValues.aws_access_key_id, + aws_secret_access_key: newValues.aws_secret_access_key, + }, + type: storageTypes.s3, }); if (isErrorResult(result)) { @@ -89,8 +97,9 @@ const S3StorageConfiguration = ({ - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
- + + + +
)} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx index 49ed5bee75..0a3a2d75c6 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx @@ -10,14 +10,16 @@ import { Text, } from "@fidesui/react"; import NextLink from "next/link"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import Layout from "~/features/common/Layout"; +import { storageTypes } from "~/features/privacy-requests/constants"; import { useCreateConfigurationSettingsMutation, useCreateStorageMutation, + useGetActiveStorageQuery, useGetStorageDetailsQuery, } from "~/features/privacy-requests/privacy-requests.slice"; @@ -27,37 +29,43 @@ const StorageConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); const [storageValue, setStorageValue] = useState(""); - const [saveStorageType, { isLoading }] = useCreateStorageMutation(); - const [saveActiveStorage] = useCreateConfigurationSettingsMutation(); + + const { data: activeStorage } = useGetActiveStorageQuery(); const { data: storageDetails } = useGetStorageDetailsQuery({ type: storageValue, }); + const [saveStorageType, { isLoading }] = useCreateStorageMutation(); + const [saveActiveStorage] = useCreateConfigurationSettingsMutation(); + + useEffect(() => { + if (activeStorage) { + setStorageValue(activeStorage.type); + } + }, [activeStorage]); const handleChange = async (value: string) => { - if (value === "local") { + if (value === storageTypes.local) { const storageDetailsResult = await saveStorageType({ type: value, + details: {}, format: "json", }); if (isErrorResult(storageDetailsResult)) { handleError(storageDetailsResult.error); } else { - successAlert(`Configure storage type details saved successfully.`); + successAlert(`Configured storage details successfully.`); } } const activeStorageResults = await saveActiveStorage({ - fides: { - storage: { - active_default_storage_type: value, - }, + storage: { + active_default_storage_type: value, }, }); if (isErrorResult(activeStorageResults)) { handleError(activeStorageResults.error); } else { - successAlert(`Configure active storage type saved successfully.`); setStorageValue(value); } }; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx index ce828e184c..94f3adb9e3 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx @@ -5,6 +5,7 @@ import { useState } from "react"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; +import { messagingProviders } from "~/features/privacy-requests/constants"; import { useCreateMessagingConfigurationMutation, useCreateMessagingConfigurationSecretsMutation, @@ -16,7 +17,7 @@ const TwilioEmailConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ - type: "twilio_email", + type: messagingProviders.twilio_email, }); const [createMessagingConfiguration] = useCreateMessagingConfigurationMutation(); @@ -25,7 +26,7 @@ const TwilioEmailConfiguration = () => { const handleTwilioEmailConfiguration = async (value: { email: string }) => { const result = await createMessagingConfiguration({ - type: "twilio_email", + service_type: messagingProviders.twilio_email, details: { twilio_email_from: value.email, }, @@ -45,7 +46,10 @@ const TwilioEmailConfiguration = () => { api_key: string; }) => { const result = await createMessagingConfigurationSecrets({ - twilio_api_key: value.api_key, + details: { + twilio_api_key: value.api_key, + }, + service_type: messagingProviders.twilio_email, }); if (isErrorResult(result)) { @@ -57,11 +61,11 @@ const TwilioEmailConfiguration = () => { }; const initialValues = { - email: messagingDetails?.email ?? "", + email: messagingDetails?.details.twilio_email_from ?? "", }; const initialAPIKeyValues = { - api_key: messagingDetails?.key ?? "", + api_key: "", }; return ( @@ -73,19 +77,21 @@ const TwilioEmailConfiguration = () => { - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
- + + + + )}
diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx index be088e52d8..97e5191380 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx @@ -4,17 +4,12 @@ import { Form, Formik } from "formik"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; -import { - useCreateMessagingConfigurationSecretsMutation, - useGetMessagingConfigurationDetailsQuery, -} from "~/features/privacy-requests/privacy-requests.slice"; +import { messagingProviders } from "~/features/privacy-requests/constants"; +import { useCreateMessagingConfigurationSecretsMutation } from "~/features/privacy-requests/privacy-requests.slice"; const TwilioSMSConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); - const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ - type: "twilio_text", - }); const [createMessagingConfigurationSecrets] = useCreateMessagingConfigurationSecretsMutation(); @@ -25,10 +20,13 @@ const TwilioSMSConfiguration = () => { phone: string; }) => { const result = await createMessagingConfigurationSecrets({ - twilio_account_sid: value.account_sid, - twilio_auth_token: value.auth_token, - twilio_messaging_service_sid: value.messaging_service_sid, - twilio_sender_phone_number: value.phone, + details: { + twilio_account_sid: value.account_sid, + twilio_auth_token: value.auth_token, + twilio_messaging_service_sid: value.messaging_service_sid, + twilio_sender_phone_number: value.phone, + }, + service_type: messagingProviders.twilio_text, }); if (isErrorResult(result)) { @@ -39,10 +37,10 @@ const TwilioSMSConfiguration = () => { }; const initialValues = { - account_sid: messagingDetails?.twilio_account_sid ?? "", - auth_token: messagingDetails?.twilio_auth_token ?? "", - messaging_service_sid: messagingDetails?.twilio_messaging_service_sid ?? "", - phone: messagingDetails?.twilio_sender_phone_number ?? "", + account_sid: "", + auth_token: "", + messaging_service_sid: "", + phone: "", }; return ( @@ -54,20 +52,23 @@ const TwilioSMSConfiguration = () => { - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
{ + + +
+ )} + + + + ); +}; + +export default TestMessagingProviderConnectionButton; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx index 94f3adb9e3..a62f007096 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx @@ -12,6 +12,8 @@ import { useGetMessagingConfigurationDetailsQuery, } from "~/features/privacy-requests/privacy-requests.slice"; +import TestMessagingProviderConnectionButton from "./TestMessagingProviderConnectionButton"; + const TwilioEmailConfiguration = () => { const [configurationStep, setConfigurationStep] = useState(""); const { successAlert } = useAlert(); @@ -56,7 +58,7 @@ const TwilioEmailConfiguration = () => { handleError(result.error); } else { successAlert(`Twilio email secrets successfully updated.`); - setConfigurationStep("configureTwilioEmailSecrets"); + setConfigurationStep("testConnection"); } }; @@ -112,7 +114,8 @@ const TwilioEmailConfiguration = () => { )} - {configurationStep === "configureTwilioEmailSecrets" ? ( + {configurationStep === "configureTwilioEmailSecrets" || + configurationStep === "testConnection" ? ( <> @@ -158,6 +161,11 @@ const TwilioEmailConfiguration = () => { ) : null} + {configurationStep === "testConnection" ? ( + + ) : null} ); }; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx index 97e5191380..82fe82f096 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx @@ -1,15 +1,25 @@ import { Box, Button, Heading, Stack } from "@fidesui/react"; import { Form, Formik } from "formik"; +import { useState } from "react"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import { messagingProviders } from "~/features/privacy-requests/constants"; -import { useCreateMessagingConfigurationSecretsMutation } from "~/features/privacy-requests/privacy-requests.slice"; +import { + useCreateMessagingConfigurationSecretsMutation, + useGetMessagingConfigurationDetailsQuery, +} from "~/features/privacy-requests/privacy-requests.slice"; + +import TestMessagingProviderConnectionButton from "./TestMessagingProviderConnectionButton"; const TwilioSMSConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); + const [configurationStep, setConfigurationStep] = useState(""); + const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ + type: "twilio_text", + }); const [createMessagingConfigurationSecrets] = useCreateMessagingConfigurationSecretsMutation(); @@ -33,6 +43,7 @@ const TwilioSMSConfiguration = () => { handleError(result.error); } else { successAlert(`Twilio text secrets successfully updated.`); + setConfigurationStep("testConnection"); } }; @@ -104,6 +115,11 @@ const TwilioSMSConfiguration = () => { )} + {configurationStep === "testConnection" ? ( + + ) : null} ); }; diff --git a/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts b/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts index d4b67a75dc..db385c62ef 100644 --- a/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts +++ b/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts @@ -416,6 +416,13 @@ export const privacyRequestApi = createApi({ body: params.details, }), }), + createTestConnectionMessage: build.mutation({ + query: (params) => ({ + url: `messaging/config/test`, + method: "POST", + body: params, + }), + }), uploadManualWebhookData: build.mutation< any, PatchUploadManualWebhookDataRequest @@ -449,4 +456,5 @@ export const { useGetActiveStorageQuery, useCreateMessagingConfigurationMutation, useCreateMessagingConfigurationSecretsMutation, + useCreateTestConnectionMessageMutation, } = privacyRequestApi; From 987b05bcda276054f8a2a4a95a333bb1249c813d Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 5 Mar 2023 21:30:38 -0800 Subject: [PATCH 10/20] Overhaul CLI documentation and aesthetics (#2703) --- .fides/systems.yml | 2 +- CHANGELOG.md | 1 + requirements.txt | 2 +- src/fides/cli/__init__.py | 21 ++++-- src/fides/cli/cli_formatting.py | 114 +++++++++++++++++++++++++++++ src/fides/cli/commands/annotate.py | 10 +-- src/fides/cli/commands/core.py | 49 +++++-------- src/fides/cli/commands/crud.py | 33 +++++++-- src/fides/cli/commands/db.py | 8 +- src/fides/cli/commands/export.py | 20 ++--- src/fides/cli/commands/generate.py | 41 +++-------- src/fides/cli/commands/scan.py | 36 +++------ src/fides/cli/commands/user.py | 12 +-- src/fides/cli/commands/util.py | 30 ++++---- src/fides/cli/commands/view.py | 29 +++++++- src/fides/cli/options.py | 41 ++++++----- src/fides/cli/utils.py | 2 +- tests/ctl/cli/test_cli.py | 51 ++++++++++--- 18 files changed, 323 insertions(+), 179 deletions(-) create mode 100644 src/fides/cli/cli_formatting.py diff --git a/.fides/systems.yml b/.fides/systems.yml index e5d36896f2..429873ca8f 100644 --- a/.fides/systems.yml +++ b/.fides/systems.yml @@ -28,7 +28,7 @@ system: - fides_db # System Info - - fides_key: privacy_request_fullfillment + - fides_key: privacy_request_fulfillment name: Fides Privacy Request Fulfillment organization_fides_key: default_organization description: Privacy request fufillment. diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cfd6c1c5..8d66dfed1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ The types of changes are: * Add warning to 'fides deploy' when installed outside of a virtual environment [#2641](https://github.com/ethyca/fides/pull/2641) * Redesigned the default/init config file to be auto-documented. Also updates the `fides init` logic and analytics consent logic [#2694](https://github.com/ethyca/fides/pull/2694) * Change how config creation/import is handled across the application [#2622](https://github.com/ethyca/fides/pull/2622) +* Update the CLI aesthetics & docstrings [#2703](https://github.com/ethyca/fides/pull/2703) * Updates Roles->Scopes Mapping [#2744](https://github.com/ethyca/fides/pull/2744) * Return user scopes as an enum, as well as total scopes [#2741](https://github.com/ethyca/fides/pull/2741) diff --git a/requirements.txt b/requirements.txt index d4be59d3b8..ad520d2842 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,6 @@ APScheduler==3.9.1.post1 asyncpg==0.25.0 boto3==1.26.1 celery[pytest]==5.2.7 -click==8.1.3 colorama>=0.4.3 cryptography==38.0.3 dask==2022.9.2 @@ -35,6 +34,7 @@ PyMySQL==1.0.2 python-jose[cryptography]==3.3.0 pyyaml>=5,<6 redis==3.5.3 +rich-click==1.6.1 sendgrid==6.9.7 slowapi==0.1.5 snowflake-sqlalchemy==1.4.3 diff --git a/src/fides/cli/__init__.py b/src/fides/cli/__init__.py index efe8d2714f..fe02a594fc 100644 --- a/src/fides/cli/__init__.py +++ b/src/fides/cli/__init__.py @@ -1,14 +1,17 @@ -"""Contains the groups and setup for the CLI.""" +""" +Entrypoint for the Fides command-line. +""" from importlib.metadata import version from platform import system -import click +import rich_click as click from fideslog.sdk.python.client import AnalyticsClient import fides from fides.cli.utils import check_server from fides.core.config import get_config +from . import cli_formatting from .commands.annotate import annotate from .commands.core import evaluate, parse, pull, push from .commands.crud import delete, get_resource, list_resources @@ -58,24 +61,28 @@ "--config-path", "-f", "config_path", - default="", - help="Path to a configuration file. Use 'fides view-config' to print the config. Not compatible with the 'fides webserver' subcommand.", + show_default=True, + help="Path to a Fides config file. _Defaults to `.fides/fides.toml`._", ) @click.option( "--local", is_flag=True, - help="Run in 'local_mode'. This mode doesn't make API calls and can be used without the API server/database.", + help="Run in `local_mode`. This mode doesn't make API calls and can be used without the API server/database.", ) @click.pass_context def cli(ctx: click.Context, config_path: str, local: bool) -> None: """ - The parent group for the Fides CLI. + __Command-line tool for the Fides privacy engineering platform.__ + + --- + + _Note: The common MANIFESTS_DIR argument _always_ defaults to ".fides/" if not specified._ """ ctx.ensure_object(dict) config = get_config(config_path, verbose=True) - # Dyanmically add commands to the CLI + # Dynamically add commands to the CLI cli.commands = LOCAL_COMMAND_DICT if not (local or config.cli.local_mode): diff --git a/src/fides/cli/cli_formatting.py b/src/fides/cli/cli_formatting.py new file mode 100644 index 0000000000..1da85f45f2 --- /dev/null +++ b/src/fides/cli/cli_formatting.py @@ -0,0 +1,114 @@ +""" +This is a configuration file for `rich_click`, used to customize the +visual aesthetic and output of the CLI. + +This is the list of all documented configuration options for `rich_click`. + +You can set a style attribute by adding one or more of the following words: + +- "bold" or "b" for bold text. +- "blink" for text that flashes (use this one sparingly). +- "blink2" for text that flashes rapidly (not supported by most terminals). +- "conceal" for concealed text (not supported by most terminals). +- "italic" or "i" for italic text (not supported on Windows). +- "reverse" or "r" for text with foreground and background colors reversed. +- "strike" or "s" for text with a line through it. +- "underline" or "u" for underlined text. +- "underline2" or "uu" for doubly underlined text. +- "frame" for framed text. +- "encircle" for encircled text. +- "overline" or "o" for overlined text. + +The list of valid colors is here: +- https://rich.readthedocs.io/en/stable/appendix/colors.html +""" + +from rich_click import rich_click + +# Default styles +rich_click.STYLE_OPTION = "bold #fca311" +rich_click.STYLE_SWITCH = "bold #fca311" +rich_click.STYLE_ARGUMENT = "bold #00ff5f" +rich_click.STYLE_METAVAR = "bold #8700af" +rich_click.STYLE_METAVAR_APPEND = "dim yellow" +rich_click.STYLE_METAVAR_SEPARATOR = "dim" +rich_click.STYLE_HEADER_TEXT = "" +rich_click.STYLE_FOOTER_TEXT = "" +rich_click.STYLE_USAGE = "yellow" +rich_click.STYLE_USAGE_COMMAND = "bold" +rich_click.STYLE_DEPRECATED = "red" +rich_click.STYLE_HELPTEXT_FIRST_LINE = "" +rich_click.STYLE_HELPTEXT = "" +rich_click.STYLE_OPTION_HELP = "" +rich_click.STYLE_OPTION_DEFAULT = "bold #8700af" +rich_click.STYLE_OPTION_ENVVAR = "dim yellow" +rich_click.STYLE_REQUIRED_SHORT = "red" +rich_click.STYLE_REQUIRED_LONG = "dim red" +rich_click.STYLE_OPTIONS_PANEL_BORDER = "dim" +rich_click.ALIGN_OPTIONS_PANEL = "left" +rich_click.STYLE_OPTIONS_TABLE_SHOW_LINES = False +rich_click.STYLE_OPTIONS_TABLE_LEADING = 0 +rich_click.STYLE_OPTIONS_TABLE_PAD_EDGE = False +rich_click.STYLE_OPTIONS_TABLE_PADDING = (0, 1) +rich_click.STYLE_OPTIONS_TABLE_BOX = "" +rich_click.STYLE_OPTIONS_TABLE_ROW_STYLES = None +rich_click.STYLE_OPTIONS_TABLE_BORDER_STYLE = None +rich_click.STYLE_COMMANDS_PANEL_BORDER = "dim" +rich_click.ALIGN_COMMANDS_PANEL = "left" +rich_click.STYLE_COMMANDS_TABLE_SHOW_LINES = False +rich_click.STYLE_COMMANDS_TABLE_LEADING = 0 +rich_click.STYLE_COMMANDS_TABLE_PAD_EDGE = False +rich_click.STYLE_COMMANDS_TABLE_PADDING = (0, 1) +rich_click.STYLE_COMMANDS_TABLE_BOX = "" +rich_click.STYLE_COMMANDS_TABLE_ROW_STYLES = None +rich_click.STYLE_COMMANDS_TABLE_BORDER_STYLE = None +rich_click.STYLE_ERRORS_PANEL_BORDER = "red" +rich_click.ALIGN_ERRORS_PANEL = "left" +rich_click.STYLE_ERRORS_SUGGESTION = "dim" +rich_click.STYLE_ABORTED = "red" +rich_click.MAX_WIDTH = None # Set to an int to limit to that many characters +rich_click.COLOR_SYSTEM = "auto" # Set to None to disable colors + +# Fixed strings +rich_click.HEADER_TEXT = None +rich_click.FOOTER_TEXT = None +rich_click.DEPRECATED_STRING = "(Deprecated) " +rich_click.DEFAULT_STRING = "[default: {}]" +rich_click.ENVVAR_STRING = "[env var: {}]" +rich_click.REQUIRED_SHORT_STRING = "*" +rich_click.REQUIRED_LONG_STRING = "[required]" +rich_click.RANGE_STRING = " [{}]" +rich_click.APPEND_METAVARS_HELP_STRING = "({})" +rich_click.ARGUMENTS_PANEL_TITLE = "Arguments" +rich_click.OPTIONS_PANEL_TITLE = "Options" +rich_click.COMMANDS_PANEL_TITLE = "Commands" +rich_click.ERRORS_PANEL_TITLE = "Error" +rich_click.ERRORS_SUGGESTION = ( + None # Default: Try 'cmd -h' for help. Set to False to disable. +) +rich_click.ERRORS_EPILOGUE = None +rich_click.ABORTED_TEXT = "Aborted." + +# Behaviours +rich_click.SHOW_ARGUMENTS = True # Show positional arguments +rich_click.SHOW_METAVARS_COLUMN = ( + True # Show a column with the option metavar (eg. INTEGER) +) +rich_click.APPEND_METAVARS_HELP = ( + False # Append metavar (eg. [TEXT]) after the help text +) +rich_click.GROUP_ARGUMENTS_OPTIONS = ( + False # Show arguments with options instead of in own panel +) +rich_click.USE_MARKDOWN = True # Parse help strings as markdown +rich_click.USE_MARKDOWN_EMOJI = True # Parse emoji codes in markdown :smile: +rich_click.USE_RICH_MARKUP = ( + False # Parse help strings for rich markup (eg. [red]my text[/]) +) +rich_click.COMMAND_GROUPS = {} # Define sorted groups of panels to display subcommands +rich_click.OPTION_GROUPS = ( + {} +) # Define sorted groups of panels to display options and arguments +rich_click.USE_CLICK_SHORT_HELP = ( + False # Use click's default function to truncate help text +) diff --git a/src/fides/cli/commands/annotate.py b/src/fides/cli/commands/annotate.py index 4c6e390bee..95460b82d4 100644 --- a/src/fides/cli/commands/annotate.py +++ b/src/fides/cli/commands/annotate.py @@ -1,6 +1,6 @@ """Contains the annotate group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.utils import with_analytics from fides.core import annotate_dataset as _annotate_dataset @@ -10,7 +10,7 @@ @click.pass_context def annotate(ctx: click.Context) -> None: """ - Annotate fides resource types + Interactively annotate Fides resources. """ @@ -21,21 +21,21 @@ def annotate(ctx: click.Context) -> None: "-a", "--all-members", is_flag=True, - help="Annotate all dataset members, not just fields", + help="Annotate all parts of the dataset including schemas and tables.", ) @click.option( "-v", "--validate", is_flag=True, default=False, - help="Strictly validate annotation inputs.", + help="Validate annotation inputs.", ) @with_analytics def annotate_dataset( ctx: click.Context, input_filename: str, all_members: bool, validate: bool ) -> None: """ - Guided flow for annotating datasets. The dataset file will be edited in-place. + Interactively annotate a dataset file in-place. """ config = ctx.obj["CONFIG"] _annotate_dataset.annotate_dataset( diff --git a/src/fides/cli/commands/core.py b/src/fides/cli/commands/core.py index e929322fc7..7c80437d6e 100644 --- a/src/fides/cli/commands/core.py +++ b/src/fides/cli/commands/core.py @@ -1,14 +1,9 @@ """Contains all of the core CLI commands for fides.""" from typing import Optional -import click +import rich_click as click -from fides.cli.options import ( - dry_flag, - fides_key_option, - manifests_dir_argument, - verbose_flag, -) +from fides.cli.options import dry_flag, manifests_dir_argument, verbose_flag from fides.cli.utils import pretty_echo, print_divider, with_analytics from fides.core import audit as _audit from fides.core import evaluate as _evaluate @@ -24,13 +19,13 @@ @click.option( "--diff", is_flag=True, - help="Include any changes between server and local resources in the command output", + help="Print any diffs between the local & server objects", ) @manifests_dir_argument @with_analytics def push(ctx: click.Context, dry: bool, diff: bool, manifests_dir: str) -> None: """ - Validate local manifest files and persist any changes via the API server. + Parse local manifest files and upload them to the server. """ config = ctx.obj["CONFIG"] @@ -47,19 +42,28 @@ def push(ctx: click.Context, dry: bool, diff: bool, manifests_dir: str) -> None: @click.command() @click.pass_context @manifests_dir_argument -@fides_key_option +@click.option( + "--fides-key", + "-k", + help="The fides_key of a specific policy to evaluate.", + default="", +) @click.option( "-m", "--message", - help="A message that you can supply to describe the context of this evaluation.", + help="Describe the context of this evaluation.", ) @click.option( "-a", "--audit", is_flag=True, - help="Raise errors if resources are missing attributes required for building a data map.", + help="Validate that the objects in this evaluation produce a valid data map.", +) +@click.option( + "--dry", + is_flag=True, + help="Do not upload objects or results to the Fides webserver.", ) -@dry_flag @with_analytics def evaluate( ctx: click.Context, @@ -70,12 +74,7 @@ def evaluate( dry: bool, ) -> None: """ - Compare your System's Privacy Declarations with your Organization's Policy Rules. - - All local resources are applied to the server before evaluation. - - If your policy evaluation fails, it is expected that you will need to - either adjust your Privacy Declarations, Datasets, or Policies before trying again. + Evaluate System-level Privacy Declarations against Organization-level Policy Rules. """ config = ctx.obj["CONFIG"] @@ -127,10 +126,7 @@ def evaluate( @with_analytics def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None: """ - Reads the resource files that are stored in MANIFESTS_DIR and its subdirectories to verify - the validity of all manifest files. - - If the taxonomy is invalid, this command prints the error messages and triggers a non-zero exit code. + Parse all Fides objects located in the supplied directory. """ taxonomy = _parse.parse(manifests_dir=manifests_dir) if verbose: @@ -149,12 +145,7 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @with_analytics def pull(ctx: click.Context, manifests_dir: str, all_resources: Optional[str]) -> None: """ - Update local resource files by their fides_key to match their server versions. - - Alternatively, with the "--all" flag all resources from the server will be pulled - down into a local file. - - The pull is aborted if there are unstaged or untracked files in the manifests dir. + Update local resource files based on the state of the objects on the server. """ # Make the resources that are pulled configurable diff --git a/src/fides/cli/commands/crud.py b/src/fides/cli/commands/crud.py index a2826fbcbc..cdd0578c67 100644 --- a/src/fides/cli/commands/crud.py +++ b/src/fides/cli/commands/crud.py @@ -1,12 +1,12 @@ """Contains all of the CRUD-type CLI commands for fides.""" -import click +import rich_click as click import yaml from fides.cli.options import fides_key_argument, resource_type_argument from fides.cli.utils import handle_cli_response, print_divider, with_analytics from fides.core import api as _api from fides.core.api_helpers import get_server_resource, list_server_resources -from fides.core.utils import echo_green +from fides.core.utils import echo_green, echo_red @click.command() @@ -16,7 +16,7 @@ @with_analytics def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: """ - Delete a resource on the server. + Delete an object from the server. """ config = ctx.obj["CONFIG"] handle_cli_response( @@ -25,7 +25,11 @@ def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: resource_type=resource_type, resource_id=fides_key, headers=config.user.auth_header, - ) + ), + verbose=False, + ) + echo_green( + f"{resource_type.capitalize()} with fides_key '{fides_key}' successfully deleted." ) @@ -36,7 +40,7 @@ def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: @with_analytics def get_resource(ctx: click.Context, resource_type: str, fides_key: str) -> None: """ - View a resource from the server as a YAML object. + View an object from the server. """ config = ctx.obj["CONFIG"] resource = get_server_resource( @@ -54,9 +58,12 @@ def get_resource(ctx: click.Context, resource_type: str, fides_key: str) -> None @click.pass_context @resource_type_argument @with_analytics -def list_resources(ctx: click.Context, resource_type: str) -> None: +@click.option( + "--verbose", "-v", is_flag=True, help="Displays the entire object list as YAML." +) +def list_resources(ctx: click.Context, verbose: bool, resource_type: str) -> None: """ - Get a list of all resources of this type from the server and display them as YAML. + View all objects of a single type from the server. """ config = ctx.obj["CONFIG"] resources = list_server_resources( @@ -67,4 +74,14 @@ def list_resources(ctx: click.Context, resource_type: str) -> None: raw=True, ) print_divider() - echo_green(yaml.dump({resource_type: resources})) + if verbose: + echo_green(yaml.dump({resource_type: resources})) + else: + if resources: + sorted_fides_keys = sorted( + {resource["fides_key"] for resource in resources if resource} + ) + formatted_fides_keys = "\n ".join(sorted_fides_keys) + echo_green(f"{resource_type.capitalize()} list:\n {formatted_fides_keys}") + else: + echo_red(f"No {resource_type.capitalize()} resources found!") diff --git a/src/fides/cli/commands/db.py b/src/fides/cli/commands/db.py index 7f27a466a7..0d8d45faeb 100644 --- a/src/fides/cli/commands/db.py +++ b/src/fides/cli/commands/db.py @@ -1,5 +1,5 @@ """Contains the db group of the commands for fides.""" -import click +import rich_click as click from fides.cli.options import yes_flag from fides.cli.utils import handle_cli_response, with_analytics @@ -11,7 +11,7 @@ @click.pass_context def database(ctx: click.Context) -> None: """ - Database utility commands + Run actions against the application database. """ @@ -20,7 +20,7 @@ def database(ctx: click.Context) -> None: @with_analytics def db_init(ctx: click.Context) -> None: """ - Initialize the fides database. + Initialize the Fides database. """ config = ctx.obj["CONFIG"] handle_cli_response( @@ -38,7 +38,7 @@ def db_init(ctx: click.Context) -> None: @with_analytics def db_reset(ctx: click.Context, yes: bool) -> None: """ - Wipes all user-created data and resets the database back to its freshly initialized state. + Reset the database back to its initial state. """ config = ctx.obj["CONFIG"] if yes: diff --git a/src/fides/cli/commands/export.py b/src/fides/cli/commands/export.py index cc6daebb39..5d6879f944 100644 --- a/src/fides/cli/commands/export.py +++ b/src/fides/cli/commands/export.py @@ -1,5 +1,5 @@ """Contains the export group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( dry_flag, @@ -16,7 +16,7 @@ @click.pass_context def export(ctx: click.Context) -> None: """ - Export fides resource types + Export Fides data maps. """ @@ -31,7 +31,7 @@ def export_system( dry: bool, ) -> None: """ - Export a system in a data map format. + Export systems in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -55,7 +55,7 @@ def export_dataset( dry: bool, ) -> None: """ - Export a dataset in a data map format. + Export datasets in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -79,7 +79,7 @@ def export_organization( dry: bool, ) -> None: """ - Export an organization in a data map format. + Export organizations in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -111,17 +111,9 @@ def export_datamap( csv: bool, ) -> None: """ - Export a formatted data map to excel using the fides template. + Export a data map using the standard Fides template. The data map is comprised of an Organization, Systems, and Datasets. - - The default organization is used, however a custom one can be - passed if required. - - A custom manifest directory can be provided for the output location. - - The csv flag can be used to output data as csv, while the dry - flag can be used to return data to the console instead. """ config = ctx.obj["CONFIG"] _export.export_datamap( diff --git a/src/fides/cli/commands/generate.py b/src/fides/cli/commands/generate.py index edb5036853..5cd8d64392 100644 --- a/src/fides/cli/commands/generate.py +++ b/src/fides/cli/commands/generate.py @@ -1,5 +1,5 @@ """Contains the generate group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( aws_access_key_id_option, @@ -27,7 +27,7 @@ @click.pass_context def generate(ctx: click.Context) -> None: """ - Generate fides resource types + Programmatically generate Fides objects. """ @@ -35,7 +35,7 @@ def generate(ctx: click.Context) -> None: @click.pass_context def generate_dataset(ctx: click.Context) -> None: """ - Generate fides Dataset resources + Generate Fides datasets. """ @@ -54,13 +54,7 @@ def generate_dataset_db( include_null: bool, ) -> None: """ - Connect to a database directly via a SQLAlchemy-style connection string and - generate a dataset manifest file that consists of every schema/table/field. - Connection string can be supplied as an option or a credentials reference - to fides config. - - This is a one-time operation that does not track the state of the database. - It will need to be run again if the database schema changes. + Generate a Fides dataset by walking a database and recording every schema/table/field. """ actual_connection_string = handle_database_credentials_options( fides_config=ctx.obj["CONFIG"], @@ -79,7 +73,7 @@ def generate_dataset_db( @click.pass_context def generate_dataset_gcp(ctx: click.Context) -> None: """ - Generate fides Dataset resources for Google Cloud Platform + Generate Fides datasets from Google Cloud Platform. """ @@ -100,13 +94,7 @@ def generate_dataset_bigquery( include_null: bool, ) -> None: """ - Connect to a BigQuery dataset directly via a SQLAlchemy connection and - generate a dataset manifest file that consists of every schema/table/field. - A path to a google authorization keyfile can be supplied as an option, or a - credentials reference to fides config. - - This is a one-time operation that does not track the state of the dataset. - It will need to be run again if the dataset schema changes. + Generate a dataset object from BigQuery using a SQLAlchemy connection string. """ bigquery_config = handle_bigquery_config_options( @@ -127,7 +115,7 @@ def generate_dataset_bigquery( @click.pass_context def generate_system(ctx: click.Context) -> None: """ - Generate fides System resources + Generate Fides systems. """ @@ -150,13 +138,8 @@ def generate_system_okta( org_key: str, ) -> None: """ - Generates systems for your Okta applications. Connect to an Okta admin - account by providing an organization url and auth token or a credentials - reference to fides config. Auth token and organization url can also - be supplied by setting environment variables as defined by the okta python sdk. - - This is a one-time operation that does not track the state of the okta resources. - It will need to be run again if the tracked resources change. + Generates systems from your Okta applications. Connects via + an Okta admin account. """ config = ctx.obj["CONFIG"] okta_config = handle_okta_credentials_options( @@ -199,12 +182,6 @@ def generate_system_aws( """ Connect to an aws account and generate a system manifest file that consists of every tracked resource. - Credentials can be supplied as options, a credentials - reference to fides config, or boto3 environment configuration. - Tracked resources: [Redshift, RDS, DynamoDb, S3] - - This is a one-time operation that does not track the state of the aws resources. - It will need to be run again if the tracked resources change. """ config = ctx.obj["CONFIG"] aws_config = handle_aws_credentials_options( diff --git a/src/fides/cli/commands/scan.py b/src/fides/cli/commands/scan.py index 5fa89e3f84..9782069b01 100644 --- a/src/fides/cli/commands/scan.py +++ b/src/fides/cli/commands/scan.py @@ -1,6 +1,6 @@ """Contains the scan group of the commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( aws_access_key_id_option, @@ -28,7 +28,7 @@ @click.pass_context def scan(ctx: click.Context) -> None: """ - Scan external resource coverage against fides resources + Scan and report on discrepancies between Fides resource files and real infrastructure. """ @@ -36,7 +36,7 @@ def scan(ctx: click.Context) -> None: @click.pass_context def scan_dataset(ctx: click.Context) -> None: """ - Scan fides Dataset resources + Scan and report on Fides Dataset resources. """ @@ -55,15 +55,10 @@ def scan_dataset_db( coverage_threshold: int, ) -> None: """ - Connect to a database directly via a SQLAlchemy-style connection string and - compare the database objects to existing datasets. Connection string can be - supplied as an option or a credentials reference to fides config. + Scan a database directly using a SQLAlchemy-style connection string. - If there are fields within the database that aren't listed and categorized - within one of the datasets, this counts as lacking coverage. - - Outputs missing fields and has a non-zero exit if coverage is - under the stated threshold. + _If there are fields within the database that aren't listed and categorized + within one of the datasets, this counts as lacking coverage._ """ config = ctx.obj["CONFIG"] actual_connection_string = handle_database_credentials_options( @@ -85,7 +80,7 @@ def scan_dataset_db( @click.pass_context def scan_system(ctx: click.Context) -> None: """ - Scan fides System resources + Scan and report on Fides System resources. """ @@ -108,14 +103,7 @@ def scan_system_okta( coverage_threshold: int, ) -> None: """ - Scans your existing systems and compares them to found Okta applications. - Connect to an Okta admin account by providing an organization url and - auth token or a credentials reference to fides config. Auth token and - organization url can also be supplied by setting environment variables - as defined by the okta python sdk. - - Outputs missing resources and has a non-zero exit if coverage is - under the stated threshold. + Scan an Okta account and compare applications with annotated Fides Systems. """ config = ctx.obj["CONFIG"] @@ -154,13 +142,9 @@ def scan_system_aws( coverage_threshold: int, ) -> None: """ - Connect to an aws account and compares tracked resources to existing systems. - Credentials can be supplied as options, a credentials reference to fides - config, or boto3 environment configuration. - Tracked resources: [Redshift, RDS, DynamoDb, S3] + Scan an AWS account and compare objects with annotated Fides Systems. - Outputs missing resources and has a non-zero exit if coverage is - under the stated threshold. + _Scannable resources: [Redshift, RDS, DynamoDb, S3]_ """ config = ctx.obj["CONFIG"] aws_config = handle_aws_credentials_options( diff --git a/src/fides/cli/commands/user.py b/src/fides/cli/commands/user.py index 773712e547..395b1b7759 100644 --- a/src/fides/cli/commands/user.py +++ b/src/fides/cli/commands/user.py @@ -1,5 +1,5 @@ """Contains the user command group for the fides CLI.""" -import click +import rich_click as click from fides.cli.options import ( first_name_option, @@ -28,9 +28,7 @@ def create( ctx: click.Context, username: str, password: str, first_name: str, last_name: str ) -> None: """ - Use credentials from the credentials file to create a new user. - - Gives full permissions to the new user. + Use the credentials file to create a new user. Gives full permissions to the new user. """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url @@ -49,7 +47,7 @@ def create( @password_option def login(ctx: click.Context, username: str, password: str) -> None: """ - Use credentials to get a user access token and write it to a credentials file. + Generate a user access token and write it to a credentials file. """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url @@ -59,7 +57,9 @@ def login(ctx: click.Context, username: str, password: str) -> None: @user.command(name="permissions") @click.pass_context def get_permissions(ctx: click.Context) -> None: - """List the directly-assigned scopes and roles available to the current user.""" + """ + List the directly-assigned scopes and roles available to the current user. + """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url get_permissions_command(server_url=server_url) diff --git a/src/fides/cli/commands/util.py b/src/fides/cli/commands/util.py index 2c1acc2475..f0e10df194 100644 --- a/src/fides/cli/commands/util.py +++ b/src/fides/cli/commands/util.py @@ -2,7 +2,7 @@ from datetime import datetime, timezone from subprocess import CalledProcessError -import click +import rich_click as click import fides from fides.cli.utils import ( @@ -28,16 +28,14 @@ @click.command() @click.pass_context -@click.argument("fides_directory_location", default=".", type=click.Path(exists=True)) +@click.argument("fides_dir", default=".", type=click.Path(exists=True)) @click.option( "--opt-in", is_flag=True, help="Automatically opt-in to anonymous usage analytics." ) -def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> None: +def init(ctx: click.Context, fides_dir: str, opt_in: bool) -> None: """ - Initializes a fides instance, creating the default directory (`.fides/`) and - the configuration file (`fides.toml`) if necessary. - - Additionally, requests the ability to respectfully collect anonymous usage data. + Initializes a Fides instance by creating the default directory and + configuration file if not present. """ executed_at = datetime.now(timezone.utc) @@ -47,7 +45,7 @@ def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> Non click.echo("Initializing fides...") config, config_path = create_and_update_config_file( - config, fides_directory_location, opt_in=opt_in + config, fides_dir, opt_in=opt_in ) print_divider() @@ -61,7 +59,7 @@ def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> Non @with_analytics def status(ctx: click.Context) -> None: """ - Sends a request to the fides API healthcheck endpoint and prints the response. + Check Fides server availability. """ config = ctx.obj["CONFIG"] cli_version = fides.__version__ @@ -78,7 +76,9 @@ def status(ctx: click.Context) -> None: @click.option("--port", "-p", type=int, default=8080) def webserver(ctx: click.Context, port: int = 8080) -> None: """ - Starts the fides API server using Uvicorn. + Start the Fides webserver. + + _Requires Redis and Postgres to be configured and running_ """ # This has to be here to avoid a circular dependency from fides.api.main import start_webserver @@ -91,7 +91,7 @@ def webserver(ctx: click.Context, port: int = 8080) -> None: @with_analytics def worker(ctx: click.Context) -> None: """ - Starts a celery worker. + Start a Celery worker for the Fides webserver. """ # This has to be here to avoid a circular dependency from fides.api.ops.worker import start_worker @@ -123,9 +123,11 @@ def deploy(ctx: click.Context) -> None: is_flag=True, help="Disable the initialization of the Fides CLI, to run in headless mode.", ) -def up(ctx: click.Context, no_pull: bool = False, no_init: bool = False) -> None: +def up( + ctx: click.Context, no_pull: bool = False, no_init: bool = False +) -> None: # pragma: no cover """ - Starts the sample project via docker compose. + Starts a sample project via docker compose. """ check_virtualenv() @@ -138,7 +140,9 @@ def up(ctx: click.Context, no_pull: bool = False, no_init: bool = False) -> None try: check_fides_uploads_dir() + print("> Starting application...") start_application() + print("> Seeding data...") seed_example_data() click.clear() diff --git a/src/fides/cli/commands/view.py b/src/fides/cli/commands/view.py index fde0ab432c..c8711f53a3 100644 --- a/src/fides/cli/commands/view.py +++ b/src/fides/cli/commands/view.py @@ -1,9 +1,10 @@ """Contains the view group of the commands for fides.""" -import click -from toml import dumps as toml_dumps +import rich_click as click +from toml import dumps from fides.cli.utils import print_divider, with_analytics +from fides.core.utils import echo_red, get_credentials_path, read_credentials_file @click.group(name="view") @@ -27,7 +28,9 @@ def view_config( ctx: click.Context, section: str = "", exclude_unset: bool = False ) -> None: """ - Prints the configuration values being used. + Prints the configuration values being used for this command-line instance. + + _Note: To see the configuration values being used by the webserver, `GET` the `/api/v1/config` endpoint._ """ config = ctx.obj["CONFIG"] config_dict = config.dict(exclude_unset=exclude_unset) @@ -35,4 +38,22 @@ def view_config( config_dict = config_dict[section] print_divider() - print(toml_dumps(config_dict)) + print(dumps(config_dict)) + + +@view.command(name="credentials") +@click.pass_context +@with_analytics +def view_credentials(ctx: click.Context) -> None: + """ + Prints the credentials file. + """ + credentials_path = get_credentials_path() + try: + credentials = read_credentials_file(credentials_path) + except FileNotFoundError: + echo_red(f"No credentials file found at path: {credentials_path}") + raise SystemExit(1) + + print_divider() + print(dumps(credentials.dict())) diff --git a/src/fides/cli/options.py b/src/fides/cli/options.py index 5700f3ec83..6da30ac6c9 100644 --- a/src/fides/cli/options.py +++ b/src/fides/cli/options.py @@ -1,17 +1,21 @@ """ -Contains all of the options/arguments used by the CLI commands. +Reusable command-line arguments and options. """ from typing import Callable -import click +import rich_click as click from fideslang import model_list def coverage_threshold_option(command: Callable) -> Callable: - "Add a flag that assumes yes." + """An option decorator that sets a required coverage percentage.""" command = click.option( - "--coverage-threshold", "-c", type=click.IntRange(0, 100), default=100 + "--coverage-threshold", + "-c", + type=click.IntRange(0, 100), + default=100, + help="Set the coverage percentage for a passing scan.", )(command) return command @@ -40,7 +44,6 @@ def fides_key_option(command: Callable) -> Callable: "-k", "--fides-key", default="", - help="The fides_key of the single policy that you wish to evaluate.", )(command) return command @@ -48,9 +51,7 @@ def fides_key_option(command: Callable) -> Callable: def manifests_dir_argument(command: Callable) -> Callable: "Add the manifests_dir argument." command = click.argument( - "manifests_dir", - default=".fides/", - type=click.Path(exists=True), + "manifests_dir", type=click.Path(exists=True), default=".fides/" )(command) return command @@ -58,7 +59,7 @@ def manifests_dir_argument(command: Callable) -> Callable: def dry_flag(command: Callable) -> Callable: "Add a flag that prevents side-effects." command = click.option( - "--dry", is_flag=True, help="Prevent the persistance of any changes." + "--dry", is_flag=True, help="Do not upload results to the Fides webserver." )(command) return command @@ -69,7 +70,7 @@ def yes_flag(command: Callable) -> Callable: "--yes", "-y", is_flag=True, - help="Automatically responds 'yes' to any prompts.", + help="Automatically responds `yes` to any prompts.", )(command) return command @@ -90,7 +91,7 @@ def include_null_flag(command: Callable) -> Callable: command = click.option( "--include-null", is_flag=True, - help="Includes attributes that would otherwise be null.", + help="Include null attributes.", )(command) return command @@ -101,7 +102,8 @@ def organization_fides_key_option(command: Callable) -> Callable: "--org-key", "-k", default="default_organization", - help="The organization_fides_key you wish to export resources for.", + show_default=True, + help="The `organization_fides_key` of the `Organization` you want to specify.", )(command) return command @@ -112,6 +114,7 @@ def output_directory_option(command: Callable) -> Callable: "--output-dir", "-d", default=".fides/", + show_default=True, help="The output directory for the data map to be exported to.", )(command) return command @@ -122,7 +125,7 @@ def credentials_id_option(command: Callable) -> Callable: command = click.option( "--credentials-id", type=str, - help="Use credentials defined within fides config", + help="Use credentials keys defined within Fides config.", )(command) return command @@ -132,7 +135,7 @@ def connection_string_option(command: Callable) -> Callable: command = click.option( "--connection-string", type=str, - help="Use connection string option to connect to a database", + help="Use the connection string option to connect to a database.", )(command) return command @@ -142,7 +145,7 @@ def okta_org_url_option(command: Callable) -> Callable: command = click.option( "--org-url", type=str, - help="Use org url option to connect to okta. Requires options --org-url and --token", + help="Connect to Okta using an 'Org URL'. _Requires options `--org-url` & `--token`._", )(command) return command @@ -152,7 +155,7 @@ def okta_token_option(command: Callable) -> Callable: command = click.option( "--token", type=str, - help="Use token option to connect to okta. Requires options --org-url and --token", + help="Connect to Okta using a token. _Requires options `--org-url` and `--token`._", )(command) return command @@ -162,7 +165,7 @@ def aws_access_key_id_option(command: Callable) -> Callable: command = click.option( "--access_key_id", type=str, - help="Use access key id option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using an `Access Key ID`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command @@ -172,7 +175,7 @@ def aws_secret_access_key_option(command: Callable) -> Callable: command = click.option( "--secret_access_key", type=str, - help="Use access key option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using an `Access Key`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command @@ -182,7 +185,7 @@ def aws_region_option(command: Callable) -> Callable: command = click.option( "--region", type=str, - help="Use region option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using a specific `Region`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command diff --git a/src/fides/cli/utils.py b/src/fides/cli/utils.py index c0efbe22a6..5c7ec76e7a 100644 --- a/src/fides/cli/utils.py +++ b/src/fides/cli/utils.py @@ -10,8 +10,8 @@ from platform import system from typing import Any, Callable, Dict, Optional, Union -import click import requests +import rich_click as click from fideslog.sdk.python.client import AnalyticsClient from fideslog.sdk.python.event import AnalyticsEvent from fideslog.sdk.python.exceptions import AnalyticsError diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 5de3f9841e..53e2b2e241 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -51,13 +51,22 @@ def test_init_opt_in(test_cli_runner: CliRunner) -> None: assert result.exit_code == 0 -@pytest.mark.unit -def test_view_config(test_cli_runner: CliRunner) -> None: - result = test_cli_runner.invoke( - cli, ["view", "config"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} - ) - print(result.output) - assert result.exit_code == 0 +class TestView: + @pytest.mark.unit + def test_view_config(self, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, ["view", "config"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + ) + print(result.output) + assert result.exit_code == 0 + + @pytest.mark.unit + def test_view_credentials(self, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, ["view", "credentials"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + ) + print(result.output) + assert result.exit_code == 0 @pytest.mark.unit @@ -197,8 +206,8 @@ def test_audit(test_config_path: str, test_cli_runner: CliRunner) -> None: assert result.exit_code == 0 +@pytest.mark.integration class TestCRUD: - @pytest.mark.integration def test_get(self, test_config_path: str, test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke( cli, @@ -207,12 +216,36 @@ def test_get(self, test_config_path: str, test_cli_runner: CliRunner) -> None: print(result.output) assert result.exit_code == 0 - @pytest.mark.integration + def test_delete(self, test_config_path: str, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, + ["-f", test_config_path, "delete", "system", "demo_marketing_system"], + ) + print(result.output) + assert result.exit_code == 0 + def test_ls(self, test_config_path: str, test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke(cli, ["-f", test_config_path, "ls", "system"]) print(result.output) assert result.exit_code == 0 + def test_ls_verbose( + self, test_config_path: str, test_cli_runner: CliRunner + ) -> None: + result = test_cli_runner.invoke( + cli, ["-f", test_config_path, "ls", "system", "--verbose"] + ) + print(result.output) + assert result.exit_code == 0 + + def test_ls_no_resources_found( + self, test_config_path: str, test_cli_runner: CliRunner + ) -> None: + """This test only workss because we don't have any registry resources by default.""" + result = test_cli_runner.invoke(cli, ["-f", test_config_path, "ls", "registry"]) + print(result.output) + assert result.exit_code == 0 + class TestEvaluate: @pytest.mark.integration From b3140a3be383774f8c88fe0cd39c8951be1bc444 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:52:52 -0800 Subject: [PATCH 11/20] Bump plotly from 5.4 to 5.13.1 (#2707) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ad520d2842..daa52f9d8b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -24,7 +24,7 @@ openpyxl==3.0.9 packaging==21.3 pandas==1.4.3 passlib[bcrypt]==1.7.4 -plotly==5.4 +plotly==5.13.1 psycopg2-binary==2.9.5 pydantic<1.10.2 pydash==5.1.1 From 6257b42bbe9aa1902d33c66ffffa4981de5e4d9b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:56:07 -0800 Subject: [PATCH 12/20] Bump slowapi from 0.1.5 to 0.1.7 (#2513) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index daa52f9d8b..e65bd1de2d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -36,7 +36,7 @@ pyyaml>=5,<6 redis==3.5.3 rich-click==1.6.1 sendgrid==6.9.7 -slowapi==0.1.5 +slowapi==0.1.7 snowflake-sqlalchemy==1.4.3 sqlalchemy[asyncio]==1.4.27 sqlalchemy-citext==1.8.0 From 3eb6ab2dfcd6c5fc1a33dc243282e173786a6207 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:58:33 -0800 Subject: [PATCH 13/20] Bump actions/checkout from 2 to 3 (#2709) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/frontend_checks.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/frontend_checks.yml b/.github/workflows/frontend_checks.yml index 2b8aad1567..99a433f0a0 100644 --- a/.github/workflows/frontend_checks.yml +++ b/.github/workflows/frontend_checks.yml @@ -52,7 +52,7 @@ jobs: node-version: [16.x] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 @@ -118,7 +118,7 @@ jobs: node-version: [16.x] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 From 342a5d54dfecf5e6368cee4690ee535cb26eb880 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:59:23 -0800 Subject: [PATCH 14/20] Bump pytest from 7.1.2 to 7.2.2 (#2748) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index f99f4e14cc..2c0f4c9ba2 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -10,7 +10,7 @@ pylint==2.15.4 pytest-asyncio==0.19.0 pytest-cov==4.0.0 pytest-env==0.6.2 -pytest==7.1.2 +pytest==7.2.2 requests-mock==1.10.0 setuptools>=64.0.2 sqlalchemy-stubs From be67604cb236ea04bee672f8b3ed9e3ff9e0925c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 23:22:38 -0800 Subject: [PATCH 15/20] Bump hashicorp/vault-action from 2.4.2 to 2.5.0 (#2427) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/backend_checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index 41913d151e..445f95ea66 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -290,7 +290,7 @@ jobs: uses: actions/checkout@v3 - name: Get Vault Token - uses: hashicorp/vault-action@v2.4.2 + uses: hashicorp/vault-action@v2.5.0 with: url: ${{ secrets.VAULT_ADDR }} namespace: ${{ secrets.VAULT_NAMESPACE }} From 98663d3191fbd68d51a56017d4132329f7627dd2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Mar 2023 00:52:06 -0800 Subject: [PATCH 16/20] Bump fastapi[all] from 0.82.0 to 0.89.1 (#2246) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Thomas Co-authored-by: Thomas Co-authored-by: Adam Sachs --- .github/workflows/backend_checks.yml | 6 +-- noxfiles/test_setup_nox.py | 6 +-- requirements.txt | 2 +- .../api/v1/endpoints/connection_endpoints.py | 1 + .../ops/api/v1/endpoints/user_endpoints.py | 1 + .../service/connectors/fides/fides_client.py | 42 +++++++++------- .../ops/service/connectors/fides_connector.py | 7 +++ src/fides/cli/utils.py | 9 +++- src/fides/lib/oauth/schemas/oauth.py | 4 +- tests/ctl/cli/test_utils.py | 33 ++++--------- tests/ctl/conftest.py | 14 ++++-- tests/fixtures/application_fixtures.py | 4 ++ .../api/v1/endpoints/test_config_endpoints.py | 3 +- .../test_consent_request_endpoints.py | 6 ++- .../api/v1/endpoints/test_oauth_endpoints.py | 2 +- .../test_privacy_request_endpoints.py | 6 ++- .../endpoints/test_saas_config_endpoints.py | 2 +- tests/ops/integration_test_config.toml | 1 + .../connectors/fides/test_fides_client.py | 49 +++++++------------ .../connectors/test_fides_connector.py | 4 +- 20 files changed, 105 insertions(+), 97 deletions(-) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index 445f95ea66..23f114c1d6 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -195,7 +195,7 @@ jobs: matrix: python_version: ["3.8.14", "3.9.14", "3.10.7"] runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' steps: @@ -232,7 +232,7 @@ jobs: matrix: python_version: ["3.8.14", "3.9.14", "3.10.7"] runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' steps: @@ -263,7 +263,7 @@ jobs: External-SaaS-Connectors: needs: Build runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' permissions: diff --git a/noxfiles/test_setup_nox.py b/noxfiles/test_setup_nox.py index 8024d23b64..7c6b8126e7 100644 --- a/noxfiles/test_setup_nox.py +++ b/noxfiles/test_setup_nox.py @@ -39,10 +39,11 @@ def pytest_ctl(session: Session, mark: str, coverage_arg: str) -> None: "-f", INTEGRATION_COMPOSE_FILE, "up", - "-d", + "--wait", IMAGE_NAME, ) session.run(*start_command, external=True) + session.run(*LOGIN, external=True) run_command = ( "docker", "exec", @@ -60,7 +61,6 @@ def pytest_ctl(session: Session, mark: str, coverage_arg: str) -> None: "OKTA_CLIENT_TOKEN", "-e", "BIGQUERY_CONFIG", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", @@ -123,7 +123,6 @@ def pytest_ops(session: Session, mark: str, coverage_arg: str) -> None: "BIGQUERY_KEYFILE_CREDS", "-e", "BIGQUERY_DATASET", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", @@ -154,7 +153,6 @@ def pytest_ops(session: Session, mark: str, coverage_arg: str) -> None: "VAULT_NAMESPACE", "-e", "VAULT_TOKEN", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", diff --git a/requirements.txt b/requirements.txt index e65bd1de2d..65fa05f035 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ colorama>=0.4.3 cryptography==38.0.3 dask==2022.9.2 deepdiff==5.8.1 -fastapi[all]==0.82.0 +fastapi[all]==0.89.1 fastapi-caching[redis]==0.3.0 fastapi-pagination[sqlalchemy]~= 0.10.0 fideslang==1.3.3 diff --git a/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py b/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py index 71f4556eb4..8a2ffd1a20 100644 --- a/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py @@ -210,6 +210,7 @@ def patch_connections( CONNECTION_BY_KEY, dependencies=[Security(verify_oauth_client, scopes=[CONNECTION_DELETE])], status_code=HTTP_204_NO_CONTENT, + response_model=None, ) def delete_connection( connection_key: FidesKey, *, db: Session = Depends(deps.get_db) diff --git a/src/fides/api/ops/api/v1/endpoints/user_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_endpoints.py index 600e493ebe..4de49a50cf 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_endpoints.py @@ -121,6 +121,7 @@ def update_user_password( urls.USER_FORCE_PASSWORD_RESET, dependencies=[Security(verify_oauth_client, scopes=[USER_PASSWORD_RESET])], status_code=HTTP_200_OK, + response_model=UserResponse, ) def force_update_password( *, diff --git a/src/fides/api/ops/service/connectors/fides/fides_client.py b/src/fides/api/ops/service/connectors/fides/fides_client.py index f45dbca240..db214c6a4e 100644 --- a/src/fides/api/ops/service/connectors/fides/fides_client.py +++ b/src/fides/api/ops/service/connectors/fides/fides_client.py @@ -2,10 +2,9 @@ from typing import Any, Dict, List, Optional -import requests -from httpx import AsyncClient +import httpx +from httpx import AsyncClient, Client, HTTPStatusError, Request, RequestError, Timeout from loguru import logger -from requests import PreparedRequest, Request, RequestException, Session from fides.api.ctl.utils.errors import FidesError from fides.api.ops.api.v1 import urn_registry as urls @@ -32,7 +31,7 @@ class FidesClient: """ - A helper client class to broker communications between Fides servers. + A helper client to broker communications between Fides servers. """ def __init__( @@ -40,8 +39,12 @@ def __init__( uri: str, username: str, password: str, + connection_read_timeout: float = 30.0, ): - self.session = Session() + # Enable setting a custom `read` timeout + # to account for privacy request executions + self.session = Client(timeout=Timeout(5.0, read=connection_read_timeout)) + self.uri = uri self.username = username self.password = password @@ -55,14 +58,14 @@ def login(self) -> None: self.username, ) try: - response = requests.post( + response = httpx.post( f"{self.uri}{urls.V1_URL_PREFIX}{urls.LOGIN}", json=ul.dict() ) - except RequestException as e: + except RequestError as e: logger.error("Error logging in on remote Fides {}: {}", self.uri, str(e)) raise e - if response.ok: + if response.is_success: self.token = response.json()["token_data"]["access_token"] logger.info( "Successfully logged in to remote fides {} with username '{}'", @@ -81,21 +84,21 @@ def authenticated_request( query_params: Optional[Dict[str, Any]] = {}, data: Optional[Any] = None, json: Optional[Any] = None, - ) -> PreparedRequest: + ) -> Request: if not self.token: raise FidesError( f"Unable to create authenticated request. No token for Fides connector for server {self.uri}" ) - req: PreparedRequest = Request( + req: Request = self.session.build_request( method=method, url=f"{self.uri}{path}", headers=headers, params=query_params, data=data, json=json, - ).prepare() + ) req.headers["Authorization"] = f"Bearer {self.token}" return req @@ -117,15 +120,17 @@ def create_privacy_request( external_id, self.uri, ) - request: PreparedRequest = self.authenticated_request( + request: Request = self.authenticated_request( method="POST", path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUEST_AUTHENTICATED, json=[pr.dict()], ) response = self.session.send(request) - if not response.ok: + + if not response.is_success: logger.error("Error creating privacy request on remote Fides {}", self.uri) response.raise_for_status() + if response.json()["failed"]: # TODO better handle errored state here? raise FidesError( @@ -199,7 +204,7 @@ async def poll_for_request_completion( f"Privacy request [{privacy_request_id}] on remote Fides {self.uri} is in an unknown state. Look at the remote Fides for more information." ) - def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]]: + def request_status(self, privacy_request_id: str = "") -> List[Dict[str, Any]]: """ Return privacy request object that tracks its status """ @@ -215,15 +220,16 @@ def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]] self.uri, ) - request: PreparedRequest = self.authenticated_request( + request: Request = self.authenticated_request( method="GET", path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUESTS, query_params={"request_id": privacy_request_id} if privacy_request_id else None, ) - response = self.session.send(request, timeout=5) - if not response.ok: + response = self.session.send(request) + + if not response.is_success: logger.error( "Error retrieving status of privacy request [{}] on remote Fides {}", privacy_request_id, @@ -266,7 +272,7 @@ def retrieve_request_results( headers={"Authorization": f"Bearer {self.token}"}, ) response = self.session.send(request) - except requests.exceptions.HTTPError as e: + except HTTPStatusError as e: logger.error( "Error retrieving data from child server for privacy request {}: {}", privacy_request_id, diff --git a/src/fides/api/ops/service/connectors/fides_connector.py b/src/fides/api/ops/service/connectors/fides_connector.py index c0c7529248..d07f0d7540 100644 --- a/src/fides/api/ops/service/connectors/fides_connector.py +++ b/src/fides/api/ops/service/connectors/fides_connector.py @@ -48,11 +48,18 @@ def query_config(self, node: TraversalNode) -> QueryConfig[Any]: def create_client(self) -> FidesClient: """Returns a client used to connect to a Fides instance""" config = FidesConnectorSchema(**self.configuration.secrets or {}) + + # use polling_timeout here to provide a base read timeout + # on the HTTP client underlying the FidesClient, since even + # in non-polling context, we may hit a blocking HTTP call + # e.g., in privacy request creation we can block until completion client = FidesClient( uri=config.uri, username=config.username, password=config.password, + connection_read_timeout=self.polling_timeout, ) + client.login() return client diff --git a/src/fides/cli/utils.py b/src/fides/cli/utils.py index 5c7ec76e7a..89882daaac 100644 --- a/src/fides/cli/utils.py +++ b/src/fides/cli/utils.py @@ -72,6 +72,12 @@ def check_server_health(server_url: str, verbose: bool = True) -> requests.Respo return health_response +def compare_application_versions(server_version: str, cli_version: str) -> bool: + """Normalize and compare application versions.""" + normalize_version = lambda v: str(v).replace(".dirty", "", 1) + return normalize_version(server_version) == normalize_version(cli_version) + + def check_server(cli_version: str, server_url: str, quiet: bool = False) -> None: """Runs a health check and a version check against the server.""" @@ -82,8 +88,7 @@ def check_server(cli_version: str, server_url: str, quiet: bool = False) -> None raise SystemExit(1) server_version = health_response.json()["version"] - normalize_version = lambda v: str(v).replace(".dirty", "", 1) - if normalize_version(server_version) == normalize_version(cli_version): + if compare_application_versions(server_version, cli_version): if not quiet: echo_green( "Server is reachable and the client/server application versions match." diff --git a/src/fides/lib/oauth/schemas/oauth.py b/src/fides/lib/oauth/schemas/oauth.py index 7275195109..97b86a5bae 100644 --- a/src/fides/lib/oauth/schemas/oauth.py +++ b/src/fides/lib/oauth/schemas/oauth.py @@ -44,7 +44,9 @@ def __init__( ) async def __call__(self, request: Request) -> Optional[str]: - authorization: str = request.headers.get("Authorization") + authorization: Optional[str] = request.headers.get("Authorization") + if not authorization: + raise InvalidAuthorizationSchemeError() scheme, param = get_authorization_scheme_param(authorization) if not authorization or scheme.lower() != "bearer": if self.auto_error: diff --git a/tests/ctl/cli/test_utils.py b/tests/ctl/cli/test_utils.py index 2ac38eae08..0f9081c8d2 100644 --- a/tests/ctl/cli/test_utils.py +++ b/tests/ctl/cli/test_utils.py @@ -4,10 +4,9 @@ import click import pytest -from requests_mock import Mocker import fides.cli.utils as utils -from fides.core.config import FidesConfig, get_config +from fides.core.config import FidesConfig from tests.ctl.conftest import orig_requests_get @@ -24,42 +23,30 @@ def test_check_server_bad_ping(test_client, monkeypatch) -> None: @pytest.mark.unit @pytest.mark.parametrize( - "server_version, cli_version, expected_output, quiet", + "server_version, cli_version, expected_result", [ - ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5", "application versions match", False), - ("1.6.0+7.ge953df5", "1.6.0+9.ge953df5", "Mismatched versions!", False), + ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5", True), + ("1.6.0+7.ge953df5", "1.6.0+9.ge953df5", False), ( "1.6.0+7.ge953df5", "1.6.0+7.ge953df5.dirty", - "application versions match", - False, + True, ), ( "1.6.0+7.ge953df5.dirty", "1.6.0+7.ge953df5", - "application versions match", - False, + True, ), - ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5.dirty", None, True), ], ) def test_check_server_version_comparisons( - requests_mock: Mocker, - capsys: pytest.CaptureFixture, server_version: str, cli_version: str, - expected_output: str, - quiet: bool, + expected_result: str, ) -> None: - """Check that comparing versions works""" - fake_url = "http://fake_address:8080" - requests_mock.get(f"{fake_url}/health", json={"version": server_version}) - utils.check_server(cli_version, "http://fake_address:8080", quiet=quiet) - captured = capsys.readouterr() - if expected_output is None: - assert captured.out == "" - else: - assert expected_output in captured.out + """Check that version comparison works.""" + actual_result = utils.compare_application_versions(server_version, cli_version) + assert expected_result == actual_result @pytest.mark.unit diff --git a/tests/ctl/conftest.py b/tests/ctl/conftest.py index 7e86e7e0d3..be9f36eb5e 100644 --- a/tests/ctl/conftest.py +++ b/tests/ctl/conftest.py @@ -19,7 +19,10 @@ @pytest.fixture(scope="session") def monkeysession(): - """monkeypatch fixture at the session level instead of the function level""" + """ + Monkeypatch at the session level instead of the function level. + Automatically undoes the monkeypatching when the session finishes. + """ mpatch = MonkeyPatch() yield mpatch mpatch.undo() @@ -27,10 +30,11 @@ def monkeysession(): @pytest.fixture(autouse=True, scope="session") def monkeypatch_requests(test_client, monkeysession) -> None: - """The requests library makes requests against the running webserver - which talks to the application db. This monkeypatching operation - makes `requests` calls from src/fides/core/api.py in a test - context talk to the test db instead""" + """ + Some places within the application, for example `fides.core.api`, use the `requests` + library to interact with the webserver. This fixture patches those `requests` calls + so that all of those tests instead interact with the test instance. + """ monkeysession.setattr(requests, "get", test_client.get) monkeysession.setattr(requests, "post", test_client.post) monkeysession.setattr(requests, "put", test_client.put) diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index 083f5f5cab..3faa6bd326 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -126,6 +126,9 @@ "uri": pydash.get(integration_config, "fides_example.uri"), "username": pydash.get(integration_config, "fides_example.username"), "password": pydash.get(integration_config, "fides_example.password"), + "polling_timeout": pydash.get( + integration_config, "fides_example.polling_timeout" + ), }, } @@ -1604,6 +1607,7 @@ def test_fides_client( fides_connector_example_secrets["uri"], fides_connector_example_secrets["username"], fides_connector_example_secrets["password"], + fides_connector_example_secrets["polling_timeout"] ) diff --git a/tests/ops/api/v1/endpoints/test_config_endpoints.py b/tests/ops/api/v1/endpoints/test_config_endpoints.py index e76cdf29e7..018c7d04c0 100644 --- a/tests/ops/api/v1/endpoints/test_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_config_endpoints.py @@ -329,7 +329,8 @@ def test_get_application_config( # then we test that we can GET them auth_header = generate_auth_header([scopes.CONFIG_READ]) - response = api_client.get( + response = api_client.request( + "GET", url, headers=auth_header, params={"api_set": True}, diff --git a/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py b/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py index 094918993b..c691d9530c 100644 --- a/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py @@ -375,7 +375,8 @@ class TestGetConsentUnverified: def test_consent_unverified_no_consent_request_id(self, api_client): data = {"code": "12345"} - response = api_client.get( + response = api_client.request( + "GET", f"{V1_URL_PREFIX}{CONSENT_REQUEST_PREFERENCES_WITH_ID.format(consent_request_id='non_existent_consent_id')}", json=data, ) @@ -388,7 +389,8 @@ def test_consent_unverified_no_consent_request_id(self, api_client): def test_consent_unverified_verification_error(self, api_client): data = {"code": "12345"} - response = api_client.get( + response = api_client.request( + "GET", f"{V1_URL_PREFIX}{CONSENT_REQUEST_PREFERENCES_WITH_ID.format(consent_request_id='non_existent_consent_id')}", json=data, ) diff --git a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py index dc815b4987..192bc30bd7 100644 --- a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py @@ -501,7 +501,7 @@ def test_callback_for_valid_state( response = api_client.get( callback_url, params={"code": "abc", "state": "new_request"} ) - assert response.ok + response.raise_for_status() get_access_token_mock.assert_called_once() authentication_request.delete(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 611d651156..8e64bed76a 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -1532,7 +1532,8 @@ def test_sort_privacy_request_by_due_date( api_client.post(url, json=data) auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_READ]) - resp = api_client.get( + resp = api_client.request( + "GET", f"{url}?sort_direction=asc&sort_field=due_date", json=data, headers=auth_header, @@ -1542,7 +1543,8 @@ def test_sort_privacy_request_by_due_date( for i, request in enumerate(asc_response_data): assert request["days_left"] == days_left_values[i] - resp = api_client.get( + resp = api_client.request( + "GET", f"{url}?sort_direction=desc&sort_field=due_date", json=data, headers=auth_header, diff --git a/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py b/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py index 03630a7c91..06a4b785ca 100644 --- a/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py @@ -450,5 +450,5 @@ def test_get_authorize_url( authorization_url_mock.return_value = authorization_url auth_header = generate_auth_header([CONNECTION_AUTHORIZE]) response = api_client.get(authorize_url, headers=auth_header) - assert response.ok + response.raise_for_status() assert response.text == f'"{authorization_url}"' diff --git a/tests/ops/integration_test_config.toml b/tests/ops/integration_test_config.toml index a2112f7120..e34d6a1f52 100644 --- a/tests/ops/integration_test_config.toml +++ b/tests/ops/integration_test_config.toml @@ -57,3 +57,4 @@ port=5432 uri="http://fides:8080" username="root_user" password="Testpassword1!" +polling_timeout=1800 diff --git a/tests/ops/service/connectors/fides/test_fides_client.py b/tests/ops/service/connectors/fides/test_fides_client.py index 5ab79ec58a..cd74561635 100644 --- a/tests/ops/service/connectors/fides/test_fides_client.py +++ b/tests/ops/service/connectors/fides/test_fides_client.py @@ -2,8 +2,7 @@ from unittest import mock import pytest -from httpx import AsyncClient -from requests import HTTPError, Session +from httpx import AsyncClient, Client, HTTPStatusError from fides.api.ctl.utils.errors import FidesError from fides.api.ops.models.privacy_request import PrivacyRequest, PrivacyRequestStatus @@ -17,8 +16,8 @@ class MockResponse: A class to mock Fides API responses """ - def __init__(self, ok, json_data): - self.ok = ok + def __init__(self, is_success, json_data): + self.is_success = is_success self.json_data = json_data def json(self): @@ -43,7 +42,7 @@ class TestFidesClientUnit: """ @mock.patch( - "requests.post", + "httpx.post", side_effect=[ MockResponse(True, {"token_data": {"access_token": SAMPLE_TOKEN}}) ], @@ -69,7 +68,7 @@ def test_authenticated_request(self, mock_login, test_fides_client: FidesClient) ) assert request.method == "GET" assert request.url == test_fides_client.uri + "/testpath" - assert len(request.headers) == 2 + assert len(request.headers) == 7 assert "another_header" in request.headers assert request.headers["another_header"] == "header_value" assert "Authorization" in request.headers @@ -88,7 +87,7 @@ def test_authenticated_request_not_logged_in(self, test_fides_client: FidesClien assert "No token" in str(exc) @mock.patch( - "requests.post", + "httpx.post", side_effect=[ MockResponse(True, {"token_data": {"access_token": SAMPLE_TOKEN}}) ], @@ -112,22 +111,6 @@ def test_authenticated_request_parameters( == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - # test form data passed as tuples - request = test_fides_client.authenticated_request( - "POST", - path="/testpath", - query_params={"param1": "value1", "param2": "value2"}, - data=[("key1", "value1"), ("key2", "value2")], - ) - assert "Authorization" in request.headers - assert request.headers["Authorization"] == f"Bearer {SAMPLE_TOKEN}" - assert request.method == "POST" - assert ( - request.url - == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" - ) - assert request.body == "key1=value1&key2=value2" - # test form data passed as dict request = test_fides_client.authenticated_request( "POST", @@ -142,7 +125,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == "key1=value1&key2=value2" + request.read() + assert request.content == b"key1=value1&key2=value2" # test body passed as string literal request = test_fides_client.authenticated_request( @@ -158,7 +142,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == "testbody" + request.read() + assert request.content == b"testbody" # test json body passed as a dict request = test_fides_client.authenticated_request( @@ -174,7 +159,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == b'{"field1": "value1"}' + request.read() + assert request.content == b'{"field1": "value1"}' # test json body passed as a list request = test_fides_client.authenticated_request( @@ -190,7 +176,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == b'[{"field1": "value1"}]' + request.read() + assert request.content == b'[{"field1": "value1"}]' @pytest.mark.asyncio def test_poll_for_completion( @@ -301,7 +288,7 @@ def test_login_bad_credentials( # so that we don't call `create_client()`, which performs # login as part of initialization - with pytest.raises(HTTPError): + with pytest.raises(HTTPStatusError): test_fides_client_bad_credentials.login() assert test_fides_client_bad_credentials.token is None @@ -317,7 +304,7 @@ def test_create_privacy_request( Test that properly configured fides client can create and execute a valid access privacy request Inspired by `test_privacy_request_endpoints.TestCreatePrivacyRequest` """ - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) pr_id = authenticated_fides_client.create_privacy_request( external_id="test_external_id", @@ -339,14 +326,14 @@ def test_request_status_no_privacy_request( privacy request ID specified. This acts as a basic test to validate we can successfully hit authenticated endpoints. """ - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) statuses = authenticated_fides_client.request_status() assert len(statuses) == 0 def test_request_status_privacy_request( self, authenticated_fides_client: FidesClient, policy, monkeypatch, api_client ): - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) pr_id = authenticated_fides_client.create_privacy_request( external_id="test_external_id", diff --git a/tests/ops/service/connectors/test_fides_connector.py b/tests/ops/service/connectors/test_fides_connector.py index a56bc96624..92277ec05d 100644 --- a/tests/ops/service/connectors/test_fides_connector.py +++ b/tests/ops/service/connectors/test_fides_connector.py @@ -2,7 +2,7 @@ from typing import Tuple import pytest -from requests import Session +from httpx import Client from fides.api.ops.graph.traversal import TraversalNode from fides.api.ops.models.connectionconfig import ConnectionConfig, ConnectionTestStatus @@ -128,7 +128,7 @@ def test_retrieve_data( # Monkey patch both Session.send and the httpx.AsyncClient. Both of these will just # make requests to the running webserver which is connected to the application db, # but we need them to talk to the test db in pytest - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) monkeypatch.setattr( request_service, "get_async_client", lambda: async_api_client ) From 618b6265db67c3d1fc779c92a49deaff93da6dcc Mon Sep 17 00:00:00 2001 From: Sebastian Sangervasi <2236777+ssangervasi@users.noreply.github.com> Date: Mon, 6 Mar 2023 06:55:26 -0800 Subject: [PATCH 17/20] test_env/pc: Add GPC settings to test env privacy center config (#2701) --- .../data/test_env/privacy_center_config/config.json | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/fides/data/test_env/privacy_center_config/config.json b/src/fides/data/test_env/privacy_center_config/config.json index 082ad73189..2a530a619e 100644 --- a/src/fides/data/test_env/privacy_center_config/config.json +++ b/src/fides/data/test_env/privacy_center_config/config.json @@ -45,7 +45,10 @@ "name": "Data Sales or Sharing", "description": "We may use some of your personal information for behavioral advertising purposes, which may be interpreted as 'Data Sales' or 'Data Sharing' under regulations such as CCPA, CPRA, VCDPA, etc.", "url": "https://example.com/privacy#data-sales", - "default": true, + "default": { + "value": true, + "globalPrivacyControl": false + }, "highlight": false, "cookieKeys": ["data_sales"], "executable": false @@ -55,7 +58,10 @@ "name": "Email Marketing", "description": "We may use some of your personal information to contact you about our products & services.", "url": "https://example.com/privacy#email-marketing", - "default": true, + "default": { + "value": true, + "globalPrivacyControl": false + }, "highlight": false, "cookieKeys": [], "executable": false From e07f8961e744ee2c2010ee1f2b3e06b81e4cb1f2 Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Mon, 6 Mar 2023 10:03:14 -0500 Subject: [PATCH 18/20] Add custom fields to datamap (#2531) Co-authored-by: Paul Sanders Co-authored-by: SteveDMurphy --- src/fides/api/ctl/database/crud.py | 60 ++++++- src/fides/api/ctl/routes/crud.py | 4 +- src/fides/api/ctl/routes/datamap.py | 36 +++- src/fides/api/ctl/sql_models.py | 21 ++- src/fides/core/export.py | 245 ++++++++++++++++++++-------- src/fides/core/export_helpers.py | 100 +++++++++--- src/fides/core/utils.py | 11 +- tests/ctl/api/test_datamap.py | 53 +++++- tests/ctl/core/test_export.py | 65 +++++++- tests/ctl/core/test_utils.py | 4 +- tests/ctl/database/test_crud.py | 88 +++++++++- tests/ops/api/v1/test_main.py | 2 +- 12 files changed, 576 insertions(+), 113 deletions(-) diff --git a/src/fides/api/ctl/database/crud.py b/src/fides/api/ctl/database/crud.py index 5882827bbb..b4cde58e96 100644 --- a/src/fides/api/ctl/database/crud.py +++ b/src/fides/api/ctl/database/crud.py @@ -2,7 +2,7 @@ Contains all of the generic CRUD endpoints that can be generated programmatically for each resource. """ -from typing import Dict, List, Tuple +from typing import Any, Dict, List, Tuple from loguru import logger as log from sqlalchemy import column @@ -14,6 +14,10 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select +from fides.api.ctl.sql_models import ( # type: ignore[attr-defined] + CustomField, + CustomFieldDefinition, +) from fides.api.ctl.utils import errors from fides.lib.db.base import Base # type: ignore[attr-defined] @@ -84,6 +88,59 @@ async def get_resource( return sql_resource +async def get_resource_with_custom_fields( + sql_model: Base, fides_key: str, async_session: AsyncSession +) -> Dict[str, Any]: + """Get a resource from the databse by its FidesKey including it's custom fields. + + Returns a dictionary of that resource. + """ + resource = await get_resource(sql_model, fides_key, async_session) + resource_dict = resource.__dict__ + resource_dict.pop("_sa_instance_state", None) + + with log.contextualize(sql_model=sql_model.__name__, fides_key=fides_key): + async with async_session.begin(): + try: + log.debug("Fetching custom fields for resource") + query = ( + select(CustomFieldDefinition.name, CustomField.value) + .join( + CustomField, + CustomField.custom_field_definition_id + == CustomFieldDefinition.id, + ) + .where( + (CustomField.resource_id == resource.fides_key) + & ( # pylint: disable=singleton-comparison + CustomFieldDefinition.active == True + ) + ) + ) + result = await async_session.execute(query) + except SQLAlchemyError: + sa_error = errors.QueryError() + log.bind(error=sa_error.detail["error"]).error( # type: ignore[index] + "Failed to fetch custom fields" + ) + raise sa_error + + custom_fields = result.mappings().all() + + if not custom_fields: + return resource_dict + + for field in custom_fields: + if field["name"] in resource_dict: + resource_dict[ + field["name"] + ] = f"{resource_dict[field['name']]}, {', '.join(field['value'])}" + else: + resource_dict[field["name"]] = ", ".join(field["value"]) + + return resource_dict + + async def list_resource(sql_model: Base, async_session: AsyncSession) -> List[Base]: """ Get a list of all of the resources of this type from the database. @@ -216,7 +273,6 @@ async def delete_resource( else: log.debug("Deleting resource") query = _delete(sql_model).where(sql_model.fides_key == fides_key) - print(query) await async_session.execute(query) except SQLAlchemyError: error = errors.QueryError() diff --git a/src/fides/api/ctl/routes/crud.py b/src/fides/api/ctl/routes/crud.py index 4b080ac5c5..f0f6219902 100644 --- a/src/fides/api/ctl/routes/crud.py +++ b/src/fides/api/ctl/routes/crud.py @@ -18,7 +18,7 @@ from fides.api.ctl.database.crud import ( create_resource, delete_resource, - get_resource, + get_resource_with_custom_fields, list_resource, update_resource, upsert_resources, @@ -149,7 +149,7 @@ async def get( ) -> Dict: """Get a resource by its fides_key.""" sql_model = sql_model_map[resource_type] - return await get_resource(sql_model, fides_key, db) + return await get_resource_with_custom_fields(sql_model, fides_key, db) @router.put( "/", diff --git a/src/fides/api/ctl/routes/datamap.py b/src/fides/api/ctl/routes/datamap.py index f6ad898f1a..09598d1013 100644 --- a/src/fides/api/ctl/routes/datamap.py +++ b/src/fides/api/ctl/routes/datamap.py @@ -1,7 +1,7 @@ """ Contains an endpoint for extracting a data map from the server """ -from typing import Dict, List +from typing import Any, Dict, List from fastapi import Depends, Response, Security, status from fideslang.parse import parse_dict @@ -9,7 +9,11 @@ from pandas import DataFrame from sqlalchemy.ext.asyncio import AsyncSession -from fides.api.ctl.database.crud import get_resource, list_resource +from fides.api.ctl.database.crud import ( + get_resource, + get_resource_with_custom_fields, + list_resource, +) from fides.api.ctl.database.session import get_async_db from fides.api.ctl.routes.util import API_PREFIX from fides.api.ctl.sql_models import sql_model_map # type: ignore[attr-defined] @@ -44,7 +48,6 @@ "content": { "application/json": { "example": [ - DATAMAP_COLUMNS_API, { "system.name": "Demo Analytics System", "system.data_responsibility_title": "Controller", @@ -98,7 +101,7 @@ async def export_datamap( organization_fides_key: str, response: Response, db: AsyncSession = Depends(get_async_db), -) -> List[Dict[str, str]]: +) -> List[Dict[str, Any]]: """ An endpoint to return the data map for a given Organization. @@ -133,7 +136,18 @@ async def export_datamap( for resource in server_resources if resource.organization_fides_key == organization_fides_key ] + server_resource_dict[resource_type] = filtered_server_resources + + for k, v in server_resource_dict.items(): + values = [] + for value in v: + with_custom_fields = await get_resource_with_custom_fields( + sql_model_map[k], value.fides_key, db + ) + values.append(with_custom_fields) + server_resource_dict[k] = values + except DatabaseUnavailableError: database_unavailable_error = DatabaseUnavailableError( error_message="Database unavailable" @@ -143,23 +157,29 @@ async def export_datamap( ) raise database_unavailable_error - joined_system_dataset_df = build_joined_dataframe(server_resource_dict) + joined_system_dataset_df, custom_columns = build_joined_dataframe( + server_resource_dict + ) - formatted_datamap = format_datamap_values(joined_system_dataset_df) + formatted_datamap = format_datamap_values(joined_system_dataset_df, custom_columns) # prepend column names formatted_datamap = [DATAMAP_COLUMNS_API] + formatted_datamap return formatted_datamap -def format_datamap_values(joined_system_dataset_df: DataFrame) -> List[Dict[str, str]]: +def format_datamap_values( + joined_system_dataset_df: DataFrame, custom_columns: Dict[str, str] +) -> List[Dict[str, str]]: """ Formats the joined DataFrame to return the data as records. """ + columns = {**DATAMAP_COLUMNS_API, **custom_columns} + limited_columns_df = DataFrame( joined_system_dataset_df, - columns=list(DATAMAP_COLUMNS_API.keys()), + columns=list(columns.keys()), ) return limited_columns_df.to_dict("records") diff --git a/src/fides/api/ctl/sql_models.py b/src/fides/api/ctl/sql_models.py index 5a8f36f887..86a8b45b0c 100644 --- a/src/fides/api/ctl/sql_models.py +++ b/src/fides/api/ctl/sql_models.py @@ -5,9 +5,10 @@ """ from enum import Enum as EnumType -from typing import Dict +from typing import Any, Dict, List, Optional from fideslang.models import Dataset as FideslangDataset +from pydantic import BaseModel from sqlalchemy import ARRAY, BOOLEAN, JSON, Column from sqlalchemy import Enum as EnumColumn from sqlalchemy import ( @@ -286,6 +287,24 @@ class System(Base, FidesBase): ingress = Column(JSON) +class SystemModel(BaseModel): + fides_key: str + registry_id: str + meta: Optional[Dict[str, Any]] + fidesctl_meta: Optional[Dict[str, Any]] + system_type: str + data_responsibility_title: Optional[str] + system_dependencies: Optional[List[str]] + joint_controller: Optional[str] + third_country_transfers: Optional[List[str]] + privacy_declarations: Optional[Dict[str, Any]] + administrating_department: Optional[str] + data_protection_impact_assessment: Optional[Dict[str, Any]] + egress: Optional[Dict[str, Any]] + ingress: Optional[Dict[str, Any]] + value: Optional[List[Any]] + + class SystemScans(Base): """ The SQL model for System Scan instances. diff --git a/src/fides/core/export.py b/src/fides/core/export.py index f3286d5ce7..2b6673d21e 100644 --- a/src/fides/core/export.py +++ b/src/fides/core/export.py @@ -5,7 +5,7 @@ from typing import Dict, List, Tuple import pandas as pd -from fideslang.models import ContactDetails, FidesModel +from fideslang.models import ContactDetails, DataFlow, FidesModel from fides.core.api_helpers import ( get_server_resource, @@ -51,45 +51,48 @@ def generate_dataset_records( # using a set to preserve uniqueness of categories and qualifiers across fields unique_data_categories: set = set() for dataset in server_dataset_list: - dataset_name = dataset.name - dataset_description = dataset.description - dataset_fides_key = dataset.fides_key - dataset_retention = dataset.retention + if not isinstance(dataset, dict): + dataset = dataset.dict() + + dataset_name = dataset["name"] + dataset_description = dataset["description"] + dataset_fides_key = dataset["fides_key"] + dataset_retention = dataset["retention"] dataset_third_country_transfers = ", ".join( - dataset.third_country_transfers or [] + dataset["third_country_transfers"] or [] ) - if dataset.data_categories: + if dataset["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - dataset.data_qualifier, - dataset.data_categories, + dataset["data_qualifier"], + dataset["data_categories"], dataset_retention, dataset_third_country_transfers, dataset_fides_key, ) unique_data_categories = unique_data_categories.union(dataset_rows) - for collection in dataset.collections: - collection_retention = collection.retention or dataset_retention - if collection.data_categories: + for collection in dataset["collections"]: + collection_retention = collection["retention"] or dataset_retention + if collection["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - collection.data_qualifier, - collection.data_categories, + collection["data_qualifier"], + collection["data_categories"], collection_retention, dataset_third_country_transfers, dataset_fides_key, ) unique_data_categories = unique_data_categories.union(dataset_rows) - for field in get_all_level_fields(collection.fields): - if field.data_categories: + for field in get_all_level_fields(collection["fields"]): + if field["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - field.data_qualifier, - field.data_categories, - field.retention or collection_retention, + field["data_qualifier"], + field["data_categories"], + field["retention"] or collection_retention, dataset_third_country_transfers, dataset_fides_key, ) @@ -129,17 +132,22 @@ def export_dataset( print(record) -def generate_system_records( +def generate_system_records( # pylint: disable=too-many-nested-blocks, too-many-branches, too-many-statements server_resources: Dict[str, List], -) -> List[Tuple[str, ...]]: +) -> Tuple[List[Tuple[str, ...]], Dict[str, str]]: """ Takes a dictionary of resources from the server, returning a list of tuples to be used as records to be exported. The headers are defined as the first tuple in the result. """ - formatted_data_uses = format_data_uses(server_resources["data_use"]) - formatted_data_subjects = format_data_subjects(server_resources["data_subject"]) + custom_columns = {} + formatted_data_uses, custom_data_uses = format_data_uses( + server_resources["data_use"] + ) + formatted_data_subjects, custom_data_subjects = format_data_subjects( + server_resources["data_subject"] + ) output_list: List[Tuple[str, ...]] = [ ( "system.fides_key", @@ -170,41 +178,123 @@ def generate_system_records( ) ] + if custom_data_uses: + keys = list(output_list[0]) + for key, value in custom_data_uses.items(): + key_string = f"system.privacy_declaration.data_use.{key}" + custom_columns[key_string] = value + keys.append(key_string) + output_list[0] = tuple(keys) + + if custom_data_subjects: + keys = list(output_list[0]) + for key, value in custom_data_subjects.items(): + key_string = f"system.privacy_declaration.data_subjects.{key}" + custom_columns[key_string] = value + keys.append(key_string) + output_list[0] = tuple(keys) + + system_custom_field_data = {} + known_fields = ( + "fides_key", + "name", + "description", + "data_responsibility_title", + "administrating_department", + "third_country_transfers", + "system_dependencies", + "ingress", + "egress", + "privacy_declarations", + "data_protection_impact_assessment", + "registry_id", + "id", + "updated_at", + "joint_controller", + "created_at", + "organization_fides_key", + "meta", + "tags", + "fidesctl_meta", + "system_type", + ) for system in server_resources["system"]: - third_country_list = ", ".join(system.third_country_transfers or []) - system_dependencies = ", ".join(system.system_dependencies or []) + if not isinstance(system, dict): + system = system.__dict__ + + for key, value in system.items(): + if key not in known_fields: + keys = list(output_list[0]) + key_string = f"system.{key}" + keys.append(key_string) + output_list[0] = tuple(keys) + custom_columns[key_string] = key + if isinstance(value, list): + system_custom_field_data[key_string] = ", ".join(value) + else: + system_custom_field_data[key_string] = value + + third_country_list = ", ".join(system.get("third_country_transfers") or []) + system_dependencies = ", ".join(system.get("system_dependencies") or []) + if system.get("ingress"): + if isinstance(system["ingress"], DataFlow): + system["ingress"] = system["ingress"].dict() + elif isinstance(system["ingress"], list): + reformatted = [] + for ingress in system["ingress"]: + if isinstance(ingress, DataFlow): + reformatted.append(ingress.dict()) + else: + reformatted.append(ingress) + system["ingress"] = reformatted system_ingress = ", ".join( - [ingress.fides_key for ingress in system.ingress or []] + [ingress["fides_key"] for ingress in system.get("ingress") or []] + ) + if system.get("egress"): + if isinstance(system["egress"], DataFlow): + system["egress"] = system["egress"].dict() + elif isinstance(system["egress"], list): + reformatted = [] + for ingress in system["egress"]: + if isinstance(ingress, DataFlow): + reformatted.append(ingress.dict()) + else: + reformatted.append(ingress) + system["egress"] = reformatted + system_egress = ", ".join( + [egress["fides_key"] for egress in system.get("egress") or []] ) - system_egress = ", ".join([egress.fides_key for egress in system.egress or []]) data_protection_impact_assessment = ( get_formatted_data_protection_impact_assessment( - system.data_protection_impact_assessment.dict() + system.get("data_protection_impact_assessment", {}) ) ) - if system.privacy_declarations: - for declaration in system.privacy_declarations: - data_use = formatted_data_uses[declaration.data_use] - data_categories = declaration.data_categories or [] + if system.get("privacy_declarations"): + for declaration in system["privacy_declarations"]: + if not isinstance(declaration, dict): + declaration = declaration.dict() + + data_use = formatted_data_uses[declaration["data_use"]] + data_categories = declaration["data_categories"] or [] data_subjects = [ formatted_data_subjects[data_subject_fides_key] - for data_subject_fides_key in declaration.data_subjects + for data_subject_fides_key in declaration["data_subjects"] ] - dataset_references = declaration.dataset_references or [ + dataset_references = declaration["dataset_references"] or [ EMPTY_COLUMN_PLACEHOLDER ] - cartesian_product_of_declaration = [ - ( - system.fides_key, - system.name, - system.description, - system.data_responsibility_title, - system.administrating_department, + cartesian_product_of_declaration_builder = [ + [ + system["fides_key"], + system["name"], + system["description"], + system["data_responsibility_title"], + system["administrating_department"], third_country_list, system_dependencies, system_ingress, system_egress, - declaration.name, + declaration["name"], category, data_use["name"], data_use["legal_basis"], @@ -215,34 +305,51 @@ def generate_system_records( subject["name"], subject["rights_available"], subject["automated_decisions_or_profiling"], - declaration.data_qualifier, + declaration["data_qualifier"], data_protection_impact_assessment["is_required"], data_protection_impact_assessment["progress"], data_protection_impact_assessment["link"], dataset_reference, - ) + ] for category in data_categories for subject in data_subjects for dataset_reference in dataset_references ] + cartesian_product_of_declaration = [] + if system_custom_field_data: + for _, v in system_custom_field_data.items(): + for product in cartesian_product_of_declaration_builder: + product.append(v) + cartesian_product_of_declaration.append(tuple(product)) + else: + cartesian_product_of_declaration = [ + tuple(x) for x in cartesian_product_of_declaration_builder + ] + output_list += cartesian_product_of_declaration else: system_row = [ - system.fides_key, - system.name, - system.description, - system.data_responsibility_title, - system.administrating_department, + system["fides_key"], + system["name"], + system["description"], + system["data_responsibility_title"], + system["administrating_department"], third_country_list, system_dependencies, system_ingress, system_egress, - data_protection_impact_assessment["is_required"], - data_protection_impact_assessment["progress"], - data_protection_impact_assessment["link"], ] - num_privacy_declaration_fields = 12 - privacy_declaration_start_index = 9 + if system_custom_field_data: + for _, v in system_custom_field_data.items(): + system_row.append(v) + len_no_privacy = len(system_row) + system_row.append(data_protection_impact_assessment["is_required"]) + system_row.append(data_protection_impact_assessment["progress"]) + system_row.append(data_protection_impact_assessment["link"]) + num_privacy_declaration_fields = 3 + privacy_declaration_start_index = ( + len_no_privacy - num_privacy_declaration_fields + ) for i in range(num_privacy_declaration_fields): system_row.insert( i + privacy_declaration_start_index, EMPTY_COLUMN_PLACEHOLDER @@ -252,7 +359,7 @@ def generate_system_records( # also add n/a for the dataset reference output_list += [tuple(system_row)] - return output_list + return output_list, custom_columns def export_system( @@ -294,7 +401,7 @@ def export_system( url, "data_subject", list(set(data_subject_fides_keys)), headers ) - output_list = generate_system_records(server_resources) + output_list, _ = generate_system_records(server_resources) if not dry: exported_filename = export_to_csv(output_list, resource_type, manifests_dir) @@ -327,16 +434,19 @@ def generate_contact_records( # currently the output file will only truly support a single organization # need to better understand the use case for multiple and how to handle for organization in server_organization_list: + if not isinstance(organization, dict): + organization = organization.dict() + fields = tuple(ContactDetails().dict().keys()) get_values = ( - lambda x: tuple(x.dict().values()) + lambda x: tuple(x.values()) if x else tuple(ContactDetails().dict().values()) ) - controller = get_values(organization.controller) - data_protection_officer = get_values(organization.data_protection_officer) - representative = get_values(organization.representative) + controller = get_values(organization["controller"]) + data_protection_officer = get_values(organization["data_protection_officer"]) + representative = get_values(organization["representative"]) contact_details = list( zip( @@ -389,7 +499,7 @@ def export_organization( def build_joined_dataframe( server_resource_dict: Dict[str, List], -) -> pd.DataFrame: +) -> Tuple[pd.DataFrame, Dict[str, str]]: """ Return joined dataframes for datamap export @@ -400,7 +510,7 @@ def build_joined_dataframe( # systems - system_output_list = generate_system_records(server_resource_dict) + system_output_list, custom_columns = generate_system_records(server_resource_dict) systems_df = pd.DataFrame.from_records(system_output_list) systems_df.columns = systems_df.iloc[0] systems_df = systems_df[1:] @@ -459,12 +569,17 @@ def build_joined_dataframe( joined_df["system.third_country_safeguards"] = "" joined_df["system.link_to_processor_contract"] = "" + if not isinstance(server_resource_dict["organization"][0], dict): + server_resource_dict["organization"][0] = server_resource_dict["organization"][ + 0 + ].dict() + joined_df["organization.link_to_security_policy"] = ( - server_resource_dict["organization"][0].security_policy or "" + server_resource_dict["organization"][0]["security_policy"] or "" ) joined_df["dataset.source_name"] = joined_df["dataset.name"] - return joined_df + return joined_df, custom_columns def export_datamap( @@ -514,7 +629,7 @@ def export_datamap( server_resource_dict[resource_type] = filtered_server_resources # transform the resources to join a system and referenced datasets - joined_system_dataset_df = build_joined_dataframe(server_resource_dict) + joined_system_dataset_df, _ = build_joined_dataframe(server_resource_dict) if not dry and not to_csv: # build an organization dataframe if exporting to excel diff --git a/src/fides/core/export_helpers.py b/src/fides/core/export_helpers.py index d9796baa77..b1f84d4184 100644 --- a/src/fides/core/export_helpers.py +++ b/src/fides/core/export_helpers.py @@ -138,7 +138,9 @@ def export_datamap_to_excel( return filename -def format_data_uses(data_uses: List[DataUse]) -> Dict[FidesKey, Dict[str, str]]: +def format_data_uses( + data_uses: List[DataUse], +) -> Tuple[Dict[FidesKey, Dict[str, str]], Dict[str, str]]: """ This function formats data uses for use when exporting, returning the necessary values as a dict. Formatting @@ -146,38 +148,65 @@ def format_data_uses(data_uses: List[DataUse]) -> Dict[FidesKey, Dict[str, str]] """ formatted_data_uses = {} + custom_columns = {} + known_attributes = ( + "legal_basis", + "special_category", + "recipients", + "legitimate_interest_impact_assessment", + "legitimate_interest", + ) + excluded_attributes = ( + "id", + "tags", + "description", + "updated_at", + "is_default", + "organization_fides_key", + "parent", + "created_at", + "name", + "parent_key", + "fides_key", + ) for data_use in data_uses: + if not isinstance(data_use, dict): + data_use = data_use.dict() + formatted_data_use = { - "name": data_use.name, + "name": data_use["name"], } - for attribute in [ - "legal_basis", - "special_category", - "recipients", - "legitimate_interest_impact_assessment", - "legitimate_interest", - ]: - attribute_value = getattr(data_use, attribute) + for attribute in known_attributes: + attribute_value = data_use.get(attribute) if attribute_value is None: attribute_value = "N/A" elif isinstance(attribute_value, list): attribute_value = ", ".join(attribute_value) elif attribute == "legitimate_interest": if attribute_value is True: - attribute_value = getattr(data_use, "name") + attribute_value = data_use.get("name") else: attribute_value = "N/A" formatted_data_use[attribute] = attribute_value - formatted_data_uses[data_use.fides_key] = formatted_data_use - return formatted_data_uses + for k, v in data_use.items(): + if k not in known_attributes and k not in excluded_attributes: + custom_columns[k] = k + if isinstance(v, list): + formatted_data_use[k] = ", ".join(v) + else: + formatted_data_use[k] = v + + formatted_data_uses[data_use["fides_key"]] = formatted_data_use + + return formatted_data_uses, custom_columns def format_data_subjects( data_subjects: List[DataSubject], -) -> Dict[FidesKey, Dict[str, str]]: +) -> Tuple[Dict[FidesKey, Dict[str, str]], Dict[str, str]]: """ This function formats data subjects from the server, returning the necessary values as a list of dicts. @@ -194,9 +223,26 @@ def format_data_subjects( ] formatted_data_subjects: Dict[FidesKey, Dict[str, str]] = {} + excluded_attributes = ( + "tags", + "id", + "description", + "updated_at", + "automated_decisions_or_profiling", + "fides_key", + "organization_fides_key", + "name", + "created_at", + "rights", + "is_default", + "rights_available", + ) + custom_columns = {} for data_subject in data_subjects: - data_subject_dict = data_subject.dict() + if not isinstance(data_subject, dict): + data_subject = data_subject.dict() + formatted_data_subject = dict( zip( formatted_data_subject_attributes_list, @@ -208,21 +254,29 @@ def format_data_subjects( ) # calculate and format data subject rights as applicable - if data_subject_dict["rights"]: - data_subject_dict["rights_available"] = calculate_data_subject_rights( - data_subject_dict["rights"] + if data_subject.get("rights"): + data_subject["rights_available"] = calculate_data_subject_rights( + data_subject["rights"] ) else: - data_subject_dict["rights_available"] = "No data subject rights listed" + data_subject["rights_available"] = "No data subject rights listed" formatted_data_subject = { - attribute: data_subject_dict.get(attribute) or "N/A" + attribute: data_subject.get(attribute) or "N/A" for attribute in formatted_data_subject_attributes_list } - formatted_data_subjects[data_subject.fides_key] = formatted_data_subject + for k, v in data_subject.items(): + if k not in excluded_attributes: + custom_columns[k] = k + if isinstance(v, list): + formatted_data_subject[k] = ", ".join(v) + else: + formatted_data_subject[k] = v + + formatted_data_subjects[data_subject["fides_key"]] = formatted_data_subject - return formatted_data_subjects + return formatted_data_subjects, custom_columns def convert_tuple_to_string(values: Tuple[str, ...]) -> str: @@ -354,6 +408,8 @@ def get_formatted_data_protection_impact_assessment( data_protection_impact_assessment: dict, ) -> dict: "Replace None with N/A for consistent formatting of the data map" + if not isinstance(data_protection_impact_assessment, dict): + data_protection_impact_assessment = data_protection_impact_assessment.dict() return { key: ("N/A" if value is None else value) for key, value in data_protection_impact_assessment.items() diff --git a/src/fides/core/utils.py b/src/fides/core/utils.py index 8a0db447da..20cbb6ee44 100644 --- a/src/fides/core/utils.py +++ b/src/fides/core/utils.py @@ -102,9 +102,14 @@ def get_all_level_fields(fields: list) -> Iterator[DatasetField]: """ for field in fields: yield field - if field.fields: - for nested_field in get_all_level_fields(field.fields): - yield nested_field + if isinstance(field, dict): + if field["fields"]: + for nested_field in get_all_level_fields(field["fields"]): + yield nested_field + else: + if field.fields: + for nested_field in get_all_level_fields(field.fields): + yield nested_field def get_manifest_list(manifests_dir: str) -> List[str]: diff --git a/tests/ctl/api/test_datamap.py b/tests/ctl/api/test_datamap.py index f4f1b4726f..671316e8d0 100644 --- a/tests/ctl/api/test_datamap.py +++ b/tests/ctl/api/test_datamap.py @@ -3,9 +3,39 @@ from starlette.testclient import TestClient from fides.api.ctl.routes.util import API_PREFIX +from fides.api.ctl.sql_models import ( # type: ignore[attr-defined] + CustomField, + CustomFieldDefinition, +) from fides.core.config import FidesConfig +@pytest.fixture +def custom_fields(db, resources_dict): + definition = CustomFieldDefinition.create_or_update( + db=db, + data={ + "name": "my_custom_field_definition", + "description": "test", + "field_type": "string", + "resource_type": "system", + "field_definition": "string", + }, + ) + field = CustomField.create( + db=db, + data={ + "resource_type": definition.resource_type, + "resource_id": resources_dict["system"].fides_key, + "custom_field_definition_id": definition.id, + "value": "Test value 1", + }, + ) + yield + field.delete(db) + definition.delete(db) + + @pytest.mark.integration @pytest.mark.parametrize( "organization_fides_key, expected_status_code", @@ -20,7 +50,28 @@ def test_datamap( expected_status_code: int, test_client: TestClient, ) -> None: - print(test_config) + response = test_client.get( + test_config.cli.server_url + API_PREFIX + "/datamap/" + organization_fides_key, + headers=test_config.user.auth_header, + ) + assert response.status_code == expected_status_code + + +@pytest.mark.integration +@pytest.mark.parametrize( + "organization_fides_key, expected_status_code", + [ + ("fake_organization", 404), + ("default_organization", 200), + ], +) +@pytest.mark.usefixtures("custom_fields") +def test_datamap_with_custom_fields( + test_config, + organization_fides_key, + expected_status_code, + test_client, +): response = test_client.get( test_config.cli.server_url + API_PREFIX + "/datamap/" + organization_fides_key, headers=test_config.user.auth_header, diff --git a/tests/ctl/core/test_export.py b/tests/ctl/core/test_export.py index 77fbf7c132..65e05a4616 100644 --- a/tests/ctl/core/test_export.py +++ b/tests/ctl/core/test_export.py @@ -54,6 +54,51 @@ def test_sample_system_taxonomy() -> Generator[Dict, None, None]: } +@pytest.fixture() +def test_sample_system_taxonomy_with_custom_fields(): + data = { + "system": [ + System( + fides_key="test_system", + system_type="test", + system_name="test system", + system_description="system used for testing exports", + privacy_declarations=[ + PrivacyDeclaration( + name="privacy_declaration_1", + data_categories=[ + "user.contact.email", + "user.contact.name", + ], + data_use="provide.service", + data_qualifier="aggregated.anonymized", + data_subjects=["customer"], + dataset_references=["test_dataset"], + ) + ], + ).dict(), + ], + "data_subject": [DataSubject(fides_key="customer", name="customer").dict()], + "data_use": [ + DataUse( + fides_key="provide.service", name="System", parent_key="provide" + ).dict() + ], + "organization": [ + Organization( + fides_key="default_organization", + security_policy="https://www.google.com/", + ).dict() + ], + } + + data["system"][0]["custom_system_field"] = "custom system field" + data["data_subject"][0]["custom_data_subject"] = "custom data subject" + data["data_use"][0]["custom_data_use"] = "custom data use" + + yield data + + @pytest.fixture() def test_sample_dataset_taxonomy() -> Generator: yield [ @@ -104,8 +149,18 @@ def test_system_records_to_export( the header row) """ - output_list = export.generate_system_records(test_sample_system_taxonomy) - print(output_list) + output_list, _ = export.generate_system_records(test_sample_system_taxonomy) + assert len(output_list) == 3 + + +@pytest.mark.usefixtures("test_config") +def test_system_records_to_export_with_custom_fields( + test_sample_system_taxonomy_with_custom_fields, +): + + output_list, _ = export.generate_system_records( + test_sample_system_taxonomy_with_custom_fields + ) assert len(output_list) == 3 @@ -118,7 +173,7 @@ def test_system_records_to_export_sans_privacy_declaration( are returned properly (including the header row) """ test_sample_system_taxonomy["system"][0].privacy_declarations = [] - output_list = export.generate_system_records(test_sample_system_taxonomy) + output_list, _ = export.generate_system_records(test_sample_system_taxonomy) print(output_list) assert len(output_list) == 2 @@ -172,7 +227,7 @@ def test_joined_datamap_export_system_dataset_overlap( """ sample_taxonomy: Dict = test_sample_system_taxonomy sample_taxonomy["dataset"] = test_sample_dataset_taxonomy - output_list = export.build_joined_dataframe(sample_taxonomy) + output_list, _ = export.build_joined_dataframe(sample_taxonomy) assert len(output_list) == 5 @@ -239,5 +294,5 @@ def test_joined_datamap_export_system_multiple_declarations_overlap( sample_taxonomy["data_subject"].append(new_data_subject) sample_taxonomy["system"][0].privacy_declarations.append(new_declaration) sample_taxonomy["dataset"] = [] - output_list = export.build_joined_dataframe(sample_taxonomy) + output_list, _ = export.build_joined_dataframe(sample_taxonomy) assert len(output_list) == 4 diff --git a/tests/ctl/core/test_utils.py b/tests/ctl/core/test_utils.py index 09b5637aaf..27f2f7359f 100644 --- a/tests/ctl/core/test_utils.py +++ b/tests/ctl/core/test_utils.py @@ -56,8 +56,8 @@ def test_nested_fields_unpacked( """ collection = test_nested_collection_fields collected_field_names = [] - for field in utils.get_all_level_fields(collection.fields): - collected_field_names.append(field.name) + for field in utils.get_all_level_fields(collection.dict()["fields"]): + collected_field_names.append(field["name"]) assert len(collected_field_names) == 5 diff --git a/tests/ctl/database/test_crud.py b/tests/ctl/database/test_crud.py index f92a4fc5ec..5d4c083811 100644 --- a/tests/ctl/database/test_crud.py +++ b/tests/ctl/database/test_crud.py @@ -1,11 +1,19 @@ from json import dumps from typing import Generator, List +from uuid import uuid4 import pytest +from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession from fides.api.ctl import sql_models -from fides.api.ctl.database.crud import delete_resource, list_resource +from fides.api.ctl.database.crud import ( + create_resource, + delete_resource, + get_resource_with_custom_fields, + list_resource, +) +from fides.api.ctl.utils.errors import QueryError from fides.core import api as _api from fides.core.config import FidesConfig from tests.ctl.types import FixtureRequest @@ -62,3 +70,81 @@ async def test_cascade_delete_taxonomy_children( resources = await list_resource(sql_model, async_session) remaining_keys = {resource.fides_key for resource in resources} assert len(set(keys).intersection(remaining_keys)) == 0 + + +async def test_get_resource_with_custom_field(db, async_session): + system_data = { + "name": "my system", + "registry_id": "1", + "system_type": "test", + "fides_key": str(uuid4()), + } + + system = await create_resource(sql_models.System, system_data, async_session) + + custom_definition_data = { + "name": "test1", + "description": "test", + "field_type": "string", + "resource_type": "system", + "field_definition": "string", + } + + custom_field_definition = sql_models.CustomFieldDefinition.create( + db=db, data=custom_definition_data + ) + + sql_models.CustomField.create( + db=db, + data={ + "resource_type": custom_field_definition.resource_type, + "resource_id": system.fides_key, + "custom_field_definition_id": custom_field_definition.id, + "value": ["Test value 1"], + }, + ) + + sql_models.CustomField.create( + db=db, + data={ + "resource_type": custom_field_definition.resource_type, + "resource_id": system.fides_key, + "custom_field_definition_id": custom_field_definition.id, + "value": ["Test value 2"], + }, + ) + + result = await get_resource_with_custom_fields( + sql_models.System, system.fides_key, async_session + ) + + assert result["name"] == system.name + assert custom_field_definition.name in result + assert result[custom_field_definition.name] == "Test value 1, Test value 2" + + +async def test_get_resource_with_custom_field_no_custom_field(async_session): + system_data = { + "name": "my system", + "registry_id": "1", + "system_type": "test", + "fides_key": str(uuid4()), + } + + system = await create_resource(sql_models.System, system_data, async_session) + result = await get_resource_with_custom_fields( + sql_models.System, system.fides_key, async_session + ) + + assert result["name"] == system.name + + +async def test_get_resource_with_custom_field_error(async_session, monkeypatch): + async def mock_execute(*args, **kwargs): + raise SQLAlchemyError + + monkeypatch.setattr( + "fides.api.ctl.database.crud.AsyncSession.execute", mock_execute + ) + with pytest.raises(QueryError): + await get_resource_with_custom_fields(sql_models.System, "ABC", async_session) diff --git a/tests/ops/api/v1/test_main.py b/tests/ops/api/v1/test_main.py index b9e30112db..4a202589d4 100644 --- a/tests/ops/api/v1/test_main.py +++ b/tests/ops/api/v1/test_main.py @@ -13,7 +13,7 @@ def test_read_autogenerated_docs(api_client: TestClient): """Test to ensure automatically generated docs build properly""" - response = api_client.get(f"/openapi.json") + response = api_client.get("/openapi.json") assert response.status_code == 200 From 6ff436e63627cc0b347a10a8255df6463f62d90f Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 6 Mar 2023 10:40:02 -0500 Subject: [PATCH 19/20] asserts test_mode is true in task config (#2018) --- tests/ops/tasks/test_celery.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/ops/tasks/test_celery.py b/tests/ops/tasks/test_celery.py index d672f6a12f..54feeee1e0 100644 --- a/tests/ops/tasks/test_celery.py +++ b/tests/ops/tasks/test_celery.py @@ -31,6 +31,21 @@ def multiply(x, y): assert multiply.delay(4, 4).get(timeout=10) == 16 +def test_task_config_is_test_mode(celery_session_app, celery_session_worker): + @celery_session_app.task + def get_virtualised_worker_config(): + return get_config().test_mode + + # Force `celery_app` to register our new task + # See: https://github.com/celery/celery/issues/3642#issuecomment-369057682 + celery_session_worker.reload() + sync_is_test_mode = get_virtualised_worker_config.run() + async_is_test_mode = get_virtualised_worker_config.delay().get(timeout=10) + + assert sync_is_test_mode + assert async_is_test_mode + + def test_celery_default_config() -> None: config = get_config() celery_app = _create_celery(config) From e5edd4f67c6f6b35f12e8c4df2e0407862a52e8e Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Mon, 6 Mar 2023 12:08:51 -0500 Subject: [PATCH 20/20] fix messaging 'test connection' workflow on initial save (#2751) --- .../configuration/MailgunEmailConfiguration.tsx | 4 +++- .../configuration/TwilioEmailConfiguration.tsx | 6 +++++- .../features/privacy-requests/configuration/TwilioSMS.tsx | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx index c42c97e018..b69dd66094 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/MailgunEmailConfiguration.tsx @@ -166,7 +166,9 @@ const MailgunEmailConfiguration = () => { ) : null} {configurationStep === "testConnection" ? ( ) : null} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx index a62f007096..4707bebcaf 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx @@ -163,7 +163,11 @@ const TwilioEmailConfiguration = () => { ) : null} {configurationStep === "testConnection" ? ( ) : null} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx index 82fe82f096..57efafc663 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx @@ -117,7 +117,9 @@ const TwilioSMSConfiguration = () => { {configurationStep === "testConnection" ? ( ) : null}