From 9b418b6dee0ae2ce600b951c7d587d1f9ea00bbc Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Mon, 31 Jul 2023 10:27:22 -0700 Subject: [PATCH] Fixing dataset references (#3873) --- CHANGELOG.md | 26 +++++- clients/admin-ui/package.json | 3 +- .../forms/ConnectorParametersForm.tsx | 46 +++++++-- clients/package-lock.json | 1 + src/fides/api/api/v1/endpoints/system.py | 2 +- .../connection_config.py | 7 ++ .../test_connection_config.py | 93 ++++++++++++++++++- 7 files changed, 164 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff26cf5ff2..f015f319cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,29 @@ The types of changes are: - `Fixed` for any bug fixes. - `Security` in case of vulnerabilities. -## [Unreleased](https://github.com/ethyca/fides/compare/2.17.0...main) +## [Unreleased](https://github.com/ethyca/fides/compare/2.17.1...main) + +### Added +- Additional consent reporting calls from `fides-js` [#3845](https://github.com/ethyca/fides/pull/3845) +- Additional consent reporting calls from privacy center [#3847](https://github.com/ethyca/fides/pull/3847) + +### Fixed +- Fix datamap zoom for low system counts [#3835](https://github.com/ethyca/fides/pull/3835) + +### Added + +- HTTP Logging for the Privacy Center [#3783](https://github.com/ethyca/fides/pull/3783) + +### Changed + +- Simplified the file structure for HTML DSR packages [#3848](https://github.com/ethyca/fides/pull/3848) +- Changed max width of form components in "system information" form tab [#3864](https://github.com/ethyca/fides/pull/3864) +- Remove manual system selection screen [#3865](https://github.com/ethyca/fides/pull/3865) + +## [2.17.1](https://github.com/ethyca/fides/compare/2.17.0...2.17.1) + +### Fixed +- Fixed connector forms with external dataset reference fields [#3873](https://github.com/ethyca/fides/pull/3873) ## [2.17.0](https://github.com/ethyca/fides/compare/2.16.0...2.17.0) @@ -72,7 +94,7 @@ The types of changes are: - Privacy center and fides-js now pass in `Unescape-Safestr` as a header so that special characters can be rendered properly [#3706](https://github.com/ethyca/fides/pull/3706) - Fixed ValidationError for saving PrivacyPreferences [#3719](https://github.com/ethyca/fides/pull/3719) - Fixed issue preventing ConnectionConfigs with duplicate names from saving [#3770](https://github.com/ethyca/fides/pull/3770) -- Fixed creating and editing manual integrations [#3772](https://github.com/ethyca/fides/pull/3772) +- Fixed creating and editing manual integrations [#3772](https://github.com/ethyca/fides/pull/3772) - Fix lingering integration artifacts by cascading deletes from System [#3771](https://github.com/ethyca/fides/pull/3771) ### Developer Experience diff --git a/clients/admin-ui/package.json b/clients/admin-ui/package.json index c1b0538a40..80c5700e29 100644 --- a/clients/admin-ui/package.json +++ b/clients/admin-ui/package.json @@ -27,6 +27,7 @@ "test:ci": "jest" }, "dependencies": { + "@chakra-ui/icons": "1.1.7", "@emotion/react": "^11.10.6", "@emotion/styled": "^11.10.6", "@fidesui/components": "^0.7.1", @@ -35,7 +36,6 @@ "@monaco-editor/react": "^4.4.6", "@reduxjs/toolkit": "^1.9.3", "chakra-react-select": "^3.3.7", - "@chakra-ui/icons": "1.1.7", "csv-stringify": "^6.3.0", "cytoscape": "^3.23.0", "cytoscape-klay": "^3.1.4", @@ -46,6 +46,7 @@ "i18n-iso-countries": "^7.5.0", "immer": "^9.0.21", "js-yaml": "^4.1.0", + "lodash": "^4.17.21", "lodash.snakecase": "^4.1.1", "msw": "^1.2.1", "narrow-minded": "^1.2.1", diff --git a/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx b/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx index 0ab7017ebc..44c0cb7371 100644 --- a/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx +++ b/clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/ConnectorParametersForm.tsx @@ -25,6 +25,7 @@ import { import { useLazyGetDatastoreConnectionStatusQuery } from "datastore-connections/datastore-connection.slice"; import DSRCustomizationModal from "datastore-connections/system_portal_config/forms/DSRCustomizationForm/DSRCustomizationModal"; import { Field, FieldInputProps, Form, Formik, FormikProps } from "formik"; +import _ from "lodash"; import React from "react"; import { DatastoreConnectionStatus } from "src/features/datastore-connections/types"; @@ -232,25 +233,47 @@ const ConnectorParametersForm: React.FC = ({ connectionConfig.connection_type === ConnectionType.SAAS ? (connectionConfig.saas_config?.fides_key as string) : connectionConfig.key; + // @ts-ignore - initialValues.secrets = connectionConfig.secrets; + initialValues.secrets = { ...connectionConfig.secrets }; + + // check if we need we need to pre-process any secrets values + // we currently only need to do this for Fides dataset references + // to convert them from objects to dot-delimited strings + if (secretsSchema?.properties) { + Object.entries(secretsSchema.properties).forEach(([key, schema]) => { + if (schema.allOf?.[0].$ref === FIDES_DATASET_REFERENCE) { + const datasetReference = initialValues.secrets[key]; + initialValues.secrets[ + key + ] = `${datasetReference.dataset}.${datasetReference.field}`; + } + }); + } + return initialValues; } return fillInDefaults(initialValues, secretsSchema); }; - const handleSubmit = (values: any, actions: any) => { - // convert each property value of type FidesopsDatasetReference - // from a dot delimited string to a FidesopsDatasetReference - const updatedValues = { ...values }; + /** + * Preprocesses the input values. + * Currently, it is only used to convert FIDES_DATASET_REFERENCE fields. + * @param values ConnectionConfigFormValues - The original values. + * @returns ConnectionConfigFormValues - The processed values. + */ + const preprocessValues = ( + values: ConnectionConfigFormValues + ): ConnectionConfigFormValues => { + const updatedValues = _.cloneDeep(values); if (secretsSchema) { Object.keys(secretsSchema.properties).forEach((key) => { if ( secretsSchema.properties[key].allOf?.[0].$ref === FIDES_DATASET_REFERENCE ) { - const referencePath = values[key].split("."); - updatedValues[key] = { + const referencePath = updatedValues.secrets[key].split("."); + updatedValues.secrets[key] = { dataset: referencePath.shift(), field: referencePath.join("."), direction: "from", @@ -258,7 +281,14 @@ const ConnectorParametersForm: React.FC = ({ } }); } - onSaveClick(updatedValues, actions); + return updatedValues; + }; + + const handleSubmit = (values: any, actions: any) => { + // convert each property value of type FidesopsDatasetReference + // from a dot delimited string to a FidesopsDatasetReference + const processedValues = preprocessValues(values); + onSaveClick(processedValues, actions); }; const handleTestConnectionClick = async () => { diff --git a/clients/package-lock.json b/clients/package-lock.json index 4dfdaabbaf..1a4a270898 100644 --- a/clients/package-lock.json +++ b/clients/package-lock.json @@ -34,6 +34,7 @@ "i18n-iso-countries": "^7.5.0", "immer": "^9.0.21", "js-yaml": "^4.1.0", + "lodash": "^4.17.21", "lodash.snakecase": "^4.1.1", "msw": "^1.2.1", "narrow-minded": "^1.2.1", diff --git a/src/fides/api/api/v1/endpoints/system.py b/src/fides/api/api/v1/endpoints/system.py index fbe76156d3..9d3d922699 100644 --- a/src/fides/api/api/v1/endpoints/system.py +++ b/src/fides/api/api/v1/endpoints/system.py @@ -1,6 +1,6 @@ from typing import Dict, List, Optional -from fastapi import Depends, Response, Security, HTTPException +from fastapi import Depends, HTTPException, Response, Security from fastapi_pagination import Page, Params from fastapi_pagination.bases import AbstractPage from fastapi_pagination.ext.sqlalchemy import paginate diff --git a/src/fides/api/schemas/connection_configuration/connection_config.py b/src/fides/api/schemas/connection_configuration/connection_config.py index 5031b77029..2ada6fd05b 100644 --- a/src/fides/api/schemas/connection_configuration/connection_config.py +++ b/src/fides/api/schemas/connection_configuration/connection_config.py @@ -66,6 +66,13 @@ def mask_sensitive_fields( if connection_secrets is None: return connection_secrets + # The function `mask_sensitive_fields` is called recursively. The recursion can be stopped + # if no properties are found at the current level, indicating that the current value is a dictionary. + # Since individual fields within the dictionary do not need to be masked, + # the function can return early in this case. + if not secret_schema.get("properties"): + return connection_secrets + connection_secret_keys = connection_secrets.keys() secret_schema_keys = secret_schema["properties"].keys() new_connection_secrets = {} diff --git a/tests/ops/schemas/connection_configuration/test_connection_config.py b/tests/ops/schemas/connection_configuration/test_connection_config.py index deef6eca24..8dc4a8519a 100644 --- a/tests/ops/schemas/connection_configuration/test_connection_config.py +++ b/tests/ops/schemas/connection_configuration/test_connection_config.py @@ -1,9 +1,9 @@ +import pytest + from fides.api.schemas.connection_configuration.connection_config import ( mask_sensitive_fields, ) -import pytest - class TestMaskSenstiveValues: @pytest.fixture(scope="function") @@ -30,6 +30,68 @@ def secret_schema(self): "type": "object", } + @pytest.fixture(scope="function") + def secret_schema_with_dataset_references(self): + return { + "title": "doordash_schema", + "description": "Doordash secrets schema", + "type": "object", + "properties": { + "domain": { + "title": "Domain", + "default": "openapi.doordash.com", + "sensitive": False, + "type": "string", + }, + "developer_id": { + "title": "Developer ID", + "sensitive": False, + "type": "string", + }, + "key_id": {"title": "Key ID", "sensitive": False, "type": "string"}, + "signing_secret": { + "title": "Signing Secret", + "sensitive": True, + "type": "string", + }, + "doordash_delivery_id": { + "title": "Doordash Delivery ID", + "external_reference": True, + "allOf": [{"$ref": "#/definitions/FidesDatasetReference"}], + }, + }, + "required": [ + "developer_id", + "key_id", + "signing_secret", + "doordash_delivery_id", + ], + "additionalProperties": False, + "definitions": { + "EdgeDirection": { + "title": "EdgeDirection", + "description": "Direction of a FidesDataSetReference", + "enum": ["from", "to"], + "type": "string", + }, + "FidesDatasetReference": { + "title": "FidesDatasetReference", + "description": "Reference to a field from another Collection", + "type": "object", + "properties": { + "dataset": { + "title": "Dataset", + "pattern": "^[a-zA-Z0-9_.<>-]+$", + "type": "string", + }, + "field": {"title": "Field", "type": "string"}, + "direction": {"$ref": "#/definitions/EdgeDirection"}, + }, + "required": ["dataset", "field"], + }, + }, + } + @pytest.fixture(scope="function") def connection_secrets(self): return { @@ -46,6 +108,33 @@ def test_mask_sensitive_fields(self, secret_schema, connection_secrets): "domain": "api.aircall.io", } + def test_mask_dataset_reference_fields(self, secret_schema_with_dataset_references): + masked_secrets = mask_sensitive_fields( + { + "domain": "openapi.doordash.com", + "developer_id": "123", + "key_id": "123", + "signing_secret": "123", + "doordash_delivery_id": { + "dataset": "shop", + "field": "customer.id", + "direction": "from", + }, + }, + secret_schema_with_dataset_references, + ) + assert masked_secrets == { + "domain": "openapi.doordash.com", + "developer_id": "123", + "key_id": "123", + "signing_secret": "**********", + "doordash_delivery_id": { + "dataset": "shop", + "field": "customer.id", + "direction": "from", + }, + } + def test_mask_sensitive_fields_remove_non_schema_values( self, connection_secrets, secret_schema ):