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] Screenshot Warning #4670

Merged
merged 108 commits into from
Dec 21, 2022
Merged

[FEAT] Screenshot Warning #4670

merged 108 commits into from
Dec 21, 2022

Conversation

gantunesr
Copy link
Member

@gantunesr gantunesr commented Jul 13, 2022

Description

Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions,
1. What is the reason for the change?

Improve the security around SRP reveal.

2. What is the improvement/solution?

iOS specific. This PR adds a warning when the user takes a screenshot of their SRP or Private Key.

Changes:

  • Introduction of react-native-detector v0.2.1 package
  • Refactor of RevealPrivateCredential and move styles to a different file
  • Refactor of ManualBackupStep1 and move styles to a different file

it also refactors the component RevealPrivateCredential and ManualBackupStep1

Screenshots/Recordings

If applicable, add screenshots and/or recordings to visualize the before and after of your change

RPReplay_Final1657683451.MP4

Note: You can observe a similar behaviour in the onboarding flow.

Issue

Progresses https://github.com/MetaMask/mobile-planning/issues/227

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@gantunesr gantunesr added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-accounts labels Jul 13, 2022
@gantunesr gantunesr marked this pull request as ready for review July 13, 2022 23:25
@gantunesr gantunesr requested a review from a team as a code owner July 13, 2022 23:25
@gantunesr gantunesr marked this pull request as draft July 14, 2022 14:56
@gantunesr gantunesr marked this pull request as ready for review July 14, 2022 14:56
@gantunesr gantunesr removed the blocked label Dec 14, 2022
@olenapankina olenapankina added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Dec 15, 2022
@olenapankina olenapankina added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed QA in Progress QA has started on the feature. labels Dec 20, 2022
@gantunesr gantunesr added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Dec 20, 2022
@olenapankina olenapankina added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Dec 21, 2022
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

This PR is 🌮 🌮 .

Scenarios covered outside of the AC mentioned here:

  • A user attempts to take a screenshot while his language is not set to English. Ensuring that the text in the warning alert defaults to English
  • Regression on android. Nothing is broken there.

@cortisiko cortisiko merged commit 1880962 into main Dec 21, 2022
@cortisiko cortisiko deleted the feat/screenshot-alert branch December 21, 2022 22:46
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2022
@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Dec 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed A successful QA run through has been done release-5.13.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants