diff --git a/CHANGELOG.md b/CHANGELOG.md index da8b2ac969..5b7794c601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ The types of changes are: ## [Unreleased](https://github.com/ethyca/fides/compare/2.29.0...main) +### Fixed +- Fixing issue when modifying Policies, Rules, or RuleTargets as a root user [#4582](https://github.com/ethyca/fides/pull/4582) + ## [2.29.0](https://github.com/ethyca/fides/compare/2.28.0...2.29.0) ### Added diff --git a/src/fides/api/alembic/migrations/versions/68cb26f3492d_remove_client_id_constraint.py b/src/fides/api/alembic/migrations/versions/68cb26f3492d_remove_client_id_constraint.py new file mode 100644 index 0000000000..e4cc86257e --- /dev/null +++ b/src/fides/api/alembic/migrations/versions/68cb26f3492d_remove_client_id_constraint.py @@ -0,0 +1,23 @@ +"""remove client id constraint + +Revision ID: 68cb26f3492d +Revises: 956d21f13def +Create Date: 2024-02-01 18:07:16.757838 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "68cb26f3492d" +down_revision = "956d21f13def" +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column("policy", "client_id", existing_type=sa.VARCHAR(), nullable=True) + + +def downgrade(): + op.alter_column("policy", "client_id", existing_type=sa.VARCHAR(), nullable=False) diff --git a/src/fides/api/api/v1/endpoints/policy_endpoints.py b/src/fides/api/api/v1/endpoints/policy_endpoints.py index 9b2159c2f0..dfee67afb1 100644 --- a/src/fides/api/api/v1/endpoints/policy_endpoints.py +++ b/src/fides/api/api/v1/endpoints/policy_endpoints.py @@ -121,7 +121,6 @@ def create_or_update_policies( data={ "name": policy_data["name"], "key": policy_data.get("key"), - "client_id": client.id, "drp_action": policy_data.get("drp_action"), "execution_timeframe": policy_data.get("execution_timeframe"), }, @@ -290,7 +289,6 @@ def create_or_update_rules( db=db, data={ "action_type": schema.action_type, - "client_id": client.id, "key": schema.key, "name": schema.name, "policy_id": policy.id, @@ -523,7 +521,6 @@ def create_or_update_rule_targets( "key": schema.key, "data_category": schema.data_category, "rule_id": rule.id, - "client_id": client.id, }, ) except KeyOrNameAlreadyExists as exc: diff --git a/src/fides/api/models/policy.py b/src/fides/api/models/policy.py index 72b4b44ae6..813e460206 100644 --- a/src/fides/api/models/policy.py +++ b/src/fides/api/models/policy.py @@ -88,7 +88,7 @@ class Policy(Base): client_id = Column( String, ForeignKey(ClientDetail.id_field_path), - nullable=False, + nullable=True, ) client = relationship( ClientDetail, diff --git a/tests/ops/api/v1/endpoints/test_policy_endpoints.py b/tests/ops/api/v1/endpoints/test_policy_endpoints.py index 313280a632..c7b0c879a3 100644 --- a/tests/ops/api/v1/endpoints/test_policy_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_policy_endpoints.py @@ -805,6 +805,28 @@ def test_create_policy_creates_key( ).first() pol.delete(db=db) + def test_create_policy_as_root( + self, db, api_client: TestClient, root_auth_header, storage_config, url + ): + data = [ + { + "name": "test create policy api", + "action_type": "erasure", + "data_category": DataCategory("user").value, + "storage_destination_key": storage_config.key, + } + ] + auth_header = root_auth_header + resp = api_client.patch(url, json=data, headers=auth_header) + assert resp.status_code == 200 + response_data = resp.json()["succeeded"] + assert len(response_data) == 1 + + pol = Policy.filter( + db=db, conditions=(Policy.key == response_data[0]["key"]) + ).first() + pol.delete(db=db) + def test_create_policy_with_key( self, url, @@ -1020,6 +1042,36 @@ def test_create_access_rule_for_policy( assert "key" in rule_data["storage_destination"] assert "secrets" not in rule_data["storage_destination"] + def test_create_access_rule_for_policy_as_root( + self, + api_client: TestClient, + url, + root_auth_header, + policy, + storage_config, + ): + data = [ + { + "name": "test access rule", + "action_type": ActionType.access.value, + "storage_destination_key": storage_config.key, + } + ] + auth_header = root_auth_header + resp = api_client.patch( + url, + json=data, + headers=auth_header, + ) + + assert resp.status_code == 200 + response_data = resp.json()["succeeded"] + assert len(response_data) == 1 + rule_data = response_data[0] + assert "storage_destination" in rule_data + assert "key" in rule_data["storage_destination"] + assert "secrets" not in rule_data["storage_destination"] + def test_create_access_rule_for_policy_no_storage_specified( self, api_client: TestClient, @@ -1234,6 +1286,32 @@ def test_create_rule_targets( response_data = resp.json()["succeeded"] assert len(response_data) == 2 + def test_create_rule_targets_as_root( + self, + api_client: TestClient, + root_auth_header, + policy, + ): + rule = policy.rules[0] + data = [ + { + "data_category": DataCategory("user.name").value, + }, + { + "data_category": DataCategory("user.contact.email").value, + }, + ] + auth_header = root_auth_header + resp = api_client.patch( + self.get_rule_url(policy.key, rule.key), + json=data, + headers=auth_header, + ) + + assert resp.status_code == 200 + response_data = resp.json()["succeeded"] + assert len(response_data) == 2 + def test_create_rule_target_with_custom_category( self, api_client: TestClient,