joinmarket-webui / jam

Your sats. Your privacy. Your profit.
https://jamapp.org
MIT License
264 stars 54 forks source link

Make one wallet the standard #228

Open editwentyone opened 2 years ago

editwentyone commented 2 years ago

what do you think about sliming down the overall use of passwords for JAM and wallets by giving the user in the normal journey the following:

we are not getting rid of the screens and functionality that is build already, we just link them differently into another flow.

later we could also maybe build in import/ export of wallets for different cross usage like joininbox or backups if needed.

the default user would have an easier experience without too many passwords to remember/ secure and there is no lock/unlock issue with running processes in the background.

openoms commented 2 years ago

On most systems the wallet password is the only encryption standing between needing a password or having the bitcoin keys readily available for anyone with physical access to the disk.

Be aware that getting rid of wallet password there will be no security left. A slightly better solution could be to reuse the wallet password to be the app password so then it can be stored in the memory to unlock the wallet also and disappears when the node is switched off. See the raspiblitz LND wallet for example: the apps are locked with the PasswordB (which is the bitcoin password reused) the LND wallet has it's own PasswordC which is not stored by default and locks all funds on a restart.

Think of what happens when someone takes the node away. Passwords not stored (or only in RAM) are flushed and strong encryption saves the user from losing funds.

The problem with loosening the security is that JM is meant for higher amounts by nature so autounlocks and passwordless setups risk a lot potentially.

ghost commented 2 years ago

I wouldn't be comfortable with implementing completely passwordless wallets for the reasons @openoms mentioned above.

I do like the general idea of having a single wallet flow by default (with the option to create more wallets if needed). I feel like most of our users would be perfectly fine with having only one wallet. Currently the "choose wallet" screen is really prominent and I don't think it needs to be such a central thing in the UX when most users (I assume; might be wrong of course) have a single wallet only.

We could, for example, build a guided onboarding flow that is shown the first time the user opens the app. There we could guide the user to create a wallet and then make this wallet the default. After unlocking the app (and the wallet) this wallet would then be opened directly. A pro user could still reach the "switch / create wallets" screen via settings for example. There they could also choose which wallet (if they have several) is the default one. Does that make sense from a UX point of view?

editwentyone commented 2 years ago

nice, I like the insights. if its a possible way, I would love to see the user punching in the chosen wallet password as the jam login password. we need to optimize the wording for the user to understand that a login is the same as unlock the wallet in general. after that we are in a single wallet environment.

all the other features like adding another wallet, switch wallets, make a wallet the new default, lock and unlock wallets, maybe even delete wallets and export and import plus rescan wallets should live under settings. imo that makes more sense.

ghost commented 2 years ago

all the other features like adding another wallet, switch wallets, make a wallet the new default, lock and unlock wallets, maybe even delete wallets and export and import plus rescan wallets should live under settings. imo that makes more sense.

💯 agree on this!


About reusing the wallet password as the app password: While I'd love to have this from a Jam point-of-view, I'm not sure we can do that in a clean way considering the bigger picture. It might turn out to be more confusing in the end. My thinking is that by convention we're forced to use the shared app password that the nodes provide to unlock the app. But we cannot reuse this password to unlock the wallet for security reasons. That in turn means that we basically have to go down the two passwords route. I'll explain in more detail below, please tell me if I'm wrong. I might easily be miss something!

(Sorry in advance for the wall of text.)

App Password vs. Wallet Password

Currently, there's two different passwords:

  1. The app password: Protects the React app and the JoinMarket API from unauthorized access while being served on the network. This password can be stored on disk.
  2. The wallet password: Protects the funds in the JoinMarket wallet from unauthorized access even when the node is turned off. This password cannot be stored on disk since it should still protect the wallet e.g. in case of theft.

Like pointed out by openoms, a way of maybe achieving a single-password flow would be to reuse the wallet password for the app password. However, since nodes like RaspiBlitz or Umbrel work with a single shared app password ("Password B" on a RaspiBlitz) to unlock the apps (e.g. RTL, Thunderhub) running on these nodes, it would be "non-standard" in a way for us to deviate from this scheme. That could be confusing for users who are used to the shared app-password.

I feel like it's the safer route to stick with the way the nodes want us to use their app-password. We also can't reuse this app-password for the wallet of course, since it's stored on disk e.g. on the RaspiBlitz.

So assuming all my assumptions above are true (please tell me if I'm wrong!) we basically have to keep the two passwords around:

  1. The shared app password provided by the node to unlock the app
  2. A user chosen wallet password that is not stored on disk

I do see how this is confusing. We somehow have to engineer the UI in a way that it isn't weird to enter two passwords at once. Maybe we could do it like a 2FA flow since users are already accustomed to entering two passwords there. 🤔 Another option might be to stick with Basic Auth (our current approach on Umbrel) since browsers cache these passwords so it would feel like a one-password flow most of the time.

I'd love to see a simpler, one-password approach though if anyone has any ideas! Hope this is helpful background. Let me know what you think everyone!

AdamISZ commented 2 years ago

Currently the "choose wallet" screen is really prominent and I don't think it needs to be such a central thing in the UX when most users (I assume; might be wrong of course) have a single wallet only.

This is an interesting point and intuitively, correct (arguable though!).

It's worth remembering that one of the main distinguishing features of a Joinmarket wallet is that it has 5 (by default, which most don't change) accounts and they function very much like separate wallets to all intents and purposes.

There is definitely a use case for switching up to a new wallet file at least from time to time. Depending on .. stuff .., a user can end up having thousands of used addresses in a JM wallet, unlike most wallets (don't forget - even failed coinjoins "use up" addresses, because they were communicated to counterparties and are therefore considered unusable in future - notice how different that is, from most wallets). This causes more scanning slowdown, and our wallet sync is not the fastest, in the first place!

(Of course there are other reasons to switch to a new wallet file; I just mention that one as it's less obvious)

So the obvious thing I guess is to hide 'new wallet' away from the main workflow, but do keep it and don't hide it too well :)

editwentyone commented 2 years ago

🎨Figma Settings 🎨Figma Wallet

image image
dergigi commented 2 years ago

Resolved, at least partially, with #296 and #316

ghost commented 2 years ago

Agreed, it's partially resolved (also relevant: #325). For the scope of v0.0.7 we can definitely consider it resolved.

The meat of this feature will be replacing the "home" screen which is currently the "select which wallet to unlock screen" with something more sleek that just asks you to input the wallet password for the default wallet. The screen to switch wallets will then only be accessible from the settings.

@editwentyone If you're motivated to think about how this could look, feel free to open a new issue with any UI ideas! 🙏 ✨

editwentyone commented 2 years ago

lets use this to unlock the default wallet. also it would be nice to declare another wallet the "default", is this possible through settings screen?

image

https://www.figma.com/file/kfejZJFlwBywvLEnPEmJo1/?node-id=3994%3A73345

ghost commented 2 years ago

also it would be nice to declare another wallet the "default", is this possible through settings screen?

@editwentyone I'd say it'll be easiest to slap that onto the "wallet switcher" screen:

Screenshot 2022-06-20 at 17 09 29

What do you think? Any ideas on how it could look like? I'm thinking a "make default wallet" button would probably be sufficient. 🤔

MaxHillebrand commented 2 years ago

How about a star icon?

editwentyone commented 2 years ago

"make default wallet" button was also my idea, but there will be too many buttons on each row.

star icon is more the "favorite" route

editwentyone commented 2 years ago

I came up with this 2 ideas to drag drop a tag or drag drop a line to the top , let me know what you think:

Figma 🎨

image

or 3rd option with radio buttons, saved when chosen/ selected, should be the easiest one

Figma

image
ghost commented 2 years ago

or 3rd option with radio buttons, saved when chosen/ selected, should be the easiest one

I like this direction the most. Drap and drop implies an order between the elements which is not there in our case. What about the radio buttons option but make them ⭐ shaped?

editwentyone commented 2 years ago

because there can only be one default wallet, I would stay with radios. stars are more favorites and you can have more then one

ghost commented 2 years ago

See also https://github.com/joinmarket-webui/joinmarket-webui/issues/337

ghost commented 2 years ago

Finishing this is blocked by https://github.com/joinmarket-webui/joinmarket-webui/issues/337.

ghost commented 2 years ago

Moving this issue out of v0.0.10 since we need a decision / designs for https://github.com/joinmarket-webui/jam/issues/337 first.