Skip to content

Commit

Permalink
Revoke a Pending Privacy Request [#525] (#592)
Browse files Browse the repository at this point in the history
* Add the ability to cancel a pending task.  The celery task is not actually cancelled yet.

- Track cancel reason, datetime cancelled, and add a new cancelled status.

* Add drp revoke request to postman collection.

* Add drp revoke docs.

* Update down_rev after rebase.

* Fix incorrect check.

* Restore new canceled state.

* Check that the privacy request is not canceled right before starting execution.  This is really our last chance to check before we start executing the graph in dask.    The use case here might be it was canceled shortly after it was approved.

* Attempt to revoke a queued celery task if we cancel it before it starts executing.

* Prettier.

* Changelog updated.

* Add a few unit tests around how triggering the run_privacy_request_task with a cancelled task id doesn't do anything and how you can't approve a canceled privacy request.

* Fix SQLAlchemy logging to console - logging in migration propagates to the rest of the application.

* Refresh session instead of creating a new one.

* Add 200 character limit.

* Add some assertions that db.refresh is doing what we think it's doing.
  • Loading branch information
pattisdr authored Jun 23, 2022
1 parent 3ab9c4f commit 04f9d70
Show file tree
Hide file tree
Showing 19 changed files with 299 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ The types of changes are:
* Added the ability to delete a datastore from the frontend [#683] https://github.com/ethyca/fidesops/pull/683
* Added the ability to disable/enable a datastore from the frontend [#693] https://github.com/ethyca/fidesops/pull/693
* Adds Postgres and Redis health checks to health endpoint [#690](https://github.com/ethyca/fidesops/pull/690)
* Adds the ability to revoke a pending privacy request [#592](https://github.com/ethyca/fidesops/pull/592/files)
* Added health checks and better error messages on app startup for both db and cache [#686](https://github.com/ethyca/fidesops/pull/686)
* Datastore Connection Filters [#691](https://github.com/ethyca/fidesops/pull/691)

Expand Down
4 changes: 4 additions & 0 deletions clients/admin-ui/src/features/common/RequestStatusBadge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ export const statusPropMap: {
bg: "red.500",
label: "Denied",
},
canceled: {
bg: "red.600",
label: "Canceled",
},
error: {
bg: "red.800",
label: "Error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ const RequestFilters: React.FC = () => {
<StatusOption status="approved" />
<StatusOption status="complete" />
<StatusOption status="denied" />
<StatusOption status="canceled" />
<StatusOption status="error" />
<StatusOption status="in_processing" />
<StatusOption status="paused" />
Expand Down
1 change: 1 addition & 0 deletions clients/admin-ui/src/features/privacy-requests/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export type PrivacyRequestStatus =
| "error"
| "in_processing"
| "paused"
| "canceled"
| "pending";

export enum ActionType {
Expand Down
11 changes: 11 additions & 0 deletions docs/fidesops/docs/guides/data_rights_protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,15 @@ All data rights associated with existing policies may be returned via the `/data
],
"user_relationships": null
}
```

### Revoke

You can revoke a pending privacy request via the `/revoke` endpoint.

```json title="<code>GET /api/v1/drp/revoke</code>"
{
"request_id": "c789ff35-7644-4ceb-9981-4b35c264aac3",
"reason": "Accidentally submitted"
}
```
37 changes: 37 additions & 0 deletions docs/fidesops/docs/postman/Fidesops.postman_collection.json
Original file line number Diff line number Diff line change
Expand Up @@ -3508,6 +3508,43 @@
}
},
"response": []
},
{
"name": "Revoke request",
"request": {
"auth": {
"type": "bearer",
"bearer": [
{
"key": "token",
"value": "{{client_token}}",
"type": "string"
}
]
},
"method": "POST",
"header": [],
"body": {
"mode": "raw",
"raw": "{\n \"request_id\": \"{{privacy_request_id}}\", \n \"reason\": \"Accidentally submitted\"\n\n}",
"options": {
"raw": {
"language": "json"
}
}
},
"url": {
"raw": "{{host}}/drp/revoke",
"host": [
"{{host}}"
],
"path": [
"drp",
"revoke"
]
}
},
"response": []
}
]
},
Expand Down
40 changes: 39 additions & 1 deletion src/fidesops/api/v1/endpoints/drp_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from sqlalchemy.orm import Session
from starlette.status import (
HTTP_200_OK,
HTTP_400_BAD_REQUEST,
HTTP_404_NOT_FOUND,
HTTP_422_UNPROCESSABLE_ENTITY,
HTTP_424_FAILED_DEPENDENCY,
Expand All @@ -16,14 +17,18 @@
from fidesops.api import deps
from fidesops.api.v1 import scope_registry as scopes
from fidesops.api.v1 import urn_registry as urls
from fidesops.api.v1.endpoints.privacy_request_endpoints import (
get_privacy_request_or_error,
)
from fidesops.core.config import config
from fidesops.models.policy import DrpAction, Policy
from fidesops.models.privacy_request import PrivacyRequest
from fidesops.models.privacy_request import PrivacyRequest, PrivacyRequestStatus
from fidesops.schemas.drp_privacy_request import (
DRP_VERSION,
DrpDataRightsResponse,
DrpIdentity,
DrpPrivacyRequestCreate,
DrpRevokeRequest,
)
from fidesops.schemas.privacy_request import PrivacyRequestDRPStatusResponse
from fidesops.schemas.redis_cache import PrivacyRequestIdentity
Expand Down Expand Up @@ -176,3 +181,36 @@ def get_drp_data_rights(*, db: Session = Depends(deps.get_db)) -> DrpDataRightsR
return DrpDataRightsResponse(
version=DRP_VERSION, api_base=None, actions=actions, user_relationships=None
)


@router.post(
urls.DRP_REVOKE,
dependencies=[
Security(verify_oauth_client, scopes=[scopes.PRIVACY_REQUEST_REVIEW])
],
response_model=PrivacyRequestDRPStatusResponse,
)
def revoke_request(
*, db: Session = Depends(deps.get_db), data: DrpRevokeRequest
) -> PrivacyRequestDRPStatusResponse:
"""
Revoke a pending privacy request.
"""
privacy_request: PrivacyRequest = get_privacy_request_or_error(db, data.request_id)

if privacy_request.status != PrivacyRequestStatus.pending:
raise HTTPException(
status_code=HTTP_400_BAD_REQUEST,
detail=f"Invalid revoke request. Can only revoke `pending` requests. "
f"Privacy request '{privacy_request.id}' status = {privacy_request.status.value}.",
)

logger.info(f"Canceling privacy request '{privacy_request.id}'")
privacy_request.cancel_processing(db, cancel_reason=data.reason)

return PrivacyRequestDRPStatusResponse(
request_id=privacy_request.id,
received_at=privacy_request.requested_at,
status=DrpFidesopsMapper.map_status(privacy_request.status),
reason=data.reason,
)
1 change: 1 addition & 0 deletions src/fidesops/api/v1/urn_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,4 @@
DRP_EXERCISE = "/drp/exercise"
DRP_STATUS = "/drp/status"
DRP_DATA_RIGHTS = "/drp/data-rights"
DRP_REVOKE = "/drp/revoke"
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
logging.basicConfig()
logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)
logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)

revision = "55d61eb8ed12"
down_revision = "b3b68c87c4a0"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"""cancel privacy request
Revision ID: ed1b00ff963d
Revises: 55d61eb8ed12
Create Date: 2022-06-03 15:45:14.584540
"""
import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "ed1b00ff963d"
down_revision = "55d61eb8ed12"
branch_labels = None
depends_on = None


def upgrade():
op.add_column(
"privacyrequest", sa.Column("cancel_reason", sa.String(200), nullable=True)
)
op.add_column(
"privacyrequest",
sa.Column("canceled_at", sa.DateTime(timezone=True), nullable=True),
)
op.execute("alter type privacyrequeststatus add value 'canceled'")


def downgrade():
op.drop_column("privacyrequest", "canceled_at")
op.drop_column("privacyrequest", "cancel_reason")

op.execute("delete from privacyrequest where status in ('canceled')")

op.execute("alter type privacyrequeststatus rename to privacyrequeststatus_old")
op.execute(
"create type privacyrequeststatus as enum('in_processing', 'complete', 'pending', 'error', 'paused', 'approved', 'denied')"
)
op.execute(
(
"alter table privacyrequest alter column status type privacyrequeststatus using "
"status::text::privacyrequeststatus"
)
)
op.execute("drop type privacyrequeststatus_old")
19 changes: 19 additions & 0 deletions src/fidesops/models/privacy_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
)
from fidesops.schemas.masking.masking_secrets import MaskingSecretCache
from fidesops.schemas.redis_cache import PrivacyRequestIdentity
from fidesops.tasks import celery_app
from fidesops.util.cache import (
FidesopsRedis,
get_all_cache_keys_for_privacy_request,
Expand Down Expand Up @@ -100,6 +101,7 @@ class PrivacyRequestStatus(str, EnumType):
in_processing = "in_processing"
complete = "complete"
paused = "paused"
canceled = "canceled"
error = "error"


Expand Down Expand Up @@ -158,6 +160,9 @@ class PrivacyRequest(Base): # pylint: disable=R0904
backref="privacy_requests",
)

cancel_reason = Column(String(200))
canceled_at = Column(DateTime(timezone=True), nullable=True)

# passive_deletes="all" prevents execution logs from having their privacy_request_id set to null when
# a privacy_request is deleted. We want to retain for record-keeping.
execution_logs = relationship(
Expand Down Expand Up @@ -461,6 +466,20 @@ def pause_processing(self, db: Session) -> None:
},
)

def cancel_processing(self, db: Session, cancel_reason: Optional[str]) -> None:
"""Cancels a privacy request. Currently should only cancel 'pending' tasks"""
if self.canceled_at is None:
self.status = PrivacyRequestStatus.canceled
self.cancel_reason = cancel_reason
self.canceled_at = datetime.utcnow()
self.save(db)

task_id = self.get_cached_task_id()
if task_id:
logger.info(f"Revoking task {task_id} for request {self.id}")
# Only revokes if execution is not already in progress
celery_app.control.revoke(task_id, terminate=False)

def error_processing(self, db: Session) -> None:
"""Mark privacy request as errored, and note time processing was finished"""
self.update(
Expand Down
7 changes: 7 additions & 0 deletions src/fidesops/schemas/drp_privacy_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,10 @@ class DrpDataRightsResponse(BaseSchema):
api_base: Optional[str]
actions: List[DrpAction]
user_relationships: Optional[List[str]]


class DrpRevokeRequest(BaseSchema):
"""DRP Data Rights Revoke Request Body"""

request_id: str
reason: Optional[str]
1 change: 1 addition & 0 deletions src/fidesops/service/drp/drp_fidesops_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def map_status(
PrivacyRequestStatus.complete: PrivacyRequestDRPStatus.fulfilled,
PrivacyRequestStatus.paused: PrivacyRequestDRPStatus.in_progress,
PrivacyRequestStatus.error: PrivacyRequestDRPStatus.expired,
PrivacyRequestStatus.canceled: PrivacyRequestDRPStatus.revoked,
}
try:
return PRIVACY_REQUEST_STATUS_TO_DRP_MAPPING[status]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@ def run_privacy_request(
with SessionLocal() as session:

privacy_request = PrivacyRequest.get(db=session, id=privacy_request_id)
if privacy_request.status == PrivacyRequestStatus.canceled:
logging.info(
f"Terminating privacy request {privacy_request.id}: request canceled."
)
return
logging.info(f"Dispatching privacy request {privacy_request.id}")
privacy_request.start_processing(session)

Expand Down
63 changes: 63 additions & 0 deletions tests/api/v1/endpoints/test_drp_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
from fidesops.api.v1.scope_registry import (
POLICY_READ,
PRIVACY_REQUEST_READ,
PRIVACY_REQUEST_REVIEW,
STORAGE_CREATE_OR_UPDATE,
)
from fidesops.api.v1.urn_registry import (
DRP_DATA_RIGHTS,
DRP_EXERCISE,
DRP_REVOKE,
DRP_STATUS,
V1_URL_PREFIX,
)
Expand Down Expand Up @@ -424,3 +426,64 @@ def test_get_drp_data_rights_multiple_drp_policies(
)
assert 200 == response.status_code
assert response.json() == expected_response


class TestDrpRevoke:
@pytest.fixture(scope="function")
def url(self) -> str:
return V1_URL_PREFIX + DRP_REVOKE

def test_revoke_not_authenticated(
self, api_client: TestClient, privacy_request, url
):
response = api_client.post(url, headers={})
assert 401 == response.status_code

def test_revoke_wrong_scope(
self, api_client: TestClient, generate_auth_header, url
):
auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_READ])
response = api_client.post(url, headers=auth_header, json={})
assert 403 == response.status_code

def test_revoke_wrong_status(
self, db, api_client: TestClient, generate_auth_header, url, privacy_request
):
auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_REVIEW])
response = api_client.post(
url, headers=auth_header, json={"request_id": privacy_request.id}
)
assert 400 == response.status_code
assert response.json()[
"detail"
] == "Invalid revoke request. Can only revoke `pending` requests. Privacy request '{}' status = in_processing.".format(
privacy_request.id
)
db.refresh(privacy_request)
assert privacy_request.status == PrivacyRequestStatus.in_processing
assert privacy_request.canceled_at is None

def test_revoke(
self, db, api_client: TestClient, generate_auth_header, url, privacy_request
):
privacy_request.status = PrivacyRequestStatus.pending
privacy_request.save(db)
canceled_reason = "Accidentally submitted"

auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_REVIEW])
response = api_client.post(
url,
headers=auth_header,
json={"request_id": privacy_request.id, "reason": canceled_reason},
)
assert 200 == response.status_code
db.refresh(privacy_request)

assert privacy_request.status == PrivacyRequestStatus.canceled
assert privacy_request.cancel_reason == canceled_reason
assert privacy_request.canceled_at is not None

data = response.json()
assert data["request_id"] == privacy_request.id
assert data["status"] == "revoked"
assert data["reason"] == canceled_reason
Loading

0 comments on commit 04f9d70

Please sign in to comment.