-
Notifications
You must be signed in to change notification settings - Fork 16
Enable Manual Webhooks in Request Execution [#1228] #1285
Enable Manual Webhooks in Request Execution [#1228] #1285
Conversation
…lar privacy request.
…a manual webhook for a given privacy request with "requires_input" status.
…ebhook. - Add new scopes for the endpoints to upload/view manual data for webhooks. - Enforce that at least one field is added when defining a manual webhook, and add a fallback if no fields were defined.
…tus once all input has been added. None of the fields are required, but the a key for each manual webhook still needs to exist in the cache to proceed. As part of request execution check if data has been uploaded (data can be empty) for all manual webhooks. If True, we can proceed with request execution, otherwise, we put the PrivacyRequest in "requires_input" status and exits. Also adds the manual data uploaded directly to the packet we upload to the user at the very end.
…vacy request when getting its status.
…webhook_execution # Conflicts: # CHANGELOG.md
manual_data, proceed = get_access_manual_webhook_inputs( | ||
session, privacy_request | ||
) | ||
if not proceed: | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the primary change of this PR - this puts a PrivacyRequest in requires_input
status if we don't have every manual webhook confirmed. If we do have confirmation from all our manual webhooks, we retrieve any manually uploaded data, and this is passed onto the upload_access_results
function and data is combined with the automatically retrieved data at the end.
def get_access_manual_webhook_inputs( | ||
db: Session, privacy_request: PrivacyRequest | ||
) -> Tuple[Dict[str, List[Dict[str, Optional[Any]]]], bool]: | ||
"""Retrieves manually uploaded data for all AccessManualWebhooks and formats in a way | ||
to match automatically retrieved data. Also returns if execution should proceed. | ||
|
||
This data will be uploaded to the user as-is, without filtering. | ||
""" | ||
manual_inputs: Dict[str, List[Dict[str, Optional[Any]]]] = {} | ||
|
||
try: | ||
for manual_webhook in AccessManualWebhook.get_enabled(db): | ||
manual_inputs[manual_webhook.connection_config.key] = [ | ||
privacy_request.get_manual_webhook_input(manual_webhook) | ||
] | ||
except (MissingManualWebhookData, ValidationError) as exc: | ||
logger.info(exc) | ||
privacy_request.status = PrivacyRequestStatus.requires_input | ||
privacy_request.save(db) | ||
return manual_inputs, False | ||
|
||
return manual_inputs, True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally the access results have keys in the format "dataset:collection", so here I am using the manual_webhook
connectionconfig key instead, since there is no "dataset" or "collection" defined for the webhook results. I think an overlap with existing keys is unlikely since they are mashups of dataset and collection names.
As an aside, since ConnectionConfigs and AccessManualWebhooks have a 1:1 relationship, I am not giving a separate identifier to the AccessManualWebhook, and using its ConnectionConfig key everywhere. So the question is, do we want them to be separate? We could store a separate AccessManualWebhook key and use that here, and on the UI it could be surfaced connection_identifier
, like we use for saas configs. I didn't think we needed this extra identifier, but I can see an opposing argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with Sean, seems okay for now to have this 1:1 mapping and use the connection_config key as the identifier for the entire webhook.
src/fidesops/ops/service/privacy_request/request_runner_service.py
Outdated
Show resolved
Hide resolved
…ly has erasure rules.
…webhook_execution # Conflicts: # CHANGELOG.md
) | ||
PRIVACY_REQUEST_REVIEW = "privacy-request:review" | ||
PRIVACY_REQUEST_UPLOAD_DATA = "privacy-request:upload_data" | ||
PRIVACY_REQUEST_VIEW_DATA = "privacy-request:view_data" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we also need to add new scopes in fideslib at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sanders41 are we keeping fideslib scopes synced with fidesops scopes with the fides unification work, or is it okay that this is the most current list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been trying to keep them in sync, but it won't break if they get out of sync for now.
src/fidesops/ops/service/privacy_request/request_runner_service.py
Outdated
Show resolved
Hide resolved
Nice work on this @pattisdr ! Learning some new things around pydantic models and types. Just a couple Qs to address. Tagging @conceptualshark also to review docs. |
…webhook_execution # Conflicts: # CHANGELOG.md # src/fidesops/ops/service/privacy_request/request_runner_service.py # tests/ops/service/privacy_request/request_runner_service_test.py
Thanks for your comments @eastandwestwind, back to you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate all the detail on these changes, @pattisdr !
class ManualWebhookResults(BaseSchema): | ||
"""Represents manual webhook data retrieved from the cache and whether privacy request execution should continue""" | ||
|
||
manual_data: Dict[str, List[Dict[str, Optional[Any]]]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thank you for the work on the docs!
Thanks for taking a look @conceptualshark! |
* Add a method to cache data supplied for a manual webhook on a particular privacy request. * Add an endpoint to retrieve all enabled access manual webhooks. * Add an endpoint for uploading manual data corresponding to fields in a manual webhook for a given privacy request with "requires_input" status. * Add an endpoint to view data manually uploaded for an access manual webhook. - Add new scopes for the endpoints to upload/view manual data for webhooks. - Enforce that at least one field is added when defining a manual webhook, and add a fallback if no fields were defined. * Add an endpoint to resume a privacy request from "requires_input" status once all input has been added. None of the fields are required, but the a key for each manual webhook still needs to exist in the cache to proceed. As part of request execution check if data has been uploaded (data can be empty) for all manual webhooks. If True, we can proceed with request execution, otherwise, we put the PrivacyRequest in "requires_input" status and exits. Also adds the manual data uploaded directly to the packet we upload to the user at the very end. * Update postman collection. * Fix request_id query param in existing postman request. * Include additional details about how to resume a "requires_input" privacy request when getting its status. * Add docs and update changelog. * Upload new ERD diagram. * Don't put a privacy request in requires_input state if this policy only has erasure rules. * Respond to CR! * Update manual_webhooks.md
👉 Note that new scopes have been added here.
Purpose
Enable the new "Manual Webhook" Connector. In short, if manual webhooks are configured, the privacy request will be in a state of
requires_input
until data has been supplied for each webhook. Once the privacy request is resumed, the graph will run as usual and any manually uploaded data will be directly added to the final data package. The data is not used as part of the graph and is not filtered by data category. All uploaded data is passed to the user as-is.Changes
GET "/access_manual_webhook"
PATCH /privacy-request/{privacy_request_id}/access_manual_webhook/{connection_key}
manual_webhook
so the connection_key is for the ConnectionConfig.requires_input
state once all webhook data is uploaded:POST "/privacy-request/{privacy_request_id}/resume_from_requires_input"
GET /privacy-request/{privacy_request_id}/access_manual_webhook/{connection_key}
Checklist
CHANGELOG.md
fileCHANGELOG.md
file is being appended toUnreleased
section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.Run Unsafe PR Checks
label has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #1228