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

test: Add tests for masking of sensitive data #165

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 1, 2024

This tests masking of initial & updated values for password & credit card fields.

You can see that for now credit card data is not masked, which demonstrates the current behavior.
In a future PR we can then verify that our fixed masking is working.

ref getsentry/sentry-javascript#10258

add tests for sensitive data
@mydea mydea requested review from billyvg and c298lee February 1, 2024 15:07
@mydea mydea self-assigned this Feb 1, 2024
@@ -7236,7 +7236,7 @@ exports[`record integration tests mutations should work when blocked class is un
\\"attributes\\": {
\\"class\\": \\"rr-block\\",
\\"rr_width\\": \\"1904px\\",
\\"rr_height\\": \\"21px\\"
\\"rr_height\\": \\"21.5px\\"
Copy link
Member

Choose a reason for hiding this comment

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

This is flakey :(

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, sucks a lot 😬

Comment on lines +21 to +35
<label>
<input autocomplete="cc-number" value="initial" />
</label>
<label>
<input autocomplete="cc-exp" value="initial" />
</label>
<label>
<input autocomplete="cc-exp-month" value="initial" />
</label>
<label>
<input autocomplete="cc-exp-year" value="initial" />
</label>
<label>
<input autocomplete="cc-csc" value="initial" />
</label>
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

Should we add a test for dynamically inserted elements as well?

@mydea
Copy link
Member Author

mydea commented Feb 1, 2024

Should we add a test for dynamically inserted elements as well?

I'll add some in the follow up PR!

@mydea mydea merged commit 79d1fe2 into sentry-v2 Feb 1, 2024
14 checks passed
@mydea mydea deleted the fn/disallow-cc branch February 1, 2024 16:03
mydea added a commit that referenced this pull request Feb 1, 2024
This is on top of #165, actually
fixing the behavior so that certain fields cannot be unmasked.

This is a pretty straightforward fix, a bit "hacky" but should work well
enough.

Fixes getsentry/sentry-javascript#10258

---------

Co-authored-by: mydea <[email protected]>
billyvg pushed a commit that referenced this pull request Apr 26, 2024
This tests masking of initial & updated values for password & credit
card fields.

You can see that for now credit card data is not masked, which
demonstrates the current behavior.
In a future PR we can then verify that our fixed masking is working.

ref getsentry/sentry-javascript#10258
billyvg pushed a commit that referenced this pull request Apr 26, 2024
This is on top of #165, actually
fixing the behavior so that certain fields cannot be unmasked.

This is a pretty straightforward fix, a bit "hacky" but should work well
enough.

Fixes getsentry/sentry-javascript#10258

---------

Co-authored-by: mydea <[email protected]>
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.

2 participants