From c51cd090388bd0a2d3c0877a14f0b085196f6ea0 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 20 Sep 2022 04:53:29 -0500 Subject: [PATCH] Fix Corner Case: Retrieving Access Results from Cache on Retry [#1348] (#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. --- CHANGELOG.md | 1 + src/fidesops/ops/task/task_resources.py | 6 ++++-- tests/ops/task/test_task_resources.py | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b995cd94..ab44a4832 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/fidesops/ops/task/task_resources.py b/src/fidesops/ops/task/task_resources.py index cf44a8f3c..b0ad15c42 100644 --- a/src/fidesops/ops/task/task_resources.py +++ b/src/fidesops/ops/task/task_resources.py @@ -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()} diff --git a/tests/ops/task/test_task_resources.py b/tests/ops/task/test_task_resources.py index 22472815d..61ae17efb 100644 --- a/tests/ops/task/test_task_resources.py +++ b/tests/ops/task/test_task_resources.py @@ -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 ):