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

Remove Mutation PII while still updating the Cache correctly #2240

Merged
merged 15 commits into from
Mar 13, 2020

Conversation

supernova-at
Copy link
Contributor

@supernova-at supernova-at commented Mar 11, 2020

Description

Off the heels of our blog post How to keep sensitive mutation data out of your Apollo cache, this PR updates a few places in PWA Studio where we weren't handling PII and / or the Apollo cache correctly.

Here's a handy mutation PII cheat sheet:

Outgoing parameters contain PII? GraphQL response updates a cache item? Solution
T T Use the @connection directive
T F Set fetchPolicy to no-cache
F T Don't use no-cache; The @connection directive is optional (not needed)
F F Set fetchPolicy to no-cache

This PR updates the following places:

Update Scenario Strategy
setGuestEmailOnCart T / F no-cache
setShippingAddress (MiniCart) T / T @connection directive
SET_SHIPPING_ADDRESS_MUTATION (v2 Cart) T / T @connection directive
createAccount T / F no-cache

Related Issue

Closes PWA-425.

Acceptance

Verification Stakeholders

@tjwiebell @sirugh

Specification

Verification Steps

Set Guest Email on Cart

  1. Start with a clean Apollo cache by clicking Developer Tools > Application Tab > Clear Storage > Clear site data
  2. Add an item to your cart
  3. Begin checkout via MiniCart
  4. Submit a shipping address that contains an email
  5. Verify that the ROOT_MUTATION entry in the Apollo Cache does not contain a setGuestEmailOnCart property

Set Shipping Address (MiniCart)

  1. Start with a clean Apollo cache by clicking Developer Tools > Application Tab > Clear Storage > Clear site data
  2. Add an item to your cart
  3. Begin checkout via MiniCart
  4. Submit a shipping address
  5. Verify that the ROOT_MUTATION entry in the Apollo Cache contains a setShippingAddressesOnCart property but does not contain any PII

Set Shipping Address (v2 Cart)

  1. Start with a clean Apollo cache by clicking Developer Tools > Application Tab > Clear Storage > Clear site data
  2. Add an item to your cart
  3. Go to the /cart page
  4. Submit a shipping address in the first accordion section
  5. Verify that the ROOT_MUTATION entry in the Apollo Cache contains a setShippingAddressesOnCart property but does not contain any PII

Create Account

  1. Start with a clean Apollo cache by clicking Developer Tools > Application Tab > Clear Storage > Clear site data
  2. Open the left drawer and go through the create account workflow
  3. Verify that the ROOT_MUTATION entry in the Apollo Cache does not contain a createCustomer property (and therefore no PII is present)

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have updated the documentation accordingly, if necessary.
    📝 I updated the Coding Standards and Conventions Wiki with the Mutation PII Cheat Sheet above.

  • I have added tests to cover my changes, if necessary.

@m2-community-project m2-community-project bot added this to Ready for Review in Pull Request Progress Mar 11, 2020
@supernova-at supernova-at changed the title Remove Mutation PII Remove Mutation PII while still updating the Cache correctly Mar 11, 2020
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Mar 11, 2020

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: PWA-425.

Generated by 🚫 dangerJS against a207c56

@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Mar 11, 2020
@supernova-at
Copy link
Contributor Author

GraphQL queries did not pass. Make sure to execute yarn run validate-queries locally prior to committing.

I have to track down this issue and fix the validator.
The actual error is:

error Unknown directive "connection" graphql/template-strings

@supernova-at
Copy link
Contributor Author

PR Updated:

  • Fixes validate-queries script to not error on the new @connection directive

@m2-community-project m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress Mar 12, 2020
@supernova-at
Copy link
Contributor Author

⚠️ Setting @connection(key: null) isn't working how we expected it would: the mutation is still being cached and all the PII shows up in the key.

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Thanks for updating the wiki!

@dpatil-magento dpatil-magento merged commit ead0ef1 into develop Mar 13, 2020
@m2-community-project m2-community-project bot moved this from Review in Progress to Done in Pull Request Progress Mar 13, 2020
@dpatil-magento dpatil-magento deleted the supernova/425_mutation_pii branch March 13, 2020 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants