diff --git a/docs/fidesops/docs/guides/policies.md b/docs/fidesops/docs/guides/policies.md index 99db8f3b2..64cee4718 100644 --- a/docs/fidesops/docs/guides/policies.md +++ b/docs/fidesops/docs/guides/policies.md @@ -42,12 +42,22 @@ PATCH /api/v1/policy [ { "name": "User Email Address", - "key": "user_email_address_polcy" + "key": "user_email_address_polcy", + "drp_action": "access" // optional } ] ``` This policy is subtly different from the concept of a Policy in [Fidesctl](https://github.com/ethyca/fides). A [Fidesctl policy](https://ethyca.github.io/fides/language/resources/policy/) dictates which data categories can be stored where. A Fidesops policy, on the other hand, dictates how to access, mask or erase data that matches specific data categories for privacy requests. +### Policy Attributes +| Attribute | Description | +|---|---| +| `Policy.name` | User-friendly name for your Policy. | +| `Policy.key` | Unique key by which to reference the Policy. | +| `Policy.drp_action` | Optional. A [Data Rights Protocol](https://github.com/consumer-reports-digital-lab/data-rights-protocol) action to associate to this policy. | +| `access` | A data subject access request. Should be used with an `access` Rule. | +| `deletion` | A data subject erasure request. Should be used with an `erasure` Rule. | + ## Add an Access Rule to your Policy The policy creation operation returns a Policy key, which we'll use to add a Rule: diff --git a/src/fidesops/api/v1/endpoints/policy_endpoints.py b/src/fidesops/api/v1/endpoints/policy_endpoints.py index 6bbd15ec7..83cf52c19 100644 --- a/src/fidesops/api/v1/endpoints/policy_endpoints.py +++ b/src/fidesops/api/v1/endpoints/policy_endpoints.py @@ -20,6 +20,7 @@ PolicyValidationError, RuleTargetValidationError, RuleValidationError, + DrpActionValidationError, ) from fidesops.models.client import ClientDetail from fidesops.models.policy import ActionType, Policy, Rule, RuleTarget @@ -114,9 +115,14 @@ def create_or_update_policies( "name": policy_data["name"], "key": policy_data.get("key"), "client_id": client.id, + "drp_action": policy_data.get("drp_action"), }, ) - except KeyOrNameAlreadyExists as exc: + except ( + KeyOrNameAlreadyExists, + DrpActionValidationError, + IntegrityError, + ) as exc: logger.warning("Create/update failed for policy: %s", exc) failure = { "message": exc.args[0], diff --git a/src/fidesops/common_exceptions.py b/src/fidesops/common_exceptions.py index 5f4547766..1659a5326 100644 --- a/src/fidesops/common_exceptions.py +++ b/src/fidesops/common_exceptions.py @@ -83,6 +83,10 @@ class KeyOrNameAlreadyExists(Exception): """A resource already exists with this key or name.""" +class DrpActionValidationError(Exception): + """A resource already exists with this DRP Action.""" + + class KeyValidationError(Exception): """The resource you're trying to create has a key specified but not a name specified.""" diff --git a/src/fidesops/db/base_class.py b/src/fidesops/db/base_class.py index 263fee1dc..152adeffa 100644 --- a/src/fidesops/db/base_class.py +++ b/src/fidesops/db/base_class.py @@ -176,14 +176,10 @@ def create(cls, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: return cls.persist_obj(db, db_obj) @classmethod - def create_or_update(cls, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: - """ - Create an object, or update the existing version. There's an edge case where - `data["id"]` and `data["key"]` can point at different records, in which case - this method will attempt to update the fetched record with the key of another, - and a `KeyOrNameAlreadyExists` error will be thrown. If neither `key`, nor `id` are - passed in, leave `db_obj` as None and assume we are creating a new object. - """ + def get_by_key_or_id( + cls, db: Session, *, data: Dict[str, Any] + ) -> Optional[FidesopsBase]: + """Retrieves db object by id, if provided, otherwise attempts by key""" db_obj = None if data.get("id") is not None: # If `id` has been included in `data`, preference that @@ -191,12 +187,22 @@ def create_or_update(cls, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: elif data.get("key") is not None: # Otherwise, try with `key` db_obj = cls.get_by(db=db, field="key", value=data["key"]) + return db_obj + @classmethod + def create_or_update(cls, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: + """ + Create an object, or update the existing version. There's an edge case where + `data["id"]` and `data["key"]` can point at different records, in which case + this method will attempt to update the fetched record with the key of another, + and a `KeyOrNameAlreadyExists` error will be thrown. If neither `key`, nor `id` are + passed in, leave `db_obj` as None and assume we are creating a new object. + """ + db_obj: FidesopsBase = cls.get_by_key_or_id(db=db, data=data) if db_obj: db_obj.update(db=db, data=data) else: db_obj = cls.create(db=db, data=data) - return db_obj @classmethod diff --git a/src/fidesops/models/policy.py b/src/fidesops/models/policy.py index bdd7c9f01..e72f2f19a 100644 --- a/src/fidesops/models/policy.py +++ b/src/fidesops/models/policy.py @@ -60,6 +60,21 @@ class ActionType(EnumType): update = "update" +class DrpAction(EnumType): + """ + Enum to hold valid DRP actions. For more details, see: + https://github.com/consumer-reports-digital-lab/data-rights-protocol#301-supported-rights-actions + """ + + access = "access" + deletion = "deletion" + # below are not supported + sale_opt_out = "sale:opt_out" + sale_opt_in = "sale:opt_in" + access_categories = "access:categories" + access_specific = "access:specific" + + PseudonymizationPolicy = SupportedMaskingStrategies """ *Deprecated*: The method by which to pseudonymize data. @@ -70,6 +85,21 @@ class is referenced in multiple database migrations. This class is to be removed """ +def _validate_drp_action(drp_action: Optional[str]) -> None: + """Check that DRP action is supported""" + if not drp_action: + return + if drp_action in [ + DrpAction.sale_opt_in.value, + DrpAction.sale_opt_out.value, + DrpAction.access_categories.value, + DrpAction.access_specific.value, + ]: + raise common_exceptions.DrpActionValidationError( + f"{drp_action} action is not supported at this time." + ) + + def _validate_rule( action_type: Optional[str], storage_destination_id: Optional[str], @@ -78,22 +108,20 @@ def _validate_rule( """Check that the rule's action_type and storage_destination are valid.""" if not action_type: raise common_exceptions.RuleValidationError("action_type is required.") - - if action_type == ActionType.erasure.value and storage_destination_id is not None: - raise common_exceptions.RuleValidationError( - "Erasure Rules cannot have storage destinations." - ) - - if action_type == ActionType.erasure.value and masking_strategy is None: - raise common_exceptions.RuleValidationError( - "Erasure Rules must have masking strategies." - ) - - if action_type == ActionType.access.value and storage_destination_id is None: - raise common_exceptions.RuleValidationError( - "Access Rules must have a storage destination." - ) - + if action_type == ActionType.erasure.value: + if storage_destination_id is not None: + raise common_exceptions.RuleValidationError( + "Erasure Rules cannot have storage destinations." + ) + if masking_strategy is None: + raise common_exceptions.RuleValidationError( + "Erasure Rules must have masking strategies." + ) + if action_type == ActionType.access.value: + if storage_destination_id is None: + raise common_exceptions.RuleValidationError( + "Access Rules must have a storage destination." + ) if action_type in [ActionType.consent.value, ActionType.update.value]: raise common_exceptions.RuleValidationError( f"{action_type} Rules are not supported at this time." @@ -105,6 +133,7 @@ class Policy(Base): name = Column(String, unique=True, nullable=False) key = Column(String, index=True, unique=True, nullable=False) + drp_action = Column(EnumColumn(DrpAction), index=True, unique=True, nullable=True) client_id = Column( String, ForeignKey(ClientDetail.id_field_path), @@ -115,6 +144,19 @@ class Policy(Base): backref="policies", ) # Which client created the Policy + @classmethod + def create_or_update(cls, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: + """Overrides base create or update to add custom error for drp action already exists""" + db_obj = cls.get_by_key_or_id(db=db, data=data) + if hasattr(cls, "drp_action"): + data["drp_action"] = data.get("drp_action", None) + _validate_drp_action(data["drp_action"]) + if db_obj: + db_obj.update(db=db, data=data) + else: + db_obj = cls.create(db=db, data=data) + return db_obj + def delete(self, db: Session) -> Optional[FidesopsBase]: """Cascade delete all rules on deletion of a Policy.""" _ = [rule.delete(db=db) for rule in self.rules] @@ -419,7 +461,7 @@ def update(self, db: Session, *, data: Dict[str, Any]) -> FidesopsBase: """Validate data_category on object update.""" updated_data_category = data.get("data_category") if data.get("name") is None: - # Don't pass explciit `None` through for `name` because the field + # Don't pass explcit `None` through for `name` because the field # is non-nullable del data["name"] diff --git a/src/fidesops/schemas/policy.py b/src/fidesops/schemas/policy.py index ea1f8a95c..8ee82a9b1 100644 --- a/src/fidesops/schemas/policy.py +++ b/src/fidesops/schemas/policy.py @@ -3,6 +3,7 @@ from fidesops.models.policy import ( ActionType, + DrpAction, ) from fidesops.schemas.api import BulkResponse, BulkUpdateFailed from fidesops.schemas.base_class import BaseSchema @@ -86,12 +87,20 @@ class Policy(BaseSchema): name: str key: Optional[FidesOpsKey] + drp_action: Optional[DrpAction] + + class Config: + """Populate models with the raw value of enum fields, rather than the enum itself""" + + use_enum_values = True + orm_mode = True class PolicyResponse(Policy): """A holistic view of a Policy record, including all foreign keys by default.""" rules: Optional[List[RuleResponse]] + drp_action: Optional[DrpAction] class BulkPutRuleTargetResponse(BulkResponse): diff --git a/src/migrations/versions/5078badb90b9_adds_drp_action_to_policy.py b/src/migrations/versions/5078badb90b9_adds_drp_action_to_policy.py new file mode 100644 index 000000000..8b5d282ea --- /dev/null +++ b/src/migrations/versions/5078badb90b9_adds_drp_action_to_policy.py @@ -0,0 +1,40 @@ +"""adds DRP action to policy + +Revision ID: 5078badb90b9 +Revises: c98da12d76f8 +Create Date: 2022-05-04 17:22:46.500067 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +from sqlalchemy.dialects import postgresql + +revision = "5078badb90b9" +down_revision = "c98da12d76f8" +branch_labels = None +depends_on = None + + +def upgrade(): + drpaction = postgresql.ENUM( + "access", + "deletion", + "sale_opt_out", + "sale_opt_in", + "access_categories", + "access_specific", + name="drpaction", + create_type=False, + ) + drpaction.create(op.get_bind()) + op.add_column("policy", sa.Column("drp_action", drpaction, nullable=True)) + op.create_index(op.f("ix_policy_drp_action"), "policy", ["drp_action"], unique=True) + + +def downgrade(): + op.drop_index(op.f("ix_policy_drp_action"), table_name="policy") + op.drop_column("policy", "drp_action") + op.execute("DROP TYPE drpaction;") diff --git a/tests/api/v1/endpoints/test_policy_endpoints.py b/tests/api/v1/endpoints/test_policy_endpoints.py index d26c58e05..b6bc48ba1 100644 --- a/tests/api/v1/endpoints/test_policy_endpoints.py +++ b/tests/api/v1/endpoints/test_policy_endpoints.py @@ -19,7 +19,7 @@ ActionType, Policy, Rule, - RuleTarget, + RuleTarget, DrpAction, ) from fidesops.service.masking.strategy.masking_strategy_nullify import NULL_REWRITE from fidesops.util.data_category import DataCategory, generate_fides_data_categories @@ -142,6 +142,27 @@ def test_get_invalid_policy( ) assert resp.status_code == 404 + def test_get_policy_returns_drp_action( + self, api_client: TestClient, generate_auth_header, policy_drp_action, url + ): + auth_header = generate_auth_header(scopes=[scopes.POLICY_READ]) + resp = api_client.get( + V1_URL_PREFIX + POLICY_DETAIL_URI.format(policy_key=policy_drp_action.key), + headers=auth_header, + ) + assert resp.status_code == 200 + data = resp.json() + print(json.dumps(resp.json(), indent=2)) + assert data["key"] == policy_drp_action.key + assert data["drp_action"] == DrpAction.access.value + assert "rules" in data + assert len(data["rules"]) == 1 + + rule = data["rules"][0] + assert rule["key"] == "access_request_rule_drp" + assert rule["action_type"] == "access" + assert rule["storage_destination"]["type"] == "s3" + def test_get_policy_returns_rules( self, api_client: TestClient, generate_auth_header, policy, url ): @@ -303,6 +324,136 @@ def test_create_policy_with_duplicate_key( data = resp.json() assert len(data["failed"]) == 2 + def test_create_policy_with_duplicate_drp_action( + self, + url, + api_client: TestClient, + generate_auth_header, + policy_drp_action, + storage_config, + ): + data = [ + { + "name": "policy with pre-existing drp action", + "action_type": ActionType.access.value, + "drp_action": DrpAction.access.value + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + resp = api_client.patch(url, json=data, headers=auth_header) + assert resp.status_code == 200 + + data = resp.json() + assert len(data["failed"]) == 1 + + def test_update_policy_with_duplicate_drp_action( + self, + db, + url, + api_client: TestClient, + generate_auth_header, + policy_drp_action, + storage_config, + ): + # creates a new drp policy + data = [ + { + "key": "erasure_drp_policy", + "name": "erasure drp policy", + "action_type": ActionType.erasure.value, + "drp_action": DrpAction.deletion.value + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + valid_drp_resp = api_client.patch(url, json=data, headers=auth_header) + valid_response_data = valid_drp_resp.json()["succeeded"] + assert valid_drp_resp.status_code == 200 + + # try to update the above policy with a pre-existing drp action + data = [ + { + "key": "erasure_drp_policy", + "name": "policy with pre-existing drp action", + "action_type": ActionType.access.value, + "drp_action": DrpAction.access.value + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + resp = api_client.patch(url, json=data, headers=auth_header) + assert resp.status_code == 200 + + data = resp.json() + assert len(data["failed"]) == 1 + + pol = Policy.filter( + db=db, conditions=(Policy.key == valid_response_data[0]["key"]) + ).first() + pol.delete(db=db) + + def test_update_policy_with_drp_action( + self, + url, + api_client: TestClient, + generate_auth_header, + policy, + storage_config, + ): + data = [ + { + "key": policy.key, + "name": "updated name", + "action_type": ActionType.access.value, + "drp_action": DrpAction.access.value + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + 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 + + def test_create_policy_invalid_drp_action( + self, url, api_client: TestClient, payload, generate_auth_header, storage_config + ): + payload = [ + { + "name": "policy 1", + "action_type": "erasure", + "drp_action": "invalid", + "data_category": DataCategory("user.provided.identifiable").value, + "storage_destination_key": storage_config.key, + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + resp = api_client.patch(url, json=payload, headers=auth_header) + assert resp.status_code == 422 + + response_body = json.loads(resp.text) + assert "value is not a valid enumeration member; permitted: 'access', 'deletion', 'sale:opt_out', 'sale:opt_in', 'access:categories', 'access:specific'" == response_body["detail"][0]["msg"] + + def test_create_policy_with_drp_action( + self, db, url, api_client: TestClient, payload, generate_auth_header, storage_config + ): + payload = [ + { + "name": "policy 1", + "action_type": "erasure", + "drp_action": "deletion", + "data_category": DataCategory("user.provided.identifiable").value, + "storage_destination_key": storage_config.key, + } + ] + auth_header = generate_auth_header(scopes=[scopes.POLICY_CREATE_OR_UPDATE]) + resp = api_client.patch(url, json=payload, 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_creates_key( self, db, api_client: TestClient, generate_auth_header, storage_config, url ): @@ -436,6 +587,28 @@ def test_create_rules_invalid_policy( resp = api_client.patch(url, headers=auth_header, json=data) assert resp.status_code == 404 + def test_create_rules_mismatching_drp_policy( + self, api_client: TestClient, generate_auth_header, policy_drp_action, storage_config + ): + data = [ + { + "name": "test access rule", + "action_type": ActionType.erasure.value, + "storage_destination_key": storage_config.key, + } + ] + url = V1_URL_PREFIX + RULE_CREATE_URI.format(policy_key=policy_drp_action.key) + auth_header = generate_auth_header(scopes=[scopes.RULE_CREATE_OR_UPDATE]) + resp = api_client.patch( + url, + json=data, + headers=auth_header, + ) + + assert resp.status_code == 200 + response_data = resp.json()["failed"] + assert len(response_data) == 1 + def test_create_rules_limit_exceeded( self, api_client: TestClient, generate_auth_header, url, storage_config ): diff --git a/tests/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/api/v1/endpoints/test_privacy_request_endpoints.py index e536991cc..827669687 100644 --- a/tests/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/api/v1/endpoints/test_privacy_request_endpoints.py @@ -478,6 +478,7 @@ def test_get_privacy_requests_by_id( "reviewed_by": None, "reviewer": None, "policy": { + "drp_action": None, "name": privacy_request.policy.name, "key": privacy_request.policy.key, }, @@ -522,6 +523,7 @@ def test_get_privacy_requests_by_partial_id( "reviewed_by": None, "reviewer": None, "policy": { + "drp_action": None, "name": privacy_request.policy.name, "key": privacy_request.policy.key, }, @@ -790,6 +792,7 @@ def test_verbose_privacy_requests( "reviewed_by": None, "reviewer": None, "policy": { + "drp_action": None, "name": privacy_request.policy.name, "key": privacy_request.policy.key, }, @@ -1518,6 +1521,7 @@ def test_resume_privacy_request( "reviewed_by": None, "reviewer": None, "policy": { + "drp_action": None, "key": privacy_request.policy.key, "name": privacy_request.policy.name, }, diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index dd4054705..ca9e9a894 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -513,6 +513,56 @@ def policy( pass +@pytest.fixture(scope="function") +def policy_drp_action( + db: Session, + oauth_client: ClientDetail, + storage_config: StorageConfig, +) -> Generator: + access_request_policy = Policy.create( + db=db, + data={ + "name": "example access request policy drp", + "key": "example_access_request_policy_drp", + "drp_action": "access", + "client_id": oauth_client.id, + }, + ) + + access_request_rule = Rule.create( + db=db, + data={ + "action_type": ActionType.access.value, + "client_id": oauth_client.id, + "name": "Access Request Rule DRP", + "policy_id": access_request_policy.id, + "storage_destination_id": storage_config.id, + }, + ) + + rule_target = RuleTarget.create( + db=db, + data={ + "client_id": oauth_client.id, + "data_category": DataCategory("user.provided.identifiable").value, + "rule_id": access_request_rule.id, + }, + ) + yield access_request_policy + try: + rule_target.delete(db) + except ObjectDeletedError: + pass + try: + access_request_rule.delete(db) + except ObjectDeletedError: + pass + try: + access_request_policy.delete(db) + except ObjectDeletedError: + pass + + @pytest.fixture(scope="function") def erasure_policy_string_rewrite( db: Session,