[Backend] Privacy Notice Templates #3120
Conversation
…vacyNotice and PrivacyNoticeHistory back to those templates. - Define a yaml with the default privacy notices and add a separate schema migration to populate all three tables - Split out the code from the endpoint to create privacy notices to also use to create templates and initial notices
| regions: | ||
| - eu_be | ||
| - eu_bg | ||
| - eu_cz | ||
| - eu_dk | ||
| - eu_de | ||
| - eu_ee | ||
| - eu_ie | ||
| - eu_el | ||
| - eu_es | ||
| - eu_fr | ||
| - eu_hr | ||
| - eu_it | ||
| - eu_cy | ||
| - eu_lv | ||
| - eu_lt | ||
| - eu_lu | ||
| - eu_hu | ||
| - eu_mt | ||
| - eu_nl | ||
| - eu_at | ||
| - eu_pl | ||
| - eu_pt | ||
| - eu_ro | ||
| - eu_si | ||
| - eu_sk | ||
| - eu_fi | ||
| - eu_se |
There was a problem hiding this comment.
Currently listing all the EU regions out individually, let's double check this is how we want to define this
| - eu_se | ||
| consent_mechanism: notice_only | ||
| data_uses: | ||
| - provide |
There was a problem hiding this comment.
Our current validation doesn't allow data uses in the same hierarchy to be used on the same region, including if they are on the same notice.
This is defined in JIRA as being both provide and provide.service. I've added just provide here. Alternatively, we could change the validation, but I also think provide.service would be irrelevant if provide was also added here.
There was a problem hiding this comment.
@pattisdr This sounds correct, and we will get the templated notices updated ASAP. One other question though: Can we still use two different child data uses from the same hierarchy in the same region? For example, could I create two notices for CA that each had one of these uses: advertising.first_party.personalized and advertising.third_party.personalized ?
There was a problem hiding this comment.
@mfbrown not currently no, an advertising.first_party.personalized for CA and an advertising.third_party.personalized for CA would be disallowed.It's pretty strict at the moment. I can look into allowing this case.
There was a problem hiding this comment.
Thanks @mfbrown I've reticketed this here: advertising.third_party.personalized
There was a problem hiding this comment.
Ah I was wrong, it does work currently, so I'm just adding more tests for this.
| op.create_foreign_key( | ||
| "privacynotice_origin_fkey", | ||
| "privacynotice", | ||
| "privacynoticetemplate", | ||
| ["origin"], | ||
| ["id"], | ||
| ) | ||
| op.create_foreign_key( | ||
| "privacynoticehistory_origin_fkey", | ||
| "privacynoticehistory", | ||
| "privacynoticetemplate", | ||
| ["origin"], | ||
| ["id"], | ||
| ) |
There was a problem hiding this comment.
I don't know if this really needs to be a FK to the template. The PrivacyNoticeTemplate table might be a temporary solution, but I think it's easier to add it up front and remove it later, then to handle the reverse.
src/fides/api/ctl/migrations/versions/d8a50048dfed_add_privacy_notice_templates.py
Outdated
Show resolved
Hide resolved
| existing_notices = ( | ||
| PrivacyNotice.query(db).filter(PrivacyNotice.disabled.is_(False)).all() | ||
| ) | ||
|
|
||
| new_notices = [ | ||
| PrivacyNotice(**privacy_notice.dict(exclude_unset=True)) | ||
| for privacy_notice in privacy_notices | ||
| ] | ||
| try: | ||
| check_conflicting_data_uses(new_notices, existing_notices) | ||
| except ValidationError as e: | ||
| raise HTTPException(HTTP_422_UNPROCESSABLE_ENTITY, detail=e.message) | ||
|
|
||
| return [ | ||
| PrivacyNotice.create( | ||
| db=db, data=privacy_notice.dict(exclude_unset=True), check_name=False | ||
| ) | ||
| for privacy_notice in privacy_notices | ||
| ] | ||
| return create_privacy_notices_util(db, privacy_notices, PrivacyNotice) # type: ignore[return-value] |
There was a problem hiding this comment.
I move all this into create_privacy_notices_util so I can share all this nice validation when populating the templates and then the notices from those templates. The edge cases are well-tested in the endpoint so I just add a light pass of tests on the new method.
src/fides/api/ctl/migrations/versions/8173a96aad1a_populate_default_notice_templates.py
Outdated
Show resolved
Hide resolved
| - eu_se | ||
| consent_mechanism: opt_in | ||
| data_uses: | ||
| - advertising.third_party.personalized |
There was a problem hiding this comment.
Data uses are a required item for creating a privacy notice so I've had to omit all privacy notices without them - only ~8 are added here
There was a problem hiding this comment.
Makes sense. These will take their permanent shape with the Fideslang 1.4 update.
There was a problem hiding this comment.
@mfbrown are you sure we shouldn't wait to release this until Fideslang 1.4?
Passing run #1503 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
||||||||||||||||||||
| PrivacyNotices and PrivacyNoticeHistories from there""" | ||
| sessionlocal = get_db_session(CONFIG) | ||
| with sessionlocal() as session: | ||
| load_default_notices(session, DEFAULT_PRIVACY_NOTICES_PATH) |
There was a problem hiding this comment.
This is another example of something that is problematic in ops unit testing due to #2016
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #3120 +/- ##
==========================================
+ Coverage 87.54% 87.60% +0.06%
==========================================
Files 307 308 +1
Lines 17734 17809 +75
Branches 2288 2296 +8
==========================================
+ Hits 15525 15602 +77
+ Misses 1793 1792 -1
+ Partials 416 415 -1
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| data_uses: | ||
| - improve.system | ||
| enforcement_level: frontend | ||
| disabled: False |
There was a problem hiding this comment.
Should we load in as disabled by default?
pattisdr
changed the title
…he-box templates upfront. On startup, upsert these templates. If the id doesn't exist, create a new template. If an id does exist, update its contents. If the template is new, create a PrivacyNotice and PrivacyNoticeHistory.
|
@adamsachs I've mostly repurposed your logic from creating and updating privacy notices to be able to upsert privacy notice templates and then create privacy notices from those templates on startup. Let me know what you think! |
| def dry_update(self, *, data: dict[str, Any]) -> PrivacyNoticeBase: | ||
| """ | ||
| A utility method to get an updated object without saving it to the db. | ||
|
|
||
| This is used to see what an object update would look like, in memory, | ||
| without actually persisting the update to the db | ||
| """ | ||
| # Update our attributes with values in data | ||
| cloned_attributes = self.__dict__.copy() | ||
| for key, val in data.items(): | ||
| cloned_attributes[key] = val | ||
|
|
||
| # remove protected fields from the cloned dict | ||
| cloned_attributes.pop("_sa_instance_state") | ||
|
|
||
| # create a new object with the updated attribute data to keep this | ||
| # ORM object (i.e., `self`) pristine | ||
| return self.__class__(**cloned_attributes) |
There was a problem hiding this comment.
Moved @adamsachs's dry update here to share between updating templates and notices.
| def prepare_privacy_notice_patches( | ||
| privacy_notice_updates: List[PrivacyNoticeWithId], | ||
| db: Session, | ||
| model: Union[Type[PrivacyNotice], Type[PrivacyNoticeTemplate]], | ||
| allow_create: bool = False, | ||
| ignore_disabled: bool = True, | ||
| ) -> List[ | ||
| Tuple[PrivacyNoticeWithId, Optional[Union[PrivacyNotice, PrivacyNoticeTemplate]]] | ||
| ]: | ||
| """ | ||
| Prepares PrivacyNotice/Template creates and updates including performing data use | ||
| conflict validation on proposed changes. | ||
|
|
||
| Returns a list of tuples that have the PrivacyNotice update data (API schema) alongside | ||
| their associated existing PrivacyNotice db record that will be updated | ||
|
|
||
| :param privacy_notice_updates: List of privacy notice schemas with ids: appropriate | ||
| for editing PrivacyNotices or upserting PrivacyNoticeTemplates | ||
| :param db: Session | ||
| :param model: one of PrivacyNotice or PrivacyNoticeTemplate | ||
| :param allow_create: If True, this method can prepare data to be both created and updated. Otherwise, | ||
| this is just intended for updates and will fail if a record doesn't exist. | ||
| :param ignore_disabled: Should we skip checking disabled data uses? | ||
| :return: | ||
| """ |
There was a problem hiding this comment.
Adapted this method that @adamsachs already wrote for updating privacy notices. This can also handle upserting Privacy Notice templates, shared between both locations.
| def create_privacy_notices_util( | ||
| db: Session, privacy_notice_schemas: List[PrivacyNoticeCreation] | ||
| ) -> List[PrivacyNotice]: | ||
| """Performs validation before creating Privacy Notices and Privacy Notice History records""" | ||
| validate_notice_data_uses(privacy_notice_schemas, db) # type: ignore[arg-type] | ||
|
|
||
| existing_notices = PrivacyNotice.query(db).filter(PrivacyNotice.disabled.is_(False)).all() # type: ignore[attr-defined] | ||
|
|
||
| new_notices = [ | ||
| PrivacyNotice(**privacy_notice.dict(exclude_unset=True)) | ||
| for privacy_notice in privacy_notice_schemas | ||
| ] | ||
| check_conflicting_data_uses(new_notices, existing_notices) | ||
|
|
||
| return [ | ||
| PrivacyNotice.create( | ||
| db=db, data=privacy_notice.dict(exclude_unset=True), check_name=False | ||
| ) | ||
| for privacy_notice in privacy_notice_schemas | ||
| ] |
There was a problem hiding this comment.
Moved here from @adamsachs create privacy notices endpoint to share so we can add notices on startup as well.
| def validate_notice_data_uses( | ||
| privacy_notices: List[Union[PrivacyNoticeWithId, PrivacyNoticeCreation]], | ||
| db: Session, | ||
| ) -> None: | ||
| """ | ||
| Ensures that all the provided `PrivacyNotice`s have valid data uses. | ||
| Raises a 422 HTTP exception if an unknown data use is found on any `PrivacyNotice` | ||
| """ | ||
| valid_data_uses = [data_use.fides_key for data_use in DataUse.query(db).all()] | ||
| try: | ||
| for privacy_notice in privacy_notices: | ||
| privacy_notice.validate_data_uses(valid_data_uses) | ||
| except ValueError as e: | ||
| raise HTTPException(HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) | ||
|
|
||
|
|
||
| def ensure_unique_ids( | ||
| privacy_notices: List[PrivacyNoticeWithId], | ||
| ) -> None: | ||
| """ | ||
| Ensures that all the provided PrivacyNotices in the request body have unique ids | ||
| Raises a 422 HTTP exception if there is more than one PrivacyNotice with the same ID | ||
| """ | ||
| ids = set() | ||
| for privacy_notice in privacy_notices: | ||
| if privacy_notice.id not in ids: | ||
| ids.add(privacy_notice.id) | ||
| else: | ||
| raise HTTPException( | ||
| HTTP_422_UNPROCESSABLE_ENTITY, | ||
| detail=f"More than one provided PrivacyNotice with ID {privacy_notice.id}.", | ||
| ) | ||
|
|
There was a problem hiding this comment.
Likewise moved here from privacy notice endpoints -
| @@ -0,0 +1,235 @@ | |||
| privacy_notices: | |||
| - name: Essential | |||
| id: pri_51c85583-40ec-4257-95ef-db3eea860c52 | |||
There was a problem hiding this comment.
Felt like we needed to specify id's here so we can upsert these as needed in the future. I think this supports a future goal of being able to do a "fides push"-like command and update the existing privacy notice templates without having to release a new version of fides.
|
@adamsachs No need to review this any longer, I'm moving this into blocked for now. The privacy notices data uses will change a lot with fideslang 1.4. Further I was only able to template a subset of privacy notices as several were missing. I think this does lay out a way that we can add these templated privacy notices to the db and automatically populate starting privacy notices from there. Will need to get this updated again when we're ready to surface. |
|
I'm going to close this PR for now as it's going to get out of date quickly, but I'll borrow heavily from this when we're ready to add the privacy notices post- fideslang 1.4. |
Closes #2833
Code Changes
Steps to Confirm
nox -s devprivacy_notice_templates.yml(such as a bad data use, or duplicate data uses, or an invalid consent mechanism). Stop the server and then restart. Verify you get validation errors in the logs but server still comes up.Pre-Merge Checklist
CHANGELOG.mdDescription Of Changes
Start fides with an initial set of privacy notices. Create a yaml file to define them and a separate table to store the templates. On startup, upsert the templates into the PrivacyNoticeTemplate table (the templates have ids already in the yaml). Then if the templates are new, create corresponding PrivacyNotice and PrivacyNoticeHistory records. Otherwise don't touch those tables.
Note that templates are not what's shown to users, it's the enabled notices in the PrivacyNotice table. But this action lets us give customers a starting set of notices out of the box.
Adds what templates I can from: https://ethyca.atlassian.net/wiki/spaces/PM/pages/2656108545/Privacy+Notice+Templates