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: Add optional visual screen privacy safeguard for private data. #164

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@nottoseethesun
Copy link

nottoseethesun commented Feb 10, 2018

Summary

This adds a simple checkbox form element, to enable the user to visually hide the private info on the page.

Motivation

Consider a scenario where a more technical person is helping someone verify their 24 word recovery phrase for their hardware wallet. The technical person would typically need to help verify the generated hash addresses, to make sure that the 24 words were correct.

In this scenario, ideally, the owner of the hardware wallet should be able to hide the recovery phrase and qr code from view. That way, the assistant can look at the bip39 page to verify the non-private hash addresses without seeing the hardware wallet owner's private info.

Tests

At this point, both iancoleman/bip39 master branch and my fork produce these results on my box, which all seem to be due to the test tool's timeout being too short for my local set-up (I am running a laptop with only small graphics capability, so perhaps that is affecting the tests):

Using default browser: chrome
Started
...............................................................**..........FF.....F........................................................................FF....................F...............

I tried a larger jasmine.DEFAULT_TIMEOUT_INTERVAL and other intervals, but even at sixteen times the intervals that still left three failures for both the original and this fork:

...............................................................**.................F.......F................................................................F.....................................

I've attached text files showing the complete logs for the test run on both the original and this fork. The only issues seem to be due to timeouts.

Here's what I did to try to eliminate the timeouts:

// Slower computer factor: Required to run tests on slower machines.
var SLOWER_FACTOR = 16;

// Jasmine Settings
jasmine.DEFAULT_TIMEOUT_INTERVAL *= SLOWER_FACTOR;
....
// Delays in ms
var generateDelay = 1500 * SLOWER_FACTOR;
var feedbackDelay = 500 * SLOWER_FACTOR;
var entropyFeedbackDelay = 500 * SLOWER_FACTOR;
var bip38delay = 15000 * SLOWER_FACTOR;

It seems that since both master and my fork produce the same test result, my change is orthogonal to any test issues. Those test issues would seem to be a separate topic.

privacy-screenshot from 2018-02-10 01-17-21

privacy-screenshot from 2018-02-10 01-18-03

test-logs-original-iancoleman.txt

test-logs-treelogic-swe-fork.txt

@iancoleman

This comment has been minimized.

Copy link
Owner

iancoleman commented May 29, 2018

This pull request ended up with conflicts and some changes from the original PR.

I've added the feature in 0b6e351 and is included in the current release 0.3.7

Some differences:

  • more fields are private, eg in the manual entropy section
  • fields are completely hidden rather than blanked out to avoid revealing of secrets by selecting the text. This also means the qr code is not shown for private data but is for public data.
  • label / checkbox layout is consistent with existing layout

Thanks for the suggestion, I think this is a great feature.

@iancoleman iancoleman closed this May 29, 2018

@nottoseethesun

This comment has been minimized.

Copy link
Author

nottoseethesun commented May 29, 2018

Thank you, and I'm looking forward to the new feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.