Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: privacy policy evaluation context #162

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

wschurman
Copy link
Member

Why

Just like cascading info was added into triggers in #145, this information may be useful when evaluating a privacy policy.

Concretely we have the following use case:

  • User has permission to create an object owned by a parent object (user has permission on parent object, privacy policy of child object is derived from permission on parent object). On this object the user's ID is recorded for audit purposes with SET_NULL behavior on the column.
  • User creates that object
  • User's permission is revoked on parent object but child object is still there (since revoking permission doesn't mean that the data should no longer exist) and therefore still references user.
  • User deletes their account, and therefore the SET_NULL deletion behavior is executed. Unfortunately, during this update the user no longer has permission to update the child object and the privacy policy evaluation fails.

How

This PR adds another piece of information available to privacy policy evaluation, the cascadingDeleteCause.

This will fix the situation above since we can now check if the update (to SET_NULL) is being executed via a cascade to delete a user, and if so allow in the privacy policy.

Test Plan

Run all tests.

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

Merging #162 (989777e) into master (bc378b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #162   +/-   ##
=======================================
  Coverage   95.98%   95.99%           
=======================================
  Files          77       77           
  Lines        1918     1921    +3     
  Branches      208      208           
=======================================
+ Hits         1841     1844    +3     
  Misses         76       76           
  Partials        1        1           
Flag Coverage Δ
integration 95.99% <100.00%> (+<0.01%) ⬆️
unittest 95.99% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...xample/src/entities/AllowIfUserOwnerPrivacyRule.ts 100.00% <ø> (ø)
packages/entity/src/Entity.ts 100.00% <ø> (ø)
packages/entity/src/EntityAssociationLoader.ts 100.00% <ø> (ø)
packages/entity/src/EntityMutationInfo.ts 100.00% <ø> (ø)
packages/entity/src/ReadonlyEntity.ts 100.00% <ø> (ø)
...s/entity/src/rules/AlwaysAllowPrivacyPolicyRule.ts 100.00% <ø> (ø)
...es/entity/src/rules/AlwaysDenyPrivacyPolicyRule.ts 100.00% <ø> (ø)
...es/entity/src/rules/AlwaysSkipPrivacyPolicyRule.ts 100.00% <ø> (ø)
packages/entity/src/rules/PrivacyPolicyRule.ts 100.00% <ø> (ø)
packages/entity/src/EntityLoader.ts 89.36% <100.00%> (+0.11%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc378b6...989777e. Read the comment docs.

@wschurman wschurman requested review from ide and quinlanj March 8, 2022 21:09
@wschurman wschurman marked this pull request as ready for review March 8, 2022 21:09
Copy link
Member

@quinlanj quinlanj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@wschurman wschurman merged commit 3a81a4c into master Mar 10, 2022
@wschurman wschurman deleted the @wschurman/privacy-context branch March 10, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants