leather-io / extension

Leather browser extension
https://leather.io
MIT License
305 stars 144 forks source link

Fix ability to authenticate with renewed username #504

Closed markmhendrickson closed 2 years ago

markmhendrickson commented 4 years ago

My username markmhendrickson.id expired but remained listed in the "Choose username" modal during authentication. Having since re-purchased it, I'm unable to authenticate with any apps (e.g. https://app.sigle.io/, http://humans.name/).

The selection interaction for this username works in the popup, but then I see an error in the app's console after the popup closes automatically.

Screen Shot 2020-07-11 at 20 54 23 Screen Shot 2020-07-11 at 20 54 49
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

This issue has been automatically closed. Please reopen if needed.

markmhendrickson commented 2 years ago

This issue may become particularly relevant again soon since usernames are set to expire en masse after their 1-year extension upon the launch of Stacks 2.0. Those who renew their usernames in time may find themselves subsequently unable to authenticate.

fbwoolf commented 2 years ago

@markmhx when I look at your username, the expire block is 52595 and the current block is 51673, so why do you think your username is expired? https://stacks-node-api.mainnet.stacks.co/v1/names/markmhendrickson.id

fbwoolf commented 2 years ago

I can't reproduce this with the secret key @kyranjamie gave me bc his is the same: https://stacks-node-api.mainnet.stacks.co/v1/names/kyranjamie.id

fbwoolf commented 2 years ago

There are 3 requests made to the names endpoint 2 are successful and 1 returns an error:

200 Success: https://stacks-node-api.mainnet.stacks.co/v1/names/kyranjamie.id.blockstack https://stacks-node-api.stacks.co/v1/names/kyranjamie.id.blockstack Screen Shot 2022-03-09 at 7 34 29 PM

400 Error: https://registrar.stacks.co/v1/names/kyranjamie.id.blockstack Screen Shot 2022-03-09 at 7 36 07 PM

fbwoolf commented 2 years ago

I feel like I am seeing a different welcome screen than shown in the issue description for Sigle: Screen Shot 2022-03-09 at 7 39 58 PM

fbwoolf commented 2 years ago

Going to drop a few links here in stacks.js that might be relevant to this issue:

handlePendingSignIn https://github.com/hirosystems/stacks.js/blob/48159007703933326f0e5a1ff6a2c061f2c753e6/packages/auth/src/userSession.ts#L260

verifyAuthResponse https://github.com/hirosystems/stacks.js/blob/48159007703933326f0e5a1ff6a2c061f2c753e6/packages/auth/src/verification.ts#L278

doPublicKeysMatchUsername https://github.com/hirosystems/stacks.js/blob/48159007703933326f0e5a1ff6a2c061f2c753e6/packages/auth/src/verification.ts#L82

fbwoolf commented 2 years ago

https://stacks-node-api.mainnet.stacks.co/v1/names/fara.btc

My expire_block is 0 so now I'm confused?

fbwoolf commented 2 years ago

I see the zonefile now for markmhendrickson.id does show the name is expired: https://gaia.blockstack.org/hub/13ceLK6xjw367B8QpVLX6i2mDtoYg6iawt/profile.json

But, kyranjamie.id is not (which is likely why I can't repro the issue): https://gaia.blockstack.org/hub/1JH5sHigAypQ5hTjioLRK31uasnv2HG35d/profile.json

andresgalante commented 2 years ago

@markmhx please confirm that this is still a bug and provide steps to reproduce it

markmhendrickson commented 2 years ago

I tried to authenticate with markmhendrickson.id again but the app (Arkadiko) didn't detect it.

image

Note the errors appear different than my original report from the days of Stacks 1.0.

fbwoolf commented 2 years ago

Note the errors appear different than my original report from the days of Stacks 1.0.

Looks like the error is coming from handlePendingSignIn which I had suspected might be where we would need to make changes ...linked to it in stacks.js here: https://github.com/hirosystems/stacks-wallet-web/issues/504#issuecomment-1064171378

larrysalibra commented 2 years ago

I have a similar error when signing in with a .id that has NOT yet been renewed since the stacks 2.0 launch but has been "upgraded" (transferred from the legacy owner key to the wallet key)

Digging around it seems like the error is coming not from the wallet but from Stacks Connect.

Screen Shot 2022-03-16 at 17 25 28
fbwoolf commented 2 years ago

Digging around it seems like the error is coming not from the wallet but from Stacks Connect.

Yes, Connect uses @stacks/auth userSession.

fbwoolf commented 2 years ago

@markmhx It looks like it is making a call to this endpoint and failing: https://registrar.stacks.co/v1/names/markhendrickson.id

It is throwing the error here: https://github.com/hirosystems/stacks.js/blob/b0d039671b8fe5bc0f35ed1bb9557c08d1f9e3b4/packages/auth/src/userSession.ts#L262

I suspect that doPublicKeysMatchUsername is returning false and throwing that error?

How do we tackle a fix needed in stacks.js?

markmhendrickson commented 2 years ago

@janniks could you possibly help determine and execute the fix here on the Stacks.js side?

janniks commented 2 years ago

Yes, I can look into this.

Do we have testnet id's (or something similar) for an expired and a valid id, for reproducing?

markmhendrickson commented 2 years ago

Good question. @Eshwari007 @timstackblock do you have any such usernames in your arsenal?

Eshwari007 commented 2 years ago

Good question. @Eshwari007 @timstackblock do you have any such usernames in your arsenal?

@markmhx @janniks We do have a couple of IDs in our test account. Not sure if they were from v1 though.

Screen Shot 2022-03-17 at 9 55 24 AM
markmhendrickson commented 2 years ago

Those usernames appear to be registered on Stacks 2.0 and therefore haven't expired yet.

@janniks it may be easiest if we meet via Zoom with screen share since I have an expired username in my personal wallet?

larrysalibra commented 2 years ago

I spent a bit more time digging into this today. I'm using the name "newinternetlabs.id" which was both renewed and transferred to the wallet address (aka upgraded). I haven't been able to sign in with it using Stacks connect since i transferred it to the wallet address.

Signing in with stacks todos sample app gives me a bit more information than in an app using Stacks Connect.

It says that that there was an error checking doPublicKeysMatchUsername.

In this function, we look at the issuer address from the authentication token and compare it to the address that owns the name.

Unfortunately, I can't figure out how to print out the token to console so that I can inspect its contents. Prior to stacks connect, this was passed in the address bar URL, but another method seems to be used. Happy to take a look if someone can point me in the right direction.

It looks like it is making a call to this endpoint and failing: https://registrar.stacks.co/v1/names/markhendrickson.id

With regards to that error, I don't think that is the root of this problem. The function that verifies the username owner has several look up addresses and if any one of them succeeds, then the authentication will succeed. From what I've seen, this url: https://registrar.stacks.co/v1/names/ fails even on successful authentications because that url doesn't actually look up names. Perhaps it should be removed as a fallback url?

Screen Shot 2022-03-22 at 20 57 10
markmhendrickson commented 2 years ago

@janniks has prepared this build that removes all logic for checking usernames from the authentication process: https://www.dropbox.com/s/zpjwoddge0soy13/stacks-wallet-chromium.zip?dl=0

It can be used to test whether this issue is resolved, and whether there are any undesired side effects to doing so.

@larrysalibra our suspicion here is that it's not necessary to block authentication with such checks in general, and it's most expeditious to just get rid of them if they're causing issues with renewed and expired names. Do you recall why these lookups were integrated into blockstack.js in the first place?

MountainMaster commented 2 years ago

The DEV-Version (3.3.0?) did not work for me. On btc.us it does not login without any response and on arkadiko I get the following error

code: "login_failed"
message: "Failed to login: Invalid authentication response."
name: "LoginFailedError"

image

markmhendrickson commented 2 years ago

I'm getting the same – note the "doPublicKeysMatchUsername" error checking message

image image
markmhendrickson commented 2 years ago

@janniks do you expect this issue to be resolved fully app-side with https://github.com/hirosystems/stacks.js/pull/1230?