Skip to content

Commit

Permalink
414- adds DRP action to Policy (#453)
Browse files Browse the repository at this point in the history
* adds drp action type to db, adds docs, tests

* fix lint, tests

* format

* remove unneeded func arg

* lint

* more lint

* CR changes

* update alembic down migration number

* minor docs updates for drp

* integration tests and updating alembic

* catches integrity error at endpoint level

* lint

Co-authored-by: Cole Isaac <cole@ethyca.com>
  • Loading branch information
eastandwestwind and Cole Isaac authored May 7, 2022
1 parent dd4aac8 commit b4d6f0b
Show file tree
Hide file tree
Showing 10 changed files with 373 additions and 29 deletions.
12 changes: 11 additions & 1 deletion docs/fidesops/docs/guides/policies.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` | <b>Optional.</b> 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:

Expand Down
8 changes: 7 additions & 1 deletion src/fidesops/api/v1/endpoints/policy_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
PolicyValidationError,
RuleTargetValidationError,
RuleValidationError,
DrpActionValidationError,
)
from fidesops.models.client import ClientDetail
from fidesops.models.policy import ActionType, Policy, Rule, RuleTarget
Expand Down Expand Up @@ -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],
Expand Down
4 changes: 4 additions & 0 deletions src/fidesops/common_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
24 changes: 15 additions & 9 deletions src/fidesops/db/base_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,27 +176,33 @@ 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
db_obj = cls.get(db=db, id=data["id"])
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
Expand Down
76 changes: 59 additions & 17 deletions src/fidesops/models/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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],
Expand All @@ -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."
Expand All @@ -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),
Expand All @@ -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]
Expand Down Expand Up @@ -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"]

Expand Down
9 changes: 9 additions & 0 deletions src/fidesops/schemas/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
40 changes: 40 additions & 0 deletions src/migrations/versions/5078badb90b9_adds_drp_action_to_policy.py
Original file line number Diff line number Diff line change
@@ -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;")
Loading

0 comments on commit b4d6f0b

Please sign in to comment.