Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Commit

Permalink
Fix Corner Case: Retrieving Access Results from Cache on Retry [#1348] (
Browse files Browse the repository at this point in the history
#1349)

* Fix bug where erasure count confirmations may be returned instead of selected access request results when attempting to get access requests from the cache.

Because erasure requests are run after access requests, there normally isn't erasure results in the cache when we attempt to get access results.  However, if we were retrying a privacy request from a checkpoint that had already erased data, we could possibly return erasure results instead of access results, depending on if the access or erasure key was accessed last.

* Update CHANGELOG.
  • Loading branch information
pattisdr authored Sep 20, 2022
1 parent ccbba9f commit 301feb6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ The types of changes are:
* Remove masking of redis error log [#1288](https://github.com/ethyca/fidesops/pull/1288)
* Logout with malformed or expired token [#1305](https://github.com/ethyca/fidesops/pull/1305)
* The `toml` package is now included in the list of direct dependencies (`requirements.txt`) [#1338](https://github.com/ethyca/fidesops/pull/1338)
* Fix bug where erasure counts instead of access results may be retrieved for certain collections on request retry [#1349](https://github.com/ethyca/fidesops/pull/1349)

### Security

Expand Down
6 changes: 4 additions & 2 deletions src/fidesops/ops/task/task_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,10 @@ def cache_object(self, key: str, value: Any) -> None:
self.cache.set_encoded_object(f"{self.request.id}__{key}", value)

def get_all_cached_objects(self) -> Dict[str, Optional[List[Row]]]:
"""Retrieve the results of all steps (cache_object)"""
value_dict = self.cache.get_encoded_objects_by_prefix(self.request.id)
"""Retrieve the access results of all steps (cache_object)"""
value_dict = self.cache.get_encoded_objects_by_prefix(
f"{self.request.id}__access_request"
)
# extract request id to return a map of address:value
return {k.split("__")[-1]: v for k, v in value_dict.items()}

Expand Down
24 changes: 24 additions & 0 deletions tests/ops/task/test_task_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@


class TestTaskResources:
def test_cache_object(self, db, privacy_request, policy, integration_manual_config):
resources = TaskResources(
privacy_request, policy, [integration_manual_config], db
)

assert resources.get_all_cached_objects() == {}

resources.cache_object(
"access_request__postgres_example:customer", [{"id": 1, "last_name": "Doe"}]
)
resources.cache_object(
"access_request__postgres_example:payment",
[{"id": 2, "ccn": "111-111-1111-1111", "customer_id": 1}],
)
resources.cache_erasure("manual_example:filing-cabinet", 2)

# Only access results from "cache_object" are returned
assert resources.get_all_cached_objects() == {
"postgres_example:payment": [
{"id": 2, "ccn": "111-111-1111-1111", "customer_id": 1}
],
"postgres_example:customer": [{"id": 1, "last_name": "Doe"}],
}

def test_cache_erasure(
self, db, privacy_request, policy, integration_manual_config
):
Expand Down

0 comments on commit 301feb6

Please sign in to comment.