fides
fides copied to clipboard
Prevent modifying default taxonomy fields
Closes https://github.com/ethyca/fides/issues/986
Code Changes
- [ ] list your code changes here
Steps to Confirm
- [ ] list any manual steps taken to confirm the changes
Pre-Merge Checklist
- [ ] All CI Pipelines Succeeded
- Documentation Updated:
- [ ] documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
- [ ] documentation issue created (tag docs-team to complete issue separately)
- [ ] Issue Requirements are Met
- [ ] Relevant Follow-Up Issues Created
- [ ] Update
CHANGELOG.md
Description Of Changes
Write some things here about the changes and any potential caveats
@ThomasLaPiana would love to get your thoughts on if this is the right track. I think there are two things that need to be covered:
- Making sure no edits occur on fields that are default (will this have an undesired effect on the
load_default_taxonomyupsert though? i.e. if we make a change in the future, we may not be able to actually apply it without a workaround) - Making sure nothing can edit the
is_defaultfield (the db will automatically fill it in as False). Thinking about this now, this will probably makeload_default_taxonomynot work since it's trying to insert everything asis_default=True... so maybe this isn't the right way to go
@allisonking left some comments, lmk if those make sense and seem practical
There's a failing test in CI that I'm not sure how to fix. It doesn't fail locally which makes it difficult to debug... but it seems to come down to that in CI the resources_dict somehow sets is_default=True even though it should default to False when there isn't a value given. This causes a 403 which is expected when is_default=True and trying to create via the API, but in this case, it shouldn't be is_default=True in the first place, so I'm not sure what's going on.
https://github.com/ethyca/fides/runs/7882107794?check_suite_focus=true#step:6:232
There's a failing test in CI that I'm not sure how to fix. It doesn't fail locally which makes it difficult to debug... but it seems to come down to that in CI the
resources_dictsomehow setsis_default=Trueeven though it should default to False when there isn't a value given. This causes a 403 which is expected whenis_default=Trueand trying to create via the API, but in this case, it shouldn't beis_default=Truein the first place, so I'm not sure what's going on.https://github.com/ethyca/fides/runs/7882107794?check_suite_focus=true#step:6:232
Eduardo figured it out, it was an issue with the pytest fixture scope for resources_dict! Changing the scope to function fixes things nicely.
This PR may need to be reworked pending updated requirements from @mfbrown . The feature:
Adds a check to put, upsert, and delete which prevents those endpoints from taking any action on models with an is_default=True property. If the attempt is made, throws a 403 forbidden.
is likely not what we want, as it turns out some fields of default taxonomies should be able to be edited.
Okay, updated the initial comment for the new behavior, should be ready for review again 👍