From 606c8d9924d6957368957b15889fc1f7829c6cd2 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 28 Feb 2023 18:09:31 -0800 Subject: [PATCH 01/26] Auto-Generate config documentation (#2694) Co-authored-by: Steve Murphy --- .github/workflows/backend_checks.yml | 1 + .gitignore | 3 +- CHANGELOG.md | 1 + docs/fides/docs/{cli.md => cli/index.md} | 2 +- docs/fides/docs/config/index.md | 103 +++++++ docs/fides/mkdocs.yml | 6 +- noxfiles/ci_nox.py | 7 + scripts/generate_docs.py | 15 +- src/fides/cli/commands/util.py | 20 +- src/fides/cli/utils.py | 99 +++---- src/fides/core/config/__init__.py | 8 +- src/fides/core/config/admin_ui_settings.py | 2 +- src/fides/core/config/cli_settings.py | 4 +- src/fides/core/config/create.py | 253 ++++++++++++++++++ src/fides/core/config/database_settings.py | 2 +- src/fides/core/config/execution_settings.py | 2 +- src/fides/core/config/helpers.py | 37 --- src/fides/core/config/logging_settings.py | 2 +- .../core/config/notification_settings.py | 2 +- src/fides/core/config/redis_settings.py | 9 +- src/fides/core/config/security_settings.py | 16 +- src/fides/core/config/user_settings.py | 9 +- src/fides/core/config/utils.py | 23 ++ tests/ctl/cli/test_cli.py | 10 + .../cli/{test_cli_utils.py => test_utils.py} | 6 +- tests/ctl/core/{ => config}/test_config.py | 0 .../core/{ => config}/test_config_helpers.py | 40 --- tests/ctl/core/config/test_create.py | 107 ++++++++ .../test_utils.py} | 22 ++ 29 files changed, 633 insertions(+), 178 deletions(-) rename docs/fides/docs/{cli.md => cli/index.md} (54%) create mode 100644 docs/fides/docs/config/index.md create mode 100644 src/fides/core/config/create.py rename tests/ctl/cli/{test_cli_utils.py => test_utils.py} (99%) rename tests/ctl/core/{ => config}/test_config.py (100%) rename tests/ctl/core/{ => config}/test_config_helpers.py (56%) create mode 100644 tests/ctl/core/config/test_create.py rename tests/ctl/core/{test_config_utils.py => config/test_utils.py} (70%) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index ac670bb39e..41913d151e 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -103,6 +103,7 @@ jobs: test_selection: - "check_fides_annotations" - "fides_db_scan" + - "generate_docs" - "docs_check" - "minimal_config_startup" diff --git a/.gitignore b/.gitignore index 23a8418bf6..19f4aa47c2 100644 --- a/.gitignore +++ b/.gitignore @@ -25,8 +25,7 @@ tmp/* # docs docs/fides/site/ docs/fides/docs/api/openapi.json -docs/fidesops/docs/api/openapi.json -docs/fidesops/site +docs/fides/docs/config/fides.toml # python specific *.pyc diff --git a/CHANGELOG.md b/CHANGELOG.md index 27e2b676d6..5d3916ed0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ The types of changes are: * Delete flow for privacy declarations [#2664](https://github.com/ethyca/fides/pull/2664) * Convert all config values to Pydantic `Field` objects [#2613](https://github.com/ethyca/fides/pull/2613) * Add warning to 'fides deploy' when installed outside of a virtual environment [#2641](https://github.com/ethyca/fides/pull/2641) +* Redesigned the default/init config file to be auto-documented. Also updates the `fides init` logic and analytics consent logic [#2694](https://github.com/ethyca/fides/pull/2694) * Change how config creation/import is handled across the application [#2622](https://github.com/ethyca/fides/pull/2622) ### Developer Experience diff --git a/docs/fides/docs/cli.md b/docs/fides/docs/cli/index.md similarity index 54% rename from docs/fides/docs/cli.md rename to docs/fides/docs/cli/index.md index 9b4ed732d7..e09ef8c875 100644 --- a/docs/fides/docs/cli.md +++ b/docs/fides/docs/cli/index.md @@ -1,6 +1,6 @@ # CLI -These docs reflect the latest PyPI release. +These are autogenerated CLI docs that reflect the latest PyPI release. --- diff --git a/docs/fides/docs/config/index.md b/docs/fides/docs/config/index.md new file mode 100644 index 0000000000..f96bc771c3 --- /dev/null +++ b/docs/fides/docs/config/index.md @@ -0,0 +1,103 @@ +# Configuration + +--- + +## Setting Configuration Values + +Fides can be configured in two different ways: either via a `toml` file or via environment variables. +Both methods can be used simultaneously, with environment variables taking precedence over the `toml` file values. + +### Using a Configuration File + +Fides will use the first config file it can read from the following locations. Listed in order of precedence they are: + +1. At the path specified using the config file argument passed through the CLI, i.e. `fides -f ` +1. At the path specified by the `FIDES__CONFIG_PATH` environment variable +1. In the current working directory it will check for a subdir `.fides` and a file within named `fides.toml`, i.e. `./.fides/fides.toml` + +### Generating a Config File + +If you'd like to generate a new config file automatically using default values, run `fides init`. +This will create the file at the default location of `./.fides/fides.toml` + +### Setting Values Via Environment Variables + +Fides follows a set pattern for configuration via environment variables. +It looks for environment variables that start with `FIDES` followed by the config subsection name, followed by the key name, all separated with double underscores. +In practice this would look like `FIDES____` + +As a `toml` configuration value: + +```toml +[database] +host = config_example +``` + +As an environment variable: + +```sh +EXPORT FIDES__DATABASE__HOST=config_example +``` + +--- + +## Viewing your configuration + +You can view the current configuration of your application via either the CLI or API. + +### CLI + +To view your application configuration via the CLI, run: + +```sh +fides view config +``` + +This will show _all_ configuration variables, including sensitive ones. +It is printed to the console as valid `toml`, so this can be copy/pasted as needed. + +### API + +To view your application configuration in the API, run: + +```sh +GET /api/v1/config +``` + +For security reasons, sensitive configuration values will not be shown here. + +## Special Sections + +There are a few "special" configuration sections that behave in unique ways compared to the other sections. These sections will be addressed in the following documentation. + +### Celery + +Fides uses [Celery](https://docs.celeryq.dev/en/stable/index.html) for asynchronous task management. + +To simplify deployments and remove the need for two different toml configuration files, it is possible to configure Celery via the Fides configuration file. Any valid configuration key/value pair for Celery can instead be added to the Fides toml configuration file and will automatically be passed through to the Celery deployment. Note that Fides will not validate any of these key/value pairs. See the above configuration file reference for an example of using celery configuration pass-through. + +For a full list of possible variable overrides, see the [Celery configuration documentation](https://docs.celeryq.dev/en/stable/userguide/configuration.html). + +```toml title="Example Celery Section" +[celery] +event_queue_prefix = "fides_worker" +task_default_queue = "fides" +task_always_eager = true +``` + +### Credentials + +The credentials section uses custom keys which can be referenced in specific commands that take the --credentials-id option. For example, a command that uses a credential might look like `fides scan dataset db --credentials-id app_postgres`. The credential object itself will be validated at the time of use depending on what type of credential is required. For instance if `fides scan system okta` is used, it will expect the object to contain orgUrl and token key/value pairs. In the case of a typical database like postgres, it will only expect a connection_string. The following is an example of what a credentials section might look like in a given deployment with various applications: + +```toml title="Example Credentials Section" +[credentials] +app_postgres = {connection_string="postgresql+psycopg2://postgres:fides@fides-db:5432/fides"} +``` + +## Configuration File Reference + +This following file is an autogenerated configuration reference file. It shows application defaults and is a valid `toml` file that can be used for configuration of Fides. + +```toml title="fides.toml" +-8<- "config/fides.toml" +``` diff --git a/docs/fides/mkdocs.yml b/docs/fides/mkdocs.yml index 94ab71246f..44e5f7e4fd 100644 --- a/docs/fides/mkdocs.yml +++ b/docs/fides/mkdocs.yml @@ -11,7 +11,8 @@ nav: - Fides: - What is Fides?: index.md - API: api/index.md - - CLI: cli.md + - CLI: cli/index.md + - Configuration: config/index.md - Community: - Github, Slack, and Discord: community/overview.md - Community Hints and Tips: community/hints_tips.md @@ -66,7 +67,8 @@ theme: markdown_extensions: - attr_list - pymdownx.superfences - - pymdownx.snippets + - pymdownx.snippets: + base_path: 'docs/' - pymdownx.inlinehilite - pymdownx.tabbed - admonition diff --git a/noxfiles/ci_nox.py b/noxfiles/ci_nox.py index 5fbfc4964e..4675e6ae3f 100644 --- a/noxfiles/ci_nox.py +++ b/noxfiles/ci_nox.py @@ -98,6 +98,13 @@ def xenon(session: nox.Session) -> None: session.run(*command) +@nox.session() +def generate_docs(session: nox.Session) -> None: + """Check that the autogenerated docs build succeeds.""" + session.install(".") + session.run("python", "scripts/generate_docs.py") + + ################## ## Fides Checks ## ################## diff --git a/scripts/generate_docs.py b/scripts/generate_docs.py index 07e39a17ec..aa67d30cce 100644 --- a/scripts/generate_docs.py +++ b/scripts/generate_docs.py @@ -5,9 +5,11 @@ import sys from fides.api.main import app +from fides.core.config import CONFIG +from fides.core.config.create import generate_config_docs -def generate_openapi(outfile_dir: str) -> None: +def generate_openapi(outfile_dir: str) -> str: "Write out an openapi.json file for the API." outfile_name = "api/openapi.json" @@ -16,8 +18,17 @@ def generate_openapi(outfile_dir: str) -> None: with open(outfile_path, "w") as outfile: json.dump(app.openapi(), outfile, indent=2) print(f"Exported OpenAPI JSON from the API to '{outfile_path}'") + return outfile_path if __name__ == "__main__": - outfile_dir = sys.argv[1] + default_outfile_dir = "docs/fides/docs" + try: + outfile_dir = sys.argv[1] + except IndexError: + outfile_dir = default_outfile_dir + generate_openapi(outfile_dir) + generate_config_docs( + config=CONFIG, outfile_path="docs/fides/docs/config/fides.toml" + ) diff --git a/src/fides/cli/commands/util.py b/src/fides/cli/commands/util.py index 12d537dd75..2c1acc2475 100644 --- a/src/fides/cli/commands/util.py +++ b/src/fides/cli/commands/util.py @@ -7,13 +7,12 @@ import fides from fides.cli.utils import ( FIDES_ASCII_ART, - check_and_update_analytics_config, check_server, print_divider, send_init_analytics, with_analytics, ) -from fides.core.config.helpers import create_config_file +from fides.core.config.create import create_and_update_config_file from fides.core.deploy import ( check_docker_version, check_fides_uploads_dir, @@ -30,7 +29,10 @@ @click.command() @click.pass_context @click.argument("fides_directory_location", default=".", type=click.Path(exists=True)) -def init(ctx: click.Context, fides_directory_location: str) -> None: +@click.option( + "--opt-in", is_flag=True, help="Automatically opt-in to anonymous usage analytics." +) +def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> None: """ Initializes a fides instance, creating the default directory (`.fides/`) and the configuration file (`fides.toml`) if necessary. @@ -44,14 +46,11 @@ def init(ctx: click.Context, fides_directory_location: str) -> None: click.echo(FIDES_ASCII_ART) click.echo("Initializing fides...") - # create the config file as needed - config_path = create_config_file( - config=config, fides_directory_location=fides_directory_location + config, config_path = create_and_update_config_file( + config, fides_directory_location, opt_in=opt_in ) - print_divider() - # request explicit consent for analytics collection - check_and_update_analytics_config(config=config, config_path=config_path) + print_divider() send_init_analytics(config.user.analytics_opt_out, config_path, executed_at) echo_green("fides initialization complete.") @@ -146,8 +145,7 @@ def up(ctx: click.Context, no_pull: bool = False, no_init: bool = False) -> None # Deployment is ready! Perform the same steps as `fides init` to setup CLI if not no_init: echo_green("Deployment successful! Initializing fides...") - config_path = create_config_file(config=config) - check_and_update_analytics_config(config=config, config_path=config_path) + create_and_update_config_file(config=config) else: echo_green( "Deployment successful! Skipping CLI initialization (run 'fides init' to initialize)" diff --git a/src/fides/cli/utils.py b/src/fides/cli/utils.py index a23875b368..c0efbe22a6 100644 --- a/src/fides/cli/utils.py +++ b/src/fides/cli/utils.py @@ -20,7 +20,6 @@ EMAIL_PROMPT, FIDESCTL_CLI, OPT_OUT_COPY, - OPT_OUT_PROMPT, ORGANIZATION_PROMPT, generate_client_id, ) @@ -42,7 +41,7 @@ get_config_database_credentials, get_config_okta_credentials, ) -from fides.core.config.helpers import get_config_from_file, update_config_file +from fides.core.config.helpers import get_config_from_file from fides.core.config.utils import get_dev_mode from fides.core.utils import check_response, echo_green, echo_red @@ -144,70 +143,50 @@ def register_user(config: FidesConfig, email: str, organization: str) -> None: ) -def check_and_update_analytics_config( - config: FidesConfig, config_path: str -) -> FidesConfig: +def request_analytics_consent(config: FidesConfig, opt_in: bool = False) -> FidesConfig: """ - Verify that the analytics opt-out value is populated. If not, - prompt the user to opt-in to analytics and update the config - file with their preferences if needed. - - NOTE: This doesn't handle the case where we've collected consent for this - CLI instance, but are connected to a server for the first time that is - unregistered. This *should* be something we can detect and then - "re-prompt" the user for their email/org information, but right - now a lot of our test automation runs headless and this kind of - prompt can't be skipped otherwise. + Request the user's consent for analytics collection. + + This function should only be called when specifically wanting to ask + for the user's consent, otherwise it will ask repeatedly for consent + unless they've opted in. """ - config_updates: Dict[str, Dict] = {} - if config.user.analytics_opt_out is None: - click.echo(OPT_OUT_COPY) - config.user.analytics_opt_out = bool( - input(OPT_OUT_PROMPT + "\n").lower() == "n" + analytics_env_var = getenv("FIDES__USER__ANALYTICS_OPT_OUT") + if analytics_env_var and analytics_env_var.lower() != "false": + return config + + if analytics_env_var and analytics_env_var.lower() == "true": + opt_in = True + + # Otherwise, ask for consent + print(OPT_OUT_COPY) + if not opt_in: + config.user.analytics_opt_out = not click.confirm( + "Opt-in to anonymous usage analytics?" ) + else: + config.user.analytics_opt_out = opt_in - config_updates.update(user={"analytics_opt_out": config.user.analytics_opt_out}) - - # If we've not opted out, attempt to register the user if they are - # currently connected to a Fides server - if config.user.analytics_opt_out is False: - server_url = str(config.cli.server_url) or "" - try: - check_server_health(server_url, verbose=False) - should_attempt_registration = not is_user_registered(config) - except SystemExit: - should_attempt_registration = False - - if should_attempt_registration: - email = input(EMAIL_PROMPT) - organization = input(ORGANIZATION_PROMPT) - if email and organization: - register_user(config, email, organization) - - # Either way, thank the user for their opt-in for analytics! - click.echo(CONFIRMATION_COPY) - - # Update the analytics ID in the config file if necessary - is_analytics_id_config_empty = get_config_from_file( - config_path, - "cli", - "analytics_id", - ) in ("", None) - is_analytics_id_env_var_set = getenv("FIDES__CLI__ANALYTICS_ID") - if ( - not config.user.analytics_opt_out - and is_analytics_id_config_empty - and not is_analytics_id_env_var_set - ): - config_updates.update(cli={"analytics_id": config.cli.analytics_id}) - - if len(config_updates) > 0: + # If we've not opted out, attempt to register the user if they are + # currently connected to a Fides server + if not config.user.analytics_opt_out: + server_url = str(config.cli.server_url) or "" try: - update_config_file(config_updates, config_path) - except FileNotFoundError as err: - echo_red(f"Failed to update config file ({config_path}): {err.strerror}") - click.echo("Run 'fides init' to create a configuration file.") + check_server_health(server_url, verbose=False) + should_attempt_registration = not is_user_registered(config) + except SystemExit: + should_attempt_registration = False + + if should_attempt_registration: + email = input(EMAIL_PROMPT) + organization = input(ORGANIZATION_PROMPT) + if email and organization: + register_user(config, email, organization) + + # Either way, thank the user for their opt-in for analytics! + click.echo(CONFIRMATION_COPY) + return config diff --git a/src/fides/core/config/__init__.py b/src/fides/core/config/__init__.py index 835bc8c87f..ceb63f5f3f 100644 --- a/src/fides/core/config/__init__.py +++ b/src/fides/core/config/__init__.py @@ -67,8 +67,12 @@ class FidesConfig(FidesSettings): # These should match the `settings_map` in `build_config` admin_ui: AdminUISettings cli: CLISettings - celery: Dict - credentials: Dict + celery: Dict = Field( + description="This section can be used to pass config vars to Celery directly.", + ) + credentials: Dict = Field( + description="This is a special section that is used to store arbitrary key/value pairs to be used as credentials." + ) database: DatabaseSettings execution: ExecutionSettings logging: LoggingSettings diff --git a/src/fides/core/config/admin_ui_settings.py b/src/fides/core/config/admin_ui_settings.py index 21a5f0d7c0..0f5cf0804a 100644 --- a/src/fides/core/config/admin_ui_settings.py +++ b/src/fides/core/config/admin_ui_settings.py @@ -4,7 +4,7 @@ class AdminUISettings(FidesSettings): - """Configuration settings for Analytics variables.""" + """Configuration settings for the Admin UI.""" enabled: bool = Field( default=True, description="Toggle whether the Admin UI is served." diff --git a/src/fides/core/config/cli_settings.py b/src/fides/core/config/cli_settings.py index a832b43250..59f634d237 100644 --- a/src/fides/core/config/cli_settings.py +++ b/src/fides/core/config/cli_settings.py @@ -11,11 +11,11 @@ class CLISettings(FidesSettings): - """Class used to store values from the 'cli' section of the config.""" + """Configuration settings for the command-line application.""" analytics_id: str = Field( default=generate_client_id(FIDESCTL_CLI), - description="A fully anonymized unique identifier that is automatically generated by the application and stored in the toml file.", + description="A fully anonymized unique identifier that is automatically generated by the application. Used for anonymous analytics when opted-in.", ) local_mode: bool = Field( default=False, diff --git a/src/fides/core/config/create.py b/src/fides/core/config/create.py new file mode 100644 index 0000000000..a826526752 --- /dev/null +++ b/src/fides/core/config/create.py @@ -0,0 +1,253 @@ +""" +This module auto-generates a documented config from the config source. +""" +import os +from textwrap import wrap +from typing import Dict, List, Set, Tuple + +import toml +from click import echo +from pydantic import BaseSettings + +from fides.cli.utils import request_analytics_consent +from fides.core.config import FidesConfig, build_config +from fides.core.config.utils import replace_config_value + +CONFIG_DOCS_URL = "https://ethyca.github.io/fides/stable/config/" +HELP_LINK = f"# For more info, please visit: {CONFIG_DOCS_URL}" + + +def get_nested_settings(config: FidesConfig) -> Dict[str, BaseSettings]: + """ + Get the list of fields from the full configuration settings that refer to + other settings objects. + + The filter here is a reversal of `get_non_object_fields` with + some additional complexity added around getting the name of the class + that gets used later. + + The returned object contains the name of the settings field as the key, + with the value being the Pydantic settings class itself. + """ + nested_settings = { + settings_name + for settings_name, settings_info in config.schema()["properties"].items() + if not settings_info.get("type") + } + + nested_settings_objects = { + settings_name: getattr(config, settings_name) + for settings_name in nested_settings + } + return nested_settings_objects + + +def format_value_for_toml(value: str, value_type: str) -> str: + """Format the value into valid TOML.""" + if value_type == "string": + return f'"{value}"' + if value_type == "boolean": + return str(value).lower() + if value_type == "array": + return "[]" + return value + + +def build_field_documentation(field_name: str, field_info: Dict[str, str]) -> str: + """Build a docstring for an individual docstring.""" + try: + field_type = field_info["type"] + field_description = "\n".join( + wrap( + text=field_info["description"], + width=71, + subsequent_indent="# ", + initial_indent="# ", + ) + ) + field_default = format_value_for_toml(field_info.get("default", ""), field_type) + doc_string = ( + f"{field_description}\n{field_name} = {field_default} # {field_type}\n" + ) + return doc_string + except KeyError: + print(field_info) + raise SystemExit(f"!Failed to parse field: {field_name}!") + + +def build_section_header(title: str) -> str: + """Build a pretty, TOML-valid section header.""" + title_piece = f"#-- {title.upper()} --#\n" + top_piece = f"#{'-' * (len(title_piece) - 3)}#\n" + bottom_piece = f"#{'-' * 68}#\n" + header = top_piece + title_piece + bottom_piece + return header + + +def convert_object_to_toml_docs(object_name: str, object_info: Dict[str, str]) -> str: + """ + Takes a Pydantic field of type `object` and returns a formatted string with included metadata. + + This is used to handle "special-case" top-level fields that aren't normal "settings" objects. + """ + title_header = build_section_header(object_name) + + # Build the Section docstring + settings_description = object_info["description"] + settings_docstring = ( + f"[{object_name}] # {settings_description}\n{HELP_LINK}#{object_name}\n" + ) + + # Build the field docstrings + full_docstring = title_header + settings_docstring + return full_docstring + + +def convert_settings_to_toml_docs(settings_name: str, settings: BaseSettings) -> str: + """ + Takes a Pydantic settings object and returns a + formatted string with included metadata. + + The string is expected to be valid TOML. + """ + settings_schema = settings.schema() + included_keys = set(settings.dict().keys()) + title_header = build_section_header(settings_name) + + # Build the Section docstring + settings_description = settings_schema["description"] + settings_docstring = f"[{settings_name}] # {settings_description}\n" + + # Build the field docstrings + fields = remove_excluded_fields(settings_schema["properties"], included_keys) + field_docstrings = [ + build_field_documentation(field_name, field_info) + for field_name, field_info in fields.items() + ] + full_docstring = ( + title_header + settings_docstring + "\n" + "\n".join(field_docstrings) + ) + return full_docstring + + +def remove_excluded_fields( + fields: Dict[str, Dict], included_fields: Set[str] +) -> Dict[str, Dict]: + """Remove fields that are marked as 'excluded=True' in their field.""" + without_excluded_fields = { + key: value for key, value in fields.items() if key in included_fields + } + return without_excluded_fields + + +def build_config_header() -> str: + """Build the header to be used at the top of the config file.""" + config_header = f"# Fides Configuration File\n# Additional Documentation at : {CONFIG_DOCS_URL}\n\n" + return config_header + + +def validate_generated_config(config_docs: str) -> None: + """Run a few checks on the generated configuration docs.""" + toml_docs = toml.loads(config_docs) + build_config(toml_docs) + if "# TODO" in config_docs: + raise ValueError( + "All fields require documentation, no description TODOs allowed!" + ) + + +def generate_config_docs( + config: FidesConfig, outfile_path: str = ".fides/fides.toml" +) -> None: + """ + Autogenerate the schema for the configuration file + and format it nicely as valid TOML. + + _Any individual value at the top-level of the config is ignored!_ + """ + + # Create the docs for the special "object" fields + schema_properties: Dict[str, Dict] = config.schema()["properties"] + object_fields = { + settings_name: settings_info + for settings_name, settings_info in schema_properties.items() + if settings_info.get("type") == "object" + } + object_docs = [ + convert_object_to_toml_docs(object_name, object_info) + for object_name, object_info in object_fields.items() + ] + + # Create the docs for the nested settings objects + settings: Dict[str, BaseSettings] = get_nested_settings(config) + ordered_settings: Dict[str, BaseSettings] = { + name: settings[name] for name in sorted(set(settings.keys())) + } + nested_settings_docs: List[str] = [ + convert_settings_to_toml_docs(settings_name, settings_schema) + for settings_name, settings_schema in ordered_settings.items() + ] + + # Combine all of the docs + docs: str = build_config_header() + "\n".join(nested_settings_docs + object_docs) + + # Verify it is a valid Fides config file + validate_generated_config(docs) + + with open(outfile_path, "w", encoding="utf-8") as output_file: + output_file.write(docs) + print(f"Exported configuration file to: {outfile_path}") + + +def create_config_file(config: FidesConfig, fides_directory_location: str = ".") -> str: + """ + Initializes the .fides/fides.toml file if it doesn't exist. + + Returns the config_path if successful. + """ + fides_dir_name = ".fides" + fides_dir_path = f"{fides_directory_location}/{fides_dir_name}" + config_file_name = "fides.toml" + config_path = f"{fides_dir_path}/{config_file_name}" + + # create the .fides dir if it doesn't exist + if not os.path.exists(fides_dir_path): + os.mkdir(fides_dir_path) + echo(f"Created a '{fides_dir_path}' directory.") + else: + echo(f"Directory '{fides_dir_path}' already exists.") + + # create a fides.toml config file if it doesn't exist + if not os.path.isfile(config_path): + generate_config_docs(config, config_path) + else: + echo(f"Configuration file already exists: {config_path}") + + echo("To learn more about configuring fides, see:") + echo("\thttps://ethyca.github.io/fides/config/") + + return config_path + + +def create_and_update_config_file( + config: FidesConfig, + fides_directory_location: str = ".", + opt_in: bool = False, +) -> Tuple[FidesConfig, str]: + # request explicit consent for analytics collection + config = request_analytics_consent(config=config, opt_in=opt_in) + + # create the config file as needed + config_path = create_config_file( + config=config, fides_directory_location=fides_directory_location + ) + + # Update the value in the config file if it differs from the default + if not config.user.analytics_opt_out: + replace_config_value( + fides_directory_location=fides_directory_location, + key="analytics_opt_out", + old_value="true", + new_value="false", + ) + return (config, config_path) diff --git a/src/fides/core/config/database_settings.py b/src/fides/core/config/database_settings.py index d25e4969b6..81f47e86ad 100644 --- a/src/fides/core/config/database_settings.py +++ b/src/fides/core/config/database_settings.py @@ -14,7 +14,7 @@ class DatabaseSettings(FidesSettings): - """Configuration settings for Postgres.""" + """Configuration settings for the application database.""" api_engine_pool_size: int = Field( default=50, diff --git a/src/fides/core/config/execution_settings.py b/src/fides/core/config/execution_settings.py index a6a1fe1b5c..eaa41e1b4f 100644 --- a/src/fides/core/config/execution_settings.py +++ b/src/fides/core/config/execution_settings.py @@ -6,7 +6,7 @@ class ExecutionSettings(FidesSettings): - """Configuration settings for execution.""" + """Configuration settings for DSR execution.""" masking_strict: bool = Field( default=True, diff --git a/src/fides/core/config/helpers.py b/src/fides/core/config/helpers.py index a23f8d9a40..2bbd9cd4ab 100644 --- a/src/fides/core/config/helpers.py +++ b/src/fides/core/config/helpers.py @@ -5,10 +5,8 @@ from re import compile as regex from typing import Any, Dict, List, Union -import toml from click import echo from loguru import logger -from pydantic import BaseSettings from toml import dump, load from fides.core.utils import echo_red @@ -103,41 +101,6 @@ def update_config_file( # type: ignore echo(f"\tSet {key}.{subkey} = {val}") -def create_config_file( - config: BaseSettings, fides_directory_location: str = "." -) -> str: - """ - Creates the .fides/fides.toml file and initializes it, if it doesn't exist. - - Returns the config_path if successful. - """ - fides_dir_name = ".fides" - fides_dir_path = f"{fides_directory_location}/{fides_dir_name}" - config_file_name = "fides.toml" - config_path = f"{fides_dir_path}/{config_file_name}" - - # create the .fides dir if it doesn't exist - if not os.path.exists(fides_dir_path): - os.mkdir(fides_dir_path) - echo(f"Created a '{fides_dir_path}' directory.") - else: - echo(f"Directory '{fides_dir_path}' already exists.") - - # create a fides.toml config file if it doesn't exist - if not os.path.isfile(config_path): - with open(config_path, "w", encoding="utf-8") as config_file: - config_dict = config.dict() # type: ignore[arg-type] - toml.dump(config_dict, config_file) - echo(f"Created a fides config file: {config_path}") - else: - echo(f"Configuration file already exists: {config_path}") - - echo("To learn more about configuring fides, see:") - echo("\thttps://ethyca.github.io/fides/installation/configuration/") - - return config_path - - def handle_deprecated_fields(settings: Dict[str, Any]) -> Dict[str, Any]: """Custom logic for handling deprecated values.""" diff --git a/src/fides/core/config/logging_settings.py b/src/fides/core/config/logging_settings.py index 122fc0410f..09492b7476 100644 --- a/src/fides/core/config/logging_settings.py +++ b/src/fides/core/config/logging_settings.py @@ -13,7 +13,7 @@ class LoggingSettings(FidesSettings): - """Class used to store values from the 'logging' section of the config.""" + """Configuration settings for application logging.""" # Logging destination: str = Field( diff --git a/src/fides/core/config/notification_settings.py b/src/fides/core/config/notification_settings.py index e7c6e00443..8be55c0e74 100644 --- a/src/fides/core/config/notification_settings.py +++ b/src/fides/core/config/notification_settings.py @@ -8,7 +8,7 @@ class NotificationSettings(FidesSettings): - """Configuration settings for data subject and/or data processor notifications""" + """Configuration settings for Data Subject and/or Data Processor notifications.""" notification_service_type: Optional[str] = Field( default=None, diff --git a/src/fides/core/config/redis_settings.py b/src/fides/core/config/redis_settings.py index 9c715286c9..42fce9730c 100644 --- a/src/fides/core/config/redis_settings.py +++ b/src/fides/core/config/redis_settings.py @@ -15,11 +15,14 @@ class RedisSettings(FidesSettings): default="utf8", description="Character set to use for Redis, defaults to 'utf8'. Not recommended to change.", ) - db_index: Optional[int] = Field( - default=None, + db_index: int = Field( + default=0, description="The application will use this index in the Redis cache to cache data.", ) - decode_responses: bool = Field(default=True, description="TODO") + decode_responses: bool = Field( + default=True, + description="Whether or not to automatically decode the values fetched from Redis. Decodes using the `charset` configuration value.", + ) default_ttl_seconds: int = Field( default=604800, description="The number of seconds for which data will live in Redis before automatically expiring.", diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index d33b578dfa..5644f83b58 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -17,7 +17,7 @@ class SecuritySettings(FidesSettings): - """Configuration settings for Security variables.""" + """Configuration settings for application security.""" aes_encryption_key_length: int = Field( default=16, @@ -30,11 +30,14 @@ class SecuritySettings(FidesSettings): app_encryption_key: str = Field( default="", description="The key used to sign Fides API access tokens." ) - cors_origins: Union[str, List[str]] = Field( + cors_origins: List[str] = Field( default=[], - description="A list of pre-approved addresses of clients allowed to communicate with the Fides application server.", + description="A list of client addresses allowed to communicate with the Fides webserver.", + ) + cors_origin_regex: Optional[Pattern] = Field( + default=None, + description="A regex pattern used to set the CORS origin allowlist.", ) - cors_origin_regex: Optional[Pattern] = Field(default=None, description="TODO") drp_jwt_secret: Optional[str] = Field( default=None, description="JWT secret by which passed-in identity is decrypted according to the HS256 algorithm.", @@ -46,7 +49,10 @@ class SecuritySettings(FidesSettings): default="dev", description="The default, `dev`, does not apply authentication to endpoints typically used by the CLI. The other option, `prod`, requires authentication for _all_ endpoints that may contain sensitive information.", ) - identity_verification_attempt_limit: int = Field(default=3, description="") + identity_verification_attempt_limit: int = Field( + default=3, + description="The number of times identity verification will be attempted before raising an error.", + ) oauth_root_client_id: str = Field( default="", description="The value used to identify the Fides application root API client.", diff --git a/src/fides/core/config/user_settings.py b/src/fides/core/config/user_settings.py index 67eb5e8fdd..e8978c61fa 100644 --- a/src/fides/core/config/user_settings.py +++ b/src/fides/core/config/user_settings.py @@ -1,7 +1,7 @@ """This module handles finding and parsing fides configuration files.""" # pylint: disable=C0115,C0116, E0213 -from typing import Dict, Optional +from typing import Dict from pydantic import Field @@ -21,15 +21,16 @@ def try_get_auth_header() -> Dict[str, str]: class UserSettings(FidesSettings): - """Class used to store values from the 'user' section of the config.""" + """Configuration settings that apply to the current user as opposed to the entire application instance.""" auth_header: Dict[str, str] = Field( default=try_get_auth_header(), description="Authentication header built automatically from the credentials file.", exclude=True, ) - analytics_opt_out: Optional[bool] = Field( - description="When set to true, prevents sending anonymous analytics data to Ethyca." + analytics_opt_out: bool = Field( + default=True, + description="When set to true, prevents sending privacy-respecting anonymous analytics data to Ethyca.", ) encryption_key: str = Field( default="test_encryption_key", diff --git a/src/fides/core/config/utils.py b/src/fides/core/config/utils.py index e3f4737eca..e8d08c4f8f 100644 --- a/src/fides/core/config/utils.py +++ b/src/fides/core/config/utils.py @@ -4,6 +4,29 @@ DEFAULT_CONFIG_PATH_ENV_VAR = "FIDES__CONFIG_PATH" +def replace_config_value( + fides_directory_location: str, key: str, old_value: str, new_value: str +) -> None: + """Use string replacment to update a value in the fides.toml""" + + # This matches the logic used in `docs.py` + fides_dir_name = ".fides" + fides_dir_path = f"{fides_directory_location}/{fides_dir_name}" + config_file_name = "fides.toml" + config_path = f"{fides_dir_path}/{config_file_name}" + + with open(config_path, "r", encoding="utf8") as config_file: + previous_config = config_file.read() + new_config = previous_config.replace( + f"{key} = {old_value}", f"{key} = {new_value}" + ) + + with open(config_path, "w", encoding="utf8") as config_file: + config_file.write(new_config) + + print(f"Config key: {key} value changed: {old_value} -> {new_value}") + + def get_test_mode() -> bool: test_mode = getenv("FIDES__TEST_MODE", "").lower() == "true" return test_mode diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 4916eddbcc..35fce30e08 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -41,6 +41,16 @@ def test_init(test_cli_runner: CliRunner) -> None: assert result.exit_code == 0 +@pytest.mark.integration +def test_init_opt_in(test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, + ["init", "--opt-in"], + ) + print(result.output) + assert result.exit_code == 0 + + @pytest.mark.unit def test_view_config(test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke( diff --git a/tests/ctl/cli/test_cli_utils.py b/tests/ctl/cli/test_utils.py similarity index 99% rename from tests/ctl/cli/test_cli_utils.py rename to tests/ctl/cli/test_utils.py index 95455aeb19..2ac38eae08 100644 --- a/tests/ctl/cli/test_cli_utils.py +++ b/tests/ctl/cli/test_utils.py @@ -1,11 +1,13 @@ # pylint: disable=missing-docstring, redefined-outer-name +import os +from unittest.mock import patch + import click import pytest from requests_mock import Mocker import fides.cli.utils as utils -from fides.api.ctl.routes.util import API_PREFIX -from fides.core.config import FidesConfig +from fides.core.config import FidesConfig, get_config from tests.ctl.conftest import orig_requests_get diff --git a/tests/ctl/core/test_config.py b/tests/ctl/core/config/test_config.py similarity index 100% rename from tests/ctl/core/test_config.py rename to tests/ctl/core/config/test_config.py diff --git a/tests/ctl/core/test_config_helpers.py b/tests/ctl/core/config/test_config_helpers.py similarity index 56% rename from tests/ctl/core/test_config_helpers.py rename to tests/ctl/core/config/test_config_helpers.py index 93cb5abd51..9345b83f92 100644 --- a/tests/ctl/core/test_config_helpers.py +++ b/tests/ctl/core/config/test_config_helpers.py @@ -35,46 +35,6 @@ def test_get_config_from_file_none(self, section, option, tmp_path): assert helpers.get_config_from_file(file, "bad", "missing") is None - def test_create_config_file(self, config, tmp_path, capfd): - config_path = helpers.create_config_file(config, tmp_path) - - fides_directory = tmp_path / ".fides" - fides_file_path = fides_directory / "fides.toml" - out, _ = capfd.readouterr() - - assert f"Created a '{fides_directory}' directory" in out - assert f"Created a fides config file: {fides_file_path}" in out - assert config_path == str(fides_file_path) - - def test_create_config_file_dir_exists(self, config, tmp_path, capfd): - fides_directory = tmp_path / ".fides" - fides_directory.mkdir() - fides_file_path = fides_directory / "fides.toml" - - config_path = helpers.create_config_file(config, tmp_path) - - out, _ = capfd.readouterr() - - assert f"Directory '{fides_directory}' already exists" in out - assert f"Created a fides config file: {fides_file_path}" in out - assert config_path == str(fides_file_path) - - def test_create_config_file_exists(self, config, tmp_path, capfd): - fides_directory = tmp_path / ".fides" - fides_directory.mkdir() - fides_file_path = fides_directory / "fides.toml" - - with open(fides_file_path, "w", encoding="utf-8") as f: - toml.dump(config.dict(), f) - - config_path = helpers.create_config_file(config, tmp_path) - - out, _ = capfd.readouterr() - - assert f"Directory '{fides_directory}' already exists" in out - assert f"Configuration file already exists: {fides_file_path}" in out - assert config_path == str(fides_file_path) - @patch("fides.core.config.helpers.get_config_from_file") def test_check_required_webserver_config_values(self, mock_get_config, capfd): mock_get_config.return_value = None diff --git a/tests/ctl/core/config/test_create.py b/tests/ctl/core/config/test_create.py new file mode 100644 index 0000000000..d44ac91e8f --- /dev/null +++ b/tests/ctl/core/config/test_create.py @@ -0,0 +1,107 @@ +import os + +import pytest +import toml +from py._path.local import LocalPath + +from fides.core.config import FidesConfig +from fides.core.config.create import ( + build_field_documentation, + create_and_update_config_file, + create_config_file, + validate_generated_config, +) + + +@pytest.mark.unit +def test_create_and_update_config_file_opt_in( + tmpdir: LocalPath, test_config: FidesConfig +) -> None: + """Test that config creation works when opting-in to analytics.""" + + create_and_update_config_file( + config=test_config, fides_directory_location=str(tmpdir), opt_in=True + ) + assert True + + +@pytest.mark.unit +class TestValidateGeneratedConfig: + def test_valid_config(self) -> None: + """Test that a minimal, but still valid config, can be built.""" + config_docs = toml.dumps({"database": {"server_port": "1234"}}) + validate_generated_config(config_docs) + assert True + + def test_invalid_toml(self) -> None: + """Test that a config with invalid toml throws an error.""" + with pytest.raises(ValueError): + config_docs = "[database]\nsom_key = # Empty value, will cause error" + validate_generated_config(config_docs) + + def test_includes_todo(self) -> None: + """Test that a valid config that contains '# TODO' is invalid.""" + with pytest.raises(ValueError): + config_docs = toml.dumps({"database": {"server_port": "# TODO"}}) + validate_generated_config(config_docs) + assert True + + +@pytest.fixture() +def remove_fides_dir(tmp_path) -> None: + try: + os.remove(tmp_path / ".fides/fides.toml") + os.rmdir(tmp_path / ".fides") + except FileNotFoundError: + pass + except NotADirectoryError: + pass + + +@pytest.mark.unit +class TestCreateConfigFile: + def test_create_config_file( + self, config, tmp_path, capfd, remove_fides_dir + ) -> None: + config_path = create_config_file(config, tmp_path) + + fides_directory = tmp_path / ".fides" + fides_file_path = fides_directory / "fides.toml" + out, _ = capfd.readouterr() + + assert f"Created a '{fides_directory}' directory" in out + assert f"Exported configuration file to: {fides_file_path}" in out + assert config_path == str(fides_file_path) + + def test_create_config_file_dir_exists( + self, config, tmp_path, capfd, remove_fides_dir + ) -> None: + fides_directory = tmp_path / ".fides" + fides_directory.mkdir() + fides_file_path = fides_directory / "fides.toml" + + config_path = create_config_file(config, tmp_path) + + out, _ = capfd.readouterr() + + assert f"Directory '{fides_directory}' already exists" in out + assert f"Exported configuration file to: {fides_file_path}" in out + assert config_path == str(fides_file_path) + + def test_create_config_file_exists( + self, config, tmp_path, capfd, remove_fides_dir + ) -> None: + fides_directory = tmp_path / ".fides" + fides_directory.mkdir() + fides_file_path = fides_directory / "fides.toml" + + with open(fides_file_path, "w", encoding="utf-8") as f: + toml.dump(config.dict(), f) + + config_path = create_config_file(config, tmp_path) + + out, _ = capfd.readouterr() + + assert f"Directory '{fides_directory}' already exists" in out + assert f"Configuration file already exists: {fides_file_path}" in out + assert config_path == str(fides_file_path) diff --git a/tests/ctl/core/test_config_utils.py b/tests/ctl/core/config/test_utils.py similarity index 70% rename from tests/ctl/core/test_config_utils.py rename to tests/ctl/core/config/test_utils.py index 6be76c43b6..c9b5f58566 100644 --- a/tests/ctl/core/test_config_utils.py +++ b/tests/ctl/core/config/test_utils.py @@ -8,6 +8,7 @@ from fides.core.config import FidesConfig from fides.core.config.helpers import update_config_file +from fides.core.config.utils import replace_config_value @pytest.fixture @@ -17,6 +18,27 @@ def test_change_config() -> Generator: yield {"cli": {"analytics_id": "initial_id"}} +@pytest.mark.unit +def test_replace_config_value(tmpdir: LocalPath) -> None: + config_dir = tmpdir / ".fides" + os.mkdir(config_dir) + config_path = config_dir / "fides.toml" + + expected_result = "# test_value = true" + test_file = "# test_value = false" + + with open(config_path, "w") as config_file: + config_file.write(test_file) + + replace_config_value(str(tmpdir), "test_value", "false", "true") + + with open(config_path, "r") as config_file: + actual_result = config_file.read() + + print(actual_result) + assert actual_result == expected_result + + @pytest.mark.unit def test_update_config_file_new_value( test_change_config: FidesConfig, tmpdir: LocalPath From c6b88200156d329167686582dc276224f6c67772 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 09:17:11 +0000 Subject: [PATCH 02/26] allow template for sendgrid if exists --- .../messaging/message_dispatch_service.py | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index e6f7673f77..afd65c1fc9 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -6,7 +6,7 @@ import requests import sendgrid from loguru import logger -from sendgrid.helpers.mail import Content, Email, Mail, To +from sendgrid.helpers.mail import Content, Email, Mail, Personalization, TemplateId, To from sqlalchemy.orm import Session from twilio.base.exceptions import TwilioRestException from twilio.rest import Client @@ -45,6 +45,7 @@ from fides.core.config.config_proxy import ConfigProxy EMAIL_JOIN_STRING = ", " +EMAIL_TEMPLATE_NAME = "fides" def check_and_dispatch_error_notifications(db: Session) -> None: @@ -378,7 +379,7 @@ def _mailgun_dispatcher( try: # Check if a fides template exists template_test = requests.get( - f"{base_url}/{messaging_config.details[MessagingServiceDetails.API_VERSION.value]}/{domain}/templates/fides", + f"{base_url}/{messaging_config.details[MessagingServiceDetails.API_VERSION.value]}/{domain}/templates/{EMAIL_TEMPLATE_NAME}", auth=( "api", messaging_config.secrets[MessagingServiceSecrets.MAILGUN_API_KEY.value], @@ -392,7 +393,7 @@ def _mailgun_dispatcher( } if template_test.status_code == 200: - data["template"] = "fides" + data["template"] = EMAIL_TEMPLATE_NAME data["h:X-Mailgun-Variables"] = json.dumps( {"fides_email_body": message.body} ) @@ -456,18 +457,27 @@ def _twilio_email_dispatcher( ) try: + sg = sendgrid.SendGridAPIClient( api_key=messaging_config.secrets[ MessagingServiceSecrets.TWILIO_API_KEY.value ] ) + template_test = _get_template_id_if_exists(sg, EMAIL_TEMPLATE_NAME) + from_email = Email( messaging_config.details[MessagingServiceDetails.TWILIO_EMAIL_FROM.value] ) to_email = To(to.strip()) subject = message.subject - content = Content("text/html", message.body) - mail = Mail(from_email, to_email, subject, content) + if template_test: + personalization = Personalization() + personalization.dynamic_template_data = {"fides_email_body": message.body} + template_id = TemplateId(template_test) + mail = Mail(from_email, to_email, personalization, template_id) + else: + content = Content("text/html", message.body) + mail = Mail(from_email, to_email, subject, content) response = sg.client.mail.send.post(request_body=mail.get()) if response.status_code >= 400: logger.error( @@ -526,3 +536,22 @@ def _twilio_sms_dispatcher( except TwilioRestException as e: logger.error("Twilio SMS failed to send: {}", Pii(str(e))) raise MessageDispatchException(f"Twilio SMS failed to send due to: {Pii(e)}") + + +def _get_template_id_if_exists( + sg: sendgrid.SendGridAPIClient, template_name: str +) -> Optional[str]: + """ + Checks to see if a SendGrid template exists for Fides, returning the id if so + """ + + params = {"generations": "legacy,dynamic", "page_size": 18} + + response = sg.client.templates.get(query_params=params) + + response_body_dict = json.loads(response.body) + + for template in response_body_dict["result"]: + if template["name"].lower() == template_name.lower(): + return template["id"] + return None From ab4ddc7b39b01a364b0b61f659227e62381f7ab3 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 14:58:59 +0000 Subject: [PATCH 03/26] reconfigure sending from a template, dynamic only --- .../api/ops/service/messaging/message_dispatch_service.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index afd65c1fc9..ef143edcb9 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -471,10 +471,12 @@ def _twilio_email_dispatcher( to_email = To(to.strip()) subject = message.subject if template_test: + mail = Mail(from_email=from_email, subject=subject) + mail.template_id = TemplateId(template_test) personalization = Personalization() personalization.dynamic_template_data = {"fides_email_body": message.body} - template_id = TemplateId(template_test) - mail = Mail(from_email, to_email, personalization, template_id) + personalization.add_email(to_email) + mail.add_personalization(personalization) else: content = Content("text/html", message.body) mail = Mail(from_email, to_email, subject, content) @@ -545,7 +547,7 @@ def _get_template_id_if_exists( Checks to see if a SendGrid template exists for Fides, returning the id if so """ - params = {"generations": "legacy,dynamic", "page_size": 18} + params = {"generations": "dynamic", "page_size": 18} response = sg.client.templates.get(query_params=params) From c6df2577bd74414e894acff6bc1514b2c587d624 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 1 Mar 2023 18:14:37 +0000 Subject: [PATCH 04/26] use max page limit due to broken client pagination --- .../api/ops/service/messaging/message_dispatch_service.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fides/api/ops/service/messaging/message_dispatch_service.py b/src/fides/api/ops/service/messaging/message_dispatch_service.py index ef143edcb9..97fb676578 100644 --- a/src/fides/api/ops/service/messaging/message_dispatch_service.py +++ b/src/fides/api/ops/service/messaging/message_dispatch_service.py @@ -547,10 +547,11 @@ def _get_template_id_if_exists( Checks to see if a SendGrid template exists for Fides, returning the id if so """ - params = {"generations": "dynamic", "page_size": 18} - + # the pagination via the client actually doesn't work + # in lieu of over-engineering this we can manually call + # the next page if/when we hit the limit here + params = {"generations": "dynamic", "page_size": 200} response = sg.client.templates.get(query_params=params) - response_body_dict = json.loads(response.body) for template in response_body_dict["result"]: From 5368c054adddc97cac07411a64a8a7ca93761271 Mon Sep 17 00:00:00 2001 From: Sebastian Sangervasi <2236777+ssangervasi@users.noreply.github.com> Date: Wed, 1 Mar 2023 11:19:16 -0800 Subject: [PATCH 05/26] [2460]: Change "manual webhook" to "manual process" in FE (#2717) --- CHANGELOG.md | 1 + .../cypress/fixtures/connectors/connection_types.json | 2 +- clients/admin-ui/src/constants.ts | 6 +++--- .../manual-processing/ManualProcessingList.tsx | 2 +- src/fides/api/ops/models/connectionconfig.py | 2 +- .../api/v1/endpoints/test_connection_template_endpoints.py | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d3916ed0b..26c8ad03d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ The types of changes are: * Add flow for selecting system types when manually creating a system [#2530](https://github.com/ethyca/fides/pull/2530) * Updated forms for privacy declarations [#2648](https://github.com/ethyca/fides/pull/2648) * Delete flow for privacy declarations [#2664](https://github.com/ethyca/fides/pull/2664) + * "Manual Webhook" has been renamed to "Manual Process". [#2717](https://github.com/ethyca/fides/pull/2717) * Convert all config values to Pydantic `Field` objects [#2613](https://github.com/ethyca/fides/pull/2613) * Add warning to 'fides deploy' when installed outside of a virtual environment [#2641](https://github.com/ethyca/fides/pull/2641) * Redesigned the default/init config file to be auto-documented. Also updates the `fides init` logic and analytics consent logic [#2694](https://github.com/ethyca/fides/pull/2694) diff --git a/clients/admin-ui/cypress/fixtures/connectors/connection_types.json b/clients/admin-ui/cypress/fixtures/connectors/connection_types.json index 5804649ea6..96490a753e 100644 --- a/clients/admin-ui/cypress/fixtures/connectors/connection_types.json +++ b/clients/admin-ui/cypress/fixtures/connectors/connection_types.json @@ -57,7 +57,7 @@ { "identifier": "manual_webhook", "type": "manual", - "human_readable": "Manual Webhook", + "human_readable": "Manual Process", "encoded_icon": null }, { diff --git a/clients/admin-ui/src/constants.ts b/clients/admin-ui/src/constants.ts index 8ff6e55b7d..4861d42447 100644 --- a/clients/admin-ui/src/constants.ts +++ b/clients/admin-ui/src/constants.ts @@ -109,15 +109,15 @@ export const USER_PRIVILEGES: UserPrivileges[] = [ scope: "privacy-request:view_data", }, { - privilege: "Create manual webhooks", + privilege: "Create manual processes", scope: "webhook:create_or_update", }, { - privilege: "Read manual webhooks", + privilege: "Read manual processes", scope: "webhook:read", }, { - privilege: "Delete manual webhooks", + privilege: "Delete manual processes", scope: "webhook:delete", }, ]; diff --git a/clients/admin-ui/src/features/privacy-requests/manual-processing/ManualProcessingList.tsx b/clients/admin-ui/src/features/privacy-requests/manual-processing/ManualProcessingList.tsx index efb0aa28cf..3b1340c867 100644 --- a/clients/admin-ui/src/features/privacy-requests/manual-processing/ManualProcessingList.tsx +++ b/clients/admin-ui/src/features/privacy-requests/manual-processing/ManualProcessingList.tsx @@ -229,7 +229,7 @@ const ManualProcessingList: React.FC = ({
- You don‘t have any Manual Webhook connections + You don‘t have any Manual Process connections set up yet.
diff --git a/src/fides/api/ops/models/connectionconfig.py b/src/fides/api/ops/models/connectionconfig.py index ef9a6a78e9..07095ef52d 100644 --- a/src/fides/api/ops/models/connectionconfig.py +++ b/src/fides/api/ops/models/connectionconfig.py @@ -70,7 +70,7 @@ def human_readable(self) -> str: ConnectionType.bigquery.value: "BigQuery", ConnectionType.manual.value: "Manual Connector", ConnectionType.email.value: "Email Connector", - ConnectionType.manual_webhook.value: "Manual Webhook", + ConnectionType.manual_webhook.value: "Manual Process", ConnectionType.timescale.value: "TimescaleDB", ConnectionType.fides.value: "Fides Connector", ConnectionType.sovrn.value: "Sovrn", diff --git a/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py b/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py index 074466a062..66e12fa31a 100644 --- a/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py @@ -310,7 +310,7 @@ def test_search_manual_system_type(self, api_client, generate_auth_header, url): { "identifier": "manual_webhook", "type": "manual", - "human_readable": "Manual Webhook", + "human_readable": "Manual Process", "encoded_icon": None, } ] From 0e7d3f78c47daa5d9ffad4610d8f58bbae96eb5f Mon Sep 17 00:00:00 2001 From: Allison King Date: Wed, 1 Mar 2023 15:54:56 -0500 Subject: [PATCH 06/26] Privacy declaration updates (#2723) --- clients/admin-ui/cypress/e2e/systems.cy.ts | 15 ++++- .../src/features/common/form/inputs.tsx | 7 +- .../src/features/config-wizard/AddSystem.tsx | 4 +- .../features/config-wizard/SystemOption.tsx | 2 +- .../src/features/dataset/dataset.slice.ts | 7 ++ .../features/system/SystemInformationForm.tsx | 1 + .../PrivacyDeclarationAccordion.tsx | 1 + .../PrivacyDeclarationForm.tsx | 67 +++++++++---------- .../PrivacyDeclarationManager.tsx | 1 + .../PrivacyDeclarationStep.tsx | 7 +- .../system/privacy-declarations/hooks.ts | 44 ++++++++++++ clients/admin-ui/src/flags.json | 6 ++ 12 files changed, 121 insertions(+), 41 deletions(-) create mode 100644 clients/admin-ui/src/features/system/privacy-declarations/hooks.ts diff --git a/clients/admin-ui/cypress/e2e/systems.cy.ts b/clients/admin-ui/cypress/e2e/systems.cy.ts index c4e5e732b0..e6b9647bde 100644 --- a/clients/admin-ui/cypress/e2e/systems.cy.ts +++ b/clients/admin-ui/cypress/e2e/systems.cy.ts @@ -56,6 +56,9 @@ describe("System management page", () => { beforeEach(() => { stubTaxonomyEntities(); stubSystemCrud(); + cy.intercept("GET", "/api/v1/dataset", { fixture: "datasets.json" }).as( + "getDatasets" + ); cy.intercept("GET", "/api/v1/connection_type*", { fixture: "connectors/connection_types.json", }).as("getConnectionTypes"); @@ -128,7 +131,12 @@ describe("System management page", () => { // Fill in the privacy declaration form cy.getByTestId("tab-Data uses").click(); cy.getByTestId("add-btn").click(); - cy.wait(["@getDataCategories", "@getDataSubjects", "@getDataUses"]); + cy.wait([ + "@getDataCategories", + "@getDataSubjects", + "@getDataUses", + "@getDatasets", + ]); cy.getByTestId("new-declaration-form"); const declaration = system.privacy_declarations[0]; cy.getByTestId("input-data_use").click(); @@ -141,6 +149,10 @@ describe("System management page", () => { declaration.data_subjects.forEach((ds) => { cy.getByTestId("input-data_subjects").type(`${ds}{enter}`); }); + cy.getByTestId("input-dataset_references").click(); + cy.getByTestId("input-dataset_references").within(() => { + cy.contains("Demo Users Dataset 2").click(); + }); cy.getByTestId("save-btn").click(); cy.wait("@putSystem").then((interception) => { @@ -150,6 +162,7 @@ describe("System management page", () => { data_use: declaration.data_use, data_categories: declaration.data_categories, data_subjects: declaration.data_subjects, + dataset_references: ["demo_users_dataset_2"], }); }); cy.getByTestId("new-declaration-form").within(() => { diff --git a/clients/admin-ui/src/features/common/form/inputs.tsx b/clients/admin-ui/src/features/common/form/inputs.tsx index a6186266e6..2cd2de5810 100644 --- a/clients/admin-ui/src/features/common/form/inputs.tsx +++ b/clients/admin-ui/src/features/common/form/inputs.tsx @@ -80,7 +80,12 @@ const TextInput = ({ return ( - + {isPassword ? ( { Manually add systems - + } @@ -67,7 +67,7 @@ const AddSystem = () => { Automated infrastructure scanning - + - + {icon} {label} diff --git a/clients/admin-ui/src/features/dataset/dataset.slice.ts b/clients/admin-ui/src/features/dataset/dataset.slice.ts index c927e706ed..296ee8ac9d 100644 --- a/clients/admin-ui/src/features/dataset/dataset.slice.ts +++ b/clients/admin-ui/src/features/dataset/dataset.slice.ts @@ -153,6 +153,13 @@ export const { reducer } = datasetSlice; const selectDataset = (state: RootState) => state.dataset; +const emptyDatasets: Dataset[] = []; +export const selectAllDatasets: (state: RootState) => Dataset[] = + createSelector( + datasetApi.endpoints.getAllDatasets.select(), + ({ data }) => data ?? emptyDatasets + ); + export const selectActiveDatasetFidesKey = createSelector( selectDataset, (state) => state.activeDatasetFidesKey diff --git a/clients/admin-ui/src/features/system/SystemInformationForm.tsx b/clients/admin-ui/src/features/system/SystemInformationForm.tsx index 37f6ab1dbb..0bbec56b8c 100644 --- a/clients/admin-ui/src/features/system/SystemInformationForm.tsx +++ b/clients/admin-ui/src/features/system/SystemInformationForm.tsx @@ -35,6 +35,7 @@ import SystemInformationFormExtension from "~/features/system/SystemInformationF import { ResourceTypes, System } from "~/types/api"; const ValidationSchema = Yup.object().shape({ + name: Yup.string().required().label("System name"), fides_key: Yup.string().required().label("System key"), }); diff --git a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx index 12c442086b..32f2919d3c 100644 --- a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx +++ b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx @@ -24,6 +24,7 @@ interface AccordionProps extends DataProps { newDeclaration: PrivacyDeclaration ) => Promise; onDelete: (declaration: PrivacyDeclaration) => Promise; + includeDeprecatedFields?: boolean; } const PrivacyDeclarationAccordionItem = ({ diff --git a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx index ea70fb4160..7b0f780f10 100644 --- a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx +++ b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx @@ -17,23 +17,11 @@ import { Form, Formik, FormikHelpers, useFormikContext } from "formik"; import { useMemo, useState } from "react"; import * as Yup from "yup"; -import { useAppSelector } from "~/app/hooks"; import ConfirmationModal from "~/features/common/ConfirmationModal"; -import { CustomSelect } from "~/features/common/form/inputs"; -import { - selectDataSubjects, - useGetAllDataSubjectsQuery, -} from "~/features/data-subjects/data-subject.slice"; -import { - selectDataUses, - useGetAllDataUsesQuery, -} from "~/features/data-use/data-use.slice"; -import { - selectDataCategories, - useGetAllDataCategoriesQuery, -} from "~/features/taxonomy/taxonomy.slice"; +import { CustomSelect, CustomTextInput } from "~/features/common/form/inputs"; import { DataCategory, + Dataset, DataSubject, DataUse, PrivacyDeclaration, @@ -53,6 +41,7 @@ const defaultInitialValues: PrivacyDeclaration = { data_categories: [], data_subjects: [], data_use: "", + dataset_references: [], }; type FormValues = typeof defaultInitialValues; @@ -61,18 +50,28 @@ export interface DataProps { allDataCategories: DataCategory[]; allDataUses: DataUse[]; allDataSubjects: DataSubject[]; + allDatasets?: Dataset[]; } export const PrivacyDeclarationFormComponents = ({ allDataUses, allDataCategories, allDataSubjects, + allDatasets, onDelete, -}: DataProps & Pick) => { + includeDeprecatedFields, +}: DataProps & Pick) => { const { dirty, isSubmitting, isValid, initialValues } = useFormikContext(); const deleteModal = useDisclosure(); + const datasetOptions = allDatasets + ? allDatasets.map((d) => ({ + label: d.name ?? d.fides_key, + value: d.fides_key, + })) + : []; + const handleDelete = async () => { await onDelete(initialValues); deleteModal.onClose(); @@ -94,6 +93,14 @@ export const PrivacyDeclarationFormComponents = ({ variant="stacked" singleValueBlock /> + {includeDeprecatedFields ? ( + + ) : null} + {allDatasets ? ( + + ) : null} - + + + + )} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx index 1a50d8f717..8b9259e51b 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/MessagingConfiguration.tsx @@ -10,14 +10,16 @@ import { Text, } from "@fidesui/react"; import NextLink from "next/link"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import Layout from "~/features/common/Layout"; +import { messagingProviders } from "~/features/privacy-requests/constants"; import { useCreateConfigurationSettingsMutation, useCreateMessagingConfigurationMutation, + useGetActiveMessagingProviderQuery, } from "~/features/privacy-requests/privacy-requests.slice"; import MailgunEmailConfiguration from "./MailgunEmailConfiguration"; @@ -31,34 +33,41 @@ const MessagingConfiguration = () => { const [createMessagingConfiguration] = useCreateMessagingConfigurationMutation(); const [saveActiveConfiguration] = useCreateConfigurationSettingsMutation(); + const { data: activeMessagingProvider } = + useGetActiveMessagingProviderQuery(); + + useEffect(() => { + if (activeMessagingProvider) { + setMessagingValue(activeMessagingProvider?.service_type); + } + }, [activeMessagingProvider]); const handleChange = async (value: string) => { const result = await saveActiveConfiguration({ - fides: { - notifications: { - notification_service_type: value, - send_request_completion_notification: true, - send_request_receipt_notification: true, - send_request_review_notification: true, - subject_identity_verification_required: true, - }, + notifications: { + notification_service_type: value, + send_request_completion_notification: true, + send_request_receipt_notification: true, + send_request_review_notification: true, + }, + execution: { + subject_identity_verification_required: true, }, }); if (isErrorResult(result)) { handleError(result.error); - } else if (value !== "twilio_text") { - successAlert(`Configured storage type successfully.`); + } else if (value !== messagingProviders.twilio_text) { setMessagingValue(value); } else { const twilioTextResult = await createMessagingConfiguration({ - type: "twilio_text", + service_type: messagingProviders.twilio_text, }); if (isErrorResult(twilioTextResult)) { handleError(twilioTextResult.error); } else { - successAlert(`Configure messaging provider saved successfully.`); + successAlert(`Messaging provider saved successfully.`); setMessagingValue(value); } } @@ -118,36 +127,38 @@ const MessagingConfiguration = () => { > - Mailgun email + Mailgun Email - Twilio email + Twilio Email - Twilio sms + Twilio SMS - {messagingValue === "mailgun-email" ? ( + {messagingValue === messagingProviders.mailgun ? ( ) : null} - {messagingValue === "twilio-email" ? ( + {messagingValue === messagingProviders.twilio_email ? ( ) : null} - {messagingValue === "twilio-text" ? : null} + {messagingValue === messagingProviders.twilio_text ? ( + + ) : null} ); diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx index 1aa2083c15..6f71480d34 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/S3StorageConfiguration.tsx @@ -1,31 +1,38 @@ -import { Button, Divider, Heading, Stack } from "@fidesui/react"; +import { Box, Button, Divider, Heading, Stack } from "@fidesui/react"; import { Form, Formik } from "formik"; import { useState } from "react"; import { CustomSelect, CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; +import { storageTypes } from "~/features/privacy-requests/constants"; import { useCreateStorageMutation, useCreateStorageSecretsMutation, } from "~/features/privacy-requests/privacy-requests.slice"; +interface SavedStorageDetails { + storageDetails: { + details: { + auth_method: string; + bucket: string; + }; + format: string; + }; +} interface StorageDetails { - type: string; - details: { + storageDetails: { auth_method: string; bucket: string; + format: string; }; - format: string; } interface SecretsStorageData { aws_access_key_id: string; aws_secret_access_key: string; } -const S3StorageConfiguration = ({ - storageDetails: { auth_method, bucket, format }, -}: any) => { +const S3StorageConfiguration = ({ storageDetails }: SavedStorageDetails) => { const [authMethod, setAuthMethod] = useState(""); const [saveStorageDetails] = useCreateStorageMutation(); const [setStorageSecrets] = useCreateStorageSecretsMutation(); @@ -34,12 +41,10 @@ const S3StorageConfiguration = ({ const { successAlert } = useAlert(); const initialValues = { - type: "s3", - details: { - auth_method: auth_method ?? "", - bucket: bucket ?? "", - }, - format: format ?? "", + type: storageTypes.s3, + auth_method: storageDetails?.details?.auth_method ?? "", + bucket: storageDetails?.details?.bucket ?? "", + format: storageDetails?.format ?? "", }; const initialSecretValues = { @@ -48,13 +53,13 @@ const S3StorageConfiguration = ({ }; const handleSubmitStorageConfiguration = async ( - newValues: StorageDetails + newValues: StorageDetails["storageDetails"] ) => { const result = await saveStorageDetails({ - type: "s3", + type: storageTypes.s3, details: { - auth_method: newValues.details.auth_method, - bucket: newValues.details.bucket, + auth_method: newValues.auth_method, + bucket: newValues.bucket, }, format: newValues.format, }); @@ -62,15 +67,18 @@ const S3StorageConfiguration = ({ if (isErrorResult(result)) { handleError(result.error); } else { - setAuthMethod(newValues.details.auth_method); + setAuthMethod(newValues.auth_method); successAlert(`S3 storage credentials successfully updated.`); } }; const handleSubmitStorageSecrets = async (newValues: SecretsStorageData) => { const result = await setStorageSecrets({ - aws_access_key_id: newValues.aws_access_key_id, - aws_secret_access_key: newValues.aws_secret_access_key, + details: { + aws_access_key_id: newValues.aws_access_key_id, + aws_secret_access_key: newValues.aws_secret_access_key, + }, + type: storageTypes.s3, }); if (isErrorResult(result)) { @@ -89,8 +97,9 @@ const S3StorageConfiguration = ({ - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
- + + + +
)} diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx index 49ed5bee75..0a3a2d75c6 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/StorageConfiguration.tsx @@ -10,14 +10,16 @@ import { Text, } from "@fidesui/react"; import NextLink from "next/link"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import Layout from "~/features/common/Layout"; +import { storageTypes } from "~/features/privacy-requests/constants"; import { useCreateConfigurationSettingsMutation, useCreateStorageMutation, + useGetActiveStorageQuery, useGetStorageDetailsQuery, } from "~/features/privacy-requests/privacy-requests.slice"; @@ -27,37 +29,43 @@ const StorageConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); const [storageValue, setStorageValue] = useState(""); - const [saveStorageType, { isLoading }] = useCreateStorageMutation(); - const [saveActiveStorage] = useCreateConfigurationSettingsMutation(); + + const { data: activeStorage } = useGetActiveStorageQuery(); const { data: storageDetails } = useGetStorageDetailsQuery({ type: storageValue, }); + const [saveStorageType, { isLoading }] = useCreateStorageMutation(); + const [saveActiveStorage] = useCreateConfigurationSettingsMutation(); + + useEffect(() => { + if (activeStorage) { + setStorageValue(activeStorage.type); + } + }, [activeStorage]); const handleChange = async (value: string) => { - if (value === "local") { + if (value === storageTypes.local) { const storageDetailsResult = await saveStorageType({ type: value, + details: {}, format: "json", }); if (isErrorResult(storageDetailsResult)) { handleError(storageDetailsResult.error); } else { - successAlert(`Configure storage type details saved successfully.`); + successAlert(`Configured storage details successfully.`); } } const activeStorageResults = await saveActiveStorage({ - fides: { - storage: { - active_default_storage_type: value, - }, + storage: { + active_default_storage_type: value, }, }); if (isErrorResult(activeStorageResults)) { handleError(activeStorageResults.error); } else { - successAlert(`Configure active storage type saved successfully.`); setStorageValue(value); } }; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx index ce828e184c..94f3adb9e3 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx @@ -5,6 +5,7 @@ import { useState } from "react"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; +import { messagingProviders } from "~/features/privacy-requests/constants"; import { useCreateMessagingConfigurationMutation, useCreateMessagingConfigurationSecretsMutation, @@ -16,7 +17,7 @@ const TwilioEmailConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ - type: "twilio_email", + type: messagingProviders.twilio_email, }); const [createMessagingConfiguration] = useCreateMessagingConfigurationMutation(); @@ -25,7 +26,7 @@ const TwilioEmailConfiguration = () => { const handleTwilioEmailConfiguration = async (value: { email: string }) => { const result = await createMessagingConfiguration({ - type: "twilio_email", + service_type: messagingProviders.twilio_email, details: { twilio_email_from: value.email, }, @@ -45,7 +46,10 @@ const TwilioEmailConfiguration = () => { api_key: string; }) => { const result = await createMessagingConfigurationSecrets({ - twilio_api_key: value.api_key, + details: { + twilio_api_key: value.api_key, + }, + service_type: messagingProviders.twilio_email, }); if (isErrorResult(result)) { @@ -57,11 +61,11 @@ const TwilioEmailConfiguration = () => { }; const initialValues = { - email: messagingDetails?.email ?? "", + email: messagingDetails?.details.twilio_email_from ?? "", }; const initialAPIKeyValues = { - api_key: messagingDetails?.key ?? "", + api_key: "", }; return ( @@ -73,19 +77,21 @@ const TwilioEmailConfiguration = () => { - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
- + + + + )}
diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx index be088e52d8..97e5191380 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx @@ -4,17 +4,12 @@ import { Form, Formik } from "formik"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; -import { - useCreateMessagingConfigurationSecretsMutation, - useGetMessagingConfigurationDetailsQuery, -} from "~/features/privacy-requests/privacy-requests.slice"; +import { messagingProviders } from "~/features/privacy-requests/constants"; +import { useCreateMessagingConfigurationSecretsMutation } from "~/features/privacy-requests/privacy-requests.slice"; const TwilioSMSConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); - const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ - type: "twilio_text", - }); const [createMessagingConfigurationSecrets] = useCreateMessagingConfigurationSecretsMutation(); @@ -25,10 +20,13 @@ const TwilioSMSConfiguration = () => { phone: string; }) => { const result = await createMessagingConfigurationSecrets({ - twilio_account_sid: value.account_sid, - twilio_auth_token: value.auth_token, - twilio_messaging_service_sid: value.messaging_service_sid, - twilio_sender_phone_number: value.phone, + details: { + twilio_account_sid: value.account_sid, + twilio_auth_token: value.auth_token, + twilio_messaging_service_sid: value.messaging_service_sid, + twilio_sender_phone_number: value.phone, + }, + service_type: messagingProviders.twilio_text, }); if (isErrorResult(result)) { @@ -39,10 +37,10 @@ const TwilioSMSConfiguration = () => { }; const initialValues = { - account_sid: messagingDetails?.twilio_account_sid ?? "", - auth_token: messagingDetails?.twilio_auth_token ?? "", - messaging_service_sid: messagingDetails?.twilio_messaging_service_sid ?? "", - phone: messagingDetails?.twilio_sender_phone_number ?? "", + account_sid: "", + auth_token: "", + messaging_service_sid: "", + phone: "", }; return ( @@ -54,20 +52,23 @@ const TwilioSMSConfiguration = () => { - {({ isSubmitting, resetForm }) => ( + {({ isSubmitting, handleReset }) => (
{ + + +
+ )} + + + + ); +}; + +export default TestMessagingProviderConnectionButton; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx index 94f3adb9e3..a62f007096 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioEmailConfiguration.tsx @@ -12,6 +12,8 @@ import { useGetMessagingConfigurationDetailsQuery, } from "~/features/privacy-requests/privacy-requests.slice"; +import TestMessagingProviderConnectionButton from "./TestMessagingProviderConnectionButton"; + const TwilioEmailConfiguration = () => { const [configurationStep, setConfigurationStep] = useState(""); const { successAlert } = useAlert(); @@ -56,7 +58,7 @@ const TwilioEmailConfiguration = () => { handleError(result.error); } else { successAlert(`Twilio email secrets successfully updated.`); - setConfigurationStep("configureTwilioEmailSecrets"); + setConfigurationStep("testConnection"); } }; @@ -112,7 +114,8 @@ const TwilioEmailConfiguration = () => { )} - {configurationStep === "configureTwilioEmailSecrets" ? ( + {configurationStep === "configureTwilioEmailSecrets" || + configurationStep === "testConnection" ? ( <> @@ -158,6 +161,11 @@ const TwilioEmailConfiguration = () => { ) : null} + {configurationStep === "testConnection" ? ( + + ) : null} ); }; diff --git a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx index 97e5191380..82fe82f096 100644 --- a/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx +++ b/clients/admin-ui/src/features/privacy-requests/configuration/TwilioSMS.tsx @@ -1,15 +1,25 @@ import { Box, Button, Heading, Stack } from "@fidesui/react"; import { Form, Formik } from "formik"; +import { useState } from "react"; import { CustomTextInput } from "~/features/common/form/inputs"; import { isErrorResult } from "~/features/common/helpers"; import { useAlert, useAPIHelper } from "~/features/common/hooks"; import { messagingProviders } from "~/features/privacy-requests/constants"; -import { useCreateMessagingConfigurationSecretsMutation } from "~/features/privacy-requests/privacy-requests.slice"; +import { + useCreateMessagingConfigurationSecretsMutation, + useGetMessagingConfigurationDetailsQuery, +} from "~/features/privacy-requests/privacy-requests.slice"; + +import TestMessagingProviderConnectionButton from "./TestMessagingProviderConnectionButton"; const TwilioSMSConfiguration = () => { const { successAlert } = useAlert(); const { handleError } = useAPIHelper(); + const [configurationStep, setConfigurationStep] = useState(""); + const { data: messagingDetails } = useGetMessagingConfigurationDetailsQuery({ + type: "twilio_text", + }); const [createMessagingConfigurationSecrets] = useCreateMessagingConfigurationSecretsMutation(); @@ -33,6 +43,7 @@ const TwilioSMSConfiguration = () => { handleError(result.error); } else { successAlert(`Twilio text secrets successfully updated.`); + setConfigurationStep("testConnection"); } }; @@ -104,6 +115,11 @@ const TwilioSMSConfiguration = () => { )} + {configurationStep === "testConnection" ? ( + + ) : null} ); }; diff --git a/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts b/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts index d4b67a75dc..db385c62ef 100644 --- a/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts +++ b/clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts @@ -416,6 +416,13 @@ export const privacyRequestApi = createApi({ body: params.details, }), }), + createTestConnectionMessage: build.mutation({ + query: (params) => ({ + url: `messaging/config/test`, + method: "POST", + body: params, + }), + }), uploadManualWebhookData: build.mutation< any, PatchUploadManualWebhookDataRequest @@ -449,4 +456,5 @@ export const { useGetActiveStorageQuery, useCreateMessagingConfigurationMutation, useCreateMessagingConfigurationSecretsMutation, + useCreateTestConnectionMessageMutation, } = privacyRequestApi; From 987b05bcda276054f8a2a4a95a333bb1249c813d Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 5 Mar 2023 21:30:38 -0800 Subject: [PATCH 18/26] Overhaul CLI documentation and aesthetics (#2703) --- .fides/systems.yml | 2 +- CHANGELOG.md | 1 + requirements.txt | 2 +- src/fides/cli/__init__.py | 21 ++++-- src/fides/cli/cli_formatting.py | 114 +++++++++++++++++++++++++++++ src/fides/cli/commands/annotate.py | 10 +-- src/fides/cli/commands/core.py | 49 +++++-------- src/fides/cli/commands/crud.py | 33 +++++++-- src/fides/cli/commands/db.py | 8 +- src/fides/cli/commands/export.py | 20 ++--- src/fides/cli/commands/generate.py | 41 +++-------- src/fides/cli/commands/scan.py | 36 +++------ src/fides/cli/commands/user.py | 12 +-- src/fides/cli/commands/util.py | 30 ++++---- src/fides/cli/commands/view.py | 29 +++++++- src/fides/cli/options.py | 41 ++++++----- src/fides/cli/utils.py | 2 +- tests/ctl/cli/test_cli.py | 51 ++++++++++--- 18 files changed, 323 insertions(+), 179 deletions(-) create mode 100644 src/fides/cli/cli_formatting.py diff --git a/.fides/systems.yml b/.fides/systems.yml index e5d36896f2..429873ca8f 100644 --- a/.fides/systems.yml +++ b/.fides/systems.yml @@ -28,7 +28,7 @@ system: - fides_db # System Info - - fides_key: privacy_request_fullfillment + - fides_key: privacy_request_fulfillment name: Fides Privacy Request Fulfillment organization_fides_key: default_organization description: Privacy request fufillment. diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cfd6c1c5..8d66dfed1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ The types of changes are: * Add warning to 'fides deploy' when installed outside of a virtual environment [#2641](https://github.com/ethyca/fides/pull/2641) * Redesigned the default/init config file to be auto-documented. Also updates the `fides init` logic and analytics consent logic [#2694](https://github.com/ethyca/fides/pull/2694) * Change how config creation/import is handled across the application [#2622](https://github.com/ethyca/fides/pull/2622) +* Update the CLI aesthetics & docstrings [#2703](https://github.com/ethyca/fides/pull/2703) * Updates Roles->Scopes Mapping [#2744](https://github.com/ethyca/fides/pull/2744) * Return user scopes as an enum, as well as total scopes [#2741](https://github.com/ethyca/fides/pull/2741) diff --git a/requirements.txt b/requirements.txt index d4be59d3b8..ad520d2842 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,6 @@ APScheduler==3.9.1.post1 asyncpg==0.25.0 boto3==1.26.1 celery[pytest]==5.2.7 -click==8.1.3 colorama>=0.4.3 cryptography==38.0.3 dask==2022.9.2 @@ -35,6 +34,7 @@ PyMySQL==1.0.2 python-jose[cryptography]==3.3.0 pyyaml>=5,<6 redis==3.5.3 +rich-click==1.6.1 sendgrid==6.9.7 slowapi==0.1.5 snowflake-sqlalchemy==1.4.3 diff --git a/src/fides/cli/__init__.py b/src/fides/cli/__init__.py index efe8d2714f..fe02a594fc 100644 --- a/src/fides/cli/__init__.py +++ b/src/fides/cli/__init__.py @@ -1,14 +1,17 @@ -"""Contains the groups and setup for the CLI.""" +""" +Entrypoint for the Fides command-line. +""" from importlib.metadata import version from platform import system -import click +import rich_click as click from fideslog.sdk.python.client import AnalyticsClient import fides from fides.cli.utils import check_server from fides.core.config import get_config +from . import cli_formatting from .commands.annotate import annotate from .commands.core import evaluate, parse, pull, push from .commands.crud import delete, get_resource, list_resources @@ -58,24 +61,28 @@ "--config-path", "-f", "config_path", - default="", - help="Path to a configuration file. Use 'fides view-config' to print the config. Not compatible with the 'fides webserver' subcommand.", + show_default=True, + help="Path to a Fides config file. _Defaults to `.fides/fides.toml`._", ) @click.option( "--local", is_flag=True, - help="Run in 'local_mode'. This mode doesn't make API calls and can be used without the API server/database.", + help="Run in `local_mode`. This mode doesn't make API calls and can be used without the API server/database.", ) @click.pass_context def cli(ctx: click.Context, config_path: str, local: bool) -> None: """ - The parent group for the Fides CLI. + __Command-line tool for the Fides privacy engineering platform.__ + + --- + + _Note: The common MANIFESTS_DIR argument _always_ defaults to ".fides/" if not specified._ """ ctx.ensure_object(dict) config = get_config(config_path, verbose=True) - # Dyanmically add commands to the CLI + # Dynamically add commands to the CLI cli.commands = LOCAL_COMMAND_DICT if not (local or config.cli.local_mode): diff --git a/src/fides/cli/cli_formatting.py b/src/fides/cli/cli_formatting.py new file mode 100644 index 0000000000..1da85f45f2 --- /dev/null +++ b/src/fides/cli/cli_formatting.py @@ -0,0 +1,114 @@ +""" +This is a configuration file for `rich_click`, used to customize the +visual aesthetic and output of the CLI. + +This is the list of all documented configuration options for `rich_click`. + +You can set a style attribute by adding one or more of the following words: + +- "bold" or "b" for bold text. +- "blink" for text that flashes (use this one sparingly). +- "blink2" for text that flashes rapidly (not supported by most terminals). +- "conceal" for concealed text (not supported by most terminals). +- "italic" or "i" for italic text (not supported on Windows). +- "reverse" or "r" for text with foreground and background colors reversed. +- "strike" or "s" for text with a line through it. +- "underline" or "u" for underlined text. +- "underline2" or "uu" for doubly underlined text. +- "frame" for framed text. +- "encircle" for encircled text. +- "overline" or "o" for overlined text. + +The list of valid colors is here: +- https://rich.readthedocs.io/en/stable/appendix/colors.html +""" + +from rich_click import rich_click + +# Default styles +rich_click.STYLE_OPTION = "bold #fca311" +rich_click.STYLE_SWITCH = "bold #fca311" +rich_click.STYLE_ARGUMENT = "bold #00ff5f" +rich_click.STYLE_METAVAR = "bold #8700af" +rich_click.STYLE_METAVAR_APPEND = "dim yellow" +rich_click.STYLE_METAVAR_SEPARATOR = "dim" +rich_click.STYLE_HEADER_TEXT = "" +rich_click.STYLE_FOOTER_TEXT = "" +rich_click.STYLE_USAGE = "yellow" +rich_click.STYLE_USAGE_COMMAND = "bold" +rich_click.STYLE_DEPRECATED = "red" +rich_click.STYLE_HELPTEXT_FIRST_LINE = "" +rich_click.STYLE_HELPTEXT = "" +rich_click.STYLE_OPTION_HELP = "" +rich_click.STYLE_OPTION_DEFAULT = "bold #8700af" +rich_click.STYLE_OPTION_ENVVAR = "dim yellow" +rich_click.STYLE_REQUIRED_SHORT = "red" +rich_click.STYLE_REQUIRED_LONG = "dim red" +rich_click.STYLE_OPTIONS_PANEL_BORDER = "dim" +rich_click.ALIGN_OPTIONS_PANEL = "left" +rich_click.STYLE_OPTIONS_TABLE_SHOW_LINES = False +rich_click.STYLE_OPTIONS_TABLE_LEADING = 0 +rich_click.STYLE_OPTIONS_TABLE_PAD_EDGE = False +rich_click.STYLE_OPTIONS_TABLE_PADDING = (0, 1) +rich_click.STYLE_OPTIONS_TABLE_BOX = "" +rich_click.STYLE_OPTIONS_TABLE_ROW_STYLES = None +rich_click.STYLE_OPTIONS_TABLE_BORDER_STYLE = None +rich_click.STYLE_COMMANDS_PANEL_BORDER = "dim" +rich_click.ALIGN_COMMANDS_PANEL = "left" +rich_click.STYLE_COMMANDS_TABLE_SHOW_LINES = False +rich_click.STYLE_COMMANDS_TABLE_LEADING = 0 +rich_click.STYLE_COMMANDS_TABLE_PAD_EDGE = False +rich_click.STYLE_COMMANDS_TABLE_PADDING = (0, 1) +rich_click.STYLE_COMMANDS_TABLE_BOX = "" +rich_click.STYLE_COMMANDS_TABLE_ROW_STYLES = None +rich_click.STYLE_COMMANDS_TABLE_BORDER_STYLE = None +rich_click.STYLE_ERRORS_PANEL_BORDER = "red" +rich_click.ALIGN_ERRORS_PANEL = "left" +rich_click.STYLE_ERRORS_SUGGESTION = "dim" +rich_click.STYLE_ABORTED = "red" +rich_click.MAX_WIDTH = None # Set to an int to limit to that many characters +rich_click.COLOR_SYSTEM = "auto" # Set to None to disable colors + +# Fixed strings +rich_click.HEADER_TEXT = None +rich_click.FOOTER_TEXT = None +rich_click.DEPRECATED_STRING = "(Deprecated) " +rich_click.DEFAULT_STRING = "[default: {}]" +rich_click.ENVVAR_STRING = "[env var: {}]" +rich_click.REQUIRED_SHORT_STRING = "*" +rich_click.REQUIRED_LONG_STRING = "[required]" +rich_click.RANGE_STRING = " [{}]" +rich_click.APPEND_METAVARS_HELP_STRING = "({})" +rich_click.ARGUMENTS_PANEL_TITLE = "Arguments" +rich_click.OPTIONS_PANEL_TITLE = "Options" +rich_click.COMMANDS_PANEL_TITLE = "Commands" +rich_click.ERRORS_PANEL_TITLE = "Error" +rich_click.ERRORS_SUGGESTION = ( + None # Default: Try 'cmd -h' for help. Set to False to disable. +) +rich_click.ERRORS_EPILOGUE = None +rich_click.ABORTED_TEXT = "Aborted." + +# Behaviours +rich_click.SHOW_ARGUMENTS = True # Show positional arguments +rich_click.SHOW_METAVARS_COLUMN = ( + True # Show a column with the option metavar (eg. INTEGER) +) +rich_click.APPEND_METAVARS_HELP = ( + False # Append metavar (eg. [TEXT]) after the help text +) +rich_click.GROUP_ARGUMENTS_OPTIONS = ( + False # Show arguments with options instead of in own panel +) +rich_click.USE_MARKDOWN = True # Parse help strings as markdown +rich_click.USE_MARKDOWN_EMOJI = True # Parse emoji codes in markdown :smile: +rich_click.USE_RICH_MARKUP = ( + False # Parse help strings for rich markup (eg. [red]my text[/]) +) +rich_click.COMMAND_GROUPS = {} # Define sorted groups of panels to display subcommands +rich_click.OPTION_GROUPS = ( + {} +) # Define sorted groups of panels to display options and arguments +rich_click.USE_CLICK_SHORT_HELP = ( + False # Use click's default function to truncate help text +) diff --git a/src/fides/cli/commands/annotate.py b/src/fides/cli/commands/annotate.py index 4c6e390bee..95460b82d4 100644 --- a/src/fides/cli/commands/annotate.py +++ b/src/fides/cli/commands/annotate.py @@ -1,6 +1,6 @@ """Contains the annotate group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.utils import with_analytics from fides.core import annotate_dataset as _annotate_dataset @@ -10,7 +10,7 @@ @click.pass_context def annotate(ctx: click.Context) -> None: """ - Annotate fides resource types + Interactively annotate Fides resources. """ @@ -21,21 +21,21 @@ def annotate(ctx: click.Context) -> None: "-a", "--all-members", is_flag=True, - help="Annotate all dataset members, not just fields", + help="Annotate all parts of the dataset including schemas and tables.", ) @click.option( "-v", "--validate", is_flag=True, default=False, - help="Strictly validate annotation inputs.", + help="Validate annotation inputs.", ) @with_analytics def annotate_dataset( ctx: click.Context, input_filename: str, all_members: bool, validate: bool ) -> None: """ - Guided flow for annotating datasets. The dataset file will be edited in-place. + Interactively annotate a dataset file in-place. """ config = ctx.obj["CONFIG"] _annotate_dataset.annotate_dataset( diff --git a/src/fides/cli/commands/core.py b/src/fides/cli/commands/core.py index e929322fc7..7c80437d6e 100644 --- a/src/fides/cli/commands/core.py +++ b/src/fides/cli/commands/core.py @@ -1,14 +1,9 @@ """Contains all of the core CLI commands for fides.""" from typing import Optional -import click +import rich_click as click -from fides.cli.options import ( - dry_flag, - fides_key_option, - manifests_dir_argument, - verbose_flag, -) +from fides.cli.options import dry_flag, manifests_dir_argument, verbose_flag from fides.cli.utils import pretty_echo, print_divider, with_analytics from fides.core import audit as _audit from fides.core import evaluate as _evaluate @@ -24,13 +19,13 @@ @click.option( "--diff", is_flag=True, - help="Include any changes between server and local resources in the command output", + help="Print any diffs between the local & server objects", ) @manifests_dir_argument @with_analytics def push(ctx: click.Context, dry: bool, diff: bool, manifests_dir: str) -> None: """ - Validate local manifest files and persist any changes via the API server. + Parse local manifest files and upload them to the server. """ config = ctx.obj["CONFIG"] @@ -47,19 +42,28 @@ def push(ctx: click.Context, dry: bool, diff: bool, manifests_dir: str) -> None: @click.command() @click.pass_context @manifests_dir_argument -@fides_key_option +@click.option( + "--fides-key", + "-k", + help="The fides_key of a specific policy to evaluate.", + default="", +) @click.option( "-m", "--message", - help="A message that you can supply to describe the context of this evaluation.", + help="Describe the context of this evaluation.", ) @click.option( "-a", "--audit", is_flag=True, - help="Raise errors if resources are missing attributes required for building a data map.", + help="Validate that the objects in this evaluation produce a valid data map.", +) +@click.option( + "--dry", + is_flag=True, + help="Do not upload objects or results to the Fides webserver.", ) -@dry_flag @with_analytics def evaluate( ctx: click.Context, @@ -70,12 +74,7 @@ def evaluate( dry: bool, ) -> None: """ - Compare your System's Privacy Declarations with your Organization's Policy Rules. - - All local resources are applied to the server before evaluation. - - If your policy evaluation fails, it is expected that you will need to - either adjust your Privacy Declarations, Datasets, or Policies before trying again. + Evaluate System-level Privacy Declarations against Organization-level Policy Rules. """ config = ctx.obj["CONFIG"] @@ -127,10 +126,7 @@ def evaluate( @with_analytics def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None: """ - Reads the resource files that are stored in MANIFESTS_DIR and its subdirectories to verify - the validity of all manifest files. - - If the taxonomy is invalid, this command prints the error messages and triggers a non-zero exit code. + Parse all Fides objects located in the supplied directory. """ taxonomy = _parse.parse(manifests_dir=manifests_dir) if verbose: @@ -149,12 +145,7 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @with_analytics def pull(ctx: click.Context, manifests_dir: str, all_resources: Optional[str]) -> None: """ - Update local resource files by their fides_key to match their server versions. - - Alternatively, with the "--all" flag all resources from the server will be pulled - down into a local file. - - The pull is aborted if there are unstaged or untracked files in the manifests dir. + Update local resource files based on the state of the objects on the server. """ # Make the resources that are pulled configurable diff --git a/src/fides/cli/commands/crud.py b/src/fides/cli/commands/crud.py index a2826fbcbc..cdd0578c67 100644 --- a/src/fides/cli/commands/crud.py +++ b/src/fides/cli/commands/crud.py @@ -1,12 +1,12 @@ """Contains all of the CRUD-type CLI commands for fides.""" -import click +import rich_click as click import yaml from fides.cli.options import fides_key_argument, resource_type_argument from fides.cli.utils import handle_cli_response, print_divider, with_analytics from fides.core import api as _api from fides.core.api_helpers import get_server_resource, list_server_resources -from fides.core.utils import echo_green +from fides.core.utils import echo_green, echo_red @click.command() @@ -16,7 +16,7 @@ @with_analytics def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: """ - Delete a resource on the server. + Delete an object from the server. """ config = ctx.obj["CONFIG"] handle_cli_response( @@ -25,7 +25,11 @@ def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: resource_type=resource_type, resource_id=fides_key, headers=config.user.auth_header, - ) + ), + verbose=False, + ) + echo_green( + f"{resource_type.capitalize()} with fides_key '{fides_key}' successfully deleted." ) @@ -36,7 +40,7 @@ def delete(ctx: click.Context, resource_type: str, fides_key: str) -> None: @with_analytics def get_resource(ctx: click.Context, resource_type: str, fides_key: str) -> None: """ - View a resource from the server as a YAML object. + View an object from the server. """ config = ctx.obj["CONFIG"] resource = get_server_resource( @@ -54,9 +58,12 @@ def get_resource(ctx: click.Context, resource_type: str, fides_key: str) -> None @click.pass_context @resource_type_argument @with_analytics -def list_resources(ctx: click.Context, resource_type: str) -> None: +@click.option( + "--verbose", "-v", is_flag=True, help="Displays the entire object list as YAML." +) +def list_resources(ctx: click.Context, verbose: bool, resource_type: str) -> None: """ - Get a list of all resources of this type from the server and display them as YAML. + View all objects of a single type from the server. """ config = ctx.obj["CONFIG"] resources = list_server_resources( @@ -67,4 +74,14 @@ def list_resources(ctx: click.Context, resource_type: str) -> None: raw=True, ) print_divider() - echo_green(yaml.dump({resource_type: resources})) + if verbose: + echo_green(yaml.dump({resource_type: resources})) + else: + if resources: + sorted_fides_keys = sorted( + {resource["fides_key"] for resource in resources if resource} + ) + formatted_fides_keys = "\n ".join(sorted_fides_keys) + echo_green(f"{resource_type.capitalize()} list:\n {formatted_fides_keys}") + else: + echo_red(f"No {resource_type.capitalize()} resources found!") diff --git a/src/fides/cli/commands/db.py b/src/fides/cli/commands/db.py index 7f27a466a7..0d8d45faeb 100644 --- a/src/fides/cli/commands/db.py +++ b/src/fides/cli/commands/db.py @@ -1,5 +1,5 @@ """Contains the db group of the commands for fides.""" -import click +import rich_click as click from fides.cli.options import yes_flag from fides.cli.utils import handle_cli_response, with_analytics @@ -11,7 +11,7 @@ @click.pass_context def database(ctx: click.Context) -> None: """ - Database utility commands + Run actions against the application database. """ @@ -20,7 +20,7 @@ def database(ctx: click.Context) -> None: @with_analytics def db_init(ctx: click.Context) -> None: """ - Initialize the fides database. + Initialize the Fides database. """ config = ctx.obj["CONFIG"] handle_cli_response( @@ -38,7 +38,7 @@ def db_init(ctx: click.Context) -> None: @with_analytics def db_reset(ctx: click.Context, yes: bool) -> None: """ - Wipes all user-created data and resets the database back to its freshly initialized state. + Reset the database back to its initial state. """ config = ctx.obj["CONFIG"] if yes: diff --git a/src/fides/cli/commands/export.py b/src/fides/cli/commands/export.py index cc6daebb39..5d6879f944 100644 --- a/src/fides/cli/commands/export.py +++ b/src/fides/cli/commands/export.py @@ -1,5 +1,5 @@ """Contains the export group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( dry_flag, @@ -16,7 +16,7 @@ @click.pass_context def export(ctx: click.Context) -> None: """ - Export fides resource types + Export Fides data maps. """ @@ -31,7 +31,7 @@ def export_system( dry: bool, ) -> None: """ - Export a system in a data map format. + Export systems in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -55,7 +55,7 @@ def export_dataset( dry: bool, ) -> None: """ - Export a dataset in a data map format. + Export datasets in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -79,7 +79,7 @@ def export_organization( dry: bool, ) -> None: """ - Export an organization in a data map format. + Export organizations in a data map format. """ config = ctx.obj["CONFIG"] taxonomy = _parse.parse(manifests_dir) @@ -111,17 +111,9 @@ def export_datamap( csv: bool, ) -> None: """ - Export a formatted data map to excel using the fides template. + Export a data map using the standard Fides template. The data map is comprised of an Organization, Systems, and Datasets. - - The default organization is used, however a custom one can be - passed if required. - - A custom manifest directory can be provided for the output location. - - The csv flag can be used to output data as csv, while the dry - flag can be used to return data to the console instead. """ config = ctx.obj["CONFIG"] _export.export_datamap( diff --git a/src/fides/cli/commands/generate.py b/src/fides/cli/commands/generate.py index edb5036853..5cd8d64392 100644 --- a/src/fides/cli/commands/generate.py +++ b/src/fides/cli/commands/generate.py @@ -1,5 +1,5 @@ """Contains the generate group of CLI commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( aws_access_key_id_option, @@ -27,7 +27,7 @@ @click.pass_context def generate(ctx: click.Context) -> None: """ - Generate fides resource types + Programmatically generate Fides objects. """ @@ -35,7 +35,7 @@ def generate(ctx: click.Context) -> None: @click.pass_context def generate_dataset(ctx: click.Context) -> None: """ - Generate fides Dataset resources + Generate Fides datasets. """ @@ -54,13 +54,7 @@ def generate_dataset_db( include_null: bool, ) -> None: """ - Connect to a database directly via a SQLAlchemy-style connection string and - generate a dataset manifest file that consists of every schema/table/field. - Connection string can be supplied as an option or a credentials reference - to fides config. - - This is a one-time operation that does not track the state of the database. - It will need to be run again if the database schema changes. + Generate a Fides dataset by walking a database and recording every schema/table/field. """ actual_connection_string = handle_database_credentials_options( fides_config=ctx.obj["CONFIG"], @@ -79,7 +73,7 @@ def generate_dataset_db( @click.pass_context def generate_dataset_gcp(ctx: click.Context) -> None: """ - Generate fides Dataset resources for Google Cloud Platform + Generate Fides datasets from Google Cloud Platform. """ @@ -100,13 +94,7 @@ def generate_dataset_bigquery( include_null: bool, ) -> None: """ - Connect to a BigQuery dataset directly via a SQLAlchemy connection and - generate a dataset manifest file that consists of every schema/table/field. - A path to a google authorization keyfile can be supplied as an option, or a - credentials reference to fides config. - - This is a one-time operation that does not track the state of the dataset. - It will need to be run again if the dataset schema changes. + Generate a dataset object from BigQuery using a SQLAlchemy connection string. """ bigquery_config = handle_bigquery_config_options( @@ -127,7 +115,7 @@ def generate_dataset_bigquery( @click.pass_context def generate_system(ctx: click.Context) -> None: """ - Generate fides System resources + Generate Fides systems. """ @@ -150,13 +138,8 @@ def generate_system_okta( org_key: str, ) -> None: """ - Generates systems for your Okta applications. Connect to an Okta admin - account by providing an organization url and auth token or a credentials - reference to fides config. Auth token and organization url can also - be supplied by setting environment variables as defined by the okta python sdk. - - This is a one-time operation that does not track the state of the okta resources. - It will need to be run again if the tracked resources change. + Generates systems from your Okta applications. Connects via + an Okta admin account. """ config = ctx.obj["CONFIG"] okta_config = handle_okta_credentials_options( @@ -199,12 +182,6 @@ def generate_system_aws( """ Connect to an aws account and generate a system manifest file that consists of every tracked resource. - Credentials can be supplied as options, a credentials - reference to fides config, or boto3 environment configuration. - Tracked resources: [Redshift, RDS, DynamoDb, S3] - - This is a one-time operation that does not track the state of the aws resources. - It will need to be run again if the tracked resources change. """ config = ctx.obj["CONFIG"] aws_config = handle_aws_credentials_options( diff --git a/src/fides/cli/commands/scan.py b/src/fides/cli/commands/scan.py index 5fa89e3f84..9782069b01 100644 --- a/src/fides/cli/commands/scan.py +++ b/src/fides/cli/commands/scan.py @@ -1,6 +1,6 @@ """Contains the scan group of the commands for fides.""" -import click +import rich_click as click from fides.cli.options import ( aws_access_key_id_option, @@ -28,7 +28,7 @@ @click.pass_context def scan(ctx: click.Context) -> None: """ - Scan external resource coverage against fides resources + Scan and report on discrepancies between Fides resource files and real infrastructure. """ @@ -36,7 +36,7 @@ def scan(ctx: click.Context) -> None: @click.pass_context def scan_dataset(ctx: click.Context) -> None: """ - Scan fides Dataset resources + Scan and report on Fides Dataset resources. """ @@ -55,15 +55,10 @@ def scan_dataset_db( coverage_threshold: int, ) -> None: """ - Connect to a database directly via a SQLAlchemy-style connection string and - compare the database objects to existing datasets. Connection string can be - supplied as an option or a credentials reference to fides config. + Scan a database directly using a SQLAlchemy-style connection string. - If there are fields within the database that aren't listed and categorized - within one of the datasets, this counts as lacking coverage. - - Outputs missing fields and has a non-zero exit if coverage is - under the stated threshold. + _If there are fields within the database that aren't listed and categorized + within one of the datasets, this counts as lacking coverage._ """ config = ctx.obj["CONFIG"] actual_connection_string = handle_database_credentials_options( @@ -85,7 +80,7 @@ def scan_dataset_db( @click.pass_context def scan_system(ctx: click.Context) -> None: """ - Scan fides System resources + Scan and report on Fides System resources. """ @@ -108,14 +103,7 @@ def scan_system_okta( coverage_threshold: int, ) -> None: """ - Scans your existing systems and compares them to found Okta applications. - Connect to an Okta admin account by providing an organization url and - auth token or a credentials reference to fides config. Auth token and - organization url can also be supplied by setting environment variables - as defined by the okta python sdk. - - Outputs missing resources and has a non-zero exit if coverage is - under the stated threshold. + Scan an Okta account and compare applications with annotated Fides Systems. """ config = ctx.obj["CONFIG"] @@ -154,13 +142,9 @@ def scan_system_aws( coverage_threshold: int, ) -> None: """ - Connect to an aws account and compares tracked resources to existing systems. - Credentials can be supplied as options, a credentials reference to fides - config, or boto3 environment configuration. - Tracked resources: [Redshift, RDS, DynamoDb, S3] + Scan an AWS account and compare objects with annotated Fides Systems. - Outputs missing resources and has a non-zero exit if coverage is - under the stated threshold. + _Scannable resources: [Redshift, RDS, DynamoDb, S3]_ """ config = ctx.obj["CONFIG"] aws_config = handle_aws_credentials_options( diff --git a/src/fides/cli/commands/user.py b/src/fides/cli/commands/user.py index 773712e547..395b1b7759 100644 --- a/src/fides/cli/commands/user.py +++ b/src/fides/cli/commands/user.py @@ -1,5 +1,5 @@ """Contains the user command group for the fides CLI.""" -import click +import rich_click as click from fides.cli.options import ( first_name_option, @@ -28,9 +28,7 @@ def create( ctx: click.Context, username: str, password: str, first_name: str, last_name: str ) -> None: """ - Use credentials from the credentials file to create a new user. - - Gives full permissions to the new user. + Use the credentials file to create a new user. Gives full permissions to the new user. """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url @@ -49,7 +47,7 @@ def create( @password_option def login(ctx: click.Context, username: str, password: str) -> None: """ - Use credentials to get a user access token and write it to a credentials file. + Generate a user access token and write it to a credentials file. """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url @@ -59,7 +57,9 @@ def login(ctx: click.Context, username: str, password: str) -> None: @user.command(name="permissions") @click.pass_context def get_permissions(ctx: click.Context) -> None: - """List the directly-assigned scopes and roles available to the current user.""" + """ + List the directly-assigned scopes and roles available to the current user. + """ config = ctx.obj["CONFIG"] server_url = config.cli.server_url get_permissions_command(server_url=server_url) diff --git a/src/fides/cli/commands/util.py b/src/fides/cli/commands/util.py index 2c1acc2475..f0e10df194 100644 --- a/src/fides/cli/commands/util.py +++ b/src/fides/cli/commands/util.py @@ -2,7 +2,7 @@ from datetime import datetime, timezone from subprocess import CalledProcessError -import click +import rich_click as click import fides from fides.cli.utils import ( @@ -28,16 +28,14 @@ @click.command() @click.pass_context -@click.argument("fides_directory_location", default=".", type=click.Path(exists=True)) +@click.argument("fides_dir", default=".", type=click.Path(exists=True)) @click.option( "--opt-in", is_flag=True, help="Automatically opt-in to anonymous usage analytics." ) -def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> None: +def init(ctx: click.Context, fides_dir: str, opt_in: bool) -> None: """ - Initializes a fides instance, creating the default directory (`.fides/`) and - the configuration file (`fides.toml`) if necessary. - - Additionally, requests the ability to respectfully collect anonymous usage data. + Initializes a Fides instance by creating the default directory and + configuration file if not present. """ executed_at = datetime.now(timezone.utc) @@ -47,7 +45,7 @@ def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> Non click.echo("Initializing fides...") config, config_path = create_and_update_config_file( - config, fides_directory_location, opt_in=opt_in + config, fides_dir, opt_in=opt_in ) print_divider() @@ -61,7 +59,7 @@ def init(ctx: click.Context, fides_directory_location: str, opt_in: bool) -> Non @with_analytics def status(ctx: click.Context) -> None: """ - Sends a request to the fides API healthcheck endpoint and prints the response. + Check Fides server availability. """ config = ctx.obj["CONFIG"] cli_version = fides.__version__ @@ -78,7 +76,9 @@ def status(ctx: click.Context) -> None: @click.option("--port", "-p", type=int, default=8080) def webserver(ctx: click.Context, port: int = 8080) -> None: """ - Starts the fides API server using Uvicorn. + Start the Fides webserver. + + _Requires Redis and Postgres to be configured and running_ """ # This has to be here to avoid a circular dependency from fides.api.main import start_webserver @@ -91,7 +91,7 @@ def webserver(ctx: click.Context, port: int = 8080) -> None: @with_analytics def worker(ctx: click.Context) -> None: """ - Starts a celery worker. + Start a Celery worker for the Fides webserver. """ # This has to be here to avoid a circular dependency from fides.api.ops.worker import start_worker @@ -123,9 +123,11 @@ def deploy(ctx: click.Context) -> None: is_flag=True, help="Disable the initialization of the Fides CLI, to run in headless mode.", ) -def up(ctx: click.Context, no_pull: bool = False, no_init: bool = False) -> None: +def up( + ctx: click.Context, no_pull: bool = False, no_init: bool = False +) -> None: # pragma: no cover """ - Starts the sample project via docker compose. + Starts a sample project via docker compose. """ check_virtualenv() @@ -138,7 +140,9 @@ def up(ctx: click.Context, no_pull: bool = False, no_init: bool = False) -> None try: check_fides_uploads_dir() + print("> Starting application...") start_application() + print("> Seeding data...") seed_example_data() click.clear() diff --git a/src/fides/cli/commands/view.py b/src/fides/cli/commands/view.py index fde0ab432c..c8711f53a3 100644 --- a/src/fides/cli/commands/view.py +++ b/src/fides/cli/commands/view.py @@ -1,9 +1,10 @@ """Contains the view group of the commands for fides.""" -import click -from toml import dumps as toml_dumps +import rich_click as click +from toml import dumps from fides.cli.utils import print_divider, with_analytics +from fides.core.utils import echo_red, get_credentials_path, read_credentials_file @click.group(name="view") @@ -27,7 +28,9 @@ def view_config( ctx: click.Context, section: str = "", exclude_unset: bool = False ) -> None: """ - Prints the configuration values being used. + Prints the configuration values being used for this command-line instance. + + _Note: To see the configuration values being used by the webserver, `GET` the `/api/v1/config` endpoint._ """ config = ctx.obj["CONFIG"] config_dict = config.dict(exclude_unset=exclude_unset) @@ -35,4 +38,22 @@ def view_config( config_dict = config_dict[section] print_divider() - print(toml_dumps(config_dict)) + print(dumps(config_dict)) + + +@view.command(name="credentials") +@click.pass_context +@with_analytics +def view_credentials(ctx: click.Context) -> None: + """ + Prints the credentials file. + """ + credentials_path = get_credentials_path() + try: + credentials = read_credentials_file(credentials_path) + except FileNotFoundError: + echo_red(f"No credentials file found at path: {credentials_path}") + raise SystemExit(1) + + print_divider() + print(dumps(credentials.dict())) diff --git a/src/fides/cli/options.py b/src/fides/cli/options.py index 5700f3ec83..6da30ac6c9 100644 --- a/src/fides/cli/options.py +++ b/src/fides/cli/options.py @@ -1,17 +1,21 @@ """ -Contains all of the options/arguments used by the CLI commands. +Reusable command-line arguments and options. """ from typing import Callable -import click +import rich_click as click from fideslang import model_list def coverage_threshold_option(command: Callable) -> Callable: - "Add a flag that assumes yes." + """An option decorator that sets a required coverage percentage.""" command = click.option( - "--coverage-threshold", "-c", type=click.IntRange(0, 100), default=100 + "--coverage-threshold", + "-c", + type=click.IntRange(0, 100), + default=100, + help="Set the coverage percentage for a passing scan.", )(command) return command @@ -40,7 +44,6 @@ def fides_key_option(command: Callable) -> Callable: "-k", "--fides-key", default="", - help="The fides_key of the single policy that you wish to evaluate.", )(command) return command @@ -48,9 +51,7 @@ def fides_key_option(command: Callable) -> Callable: def manifests_dir_argument(command: Callable) -> Callable: "Add the manifests_dir argument." command = click.argument( - "manifests_dir", - default=".fides/", - type=click.Path(exists=True), + "manifests_dir", type=click.Path(exists=True), default=".fides/" )(command) return command @@ -58,7 +59,7 @@ def manifests_dir_argument(command: Callable) -> Callable: def dry_flag(command: Callable) -> Callable: "Add a flag that prevents side-effects." command = click.option( - "--dry", is_flag=True, help="Prevent the persistance of any changes." + "--dry", is_flag=True, help="Do not upload results to the Fides webserver." )(command) return command @@ -69,7 +70,7 @@ def yes_flag(command: Callable) -> Callable: "--yes", "-y", is_flag=True, - help="Automatically responds 'yes' to any prompts.", + help="Automatically responds `yes` to any prompts.", )(command) return command @@ -90,7 +91,7 @@ def include_null_flag(command: Callable) -> Callable: command = click.option( "--include-null", is_flag=True, - help="Includes attributes that would otherwise be null.", + help="Include null attributes.", )(command) return command @@ -101,7 +102,8 @@ def organization_fides_key_option(command: Callable) -> Callable: "--org-key", "-k", default="default_organization", - help="The organization_fides_key you wish to export resources for.", + show_default=True, + help="The `organization_fides_key` of the `Organization` you want to specify.", )(command) return command @@ -112,6 +114,7 @@ def output_directory_option(command: Callable) -> Callable: "--output-dir", "-d", default=".fides/", + show_default=True, help="The output directory for the data map to be exported to.", )(command) return command @@ -122,7 +125,7 @@ def credentials_id_option(command: Callable) -> Callable: command = click.option( "--credentials-id", type=str, - help="Use credentials defined within fides config", + help="Use credentials keys defined within Fides config.", )(command) return command @@ -132,7 +135,7 @@ def connection_string_option(command: Callable) -> Callable: command = click.option( "--connection-string", type=str, - help="Use connection string option to connect to a database", + help="Use the connection string option to connect to a database.", )(command) return command @@ -142,7 +145,7 @@ def okta_org_url_option(command: Callable) -> Callable: command = click.option( "--org-url", type=str, - help="Use org url option to connect to okta. Requires options --org-url and --token", + help="Connect to Okta using an 'Org URL'. _Requires options `--org-url` & `--token`._", )(command) return command @@ -152,7 +155,7 @@ def okta_token_option(command: Callable) -> Callable: command = click.option( "--token", type=str, - help="Use token option to connect to okta. Requires options --org-url and --token", + help="Connect to Okta using a token. _Requires options `--org-url` and `--token`._", )(command) return command @@ -162,7 +165,7 @@ def aws_access_key_id_option(command: Callable) -> Callable: command = click.option( "--access_key_id", type=str, - help="Use access key id option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using an `Access Key ID`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command @@ -172,7 +175,7 @@ def aws_secret_access_key_option(command: Callable) -> Callable: command = click.option( "--secret_access_key", type=str, - help="Use access key option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using an `Access Key`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command @@ -182,7 +185,7 @@ def aws_region_option(command: Callable) -> Callable: command = click.option( "--region", type=str, - help="Use region option to connect to aws. Requires options --access_key_id, --secret_access_key and --region", + help="Connect to AWS using a specific `Region`. _Requires options `--access_key_id`, `--secret_access_key` & `--region`._", )(command) return command diff --git a/src/fides/cli/utils.py b/src/fides/cli/utils.py index c0efbe22a6..5c7ec76e7a 100644 --- a/src/fides/cli/utils.py +++ b/src/fides/cli/utils.py @@ -10,8 +10,8 @@ from platform import system from typing import Any, Callable, Dict, Optional, Union -import click import requests +import rich_click as click from fideslog.sdk.python.client import AnalyticsClient from fideslog.sdk.python.event import AnalyticsEvent from fideslog.sdk.python.exceptions import AnalyticsError diff --git a/tests/ctl/cli/test_cli.py b/tests/ctl/cli/test_cli.py index 5de3f9841e..53e2b2e241 100644 --- a/tests/ctl/cli/test_cli.py +++ b/tests/ctl/cli/test_cli.py @@ -51,13 +51,22 @@ def test_init_opt_in(test_cli_runner: CliRunner) -> None: assert result.exit_code == 0 -@pytest.mark.unit -def test_view_config(test_cli_runner: CliRunner) -> None: - result = test_cli_runner.invoke( - cli, ["view", "config"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} - ) - print(result.output) - assert result.exit_code == 0 +class TestView: + @pytest.mark.unit + def test_view_config(self, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, ["view", "config"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + ) + print(result.output) + assert result.exit_code == 0 + + @pytest.mark.unit + def test_view_credentials(self, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, ["view", "credentials"], env={"FIDES__USER__ANALYTICS_OPT_OUT": "true"} + ) + print(result.output) + assert result.exit_code == 0 @pytest.mark.unit @@ -197,8 +206,8 @@ def test_audit(test_config_path: str, test_cli_runner: CliRunner) -> None: assert result.exit_code == 0 +@pytest.mark.integration class TestCRUD: - @pytest.mark.integration def test_get(self, test_config_path: str, test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke( cli, @@ -207,12 +216,36 @@ def test_get(self, test_config_path: str, test_cli_runner: CliRunner) -> None: print(result.output) assert result.exit_code == 0 - @pytest.mark.integration + def test_delete(self, test_config_path: str, test_cli_runner: CliRunner) -> None: + result = test_cli_runner.invoke( + cli, + ["-f", test_config_path, "delete", "system", "demo_marketing_system"], + ) + print(result.output) + assert result.exit_code == 0 + def test_ls(self, test_config_path: str, test_cli_runner: CliRunner) -> None: result = test_cli_runner.invoke(cli, ["-f", test_config_path, "ls", "system"]) print(result.output) assert result.exit_code == 0 + def test_ls_verbose( + self, test_config_path: str, test_cli_runner: CliRunner + ) -> None: + result = test_cli_runner.invoke( + cli, ["-f", test_config_path, "ls", "system", "--verbose"] + ) + print(result.output) + assert result.exit_code == 0 + + def test_ls_no_resources_found( + self, test_config_path: str, test_cli_runner: CliRunner + ) -> None: + """This test only workss because we don't have any registry resources by default.""" + result = test_cli_runner.invoke(cli, ["-f", test_config_path, "ls", "registry"]) + print(result.output) + assert result.exit_code == 0 + class TestEvaluate: @pytest.mark.integration From b3140a3be383774f8c88fe0cd39c8951be1bc444 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:52:52 -0800 Subject: [PATCH 19/26] Bump plotly from 5.4 to 5.13.1 (#2707) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ad520d2842..daa52f9d8b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -24,7 +24,7 @@ openpyxl==3.0.9 packaging==21.3 pandas==1.4.3 passlib[bcrypt]==1.7.4 -plotly==5.4 +plotly==5.13.1 psycopg2-binary==2.9.5 pydantic<1.10.2 pydash==5.1.1 From 6257b42bbe9aa1902d33c66ffffa4981de5e4d9b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:56:07 -0800 Subject: [PATCH 20/26] Bump slowapi from 0.1.5 to 0.1.7 (#2513) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index daa52f9d8b..e65bd1de2d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -36,7 +36,7 @@ pyyaml>=5,<6 redis==3.5.3 rich-click==1.6.1 sendgrid==6.9.7 -slowapi==0.1.5 +slowapi==0.1.7 snowflake-sqlalchemy==1.4.3 sqlalchemy[asyncio]==1.4.27 sqlalchemy-citext==1.8.0 From 3eb6ab2dfcd6c5fc1a33dc243282e173786a6207 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:58:33 -0800 Subject: [PATCH 21/26] Bump actions/checkout from 2 to 3 (#2709) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/frontend_checks.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/frontend_checks.yml b/.github/workflows/frontend_checks.yml index 2b8aad1567..99a433f0a0 100644 --- a/.github/workflows/frontend_checks.yml +++ b/.github/workflows/frontend_checks.yml @@ -52,7 +52,7 @@ jobs: node-version: [16.x] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 @@ -118,7 +118,7 @@ jobs: node-version: [16.x] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 From 342a5d54dfecf5e6368cee4690ee535cb26eb880 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 21:59:23 -0800 Subject: [PATCH 22/26] Bump pytest from 7.1.2 to 7.2.2 (#2748) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index f99f4e14cc..2c0f4c9ba2 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -10,7 +10,7 @@ pylint==2.15.4 pytest-asyncio==0.19.0 pytest-cov==4.0.0 pytest-env==0.6.2 -pytest==7.1.2 +pytest==7.2.2 requests-mock==1.10.0 setuptools>=64.0.2 sqlalchemy-stubs From be67604cb236ea04bee672f8b3ed9e3ff9e0925c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Mar 2023 23:22:38 -0800 Subject: [PATCH 23/26] Bump hashicorp/vault-action from 2.4.2 to 2.5.0 (#2427) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/backend_checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index 41913d151e..445f95ea66 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -290,7 +290,7 @@ jobs: uses: actions/checkout@v3 - name: Get Vault Token - uses: hashicorp/vault-action@v2.4.2 + uses: hashicorp/vault-action@v2.5.0 with: url: ${{ secrets.VAULT_ADDR }} namespace: ${{ secrets.VAULT_NAMESPACE }} From 98663d3191fbd68d51a56017d4132329f7627dd2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Mar 2023 00:52:06 -0800 Subject: [PATCH 24/26] Bump fastapi[all] from 0.82.0 to 0.89.1 (#2246) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Thomas Co-authored-by: Thomas Co-authored-by: Adam Sachs --- .github/workflows/backend_checks.yml | 6 +-- noxfiles/test_setup_nox.py | 6 +-- requirements.txt | 2 +- .../api/v1/endpoints/connection_endpoints.py | 1 + .../ops/api/v1/endpoints/user_endpoints.py | 1 + .../service/connectors/fides/fides_client.py | 42 +++++++++------- .../ops/service/connectors/fides_connector.py | 7 +++ src/fides/cli/utils.py | 9 +++- src/fides/lib/oauth/schemas/oauth.py | 4 +- tests/ctl/cli/test_utils.py | 33 ++++--------- tests/ctl/conftest.py | 14 ++++-- tests/fixtures/application_fixtures.py | 4 ++ .../api/v1/endpoints/test_config_endpoints.py | 3 +- .../test_consent_request_endpoints.py | 6 ++- .../api/v1/endpoints/test_oauth_endpoints.py | 2 +- .../test_privacy_request_endpoints.py | 6 ++- .../endpoints/test_saas_config_endpoints.py | 2 +- tests/ops/integration_test_config.toml | 1 + .../connectors/fides/test_fides_client.py | 49 +++++++------------ .../connectors/test_fides_connector.py | 4 +- 20 files changed, 105 insertions(+), 97 deletions(-) diff --git a/.github/workflows/backend_checks.yml b/.github/workflows/backend_checks.yml index 445f95ea66..23f114c1d6 100644 --- a/.github/workflows/backend_checks.yml +++ b/.github/workflows/backend_checks.yml @@ -195,7 +195,7 @@ jobs: matrix: python_version: ["3.8.14", "3.9.14", "3.10.7"] runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' steps: @@ -232,7 +232,7 @@ jobs: matrix: python_version: ["3.8.14", "3.9.14", "3.10.7"] runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' steps: @@ -263,7 +263,7 @@ jobs: External-SaaS-Connectors: needs: Build runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 20 # In PRs run with the "unsafe" label, or run on a "push" event to main if: contains(github.event.pull_request.labels.*.name, 'run unsafe ci checks') || github.event_name == 'push' permissions: diff --git a/noxfiles/test_setup_nox.py b/noxfiles/test_setup_nox.py index 8024d23b64..7c6b8126e7 100644 --- a/noxfiles/test_setup_nox.py +++ b/noxfiles/test_setup_nox.py @@ -39,10 +39,11 @@ def pytest_ctl(session: Session, mark: str, coverage_arg: str) -> None: "-f", INTEGRATION_COMPOSE_FILE, "up", - "-d", + "--wait", IMAGE_NAME, ) session.run(*start_command, external=True) + session.run(*LOGIN, external=True) run_command = ( "docker", "exec", @@ -60,7 +61,6 @@ def pytest_ctl(session: Session, mark: str, coverage_arg: str) -> None: "OKTA_CLIENT_TOKEN", "-e", "BIGQUERY_CONFIG", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", @@ -123,7 +123,6 @@ def pytest_ops(session: Session, mark: str, coverage_arg: str) -> None: "BIGQUERY_KEYFILE_CREDS", "-e", "BIGQUERY_DATASET", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", @@ -154,7 +153,6 @@ def pytest_ops(session: Session, mark: str, coverage_arg: str) -> None: "VAULT_NAMESPACE", "-e", "VAULT_TOKEN", - "--rm", CI_ARGS_EXEC, CONTAINER_NAME, "pytest", diff --git a/requirements.txt b/requirements.txt index e65bd1de2d..65fa05f035 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ colorama>=0.4.3 cryptography==38.0.3 dask==2022.9.2 deepdiff==5.8.1 -fastapi[all]==0.82.0 +fastapi[all]==0.89.1 fastapi-caching[redis]==0.3.0 fastapi-pagination[sqlalchemy]~= 0.10.0 fideslang==1.3.3 diff --git a/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py b/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py index 71f4556eb4..8a2ffd1a20 100644 --- a/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/connection_endpoints.py @@ -210,6 +210,7 @@ def patch_connections( CONNECTION_BY_KEY, dependencies=[Security(verify_oauth_client, scopes=[CONNECTION_DELETE])], status_code=HTTP_204_NO_CONTENT, + response_model=None, ) def delete_connection( connection_key: FidesKey, *, db: Session = Depends(deps.get_db) diff --git a/src/fides/api/ops/api/v1/endpoints/user_endpoints.py b/src/fides/api/ops/api/v1/endpoints/user_endpoints.py index 600e493ebe..4de49a50cf 100644 --- a/src/fides/api/ops/api/v1/endpoints/user_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/user_endpoints.py @@ -121,6 +121,7 @@ def update_user_password( urls.USER_FORCE_PASSWORD_RESET, dependencies=[Security(verify_oauth_client, scopes=[USER_PASSWORD_RESET])], status_code=HTTP_200_OK, + response_model=UserResponse, ) def force_update_password( *, diff --git a/src/fides/api/ops/service/connectors/fides/fides_client.py b/src/fides/api/ops/service/connectors/fides/fides_client.py index f45dbca240..db214c6a4e 100644 --- a/src/fides/api/ops/service/connectors/fides/fides_client.py +++ b/src/fides/api/ops/service/connectors/fides/fides_client.py @@ -2,10 +2,9 @@ from typing import Any, Dict, List, Optional -import requests -from httpx import AsyncClient +import httpx +from httpx import AsyncClient, Client, HTTPStatusError, Request, RequestError, Timeout from loguru import logger -from requests import PreparedRequest, Request, RequestException, Session from fides.api.ctl.utils.errors import FidesError from fides.api.ops.api.v1 import urn_registry as urls @@ -32,7 +31,7 @@ class FidesClient: """ - A helper client class to broker communications between Fides servers. + A helper client to broker communications between Fides servers. """ def __init__( @@ -40,8 +39,12 @@ def __init__( uri: str, username: str, password: str, + connection_read_timeout: float = 30.0, ): - self.session = Session() + # Enable setting a custom `read` timeout + # to account for privacy request executions + self.session = Client(timeout=Timeout(5.0, read=connection_read_timeout)) + self.uri = uri self.username = username self.password = password @@ -55,14 +58,14 @@ def login(self) -> None: self.username, ) try: - response = requests.post( + response = httpx.post( f"{self.uri}{urls.V1_URL_PREFIX}{urls.LOGIN}", json=ul.dict() ) - except RequestException as e: + except RequestError as e: logger.error("Error logging in on remote Fides {}: {}", self.uri, str(e)) raise e - if response.ok: + if response.is_success: self.token = response.json()["token_data"]["access_token"] logger.info( "Successfully logged in to remote fides {} with username '{}'", @@ -81,21 +84,21 @@ def authenticated_request( query_params: Optional[Dict[str, Any]] = {}, data: Optional[Any] = None, json: Optional[Any] = None, - ) -> PreparedRequest: + ) -> Request: if not self.token: raise FidesError( f"Unable to create authenticated request. No token for Fides connector for server {self.uri}" ) - req: PreparedRequest = Request( + req: Request = self.session.build_request( method=method, url=f"{self.uri}{path}", headers=headers, params=query_params, data=data, json=json, - ).prepare() + ) req.headers["Authorization"] = f"Bearer {self.token}" return req @@ -117,15 +120,17 @@ def create_privacy_request( external_id, self.uri, ) - request: PreparedRequest = self.authenticated_request( + request: Request = self.authenticated_request( method="POST", path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUEST_AUTHENTICATED, json=[pr.dict()], ) response = self.session.send(request) - if not response.ok: + + if not response.is_success: logger.error("Error creating privacy request on remote Fides {}", self.uri) response.raise_for_status() + if response.json()["failed"]: # TODO better handle errored state here? raise FidesError( @@ -199,7 +204,7 @@ async def poll_for_request_completion( f"Privacy request [{privacy_request_id}] on remote Fides {self.uri} is in an unknown state. Look at the remote Fides for more information." ) - def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]]: + def request_status(self, privacy_request_id: str = "") -> List[Dict[str, Any]]: """ Return privacy request object that tracks its status """ @@ -215,15 +220,16 @@ def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]] self.uri, ) - request: PreparedRequest = self.authenticated_request( + request: Request = self.authenticated_request( method="GET", path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUESTS, query_params={"request_id": privacy_request_id} if privacy_request_id else None, ) - response = self.session.send(request, timeout=5) - if not response.ok: + response = self.session.send(request) + + if not response.is_success: logger.error( "Error retrieving status of privacy request [{}] on remote Fides {}", privacy_request_id, @@ -266,7 +272,7 @@ def retrieve_request_results( headers={"Authorization": f"Bearer {self.token}"}, ) response = self.session.send(request) - except requests.exceptions.HTTPError as e: + except HTTPStatusError as e: logger.error( "Error retrieving data from child server for privacy request {}: {}", privacy_request_id, diff --git a/src/fides/api/ops/service/connectors/fides_connector.py b/src/fides/api/ops/service/connectors/fides_connector.py index c0c7529248..d07f0d7540 100644 --- a/src/fides/api/ops/service/connectors/fides_connector.py +++ b/src/fides/api/ops/service/connectors/fides_connector.py @@ -48,11 +48,18 @@ def query_config(self, node: TraversalNode) -> QueryConfig[Any]: def create_client(self) -> FidesClient: """Returns a client used to connect to a Fides instance""" config = FidesConnectorSchema(**self.configuration.secrets or {}) + + # use polling_timeout here to provide a base read timeout + # on the HTTP client underlying the FidesClient, since even + # in non-polling context, we may hit a blocking HTTP call + # e.g., in privacy request creation we can block until completion client = FidesClient( uri=config.uri, username=config.username, password=config.password, + connection_read_timeout=self.polling_timeout, ) + client.login() return client diff --git a/src/fides/cli/utils.py b/src/fides/cli/utils.py index 5c7ec76e7a..89882daaac 100644 --- a/src/fides/cli/utils.py +++ b/src/fides/cli/utils.py @@ -72,6 +72,12 @@ def check_server_health(server_url: str, verbose: bool = True) -> requests.Respo return health_response +def compare_application_versions(server_version: str, cli_version: str) -> bool: + """Normalize and compare application versions.""" + normalize_version = lambda v: str(v).replace(".dirty", "", 1) + return normalize_version(server_version) == normalize_version(cli_version) + + def check_server(cli_version: str, server_url: str, quiet: bool = False) -> None: """Runs a health check and a version check against the server.""" @@ -82,8 +88,7 @@ def check_server(cli_version: str, server_url: str, quiet: bool = False) -> None raise SystemExit(1) server_version = health_response.json()["version"] - normalize_version = lambda v: str(v).replace(".dirty", "", 1) - if normalize_version(server_version) == normalize_version(cli_version): + if compare_application_versions(server_version, cli_version): if not quiet: echo_green( "Server is reachable and the client/server application versions match." diff --git a/src/fides/lib/oauth/schemas/oauth.py b/src/fides/lib/oauth/schemas/oauth.py index 7275195109..97b86a5bae 100644 --- a/src/fides/lib/oauth/schemas/oauth.py +++ b/src/fides/lib/oauth/schemas/oauth.py @@ -44,7 +44,9 @@ def __init__( ) async def __call__(self, request: Request) -> Optional[str]: - authorization: str = request.headers.get("Authorization") + authorization: Optional[str] = request.headers.get("Authorization") + if not authorization: + raise InvalidAuthorizationSchemeError() scheme, param = get_authorization_scheme_param(authorization) if not authorization or scheme.lower() != "bearer": if self.auto_error: diff --git a/tests/ctl/cli/test_utils.py b/tests/ctl/cli/test_utils.py index 2ac38eae08..0f9081c8d2 100644 --- a/tests/ctl/cli/test_utils.py +++ b/tests/ctl/cli/test_utils.py @@ -4,10 +4,9 @@ import click import pytest -from requests_mock import Mocker import fides.cli.utils as utils -from fides.core.config import FidesConfig, get_config +from fides.core.config import FidesConfig from tests.ctl.conftest import orig_requests_get @@ -24,42 +23,30 @@ def test_check_server_bad_ping(test_client, monkeypatch) -> None: @pytest.mark.unit @pytest.mark.parametrize( - "server_version, cli_version, expected_output, quiet", + "server_version, cli_version, expected_result", [ - ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5", "application versions match", False), - ("1.6.0+7.ge953df5", "1.6.0+9.ge953df5", "Mismatched versions!", False), + ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5", True), + ("1.6.0+7.ge953df5", "1.6.0+9.ge953df5", False), ( "1.6.0+7.ge953df5", "1.6.0+7.ge953df5.dirty", - "application versions match", - False, + True, ), ( "1.6.0+7.ge953df5.dirty", "1.6.0+7.ge953df5", - "application versions match", - False, + True, ), - ("1.6.0+7.ge953df5", "1.6.0+7.ge953df5.dirty", None, True), ], ) def test_check_server_version_comparisons( - requests_mock: Mocker, - capsys: pytest.CaptureFixture, server_version: str, cli_version: str, - expected_output: str, - quiet: bool, + expected_result: str, ) -> None: - """Check that comparing versions works""" - fake_url = "http://fake_address:8080" - requests_mock.get(f"{fake_url}/health", json={"version": server_version}) - utils.check_server(cli_version, "http://fake_address:8080", quiet=quiet) - captured = capsys.readouterr() - if expected_output is None: - assert captured.out == "" - else: - assert expected_output in captured.out + """Check that version comparison works.""" + actual_result = utils.compare_application_versions(server_version, cli_version) + assert expected_result == actual_result @pytest.mark.unit diff --git a/tests/ctl/conftest.py b/tests/ctl/conftest.py index 7e86e7e0d3..be9f36eb5e 100644 --- a/tests/ctl/conftest.py +++ b/tests/ctl/conftest.py @@ -19,7 +19,10 @@ @pytest.fixture(scope="session") def monkeysession(): - """monkeypatch fixture at the session level instead of the function level""" + """ + Monkeypatch at the session level instead of the function level. + Automatically undoes the monkeypatching when the session finishes. + """ mpatch = MonkeyPatch() yield mpatch mpatch.undo() @@ -27,10 +30,11 @@ def monkeysession(): @pytest.fixture(autouse=True, scope="session") def monkeypatch_requests(test_client, monkeysession) -> None: - """The requests library makes requests against the running webserver - which talks to the application db. This monkeypatching operation - makes `requests` calls from src/fides/core/api.py in a test - context talk to the test db instead""" + """ + Some places within the application, for example `fides.core.api`, use the `requests` + library to interact with the webserver. This fixture patches those `requests` calls + so that all of those tests instead interact with the test instance. + """ monkeysession.setattr(requests, "get", test_client.get) monkeysession.setattr(requests, "post", test_client.post) monkeysession.setattr(requests, "put", test_client.put) diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index 083f5f5cab..3faa6bd326 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -126,6 +126,9 @@ "uri": pydash.get(integration_config, "fides_example.uri"), "username": pydash.get(integration_config, "fides_example.username"), "password": pydash.get(integration_config, "fides_example.password"), + "polling_timeout": pydash.get( + integration_config, "fides_example.polling_timeout" + ), }, } @@ -1604,6 +1607,7 @@ def test_fides_client( fides_connector_example_secrets["uri"], fides_connector_example_secrets["username"], fides_connector_example_secrets["password"], + fides_connector_example_secrets["polling_timeout"] ) diff --git a/tests/ops/api/v1/endpoints/test_config_endpoints.py b/tests/ops/api/v1/endpoints/test_config_endpoints.py index e76cdf29e7..018c7d04c0 100644 --- a/tests/ops/api/v1/endpoints/test_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_config_endpoints.py @@ -329,7 +329,8 @@ def test_get_application_config( # then we test that we can GET them auth_header = generate_auth_header([scopes.CONFIG_READ]) - response = api_client.get( + response = api_client.request( + "GET", url, headers=auth_header, params={"api_set": True}, diff --git a/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py b/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py index 094918993b..c691d9530c 100644 --- a/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_consent_request_endpoints.py @@ -375,7 +375,8 @@ class TestGetConsentUnverified: def test_consent_unverified_no_consent_request_id(self, api_client): data = {"code": "12345"} - response = api_client.get( + response = api_client.request( + "GET", f"{V1_URL_PREFIX}{CONSENT_REQUEST_PREFERENCES_WITH_ID.format(consent_request_id='non_existent_consent_id')}", json=data, ) @@ -388,7 +389,8 @@ def test_consent_unverified_no_consent_request_id(self, api_client): def test_consent_unverified_verification_error(self, api_client): data = {"code": "12345"} - response = api_client.get( + response = api_client.request( + "GET", f"{V1_URL_PREFIX}{CONSENT_REQUEST_PREFERENCES_WITH_ID.format(consent_request_id='non_existent_consent_id')}", json=data, ) diff --git a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py index dc815b4987..192bc30bd7 100644 --- a/tests/ops/api/v1/endpoints/test_oauth_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_oauth_endpoints.py @@ -501,7 +501,7 @@ def test_callback_for_valid_state( response = api_client.get( callback_url, params={"code": "abc", "state": "new_request"} ) - assert response.ok + response.raise_for_status() get_access_token_mock.assert_called_once() authentication_request.delete(db) diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index 611d651156..8e64bed76a 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -1532,7 +1532,8 @@ def test_sort_privacy_request_by_due_date( api_client.post(url, json=data) auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_READ]) - resp = api_client.get( + resp = api_client.request( + "GET", f"{url}?sort_direction=asc&sort_field=due_date", json=data, headers=auth_header, @@ -1542,7 +1543,8 @@ def test_sort_privacy_request_by_due_date( for i, request in enumerate(asc_response_data): assert request["days_left"] == days_left_values[i] - resp = api_client.get( + resp = api_client.request( + "GET", f"{url}?sort_direction=desc&sort_field=due_date", json=data, headers=auth_header, diff --git a/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py b/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py index 03630a7c91..06a4b785ca 100644 --- a/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_saas_config_endpoints.py @@ -450,5 +450,5 @@ def test_get_authorize_url( authorization_url_mock.return_value = authorization_url auth_header = generate_auth_header([CONNECTION_AUTHORIZE]) response = api_client.get(authorize_url, headers=auth_header) - assert response.ok + response.raise_for_status() assert response.text == f'"{authorization_url}"' diff --git a/tests/ops/integration_test_config.toml b/tests/ops/integration_test_config.toml index a2112f7120..e34d6a1f52 100644 --- a/tests/ops/integration_test_config.toml +++ b/tests/ops/integration_test_config.toml @@ -57,3 +57,4 @@ port=5432 uri="http://fides:8080" username="root_user" password="Testpassword1!" +polling_timeout=1800 diff --git a/tests/ops/service/connectors/fides/test_fides_client.py b/tests/ops/service/connectors/fides/test_fides_client.py index 5ab79ec58a..cd74561635 100644 --- a/tests/ops/service/connectors/fides/test_fides_client.py +++ b/tests/ops/service/connectors/fides/test_fides_client.py @@ -2,8 +2,7 @@ from unittest import mock import pytest -from httpx import AsyncClient -from requests import HTTPError, Session +from httpx import AsyncClient, Client, HTTPStatusError from fides.api.ctl.utils.errors import FidesError from fides.api.ops.models.privacy_request import PrivacyRequest, PrivacyRequestStatus @@ -17,8 +16,8 @@ class MockResponse: A class to mock Fides API responses """ - def __init__(self, ok, json_data): - self.ok = ok + def __init__(self, is_success, json_data): + self.is_success = is_success self.json_data = json_data def json(self): @@ -43,7 +42,7 @@ class TestFidesClientUnit: """ @mock.patch( - "requests.post", + "httpx.post", side_effect=[ MockResponse(True, {"token_data": {"access_token": SAMPLE_TOKEN}}) ], @@ -69,7 +68,7 @@ def test_authenticated_request(self, mock_login, test_fides_client: FidesClient) ) assert request.method == "GET" assert request.url == test_fides_client.uri + "/testpath" - assert len(request.headers) == 2 + assert len(request.headers) == 7 assert "another_header" in request.headers assert request.headers["another_header"] == "header_value" assert "Authorization" in request.headers @@ -88,7 +87,7 @@ def test_authenticated_request_not_logged_in(self, test_fides_client: FidesClien assert "No token" in str(exc) @mock.patch( - "requests.post", + "httpx.post", side_effect=[ MockResponse(True, {"token_data": {"access_token": SAMPLE_TOKEN}}) ], @@ -112,22 +111,6 @@ def test_authenticated_request_parameters( == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - # test form data passed as tuples - request = test_fides_client.authenticated_request( - "POST", - path="/testpath", - query_params={"param1": "value1", "param2": "value2"}, - data=[("key1", "value1"), ("key2", "value2")], - ) - assert "Authorization" in request.headers - assert request.headers["Authorization"] == f"Bearer {SAMPLE_TOKEN}" - assert request.method == "POST" - assert ( - request.url - == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" - ) - assert request.body == "key1=value1&key2=value2" - # test form data passed as dict request = test_fides_client.authenticated_request( "POST", @@ -142,7 +125,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == "key1=value1&key2=value2" + request.read() + assert request.content == b"key1=value1&key2=value2" # test body passed as string literal request = test_fides_client.authenticated_request( @@ -158,7 +142,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == "testbody" + request.read() + assert request.content == b"testbody" # test json body passed as a dict request = test_fides_client.authenticated_request( @@ -174,7 +159,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == b'{"field1": "value1"}' + request.read() + assert request.content == b'{"field1": "value1"}' # test json body passed as a list request = test_fides_client.authenticated_request( @@ -190,7 +176,8 @@ def test_authenticated_request_parameters( request.url == test_fides_client.uri + "/testpath?param1=value1¶m2=value2" ) - assert request.body == b'[{"field1": "value1"}]' + request.read() + assert request.content == b'[{"field1": "value1"}]' @pytest.mark.asyncio def test_poll_for_completion( @@ -301,7 +288,7 @@ def test_login_bad_credentials( # so that we don't call `create_client()`, which performs # login as part of initialization - with pytest.raises(HTTPError): + with pytest.raises(HTTPStatusError): test_fides_client_bad_credentials.login() assert test_fides_client_bad_credentials.token is None @@ -317,7 +304,7 @@ def test_create_privacy_request( Test that properly configured fides client can create and execute a valid access privacy request Inspired by `test_privacy_request_endpoints.TestCreatePrivacyRequest` """ - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) pr_id = authenticated_fides_client.create_privacy_request( external_id="test_external_id", @@ -339,14 +326,14 @@ def test_request_status_no_privacy_request( privacy request ID specified. This acts as a basic test to validate we can successfully hit authenticated endpoints. """ - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) statuses = authenticated_fides_client.request_status() assert len(statuses) == 0 def test_request_status_privacy_request( self, authenticated_fides_client: FidesClient, policy, monkeypatch, api_client ): - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) pr_id = authenticated_fides_client.create_privacy_request( external_id="test_external_id", diff --git a/tests/ops/service/connectors/test_fides_connector.py b/tests/ops/service/connectors/test_fides_connector.py index a56bc96624..92277ec05d 100644 --- a/tests/ops/service/connectors/test_fides_connector.py +++ b/tests/ops/service/connectors/test_fides_connector.py @@ -2,7 +2,7 @@ from typing import Tuple import pytest -from requests import Session +from httpx import Client from fides.api.ops.graph.traversal import TraversalNode from fides.api.ops.models.connectionconfig import ConnectionConfig, ConnectionTestStatus @@ -128,7 +128,7 @@ def test_retrieve_data( # Monkey patch both Session.send and the httpx.AsyncClient. Both of these will just # make requests to the running webserver which is connected to the application db, # but we need them to talk to the test db in pytest - monkeypatch.setattr(Session, "send", api_client.send) + monkeypatch.setattr(Client, "send", api_client.send) monkeypatch.setattr( request_service, "get_async_client", lambda: async_api_client ) From 618b6265db67c3d1fc779c92a49deaff93da6dcc Mon Sep 17 00:00:00 2001 From: Sebastian Sangervasi <2236777+ssangervasi@users.noreply.github.com> Date: Mon, 6 Mar 2023 06:55:26 -0800 Subject: [PATCH 25/26] test_env/pc: Add GPC settings to test env privacy center config (#2701) --- .../data/test_env/privacy_center_config/config.json | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/fides/data/test_env/privacy_center_config/config.json b/src/fides/data/test_env/privacy_center_config/config.json index 082ad73189..2a530a619e 100644 --- a/src/fides/data/test_env/privacy_center_config/config.json +++ b/src/fides/data/test_env/privacy_center_config/config.json @@ -45,7 +45,10 @@ "name": "Data Sales or Sharing", "description": "We may use some of your personal information for behavioral advertising purposes, which may be interpreted as 'Data Sales' or 'Data Sharing' under regulations such as CCPA, CPRA, VCDPA, etc.", "url": "https://example.com/privacy#data-sales", - "default": true, + "default": { + "value": true, + "globalPrivacyControl": false + }, "highlight": false, "cookieKeys": ["data_sales"], "executable": false @@ -55,7 +58,10 @@ "name": "Email Marketing", "description": "We may use some of your personal information to contact you about our products & services.", "url": "https://example.com/privacy#email-marketing", - "default": true, + "default": { + "value": true, + "globalPrivacyControl": false + }, "highlight": false, "cookieKeys": [], "executable": false From e07f8961e744ee2c2010ee1f2b3e06b81e4cb1f2 Mon Sep 17 00:00:00 2001 From: Paul Sanders Date: Mon, 6 Mar 2023 10:03:14 -0500 Subject: [PATCH 26/26] Add custom fields to datamap (#2531) Co-authored-by: Paul Sanders Co-authored-by: SteveDMurphy --- src/fides/api/ctl/database/crud.py | 60 ++++++- src/fides/api/ctl/routes/crud.py | 4 +- src/fides/api/ctl/routes/datamap.py | 36 +++- src/fides/api/ctl/sql_models.py | 21 ++- src/fides/core/export.py | 245 ++++++++++++++++++++-------- src/fides/core/export_helpers.py | 100 +++++++++--- src/fides/core/utils.py | 11 +- tests/ctl/api/test_datamap.py | 53 +++++- tests/ctl/core/test_export.py | 65 +++++++- tests/ctl/core/test_utils.py | 4 +- tests/ctl/database/test_crud.py | 88 +++++++++- tests/ops/api/v1/test_main.py | 2 +- 12 files changed, 576 insertions(+), 113 deletions(-) diff --git a/src/fides/api/ctl/database/crud.py b/src/fides/api/ctl/database/crud.py index 5882827bbb..b4cde58e96 100644 --- a/src/fides/api/ctl/database/crud.py +++ b/src/fides/api/ctl/database/crud.py @@ -2,7 +2,7 @@ Contains all of the generic CRUD endpoints that can be generated programmatically for each resource. """ -from typing import Dict, List, Tuple +from typing import Any, Dict, List, Tuple from loguru import logger as log from sqlalchemy import column @@ -14,6 +14,10 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.future import select +from fides.api.ctl.sql_models import ( # type: ignore[attr-defined] + CustomField, + CustomFieldDefinition, +) from fides.api.ctl.utils import errors from fides.lib.db.base import Base # type: ignore[attr-defined] @@ -84,6 +88,59 @@ async def get_resource( return sql_resource +async def get_resource_with_custom_fields( + sql_model: Base, fides_key: str, async_session: AsyncSession +) -> Dict[str, Any]: + """Get a resource from the databse by its FidesKey including it's custom fields. + + Returns a dictionary of that resource. + """ + resource = await get_resource(sql_model, fides_key, async_session) + resource_dict = resource.__dict__ + resource_dict.pop("_sa_instance_state", None) + + with log.contextualize(sql_model=sql_model.__name__, fides_key=fides_key): + async with async_session.begin(): + try: + log.debug("Fetching custom fields for resource") + query = ( + select(CustomFieldDefinition.name, CustomField.value) + .join( + CustomField, + CustomField.custom_field_definition_id + == CustomFieldDefinition.id, + ) + .where( + (CustomField.resource_id == resource.fides_key) + & ( # pylint: disable=singleton-comparison + CustomFieldDefinition.active == True + ) + ) + ) + result = await async_session.execute(query) + except SQLAlchemyError: + sa_error = errors.QueryError() + log.bind(error=sa_error.detail["error"]).error( # type: ignore[index] + "Failed to fetch custom fields" + ) + raise sa_error + + custom_fields = result.mappings().all() + + if not custom_fields: + return resource_dict + + for field in custom_fields: + if field["name"] in resource_dict: + resource_dict[ + field["name"] + ] = f"{resource_dict[field['name']]}, {', '.join(field['value'])}" + else: + resource_dict[field["name"]] = ", ".join(field["value"]) + + return resource_dict + + async def list_resource(sql_model: Base, async_session: AsyncSession) -> List[Base]: """ Get a list of all of the resources of this type from the database. @@ -216,7 +273,6 @@ async def delete_resource( else: log.debug("Deleting resource") query = _delete(sql_model).where(sql_model.fides_key == fides_key) - print(query) await async_session.execute(query) except SQLAlchemyError: error = errors.QueryError() diff --git a/src/fides/api/ctl/routes/crud.py b/src/fides/api/ctl/routes/crud.py index 4b080ac5c5..f0f6219902 100644 --- a/src/fides/api/ctl/routes/crud.py +++ b/src/fides/api/ctl/routes/crud.py @@ -18,7 +18,7 @@ from fides.api.ctl.database.crud import ( create_resource, delete_resource, - get_resource, + get_resource_with_custom_fields, list_resource, update_resource, upsert_resources, @@ -149,7 +149,7 @@ async def get( ) -> Dict: """Get a resource by its fides_key.""" sql_model = sql_model_map[resource_type] - return await get_resource(sql_model, fides_key, db) + return await get_resource_with_custom_fields(sql_model, fides_key, db) @router.put( "/", diff --git a/src/fides/api/ctl/routes/datamap.py b/src/fides/api/ctl/routes/datamap.py index f6ad898f1a..09598d1013 100644 --- a/src/fides/api/ctl/routes/datamap.py +++ b/src/fides/api/ctl/routes/datamap.py @@ -1,7 +1,7 @@ """ Contains an endpoint for extracting a data map from the server """ -from typing import Dict, List +from typing import Any, Dict, List from fastapi import Depends, Response, Security, status from fideslang.parse import parse_dict @@ -9,7 +9,11 @@ from pandas import DataFrame from sqlalchemy.ext.asyncio import AsyncSession -from fides.api.ctl.database.crud import get_resource, list_resource +from fides.api.ctl.database.crud import ( + get_resource, + get_resource_with_custom_fields, + list_resource, +) from fides.api.ctl.database.session import get_async_db from fides.api.ctl.routes.util import API_PREFIX from fides.api.ctl.sql_models import sql_model_map # type: ignore[attr-defined] @@ -44,7 +48,6 @@ "content": { "application/json": { "example": [ - DATAMAP_COLUMNS_API, { "system.name": "Demo Analytics System", "system.data_responsibility_title": "Controller", @@ -98,7 +101,7 @@ async def export_datamap( organization_fides_key: str, response: Response, db: AsyncSession = Depends(get_async_db), -) -> List[Dict[str, str]]: +) -> List[Dict[str, Any]]: """ An endpoint to return the data map for a given Organization. @@ -133,7 +136,18 @@ async def export_datamap( for resource in server_resources if resource.organization_fides_key == organization_fides_key ] + server_resource_dict[resource_type] = filtered_server_resources + + for k, v in server_resource_dict.items(): + values = [] + for value in v: + with_custom_fields = await get_resource_with_custom_fields( + sql_model_map[k], value.fides_key, db + ) + values.append(with_custom_fields) + server_resource_dict[k] = values + except DatabaseUnavailableError: database_unavailable_error = DatabaseUnavailableError( error_message="Database unavailable" @@ -143,23 +157,29 @@ async def export_datamap( ) raise database_unavailable_error - joined_system_dataset_df = build_joined_dataframe(server_resource_dict) + joined_system_dataset_df, custom_columns = build_joined_dataframe( + server_resource_dict + ) - formatted_datamap = format_datamap_values(joined_system_dataset_df) + formatted_datamap = format_datamap_values(joined_system_dataset_df, custom_columns) # prepend column names formatted_datamap = [DATAMAP_COLUMNS_API] + formatted_datamap return formatted_datamap -def format_datamap_values(joined_system_dataset_df: DataFrame) -> List[Dict[str, str]]: +def format_datamap_values( + joined_system_dataset_df: DataFrame, custom_columns: Dict[str, str] +) -> List[Dict[str, str]]: """ Formats the joined DataFrame to return the data as records. """ + columns = {**DATAMAP_COLUMNS_API, **custom_columns} + limited_columns_df = DataFrame( joined_system_dataset_df, - columns=list(DATAMAP_COLUMNS_API.keys()), + columns=list(columns.keys()), ) return limited_columns_df.to_dict("records") diff --git a/src/fides/api/ctl/sql_models.py b/src/fides/api/ctl/sql_models.py index 5a8f36f887..86a8b45b0c 100644 --- a/src/fides/api/ctl/sql_models.py +++ b/src/fides/api/ctl/sql_models.py @@ -5,9 +5,10 @@ """ from enum import Enum as EnumType -from typing import Dict +from typing import Any, Dict, List, Optional from fideslang.models import Dataset as FideslangDataset +from pydantic import BaseModel from sqlalchemy import ARRAY, BOOLEAN, JSON, Column from sqlalchemy import Enum as EnumColumn from sqlalchemy import ( @@ -286,6 +287,24 @@ class System(Base, FidesBase): ingress = Column(JSON) +class SystemModel(BaseModel): + fides_key: str + registry_id: str + meta: Optional[Dict[str, Any]] + fidesctl_meta: Optional[Dict[str, Any]] + system_type: str + data_responsibility_title: Optional[str] + system_dependencies: Optional[List[str]] + joint_controller: Optional[str] + third_country_transfers: Optional[List[str]] + privacy_declarations: Optional[Dict[str, Any]] + administrating_department: Optional[str] + data_protection_impact_assessment: Optional[Dict[str, Any]] + egress: Optional[Dict[str, Any]] + ingress: Optional[Dict[str, Any]] + value: Optional[List[Any]] + + class SystemScans(Base): """ The SQL model for System Scan instances. diff --git a/src/fides/core/export.py b/src/fides/core/export.py index f3286d5ce7..2b6673d21e 100644 --- a/src/fides/core/export.py +++ b/src/fides/core/export.py @@ -5,7 +5,7 @@ from typing import Dict, List, Tuple import pandas as pd -from fideslang.models import ContactDetails, FidesModel +from fideslang.models import ContactDetails, DataFlow, FidesModel from fides.core.api_helpers import ( get_server_resource, @@ -51,45 +51,48 @@ def generate_dataset_records( # using a set to preserve uniqueness of categories and qualifiers across fields unique_data_categories: set = set() for dataset in server_dataset_list: - dataset_name = dataset.name - dataset_description = dataset.description - dataset_fides_key = dataset.fides_key - dataset_retention = dataset.retention + if not isinstance(dataset, dict): + dataset = dataset.dict() + + dataset_name = dataset["name"] + dataset_description = dataset["description"] + dataset_fides_key = dataset["fides_key"] + dataset_retention = dataset["retention"] dataset_third_country_transfers = ", ".join( - dataset.third_country_transfers or [] + dataset["third_country_transfers"] or [] ) - if dataset.data_categories: + if dataset["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - dataset.data_qualifier, - dataset.data_categories, + dataset["data_qualifier"], + dataset["data_categories"], dataset_retention, dataset_third_country_transfers, dataset_fides_key, ) unique_data_categories = unique_data_categories.union(dataset_rows) - for collection in dataset.collections: - collection_retention = collection.retention or dataset_retention - if collection.data_categories: + for collection in dataset["collections"]: + collection_retention = collection["retention"] or dataset_retention + if collection["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - collection.data_qualifier, - collection.data_categories, + collection["data_qualifier"], + collection["data_categories"], collection_retention, dataset_third_country_transfers, dataset_fides_key, ) unique_data_categories = unique_data_categories.union(dataset_rows) - for field in get_all_level_fields(collection.fields): - if field.data_categories: + for field in get_all_level_fields(collection["fields"]): + if field["data_categories"]: dataset_rows = generate_data_category_rows( dataset_name, dataset_description, - field.data_qualifier, - field.data_categories, - field.retention or collection_retention, + field["data_qualifier"], + field["data_categories"], + field["retention"] or collection_retention, dataset_third_country_transfers, dataset_fides_key, ) @@ -129,17 +132,22 @@ def export_dataset( print(record) -def generate_system_records( +def generate_system_records( # pylint: disable=too-many-nested-blocks, too-many-branches, too-many-statements server_resources: Dict[str, List], -) -> List[Tuple[str, ...]]: +) -> Tuple[List[Tuple[str, ...]], Dict[str, str]]: """ Takes a dictionary of resources from the server, returning a list of tuples to be used as records to be exported. The headers are defined as the first tuple in the result. """ - formatted_data_uses = format_data_uses(server_resources["data_use"]) - formatted_data_subjects = format_data_subjects(server_resources["data_subject"]) + custom_columns = {} + formatted_data_uses, custom_data_uses = format_data_uses( + server_resources["data_use"] + ) + formatted_data_subjects, custom_data_subjects = format_data_subjects( + server_resources["data_subject"] + ) output_list: List[Tuple[str, ...]] = [ ( "system.fides_key", @@ -170,41 +178,123 @@ def generate_system_records( ) ] + if custom_data_uses: + keys = list(output_list[0]) + for key, value in custom_data_uses.items(): + key_string = f"system.privacy_declaration.data_use.{key}" + custom_columns[key_string] = value + keys.append(key_string) + output_list[0] = tuple(keys) + + if custom_data_subjects: + keys = list(output_list[0]) + for key, value in custom_data_subjects.items(): + key_string = f"system.privacy_declaration.data_subjects.{key}" + custom_columns[key_string] = value + keys.append(key_string) + output_list[0] = tuple(keys) + + system_custom_field_data = {} + known_fields = ( + "fides_key", + "name", + "description", + "data_responsibility_title", + "administrating_department", + "third_country_transfers", + "system_dependencies", + "ingress", + "egress", + "privacy_declarations", + "data_protection_impact_assessment", + "registry_id", + "id", + "updated_at", + "joint_controller", + "created_at", + "organization_fides_key", + "meta", + "tags", + "fidesctl_meta", + "system_type", + ) for system in server_resources["system"]: - third_country_list = ", ".join(system.third_country_transfers or []) - system_dependencies = ", ".join(system.system_dependencies or []) + if not isinstance(system, dict): + system = system.__dict__ + + for key, value in system.items(): + if key not in known_fields: + keys = list(output_list[0]) + key_string = f"system.{key}" + keys.append(key_string) + output_list[0] = tuple(keys) + custom_columns[key_string] = key + if isinstance(value, list): + system_custom_field_data[key_string] = ", ".join(value) + else: + system_custom_field_data[key_string] = value + + third_country_list = ", ".join(system.get("third_country_transfers") or []) + system_dependencies = ", ".join(system.get("system_dependencies") or []) + if system.get("ingress"): + if isinstance(system["ingress"], DataFlow): + system["ingress"] = system["ingress"].dict() + elif isinstance(system["ingress"], list): + reformatted = [] + for ingress in system["ingress"]: + if isinstance(ingress, DataFlow): + reformatted.append(ingress.dict()) + else: + reformatted.append(ingress) + system["ingress"] = reformatted system_ingress = ", ".join( - [ingress.fides_key for ingress in system.ingress or []] + [ingress["fides_key"] for ingress in system.get("ingress") or []] + ) + if system.get("egress"): + if isinstance(system["egress"], DataFlow): + system["egress"] = system["egress"].dict() + elif isinstance(system["egress"], list): + reformatted = [] + for ingress in system["egress"]: + if isinstance(ingress, DataFlow): + reformatted.append(ingress.dict()) + else: + reformatted.append(ingress) + system["egress"] = reformatted + system_egress = ", ".join( + [egress["fides_key"] for egress in system.get("egress") or []] ) - system_egress = ", ".join([egress.fides_key for egress in system.egress or []]) data_protection_impact_assessment = ( get_formatted_data_protection_impact_assessment( - system.data_protection_impact_assessment.dict() + system.get("data_protection_impact_assessment", {}) ) ) - if system.privacy_declarations: - for declaration in system.privacy_declarations: - data_use = formatted_data_uses[declaration.data_use] - data_categories = declaration.data_categories or [] + if system.get("privacy_declarations"): + for declaration in system["privacy_declarations"]: + if not isinstance(declaration, dict): + declaration = declaration.dict() + + data_use = formatted_data_uses[declaration["data_use"]] + data_categories = declaration["data_categories"] or [] data_subjects = [ formatted_data_subjects[data_subject_fides_key] - for data_subject_fides_key in declaration.data_subjects + for data_subject_fides_key in declaration["data_subjects"] ] - dataset_references = declaration.dataset_references or [ + dataset_references = declaration["dataset_references"] or [ EMPTY_COLUMN_PLACEHOLDER ] - cartesian_product_of_declaration = [ - ( - system.fides_key, - system.name, - system.description, - system.data_responsibility_title, - system.administrating_department, + cartesian_product_of_declaration_builder = [ + [ + system["fides_key"], + system["name"], + system["description"], + system["data_responsibility_title"], + system["administrating_department"], third_country_list, system_dependencies, system_ingress, system_egress, - declaration.name, + declaration["name"], category, data_use["name"], data_use["legal_basis"], @@ -215,34 +305,51 @@ def generate_system_records( subject["name"], subject["rights_available"], subject["automated_decisions_or_profiling"], - declaration.data_qualifier, + declaration["data_qualifier"], data_protection_impact_assessment["is_required"], data_protection_impact_assessment["progress"], data_protection_impact_assessment["link"], dataset_reference, - ) + ] for category in data_categories for subject in data_subjects for dataset_reference in dataset_references ] + cartesian_product_of_declaration = [] + if system_custom_field_data: + for _, v in system_custom_field_data.items(): + for product in cartesian_product_of_declaration_builder: + product.append(v) + cartesian_product_of_declaration.append(tuple(product)) + else: + cartesian_product_of_declaration = [ + tuple(x) for x in cartesian_product_of_declaration_builder + ] + output_list += cartesian_product_of_declaration else: system_row = [ - system.fides_key, - system.name, - system.description, - system.data_responsibility_title, - system.administrating_department, + system["fides_key"], + system["name"], + system["description"], + system["data_responsibility_title"], + system["administrating_department"], third_country_list, system_dependencies, system_ingress, system_egress, - data_protection_impact_assessment["is_required"], - data_protection_impact_assessment["progress"], - data_protection_impact_assessment["link"], ] - num_privacy_declaration_fields = 12 - privacy_declaration_start_index = 9 + if system_custom_field_data: + for _, v in system_custom_field_data.items(): + system_row.append(v) + len_no_privacy = len(system_row) + system_row.append(data_protection_impact_assessment["is_required"]) + system_row.append(data_protection_impact_assessment["progress"]) + system_row.append(data_protection_impact_assessment["link"]) + num_privacy_declaration_fields = 3 + privacy_declaration_start_index = ( + len_no_privacy - num_privacy_declaration_fields + ) for i in range(num_privacy_declaration_fields): system_row.insert( i + privacy_declaration_start_index, EMPTY_COLUMN_PLACEHOLDER @@ -252,7 +359,7 @@ def generate_system_records( # also add n/a for the dataset reference output_list += [tuple(system_row)] - return output_list + return output_list, custom_columns def export_system( @@ -294,7 +401,7 @@ def export_system( url, "data_subject", list(set(data_subject_fides_keys)), headers ) - output_list = generate_system_records(server_resources) + output_list, _ = generate_system_records(server_resources) if not dry: exported_filename = export_to_csv(output_list, resource_type, manifests_dir) @@ -327,16 +434,19 @@ def generate_contact_records( # currently the output file will only truly support a single organization # need to better understand the use case for multiple and how to handle for organization in server_organization_list: + if not isinstance(organization, dict): + organization = organization.dict() + fields = tuple(ContactDetails().dict().keys()) get_values = ( - lambda x: tuple(x.dict().values()) + lambda x: tuple(x.values()) if x else tuple(ContactDetails().dict().values()) ) - controller = get_values(organization.controller) - data_protection_officer = get_values(organization.data_protection_officer) - representative = get_values(organization.representative) + controller = get_values(organization["controller"]) + data_protection_officer = get_values(organization["data_protection_officer"]) + representative = get_values(organization["representative"]) contact_details = list( zip( @@ -389,7 +499,7 @@ def export_organization( def build_joined_dataframe( server_resource_dict: Dict[str, List], -) -> pd.DataFrame: +) -> Tuple[pd.DataFrame, Dict[str, str]]: """ Return joined dataframes for datamap export @@ -400,7 +510,7 @@ def build_joined_dataframe( # systems - system_output_list = generate_system_records(server_resource_dict) + system_output_list, custom_columns = generate_system_records(server_resource_dict) systems_df = pd.DataFrame.from_records(system_output_list) systems_df.columns = systems_df.iloc[0] systems_df = systems_df[1:] @@ -459,12 +569,17 @@ def build_joined_dataframe( joined_df["system.third_country_safeguards"] = "" joined_df["system.link_to_processor_contract"] = "" + if not isinstance(server_resource_dict["organization"][0], dict): + server_resource_dict["organization"][0] = server_resource_dict["organization"][ + 0 + ].dict() + joined_df["organization.link_to_security_policy"] = ( - server_resource_dict["organization"][0].security_policy or "" + server_resource_dict["organization"][0]["security_policy"] or "" ) joined_df["dataset.source_name"] = joined_df["dataset.name"] - return joined_df + return joined_df, custom_columns def export_datamap( @@ -514,7 +629,7 @@ def export_datamap( server_resource_dict[resource_type] = filtered_server_resources # transform the resources to join a system and referenced datasets - joined_system_dataset_df = build_joined_dataframe(server_resource_dict) + joined_system_dataset_df, _ = build_joined_dataframe(server_resource_dict) if not dry and not to_csv: # build an organization dataframe if exporting to excel diff --git a/src/fides/core/export_helpers.py b/src/fides/core/export_helpers.py index d9796baa77..b1f84d4184 100644 --- a/src/fides/core/export_helpers.py +++ b/src/fides/core/export_helpers.py @@ -138,7 +138,9 @@ def export_datamap_to_excel( return filename -def format_data_uses(data_uses: List[DataUse]) -> Dict[FidesKey, Dict[str, str]]: +def format_data_uses( + data_uses: List[DataUse], +) -> Tuple[Dict[FidesKey, Dict[str, str]], Dict[str, str]]: """ This function formats data uses for use when exporting, returning the necessary values as a dict. Formatting @@ -146,38 +148,65 @@ def format_data_uses(data_uses: List[DataUse]) -> Dict[FidesKey, Dict[str, str]] """ formatted_data_uses = {} + custom_columns = {} + known_attributes = ( + "legal_basis", + "special_category", + "recipients", + "legitimate_interest_impact_assessment", + "legitimate_interest", + ) + excluded_attributes = ( + "id", + "tags", + "description", + "updated_at", + "is_default", + "organization_fides_key", + "parent", + "created_at", + "name", + "parent_key", + "fides_key", + ) for data_use in data_uses: + if not isinstance(data_use, dict): + data_use = data_use.dict() + formatted_data_use = { - "name": data_use.name, + "name": data_use["name"], } - for attribute in [ - "legal_basis", - "special_category", - "recipients", - "legitimate_interest_impact_assessment", - "legitimate_interest", - ]: - attribute_value = getattr(data_use, attribute) + for attribute in known_attributes: + attribute_value = data_use.get(attribute) if attribute_value is None: attribute_value = "N/A" elif isinstance(attribute_value, list): attribute_value = ", ".join(attribute_value) elif attribute == "legitimate_interest": if attribute_value is True: - attribute_value = getattr(data_use, "name") + attribute_value = data_use.get("name") else: attribute_value = "N/A" formatted_data_use[attribute] = attribute_value - formatted_data_uses[data_use.fides_key] = formatted_data_use - return formatted_data_uses + for k, v in data_use.items(): + if k not in known_attributes and k not in excluded_attributes: + custom_columns[k] = k + if isinstance(v, list): + formatted_data_use[k] = ", ".join(v) + else: + formatted_data_use[k] = v + + formatted_data_uses[data_use["fides_key"]] = formatted_data_use + + return formatted_data_uses, custom_columns def format_data_subjects( data_subjects: List[DataSubject], -) -> Dict[FidesKey, Dict[str, str]]: +) -> Tuple[Dict[FidesKey, Dict[str, str]], Dict[str, str]]: """ This function formats data subjects from the server, returning the necessary values as a list of dicts. @@ -194,9 +223,26 @@ def format_data_subjects( ] formatted_data_subjects: Dict[FidesKey, Dict[str, str]] = {} + excluded_attributes = ( + "tags", + "id", + "description", + "updated_at", + "automated_decisions_or_profiling", + "fides_key", + "organization_fides_key", + "name", + "created_at", + "rights", + "is_default", + "rights_available", + ) + custom_columns = {} for data_subject in data_subjects: - data_subject_dict = data_subject.dict() + if not isinstance(data_subject, dict): + data_subject = data_subject.dict() + formatted_data_subject = dict( zip( formatted_data_subject_attributes_list, @@ -208,21 +254,29 @@ def format_data_subjects( ) # calculate and format data subject rights as applicable - if data_subject_dict["rights"]: - data_subject_dict["rights_available"] = calculate_data_subject_rights( - data_subject_dict["rights"] + if data_subject.get("rights"): + data_subject["rights_available"] = calculate_data_subject_rights( + data_subject["rights"] ) else: - data_subject_dict["rights_available"] = "No data subject rights listed" + data_subject["rights_available"] = "No data subject rights listed" formatted_data_subject = { - attribute: data_subject_dict.get(attribute) or "N/A" + attribute: data_subject.get(attribute) or "N/A" for attribute in formatted_data_subject_attributes_list } - formatted_data_subjects[data_subject.fides_key] = formatted_data_subject + for k, v in data_subject.items(): + if k not in excluded_attributes: + custom_columns[k] = k + if isinstance(v, list): + formatted_data_subject[k] = ", ".join(v) + else: + formatted_data_subject[k] = v + + formatted_data_subjects[data_subject["fides_key"]] = formatted_data_subject - return formatted_data_subjects + return formatted_data_subjects, custom_columns def convert_tuple_to_string(values: Tuple[str, ...]) -> str: @@ -354,6 +408,8 @@ def get_formatted_data_protection_impact_assessment( data_protection_impact_assessment: dict, ) -> dict: "Replace None with N/A for consistent formatting of the data map" + if not isinstance(data_protection_impact_assessment, dict): + data_protection_impact_assessment = data_protection_impact_assessment.dict() return { key: ("N/A" if value is None else value) for key, value in data_protection_impact_assessment.items() diff --git a/src/fides/core/utils.py b/src/fides/core/utils.py index 8a0db447da..20cbb6ee44 100644 --- a/src/fides/core/utils.py +++ b/src/fides/core/utils.py @@ -102,9 +102,14 @@ def get_all_level_fields(fields: list) -> Iterator[DatasetField]: """ for field in fields: yield field - if field.fields: - for nested_field in get_all_level_fields(field.fields): - yield nested_field + if isinstance(field, dict): + if field["fields"]: + for nested_field in get_all_level_fields(field["fields"]): + yield nested_field + else: + if field.fields: + for nested_field in get_all_level_fields(field.fields): + yield nested_field def get_manifest_list(manifests_dir: str) -> List[str]: diff --git a/tests/ctl/api/test_datamap.py b/tests/ctl/api/test_datamap.py index f4f1b4726f..671316e8d0 100644 --- a/tests/ctl/api/test_datamap.py +++ b/tests/ctl/api/test_datamap.py @@ -3,9 +3,39 @@ from starlette.testclient import TestClient from fides.api.ctl.routes.util import API_PREFIX +from fides.api.ctl.sql_models import ( # type: ignore[attr-defined] + CustomField, + CustomFieldDefinition, +) from fides.core.config import FidesConfig +@pytest.fixture +def custom_fields(db, resources_dict): + definition = CustomFieldDefinition.create_or_update( + db=db, + data={ + "name": "my_custom_field_definition", + "description": "test", + "field_type": "string", + "resource_type": "system", + "field_definition": "string", + }, + ) + field = CustomField.create( + db=db, + data={ + "resource_type": definition.resource_type, + "resource_id": resources_dict["system"].fides_key, + "custom_field_definition_id": definition.id, + "value": "Test value 1", + }, + ) + yield + field.delete(db) + definition.delete(db) + + @pytest.mark.integration @pytest.mark.parametrize( "organization_fides_key, expected_status_code", @@ -20,7 +50,28 @@ def test_datamap( expected_status_code: int, test_client: TestClient, ) -> None: - print(test_config) + response = test_client.get( + test_config.cli.server_url + API_PREFIX + "/datamap/" + organization_fides_key, + headers=test_config.user.auth_header, + ) + assert response.status_code == expected_status_code + + +@pytest.mark.integration +@pytest.mark.parametrize( + "organization_fides_key, expected_status_code", + [ + ("fake_organization", 404), + ("default_organization", 200), + ], +) +@pytest.mark.usefixtures("custom_fields") +def test_datamap_with_custom_fields( + test_config, + organization_fides_key, + expected_status_code, + test_client, +): response = test_client.get( test_config.cli.server_url + API_PREFIX + "/datamap/" + organization_fides_key, headers=test_config.user.auth_header, diff --git a/tests/ctl/core/test_export.py b/tests/ctl/core/test_export.py index 77fbf7c132..65e05a4616 100644 --- a/tests/ctl/core/test_export.py +++ b/tests/ctl/core/test_export.py @@ -54,6 +54,51 @@ def test_sample_system_taxonomy() -> Generator[Dict, None, None]: } +@pytest.fixture() +def test_sample_system_taxonomy_with_custom_fields(): + data = { + "system": [ + System( + fides_key="test_system", + system_type="test", + system_name="test system", + system_description="system used for testing exports", + privacy_declarations=[ + PrivacyDeclaration( + name="privacy_declaration_1", + data_categories=[ + "user.contact.email", + "user.contact.name", + ], + data_use="provide.service", + data_qualifier="aggregated.anonymized", + data_subjects=["customer"], + dataset_references=["test_dataset"], + ) + ], + ).dict(), + ], + "data_subject": [DataSubject(fides_key="customer", name="customer").dict()], + "data_use": [ + DataUse( + fides_key="provide.service", name="System", parent_key="provide" + ).dict() + ], + "organization": [ + Organization( + fides_key="default_organization", + security_policy="https://www.google.com/", + ).dict() + ], + } + + data["system"][0]["custom_system_field"] = "custom system field" + data["data_subject"][0]["custom_data_subject"] = "custom data subject" + data["data_use"][0]["custom_data_use"] = "custom data use" + + yield data + + @pytest.fixture() def test_sample_dataset_taxonomy() -> Generator: yield [ @@ -104,8 +149,18 @@ def test_system_records_to_export( the header row) """ - output_list = export.generate_system_records(test_sample_system_taxonomy) - print(output_list) + output_list, _ = export.generate_system_records(test_sample_system_taxonomy) + assert len(output_list) == 3 + + +@pytest.mark.usefixtures("test_config") +def test_system_records_to_export_with_custom_fields( + test_sample_system_taxonomy_with_custom_fields, +): + + output_list, _ = export.generate_system_records( + test_sample_system_taxonomy_with_custom_fields + ) assert len(output_list) == 3 @@ -118,7 +173,7 @@ def test_system_records_to_export_sans_privacy_declaration( are returned properly (including the header row) """ test_sample_system_taxonomy["system"][0].privacy_declarations = [] - output_list = export.generate_system_records(test_sample_system_taxonomy) + output_list, _ = export.generate_system_records(test_sample_system_taxonomy) print(output_list) assert len(output_list) == 2 @@ -172,7 +227,7 @@ def test_joined_datamap_export_system_dataset_overlap( """ sample_taxonomy: Dict = test_sample_system_taxonomy sample_taxonomy["dataset"] = test_sample_dataset_taxonomy - output_list = export.build_joined_dataframe(sample_taxonomy) + output_list, _ = export.build_joined_dataframe(sample_taxonomy) assert len(output_list) == 5 @@ -239,5 +294,5 @@ def test_joined_datamap_export_system_multiple_declarations_overlap( sample_taxonomy["data_subject"].append(new_data_subject) sample_taxonomy["system"][0].privacy_declarations.append(new_declaration) sample_taxonomy["dataset"] = [] - output_list = export.build_joined_dataframe(sample_taxonomy) + output_list, _ = export.build_joined_dataframe(sample_taxonomy) assert len(output_list) == 4 diff --git a/tests/ctl/core/test_utils.py b/tests/ctl/core/test_utils.py index 09b5637aaf..27f2f7359f 100644 --- a/tests/ctl/core/test_utils.py +++ b/tests/ctl/core/test_utils.py @@ -56,8 +56,8 @@ def test_nested_fields_unpacked( """ collection = test_nested_collection_fields collected_field_names = [] - for field in utils.get_all_level_fields(collection.fields): - collected_field_names.append(field.name) + for field in utils.get_all_level_fields(collection.dict()["fields"]): + collected_field_names.append(field["name"]) assert len(collected_field_names) == 5 diff --git a/tests/ctl/database/test_crud.py b/tests/ctl/database/test_crud.py index f92a4fc5ec..5d4c083811 100644 --- a/tests/ctl/database/test_crud.py +++ b/tests/ctl/database/test_crud.py @@ -1,11 +1,19 @@ from json import dumps from typing import Generator, List +from uuid import uuid4 import pytest +from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession from fides.api.ctl import sql_models -from fides.api.ctl.database.crud import delete_resource, list_resource +from fides.api.ctl.database.crud import ( + create_resource, + delete_resource, + get_resource_with_custom_fields, + list_resource, +) +from fides.api.ctl.utils.errors import QueryError from fides.core import api as _api from fides.core.config import FidesConfig from tests.ctl.types import FixtureRequest @@ -62,3 +70,81 @@ async def test_cascade_delete_taxonomy_children( resources = await list_resource(sql_model, async_session) remaining_keys = {resource.fides_key for resource in resources} assert len(set(keys).intersection(remaining_keys)) == 0 + + +async def test_get_resource_with_custom_field(db, async_session): + system_data = { + "name": "my system", + "registry_id": "1", + "system_type": "test", + "fides_key": str(uuid4()), + } + + system = await create_resource(sql_models.System, system_data, async_session) + + custom_definition_data = { + "name": "test1", + "description": "test", + "field_type": "string", + "resource_type": "system", + "field_definition": "string", + } + + custom_field_definition = sql_models.CustomFieldDefinition.create( + db=db, data=custom_definition_data + ) + + sql_models.CustomField.create( + db=db, + data={ + "resource_type": custom_field_definition.resource_type, + "resource_id": system.fides_key, + "custom_field_definition_id": custom_field_definition.id, + "value": ["Test value 1"], + }, + ) + + sql_models.CustomField.create( + db=db, + data={ + "resource_type": custom_field_definition.resource_type, + "resource_id": system.fides_key, + "custom_field_definition_id": custom_field_definition.id, + "value": ["Test value 2"], + }, + ) + + result = await get_resource_with_custom_fields( + sql_models.System, system.fides_key, async_session + ) + + assert result["name"] == system.name + assert custom_field_definition.name in result + assert result[custom_field_definition.name] == "Test value 1, Test value 2" + + +async def test_get_resource_with_custom_field_no_custom_field(async_session): + system_data = { + "name": "my system", + "registry_id": "1", + "system_type": "test", + "fides_key": str(uuid4()), + } + + system = await create_resource(sql_models.System, system_data, async_session) + result = await get_resource_with_custom_fields( + sql_models.System, system.fides_key, async_session + ) + + assert result["name"] == system.name + + +async def test_get_resource_with_custom_field_error(async_session, monkeypatch): + async def mock_execute(*args, **kwargs): + raise SQLAlchemyError + + monkeypatch.setattr( + "fides.api.ctl.database.crud.AsyncSession.execute", mock_execute + ) + with pytest.raises(QueryError): + await get_resource_with_custom_fields(sql_models.System, "ABC", async_session) diff --git a/tests/ops/api/v1/test_main.py b/tests/ops/api/v1/test_main.py index b9e30112db..4a202589d4 100644 --- a/tests/ops/api/v1/test_main.py +++ b/tests/ops/api/v1/test_main.py @@ -13,7 +13,7 @@ def test_read_autogenerated_docs(api_client: TestClient): """Test to ensure automatically generated docs build properly""" - response = api_client.get(f"/openapi.json") + response = api_client.get("/openapi.json") assert response.status_code == 200