From 342cd4cd3d934829e457c9228de18113da5e7d28 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 20 Jul 2023 12:48:20 +0800 Subject: [PATCH 01/25] Fix Fideslang Typing --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 49622cbda3..33a2c6744e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,6 @@ module = [ "dask.*", "deepdiff.*", "defusedxml.ElementTree.*", - "fideslang.*", "fideslog.*", "firebase_admin.*", "google.api_core.*", From f738bb7c3aa0040bab1d3971a0b70c48640e3d25 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 20 Jul 2023 14:05:44 +0800 Subject: [PATCH 02/25] fix: implicit exports causing mypy issues --- src/fides/api/api/v1/endpoints/generic.py | 2 +- src/fides/api/api/v1/endpoints/router_factory.py | 3 ++- src/fides/api/db/samples.py | 2 +- src/fides/api/db/seed.py | 2 +- src/fides/api/models/policy.py | 2 +- .../connection_configuration/connection_secrets_saas.py | 2 +- src/fides/api/schemas/messaging/messaging.py | 2 +- src/fides/api/util/data_category.py | 2 +- src/fides/core/annotate_dataset.py | 6 ++++-- src/fides/core/api_helpers.py | 2 +- src/fides/core/parse.py | 2 +- src/fides/core/push.py | 2 +- 12 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/generic.py b/src/fides/api/api/v1/endpoints/generic.py index 5158da86a1..88fb016f06 100644 --- a/src/fides/api/api/v1/endpoints/generic.py +++ b/src/fides/api/api/v1/endpoints/generic.py @@ -2,7 +2,7 @@ This module generates all of the routers for the boilerplate/generic objects that don't require any extra logic. """ -from fideslang import Dataset, Evaluation, Organization, Policy, Registry +from fideslang.models import Dataset, Evaluation, Organization, Policy, Registry from fides.api.schemas.taxonomy_extensions import ( DataCategory, diff --git a/src/fides/api/api/v1/endpoints/router_factory.py b/src/fides/api/api/v1/endpoints/router_factory.py index 09fa13f4a9..863b54202d 100644 --- a/src/fides/api/api/v1/endpoints/router_factory.py +++ b/src/fides/api/api/v1/endpoints/router_factory.py @@ -8,7 +8,8 @@ from typing import Dict, List from fastapi import Depends, HTTPException, Response, Security, status -from fideslang import Dataset, FidesModelType +from fideslang.models import Dataset +from fideslang import FidesModelType from fideslang.validation import FidesKey from pydantic import ValidationError as PydanticValidationError from sqlalchemy.ext.asyncio import AsyncSession diff --git a/src/fides/api/db/samples.py b/src/fides/api/db/samples.py index af2282a06b..ec793e217a 100644 --- a/src/fides/api/db/samples.py +++ b/src/fides/api/db/samples.py @@ -8,7 +8,7 @@ import yaml from expandvars import expandvars # type: ignore -from fideslang import Taxonomy +from fideslang.models import Taxonomy from fideslang.validation import FidesKey # DEFER: This can be changed to importlib.resources once we drop support for Python 3.8 diff --git a/src/fides/api/db/seed.py b/src/fides/api/db/seed.py index 151b4d94aa..39f2caf401 100644 --- a/src/fides/api/db/seed.py +++ b/src/fides/api/db/seed.py @@ -3,7 +3,7 @@ """ from typing import Dict, List, Optional -from fideslang import DEFAULT_TAXONOMY +from fideslang.default_taxonomy import DEFAULT_TAXONOMY from loguru import logger as log from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import Session diff --git a/src/fides/api/models/policy.py b/src/fides/api/models/policy.py index 25326d7b3d..72b4b44ae6 100644 --- a/src/fides/api/models/policy.py +++ b/src/fides/api/models/policy.py @@ -2,7 +2,7 @@ from enum import Enum as EnumType from typing import Any, Dict, List, Optional, Tuple, Union -from fideslang import DEFAULT_TAXONOMY +from fideslang.default_taxonomy import DEFAULT_TAXONOMY from fideslang.models import DataCategory as FideslangDataCategory from fideslang.validation import FidesKey from sqlalchemy import Column diff --git a/src/fides/api/schemas/connection_configuration/connection_secrets_saas.py b/src/fides/api/schemas/connection_configuration/connection_secrets_saas.py index 12dd1d59c8..fa095e870e 100644 --- a/src/fides/api/schemas/connection_configuration/connection_secrets_saas.py +++ b/src/fides/api/schemas/connection_configuration/connection_secrets_saas.py @@ -1,7 +1,7 @@ import abc from typing import Any, Dict, List, Type -from fideslang import FidesDatasetReference +from fideslang.models import FidesDatasetReference from pydantic import BaseModel, Extra, Field, PrivateAttr, create_model, root_validator from pydantic.fields import FieldInfo from sqlalchemy.orm import Session diff --git a/src/fides/api/schemas/messaging/messaging.py b/src/fides/api/schemas/messaging/messaging.py index 753cb0d32b..c0a685ae64 100644 --- a/src/fides/api/schemas/messaging/messaging.py +++ b/src/fides/api/schemas/messaging/messaging.py @@ -3,7 +3,7 @@ from enum import Enum from typing import Any, Dict, List, Optional, Tuple, Type, Union -from fideslang import DEFAULT_TAXONOMY +from fideslang.default_taxonomy import DEFAULT_TAXONOMY from fideslang.validation import FidesKey from pydantic import BaseModel, Extra, root_validator diff --git a/src/fides/api/util/data_category.py b/src/fides/api/util/data_category.py index 472cc71d70..40da0bab38 100644 --- a/src/fides/api/util/data_category.py +++ b/src/fides/api/util/data_category.py @@ -1,7 +1,7 @@ from enum import Enum as EnumType from typing import List, Type -from fideslang import DEFAULT_TAXONOMY +from fideslang.default_taxonomy import DEFAULT_TAXONOMY from fideslang.validation import FidesKey from sqlalchemy.orm import Session diff --git a/src/fides/core/annotate_dataset.py b/src/fides/core/annotate_dataset.py index 305e3a5c3a..b89f0f3966 100644 --- a/src/fides/core/annotate_dataset.py +++ b/src/fides/core/annotate_dataset.py @@ -4,9 +4,11 @@ from typing import List, Union import click -from fideslang import FidesModel, manifests +from fideslang.models import FidesModel +from fideslang import manifests from fideslang.manifests import ingest_manifests -from fideslang.models import Dataset, DatasetCollection, DatasetField, FidesKey +from fideslang.models import Dataset, DatasetCollection, DatasetField +from fideslang.validation import FidesKey from fideslang.validation import FidesValidationError from fides.common.utils import echo_green diff --git a/src/fides/core/api_helpers.py b/src/fides/core/api_helpers.py index eb3061df40..368247e5e9 100644 --- a/src/fides/core/api_helpers.py +++ b/src/fides/core/api_helpers.py @@ -4,7 +4,7 @@ from typing import Dict, List, Optional, Union -from fideslang import FidesModel +from fideslang.models import FidesModel from fideslang.parse import parse_dict from fideslang.validation import FidesKey from requests import Response diff --git a/src/fides/core/parse.py b/src/fides/core/parse.py index bb5c76f33e..6e36c1a3e2 100644 --- a/src/fides/core/parse.py +++ b/src/fides/core/parse.py @@ -1,5 +1,5 @@ """This module is responsible for parsing and verifying file, either with or without a server being available.""" -from fideslang import Taxonomy +from fideslang.models import Taxonomy from fideslang.manifests import ingest_manifests from fideslang.parse import load_manifests_into_taxonomy diff --git a/src/fides/core/push.py b/src/fides/core/push.py index 1101fc3b2c..22b425cb67 100644 --- a/src/fides/core/push.py +++ b/src/fides/core/push.py @@ -4,7 +4,7 @@ from typing import Dict, List, Tuple from deepdiff import DeepDiff -from fideslang import FidesModel, Taxonomy +from fideslang.models import FidesModel, Taxonomy from fides.common.utils import echo_green, echo_red, handle_cli_response from fides.core import api From 3e2bccf9c510e6a86b3e72f32e6615627f368e86 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 20 Jul 2023 14:33:23 +0800 Subject: [PATCH 03/25] fix: more mypy --- src/fides/api/db/system.py | 14 ++++++++++---- src/fides/api/schemas/system.py | 4 ++-- src/fides/core/annotate_dataset.py | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/fides/api/db/system.py b/src/fides/api/db/system.py index 0ec276930a..f3e3f1d4c4 100644 --- a/src/fides/api/db/system.py +++ b/src/fides/api/db/system.py @@ -142,11 +142,17 @@ async def upsert_cookies( privacy_declaration: PrivacyDeclaration, system: System, ) -> None: - """Upsert cookies for the given privacy declaration: retrieve cookies by name/system/privacy declaration + """ + Upsert cookies for the given privacy declaration: retrieve cookies + by name/system/privacy declaration. + Remove any existing cookies that aren't specified here. """ - cookie_list: List[CookieSchema] = cookies or [] - for cookie_data in cookie_list: + + if not cookies: + return + + for cookie_data in cookies: # Check if cookie exists for this name/system/privacy declaration result = await async_session.execute( select(Cookies).where( @@ -180,7 +186,7 @@ async def upsert_cookies( delete_result = await async_session.execute( select(Cookies).where( and_( - Cookies.name.notin_([cookie["name"] for cookie in cookie_list]), + Cookies.name.notin_([cookie["name"] for cookie in cookies]), Cookies.system_id == system.id, Cookies.privacy_declaration_id == privacy_declaration.id, ) diff --git a/src/fides/api/schemas/system.py b/src/fides/api/schemas/system.py index 9111005698..3170a09065 100644 --- a/src/fides/api/schemas/system.py +++ b/src/fides/api/schemas/system.py @@ -1,4 +1,4 @@ -from typing import List, Optional +from typing import List, Optional, Sequence from fideslang.models import Cookies, PrivacyDeclaration, System from pydantic import Field @@ -20,7 +20,7 @@ class PrivacyDeclarationResponse(PrivacyDeclaration): class SystemResponse(System): """Extension of base pydantic model to include `privacy_declarations.id` fields in responses""" - privacy_declarations: List[PrivacyDeclarationResponse] = Field( + privacy_declarations: Sequence[PrivacyDeclarationResponse] = Field( description=PrivacyDeclarationResponse.__doc__, ) diff --git a/src/fides/core/annotate_dataset.py b/src/fides/core/annotate_dataset.py index b89f0f3966..745bdb94df 100644 --- a/src/fides/core/annotate_dataset.py +++ b/src/fides/core/annotate_dataset.py @@ -108,7 +108,7 @@ def annotate_dataset( """ output_dataset = [] - datasets = ingest_manifests(dataset_file)["dataset"] + datasets = [Dataset.parse_obj(dataset) for dataset in ingest_manifests(dataset_file)["dataset"]] resources = api_helpers.list_server_resources( url=str(config.cli.server_url), resource_type=resource_type, @@ -132,7 +132,7 @@ def annotate_dataset( ] for dataset in datasets: - current_dataset = Dataset(**dataset) + current_dataset = Dataset.parse_obj(dataset) try: click.secho(f"\n####\nAnnotating Dataset: [{current_dataset.name}]") From 845e770a9a6bf591ae02311f14e83223de7ccdd7 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 27 Jul 2023 21:48:39 +0800 Subject: [PATCH 04/25] feat: fix more mypy issues --- .../privacy_request/request_runner_service.py | 2 +- src/fides/api/util/connection_util.py | 2 +- src/fides/cli/commands/ungrouped.py | 4 ++-- src/fides/core/api_helpers.py | 20 ++++++++++--------- src/fides/core/audit.py | 10 +++++++--- src/fides/core/dataset.py | 4 +++- src/fides/core/evaluate.py | 5 +++-- 7 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index 1e854150e0..d1e500b8fd 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -502,7 +502,7 @@ def build_consent_dataset_graph(datasets: List[DatasetConfig]) -> DatasetGraph: and saas_config.get("consent_requests") ): consent_datasets.append( - dataset_config.get_dataset_with_stubbed_collection() + dataset_config.get_dataset_with_stubbed_collection() # type: ignore[misc] ) return DatasetGraph(*consent_datasets) diff --git a/src/fides/api/util/connection_util.py b/src/fides/api/util/connection_util.py index e47037e2c4..2aa0cbeacf 100644 --- a/src/fides/api/util/connection_util.py +++ b/src/fides/api/util/connection_util.py @@ -325,7 +325,7 @@ def connection_status( connector = get_connector(connection_config) try: - status: ConnectionTestStatus | None = connector.test_connection() + status: Optional[ConnectionTestStatus] = connector.test_connection() except (ConnectionException, ClientUnsuccessfulException) as exc: logger.warning( diff --git a/src/fides/cli/commands/ungrouped.py b/src/fides/cli/commands/ungrouped.py index 2d8477a2fe..a7051739cc 100644 --- a/src/fides/cli/commands/ungrouped.py +++ b/src/fides/cli/commands/ungrouped.py @@ -1,6 +1,6 @@ """Contains all of the ungrouped CLI commands for fides.""" from datetime import datetime, timezone -from typing import Optional +from typing import Optional, Dict import rich_click as click import yaml @@ -107,7 +107,7 @@ def list_resources(ctx: click.Context, verbose: bool, resource_type: str) -> Non else: if resources: sorted_fides_keys = sorted( - {resource["fides_key"] for resource in resources if resource} + {resource["fides_key"] for resource in resources if resource} # type: ignore[index] ) formatted_fides_keys = "\n ".join(sorted_fides_keys) echo_green( diff --git a/src/fides/core/api_helpers.py b/src/fides/core/api_helpers.py index 368247e5e9..015e56a982 100644 --- a/src/fides/core/api_helpers.py +++ b/src/fides/core/api_helpers.py @@ -67,20 +67,21 @@ def get_server_resource( ) ) - server_resource: Optional[FidesModel] = ( + raw_server_resource: Optional[Dict] = ( raw_server_response.json() if raw_server_response.status_code >= 200 and raw_server_response.status_code <= 299 else None ) - if not raw and server_resource: - server_resource = parse_dict( + if not raw and raw_server_resource: + parsed_server_resource: FidesModel = parse_dict( resource_type=resource_type, resource=raw_server_response.json(), from_server=True, ) + return parsed_server_resource - return server_resource or {} + return raw_server_resource or {} def list_server_resources( @@ -98,7 +99,7 @@ def list_server_resources( response: Response = check_response_auth( api.ls(url=url, resource_type=resource_type, headers=headers) ) - server_resources = ( + raw_server_resources: Optional[List[Dict]] = ( [ resource for resource in response.json() @@ -108,14 +109,15 @@ def list_server_resources( else [] ) - if not raw and server_resources: - server_resources = [ + if not raw and raw_server_resources: + parsed_server_resources: List[FidesModel] = [ parse_dict( resource_type=resource_type, resource=resource_dict, from_server=True, ) - for resource_dict in server_resources + for resource_dict in raw_server_resources ] + return parsed_server_resources - return server_resources + return raw_server_resources diff --git a/src/fides/core/audit.py b/src/fides/core/audit.py index 4bd048fc3c..3d34c39636 100644 --- a/src/fides/core/audit.py +++ b/src/fides/core/audit.py @@ -42,6 +42,7 @@ def audit_systems( pretty_echo( f"Auditing System: {system.name if isinstance(system, FidesModel) else system['name']}" ) + assert isinstance(system, System) new_findings = validate_system_attributes(system, url, headers) audit_findings = audit_findings + new_findings @@ -73,14 +74,16 @@ def validate_system_attributes( data_use = get_server_resource( url, "data_use", privacy_declaration.data_use, headers ) - data_use_findings = audit_data_use_attributes(data_use, system.name) + assert isinstance(data_use, DataUse) + data_use_findings = audit_data_use_attributes(data_use, system.name or "") new_findings = new_findings + data_use_findings for data_subject_fides_key in privacy_declaration.data_subjects: data_subject = get_server_resource( url, "data_subject", data_subject_fides_key, headers ) + assert isinstance(data_subject, DataSubject) data_subject_findings = audit_data_subject_attributes( - data_subject, system.name + data_subject, system.name or "" ) new_findings += data_subject_findings return new_findings @@ -162,6 +165,7 @@ def audit_organizations( print( f"Auditing Organization: {organization.name if isinstance(organization, FidesModel) else organization['name']}" ) + assert isinstance(organization, Organization) new_findings = audit_organization_attributes(organization) audit_findings += new_findings if audit_findings > 0: @@ -188,7 +192,7 @@ def audit_organization_attributes(organization: Organization) -> int: for attribute in organization_attributes: attribute_is_set = getattr(organization, attribute) is not None compliance_messaging( - attribute_is_set, organization.fides_key, attribute, organization.name + attribute_is_set, organization.fides_key, attribute, organization.name or "" ) if not attribute_is_set: audit_findings += 1 diff --git a/src/fides/core/dataset.py b/src/fides/core/dataset.py index ee7a6ee290..bf0f3ddf87 100644 --- a/src/fides/core/dataset.py +++ b/src/fides/core/dataset.py @@ -259,7 +259,9 @@ def scan_dataset_db( datasets in a local manifest (if one is provided). """ manifest_taxonomy = parse(manifest_dir) if manifest_dir else None - manifest_datasets = manifest_taxonomy.dataset if manifest_taxonomy else [] + manifest_datasets = [] + if manifest_taxonomy: + manifest_datasets = manifest_taxonomy.dataset or [] if not local: server_datasets = ( diff --git a/src/fides/core/evaluate.py b/src/fides/core/evaluate.py index 4b4213a33c..5da66c4fd5 100644 --- a/src/fides/core/evaluate.py +++ b/src/fides/core/evaluate.py @@ -55,6 +55,7 @@ def get_evaluation_policies( resource_key=evaluate_fides_key, headers=headers, ) + assert isinstance(server_policy_found, Policy) return [server_policy_found] if server_policy_found else [] local_policy_keys = ( @@ -87,7 +88,7 @@ def get_all_server_policies( policy_list = get_server_resources( url=url, resource_type="policy", headers=headers, existing_keys=policy_keys ) - return policy_list + return policy_list # type: ignore[return-value] def validate_policies_exist(policies: List[Policy], evaluate_fides_key: str) -> None: @@ -114,7 +115,7 @@ def get_fides_key_parent_hierarchy( current_key = fides_key fides_key_parent_hierarchy = [] while True: - fides_key_parent_hierarchy.append(current_key) + fides_key_parent_hierarchy.append(FidesKey(current_key)) found_resource_map = get_resource_by_fides_key( taxonomy=taxonomy, fides_key=current_key ) From a72c7098635b73167557337c2f76f88c06b39dc9 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 27 Jul 2023 22:01:27 +0800 Subject: [PATCH 05/25] fix: more mypy issues --- src/fides/core/evaluate.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/fides/core/evaluate.py b/src/fides/core/evaluate.py index 5da66c4fd5..b42465ee26 100644 --- a/src/fides/core/evaluate.py +++ b/src/fides/core/evaluate.py @@ -212,7 +212,9 @@ def evaluate_policy_rule( # A data subject does not have a hierarchical structure data_subject_violations = compare_rule_to_declaration( rule_types=policy_rule.data_subjects.values, - declaration_type_hierarchies=[[data_subject] for data_subject in data_subjects], + declaration_type_hierarchies=[ + [FidesKey(data_subject)] for data_subject in data_subjects + ], rule_match=policy_rule.data_subjects.matches, ) @@ -241,9 +243,9 @@ def evaluate_policy_rule( ",".join(data_subject_violations), ), violating_attributes=ViolationAttributes( - data_categories=data_category_violations, - data_uses=data_use_violations, - data_subjects=data_subject_violations, + data_categories=list(data_category_violations), + data_uses=list(data_use_violations), + data_subjects=list(data_subject_violations), data_qualifier=data_qualifier, ), ) @@ -256,6 +258,7 @@ def get_dataset_by_fides_key(taxonomy: Taxonomy, fides_key: str) -> Optional[Dat """ Returns a dataset within the taxonomy for a given fides key """ + taxonomy.dataset = getattr(taxonomy, "dataset") or [] dataset = next( iter( [dataset for dataset in taxonomy.dataset if dataset.fides_key == fides_key] @@ -290,8 +293,8 @@ def evaluate_dataset_reference( dataset_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, - data_subjects=privacy_declaration.data_subjects, - data_categories=dataset.data_categories, + data_subjects=[str(x) for x in privacy_declaration.data_subjects], + data_categories=[str(x) for x in dataset.data_categories], data_qualifier=dataset.data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=dataset_violation_message, @@ -312,8 +315,8 @@ def evaluate_dataset_reference( dataset_collection_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, - data_subjects=privacy_declaration.data_subjects, - data_categories=collection.data_categories, + data_subjects=[str(x) for x in privacy_declaration.data_subjects], + data_categories=[str(x) for x in collection.data_categories], data_qualifier=collection.data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=collection_violation_message, @@ -334,8 +337,8 @@ def evaluate_dataset_reference( field_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, - data_subjects=privacy_declaration.data_subjects, - data_categories=field.data_categories, + data_subjects=[str(x) for x in privacy_declaration.data_subjects], + data_categories=[str(x) for x in field.data_categories], data_qualifier=field.data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=field_violation_message, @@ -371,8 +374,8 @@ def evaluate_privacy_declaration( declaration_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, - data_subjects=privacy_declaration.data_subjects, - data_categories=privacy_declaration.data_categories, + data_subjects=[str(x) for x in privacy_declaration.data_subjects], + data_categories=[str(x) for x in privacy_declaration.data_categories], data_qualifier=privacy_declaration.data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=declaration_violation_message, @@ -409,6 +412,8 @@ def execute_evaluation(taxonomy: Taxonomy) -> Evaluation: each system's privacy declarations. """ evaluation_violation_list = [] + taxonomy.policy = getattr(taxonomy, "policy") or [] + taxonomy.system = getattr(taxonomy, "system") or [] for policy in taxonomy.policy: for rule in policy.rules: for system in taxonomy.system: @@ -425,7 +430,7 @@ def execute_evaluation(taxonomy: Taxonomy) -> Evaluation: ) new_uuid = str(uuid.uuid4()).replace("-", "_") evaluation = Evaluation( - fides_key=new_uuid, + fides_key=FidesKey(new_uuid), status=status_enum, violations=evaluation_violation_list, ) @@ -470,7 +475,7 @@ def populate_referenced_keys( Recursively calls itself after every hydration to make sure there are no new missing keys. """ - missing_resource_keys = get_referenced_missing_keys(taxonomy) + missing_resource_keys = list(get_referenced_missing_keys(taxonomy)) keys_not_found = set(last_keys).intersection(set(missing_resource_keys)) if keys_not_found: echo_red(f"Missing resource keys: {keys_not_found}") From 4f04a3699ee3b466f80501b69be460eb56a168dd Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 31 Jul 2023 12:22:42 +0800 Subject: [PATCH 06/25] fix: more mypy issues --- src/fides/api/schemas/dataset.py | 10 +++++---- .../privacy_request/request_runner_service.py | 2 +- src/fides/cli/commands/generate.py | 6 ++++-- src/fides/cli/commands/ungrouped.py | 3 ++- src/fides/core/annotate_dataset.py | 4 ++-- src/fides/core/api_helpers.py | 8 ++++++- src/fides/core/dataset.py | 13 ++++++------ src/fides/core/evaluate.py | 1 + src/fides/core/push.py | 4 ++-- src/fides/core/system.py | 21 +++++++++++++++---- 10 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/fides/api/schemas/dataset.py b/src/fides/api/schemas/dataset.py index 7b4683dea7..7a4e5bbb6d 100644 --- a/src/fides/api/schemas/dataset.py +++ b/src/fides/api/schemas/dataset.py @@ -1,4 +1,4 @@ -from typing import Any, List, Optional, Type +from typing import Any, List, Optional, Type, Sequence from fideslang.models import Dataset, DatasetCollection, DatasetField from fideslang.validation import FidesKey @@ -25,7 +25,9 @@ def validate_data_categories_against_db( logger.info( "No data categories in the database: reverting to default data categories." ) - defined_data_categories = list(DefaultTaxonomyDataCategories.__members__.keys()) + defined_data_categories = [ + FidesKey(key) for key in DefaultTaxonomyDataCategories.__members__.keys() + ] class DataCategoryValidationMixin(BaseModel): @validator("data_categories", check_fields=False, allow_reuse=True) @@ -43,10 +45,10 @@ class FieldDataCategoryValidation(DatasetField, DataCategoryValidationMixin): class CollectionDataCategoryValidation( DatasetCollection, DataCategoryValidationMixin ): - fields: List[FieldDataCategoryValidation] = [] + fields: Sequence[FieldDataCategoryValidation] = [] class DatasetDataCategoryValidation(Dataset, DataCategoryValidationMixin): - collections: List[CollectionDataCategoryValidation] + collections: Sequence[CollectionDataCategoryValidation] DatasetDataCategoryValidation(**dataset.dict()) diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index d1e500b8fd..aeb6a8b916 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -502,7 +502,7 @@ def build_consent_dataset_graph(datasets: List[DatasetConfig]) -> DatasetGraph: and saas_config.get("consent_requests") ): consent_datasets.append( - dataset_config.get_dataset_with_stubbed_collection() # type: ignore[misc] + dataset_config.get_dataset_with_stubbed_collection() # type: ignore[arg-type, assignment] ) return DatasetGraph(*consent_datasets) diff --git a/src/fides/cli/commands/generate.py b/src/fides/cli/commands/generate.py index 6eec5a01f0..1a70b67874 100644 --- a/src/fides/cli/commands/generate.py +++ b/src/fides/cli/commands/generate.py @@ -150,10 +150,12 @@ def generate_dataset_dynamodb( credentials_id=credentials_id, ) - bigquery_datasets = _dataset.generate_dynamo_db_datasets(aws_config) + bigquery_dataset = _dataset.generate_dynamo_db_datasets(aws_config) _dataset.write_dataset_manifest( - file_name=output_filename, include_null=include_null, datasets=bigquery_datasets + file_name=output_filename, + include_null=include_null, + datasets=[bigquery_dataset], ) diff --git a/src/fides/cli/commands/ungrouped.py b/src/fides/cli/commands/ungrouped.py index a7051739cc..251ddf3ccb 100644 --- a/src/fides/cli/commands/ungrouped.py +++ b/src/fides/cli/commands/ungrouped.py @@ -278,6 +278,7 @@ def evaluate( ) if audit: + taxonomy = _parse.parse(manifests_dir) print_divider() pretty_echo("Auditing Organization Resource Compliance") _audit.audit_organizations( @@ -292,7 +293,7 @@ def evaluate( _audit.audit_systems( url=config.cli.server_url, headers=config.user.auth_header, - include_keys=[system.fides_key for system in taxonomy.system], + include_keys=[system.fides_key for system in taxonomy.system or []], ) diff --git a/src/fides/core/annotate_dataset.py b/src/fides/core/annotate_dataset.py index 745bdb94df..c138a68d85 100644 --- a/src/fides/core/annotate_dataset.py +++ b/src/fides/core/annotate_dataset.py @@ -45,7 +45,7 @@ def get_data_categories_annotation( dataset_member: Union[Dataset, DatasetCollection, DatasetField], valid_categories: List[str], validate: bool = True, -) -> List[str]: +) -> List[FidesKey]: """ Request the user's input to supply a list of data categories @@ -82,7 +82,7 @@ def get_data_categories_annotation( dataset_member, valid_categories ) - return user_response + return [FidesKey(value) for value in user_response] def annotate_dataset( diff --git a/src/fides/core/api_helpers.py b/src/fides/core/api_helpers.py index 015e56a982..8d490e4649 100644 --- a/src/fides/core/api_helpers.py +++ b/src/fides/core/api_helpers.py @@ -25,7 +25,7 @@ def get_server_resources( If the resource does not exist on the server, an error will _not_ be thrown. Instead, an empty object will be stored and then filtered out. """ - server_resources: List[FidesModel] = list( + server_resources = list( filter( None, [ @@ -34,11 +34,17 @@ def get_server_resources( resource_type=resource_type, resource_key=key, headers=headers, + raw=True, ) for key in existing_keys ], ) ) + server_resources = [ + parse_dict(resource_type=resource_type, resource=resource, from_server=True) + for resource in server_resources + ] + return server_resources diff --git a/src/fides/core/dataset.py b/src/fides/core/dataset.py index bf0f3ddf87..fbbff5a8dd 100644 --- a/src/fides/core/dataset.py +++ b/src/fides/core/dataset.py @@ -4,6 +4,7 @@ import sqlalchemy from fideslang import manifests from fideslang.models import Dataset, DatasetCollection, DatasetField +from fideslang.validation import FidesKey from pydantic import AnyHttpUrl from sqlalchemy.engine import Engine from sqlalchemy.sql import text @@ -41,13 +42,11 @@ def get_all_server_datasets( dataset_list = list_server_resources( url=url, resource_type="dataset", - exclude_keys=exclude_dataset_keys, + exclude_keys=[str(x) for x in exclude_dataset_keys], headers=headers, ) - if not dataset_list: - return None - return dataset_list + return dataset_list # type: ignore[return-value] def include_dataset_schema(schema: str, database_type: str) -> bool: @@ -135,8 +134,8 @@ def make_dataset_key_unique( to avoid naming collisions. """ - dataset.fides_key = generate_unique_fides_key( - dataset.fides_key, database_host, database_name + dataset.fides_key = FidesKey( + generate_unique_fides_key(dataset.fides_key, database_host, database_name) ) dataset.meta = {"database_host": database_host, "database_name": database_name} return dataset @@ -227,7 +226,7 @@ def print_dataset_db_scan_result( Prints uncategorized fields and raises an exception if coverage is lower than provided threshold. """ - dataset_names = [dataset.name for dataset in datasets] + dataset_names: List[str] = [dataset.name or "" for dataset in datasets] output: str = "Successfully scanned the following datasets:\n" output += "\t{}\n".format("\n\t".join(dataset_names)) echo_green(output) diff --git a/src/fides/core/evaluate.py b/src/fides/core/evaluate.py index b42465ee26..4ce3c9326a 100644 --- a/src/fides/core/evaluate.py +++ b/src/fides/core/evaluate.py @@ -541,6 +541,7 @@ def evaluate( # Determine which Policies will be evaluated policies = taxonomy.policy + assert policies, "At least one Policy must be present" if not local: # Append server-side Policies if not running in local_mode policies = get_evaluation_policies( diff --git a/src/fides/core/push.py b/src/fides/core/push.py index 22b425cb67..129330f9d3 100644 --- a/src/fides/core/push.py +++ b/src/fides/core/push.py @@ -80,14 +80,14 @@ def get_orphan_datasets(taxonomy: Taxonomy) -> List: referenced_datasets.update( [ dataset_reference - for resource in taxonomy.system + for resource in taxonomy.system or [] for privacy_declaration in resource.privacy_declarations if privacy_declaration.dataset_references is not None for dataset_reference in privacy_declaration.dataset_references ] ) - datasets.update([resource.fides_key for resource in taxonomy.dataset]) + datasets.update([resource.fides_key for resource in taxonomy.dataset or []]) missing_datasets = list(datasets - referenced_datasets) diff --git a/src/fides/core/system.py b/src/fides/core/system.py index 6fe024bcfb..ce2469f783 100644 --- a/src/fides/core/system.py +++ b/src/fides/core/system.py @@ -4,7 +4,7 @@ from typing import Dict, List, Optional from fideslang import manifests -from fideslang.models import Organization, System +from fideslang.models import Organization, System, FidesModel from pydantic import AnyHttpUrl from fides.common.utils import echo_green, echo_red, handle_cli_response @@ -100,6 +100,8 @@ def get_organization( ) ) raise SystemExit(1) + + assert isinstance(server_organization, Organization) return server_organization @@ -223,7 +225,7 @@ def generate_system_okta( def get_all_server_systems( url: AnyHttpUrl, headers: Dict[str, str], exclude_systems: List[System] -) -> List[System]: +) -> List[FidesModel]: """ Get a list of all of the Systems that exist on the server. Excludes any systems provided in exclude_systems @@ -240,6 +242,7 @@ def get_all_server_systems( system_list = get_server_resources( url=url, resource_type="system", headers=headers, existing_keys=system_keys ) + return system_list @@ -362,7 +365,11 @@ def scan_system_aws( """ manifest_taxonomy = parse(manifest_dir) if manifest_dir else None - manifest_systems = manifest_taxonomy.system if manifest_taxonomy else [] + manifest_systems = ( + manifest_taxonomy.system + if manifest_taxonomy and manifest_taxonomy.system + else [] + ) server_systems = get_all_server_systems( url=url, headers=headers, exclude_systems=manifest_systems ) @@ -376,6 +383,7 @@ def scan_system_aws( url=url, headers=headers, ) + assert organization aws_systems = generate_aws_systems(organization=organization, aws_config=aws_config) @@ -408,7 +416,11 @@ def scan_system_okta( """ manifest_taxonomy = parse(manifest_dir) if manifest_dir else None - manifest_systems = manifest_taxonomy.system if manifest_taxonomy else [] + manifest_systems = ( + manifest_taxonomy.system + if manifest_taxonomy and manifest_taxonomy.system + else [] + ) server_systems = get_all_server_systems( url=url, headers=headers, exclude_systems=manifest_systems ) @@ -421,6 +433,7 @@ def scan_system_okta( url=url, headers=headers, ) + assert organization okta_systems = asyncio.run( generate_okta_systems(organization=organization, okta_config=okta_config) From aeec9023796c1fa6b48f45eb07f257c98bb68743 Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 31 Jul 2023 14:48:23 +0800 Subject: [PATCH 07/25] fix: mypy --- .../api/api/v1/endpoints/router_factory.py | 2 +- src/fides/api/api/v1/endpoints/system.py | 2 +- src/fides/api/db/system.py | 1 - src/fides/api/models/datasetconfig.py | 6 ++-- src/fides/api/models/privacy_notice.py | 3 +- src/fides/api/models/sql_models.py | 2 +- .../api/oauth/system_manager_oauth_util.py | 8 +++-- src/fides/api/schemas/dataset.py | 8 ++--- src/fides/api/schemas/messaging/messaging.py | 2 +- src/fides/api/schemas/system.py | 2 +- src/fides/api/util/endpoint_utils.py | 23 ++++++++++++- src/fides/cli/commands/ungrouped.py | 4 +-- src/fides/core/annotate_dataset.py | 26 ++++++++------- src/fides/core/api_helpers.py | 33 ++++--------------- src/fides/core/parse.py | 2 +- src/fides/core/pull.py | 3 +- src/fides/core/system.py | 17 +++++++--- .../test_connection_config.py | 4 +-- 18 files changed, 78 insertions(+), 70 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/router_factory.py b/src/fides/api/api/v1/endpoints/router_factory.py index 863b54202d..cf0f61ef05 100644 --- a/src/fides/api/api/v1/endpoints/router_factory.py +++ b/src/fides/api/api/v1/endpoints/router_factory.py @@ -8,8 +8,8 @@ from typing import Dict, List from fastapi import Depends, HTTPException, Response, Security, status -from fideslang.models import Dataset from fideslang import FidesModelType +from fideslang.models import Dataset from fideslang.validation import FidesKey from pydantic import ValidationError as PydanticValidationError from sqlalchemy.ext.asyncio import AsyncSession 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/db/system.py b/src/fides/api/db/system.py index f3e3f1d4c4..cf1b757e00 100644 --- a/src/fides/api/db/system.py +++ b/src/fides/api/db/system.py @@ -4,7 +4,6 @@ from typing import Dict, List, Optional, Tuple from fastapi import HTTPException -from fideslang.models import Cookies as CookieSchema from fideslang.models import System as SystemSchema from loguru import logger as log from sqlalchemy import and_, delete, insert, select, update diff --git a/src/fides/api/models/datasetconfig.py b/src/fides/api/models/datasetconfig.py index 7ecce4aa8e..8a18ab5b69 100644 --- a/src/fides/api/models/datasetconfig.py +++ b/src/fides/api/models/datasetconfig.py @@ -168,7 +168,7 @@ def get_graph(self) -> GraphDataset: return dataset_graph - def get_dataset_with_stubbed_collection(self) -> Dataset: + def get_dataset_with_stubbed_collection(self) -> GraphDataset: """ Return a Dataset with a single mock Collection for use in building a graph where we only want one node per dataset, instead of one node per collection. Note that @@ -179,7 +179,7 @@ def get_dataset_with_stubbed_collection(self) -> Dataset: is that this single node represents the overall Dataset, and will execute Dataset-level requests, not Collection-level requests. """ - dataset_graph: Dataset = self.get_graph() + dataset_graph = self.get_graph() stubbed_collection = Collection(name=dataset_graph.name, fields=[], after=set()) dataset_graph.collections = [stubbed_collection] @@ -256,7 +256,7 @@ def to_graph_field( data_categories=field.data_categories, identity=identity, data_type_name=data_type_name, # type: ignore - references=references, + references=references, # type: ignore is_pk=is_pk, length=length, is_array=is_array, diff --git a/src/fides/api/models/privacy_notice.py b/src/fides/api/models/privacy_notice.py index 1d999b4d69..c2137572ed 100644 --- a/src/fides/api/models/privacy_notice.py +++ b/src/fides/api/models/privacy_notice.py @@ -187,8 +187,7 @@ def generate_notice_key(cls, name: Optional[str]) -> FidesKey: if not isinstance(name, str): raise Exception("Privacy notice keys must be generated from a string.") notice_key: str = re.sub(r"\s+", "_", name.lower().strip()) - FidesKey.validate(notice_key) - return notice_key + return FidesKey(notice_key) def dry_update(self, *, data: dict[str, Any]) -> FidesBase: """ diff --git a/src/fides/api/models/sql_models.py b/src/fides/api/models/sql_models.py index d54dbe84c7..7dd55ec67f 100644 --- a/src/fides/api/models/sql_models.py +++ b/src/fides/api/models/sql_models.py @@ -469,7 +469,7 @@ class SystemScans(Base): models_with_default_field = [ sql_model - for _, sql_model in sql_model_map.items() + for sql_model in sql_model_map.values() if hasattr(sql_model, "is_default") ] diff --git a/src/fides/api/oauth/system_manager_oauth_util.py b/src/fides/api/oauth/system_manager_oauth_util.py index b69fcce341..d4119107bc 100644 --- a/src/fides/api/oauth/system_manager_oauth_util.py +++ b/src/fides/api/oauth/system_manager_oauth_util.py @@ -88,12 +88,14 @@ async def verify_oauth_client_for_system_from_request_body( Yields a 403 forbidden error if not. """ - return has_system_permissions( + permissions = has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) + assert isinstance(permissions, SystemSchema) + return permissions async def verify_oauth_client_for_system_from_fides_key( @@ -111,12 +113,14 @@ async def verify_oauth_client_for_system_from_fides_key( Yields a 403 forbidden error if not. """ - return has_system_permissions( + permissions = has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) + assert isinstance(permissions, str) + return permissions def has_system_permissions( diff --git a/src/fides/api/schemas/dataset.py b/src/fides/api/schemas/dataset.py index 7a4e5bbb6d..63d86cede6 100644 --- a/src/fides/api/schemas/dataset.py +++ b/src/fides/api/schemas/dataset.py @@ -1,4 +1,4 @@ -from typing import Any, List, Optional, Type, Sequence +from typing import Any, List, Optional, Sequence, Type from fideslang.models import Dataset, DatasetCollection, DatasetField from fideslang.validation import FidesKey @@ -38,17 +38,17 @@ def valid_data_categories( return _valid_data_categories(v, defined_data_categories) class FieldDataCategoryValidation(DatasetField, DataCategoryValidationMixin): - fields: Optional[List["FieldDataCategoryValidation"]] + fields: Optional[List["FieldDataCategoryValidation"]] # type: ignore[assignment] FieldDataCategoryValidation.update_forward_refs() class CollectionDataCategoryValidation( DatasetCollection, DataCategoryValidationMixin ): - fields: Sequence[FieldDataCategoryValidation] = [] + fields: Sequence[FieldDataCategoryValidation] = [] # type: ignore[assignment] class DatasetDataCategoryValidation(Dataset, DataCategoryValidationMixin): - collections: Sequence[CollectionDataCategoryValidation] + collections: Sequence[CollectionDataCategoryValidation] # type: ignore[assignment] DatasetDataCategoryValidation(**dataset.dict()) diff --git a/src/fides/api/schemas/messaging/messaging.py b/src/fides/api/schemas/messaging/messaging.py index c0a685ae64..a562074989 100644 --- a/src/fides/api/schemas/messaging/messaging.py +++ b/src/fides/api/schemas/messaging/messaging.py @@ -125,7 +125,7 @@ def transform_data_use_format(cls, values: Dict[str, Any]) -> Dict[str, Any]: preference.data_use = next( ( data_use.name - for data_use in DEFAULT_TAXONOMY.data_use + for data_use in DEFAULT_TAXONOMY.data_use or [] if data_use.fides_key == preference.data_use ), preference.data_use, diff --git a/src/fides/api/schemas/system.py b/src/fides/api/schemas/system.py index 3170a09065..0e6bad7938 100644 --- a/src/fides/api/schemas/system.py +++ b/src/fides/api/schemas/system.py @@ -20,7 +20,7 @@ class PrivacyDeclarationResponse(PrivacyDeclaration): class SystemResponse(System): """Extension of base pydantic model to include `privacy_declarations.id` fields in responses""" - privacy_declarations: Sequence[PrivacyDeclarationResponse] = Field( + privacy_declarations: Sequence[PrivacyDeclarationResponse] = Field( # type: ignore[assignment] description=PrivacyDeclarationResponse.__doc__, ) diff --git a/src/fides/api/util/endpoint_utils.py b/src/fides/api/util/endpoint_utils.py index ed5bc6da6a..75a6de8ba1 100644 --- a/src/fides/api/util/endpoint_utils.py +++ b/src/fides/api/util/endpoint_utils.py @@ -10,10 +10,14 @@ from sqlalchemy.ext.asyncio import AsyncSession from starlette.status import HTTP_400_BAD_REQUEST -from fides.api.db.base import Base # type: ignore[attr-defined] +from fides.api.db.base import Base # type: ignore from fides.api.db.crud import get_resource, list_resource from fides.api.models.sql_models import ( # type: ignore[attr-defined] models_with_default_field, + DataCategory as SQLDataCategory, + DataUse as SQLDataUse, + DataSubject as SQLDataSubject, + DataQualifier as SQLDataQualifier, ) from fides.api.util import errors from fides.common.api.scope_registry import ( @@ -69,6 +73,23 @@ async def forbid_if_editing_is_default( """ if sql_model in models_with_default_field: resource = await get_resource(sql_model, fides_key, async_session) + + assert ( + isinstance(resource, SQLDataCategory) + or isinstance(resource, SQLDataQualifier) + or isinstance(resource, SQLDataSubject) + or isinstance(resource, SQLDataUse) + ) + + assert ( + isinstance(payload, SQLDataCategory) + or isinstance(payload, SQLDataQualifier) + or isinstance(payload, SQLDataSubject) + or isinstance(payload, SQLDataUse) + ) + + assert hasattr(resource, "is_default") + if resource.is_default != payload.is_default: raise errors.ForbiddenError(sql_model.__name__, fides_key) diff --git a/src/fides/cli/commands/ungrouped.py b/src/fides/cli/commands/ungrouped.py index 251ddf3ccb..556c18ad97 100644 --- a/src/fides/cli/commands/ungrouped.py +++ b/src/fides/cli/commands/ungrouped.py @@ -1,6 +1,6 @@ """Contains all of the ungrouped CLI commands for fides.""" from datetime import datetime, timezone -from typing import Optional, Dict +from typing import Optional import rich_click as click import yaml @@ -76,7 +76,6 @@ def get_resource(ctx: click.Context, resource_type: str, fides_key: str) -> None resource_type=resource_type, resource_key=fides_key, headers=config.user.auth_header, - raw=True, ) print_divider() echo_green(yaml.dump({resource_type: [resource]})) @@ -99,7 +98,6 @@ def list_resources(ctx: click.Context, verbose: bool, resource_type: str) -> Non resource_type=resource_type, headers=config.user.auth_header, exclude_keys=[], - raw=True, ) print_divider() if verbose: diff --git a/src/fides/core/annotate_dataset.py b/src/fides/core/annotate_dataset.py index c138a68d85..4e373f9a73 100644 --- a/src/fides/core/annotate_dataset.py +++ b/src/fides/core/annotate_dataset.py @@ -4,12 +4,11 @@ from typing import List, Union import click -from fideslang.models import FidesModel from fideslang import manifests +from fideslang.parse import parse_dict from fideslang.manifests import ingest_manifests -from fideslang.models import Dataset, DatasetCollection, DatasetField -from fideslang.validation import FidesKey -from fideslang.validation import FidesValidationError +from fideslang.models import Dataset, DatasetCollection, DatasetField, FidesModel +from fideslang.validation import FidesKey, FidesValidationError from fides.common.utils import echo_green from fides.config import FidesConfig @@ -108,8 +107,11 @@ def annotate_dataset( """ output_dataset = [] - datasets = [Dataset.parse_obj(dataset) for dataset in ingest_manifests(dataset_file)["dataset"]] - resources = api_helpers.list_server_resources( + datasets = [ + Dataset.parse_obj(dataset) + for dataset in ingest_manifests(dataset_file)["dataset"] + ] + raw_resources = api_helpers.list_server_resources( url=str(config.cli.server_url), resource_type=resource_type, headers=config.user.auth_header @@ -117,20 +119,20 @@ def annotate_dataset( exclude_keys=[], ) - if not resources: + if not raw_resources: click.secho( "No server resources were found.", fg="red", ) return - existing_categories: List[str] = [ - resource.fides_key - if isinstance(resource, FidesModel) - else resource["fides_key"] - for resource in resources + resources = [ + parse_dict(resource_type=resource_type, resource=resource) + for resource in raw_resources ] + existing_categories: List[str] = [resource.fides_key for resource in resources] + for dataset in datasets: current_dataset = Dataset.parse_obj(dataset) try: diff --git a/src/fides/core/api_helpers.py b/src/fides/core/api_helpers.py index 8d490e4649..25bae657d0 100644 --- a/src/fides/core/api_helpers.py +++ b/src/fides/core/api_helpers.py @@ -2,7 +2,7 @@ Reusable utilities meant to make repetitive api-related tasks easier. """ -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional from fideslang.models import FidesModel from fideslang.parse import parse_dict @@ -25,7 +25,7 @@ def get_server_resources( If the resource does not exist on the server, an error will _not_ be thrown. Instead, an empty object will be stored and then filtered out. """ - server_resources = list( + raw_server_resources = list( filter( None, [ @@ -34,15 +34,14 @@ def get_server_resources( resource_type=resource_type, resource_key=key, headers=headers, - raw=True, ) for key in existing_keys ], ) ) - server_resources = [ + server_resources: List[FidesModel] = [ parse_dict(resource_type=resource_type, resource=resource, from_server=True) - for resource in server_resources + for resource in raw_server_resources ] return server_resources @@ -53,8 +52,7 @@ def get_server_resource( resource_type: str, resource_key: str, headers: Dict[str, str], - raw: bool = False, -) -> Union[FidesModel, Dict]: +) -> Dict: """ Attempt to get a given resource from the server. @@ -79,13 +77,6 @@ def get_server_resource( and raw_server_response.status_code <= 299 else None ) - if not raw and raw_server_resource: - parsed_server_resource: FidesModel = parse_dict( - resource_type=resource_type, - resource=raw_server_response.json(), - from_server=True, - ) - return parsed_server_resource return raw_server_resource or {} @@ -95,8 +86,7 @@ def list_server_resources( headers: Dict[str, str], resource_type: str, exclude_keys: List[str], - raw: bool = False, -) -> Optional[Union[List[FidesModel], List[Dict]]]: +) -> Optional[List[Dict]]: """ Get a list of resources from the server and return them as parsed objects. @@ -115,15 +105,4 @@ def list_server_resources( else [] ) - if not raw and raw_server_resources: - parsed_server_resources: List[FidesModel] = [ - parse_dict( - resource_type=resource_type, - resource=resource_dict, - from_server=True, - ) - for resource_dict in raw_server_resources - ] - return parsed_server_resources - return raw_server_resources diff --git a/src/fides/core/parse.py b/src/fides/core/parse.py index 6e36c1a3e2..d6df561613 100644 --- a/src/fides/core/parse.py +++ b/src/fides/core/parse.py @@ -1,6 +1,6 @@ """This module is responsible for parsing and verifying file, either with or without a server being available.""" -from fideslang.models import Taxonomy from fideslang.manifests import ingest_manifests +from fideslang.models import Taxonomy from fideslang.parse import load_manifests_into_taxonomy from fides.common.utils import echo_green diff --git a/src/fides/core/pull.py b/src/fides/core/pull.py index 506bfaa5d3..fdfab156fb 100644 --- a/src/fides/core/pull.py +++ b/src/fides/core/pull.py @@ -47,7 +47,7 @@ def pull_existing_resources( existing_keys.append(fides_key) server_resource = get_server_resource( - url, resource_type, fides_key, headers, raw=True + url, resource_type, fides_key, headers ) if server_resource: @@ -83,7 +83,6 @@ def pull_missing_resources( headers=headers, resource_type=resource, exclude_keys=existing_keys, - raw=True, ) for resource in MODEL_LIST } diff --git a/src/fides/core/system.py b/src/fides/core/system.py index ce2469f783..bfa6c40551 100644 --- a/src/fides/core/system.py +++ b/src/fides/core/system.py @@ -4,7 +4,7 @@ from typing import Dict, List, Optional from fideslang import manifests -from fideslang.models import Organization, System, FidesModel +from fideslang.models import Organization, System from pydantic import AnyHttpUrl from fides.common.utils import echo_green, echo_red, handle_cli_response @@ -166,6 +166,7 @@ def generate_system_aws( url=url, headers=headers, ) + assert organization aws_systems = generate_aws_systems(organization, aws_config=aws_config) write_system_manifest( @@ -212,6 +213,7 @@ def generate_system_okta( url=url, headers=headers, ) + assert organization okta_systems = asyncio.run( generate_okta_systems(organization=organization, okta_config=okta_config) @@ -225,7 +227,7 @@ def generate_system_okta( def get_all_server_systems( url: AnyHttpUrl, headers: Dict[str, str], exclude_systems: List[System] -) -> List[FidesModel]: +) -> List[System]: """ Get a list of all of the Systems that exist on the server. Excludes any systems provided in exclude_systems @@ -239,9 +241,14 @@ def get_all_server_systems( for resource in ls_response.json() if resource["fides_key"] not in exclude_system_keys ] - system_list = get_server_resources( - url=url, resource_type="system", headers=headers, existing_keys=system_keys - ) + + # The validation here is required to guarantee the return type + system_list = [ + System.validate(x) + for x in get_server_resources( + url=url, resource_type="system", headers=headers, existing_keys=system_keys + ) + ] return system_list diff --git a/tests/ops/schemas/connection_configuration/test_connection_config.py b/tests/ops/schemas/connection_configuration/test_connection_config.py index deef6eca24..2d88f9d4ca 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") From db8eb1c77b31ff68aef32b5b303b96e591815114 Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 31 Jul 2023 15:02:58 +0800 Subject: [PATCH 08/25] fix: static_checks --- src/fides/api/schemas/dataset.py | 2 +- src/fides/api/util/endpoint_utils.py | 42 +++++++++++++++------------- src/fides/core/annotate_dataset.py | 4 +-- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/fides/api/schemas/dataset.py b/src/fides/api/schemas/dataset.py index 63d86cede6..cb6b3d7962 100644 --- a/src/fides/api/schemas/dataset.py +++ b/src/fides/api/schemas/dataset.py @@ -45,7 +45,7 @@ class FieldDataCategoryValidation(DatasetField, DataCategoryValidationMixin): class CollectionDataCategoryValidation( DatasetCollection, DataCategoryValidationMixin ): - fields: Sequence[FieldDataCategoryValidation] = [] # type: ignore[assignment] + fields: Sequence[FieldDataCategoryValidation] = [] # type: ignore[assignment] class DatasetDataCategoryValidation(Dataset, DataCategoryValidationMixin): collections: Sequence[CollectionDataCategoryValidation] # type: ignore[assignment] diff --git a/src/fides/api/util/endpoint_utils.py b/src/fides/api/util/endpoint_utils.py index 75a6de8ba1..d660445225 100644 --- a/src/fides/api/util/endpoint_utils.py +++ b/src/fides/api/util/endpoint_utils.py @@ -12,13 +12,6 @@ from fides.api.db.base import Base # type: ignore from fides.api.db.crud import get_resource, list_resource -from fides.api.models.sql_models import ( # type: ignore[attr-defined] - models_with_default_field, - DataCategory as SQLDataCategory, - DataUse as SQLDataUse, - DataSubject as SQLDataSubject, - DataQualifier as SQLDataQualifier, -) from fides.api.util import errors from fides.common.api.scope_registry import ( CTL_DATASET, @@ -34,6 +27,24 @@ ) from fides.config import CONFIG +from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip + models_with_default_field, +) + +# Because of the aliasing & skips, isort puts these last +from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip + DataCategory as SQLDataCategory, +) +from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip + DataUse as SQLDataUse, +) +from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip + DataQualifier as SQLDataQualifier, +) +from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip + DataSubject as SQLDataSubject, +) + API_PREFIX = "/api/v1" # Map the ctl model type to the scope prefix. # Policies and datasets have ctl-* prefixes to @@ -74,22 +85,13 @@ async def forbid_if_editing_is_default( if sql_model in models_with_default_field: resource = await get_resource(sql_model, fides_key, async_session) - assert ( - isinstance(resource, SQLDataCategory) - or isinstance(resource, SQLDataQualifier) - or isinstance(resource, SQLDataSubject) - or isinstance(resource, SQLDataUse) + assert isinstance( + resource, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) ) - - assert ( - isinstance(payload, SQLDataCategory) - or isinstance(payload, SQLDataQualifier) - or isinstance(payload, SQLDataSubject) - or isinstance(payload, SQLDataUse) + assert isinstance( + payload, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) ) - assert hasattr(resource, "is_default") - if resource.is_default != payload.is_default: raise errors.ForbiddenError(sql_model.__name__, fides_key) diff --git a/src/fides/core/annotate_dataset.py b/src/fides/core/annotate_dataset.py index 4e373f9a73..faf6835c2f 100644 --- a/src/fides/core/annotate_dataset.py +++ b/src/fides/core/annotate_dataset.py @@ -5,9 +5,9 @@ import click from fideslang import manifests -from fideslang.parse import parse_dict from fideslang.manifests import ingest_manifests -from fideslang.models import Dataset, DatasetCollection, DatasetField, FidesModel +from fideslang.models import Dataset, DatasetCollection, DatasetField +from fideslang.parse import parse_dict from fideslang.validation import FidesKey, FidesValidationError from fides.common.utils import echo_green From 6e27666ba9f1e294377e9cbc47953f0f6f0fb48a Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 14:45:54 +0800 Subject: [PATCH 09/25] fix: get_server_resource tests --- tests/ctl/api/test_seed.py | 2 +- tests/ctl/core/test_api_helpers.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/ctl/api/test_seed.py b/tests/ctl/api/test_seed.py index d19eef41d6..43a267c808 100644 --- a/tests/ctl/api/test_seed.py +++ b/tests/ctl/api/test_seed.py @@ -432,7 +432,7 @@ async def test_load_default_dsr_policies( assert len(access_rule.targets) == num_rule_targets - 1 -async def test_load_orginizations(loguru_caplog, async_session, monkeypatch): +async def test_load_organizations(loguru_caplog, async_session, monkeypatch): updated_default_taxonomy = DEFAULT_TAXONOMY.copy() current_orgs = len(updated_default_taxonomy.organization) updated_default_taxonomy.organization.append( diff --git a/tests/ctl/core/test_api_helpers.py b/tests/ctl/core/test_api_helpers.py index 555e9ab0bf..0dc4e112e7 100644 --- a/tests/ctl/core/test_api_helpers.py +++ b/tests/ctl/core/test_api_helpers.py @@ -1,9 +1,10 @@ # pylint: disable=missing-docstring, redefined-outer-name import uuid -from typing import Dict, Generator, List, Optional +from typing import Dict, Generator, List import pytest -from fideslang import FidesModel, model_list +from fideslang import model_list +from fideslang.models import FidesModel from fides.config import FidesConfig from fides.core import api as _api @@ -87,13 +88,13 @@ def test_get_server_resource_found_resource( """ resource_type = created_resources[0] resource_key = created_resources[1][0] - result = _api_helpers.get_server_resource( + result: Dict = _api_helpers.get_server_resource( url=test_config.cli.server_url, resource_type=resource_type, resource_key=resource_key, headers=test_config.user.auth_header, ) - assert result.fides_key == resource_key + assert result.get("fides_key") == resource_key @pytest.mark.parametrize("resource_type", PARAM_MODEL_LIST) def test_get_server_resource_missing_resource( @@ -132,6 +133,7 @@ def test_get_server_resources_found_resources( existing_keys=resource_keys, headers=test_config.user.auth_header, ) + print(result) assert set(resource_keys) == set(resource.fides_key for resource in result) @pytest.mark.parametrize("resource_type", PARAM_MODEL_LIST) From fb22e0ead4cf96f1780cf8ad62e228ee8317a255 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 15:18:00 +0800 Subject: [PATCH 10/25] fix: TestLoadDefaultTaxonomy tests --- src/fides/api/util/endpoint_utils.py | 7 ++++--- tests/ctl/api/test_seed.py | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/fides/api/util/endpoint_utils.py b/src/fides/api/util/endpoint_utils.py index d660445225..f53463a18a 100644 --- a/src/fides/api/util/endpoint_utils.py +++ b/src/fides/api/util/endpoint_utils.py @@ -13,6 +13,7 @@ from fides.api.db.base import Base # type: ignore from fides.api.db.crud import get_resource, list_resource from fides.api.util import errors +from fideslang.models import DataCategory, DataQualifier, DataUse, DataSubject from fides.common.api.scope_registry import ( CTL_DATASET, CTL_POLICY, @@ -87,10 +88,10 @@ async def forbid_if_editing_is_default( assert isinstance( resource, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) - ) + ), f"Provided Resource is not the right type!" assert isinstance( - payload, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) - ) + payload, (DataCategory, DataQualifier, DataSubject, DataUse) + ), f"Provided Payload is not the right type!" if resource.is_default != payload.is_default: raise errors.ForbiddenError(sql_model.__name__, fides_key) diff --git a/tests/ctl/api/test_seed.py b/tests/ctl/api/test_seed.py index 43a267c808..c530b96e06 100644 --- a/tests/ctl/api/test_seed.py +++ b/tests/ctl/api/test_seed.py @@ -5,7 +5,8 @@ from unittest.mock import patch import pytest -from fideslang import DEFAULT_TAXONOMY, DataCategory, Organization +from fideslang.models import DataCategory, Organization +from fideslang.default_taxonomy import DEFAULT_TAXONOMY from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select From 69a3d8e4ef7cd7ce1a788ddb035e35d3be652eec Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 15:46:17 +0800 Subject: [PATCH 11/25] fix: lib tests --- .../api/oauth/system_manager_oauth_util.py | 22 ++++++++----------- tests/ctl/core/test_api.py | 14 ++++++++++++ tests/lib/test_system_oauth_util.py | 8 +++---- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/fides/api/oauth/system_manager_oauth_util.py b/src/fides/api/oauth/system_manager_oauth_util.py index d4119107bc..0cf70e21b1 100644 --- a/src/fides/api/oauth/system_manager_oauth_util.py +++ b/src/fides/api/oauth/system_manager_oauth_util.py @@ -78,7 +78,7 @@ async def verify_oauth_client_for_system_from_request_body( authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db), system_auth_data: SystemAuthContainer = Depends(_get_system_from_request_body), -) -> SystemSchema: +) -> None: """ Verifies that the access token provided in the authorization header contains the necessary scopes to be able to access the System found in the *request body* @@ -88,14 +88,12 @@ async def verify_oauth_client_for_system_from_request_body( Yields a 403 forbidden error if not. """ - permissions = has_system_permissions( + has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) - assert isinstance(permissions, SystemSchema) - return permissions async def verify_oauth_client_for_system_from_fides_key( @@ -103,7 +101,7 @@ async def verify_oauth_client_for_system_from_fides_key( authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db), system_auth_data: SystemAuthContainer = Depends(_get_system_from_fides_key), -) -> str: +) -> None: """ Verifies that the access token provided in the authorization header contains the necessary scopes to be able to access the System from the given *fides_key* in the path parameter. @@ -113,14 +111,12 @@ async def verify_oauth_client_for_system_from_fides_key( Yields a 403 forbidden error if not. """ - permissions = has_system_permissions( + has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) - assert isinstance(permissions, str) - return permissions def has_system_permissions( @@ -128,7 +124,7 @@ def has_system_permissions( authorization: str, security_scopes: SecurityScopes, db: Session, -) -> Union[str, SystemSchema]: +) -> None: """ Helper method that verifies that the token has the proper permissions to access the system(s). @@ -146,13 +142,13 @@ def has_system_permissions( token_data, client, security_scopes, system_auth_data.system ) - has_perms: bool = has_model_level_permissions or has_system_manager_permissions + has_correct_permissions: bool = ( + has_model_level_permissions or has_system_manager_permissions + ) - if not has_perms: + if not has_correct_permissions: raise AuthorizationError(detail="Not Authorized for this action") - return system_auth_data.original_data - def _has_scope_as_system_manager( token_data: Dict[str, Any], diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index ed174c6ff8..442bf24bea 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -1198,6 +1198,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + egress=None, + ingress=None, ) ] ), @@ -1212,6 +1214,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + egress=None, + ingress=None, ), models.PrivacyDeclaration( name="declaration-name-2", @@ -1221,6 +1225,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + egress=None, + ingress=None, ), ] ), @@ -1235,6 +1241,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + ingress=None, + egress=None, ), models.PrivacyDeclaration( name="Collect data for marketing", @@ -1244,6 +1252,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + ingress=None, + egress=None, ), ] ), @@ -1258,6 +1268,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + egress=None, + ingress=None, ), models.PrivacyDeclaration( name="declaration-name-2", @@ -1267,6 +1279,8 @@ def test_system_update_privacy_declaration_cookies( data_qualifier="aggregated_data", dataset_references=[], cookies=[], + egress=None, + ingress=None, ), ] ), diff --git a/tests/lib/test_system_oauth_util.py b/tests/lib/test_system_oauth_util.py index 7e6bd65f85..05d9164c01 100644 --- a/tests/lib/test_system_oauth_util.py +++ b/tests/lib/test_system_oauth_util.py @@ -47,7 +47,7 @@ async def test_owner_role_can_always_update_system(self, owner_user, db, system) CONFIG.security.app_encryption_key, ) # Note token doesn't have system on it, but the user is an owner - response = await verify_oauth_client_for_system_from_request_body( + await verify_oauth_client_for_system_from_request_body( security_scopes=SecurityScopes(scopes=[SYSTEM_UPDATE]), authorization=token, db=db, @@ -56,7 +56,7 @@ async def test_owner_role_can_always_update_system(self, owner_user, db, system) ), ) - assert response == system.fides_key + assert True async def test_viewer_role_alone_cannot_update_system( self, viewer_user, db, system @@ -93,7 +93,7 @@ async def test_viewer_is_also_system_manager(self, system_manager, db, system): CONFIG.security.app_encryption_key, ) - resp = await verify_oauth_client_for_system_from_request_body( + await verify_oauth_client_for_system_from_request_body( security_scopes=SecurityScopes(scopes=[SYSTEM_UPDATE]), authorization=token, db=db, @@ -102,7 +102,7 @@ async def test_viewer_is_also_system_manager(self, system_manager, db, system): ), ) - assert resp == system.fides_key + assert True async def test_system_manager_no_system_found(self, system_manager, db, system): payload = { From 3f814301967261e3db25a16df80cce031fcb8965 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 15:54:42 +0800 Subject: [PATCH 12/25] fix: privacy notice tests --- src/fides/api/models/privacy_notice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fides/api/models/privacy_notice.py b/src/fides/api/models/privacy_notice.py index c2137572ed..ac438d58c5 100644 --- a/src/fides/api/models/privacy_notice.py +++ b/src/fides/api/models/privacy_notice.py @@ -187,7 +187,7 @@ def generate_notice_key(cls, name: Optional[str]) -> FidesKey: if not isinstance(name, str): raise Exception("Privacy notice keys must be generated from a string.") notice_key: str = re.sub(r"\s+", "_", name.lower().strip()) - return FidesKey(notice_key) + return FidesKey(FidesKey.validate(notice_key)) def dry_update(self, *, data: dict[str, Any]) -> FidesBase: """ From 7a522690a8cd1c8d5dfa24328da7c871ed9129b9 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 16:24:34 +0800 Subject: [PATCH 13/25] fix: static checks --- src/fides/api/util/connection_util.py | 2 +- src/fides/api/util/endpoint_utils.py | 6 +++--- tests/ctl/api/test_seed.py | 2 +- tests/ctl/core/test_api.py | 6 +++++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/fides/api/util/connection_util.py b/src/fides/api/util/connection_util.py index 246adfd224..2aa0cbeacf 100644 --- a/src/fides/api/util/connection_util.py +++ b/src/fides/api/util/connection_util.py @@ -1,4 +1,4 @@ -from typing import List, Optional, Union +from typing import List, Optional from fastapi import Depends, HTTPException from fideslang.validation import FidesKey diff --git a/src/fides/api/util/endpoint_utils.py b/src/fides/api/util/endpoint_utils.py index f53463a18a..e3340f15c3 100644 --- a/src/fides/api/util/endpoint_utils.py +++ b/src/fides/api/util/endpoint_utils.py @@ -5,6 +5,7 @@ from fastapi import HTTPException from fideslang import FidesModelType +from fideslang.models import DataCategory, DataQualifier, DataSubject, DataUse from slowapi import Limiter from slowapi.util import get_remote_address # type: ignore from sqlalchemy.ext.asyncio import AsyncSession @@ -13,7 +14,6 @@ from fides.api.db.base import Base # type: ignore from fides.api.db.crud import get_resource, list_resource from fides.api.util import errors -from fideslang.models import DataCategory, DataQualifier, DataUse, DataSubject from fides.common.api.scope_registry import ( CTL_DATASET, CTL_POLICY, @@ -88,10 +88,10 @@ async def forbid_if_editing_is_default( assert isinstance( resource, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) - ), f"Provided Resource is not the right type!" + ), "Provided Resource is not the right type!" assert isinstance( payload, (DataCategory, DataQualifier, DataSubject, DataUse) - ), f"Provided Payload is not the right type!" + ), "Provided Payload is not the right type!" if resource.is_default != payload.is_default: raise errors.ForbiddenError(sql_model.__name__, fides_key) diff --git a/tests/ctl/api/test_seed.py b/tests/ctl/api/test_seed.py index c530b96e06..4d1d8dbdbc 100644 --- a/tests/ctl/api/test_seed.py +++ b/tests/ctl/api/test_seed.py @@ -5,8 +5,8 @@ from unittest.mock import patch import pytest -from fideslang.models import DataCategory, Organization from fideslang.default_taxonomy import DEFAULT_TAXONOMY +from fideslang.models import DataCategory, Organization from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index 442bf24bea..000a08f2aa 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -439,7 +439,7 @@ def test_api_delete( @pytest.mark.unit class TestSystemCreate: @pytest.fixture(scope="function", autouse=True) - def remove_all_systems(self, db) -> SystemSchema: + def remove_all_systems(self, db) -> None: """Remove any systems (and privacy declarations) before test execution for clean state""" for privacy_declaration in PrivacyDeclaration.all(db): privacy_declaration.delete(db) @@ -463,6 +463,8 @@ def system_create_request_body(self) -> SystemSchema: data_subjects=[], data_qualifier="aggregated_data", dataset_references=[], + egress=None, + ingress=None, cookies=[ { "name": "essential_cookie", @@ -477,6 +479,8 @@ def system_create_request_body(self) -> SystemSchema: data_use="marketing.advertising", data_subjects=[], data_qualifier="aggregated_data", + egress=None, + ingress=None, dataset_references=[], ), ], From b7f0b9c231c848b24c7e2eb126398044d551c95e Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 3 Aug 2023 16:59:04 +0800 Subject: [PATCH 14/25] fix: revert bad system_manager_oauth_util change --- .../api/oauth/system_manager_oauth_util.py | 21 +++++++++++-------- tests/ctl/core/test_api.py | 4 +++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/fides/api/oauth/system_manager_oauth_util.py b/src/fides/api/oauth/system_manager_oauth_util.py index 0cf70e21b1..8ff7fc2dbf 100644 --- a/src/fides/api/oauth/system_manager_oauth_util.py +++ b/src/fides/api/oauth/system_manager_oauth_util.py @@ -78,7 +78,7 @@ async def verify_oauth_client_for_system_from_request_body( authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db), system_auth_data: SystemAuthContainer = Depends(_get_system_from_request_body), -) -> None: +) -> Union[str, System]: """ Verifies that the access token provided in the authorization header contains the necessary scopes to be able to access the System found in the *request body* @@ -88,12 +88,14 @@ async def verify_oauth_client_for_system_from_request_body( Yields a 403 forbidden error if not. """ - has_system_permissions( + + system = has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) + return system async def verify_oauth_client_for_system_from_fides_key( @@ -101,7 +103,7 @@ async def verify_oauth_client_for_system_from_fides_key( authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db), system_auth_data: SystemAuthContainer = Depends(_get_system_from_fides_key), -) -> None: +) -> Union[str, System]: """ Verifies that the access token provided in the authorization header contains the necessary scopes to be able to access the System from the given *fides_key* in the path parameter. @@ -111,12 +113,13 @@ async def verify_oauth_client_for_system_from_fides_key( Yields a 403 forbidden error if not. """ - has_system_permissions( + system = has_system_permissions( system_auth_data=system_auth_data, authorization=authorization, security_scopes=security_scopes, db=db, ) + return system def has_system_permissions( @@ -124,7 +127,7 @@ def has_system_permissions( authorization: str, security_scopes: SecurityScopes, db: Session, -) -> None: +) -> Union[str, System]: """ Helper method that verifies that the token has the proper permissions to access the system(s). @@ -142,13 +145,13 @@ def has_system_permissions( token_data, client, security_scopes, system_auth_data.system ) - has_correct_permissions: bool = ( - has_model_level_permissions or has_system_manager_permissions - ) + has_perms: bool = has_model_level_permissions or has_system_manager_permissions - if not has_correct_permissions: + if not has_perms: raise AuthorizationError(detail="Not Authorized for this action") + return system_auth_data.original_data + def _has_scope_as_system_manager( token_data: Dict[str, Any], diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index 000a08f2aa..b75b6b68a3 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -743,7 +743,7 @@ class TestSystemUpdate: updated_system_name = "Updated System Name" @pytest.fixture(scope="function", autouse=True) - def remove_all_systems(self, db) -> SystemSchema: + def remove_all_systems(self, db) -> None: """Remove any systems (and privacy declarations) before test execution for clean state""" for privacy_declaration in PrivacyDeclaration.all(db): privacy_declaration.delete(db) @@ -767,6 +767,8 @@ def system_update_request_body(self, system) -> SystemSchema: data_subjects=[], data_qualifier="aggregated_data", dataset_references=[], + ingress=None, + egress=None, ) ], ) From 672163bd555cb2cab6e2d2ee201cd36ff8291863 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 4 Aug 2023 11:35:46 +0800 Subject: [PATCH 15/25] fix: add explicit model parsing to `audit` code --- src/fides/core/audit.py | 34 +++++++++++++++++++++++++++------- tests/ctl/cli/test_cli.py | 2 +- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/fides/core/audit.py b/src/fides/core/audit.py index 3d34c39636..68adf488a3 100644 --- a/src/fides/core/audit.py +++ b/src/fides/core/audit.py @@ -1,6 +1,7 @@ from typing import Dict, List, Optional, Union from fideslang.models import DataSubject, DataUse, FidesModel, Organization, System +from fideslang.parse import parse_dict from fides.common.utils import echo_green, echo_red, pretty_echo from fides.core.api_helpers import ( @@ -71,16 +72,24 @@ def validate_system_attributes( new_findings += 1 for privacy_declaration in system.privacy_declarations: - data_use = get_server_resource( + raw_data_use = get_server_resource( url, "data_use", privacy_declaration.data_use, headers ) + data_use = parse_dict( + resource_type="data_use", resource=raw_data_use, from_server=True + ) assert isinstance(data_use, DataUse) data_use_findings = audit_data_use_attributes(data_use, system.name or "") new_findings = new_findings + data_use_findings for data_subject_fides_key in privacy_declaration.data_subjects: - data_subject = get_server_resource( + raw_data_subject = get_server_resource( url, "data_subject", data_subject_fides_key, headers ) + data_subject = parse_dict( + resource_type="data_subject", + resource=raw_data_subject, + from_server=True, + ) assert isinstance(data_subject, DataSubject) data_subject_findings = audit_data_subject_attributes( data_subject, system.name or "" @@ -143,16 +152,29 @@ def audit_organizations( correctly populated """ - organization_resources: Optional[Union[List[FidesModel], List[dict]]] + organization_resources: Optional[List[FidesModel]] if include_keys: organization_resources = get_server_resources( url, "organization", include_keys, headers ) else: - organization_resources = list_server_resources( + raw_organization_resources = list_server_resources( url, headers, "organization", exclude_keys=[] ) + if raw_organization_resources: + organization_resources = [ + # Parse this into a FidesModel here so we know we're + # always using a FidesModel downstream + parse_dict( + resource=organization, + resource_type="organization", + from_server=True, + ) + for organization in raw_organization_resources + ] + else: + organization_resources = [] if not organization_resources: print("No organization resources were found.") @@ -162,9 +184,7 @@ def audit_organizations( audit_findings = 0 for organization in organization_resources: - print( - f"Auditing Organization: {organization.name if isinstance(organization, FidesModel) else organization['name']}" - ) + print(f"Auditing Organization: {organization.name or organization.fides_key}") assert isinstance(organization, Organization) new_findings = audit_organization_attributes(organization) audit_findings += new_findings diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 9c6f898398..10f80137eb 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -35,7 +35,7 @@ def test_cli_runner() -> Generator: @pytest.mark.integration def test_init(test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke( - cli, ["init"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + cli, ["init"], tenv={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} ) print(result.output) assert result.exit_code == 0 From d140ea2df2cd14355d517eaabe516b1ae7ce22f9 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 4 Aug 2023 13:01:47 +0800 Subject: [PATCH 16/25] fix: db scanning --- src/fides/core/dataset.py | 40 ++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/fides/core/dataset.py b/src/fides/core/dataset.py index fbbff5a8dd..83f3e2231c 100644 --- a/src/fides/core/dataset.py +++ b/src/fides/core/dataset.py @@ -10,6 +10,8 @@ from sqlalchemy.sql import text from fides.common.utils import echo_green, echo_red +from fideslang.parse import parse_dict +from fideslang.models import FidesModel from fides.connectors.aws import ( create_dynamodb_dataset, describe_dynamo_tables, @@ -33,20 +35,27 @@ def get_all_server_datasets( url: AnyHttpUrl, headers: Dict[str, str], exclude_datasets: List[Dataset] -) -> Optional[List[Dataset]]: +) -> List[Dataset]: """ Get a list of all of the Datasets that exist on the server. Excludes any datasets provided in exclude_datasets """ exclude_dataset_keys = [dataset.fides_key for dataset in exclude_datasets] - dataset_list = list_server_resources( - url=url, - resource_type="dataset", - exclude_keys=[str(x) for x in exclude_dataset_keys], - headers=headers, + raw_dataset_list = ( + list_server_resources( + url=url, + resource_type="dataset", + exclude_keys=[str(x) for x in exclude_dataset_keys], + headers=headers, + ) + or [] ) + dataset_list = [ + Dataset.parse_obj(dataset) + for dataset in raw_dataset_list + ] - return dataset_list # type: ignore[return-value] + return dataset_list def include_dataset_schema(schema: str, database_type: str) -> bool: @@ -257,10 +266,11 @@ def scan_dataset_db( and fields and compares them to existing datasets and prioritizes datasets in a local manifest (if one is provided). """ - manifest_taxonomy = parse(manifest_dir) if manifest_dir else None - manifest_datasets = [] - if manifest_taxonomy: - manifest_datasets = manifest_taxonomy.dataset or [] + + if manifest_dir: + manifest_datasets = parse(manifest_dir).dataset or [] + else: + manifest_datasets = [] if not local: server_datasets = ( @@ -272,9 +282,9 @@ def scan_dataset_db( else: server_datasets = [] - dataset_keys = [ - dataset.fides_key for dataset in manifest_datasets + server_datasets - ] + all_datasets = manifest_datasets + server_datasets + + dataset_keys = [dataset.fides_key for dataset in all_datasets] echo_green( "Loaded the following dataset manifests:\n\t{}".format( "\n\t".join(dataset_keys) @@ -284,7 +294,7 @@ def scan_dataset_db( # Generate the collections and fields for the target database db_datasets = generate_db_datasets(connection_string=connection_string) uncategorized_fields, db_field_count = find_all_uncategorized_dataset_fields( - existing_datasets=manifest_datasets + server_datasets, + existing_datasets=all_datasets, source_datasets=db_datasets, ) if db_field_count < 1: From b790acb11e4c8aa50f4334ab7118bd04905a3e7d Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 4 Aug 2023 13:05:02 +0800 Subject: [PATCH 17/25] fix: cli init test --- tests/ctl/cli/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 10f80137eb..9c6f898398 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -35,7 +35,7 @@ def test_cli_runner() -> Generator: @pytest.mark.integration def test_init(test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke( - cli, ["init"], tenv={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + cli, ["init"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} ) print(result.output) assert result.exit_code == 0 From 030efe1da4f586047014c936f5430b471b79227a Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 6 Aug 2023 19:10:17 +0800 Subject: [PATCH 18/25] feat: refactor a test --- tests/ctl/core/test_api.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index b75b6b68a3..6771693419 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -1324,15 +1324,15 @@ def test_system_update_updates_declarations( # assert the declarations in our responses match those in our requests response_decs: List[dict] = result.json()["privacy_declarations"] - # remove the IDs from the response decs for easier matching with request payload - # we also are implicitly affirming the response declarations DO have an - # 'id' field, as we expect + for response_dec in response_decs: - response_dec.pop("id") - for update_dec in update_declarations: - response_decs.remove(update_dec.dict()) - # and assert we don't have any extra response declarations - assert len(response_decs) == 0 + assert "id" in response_dec.keys() + parsed_response_declaration = models.PrivacyDeclaration.parse_obj( + response_dec + ) + assert parsed_response_declaration in update_declarations + + assert len(response_decs) == len(update_declarations) # do the same for the declarations in our db record system = System.all(db)[0] From 874500f83a97645ea71a50cbeca14b32963a47a1 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 6 Aug 2023 19:45:05 +0800 Subject: [PATCH 19/25] feat: add helpful assertion messages --- tests/ctl/core/test_api.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index 6771693419..6868a4cbff 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -1325,14 +1325,22 @@ def test_system_update_updates_declarations( # assert the declarations in our responses match those in our requests response_decs: List[dict] = result.json()["privacy_declarations"] + assert len(response_decs) == len( + update_declarations + ), "Response declaration count doesn't match the number sent!" for response_dec in response_decs: - assert "id" in response_dec.keys() + assert ( + "id" in response_dec.keys() + ), "No 'id' field in the response declaration!" + parsed_response_declaration = models.PrivacyDeclaration.parse_obj( response_dec ) - assert parsed_response_declaration in update_declarations - - assert len(response_decs) == len(update_declarations) + assert ( + parsed_response_declaration in update_declarations + ), "The response declaration '{}' doesn't match anything in the request declarations!".format( + parsed_response_declaration.name + ) # do the same for the declarations in our db record system = System.all(db)[0] From c69c72a090ba437ceba98970b1c458c27d222999 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 9 Aug 2023 00:24:41 +0800 Subject: [PATCH 20/25] fix: type signature --- src/fides/api/db/system.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/fides/api/db/system.py b/src/fides/api/db/system.py index cf1b757e00..85613110b7 100644 --- a/src/fides/api/db/system.py +++ b/src/fides/api/db/system.py @@ -137,7 +137,7 @@ async def upsert_privacy_declarations( async def upsert_cookies( async_session: AsyncSession, - cookies: Optional[List[Dict]], # CookieSchema + cookies: List[Dict], # CookieSchema privacy_declaration: PrivacyDeclaration, system: System, ) -> None: @@ -148,9 +148,6 @@ async def upsert_cookies( Remove any existing cookies that aren't specified here. """ - if not cookies: - return - for cookie_data in cookies: # Check if cookie exists for this name/system/privacy declaration result = await async_session.execute( From a72383f2e91b51908c224e71fb8936c2219d90b7 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 9 Aug 2023 00:35:05 +0800 Subject: [PATCH 21/25] fix: pylint --- src/fides/core/dataset.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/fides/core/dataset.py b/src/fides/core/dataset.py index 83f3e2231c..7230cd6126 100644 --- a/src/fides/core/dataset.py +++ b/src/fides/core/dataset.py @@ -10,8 +10,6 @@ from sqlalchemy.sql import text from fides.common.utils import echo_green, echo_red -from fideslang.parse import parse_dict -from fideslang.models import FidesModel from fides.connectors.aws import ( create_dynamodb_dataset, describe_dynamo_tables, @@ -50,10 +48,7 @@ def get_all_server_datasets( ) or [] ) - dataset_list = [ - Dataset.parse_obj(dataset) - for dataset in raw_dataset_list - ] + dataset_list = [Dataset.parse_obj(dataset) for dataset in raw_dataset_list] return dataset_list From 3b5b3e2c6f0fecd5346636e600405930ea32b078 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 9 Aug 2023 15:01:01 +0800 Subject: [PATCH 22/25] fix: explicitly convert to Cookie models --- src/fides/api/db/system.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/fides/api/db/system.py b/src/fides/api/db/system.py index 85613110b7..fcbf077f20 100644 --- a/src/fides/api/db/system.py +++ b/src/fides/api/db/system.py @@ -4,6 +4,7 @@ from typing import Dict, List, Optional, Tuple from fastapi import HTTPException +from fideslang.models import Cookies as CookieSchema from fideslang.models import System as SystemSchema from loguru import logger as log from sqlalchemy import and_, delete, insert, select, update @@ -137,7 +138,7 @@ async def upsert_privacy_declarations( async def upsert_cookies( async_session: AsyncSession, - cookies: List[Dict], # CookieSchema + cookies: Optional[List[Dict]], privacy_declaration: PrivacyDeclaration, system: System, ) -> None: @@ -148,12 +149,16 @@ async def upsert_cookies( Remove any existing cookies that aren't specified here. """ - for cookie_data in cookies: + parsed_cookies = ( + [CookieSchema.parse_obj(cookie) for cookie in cookies] if cookies else [] + ) + + for cookie_data in parsed_cookies: # Check if cookie exists for this name/system/privacy declaration result = await async_session.execute( select(Cookies).where( and_( - Cookies.name == cookie_data["name"], + Cookies.name == cookie_data.name, Cookies.system_id == system.id, Cookies.privacy_declaration_id == privacy_declaration.id, ) @@ -162,16 +167,16 @@ async def upsert_cookies( row: Optional[Cookies] = result.scalars().first() if row: await async_session.execute( - update(Cookies).where(Cookies.id == row.id).values(cookie_data) + update(Cookies).where(Cookies.id == row.id).values(cookie_data.dict()) ) else: await async_session.execute( insert(Cookies).values( { - "name": cookie_data.get("name"), - "path": cookie_data.get("path"), - "domain": cookie_data.get("domain"), + "name": cookie_data.name, + "path": cookie_data.path, + "domain": cookie_data.domain, "privacy_declaration_id": privacy_declaration.id, "system_id": system.id, } @@ -182,7 +187,7 @@ async def upsert_cookies( delete_result = await async_session.execute( select(Cookies).where( and_( - Cookies.name.notin_([cookie["name"] for cookie in cookies]), + Cookies.name.notin_([cookie.name for cookie in parsed_cookies]), Cookies.system_id == system.id, Cookies.privacy_declaration_id == privacy_declaration.id, ) From 4255114aa14dbdf4fa527d24cd286429f5c8a53f Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 17 Sep 2023 07:12:05 +0800 Subject: [PATCH 23/25] fix: mypy --- .github/workflows/backend_checks.yml | 1 + noxfiles/docker_nox.py | 1 + .../versions/f17f92237383_disable_inactive_notices.py | 1 - .../api/v1/endpoints/privacy_experience_endpoints.py | 8 ++++---- src/fides/core/evaluate.py | 10 ++++++++-- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index 642e172a18..3865f4d265 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -79,6 +79,7 @@ jobs: [ "isort", "black", + "mypy", "pylint", "xenon", "check_install", diff --git a/noxfiles/docker_nox.py b/noxfiles/docker_nox.py index a88f932b93..1d50d2b8c0 100644 --- a/noxfiles/docker_nox.py +++ b/noxfiles/docker_nox.py @@ -2,6 +2,7 @@ from typing import Callable, Dict, List, Optional, Tuple import nox + from constants_nox import ( DEV_TAG_SUFFIX, IMAGE, diff --git a/src/fides/api/alembic/migrations/versions/f17f92237383_disable_inactive_notices.py b/src/fides/api/alembic/migrations/versions/f17f92237383_disable_inactive_notices.py index 07cd4dd120..a6758b8a70 100644 --- a/src/fides/api/alembic/migrations/versions/f17f92237383_disable_inactive_notices.py +++ b/src/fides/api/alembic/migrations/versions/f17f92237383_disable_inactive_notices.py @@ -13,7 +13,6 @@ from sqlalchemy.engine import Connection, ResultProxy from sqlalchemy.sql.elements import TextClause - # revision identifiers, used by Alembic. from fides.api.models.sql_models import DataUse diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 354599de89..7ef5f367ad 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -67,20 +67,20 @@ def _filter_experiences_by_region_or_country( if not region: return experience_query - region: str = escape(region).replace("-", "_").lower() - country: str = region.split("_")[0] + formatted_region: str = escape(region).replace("-", "_").lower() + country: str = formatted_region.split("_")[0] overlay: Optional[ PrivacyExperience ] = PrivacyExperience.get_experience_by_region_and_component( - db, region, ComponentType.overlay + db, formatted_region, ComponentType.overlay ) or PrivacyExperience.get_experience_by_region_and_component( db, country, ComponentType.overlay ) privacy_center: Optional[ PrivacyExperience ] = PrivacyExperience.get_experience_by_region_and_component( - db, region, ComponentType.privacy_center + db, formatted_region, ComponentType.privacy_center ) or PrivacyExperience.get_experience_by_region_and_component( db, country, ComponentType.privacy_center ) diff --git a/src/fides/core/evaluate.py b/src/fides/core/evaluate.py index 4ce3c9326a..163829e10b 100644 --- a/src/fides/core/evaluate.py +++ b/src/fides/core/evaluate.py @@ -290,12 +290,13 @@ def evaluate_dataset_reference( dataset.fides_key, ) + data_qualifier = str(dataset.data_qualifier) if dataset.data_qualifier else "" dataset_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, data_subjects=[str(x) for x in privacy_declaration.data_subjects], data_categories=[str(x) for x in dataset.data_categories], - data_qualifier=dataset.data_qualifier, + data_qualifier=data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=dataset_violation_message, ) @@ -371,12 +372,17 @@ def evaluate_privacy_declaration( ) ) + data_qualifier = ( + str(privacy_declaration.data_qualifier) + if privacy_declaration.data_qualifier + else "" + ) declaration_result_violations = evaluate_policy_rule( taxonomy=taxonomy, policy_rule=policy_rule, data_subjects=[str(x) for x in privacy_declaration.data_subjects], data_categories=[str(x) for x in privacy_declaration.data_categories], - data_qualifier=privacy_declaration.data_qualifier, + data_qualifier=data_qualifier, data_use=privacy_declaration.data_use, declaration_violation_message=declaration_violation_message, ) From 0498dd2cb5e5b3da6aac280a88ea75db1e3d2dd5 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 17 Sep 2023 12:04:25 +0800 Subject: [PATCH 24/25] fix: system scanning --- src/fides/core/system.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fides/core/system.py b/src/fides/core/system.py index bfa6c40551..14324cfbc5 100644 --- a/src/fides/core/system.py +++ b/src/fides/core/system.py @@ -101,8 +101,9 @@ def get_organization( ) raise SystemExit(1) - assert isinstance(server_organization, Organization) - return server_organization + parsed_organization = Organization.parse_obj(server_organization) + assert isinstance(parsed_organization, Organization) + return parsed_organization def generate_aws_systems( From b1e20ae73e0a957c7925197cf0fdc9f4a221f7e1 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 22 Sep 2023 09:36:48 +0800 Subject: [PATCH 25/25] feat: address review comments and add a new Protocol --- noxfiles/git_nox.py | 3 +-- .../api/api/v1/endpoints/router_factory.py | 4 +-- src/fides/api/models/sql_models.py | 10 +++---- .../api/oauth/system_manager_oauth_util.py | 3 ++- src/fides/api/util/endpoint_utils.py | 27 +++++-------------- tests/lib/test_system_oauth_util.py | 4 +-- 6 files changed, 18 insertions(+), 33 deletions(-) diff --git a/noxfiles/git_nox.py b/noxfiles/git_nox.py index d3ed2feb19..5cec609db8 100644 --- a/noxfiles/git_nox.py +++ b/noxfiles/git_nox.py @@ -4,9 +4,8 @@ from enum import Enum from typing import List, Optional -from packaging.version import Version - import nox +from packaging.version import Version RELEASE_BRANCH_REGEX = r"release-(([0-9]+\.)+[0-9]+)" RELEASE_TAG_REGEX = r"(([0-9]+\.)+[0-9]+)" diff --git a/src/fides/api/api/v1/endpoints/router_factory.py b/src/fides/api/api/v1/endpoints/router_factory.py index cf0f61ef05..4cdfeaaeee 100644 --- a/src/fides/api/api/v1/endpoints/router_factory.py +++ b/src/fides/api/api/v1/endpoints/router_factory.py @@ -26,7 +26,7 @@ from fides.api.db.ctl_session import get_async_db from fides.api.models.sql_models import ( DataCategory, - models_with_default_field, + ModelWithDefaultField, sql_model_map, ) from fides.api.oauth.utils import verify_oauth_client_prod @@ -144,7 +144,7 @@ async def create( sql_model = sql_model_map[model_type] if isinstance(resource, Dataset): await validate_data_categories(resource, db) - if sql_model in models_with_default_field and resource.is_default: + if isinstance(sql_model, ModelWithDefaultField) and resource.is_default: raise errors.ForbiddenError(model_type, resource.fides_key) return await create_resource(sql_model, resource.dict(), db) diff --git a/src/fides/api/models/sql_models.py b/src/fides/api/models/sql_models.py index 6f5ce615f6..8e079ca38d 100644 --- a/src/fides/api/models/sql_models.py +++ b/src/fides/api/models/sql_models.py @@ -30,6 +30,7 @@ from sqlalchemy.orm import Session, relationship from sqlalchemy.sql import func from sqlalchemy.sql.sqltypes import DateTime +from typing_extensions import Protocol, runtime_checkable from fides.api.common_exceptions import KeyOrNameAlreadyExists from fides.api.db.base_class import Base @@ -551,11 +552,10 @@ class SystemScans(Base): "evaluation": Evaluation, } -models_with_default_field = [ - sql_model - for sql_model in sql_model_map.values() - if hasattr(sql_model, "is_default") -] + +@runtime_checkable +class ModelWithDefaultField(Protocol): + is_default: bool class AllowedTypes(str, EnumType): diff --git a/src/fides/api/oauth/system_manager_oauth_util.py b/src/fides/api/oauth/system_manager_oauth_util.py index 8ff7fc2dbf..b1f0817736 100644 --- a/src/fides/api/oauth/system_manager_oauth_util.py +++ b/src/fides/api/oauth/system_manager_oauth_util.py @@ -103,7 +103,7 @@ async def verify_oauth_client_for_system_from_fides_key( authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db), system_auth_data: SystemAuthContainer = Depends(_get_system_from_fides_key), -) -> Union[str, System]: +) -> str: """ Verifies that the access token provided in the authorization header contains the necessary scopes to be able to access the System from the given *fides_key* in the path parameter. @@ -119,6 +119,7 @@ async def verify_oauth_client_for_system_from_fides_key( security_scopes=security_scopes, db=db, ) + assert isinstance(system, str) return system diff --git a/src/fides/api/util/endpoint_utils.py b/src/fides/api/util/endpoint_utils.py index e3340f15c3..fc6a4d8405 100644 --- a/src/fides/api/util/endpoint_utils.py +++ b/src/fides/api/util/endpoint_utils.py @@ -5,7 +5,6 @@ from fastapi import HTTPException from fideslang import FidesModelType -from fideslang.models import DataCategory, DataQualifier, DataSubject, DataUse from slowapi import Limiter from slowapi.util import get_remote_address # type: ignore from sqlalchemy.ext.asyncio import AsyncSession @@ -29,21 +28,7 @@ from fides.config import CONFIG from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip - models_with_default_field, -) - -# Because of the aliasing & skips, isort puts these last -from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip - DataCategory as SQLDataCategory, -) -from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip - DataUse as SQLDataUse, -) -from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip - DataQualifier as SQLDataQualifier, -) -from fides.api.models.sql_models import ( # type: ignore[attr-defined] # isort: skip - DataSubject as SQLDataSubject, + ModelWithDefaultField, ) API_PREFIX = "/api/v1" @@ -83,14 +68,14 @@ async def forbid_if_editing_is_default( """ Raise a forbidden error if the user is trying modify the `is_default` field """ - if sql_model in models_with_default_field: + if isinstance(sql_model, ModelWithDefaultField): resource = await get_resource(sql_model, fides_key, async_session) assert isinstance( - resource, (SQLDataCategory, SQLDataQualifier, SQLDataSubject, SQLDataUse) + resource, ModelWithDefaultField ), "Provided Resource is not the right type!" assert isinstance( - payload, (DataCategory, DataQualifier, DataSubject, DataUse) + payload, ModelWithDefaultField ), "Provided Payload is not the right type!" if resource.is_default != payload.is_default: @@ -104,7 +89,7 @@ async def forbid_if_default( Raise a forbidden error if the user is trying to operate on a resource with `is_default=True` """ - if sql_model in models_with_default_field: + if isinstance(sql_model, ModelWithDefaultField): resource = await get_resource(sql_model, fides_key, async_session) if resource.is_default: raise errors.ForbiddenError(sql_model.__name__, fides_key) @@ -117,7 +102,7 @@ async def forbid_if_editing_any_is_default( Raise a forbidden error if any of the existing resources' `is_default` field is being modified, or if there is a new resource with `is_default=True` """ - if sql_model in models_with_default_field: + if isinstance(sql_model, ModelWithDefaultField): fides_keys = [resource["fides_key"] for resource in resources] existing_resources = { r.fides_key: r diff --git a/tests/lib/test_system_oauth_util.py b/tests/lib/test_system_oauth_util.py index 05d9164c01..475eed5315 100644 --- a/tests/lib/test_system_oauth_util.py +++ b/tests/lib/test_system_oauth_util.py @@ -47,7 +47,7 @@ async def test_owner_role_can_always_update_system(self, owner_user, db, system) CONFIG.security.app_encryption_key, ) # Note token doesn't have system on it, but the user is an owner - await verify_oauth_client_for_system_from_request_body( + response = await verify_oauth_client_for_system_from_request_body( security_scopes=SecurityScopes(scopes=[SYSTEM_UPDATE]), authorization=token, db=db, @@ -56,7 +56,7 @@ async def test_owner_role_can_always_update_system(self, owner_user, db, system) ), ) - assert True + assert response == system.fides_key async def test_viewer_role_alone_cannot_update_system( self, viewer_user, db, system