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

Ledger: Define authentication flows #2355

Closed
2 of 3 tasks
landitus opened this issue Apr 7, 2022 · 15 comments
Closed
2 of 3 tasks

Ledger: Define authentication flows #2355

landitus opened this issue Apr 7, 2022 · 15 comments
Assignees

Comments

@landitus
Copy link

landitus commented Apr 7, 2022

When connecting to dApps, we need to go through the Ledger connection flow. There are technical limitations to the current Authentication flow, so we are proposing a new method for connecting with Ledger (and we plan to deprecate the current Auth code).

In order to build the Ledger authentication flow, we should cover the following:

  • Map the Authentication and Request credentials flows with Ledger
  • Prototype in Figma
  • Define tracking events
@landitus landitus self-assigned this Apr 7, 2022
@landitus landitus changed the title Ledger: Auth and transaction flows without access to the secret key Ledger: authentication flow Apr 11, 2022
@landitus landitus changed the title Ledger: authentication flow Ledger: Define authentication flow Apr 20, 2022
@landitus landitus changed the title Ledger: Define authentication flow Ledger: Define authentication flows Apr 20, 2022
@landitus
Copy link
Author

landitus commented Apr 21, 2022

Here's a draft of the User flows for the Connect wallet feature:

  • It includes the soon to be deprecated (and current) authentication flow
  • Adds a new flow for soon to be added Request Accounts method

Figma

User lifecycle flow - Connect Wallet

I will draft a prototype based on this flow so we can review it. cc/ @kyranjamie @markmhx

@landitus
Copy link
Author

In the case a user tries to connect with Ledger to an application using the Authentication method, which is not supported, should we display an error message?

Screenshot 2022-04-21 at 18 46 30

@landitus
Copy link
Author

landitus commented Apr 22, 2022

Based on feedback, the Connect wallet flow is now much more simple because there's no need to involve the Ledger during the account selection flow anymore.

We changed the branching decision between Auth and Request account. Now its based on wether the dApp needs Authentication or just the public keys.

Figma

User lifecycle flow - Connect Wallet - v2

@landitus
Copy link
Author

landitus commented Apr 22, 2022

There's one possible flow we are not covering, that can be triggered when the user is on a dApp and clicks Connect Wallet (and the Hiro wallet has not yet been setup). This seems part edger case, part something totally feasible, right?

  • Do we have any telemetry that would allow us to understand how frequent a setup happens in this scenario (for software wallets)?
  • For the Auth v1 flow, we should block the flow if the user chooses Ledger (allowing them to go back and choose a software wallet
  • For the Request accounts flow, I'm not sure what we should do. The whole Ledger setup happens inside the Connect window (check screenshot). Maybe it's a good thing it all happens without making the user go to a new tab? Still, I don't know what would happen once the Ledger is actually connected.
  • We could do nothing and add a tracking event and create report to see how many setup Ledger from the Connect window.

Wdyt @markmhx @kyranjamie ?

Here's a video on how the flow looks with the current build.

Ledger.-.Legacy.authentication.mp4

A simulation of what the user would see when Connecting though an app a brand new Wallet
simulation

@kyranjamie
Copy link
Collaborator

kyranjamie commented Apr 22, 2022

Do we have any telemetry that would allow us to understand how frequent a setup happens in this scenario?

I don't think so

For the Auth v1 flow, we should block the flow if the user chooses Ledger (allowing them to go back and choose a software wallet

A warning/tooltip, at least, would be nice

For the Request accounts flow, I'm not sure what we should do. I think the Ledger connect flow is not great due to space constrains. Still, I don't know what would happen once the Ledger is actually connected. Maybe it's a good thing it all happens without making the user go to a new tab?

Umm 🤨 . It think it might be possible to go off to full screen mode and return to the popup following onboarding. This wouldn't be super trivial, though.

@landitus
Copy link
Author

Thanks @kyranjamie. Will try these solutions first and see how they look

@andresgalante andresgalante moved this from Design to Development in Hiro Wallet (DEPRECATED) May 2, 2022
@landitus
Copy link
Author

landitus commented May 2, 2022

There's a more in depth solution for connecting an dapp to a brand new Wallet (without account created) here

@landitus
Copy link
Author

landitus commented May 4, 2022

I've updated the prototype based on this proposal and with help of @kyranjamie. This prototype shows authenticated and non-authenticated scenarios with both Auth and Request accounts methods, from the point of view of a user interacting with a dApp.

Here are the new updates:

  • We've decided to force the onboarding (or resuming the onboarding if abandoned) always in a new tab. This way, Ledger can be set up without issues
  • We are now accounting for resuming the onboarding, if the user installed the Wallet but didn't create the first account. We are proposing a new screen to be displayed in the Wallet pop-up on top of the app: "You still need to setup
 your Hiro Wallet"
  • As an enhancement, once the Wallet setup is complete, we are proposing a callback to return to the app. This will only be included if the Wallet setup originated from an dAapp.

Question
My only concern is this scenario where the user could technically go into STXNFT (or any site with the Auth method) to buy an NFT, setup a brand new Ledger wallet and realize only at the end of the whole process, that Ledger is not supported because of the old Auth method. I know we are waiting on fix on this, so mind this is reflecting the current Auth method situation cc/ @markmhx

Check the Figma prototype, where each use-case/flow has a flowchart as well.

Choose an account

Wallet home

@markmhendrickson
Copy link
Collaborator

markmhendrickson commented May 5, 2022

We are now accounting for resuming the onboarding, if the user installed the Wallet but didn't create the first account. We are proposing a new screen to be displayed in the Wallet pop-up on top of the app: "You still need to setup
 your Hiro Wallet"

Is this screen to be shown specifically when the user tries to sign a transaction having not yet set up their wallet yet? Or are there other cases in which this is shown?

As an enhancement, once the Wallet setup is complete, we are proposing a callback to return to the app. This will only be included if the Wallet setup originated from an dAapp.

We last looked into this a couple of months ago upon which @fbwoolf flagged some difficulties and @landitus was going to investigate Metamask's approach further, if I'm not mistaken.

Do we have a better sense these days of how we'd go about this?

My only concern is this scenario where the user could technically go into STXNFT (or any site with the Auth method) to buy an NFT, setup a brand new Ledger wallet and realize only at the end of the whole process, that Ledger is not supported because of the old Auth method. I know we are waiting on fix on this, so mind this is reflecting the current Auth method situation

Our current plan is to release Ledger support with app authentication support, leveraging Zondax's upgrade to the Stacks app that enables this. So, Ledger should be supported fine in this STXNFT case?

@landitus
Copy link
Author

landitus commented May 5, 2022

Is this screen to be shown specifically when the user tries to sign a transaction having not yet set up their wallet yet? Or are there other cases in which this is shown?

Yes, and also if they try to connect and authenticaticate with a wallet that was not set up.

Do we have a better sense these days of how we'd go about this?

I forgot about looking into MetaMask. But I'll take a look!

Our current plan is to release Ledger support with app authentication support, leveraging Zondax's upgrade to the Stacks app that enables this. So, Ledger should be supported fine in this STXNFT case?

If it's supported fine, there would be no issues. Would you like me to update the mocks assuming we get Ledger supported?

@markmhendrickson
Copy link
Collaborator

Yes, and also if they try to connect and authenticaticate with a wallet that was not set up.

For both transaction signing and authenticate, clicking "Setup Hiro Wallet" goes to the landing page ("Explore world of Stacks") with options to generate key or sign in?

If so, perhaps it's best to send the user directly there and simply show a message on the top saying "Please set up your wallet by choosing an option below before proceeding to [sign into Testing App|sign a transaction with Testing App]". That way we save the user a step and save ourselves the need for an entirely new screen.

wdyt?

If it's supported fine, there would be no issues. Would you like me to update the mocks assuming we get Ledger supported?

Yep I believe so. @kyranjamie can you confirm?

I forgot about looking into MetaMask. But I'll take a look!

🙏

@kyranjamie
Copy link
Collaborator

kyranjamie commented May 6, 2022

We last looked into this a couple of months ago upon which @fbwoolf #1808 (comment)

This referred to handling the returning to app scenario when user initially installs the wallet. That is not presented here. This flow describes wallet installed but not set up yet scenario.

Our current plan is to release Ledger support with app authentication support, leveraging Zondax's upgrade to the Stacks app that enables this. So, Ledger should be supported fine in this STXNFT case?

This change is not necessarily a golden ticket to full hardware wallet support. I'm not sure yet if auth alone without an appPrivateKey is that useful for many dApps. This is my next priority to figure out, in testing the new Ledger app version.

@markmhendrickson
Copy link
Collaborator

This referred to handling the returning to app scenario when user initially installs the wallet. That is not presented here. This flow describes wallet installed but not set up yet scenario.

Ah I see, got it. Should we break this out into its own issue, separate from Ledger since it's not strictly needed for it?

Same question for the new setup screen or message upon auth or signing for users who haven't completed setup. It seems we could tackle these as a non-Ledger-dependent enhancement.

This is my next priority to figure out, in testing the new Ledger app version.

Let's test ASAP so we can get clarity on this.

@landitus
Copy link
Author

landitus commented May 6, 2022

Ah I see, got it. Should we break this out into its own issue, separate from Ledger since it's not strictly needed for it?

Sounds good to me

Same question for the new setup screen or message upon auth or signing for users who haven't completed setup. It seems we could tackle these as a non-Ledger-dependent enhancement.

This one is intended for the Ledger setup UX. As we've identified here, we'd like to route all onboarding flows to a new tab, especially as Ledger is better set up that way. We could do a quick fix for this (and delay building this new screen) if we set the "I have a Ledger" link to open in a new tab. This way, at least, Ledger is not set up in the pop-up window, but in a full tab.

165317818-5016c914-cb40-45a6-9b06-d396773e8120

@markmhendrickson
Copy link
Collaborator

We could do a quick fix for this (and delay building this new screen) if we set the "I have a Ledger" link to open in a new tab. This way, at least, Ledger is not set up in the pop-up window, but in a full tab.

Yep, this is what I'd suggest here in support of the initial release of Ledger.

I think we also discussed converting the Ledger connection overlay below into its own page view so we can avoid loading the "Explore the world of Stacks" landing page first, only to display the overlay immediately on top of it in this scenario. That sounds like a good enhancement to me as well for this initial release, assuming it doesn't require a lot of code change.

image

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

No branches or pull requests

3 participants