Skip to content

Commit

Permalink
Fixing root user client ID (#4582)
Browse files Browse the repository at this point in the history
  • Loading branch information
galvana authored Feb 2, 2024
1 parent 9604e9b commit 3bacb77
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 0 additions & 3 deletions src/fides/api/api/v1/endpoints/policy_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/fides/api/models/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Policy(Base):
client_id = Column(
String,
ForeignKey(ClientDetail.id_field_path),
nullable=False,
nullable=True,
)
client = relationship(
ClientDetail,
Expand Down
78 changes: 78 additions & 0 deletions tests/ops/api/v1/endpoints/test_policy_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3bacb77

Please sign in to comment.