From 73aab64b84ff9523588122b543caf0ae263ff32e Mon Sep 17 00:00:00 2001 From: Roger Plotz <115798183+Roger-Ethyca@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:16:56 -0400 Subject: [PATCH 1/8] empty commit to start release 2.45.0 From 6b01c1a5a2e57b105b2b31c189f561e868a54873 Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Wed, 11 Sep 2024 23:32:02 -0400 Subject: [PATCH 2/8] PROD-2722, PROD-2761 (#5278) --- .../connection_secrets_base_aws.py | 1 - .../connection_secrets_dynamodb.py | 4 +++- src/fides/api/util/aws_util.py | 10 ++++---- src/fides/api/util/connection_type.py | 5 ++++ tests/ops/util/test_connection_type.py | 23 ++++++++++++++++++- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/fides/api/schemas/connection_configuration/connection_secrets_base_aws.py b/src/fides/api/schemas/connection_configuration/connection_secrets_base_aws.py index 9b23fb51bff..d0578e4d7d3 100644 --- a/src/fides/api/schemas/connection_configuration/connection_secrets_base_aws.py +++ b/src/fides/api/schemas/connection_configuration/connection_secrets_base_aws.py @@ -14,7 +14,6 @@ class BaseAWSSchema(ConnectionConfigSecretsSchema): auth_method: AWSAuthMethod = Field( title="Authentication Method", description="Determines which type of authentication method to use for connecting to Amazon Web Services. Currently accepted values are: `secret_keys` or `automatic`.", - default=AWSAuthMethod.SECRET_KEYS, ) aws_access_key_id: Optional[str] = Field( diff --git a/src/fides/api/schemas/connection_configuration/connection_secrets_dynamodb.py b/src/fides/api/schemas/connection_configuration/connection_secrets_dynamodb.py index 0be6cb6daa9..5f1ed9cbd60 100644 --- a/src/fides/api/schemas/connection_configuration/connection_secrets_dynamodb.py +++ b/src/fides/api/schemas/connection_configuration/connection_secrets_dynamodb.py @@ -16,7 +16,9 @@ class DynamoDBSchema(BaseAWSSchema): description="The AWS region where your DynamoDB table is located (ex. us-west-2).", ) - _required_components: ClassVar[List[str]] = ["region_name"] + _required_components: ClassVar[List[str]] = BaseAWSSchema._required_components + [ + "region_name" + ] class DynamoDBDocsSchema(DynamoDBSchema, NoValidationSchema): diff --git a/src/fides/api/util/aws_util.py b/src/fides/api/util/aws_util.py index 22013145a76..385b1cef563 100644 --- a/src/fides/api/util/aws_util.py +++ b/src/fides/api/util/aws_util.py @@ -19,7 +19,6 @@ def get_aws_session( If an `assume_role_arn` is provided, the secrets will be used to assume that role and return a Session instantiated with that role. """ - sts_client = None if auth_method == AWSAuthMethod.SECRET_KEYS.value: if storage_secrets is None: err_msg = "Storage secrets not found for S3 storage." @@ -34,11 +33,13 @@ def get_aws_session( region_name=storage_secrets.get("region_name"), # type: ignore ) elif auth_method == AWSAuthMethod.AUTOMATIC.value: - session = Session() + session = Session( + region_name=storage_secrets.get("region_name"), # type: ignore + ) logger.info("Successfully created automatic session") else: - logger.error("Auth method not supported for S3: {}", auth_method) - raise ValueError(f"Auth method not supported for S3: {auth_method}") + logger.error("AWS auth method not supported: {}", auth_method) + raise ValueError(f"AWS auth method not supported: {auth_method}") # Check that credentials are valid sts_client = session.client("sts") @@ -57,6 +58,7 @@ def get_aws_session( aws_access_key_id=temp_credentials["AccessKeyId"], aws_secret_access_key=temp_credentials["SecretAccessKey"], aws_session_token=temp_credentials["SessionToken"], + region_name=storage_secrets.get("region_name"), # type: ignore ) except ClientError as error: logger.exception( diff --git a/src/fides/api/util/connection_type.py b/src/fides/api/util/connection_type.py index 7937ae96108..9564a3bd3f8 100644 --- a/src/fides/api/util/connection_type.py +++ b/src/fides/api/util/connection_type.py @@ -181,6 +181,11 @@ def get_connection_type_secret_schema(*, connection_type: str) -> dict[str, Any] } if schema.get("additionalProperties"): schema.pop("additionalProperties") + + # set an empty array for the 'required' key, if there are no required fields on the schema + # this helps the FE, which is expecting _some_ value here, even if it's an empty list + if "required" not in schema: + schema["required"] = [] return schema diff --git a/tests/ops/util/test_connection_type.py b/tests/ops/util/test_connection_type.py index da3a6595601..7b5fe8fdf1c 100644 --- a/tests/ops/util/test_connection_type.py +++ b/tests/ops/util/test_connection_type.py @@ -3,10 +3,14 @@ from fides.api.models.connectionconfig import ConnectionType from fides.api.models.policy import ActionType from fides.api.schemas.connection_configuration.enums.system_type import SystemType +from fides.api.schemas.storage.storage import AWSAuthMethod from fides.api.service.connectors.saas.connector_registry_service import ( ConnectorRegistry, ) -from fides.api.util.connection_type import get_connection_types +from fides.api.util.connection_type import ( + get_connection_type_secret_schema, + get_connection_types, +) def test_get_connection_types(): @@ -300,3 +304,20 @@ def test_get_connection_types_action_type_filter( for connection_type in assert_not_in_data: obj = connection_type_objects[connection_type] assert obj not in data + + +def test_get_connection_type_secret_schemas_aws(): + """ + AWS secret schemas have inheritance from a base class, and have provided some issues in the past. + + This test covers their JSON schema serialization behavior to ensure there aren't regressions. + """ + + dynamo_db_schema = get_connection_type_secret_schema(connection_type="dynamodb") + dynamodb_required = dynamo_db_schema["required"] + assert "region_name" in dynamodb_required + assert "auth_method" in dynamodb_required + + s3_secret_schema = get_connection_type_secret_schema(connection_type="s3") + s3_required = s3_secret_schema["required"] + assert "auth_method" in s3_required From 5384a1d32bf2f6ae392983df666fdf8b079cd3ee Mon Sep 17 00:00:00 2001 From: Neville Samuell Date: Thu, 12 Sep 2024 14:52:29 -0400 Subject: [PATCH 3/8] Fix Fides.shouldShouldShowExperience() to return false for modal-only experiences (#5281) --- CHANGELOG.md | 1 + clients/fides-js/src/lib/consent-utils.ts | 6 +- clients/fides-js/src/lib/cookie.ts | 5 + .../cypress/e2e/should-show-experience.cy.ts | 150 ++++++++++++++++++ .../cypress/e2e/show-modal.cy.ts | 6 - .../fixtures/consent/experience_modal.json | 121 ++++++++++++++ .../fixtures/consent/experience_none.json | 6 + 7 files changed, 288 insertions(+), 7 deletions(-) create mode 100644 clients/privacy-center/cypress/e2e/should-show-experience.cy.ts create mode 100644 clients/privacy-center/cypress/fixtures/consent/experience_modal.json create mode 100644 clients/privacy-center/cypress/fixtures/consent/experience_none.json diff --git a/CHANGELOG.md b/CHANGELOG.md index a38351e9ade..2e12d9427bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ The types of changes are: ### Fixed - Fix wording in tooltip for Yotpo Reviews [#5274](https://github.com/ethyca/fides/pull/5274) +- Fix Fides.shouldShouldShowExperience() to return false for modal-only experiences [#5281](https://github.com/ethyca/fides/pull/5281) ## [2.44.0](https://github.com/ethyca/fides/compare/2.43.1...2.44.0) diff --git a/clients/fides-js/src/lib/consent-utils.ts b/clients/fides-js/src/lib/consent-utils.ts index 6d8a11311d9..c2d07780fe7 100644 --- a/clients/fides-js/src/lib/consent-utils.ts +++ b/clients/fides-js/src/lib/consent-utils.ts @@ -238,13 +238,17 @@ export const shouldResurfaceConsent = ( cookie: FidesCookie, savedConsent: NoticeConsent, ): boolean => { + // Always resurface consent for TCF unless the saved version_hash matches if (experience.experience_config?.component === ComponentType.TCF_OVERLAY) { if (experience.meta?.version_hash) { return experience.meta.version_hash !== cookie.tcf_version_hash; } - // Ensure we always resurface consent for TCF if for some reason experience does not have version_hash return true; } + // Never surface consent for modal-only experiences + if (experience.experience_config?.component === ComponentType.MODAL) { + return false; + } // Do not surface consent for null or empty notices if (!(experience as PrivacyExperience)?.privacy_notices?.length) { return false; diff --git a/clients/fides-js/src/lib/cookie.ts b/clients/fides-js/src/lib/cookie.ts index 46dc0cc80f2..240f15fb62a 100644 --- a/clients/fides-js/src/lib/cookie.ts +++ b/clients/fides-js/src/lib/cookie.ts @@ -260,6 +260,11 @@ export const updateExperienceFromCookieConsentNotices = ({ cookie: FidesCookie; debug?: boolean; }): PrivacyExperience => { + // If the given experience has no notices, return immediately and do not mutate + // the experience object in any way + if (!experience.privacy_notices) { + return experience; + } // DEFER (PROD-1568) - instead of updating experience here, push this logic into UI const noticesWithConsent: PrivacyNoticeWithPreference[] | undefined = experience.privacy_notices?.map((notice) => { diff --git a/clients/privacy-center/cypress/e2e/should-show-experience.cy.ts b/clients/privacy-center/cypress/e2e/should-show-experience.cy.ts new file mode 100644 index 00000000000..60a76f4f022 --- /dev/null +++ b/clients/privacy-center/cypress/e2e/should-show-experience.cy.ts @@ -0,0 +1,150 @@ +import { + CONSENT_COOKIE_NAME, + NoticeConsent, + PrivacyExperience, +} from "fides-js"; + +import { mockCookie } from "../support/mocks"; +import { OVERRIDE, stubConfig, stubTCFExperience } from "../support/stubs"; + +describe("Fides.shouldShowExperience()", () => { + interface TestCaseOptions { + fixture: + | "experience_modal.json" + | "experience_banner_modal.json" + | "experience_tcf_minimal.json" + | "experience_none.json"; + savedConsent: boolean; + expiredTcfVersionHash?: boolean; + shouldShowExperience: boolean; + } + + const testCases: TestCaseOptions[] = [ + { + fixture: "experience_modal.json", + savedConsent: false, + shouldShowExperience: false, + }, + { + fixture: "experience_modal.json", + savedConsent: true, + shouldShowExperience: false, + }, + { + fixture: "experience_banner_modal.json", + savedConsent: false, + shouldShowExperience: true, + }, + { + fixture: "experience_banner_modal.json", + savedConsent: true, + shouldShowExperience: false, + }, + { + fixture: "experience_tcf_minimal.json", + savedConsent: false, + shouldShowExperience: true, + }, + { + fixture: "experience_tcf_minimal.json", + savedConsent: true, + shouldShowExperience: false, + }, + { + fixture: "experience_tcf_minimal.json", + savedConsent: true, + expiredTcfVersionHash: true, + shouldShowExperience: true, + }, + { + fixture: "experience_none.json", + savedConsent: false, + shouldShowExperience: false, + }, + { + fixture: "experience_none.json", + savedConsent: true, + shouldShowExperience: false, + }, + ]; + + testCases.forEach( + ({ + fixture, + savedConsent, + expiredTcfVersionHash, + shouldShowExperience, + }) => { + describe(`when rendering ${fixture} and saved consent ${savedConsent ? "exists" : "does not exist"} ${expiredTcfVersionHash ? "(with expired tcf_version_hash)" : ""}`, () => { + it(`Fides.shouldShowExperience() returns ${shouldShowExperience}`, () => { + cy.fixture(`consent/${fixture}`).then((data) => { + let experience: PrivacyExperience = data.items[0] || OVERRIDE.EMPTY; + const tcfEnabled = /tcf/.test(fixture); + + // If the test requires it, generate and save a prior consent cookie + if (savedConsent) { + // Mock an opt-out consent for each notice, plus an "example" other notice + const notices = [ + ...(experience.privacy_notices || []), + ...[{ notice_key: "example" }], + ]; + const consent: NoticeConsent = Object.fromEntries( + notices.map((e) => [e.notice_key, false]), + ); + + // Mock a saved fides_string if tcf is enabled + const fides_string = tcfEnabled + ? "CPzevcAPzevcAGXABBENATEIAAIAAAAAAAAAAAAAAAAA" + : undefined; + + // Save a tcf_version_hash that matches the experience + let tcf_version_hash = experience.meta?.version_hash; + + // Mock an "expired" hash, if the test demands it + if (expiredTcfVersionHash) { + tcf_version_hash = "1a2a3a"; + } + + // Save the mock cookie + const cookie = mockCookie({ + consent, + fides_string, + tcf_version_hash, + }); + cy.setCookie(CONSENT_COOKIE_NAME, JSON.stringify(cookie)); + } + + // Load the demo page with our stubbed config + cy.log("using experience = ", experience); + if (!tcfEnabled) { + stubConfig({ experience, options: { isOverlayEnabled: true } }); + } else { + stubTCFExperience({}); + } + }); + cy.waitUntilFidesInitialized(); + + // Check that our test saved the consent cookie correctly + if (savedConsent) { + cy.getCookie(CONSENT_COOKIE_NAME).should("exist"); + } else { + cy.getCookie(CONSENT_COOKIE_NAME).should("not.exist"); + } + + // Check if shouldShowExperience() returns the expected value + cy.window() + .its("Fides") + .invoke("shouldShowExperience") + .should("eql", shouldShowExperience); + + // If shouldShowExperience() is true, the banner should show as well + if (shouldShowExperience) { + cy.get("div#fides-banner").should("be.visible"); + } else { + cy.get("div#fides-banner").should("not.exist"); + } + }); + }); + }, + ); +}); diff --git a/clients/privacy-center/cypress/e2e/show-modal.cy.ts b/clients/privacy-center/cypress/e2e/show-modal.cy.ts index ae71ab0ad35..7619d9c0461 100644 --- a/clients/privacy-center/cypress/e2e/show-modal.cy.ts +++ b/clients/privacy-center/cypress/e2e/show-modal.cy.ts @@ -10,9 +10,6 @@ describe("Fides.showModal", () => { options: { isOverlayEnabled: true, }, - experience: { - show_banner: false, - }, }); }); @@ -53,9 +50,6 @@ describe("Fides.showModal", () => { options: { isOverlayEnabled: false, }, - experience: { - show_banner: false, - }, }); }); diff --git a/clients/privacy-center/cypress/fixtures/consent/experience_modal.json b/clients/privacy-center/cypress/fixtures/consent/experience_modal.json new file mode 100644 index 00000000000..3598c0e9325 --- /dev/null +++ b/clients/privacy-center/cypress/fixtures/consent/experience_modal.json @@ -0,0 +1,121 @@ +{ + "items": [ + { + "region": "us_ca", + "available_locales": ["en", "es", "fr-CA"], + "experience_config": { + "translations": [ + { + "language": "en", + "accept_button_label": "Opt in to all", + "acknowledge_button_label": "OK", + "banner_description": "[banner] We use cookies and similar methods to recognize visitors and remember their preferences. We may also use them to measure ad campaign effectiveness, target ads, and analyze site traffic. Depending on your location, you may opt-in or opt out of the use of these technologies.", + "banner_title": "[banner] Manage your consent preferences", + "description": "We use cookies and similar methods to recognize visitors and remember their preferences. We may also use them to measure ad campaign effectiveness, target ads, and analyze site traffic. Depending on your location, you may opt-in or opt out of the use of these technologies.", + "modal_link_label": "Manage my consent preferences", + "is_default": true, + "privacy_policy_link_label": "Privacy Policy", + "privacy_policy_url": "https://privacy.example.com/", + "privacy_preferences_link_label": "Manage preferences", + "reject_button_label": "Opt out of all", + "save_button_label": "Save", + "title": "Manage your consent preferences", + "privacy_experience_config_history_id": "pri_exp-history-modal-en-000" + }, + { + "language": "es", + "accept_button_label": "Participar en todo", + "acknowledge_button_label": "Aceptar", + "banner_description": "[banner] Usamos cookies y métodos similares para reconocer a los visitantes y recordar sus preferencias. También podemos usarlos para medir la eficacia de campañas publicitarias, dirigir anuncios y analizar el tráfico del sitio. En función de su ubicación, puede optar por participar o no en el uso de estas tecnologías.", + "banner_title": "[banner] Administrar sus preferencias de consentimiento", + "description": "Usamos cookies y métodos similares para reconocer a los visitantes y recordar sus preferencias. También podemos usarlos para medir la eficacia de campañas publicitarias, dirigir anuncios y analizar el tráfico del sitio. En función de su ubicación, puede optar por participar o no en el uso de estas tecnologías.", + "modal_link_label": "Administrar mis preferencias de consentimiento", + "is_default": false, + "privacy_policy_link_label": "Política de privacidad", + "privacy_policy_url": "https://privacy.example.com/", + "privacy_preferences_link_label": "Administrar preferencias", + "reject_button_label": "No participar en nada", + "save_button_label": "Guardar", + "title": "Administrar sus preferencias de consentimiento", + "privacy_experience_config_history_id": "pri_exp-history-modal-es-000" + }, + { + "language": "fr-CA", + "accept_button_label": "Tout activer", + "acknowledge_button_label": "OK", + "banner_description": "[banner] Nous utilisons des témoins et des méthodes similaires pour reconnaitre les visiteurs et se souvenir de leurs préférences. Nous pouvons également les utiliser pour mesurer l’efficacité des campagnes de publicité, cibler les annonces et analyser le trafic du site. Selon votre emplacement, vous pouvez activer ou désactiver l’utilisation de ces technologies.", + "banner_title": "[banner] Gestion de vos préférences de consentement", + "description": "Nous utilisons des témoins et des méthodes similaires pour reconnaitre les visiteurs et se souvenir de leurs préférences. Nous pouvons également les utiliser pour mesurer l’efficacité des campagnes de publicité, cibler les annonces et analyser le trafic du site. Selon votre emplacement, vous pouvez activer ou désactiver l’utilisation de ces technologies.", + "modal_link_label": "Gérer mes préférences de consentement", + "is_default": false, + "privacy_policy_link_label": "Politique de confidentialité", + "privacy_preferences_link_label": "Gestion des préférences", + "reject_button_label": "Tout désactiver", + "save_button_label": "Sauvegarder", + "title": "Gestion de vos préférences de consentement", + "privacy_experience_config_history_id": "pri_exp-history-modal-fr-ca-000" + } + ], + "language": "en", + "dismissable": true, + "show_layer1_notices": false, + "layer1_button_options": "opt_in_opt_out", + "allow_language_selection": true, + "auto_detect_language": true, + "disabled": false, + "id": "pri_exp-modal-000", + "component": "modal", + "created_at": "2024-01-01T12:00:00.000000+00:00", + "updated_at": "2024-01-01T12:00:00.000000+00:00" + }, + "id": "pri_exp-003-exp_modal", + "created_at": "2024-01-01T12:00:00.000000+00:00", + "updated_at": "2024-01-01T12:00:00.000000+00:00", + "privacy_notices": [ + { + "name": "Data Sales and Sharing", + "notice_key": "data_sales_and_sharing", + "internal_description": null, + "origin": null, + "consent_mechanism": "opt_out", + "data_uses": [ + "marketing.advertising.first_party.targeted", + "marketing.advertising.third_party.targeted" + ], + "enforcement_level": "frontend", + "disabled": false, + "has_gpc_flag": false, + "id": "pri_notice-data-sales-000", + "created_at": "2024-01-01T12:00:00.000000+00:00", + "updated_at": "2024-01-01T12:00:00.000000+00:00", + "default_preference": "opt_in", + "cookies": [], + "translations": [ + { + "language": "en", + "title": "Data Sales and Sharing", + "description": "We may transfer or share your personal information to third parties in exchange for monetary or other valuable consideration or for the purposes of cross-contextual targeted advertising. You can learn more about what information is used for this purpose in our privacy notice.", + "privacy_notice_history_id": "pri_notice-history-data-sales-en-000" + }, + { + "language": "es", + "title": "Venta e intercambio de datos", + "description": "Podemos transferir o compartir su información personal con terceros a cambio de una contraprestación monetaria u otro tipo de valor o con fines de publicidad dirigida en contextos cruzados. En nuestra política de privacidad encontrará más detalles sobre qué tipo de información se usa para este fin.", + "privacy_notice_history_id": "pri_notice-history-data-sales-es-000" + }, + { + "language": "fr-CA", + "title": "Ventes et partage de données", + "description": "Nous pouvons transférer à ou partager vos informations personnelles avec des parties tierces en échange de contrepartie monétaire ou d’autres facteurs valables ou encore dans le but de publicité ciblée contextuelle croisée. Vous pouvez en apprendre plus sur l’information utilisée à ces fins dans notre avis de confidentialité.", + "privacy_notice_history_id": "pri_notice-history-data-sales-fr-ca-000" + } + ] + } + ], + "gvl": {} + } + ], + "total": 1, + "page": 1, + "size": 50 +} diff --git a/clients/privacy-center/cypress/fixtures/consent/experience_none.json b/clients/privacy-center/cypress/fixtures/consent/experience_none.json new file mode 100644 index 00000000000..b0993349a35 --- /dev/null +++ b/clients/privacy-center/cypress/fixtures/consent/experience_none.json @@ -0,0 +1,6 @@ +{ + "items": [], + "total": 0, + "page": 1, + "size": 50 +} From b2de707e9a22959d8bc12bc1616f9346d73ad220 Mon Sep 17 00:00:00 2001 From: erosselli <67162025+erosselli@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:57:14 -0300 Subject: [PATCH 4/8] PROD-2644 Improve logging and error visibility for TraversalErrors (#5263) --- CHANGELOG.md | 1 + src/fides/api/common_exceptions.py | 10 ++- src/fides/api/graph/traversal.py | 77 ++++++++++++++++++-- src/fides/api/models/privacy_request.py | 23 ++++++ src/fides/api/task/create_request_tasks.py | 77 +++++++++++--------- src/fides/api/task/deprecated_graph_task.py | 29 +++++++- src/fides/cli/commands/ungrouped.py | 2 - tests/fixtures/application_fixtures.py | 58 ++++++++++++++- tests/ops/task/test_create_request_tasks.py | 54 ++++++++++++++ tests/ops/task/test_deprecated_graph_task.py | 57 +++++++++++++++ 10 files changed, 342 insertions(+), 46 deletions(-) create mode 100644 tests/ops/task/test_deprecated_graph_task.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e12d9427bc..c55117e5d66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The types of changes are: - Validate no path in `server_host` var for CLI config; if there is one then take only up until the first forward slash - Update the Datamap report's Data categories column to support better expand/collapse behavior [#5265](https://github.com/ethyca/fides/pull/5265) - Rename/refactor Privacy Notice Properties to support performance improvements [#5259](https://github.com/ethyca/fides/pull/5259) +- Improved logging and error visibility for TraversalErrors [#5263](https://github.com/ethyca/fides/pull/5263) ### Developer Experience - Added performance mark timings to debug logs for fides.js [#5245](https://github.com/ethyca/fides/pull/5245) diff --git a/src/fides/api/common_exceptions.py b/src/fides/api/common_exceptions.py index 938df04da0f..0396e37f22e 100644 --- a/src/fides/api/common_exceptions.py +++ b/src/fides/api/common_exceptions.py @@ -22,7 +22,15 @@ def __init__(self, message: str, errors: List[str] = []): class TraversalError(FidesopsException): - """Fidesops error with the names of all nodes that could not be reached.""" + """Error with the names of all nodes that could not be reached.""" + + +class UnreachableNodesError(TraversalError): + """Error with the names of all nodes that could not be reached, inherits from TraversalError.""" + + +class UnreachableEdgesError(TraversalError): + """Error with the names of all edges that could not be reached, inherits from TraversalError.""" class ValidationError(FidesopsException): diff --git a/src/fides/api/graph/traversal.py b/src/fides/api/graph/traversal.py index d4aa7ff4d16..a770b0aebc8 100644 --- a/src/fides/api/graph/traversal.py +++ b/src/fides/api/graph/traversal.py @@ -6,8 +6,13 @@ import pydash.collections from fideslang.validation import FidesKey from loguru import logger +from sqlalchemy.orm import Session -from fides.api.common_exceptions import TraversalError +from fides.api.common_exceptions import ( + TraversalError, + UnreachableEdgesError, + UnreachableNodesError, +) from fides.api.graph.config import ( ROOT_COLLECTION_ADDRESS, TERMINATOR_ADDRESS, @@ -19,7 +24,12 @@ ) from fides.api.graph.execution import ExecutionNode from fides.api.graph.graph import DatasetGraph, Edge, Node -from fides.api.models.privacy_request import RequestTask, TraversalDetails +from fides.api.models.privacy_request import ( + PrivacyRequest, + RequestTask, + TraversalDetails, +) +from fides.api.schemas.policy import ActionType from fides.api.util.collection_util import Row, append, partition from fides.api.util.logger_context_utils import Contextualizable, LoggerContextKeys from fides.api.util.matching_queue import MatchingQueue @@ -377,7 +387,7 @@ def edge_ends_with_collection(_edge: Edge) -> bool: ) raise TraversalError( f"""Node could not be reached given the specified ordering: - [{','.join([str(tn.address) for tn in running_node_queue.data])}]""" + [{','.join([str(tn.address) for tn in running_node_queue.data])}]""", ) # Remove nodes that have custom request fields, since we don't care if these are reachable or not. @@ -396,8 +406,9 @@ def edge_ends_with_collection(_edge: Edge) -> bool: "Some nodes were not reachable: {}", ",".join([str(x) for x in remaining_node_keys]), ) - raise TraversalError( - f"Some nodes were not reachable: {','.join([str(x) for x in remaining_node_keys])}" + raise UnreachableNodesError( + f"Some nodes were not reachable: {','.join([str(x) for x in remaining_node_keys])}", + [key.value for key in remaining_node_keys], ) # error if there are edges that have not been visited @@ -406,8 +417,9 @@ def edge_ends_with_collection(_edge: Edge) -> bool: "Some edges were not reachable: {}", ",".join([str(x) for x in remaining_edges]), ) - raise TraversalError( - f"Some edges were not reachable: {','.join([str(x) for x in remaining_edges])}" + raise UnreachableEdgesError( + f"Some edges were not reachable: {','.join([str(x) for x in remaining_edges])}", + [f"{edge}" for edge in remaining_edges], ) end_nodes = [ @@ -416,3 +428,54 @@ def edge_ends_with_collection(_edge: Edge) -> bool: if environment: logger.debug("Found {} end nodes: {}", len(end_nodes), end_nodes) return end_nodes + + +def log_traversal_error_and_update_privacy_request( + privacy_request: PrivacyRequest, session: Session, err: TraversalError +) -> None: + """ + Logs the provided traversal error with the privacy request id, creates the corresponding + ExecutionLog instances, and marks the privacy request as errored. + + If the error is a generic TraversalError, a generic error execution log is created. + If the error is an UnreachableNodesError or UnreachableEdgesError, an execution log is created + for each node / edge on the "errors" list of the exception. + """ + logger.error( + "TraversalError encountered for privacy request {}. Error: {}", + privacy_request.id, + err, + ) + + # For generic TraversalErrors, we log a generic error execution log + if not isinstance(err, UnreachableNodesError) and not isinstance( + err, UnreachableEdgesError + ): + privacy_request.add_error_execution_log( + session, + connection_key=None, + dataset_name=None, + collection_name=None, + message=str(err), + action_type=ActionType.access, + ) + + # For specific ones, we iterate over each error in the list + for error in err.errors: + dataset, collection = ( + error.split(":") + if isinstance( + err, UnreachableNodesError + ) # For unreachable nodes, we can get the dataset and collection from the node + else (None, None) # But not for edges + ) + message = f"{'Node' if isinstance(err, UnreachableNodesError) else 'Edge'} {error} is not reachable" + privacy_request.add_error_execution_log( + session, + connection_key=None, + dataset_name=dataset, + collection_name=collection, + message=message, + action_type=ActionType.access, + ) + privacy_request.error_processing(session) diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index ede46c5519c..d569b4b0d06 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -1281,6 +1281,29 @@ def get_filtered_final_upload(self) -> Dict[str, Dict[str, List[Row]]]: """Fetched the same filtered access results we uploaded to the user""" return self.filtered_final_upload or {} + def add_error_execution_log( + self, + db: Session, + connection_key: Optional[str], + dataset_name: Optional[str], + collection_name: Optional[str], + message: str, + action_type: ActionType, + ) -> ExecutionLog: + execution_log = ExecutionLog.create( + db=db, + data={ + "privacy_request_id": self.id, + "connection_key": connection_key, + "dataset_name": dataset_name, + "collection_name": collection_name, + "status": ExecutionLogStatus.error, + "message": message, + "action_type": action_type, + }, + ) + return execution_log + class PrivacyRequestError(Base): """The DB ORM model to track PrivacyRequests error message status.""" diff --git a/src/fides/api/task/create_request_tasks.py b/src/fides/api/task/create_request_tasks.py index 5c0326b42c6..ae680d97f1d 100644 --- a/src/fides/api/task/create_request_tasks.py +++ b/src/fides/api/task/create_request_tasks.py @@ -15,7 +15,12 @@ FieldAddress, ) from fides.api.graph.graph import DatasetGraph -from fides.api.graph.traversal import ARTIFICIAL_NODES, Traversal, TraversalNode +from fides.api.graph.traversal import ( + ARTIFICIAL_NODES, + Traversal, + TraversalNode, + log_traversal_error_and_update_privacy_request, +) from fides.api.models.connectionconfig import ConnectionConfig from fides.api.models.policy import Policy from fides.api.models.privacy_request import ( @@ -443,41 +448,47 @@ def run_access_request( session, privacy_request, ActionType.access ) else: - logger.info("Building access graph for {}", privacy_request.id) - traversal: Traversal = Traversal(graph, identity) - - # Traversal.traverse populates traversal_nodes in place, adding parents and children to each traversal_node. - traversal_nodes: Dict[CollectionAddress, TraversalNode] = {} - end_nodes: List[CollectionAddress] = traversal.traverse( - traversal_nodes, collect_tasks_fn - ) - # Save Access Request Tasks to the database - ready_tasks = persist_new_access_request_tasks( - session, privacy_request, traversal, traversal_nodes, end_nodes, graph - ) - - if ( - policy.get_rules_for_action(action_type=ActionType.erasure) - and not privacy_request.erasure_tasks.count() - ): - # If applicable, go ahead and save Erasure Request Tasks to the Database. - # These erasure tasks aren't ready to run until the access graph is completed - # in full, but this makes sure the nodes in the graphs match. - erasure_end_nodes: List[CollectionAddress] = list(graph.nodes.keys()) - persist_initial_erasure_request_tasks( - session, privacy_request, traversal_nodes, erasure_end_nodes, graph + try: + logger.info("Building access graph for {}", privacy_request.id) + traversal: Traversal = Traversal(graph, identity) + + # Traversal.traverse populates traversal_nodes in place, adding parents and children to each traversal_node. + traversal_nodes: Dict[CollectionAddress, TraversalNode] = {} + end_nodes: List[CollectionAddress] = traversal.traverse( + traversal_nodes, collect_tasks_fn + ) + # Save Access Request Tasks to the database + ready_tasks = persist_new_access_request_tasks( + session, privacy_request, traversal, traversal_nodes, end_nodes, graph ) - # cache a map of collections -> data uses for the output package of access requests - privacy_request.cache_data_use_map( - format_data_use_map_for_caching( - { - coll_address: tn.node.dataset.connection_key - for (coll_address, tn) in traversal_nodes.items() - }, - connection_configs, + if ( + policy.get_rules_for_action(action_type=ActionType.erasure) + and not privacy_request.erasure_tasks.count() + ): + # If applicable, go ahead and save Erasure Request Tasks to the Database. + # These erasure tasks aren't ready to run until the access graph is completed + # in full, but this makes sure the nodes in the graphs match. + erasure_end_nodes: List[CollectionAddress] = list(graph.nodes.keys()) + persist_initial_erasure_request_tasks( + session, privacy_request, traversal_nodes, erasure_end_nodes, graph + ) + + # cache a map of collections -> data uses for the output package of access requests + privacy_request.cache_data_use_map( + format_data_use_map_for_caching( + { + coll_address: tn.node.dataset.connection_key + for (coll_address, tn) in traversal_nodes.items() + }, + connection_configs, + ) ) - ) + except TraversalError as err: + log_traversal_error_and_update_privacy_request( + privacy_request, session, err + ) + raise err for task in ready_tasks: log_task_queued(task, "main runner") diff --git a/src/fides/api/task/deprecated_graph_task.py b/src/fides/api/task/deprecated_graph_task.py index f46f549d3e8..62c31726752 100644 --- a/src/fides/api/task/deprecated_graph_task.py +++ b/src/fides/api/task/deprecated_graph_task.py @@ -5,6 +5,7 @@ from dask import delayed # type: ignore[attr-defined] from dask.core import getcycle from dask.threaded import get +from loguru import logger from sqlalchemy.orm import Session from fides.api.common_exceptions import TraversalError @@ -14,11 +15,16 @@ CollectionAddress, ) from fides.api.graph.graph import DatasetGraph -from fides.api.graph.traversal import Traversal, TraversalNode +from fides.api.graph.traversal import ( + Traversal, + TraversalNode, + log_traversal_error_and_update_privacy_request, +) from fides.api.models.connectionconfig import ConnectionConfig from fides.api.models.policy import Policy from fides.api.models.privacy_request import PrivacyRequest from fides.api.models.sql_models import System # type: ignore[attr-defined] +from fides.api.schemas.policy import ActionType from fides.api.task.graph_task import EMPTY_REQUEST_TASK, GraphTask from fides.api.task.task_resources import TaskResources from fides.api.util.collection_util import Row @@ -108,7 +114,12 @@ def run_access_request_deprecated( session: Session, ) -> Dict[str, List[Row]]: """Deprecated: Run the access request sequentially in-memory using Dask""" - traversal: Traversal = Traversal(graph, identity) + try: + traversal: Traversal = Traversal(graph, identity) + except TraversalError as err: + log_traversal_error_and_update_privacy_request(privacy_request, session, err) + raise err + with TaskResources( privacy_request, policy, connection_configs, EMPTY_REQUEST_TASK, session ) as resources: @@ -234,6 +245,20 @@ def termination_fn(*dependent_values: int) -> Dict[str, int]: # using an existing function from dask.core to detect cycles in the generated graph collection_cycle = getcycle(dsk, None) if collection_cycle: + logger.error( + "TraversalError encountered for privacy request {}. Error: The values for the `erase_after` fields caused a cycle in the following collections {}", + privacy_request.id, + collection_cycle, + ) + privacy_request.add_error_execution_log( + db=session, + connection_key=None, + collection_name=None, + dataset_name=None, + message=f"The values for the `erase_after` fields caused a cycle in the following collections {collection_cycle}", + action_type=ActionType.erasure, + ) + privacy_request.error_processing(session) raise TraversalError( f"The values for the `erase_after` fields caused a cycle in the following collections {collection_cycle}" ) diff --git a/src/fides/cli/commands/ungrouped.py b/src/fides/cli/commands/ungrouped.py index 42b2a847e59..0005b004254 100644 --- a/src/fides/cli/commands/ungrouped.py +++ b/src/fides/cli/commands/ungrouped.py @@ -150,7 +150,6 @@ def init(ctx: click.Context, fides_dir: str, opt_in: bool) -> None: @click.command() @click.pass_context @with_analytics -@with_server_health_check def status(ctx: click.Context) -> None: """ Check Fides server availability. @@ -183,7 +182,6 @@ def webserver(ctx: click.Context, port: int = 8080) -> None: @click.command() @click.pass_context @with_analytics -@with_server_health_check def worker(ctx: click.Context) -> None: """ Start a Celery worker for the Fides webserver. diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index eb5d8f9db3d..d91bbe73421 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -70,7 +70,7 @@ from fides.api.models.registration import UserRegistration from fides.api.models.sql_models import DataCategory as DataCategoryDbModel from fides.api.models.sql_models import Dataset as CtlDataset -from fides.api.models.sql_models import PrivacyDeclaration, System +from fides.api.models.sql_models import Organization, PrivacyDeclaration, System from fides.api.models.storage import ( ResponseFormat, StorageConfig, @@ -3648,3 +3648,59 @@ def postgres_and_mongo_dataset_graph( dataset_mongo = Dataset(**example_datasets[1]) mongo_graph = convert_dataset_to_graph(dataset_mongo, mongo_connection_config.key) return DatasetGraph(*[graph, mongo_graph]) + + +@pytest.fixture(scope="function") +def dataset_with_unreachable_collections( + db: Session, test_fides_org: Organization +) -> Generator[CtlDataset, None, None]: + dataset = Dataset( + **{ + "name": "dataset with unreachable collections", + "fides_key": "dataset_with_unreachable_collections", + "organization_fides_key": test_fides_org.fides_key, + "collections": [ + { + "name": "login", + "fields": [ + { + "name": "id", + "data_categories": ["user.unique_id"], + }, + { + "name": "customer_id", + "data_categories": ["user.unique_id"], + }, + ], + "fides_meta": {"skip_processing": False}, + }, + { + "name": "report", + "fields": [ + { + "name": "id", + "data_categories": ["user.unique_id"], + }, + { + "name": "email", + "data_categories": ["user.contact.email"], + }, + ], + "fides_meta": {"skip_processing": False}, + }, + ], + }, + ) + + yield dataset + + +@pytest.fixture(scope="function") +def dataset_graph_with_unreachable_collections( + dataset_with_unreachable_collections: Dataset, +) -> Generator[DatasetGraph, None, None]: + graph = convert_dataset_to_graph( + dataset_with_unreachable_collections, "unreachable-dataset-test" + ) + dataset_graph = DatasetGraph(graph) + yield dataset_graph diff --git a/tests/ops/task/test_create_request_tasks.py b/tests/ops/task/test_create_request_tasks.py index e73c433f6f1..387dc7e5494 100644 --- a/tests/ops/task/test_create_request_tasks.py +++ b/tests/ops/task/test_create_request_tasks.py @@ -12,6 +12,7 @@ from fides.api.models.connectionconfig import ConnectionConfig from fides.api.models.datasetconfig import convert_dataset_to_graph from fides.api.models.privacy_request import ( + ExecutionLog, ExecutionLogStatus, PrivacyRequestStatus, RequestTask, @@ -463,6 +464,59 @@ def test_reprocess_access_request_with_existing_request_tasks( assert run_access_node_mock.called run_access_node_mock.assert_called_with(request_task, False) + @mock.patch( + "fides.api.task.create_request_tasks.queue_request_task", + ) + def test_run_access_request_with_unreachable_nodes( + self, + run_access_node_mock, + db, + privacy_request, + policy, + dataset_graph_with_unreachable_collections: DatasetGraph, + ): + """Request tasks created by run_access_request and the root task is queued""" + with pytest.raises(TraversalError) as err: + run_access_request( + privacy_request, + policy, + dataset_graph_with_unreachable_collections, + [], + {"email": "customer-4@example.com"}, + db, + privacy_request_proceed=False, + ) + + assert "Some nodes were not reachable:" in str(err.value) + assert "dataset_with_unreachable_collections:login" in str(err.value) + assert "dataset_with_unreachable_collections:report" in str(err.value) + + db.refresh(privacy_request) + privacy_request.status == PrivacyRequestStatus.error + + # We expect two error logs, one per unreachable collection + error_logs = privacy_request.execution_logs.filter( + ExecutionLog.status == ExecutionLogStatus.error + ) + assert error_logs.count() == 2 + error_logs = sorted( + error_logs, key=lambda execution_log: execution_log.collection_name + ) + assert error_logs[0].dataset_name == "dataset_with_unreachable_collections" + assert error_logs[0].collection_name == "login" + assert ( + error_logs[0].message + == "Node dataset_with_unreachable_collections:login is not reachable" + ) + assert error_logs[1].dataset_name == "dataset_with_unreachable_collections" + assert error_logs[1].collection_name == "report" + assert ( + error_logs[1].message + == "Node dataset_with_unreachable_collections:report is not reachable" + ) + + run_access_node_mock.assert_not_called() + class TestPersistErasureRequestTasks: def test_persist_initial_erasure_request_tasks( diff --git a/tests/ops/task/test_deprecated_graph_task.py b/tests/ops/task/test_deprecated_graph_task.py new file mode 100644 index 00000000000..f2592981044 --- /dev/null +++ b/tests/ops/task/test_deprecated_graph_task.py @@ -0,0 +1,57 @@ +import pytest + +from fides.api.common_exceptions import TraversalError +from fides.api.graph.graph import DatasetGraph +from fides.api.models.privacy_request import ( + ExecutionLog, + ExecutionLogStatus, + PrivacyRequestStatus, +) +from fides.api.task.deprecated_graph_task import run_access_request_deprecated + + +class TestDeprecatedGraphTasks: + def test_run_access_request_deprecated_with_unreachable_nodes( + self, + db, + privacy_request, + policy, + dataset_graph_with_unreachable_collections: DatasetGraph, + ): + with pytest.raises(TraversalError) as err: + run_access_request_deprecated( + privacy_request, + policy, + dataset_graph_with_unreachable_collections, + [], + {"email": "customer-4@example.com"}, + db, + ) + + assert "Some nodes were not reachable:" in str(err.value) + assert "dataset_with_unreachable_collections:login" in str(err.value) + assert "dataset_with_unreachable_collections:report" in str(err.value) + + db.refresh(privacy_request) + privacy_request.status == PrivacyRequestStatus.error + + # We expect two error logs, one per unreachable collection + error_logs = privacy_request.execution_logs.filter( + ExecutionLog.status == ExecutionLogStatus.error + ) + assert error_logs.count() == 2 + error_logs = sorted( + error_logs, key=lambda execution_log: execution_log.collection_name + ) + assert error_logs[0].dataset_name == "dataset_with_unreachable_collections" + assert error_logs[0].collection_name == "login" + assert ( + error_logs[0].message + == "Node dataset_with_unreachable_collections:login is not reachable" + ) + assert error_logs[1].dataset_name == "dataset_with_unreachable_collections" + assert error_logs[1].collection_name == "report" + assert ( + error_logs[1].message + == "Node dataset_with_unreachable_collections:report is not reachable" + ) From ccfef3aa292ffc2e6aee245340824a024ce24d57 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Thu, 12 Sep 2024 14:30:27 -0500 Subject: [PATCH 5/8] Override ConnectionConfigurationResponse.ts secrets type (#5283) --- CHANGELOG.md | 1 + .../src/types/api/models/ConnectionConfigurationResponse.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c55117e5d66..1490ccbdbf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ The types of changes are: ### Fixed - Fix wording in tooltip for Yotpo Reviews [#5274](https://github.com/ethyca/fides/pull/5274) +- Hardcode ConnectionConfigurationResponse.secrets [#5283](https://github.com/ethyca/fides/pull/5283) - Fix Fides.shouldShouldShowExperience() to return false for modal-only experiences [#5281](https://github.com/ethyca/fides/pull/5281) diff --git a/clients/admin-ui/src/types/api/models/ConnectionConfigurationResponse.ts b/clients/admin-ui/src/types/api/models/ConnectionConfigurationResponse.ts index 581d6202cd1..bc511df7fee 100644 --- a/clients/admin-ui/src/types/api/models/ConnectionConfigurationResponse.ts +++ b/clients/admin-ui/src/types/api/models/ConnectionConfigurationResponse.ts @@ -22,7 +22,7 @@ export type ConnectionConfigurationResponse = { last_test_timestamp?: string | null; last_test_succeeded?: boolean | null; saas_config?: SaaSConfigBase | null; - secrets?: null; + secrets?: any; authorized?: boolean | null; enabled_actions?: Array | null; }; From a1ce570a80ef9bba353148d8075005bb462b93c4 Mon Sep 17 00:00:00 2001 From: Kirk Hardy Date: Thu, 12 Sep 2024 15:34:21 -0400 Subject: [PATCH 6/8] PROD-2663: specify dataset on fides pull (#5260) --- CHANGELOG.md | 1 + pyproject.toml | 1 + requirements.txt | 1 + src/fides/cli/__init__.py | 2 +- src/fides/cli/commands/pull.py | 178 ++++++++++++++++++++++++++++ src/fides/cli/commands/ungrouped.py | 36 ------ src/fides/cli/options.py | 26 +++- src/fides/core/pull.py | 63 +++++++++- tests/ctl/cli/test_cli.py | 17 +++ 9 files changed, 284 insertions(+), 41 deletions(-) create mode 100644 src/fides/cli/commands/pull.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1490ccbdbf7..807afa4dea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The types of changes are: - Adding erasure support for Microsoft Advertising [#5197](https://github.com/ethyca/fides/pull/5197) - Implements fuzzy search for identities in Admin-UI Request Manager [#5232](https://github.com/ethyca/fides/pull/5232) - New purpose header field for TCF banner [#5246](https://github.com/ethyca/fides/pull/5246) +- `fides` subcommand `pull` has resource name subcommands that take a `fides_key` argument allowing you to pull only one resource by name and type [#5260](https://github.com/ethyca/fides/pull/5260) ### Changed - Removed unused `username` parameter from the Delighted integration configuration [#5220](https://github.com/ethyca/fides/pull/5220) diff --git a/pyproject.toml b/pyproject.toml index 709be9a701b..554f8a565d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,7 @@ module = [ "cassandra.*", "celery.*", "citext.*", + "click_default_group.*", "dask.*", "deepdiff.*", "defusedxml.ElementTree.*", diff --git a/requirements.txt b/requirements.txt index e33dd0faa68..f9ae2e2d346 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,6 +5,7 @@ APScheduler==3.9.1.post1 asyncpg==0.27.0 boto3==1.26.1 celery[pytest]==5.2.7 +click_default_group==1.2.2 cloud-sql-python-connector==1.9.2 colorama>=0.4.3 cryptography==42.0.0 diff --git a/src/fides/cli/__init__.py b/src/fides/cli/__init__.py index dd5a3d37d33..01bf6a13f86 100644 --- a/src/fides/cli/__init__.py +++ b/src/fides/cli/__init__.py @@ -23,6 +23,7 @@ from .commands.db import database from .commands.deploy import deploy from .commands.generate import generate +from .commands.pull import pull from .commands.scan import scan from .commands.ungrouped import ( delete, @@ -31,7 +32,6 @@ init, list_resources, parse, - pull, push, status, webserver, diff --git a/src/fides/cli/commands/pull.py b/src/fides/cli/commands/pull.py new file mode 100644 index 00000000000..6bd9097d1ec --- /dev/null +++ b/src/fides/cli/commands/pull.py @@ -0,0 +1,178 @@ +from typing import Optional + +import rich_click as click +from click_default_group import DefaultGroup + +from fides.cli.options import fides_key_argument, manifests_dir_argument +from fides.cli.utils import with_analytics, with_server_health_check +from fides.common.utils import echo_red +from fides.core import parse as _parse +from fides.core import pull as _pull +from fides.core.utils import git_is_dirty + + +@click.group(cls=DefaultGroup, default="all", default_if_no_args=True) # type: ignore +@click.pass_context +def pull(ctx: click.Context) -> None: + """ + Update local resource files based on the state of the objects on the server. + """ + + +@pull.command(name="all") # type: ignore +@click.pass_context +@manifests_dir_argument +@click.option( + "--all-resources", + "-a", + default=None, + help="Pulls all locally missing resources from the server into this file.", +) +@with_analytics +@with_server_health_check +def pull_all( + ctx: click.Context, + manifests_dir: str, + all_resources: Optional[str], +) -> None: + """ + Retrieve all resources from the server and update the local manifest files. + """ + + # Make the resources that are pulled configurable + config = ctx.obj["CONFIG"] + # Do this to validate the manifests since they won't get parsed during the pull process + _parse.parse(manifests_dir) + if git_is_dirty(manifests_dir): + echo_red( + f"There are unstaged changes in your manifest directory: '{manifests_dir}' \nAborting pull!" + ) + raise SystemExit(1) + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + all_resources_file=all_resources, + fides_key=None, + resource_type=None, + ) + + +@pull.command(name="dataset") # type: ignore +@click.pass_context +@fides_key_argument +@manifests_dir_argument +def dataset( + ctx: click.Context, + fides_key: str, + manifests_dir: str, +) -> None: + """ + Retrieve a specific dataset from the server and update the local manifest files. + """ + + config = ctx.obj["CONFIG"] + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + fides_key=fides_key, + resource_type="dataset", + all_resources_file=None, + ) + + +@pull.command(name="system") # type: ignore +@click.pass_context +@fides_key_argument +@manifests_dir_argument +def system( + ctx: click.Context, + fides_key: str, + manifests_dir: str, +) -> None: + """ + Retrieve a specific system from the server and update the local manifest files. + """ + + config = ctx.obj["CONFIG"] + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + fides_key=fides_key, + resource_type="system", + all_resources_file=None, + ) + + +@pull.command(name="data_category") # type: ignore +@click.pass_context +@fides_key_argument +@manifests_dir_argument +def data_category( + ctx: click.Context, + fides_key: str, + manifests_dir: str, +) -> None: + """ + Retrieve a specific data_category from the server and update the local manifest files. + """ + + config = ctx.obj["CONFIG"] + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + fides_key=fides_key, + resource_type="data_category", + all_resources_file=None, + ) + + +@pull.command(name="data_use") # type: ignore +@click.pass_context +@fides_key_argument +@manifests_dir_argument +def data_use( + ctx: click.Context, + fides_key: str, + manifests_dir: str, +) -> None: + """ + Retrieve a specific data_use from the server and update the local manifest files. + """ + + config = ctx.obj["CONFIG"] + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + fides_key=fides_key, + resource_type="data_use", + all_resources_file=None, + ) + + +@pull.command(name="data_subject") # type: ignore +@click.pass_context +@fides_key_argument +@manifests_dir_argument +def data_subject( + ctx: click.Context, + fides_key: str, + manifests_dir: str, +) -> None: + """ + Retrieve a specific data_subject from the server and update the local manifest files. + """ + + config = ctx.obj["CONFIG"] + _pull.pull( + url=config.cli.server_url, + manifests_dir=manifests_dir, + headers=config.user.auth_header, + fides_key=fides_key, + resource_type="data_subject", + all_resources_file=None, + ) diff --git a/src/fides/cli/commands/ungrouped.py b/src/fides/cli/commands/ungrouped.py index 0005b004254..45bab0fdbb5 100644 --- a/src/fides/cli/commands/ungrouped.py +++ b/src/fides/cli/commands/ungrouped.py @@ -1,7 +1,6 @@ """Contains all of the ungrouped CLI commands for fides.""" from datetime import datetime, timezone -from typing import Optional import rich_click as click import yaml @@ -33,10 +32,8 @@ from fides.core import audit as _audit from fides.core import evaluate as _evaluate from fides.core import parse as _parse -from fides.core import pull as _pull from fides.core import push as _push from fides.core.api_helpers import get_server_resource, list_server_resources -from fides.core.utils import git_is_dirty @click.command() # type: ignore @@ -311,36 +308,3 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None taxonomy = _parse.parse(manifests_dir=manifests_dir) if verbose: pretty_echo(taxonomy.model_dump(mode="json"), color="green") - - -@click.command() # type: ignore -@click.pass_context -@manifests_dir_argument -@click.option( - "--all-resources", - "-a", - default=None, - help="Pulls all locally missing resources from the server into this file.", -) -@with_analytics -@with_server_health_check -def pull(ctx: click.Context, manifests_dir: str, all_resources: Optional[str]) -> None: - """ - Update local resource files based on the state of the objects on the server. - """ - - # Make the resources that are pulled configurable - config = ctx.obj["CONFIG"] - # Do this to validate the manifests since they won't get parsed during the pull process - _parse.parse(manifests_dir) - if git_is_dirty(manifests_dir): - echo_red( - f"There are unstaged changes in your manifest directory: '{manifests_dir}' \nAborting pull!" - ) - raise SystemExit(1) - _pull.pull( - url=config.cli.server_url, - manifests_dir=manifests_dir, - headers=config.user.auth_header, - all_resources_file=all_resources, - ) diff --git a/src/fides/cli/options.py b/src/fides/cli/options.py index 15ad01597ba..e043d3109fc 100644 --- a/src/fides/cli/options.py +++ b/src/fides/cli/options.py @@ -13,6 +13,18 @@ # If/when that issue is resolved, they can be removed. +def diff_flag(command: Callable) -> Callable: + """Print any diffs between the local & server objects""" + command = click.option( + "--diff", + is_flag=True, + help="Print any diffs between the local & server objects", + )( + command + ) # type: ignore + return command + + def coverage_threshold_option(command: Callable) -> Callable: """An option decorator that sets a required coverage percentage.""" command = click.option( @@ -28,7 +40,7 @@ def coverage_threshold_option(command: Callable) -> Callable: def resource_type_argument(command: Callable) -> Callable: - "Add the resource_type option." + "Add the resource_type argument." command = click.argument( "resource_type", type=click.Choice(model_list, case_sensitive=False), @@ -38,6 +50,18 @@ def resource_type_argument(command: Callable) -> Callable: return command +def resource_type_option(command: Callable) -> Callable: + "Add the resource_type option." + command = click.option( + "--resource-type", + default="", + help=f"Choose from {str(model_list)}", + )( + command + ) # type: ignore + return command + + def fides_key_argument(command: Callable) -> Callable: "Add the fides_key argument." command = click.argument( diff --git a/src/fides/core/pull.py b/src/fides/core/pull.py index 57d4efa1b34..ea057304520 100644 --- a/src/fides/core/pull.py +++ b/src/fides/core/pull.py @@ -6,7 +6,7 @@ from fideslang import model_list from fideslang.manifests import load_yaml_into_dict -from fides.common.utils import echo_green, print_divider +from fides.common.utils import echo_green, echo_red, print_divider from fides.core.api_helpers import get_server_resource, list_server_resources from fides.core.utils import get_manifest_list @@ -23,7 +23,9 @@ def write_manifest_file(manifest_path: str, manifest: Dict) -> None: def pull_existing_resources( - manifests_dir: str, url: str, headers: Dict[str, str] + manifests_dir: str, + url: str, + headers: Dict[str, str], ) -> List[str]: """ Update all of the pre-existing local resources to match their @@ -66,6 +68,43 @@ def pull_existing_resources( return existing_keys +def pull_resource_by_key( + manifests_dir: str, + url: str, + headers: Dict[str, str], + fides_key: str, + resource_type: str, +) -> None: + """ + Pull a resource from the server by its fides_key and update the local manifest file if it exists, + otherwise a new manifest file at {manifests_dir}/{resource_type}.yaml + """ + if manifests_dir[-1] == "/": + manifests_dir = manifests_dir[:-1] + manifest_path = f"{manifests_dir}/{resource_type}.yaml" + print(f"Pulling {resource_type} with fides_key: {fides_key}...", end=" ") + server_resource = get_server_resource(url, resource_type, fides_key, headers) + print("done.") + + if server_resource: + try: + manifest = load_yaml_into_dict(manifest_path) + except FileNotFoundError: + print( + f"Manifest file {manifest_path} does not already exist and will be created" + ) + manifest = {} + print("Writing out resource to file...", end=" ") + manifest[resource_type] = [server_resource] + write_manifest_file(manifest_path, manifest) + print("done.") + + else: + echo_red( + f"{resource_type} with fides_key: {fides_key} not found on the server." + ) + + def pull_missing_resources( manifest_path: str, url: str, @@ -102,6 +141,8 @@ def pull( manifests_dir: str, url: str, headers: Dict[str, str], + fides_key: Optional[str], + resource_type: Optional[str], all_resources_file: Optional[str], ) -> None: """ @@ -110,9 +151,25 @@ def pull( If the 'all_resources_file' option is present, pull all other server resources into a local file. + + If only `fides_key` is provided, only that resource will be pulled. """ + + if fides_key and resource_type: + pull_resource_by_key( + manifests_dir=manifests_dir, + url=url, + headers=headers, + fides_key=fides_key, + resource_type=resource_type, + ) + echo_green("Pull complete.") + return + existing_keys = pull_existing_resources( - manifests_dir=manifests_dir, url=url, headers=headers + manifests_dir=manifests_dir, + url=url, + headers=headers, ) if all_resources_file: diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 07262d747ec..c19cead247f 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -241,6 +241,23 @@ def test_pull_all( print(result.output) assert result.exit_code == 0 + def test_pull_one_resource( + self, + test_config_path: str, + test_cli_runner: CliRunner, + ) -> None: + """ + Pull only one dataset into an empty dir and check if the file is created. + """ + test_dir = ".fides/" + result = test_cli_runner.invoke( + cli, ["-f", test_config_path, "pull", "data_category", "system"] + ) + git_reset(test_dir) + print(result.output) + assert result.exit_code == 0 + assert "not found" not in result.output + @pytest.mark.integration class TestAnnotate: From a83ad23985ede4a6ee9bb911230ebfc317a9de4c Mon Sep 17 00:00:00 2001 From: erosselli <67162025+erosselli@users.noreply.github.com> Date: Mon, 16 Sep 2024 14:35:10 -0300 Subject: [PATCH 7/8] Removes the Dynamic Erasure Email connector from sample project (#5287) --- .../sample_connections/sample_connections.yml | 13 ------------- .../sample_resources/sample_systems.yml | 17 ----------------- tests/ctl/api/test_seed.py | 9 +++------ 3 files changed, 3 insertions(+), 36 deletions(-) diff --git a/src/fides/data/sample_project/sample_connections/sample_connections.yml b/src/fides/data/sample_project/sample_connections/sample_connections.yml index a3e70ef96b0..b0abed9e61c 100644 --- a/src/fides/data/sample_project/sample_connections/sample_connections.yml +++ b/src/fides/data/sample_project/sample_connections/sample_connections.yml @@ -80,16 +80,3 @@ connection: dbname: $FIDES_DEPLOY__CONNECTORS__POSTGRES__DBNAME username: $FIDES_DEPLOY__CONNECTORS__POSTGRES__USERNAME password: $FIDES_DEPLOY__CONNECTORS__POSTGRES__PASSWORD - - key: cookie_house_dynamic_email_erasure_connector - name: Dynamic Email Erasure Connector - connection_type: dynamic_erasure_email - access: write - system_key: cookie_house_dynamic_email_erasure_system - secrets: - third_party_vendor_name: - dataset: postgres_example_custom_request_field_dataset - field: dynamic_email_address_config.vendor_name - recipient_email_address: - dataset: postgres_example_custom_request_field_dataset - field: dynamic_email_address_config.email_address - test_email_address: test@test.com diff --git a/src/fides/data/sample_project/sample_resources/sample_systems.yml b/src/fides/data/sample_project/sample_resources/sample_systems.yml index 22cdf5cc125..e2a85bb3c58 100644 --- a/src/fides/data/sample_project/sample_resources/sample_systems.yml +++ b/src/fides/data/sample_project/sample_resources/sample_systems.yml @@ -95,20 +95,3 @@ system: - customer dataset_references: - postgres_example_custom_request_field_dataset - - - fides_key: cookie_house_dynamic_email_erasure_system - name: Cookie House Dynamic Email Erasure - description: Connector for erasure request emails with dynamic email addresses - system_type: Application - administrating_department: Engineering - egress: - - fides_key: cookie_house - type: system - privacy_declarations: - - data_categories: - - system - data_use: essential.service - data_subjects: - - customer - dataset_references: - - postgres_example_custom_request_field_dataset diff --git a/tests/ctl/api/test_seed.py b/tests/ctl/api/test_seed.py index 43d72fb1d95..6333c2c530f 100644 --- a/tests/ctl/api/test_seed.py +++ b/tests/ctl/api/test_seed.py @@ -494,17 +494,16 @@ async def test_load_samples( dataset_configs = ( (await async_session.execute(select(DatasetConfig))).scalars().all() ) - assert len(systems) == 7 + assert len(systems) == 6 assert len(datasets) == 5 assert len(policies) == 1 - assert len(connections) == 5 + assert len(connections) == 4 assert len(dataset_configs) == 4 assert sorted([e.fides_key for e in systems]) == [ "cookie_house", "cookie_house_custom_request_fields_database", "cookie_house_customer_database", - "cookie_house_dynamic_email_erasure_system", "cookie_house_loyalty_database", "cookie_house_marketing_system", "cookie_house_postgresql_database", @@ -524,7 +523,6 @@ async def test_load_samples( assert sorted([e.key for e in connections]) == [ "cookie_house_custom_request_fields_database", "cookie_house_customer_database_mongodb", - "cookie_house_dynamic_email_erasure_connector", "cookie_house_postgresql_database", "stripe_connector", ] @@ -600,11 +598,10 @@ async def test_load_sample_connections(self): assert False, error_message # Assert that only the connections with all their secrets are returned - assert len(connections) == 5 + assert len(connections) == 4 assert sorted([e.key for e in connections]) == [ "cookie_house_custom_request_fields_database", "cookie_house_customer_database_mongodb", - "cookie_house_dynamic_email_erasure_connector", "cookie_house_postgresql_database", "stripe_connector", ] From c43bb75eb62b5cf88b5ead09c552c007da9d98da Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Mon, 16 Sep 2024 21:28:50 -0500 Subject: [PATCH 8/8] Regenerate FE Types around Privacy Notice Aliases (#5290) Co-authored-by: Adrian Galvan --- clients/admin-ui/src/types/api/models/PrivacyNoticeResponse.ts | 2 +- .../src/types/api/models/PrivacyNoticeResponseWithRegions.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/admin-ui/src/types/api/models/PrivacyNoticeResponse.ts b/clients/admin-ui/src/types/api/models/PrivacyNoticeResponse.ts index 9e7e20b370f..55d11fd140e 100644 --- a/clients/admin-ui/src/types/api/models/PrivacyNoticeResponse.ts +++ b/clients/admin-ui/src/types/api/models/PrivacyNoticeResponse.ts @@ -32,7 +32,7 @@ export type PrivacyNoticeResponse = { created_at: string; updated_at: string; cookies?: Array; - calculated_systems_applicable?: boolean; + systems_applicable?: boolean; translations?: Array; gpp_field_mapping?: Array | null; }; diff --git a/clients/admin-ui/src/types/api/models/PrivacyNoticeResponseWithRegions.ts b/clients/admin-ui/src/types/api/models/PrivacyNoticeResponseWithRegions.ts index 646e44d7f5c..3be2ea27c39 100644 --- a/clients/admin-ui/src/types/api/models/PrivacyNoticeResponseWithRegions.ts +++ b/clients/admin-ui/src/types/api/models/PrivacyNoticeResponseWithRegions.ts @@ -26,7 +26,7 @@ export type PrivacyNoticeResponseWithRegions = { /** * A property calculated by observing which Experiences have linked this Notice */ - configured_regions_for_notice?: Array; + configured_regions?: Array; data_uses: Array; enforcement_level: EnforcementLevel; has_gpc_flag: boolean;