bitshares / bitshares-ui

Fully featured Graphical User Interface / Reference Wallet for the BitShares Blockchain
https://wallet.bitshares.org
MIT License
517 stars 570 forks source link

[5][ahdigital]Consolidate Login/Wallet to a Single type #895

Closed Xeldal closed 6 years ago

Xeldal commented 6 years ago

The following is a copy paste from an article I wrote on steem regarding this issue: https://steemit.com/bitshares/@xeldal/a-solution-to-consolidate-bitshares-2-wallet-login-types

The Bitshares GUI wallet currently has 2 types of logins that operate 2 differnt wallet types. It is a cause of confusion and one source of constant password and login issues. I propose here that it is not necessary. Only a single wallet type is required. A user should never have to "switch" wallet types.

The Givens

The Cloud wallet derives private keys from the password. The Local wallet gets private keys from an encrypted local file.

Either way you are loging in with your private-keys.

After obtaining the private key they operate [can] in exactly the same way. There's clearly no reason to seperate into two different wallet types after this point.

Unifying the Two Types

The only function a local wallet has that a cloud wallet doesn't is storing multiple accounts keys.

It is possible to unify the 2 types, while retaining all the various functions like multiple accounts, login from anywhere, backup files, etc while still maintaining the same levels of security we offer now.

Bringing the two types together is fairly simple in design, I don't know how difficult the actual code changes would be.

A user would be given a login window. Username and Password. The password can be either the private key for the account or the password from which the keys are derived(same as current cloud login).

Within any account you have what I'm calling a Key-Ring, or Key-File, or just simply a backup file. For a single account this is just a list of that accounts private keys. The Key-Ring can be downloaded(backup) and encrypted with a password of their choice. This isn't really necessary if you only have 1 account and have the password but the function is there anyway if someone has a use for it. Maybe for offline storage with simplified password, unencrypted or maybe a brain-key.

Loging in with the password or private key is the equivalent of the cloud-wallet-login. Access from anywhere.

For the local wallet I propose a simple button in the field for username that allows you to select a Key-File (key-ring , backup). The user then enters the encryption password for the key-file. The first set of keys found can work as the active account while the remaining keys can be easily switched to from a menu item(like we have now in local wallet).

A user would be able to add and remove accounts from their key-ring(backup) file whenever they want. As well as create and save any number of different key-files containing the accounts of their choosing.

Security

Currently the cloud wallet auto generates a password that is roughly equivalent *(1) in difficulty to brute-force as the private-key itself. No change is necessary here so a single account retains the same level of security as we have now.

*(1)[I understand there is a difference in security between the masterPasword and private keys. For the purpose of simplicity I'm calling them "roughly" equivalent"]

The local wallet we have now is just an encrypted file containing the private keys for the various accounts. This is no different than the proposed Key-File(backup). The Key-File can be just as secure as an individual account depending on the strength of the encryption password used. The local file is also secured by the fact that it is not a public file on the internet and only the user should ideally have access to it. I want to point out that logging in with a key-file is not necessarily more secure *(2) than logging in with the generated password or private-key. As all accounts are subject to brute-force against the private keys. The file isn't necessary for this. The keys and generated passwords are long enough though, that this kind of attack should be impossible.

*(2)[With an extremely long masterPassword the weakest link is a direct attack on the private key itself]

Switching accounts

The current Local Wallet enjoys the ability to hold multiple accounts and can switch between them easily without having to supply a new password. The proposed model would have exactly the same feature. Supplying the password for the Key-File gives access to all the keys for the multiple accounts within.

btsfav commented 6 years ago

you could activate an auto-download for single-accounts, which would help the hopeless that are inable to secure their password.

make sense?

Xeldal commented 6 years ago

I agree. Even given the fact that a single account doesn't really need a backup if they have the password. They can get away with using a much simpler password for this local keyfile and so now even if they forget the master-password or there was some problem with the process, they still have a copy of their keys in this keyfile by a password of their own choosing.

I also think it's important to prompt "where do you want to save this keyfile" so the user knows what it is, knows that they have one, and knows where to find it.

tbone-bts commented 6 years ago

@Xeldal I was about to write a response saying that I disagree with your premise, and then continue to propose my own 2 possible solutions. But as I was writing, it dawned on me that your premise may be fine, but you may not be explaining it well. Specifically, I don't believe it's correct to say there are currently 2 different wallet types. What we have is simply 2 different login types. And nothing can change the fact that the correct login type needs to be used with the correct credentials. Further, the problem appears to be that for some reason the expected login type is sometimes changing without the user realizing it. To make matters worse, there is no obvious indication of which login mode is currently active...and no immediately visible way to switch modes.

I believe this problem can be mitigated in one of two ways:


(1) indicate on the login form which mode is currently active by displaying a toggle switch. If the user enters their password and it fails to log them in, a message can be displayed suggesting to switch the login mode and try again. This would likely solve 99% of the problems we're seeing, although the user experience would not be as seamless as it could be.

(2) A more elegant solution might be to always display the account name field on the login dialog. If the user has a local file, that can simply be indicated by the account name field being pre-populated (perhaps with a notation next to the account name field indicating a .bin file was detected) and then the user just has to enter the password. If no bin file is detected, the user either (a) clicks a link next to the account name which enables them to specify a .bin file or (b) simply types both the account name and their password. Easy peasy.


Unless I'm missing something, I think the second option is by far the best way to go. And I'm starting to think this option is essentially what you have been getting at. I just think your concept of the keyring and saying we don't need 2 wallet types etc. may have caused confusion.

In any event, I can't imagine this solution would be very difficult to implement. And i think it needs to be done immediately considering all of the login problems that have been occurring.

Xeldal commented 6 years ago

@tbone-bts Thanks, we are thinking in the same direction. I say there are 2 wallet types because you can go into settings and change the login type, which changes the available option in the wallet. That option shouldn't even need to exist. So, you are right that the focus is certainly on how we login, but there are some subtle changes needed in the wallet as well. Only changing the login would be a mistake.

There are actually many more than 2 ways to log in. Master-password, wif-key, brain-key, restore, local bin, etc. The key is to consolidate them as best possible, and once the private keys are established, whether from master-password, wif-key, local bin, brain-key, restore, or whatever... the actual function of wallet from that point (login) should be identical. "switch wallet type" should be a meaningless phrase.

It's important that no matter how many accounts or keys a user has, they can always create and login from a key-file. This is the key to unifying the wallet.

Your 2nd example is pretty close to what I was suggesting. Default to username and password. With a small button in the username field that allows the user to select a key-file. Even if a .bin is automatically found, i think it should still default to the username/password. Maybe have the Key-file button glow or something if it automatically finds one.

When the Key-file button is pressed, it should bring up a file explorer prompt for the user to find and select the key-file. A user may have many different Key-files to choose from or a friend might want to use his key file at your computer, so it's important to always give the prompt to find the file. Once selected, the filepath or filename would be displayed where the username normally is. Clicking the [key icon again] filename would bring up the file explorer again to choose a new file. The key-file icon would change to a silhouette or some other icon that allows to enter a username again. [clicking anywhere inside the Account Name field would allow the user to enter an account name]

So yes there are necessarily many ways to log in. The point of all this is streamlining that experience and that switching wallet types is an unnecessary feature.

It's important that no matter how many accounts or keys a user has, they can always create a key-file. This is the key to unifying the wallet.

Xeldal commented 6 years ago

A few short hypothetical user experiences that hopefully illustrate my general frustration with all this and a summary of key points at the end:

"In my light-client I have a local wallet. I can't login with anything else, not even a different local wallet. I guess I would need to delete my wallet first in order to do it (WHAT!!??). Or go deep into setting and change Login Mode or something. I have my master-password and know all my private-keys, but i have no way to use them. Not really clear in any case."

"In my web-wallet I have a cloud wallet. I can't login with anything else. I have a key-file(bin) with thousands of keys but no way to use them. Same deal, I'm sure it's a toggle in settings somewhere. Not really clear in any case."

"I have a cloud account, and I'd like to login to that account using a local file. Can I create a key-file(bin) to login from? Hmm nope, that file is empty. Dev says cloud accounts don't need key-file backups"

"I have a cloud wallet and I'd like to have a key-file backup with a more manageable password. Maybe I'd like to login from a computer that doesn't have my password manager installed. hmm same deal, can't backup a single account, key-file is empty."

Another Summery The architecture is super simple

You can login from the Key-file, directly from a private key, derive your keys from a master password or brain-phrase.

In all cases "login" simply means you have a private key to sign with. In all cases this key is local. Never in a cloud or sent anywhere. In all cases there is only 1 active account in the wallet.
In all cases a key-file can be created, backed-up, loaded, and modified.
In all cases you can login via any method without doing anything to your setup. In all cases, there is never a need to "switch wallet type" (the idea of switching becomes meaningless) In all cases, you have a wallet, not a cloud wallet, not a local wallet. They are the same. In all cases you have a login, not cloud login, not local login. They are the same.

I know I'm repeating myself here and sorry if these posts are long. It really is a simple thing but for some reason I struggle to get the point clearly made. Hope this helps.

wmbutler commented 6 years ago

This thread is going in a productive direction. It needs some visuals and continued conversation.

wmbutler commented 6 years ago

Does this mockup handle the requirements listed above?

screen shot 2017-12-24 at 11 46 00 am

HarukaMa commented 6 years ago

While this would greatly improve the UX on user login process, please don't make it requiring login every time for current wallet model users. Unlock on operations is great now and having to go to login window every time is worse than current version.

It should be obvious but I just wanted to make sure the process is not becoming unnecessarily complicated.

Hit the submit button accidentally, sorry for the incomplete message.

wmbutler commented 6 years ago

@HarukaMa This doesn't add any requirements. It just consolidates the login process into a single screen. Please take the time to read the entire issue and the problems it is trying to solve.

Also, please finish your thought.

HarukaMa commented 6 years ago

@wmbutler specifically, this sentence is giving me some concerns:

(2) A more elegant solution might be to always display the account name field on the login dialog. If the user has a local file, that can simply be indicated by the account name field being pre-populated

If I already have my keys inside my browser, why would I need to see login window again? Thus the above comment.

Xeldal commented 6 years ago

Account field would auto populate with username or file path depending on active account or whether a file was used previously. This improvement would not add any extra steps in that regard, it would however make it possible to login with a different account because the fields are editable.

@wmbutler A big aspect of this improvement is in dropping the wording "local" and "cloud". It's an unnecessary distinction and there is really no difference in the two. There is only 1 type of wallet, and all "logins" happen locally. If you look at the image supplied in the original post, notice in the field for username the icon for supplying a key-file and read the section describing its function.

While the two are accomplishing the same thing, there is a subtle but significant difference. I could live with your mockup if instead of "clould wallet" and "local wallet" they were renamed "Username" and "Key-file" but I think the icon version from the Original Post just looks better.

Please read again the section regarding selecting the key-file for login if this doesn't make sense.

The following images are an alternative design with 2 selections. This is nearly identical in function to Bills proposed mockup above except with this you would be able to choose your bin file. I prefer the design from the OP with only 1 button.

[edit: insert images] Or even something like this with selectable icons

One change that should probably happen along side the login screen is with regard to generating the key file(aka bin file aka backup): Currently, if the user doesn't have a bin file, when they try to create and download one, it will be empty/blank. You should always be able to create the key-file (aka backup) even if you only have one account. I haven't verified this is actually what happens, just taking svk's word on it.

[edit:] And remove "Login mode" from settings.

Later, a means for managing the key file should be added. Basically just a way to add and remove accounts from the file.

[edit] Repeating here the ones that may need more attention.

In all cases "login" simply means you have a private key to sign with. In all cases this key is local. Never in a cloud or sent anywhere. ... In all cases, there is never a need to "switch wallet type" (the idea of switching becomes meaningless) In all cases, you have a WALLET, not a cloud wallet, not a local wallet. They are the same. In all cases you have a LOGIN, not cloud login, not local login. [Everything is local] They are the same.

Xeldal commented 6 years ago

Here's a further thought on my original design: Working with only 1 button.

The following pictures show how the Account field would auto populate either with a username or a file path depending on the current active account or if a file was previously used. Also shows navigating between the two if desired.

1) Showing default to active username

2) Showing default to file (if file was previously used to login) Also Shows this if the key-file button is pressed, and a file is selected

3) If the Account Name field is pressed at any time it is editable and expect a Username.

I don't think you can reduce it any further than this. You've got all the functions with only a single button.

I'm not sure it's important or useful to indicate having found a file. I don't think it matters. If it's your machine, and it's all you use, it will always indicate and will always be pre-populated. If it's a shared machine, or multi use case(alternating username and key-file), knowing that a file is found doesn't change any steps, you still click the button, your file is still pre-populated or selected.

I'd suggest for simplicity, no need to indicate "detected"

The following 2 pics are alternative designs.

If I'm wrong and there is some useful purpose for indicating a file is found, a simple way would be to have the key glow a different color. Like this:

or if you don't like the key, use a different icon

wmbutler commented 6 years ago

Ugh. You've lost me. There is no way a newbie user is going to understand wtf you are trying to accomplish. You've taken 2 options to at least 3 now and have added yet another variable to the equation. There is no way in hell I'd take the fall for this.

@svk31 am I missing something?

Xeldal commented 6 years ago

@wmbutler It's not that complicated. Your example is 2 subtle changes from being there. Let's see if I can better summarize it again.

Stop separating into 2 separate wallets. There is only 1. Drop the cloud vs local stuff. It's not necessary.
Either you have a file with your keys or you don't. That's it.

If you have a file, you click the the icon and select the file.
Enter the password and that's it.

If you don't have a file you just enter your account name and password. That's it.

All the rest of the pictures and explanation was my attempt to explain the flow of how a user would move back and forth between those two, accounting for all possible scenarios. As well as providing a few options on how it could look. Using 2 buttons vs Using 1 button.

I've used Bill's basic mockup from above and removed the unnecessary wallet switching buttons and inappropriate text labels. Here is loging in with username/password

Here is loging in from a file.

@wmbutler hope that helped. Don't give up on it. It's super simple.

I think my explanations get complicated and messy because i have to repeat myself 10 times. Hammering away at certain ideas that are not sinking in. For example I explained many times that "local" and "cloud" should not be used, and that 2 wallets are not necessary. Then in your example you have 2 wallets to select from and you use "cloud" and "local".

Xeldal commented 6 years ago

Does this mockup handle the requirements listed above? -wmbutler

No.

There is no way a newbie user is going to understand wtf you are trying to accomplish. -wmbutler

It is more simple, straight forward, and versatile than your example. If you are confused about something, how about asking a question instead of just throwing your hands in the air and cursing. Rather than me trying to re-explain everything each time, which is adding to the confusion, if I know where you are not following along, I can better communicate it to you.

You've taken 2 options to at least 3 now and have added yet another variable to the equation. -wmbutler

Not sure where you're getting this from. No extra options. No extra variables. Ask questions if you don't understand.

These are the things you are missing in your example:

If you take a moment and honestly look at my suggestion. You will see it solves all of these issues.

Xeldal commented 6 years ago

Other benefits of proposed solution:

** incognito is not a solution. It still requires the user to dive into settings to restore or import his wallet, every time he opens the browser. This proposal would allow the user to simply select the file from the login screen.

abitmore commented 6 years ago

@Xeldal I guess there are technical limitations that can't simply use a file and remember the location, so it would be hard to implement the key file thing within a web browser. For example,

Xeldal commented 6 years ago

@abitmore Thank you for this assessment. That makes sense.

I think there are at least 2 ways to address this issue:

I want to point out that we already use "the key file thing" now (bin file). So there is no question of whether it can be done. The proposal is mainly to change the way we use it to login, and remove all the unnecessary settings options and wording regarding 2 types of wallets/logins. ie "cloud" and "local". The proposal would allow bin-file users to login without having to go through settings to import or restore.

Xeldal commented 6 years ago

It's a little rough but you get the idea

wmbutler commented 6 years ago

Since most of our users use the Cloud Wallet Login mode, I still think this could fuel uncertainty from our user base.

If you can show a sequence that caters to both while not distracting cloud users, let's continue this path.

Xeldal commented 6 years ago

1)

In this alternative:

2)

In this alternative:

I have a couple other alternatives in mind for another day.

wmbutler commented 6 years ago

Getting closer...

What will you display in the key-file input field if the user has never backed up their wallet? There won't be a filepath available. And I don't think we store that filepath anywhere so we won't have it available even after an upload, although we may have the filename.

tbone-bts commented 6 years ago

I really like the direction @Xeldal has been going here and I think alternative #2 above is very close. It seems the only thing missing is a button that should appear to the right of the input field when the user clicks "Use key-file". That way the user can browse for a key-file (a) if there isn't one already detected or (b) if they want to use a key-file different than the one detected.

Beyond that (and perhaps independent of Xeldal's proposed changes) I think there needs to be a way to specify whether an account can be logged into without a key-file (i.e. using only the account name and a password). I can imagine this as part of a management screen briefly alluded to by @Xeldal where he mentioned giving the user a way to add and remove accounts from key files. I think this management screen could/should also provide some facility for the user to manage passwords.

Xeldal commented 6 years ago

What will you display in the key-file input field if the user has never backed up their wallet? There won't be a filepath available. And I don't think we store that filepath anywhere so we won't have it available even after an upload, although we may have the filename. - @wmbutler

Good question. I noticed after saving backup files, the wallet currently remembers the location where I've saved previous backups (maybe windows does this automatically). The key-file login should use this same location or remember the last used key-file location if they are different (windows might do this automatically). If no backup has ever been created and no key-file has ever been used. It can default anywhere. Downloads directory would be fine or to the desktop. I would avoid having it default to a hidden /.apps directory or program_files browser directory unless specifically chosen by the user.

the only thing missing is a button that should appear to the right of the input field when the user clicks "Use key-file". That way the user can browse for a key-file (a) if there isn't one already detected or (b) if they want to use a key-file different than the one detected. - @tbone-bts

Good suggestion. If a key-file was used in the previous login session, it should pre-populate with the file. No selection needed. The current design accommodates the other scenarios by giving an explorer window from both the "use key-file" button and if the user clicks the input field as if to edit the file path.

However, I think your suggestion is better, as it's more clear. I've added an elipsis " ... " to the input field. Clicking this brings up the file explorer.

Xeldal commented 6 years ago

3)

In this alternative:

Xeldal commented 6 years ago

4)

This alternative is exactly like 3) only it switches more like tabs.

Tabs here would make it easier to add a 3rd way to login if we wanted to add "BRAIN-KEY" as another option. uhg. Forgot about that one. : (

[edit] Question: Brain-Key is essentially just a master password right? So Account Name login should accept a Brain-Key as a possible password. Problem is it's not obvious that you could use any of the following: Master-Password, Wif-Key, or Brain-Key. Maybe a tool tip. IDK

wmbutler commented 6 years ago

You will still need to display something in the Key File input field when the user has never backed up their first key file.

Xeldal commented 6 years ago

You will still need to display something in the Key File input field when the user has never backed up their first key file. - @wmbutler

Examples 3) and 4) both show this scenario. If the user has never backed up their first key-file, the input field states "Please Select A Key-File". This satisfies that something yes?

wmbutler commented 6 years ago

Examples 3) and 4) both show this scenario. If the user has never backed up their first key-file, the input field states "Please Select A Key-File". This satisfies that something yes?

There is no reason to select a key file if the key file is already stored in the local browser. This is the edge case I'm asking you to cover... this will be true for every user who creates a local wallet and decides not to back it up.

Xeldal commented 6 years ago

Ah, ok, I see what you are asking now. Thanks for clarifying. That's an important point.

problem: We have a situation now where people have been able to create a key-file(local wallet), without actually creating the file and managing it themselves. It's just stored in their browser (Not a good idea). We want them to actually have the file.

solution: We will need a transition process that requires all "local wallets" found in the browser to create a new key-file (backup), and then remove the remnants of the old-style "local wallet" so it doesn't trigger this required backup each time they try to login. [edit: In effect, if the user doesn't have a key-file, their only option is to login with Account Name. "local wallet" will be a thing of the past.]

I hope we are in agreement that ultimately the wallet should not be storing the key-file in the browser beyond the "login" session. If the user doesn't have an actual file to pull his keys from, the wallet will not have them either. [edit: ***see following comment]

So, (rough idea outline) 1) login is triggered 2) key-file option is user-selected or automatically-selected . 3) an old "local wallet" is found 4) requires downloading the new key-file before logging in 5) key-file is downloaded 6) check box for "remove old local wallet" is optionally checked 7) login completes

Basically in order to use the old "local wallet" they have to turn it into a key-file first. Until the user checks the box "remove old local wallet" it will continue to require a new backup each time. Check box isn't really necessary but it reinforces what's happening and puts responsibility on the user for removing the old wallet. they shouldn't feel comfortable in removing the old "local wallet" unless they are positive they have the newly created file. There's a couple different ways this could be done. I'll explore these later.

I think this works well, but I will continue to think about alternatives and will get something visually showing an example tomorrow if I have time.

Xeldal commented 6 years ago

*** Explination for strikeout in previous comment:

One possible undesirable side effect of the browser not holding on to the key-file after a login session is (as abit pointed out) that the browser can't pull a file to read without a specific user action. The user would have to actually select the key-file from an explorer window each time he logs in, which is not ideal.

So, i guess if we want the stream-lined experience of not having to use the explorer each time, the browser has to hold on to the key-file.

A few suggestions:

Xeldal commented 6 years ago

3-A)

In this example:

There's not really any good way to know if the user already has a bin file corresponding to the existing browser-saved "local wallet", so I'd suggest requiring the bin download again regardless. Alternatively you could add an option to "delete" the selected wallet but i'd rather the user have multiples of a backup file than no backup at all.

tbone-bts commented 6 years ago

I'm really looking forward to the implementation of this feature as I believe it will clear up a lot of confusion. Also, I wanted to point out that we should consider how a future trezor login would fit into this new way of representing the various options to log in, currently. If I'm not mistaken, logging in with a trezor would fit in nicely as another option. Of the selection methods discussed previously by @Xeldal, perhaps it would make sense to present the user with tabs, each one representing an available login method (key-file, password, trezor, etc).

ahdigital commented 6 years ago

I just read the thread and I'd also like to see this feature implemented. Is this going to be picked up?

Whilst I could review the proposed designs and create a tidy final version, implementing the actual functionality is beyond my capability unfortunately.

wmbutler commented 6 years ago

Assigned for you to review and propose.

AustinAmoruso commented 6 years ago

I can take this one if its just implemented the UI to handle the features and not integrating it.

ahdigital commented 6 years ago

Before going too far with hi-res mockups I've put together some wireframes.

login types

I see the login form having a field called 'login with' to allow the user to switch between:

Account name would be the default option as it has the largest user base. However, if the user switches to key file login their choice will be remembered (unless incognito).

If the user selects key file they can upload their .bin file and type their password. In the case of brainkey, JSON etc. there would be a link to import those from the current settings page.

If the user has their key file stored in the browser and has a backup we will allow login with just a password, and provide a mechanism to 'upload another' key file if they want.

In the scenario where a user has their key file stored in the browser and hasn't created a backup they will be able to enter their password and login without browsing for a key locally held on their computer.

However, a notice will be displayed discouraging this practice. This notice will keep appearing if the user ignores the notice and clicks login but it can be hidden if the user understands the risks and checks the checkbox. But, if the user clicks 'create backup & login' a ZIP file containing all browser stored keys will download, and the next time the user attempts login, they will be required to browse for a key locally held on their computer.

Essentially, scenario C of the key file login will see reduced usage as we push users to scenario A to provide their key file and then scenario B where the key file is just referenced by the browser.

I think this makes sense but if anyone see's any pitfalls let me know. Otherwise, I'll move onto creating themed versions of these wires.

Xeldal commented 6 years ago

In general, the process all looks good to me.

A couple small details I want to mention.

1)

I presume selecting "upload another" will take you to Scenario A's file selection process. Is that correct?

This may be a silly distinction but I think "upload another" could use a better choice of words.
I'd prefer something like "select a file" or "use a different file" to be more clear.

"upload another" seems to suggest that the browser will be carrying multiple key-files.
and also adds a subtle and unnecessary level of complexity. "i can add more than one?", "Where am I uploading?", "Is it safe?" It sounds like I'm sending it somewhere.

2)

Login via BrainKey is a legitimate method and could be added to the "Login with" menu

as could "Legacy wallet backups (.json)" but it looks like this is handled under key-file with "Have a different file type? Import it here" --unsure--

When logging in with a "legacy wallet", it could push them to Scenario C to encourage creating a standard backup/key-file

3)

Will the .zip file contain separate .bin files (key-files) for each wallet currently in the browser? This is a good idea I think rather than having them repeat the process if there are multiple wallets.

If the browser only holds 1 wallet, is it still a .zip file? Why not just the .bin file?

Either way I think is fine, I just hope these backups contain the key-files (.bin) and not something else for some other process called "import backup" or something.... because loging in with a key-file and importing a backup are essentially the same thing.

wmbutler commented 6 years ago

Really like this. It feels clean and understandable. Regarding labeling, should we ease into the terminology change?

Login With: Account Name (Cloud Wallet) Login With Key File (Local Wallet)

Then we can drop whichever we like later. This will probably help transitional troubleshooting efforts.

But, if the user clicks 'create backup & login' a ZIP file containing all browser stored keys will download, and the next time the user attempts login, they will be required to browse for a key locally held on their computer.

I'm a little worried about this. I suppose it's possible that a user backs up their .bin file and for some reason, they immediately lose it. I think the browser should not require them to upload it in order to retain access.

This may be a silly distinction but I think "upload another" could use a better choice of words. I'd prefer something like "select a file" or "use a different file" to be more clear.

How about: Upload alternate keyfile

as could "Legacy wallet backups (.json)" but it looks like this is handled under key-file with "Have a different file type? Import it here" --unsure--

Maybe we leave legacy and brainkey out of this for the first iteration?

Will the .zip file contain separate .bin files (key-files) for each wallet currently in the browser? This is a good idea I think rather than having them repeat the process if there are multiple wallets.

What zip file are we talking about? I think this issue is just to handle the .bin

ahdigital commented 6 years ago

Will the .zip file contain separate .bin files (key-files) for each wallet currently in the browser?

Yes.

If the browser only holds 1 wallet, is it still a .zip file?

No, it would make sense to be a download of the single bin.

I'll make a more detailed reply to all points in the morning. Just felt addressing the zip ones now would help the discussion whilst you guys are online atm.

tbone-bts commented 6 years ago

Nice work, @ahdigital. I do agree with @Xeldal's comments. I had the same thought about the word "upload". I think it should be avoided for the reasons already stated. I also think the word "import" makes it sound like a time-consuming process of importing data. I think "load" (or perhaps even "select" or "specify") would make more sense to more people.

ahdigital commented 6 years ago

Xeldal's queries

This may be a silly distinction but I think "upload another" could use a better choice of words. Not silly at all. It's why I've done the wireframes first so these types of details can be thrashed out.

I agree that the wording should change and also would avoid 'upload' as a term. Your suggestion of 'Use different file' makes sense but I'll throw another two into the mix for consideration for there succinctness:

  1. Use different file
  2. Switch file
  3. Change file

Clicking on this link would indeed take the user to scenario A.

Login via BrainKey is a legitimate method and could be added to the "Login with" menu ... as could Legacy wallet backups

Adding them to the 'Login with' menu would be a perfect place for them and I could certainly weave the corresponding fields into the form.

However, as @wmbutler pointed out it might be a good idea to phase their integration. Eventually we could drop the 'Have a different file type? Import it here' link from scenario A.

I just hope these backups contain the key-files (.bin) and not something else for some other process called "import backup" or something

I think I answered the other ZIP file questions above, but to be clear it wouldn't be for another import backup process.

wmbutler's queries

Should we ease into the terminology change?

Completely happy with this approach.

I'm a little worried about this. ... I think the browser should not require them to upload it in order to retain access.

So rather than pushing the user to scenario A, we keep pushing scenario C? If we do this then I think the user will never have a way to get to scenario A or B should they decide to start using their key file.

I figured that one of our goals would be for scenario C to be used less and less since it's not recommended to store the keys just in the browser. But, for those who understand the risks they can check the checkbox 'Don't ask me to backup again' to keep using scenario C.

Does that help? Does anyone else have an opinion to add? @Xeldal @tbone-bts

What zip file are we talking about? I think this issue is just to handle the .bin

If the browser has multiple key files stored in the browser I'd suggest downloading them all, bundled inside a ZIP file. If there's only one key file, the download will just be the single .bin file.

The reason for the ZIP file is to make the process as quick as possible. Additionally, we don't have to make another step in the UI to view all key files found and have the user choose between one, some or all to download.

tbone-bts's queries

I also think the word "import" makes it sound like a time-consuming process of importing data. I think "load" ... would make more sense to more people.

I think load is probably worse than import for making it seem like a lengthy process.

How about using something we already have, 'Restore'? Restore is terminology already in use via the settings page, and doesn't (I think) indicate time.

ahdigital commented 6 years ago

For the account name (cloud wallet) login, do I need to retain existing functionality whereby we have:

  1. Identicons
  2. Text to denote 'basic account' / 'lifetime' member?
  3. '+' icon to add account as a contact

For points 2 and 3, I've personally never used this and the first time I saw it I was confused..but others might find it useful. Does anyone have any insight?

wmbutler commented 6 years ago
ahdigital commented 6 years ago

Thanks for the confirmation. So, based off of the wireframes and using ANT design system I have put together this mock up.

login types modals

svk31 commented 6 years ago

To be honest I'm having a hard time following how the final proposal is intended to work, a final summary would be good. Some comments below though:

I'm opposed to changing the local wallet to require you to restore the wallet every time, that would be extremely annoying and also costly in terms of API calls and processing power.

The local wallet does not rely on a file at all, it uses a browser-based database which is loaded on app initialization. It's only the backup of this database which is stored in an actual file.

The main differences between the two modes are these:

Key generation

Storage

AFAIK It's also not possible for a browser to know what files a user has available, since websites are strictly cutoff from the filesystem. You can only access files via input elements where the user can browse their filesystem and select file(s).

wmbutler commented 6 years ago

@svk31 thanks for the clarification. It's my understanding that the key file is displayed as the active when in local wallet mode. I agree that the browser should not be cleared by default for local wallet mode, but I do see merit in the user being able to clear it (after forcing them to download a backup). This could help in situations where a user is on a public machine and they imported their .bin file.

The greatest accomplishment of this issue is that it unhides the various connect methods making troubleshooting significantly easier. I think we could move forward on this without addressing the nuances of clearing localstorage for now.

clockworkgr commented 6 years ago

One of the main issues I have with unifying the 2 types is the fact that a "cloud" login is not recoverable via brain key.

The "proper" way to handle this in my humble opinion (which is a lot of work unfortunately) is to basically have Active authority keys generated via password (similar to the cloud mechanism) and Owner authority keys via brain-key.

That way, local wallet works as before (since it contains all the actual keys), brain key will always allow you to restore access to your account (by setting a new Active auth key) and most of the functionality of the DEX will be available via a cloud/password-based login (without local wallet) since the active key will be generated.

Obviously this means that a few actions will not be available if you log in via cloud login/password since you wont have the owner key, but I do think this would be the best approach in unifying them kind of.

ahdigital commented 6 years ago

Summary

I've updated the visual above to include references to aid the discussion:

login types modals

Scenario 1

Account name (cloud wallet) mode is active and so the user must enter account name and password to login.

Scenario 2

The user is attempting login using key file (local wallet) mode where no browser database exists. Therefore, the user must select and upload their .bin file.

Users wishing to login with another file type will initially need to use the legacy method by clicking 'Restore it here'.

Scenario 3

Key file (local wallet) mode is active and browser database exists, so the user can simply enter just their password to login.

The file name of the last .bin uploaded could be stored in the browser database for reference, without attempting to access the user's file system which we agree wouldn't work anyway.

The user also has the choice to upload a different key file, and in doing so would be taken to scenario 2.

Scenario 4

Key file (local wallet) mode is active and browser database exists, however the backend has detected* that a backup hasn't been created/downloaded by the user.

The user can continue to login and ignore the warnings, select the option to backup or supress the warnings altogether.

* I admit I don't know how it's doing it, but it must be possible as I've seen a warning about backup displayed in the footer.

Regarding:

I'm opposed to changing the local wallet to require you to restore the wallet every time

We might have different perceptions of the term 'restore'. I wasn't proposing that the user has to upload their key file upon every login attempt. Rather, leverage the browser database and have the drop down confirm the active login mode and the section below it confirm the file name of the last key file uploaded**.

Does this avoid the performance issues you raised?

** The last key file uploaded refers to just the file name stored as a string (for example) in the browser database - not accessed via the user's file system.

Further thoughts

Does anyone have anything to add (for or against) to @clockworkgr's insight? Is it more an issue related to initial registration, rather than login?

wmbutler commented 6 years ago

I think this is the correct path forward.