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

makeAuthResponse username is not populated for .btc names #1144

Closed
pradel opened this issue Nov 25, 2021 · 14 comments
Closed

makeAuthResponse username is not populated for .btc names #1144

pradel opened this issue Nov 25, 2021 · 14 comments
Assignees

Comments

@pradel
Copy link
Contributor

pradel commented Nov 25, 2021

Describe the bug

When the user login into an application via the Hiro wallet, the username field is an empty string while it should be the .btc domain.

What version of Blockstack.js are you using?

Is the bug present in the Blockstack CLI, Gaia hub, Connect or Blockstack Browser?

To Reproduce

Steps to reproduce the behavior:

  1. use the Hiro wallet to login into an app with an account that does have a .btc name
  2. call userSession.loadUserData() and see that the username field is an empty string

Expected behavior

When the user is logging in into my app I should be able to read the owner .btc username from the userSession.loadUserData() function. Similarly to .id.stx names.

Desktop (please complete the following information):

  • OS: Mac
  • Browser: chrome
  • Version 96.0.4664.55
@friedger
Copy link
Contributor

The usernames of which network would you expect? You get the stx address for mainnet and testnet and could use that to find a username. This is done in the todo example hirosystems/todos#104

@pradel
Copy link
Contributor Author

pradel commented Nov 29, 2021

My understanding is that only one name is allowed at a time, so I was expecting to have the username filled, the Hiro wallet extension is displaying the .btc name, so it's a bit confusing that it's not passed down as the other names (.id.stx ... )

@friedger
Copy link
Contributor

@pradel I agree. It is confusing. This needs to be defined in the auth protocol.

I don't like the username property in the auth response because it does not include in indicator for the network where this username was registered. Users can have different usernames on mainnet and on testnet. There is also a discussion whether a public key can own more than one username (stacksgov/sips#37)

@markmhendrickson
Copy link

Is this issue resolved or is further action needed here for Stacks.js?

@pradel
Copy link
Contributor Author

pradel commented Dec 6, 2021

@markmhx I don't know what approach you want to go with for this, but from a user perspective, the current behaviour of the username field is really confusing.

@friedger
Copy link
Contributor

I like to see the username in the auth response: stacksgov/sips#50

@crazy0ak
Copy link

crazy0ak commented Jan 5, 2022

This issue may be the cause of problems I'm experiencing when trying to use the btc.us app.

Steps to repro:

  1. Purchased crazy0ak.btc alias using fresh wallet (A)
  2. Transferred crazy0ak.btc alias to an existing wallet (B) which already has crazy0ak.id alias from a long time ago.
  3. Switch to wallet (B) which now owns crazy0ak.btc
  4. Attempt to manage crazy0ak.btc using btc.us app by clicking the "Sign In" button
  5. See the following ERROR conditions in the browser console:

Error @ btc.us/:1

Access to fetch at 'https://core.blockstack.org/v1/names/crazy0ak.id.stx' from origin 'https://btc.us' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

Error @ main.js/:10

GET https://core.blockstack.org/v1/names/crazy0ak.id.stx net::ERR_FAILED 301

Clearly the second error is unhappy with a 301. The location header in the 301 response is: https://stacks-node-api.stacks.co/v1/names/crazy0ak.id.stx

In this instance I would love to know how to destroy the old crazy0ak.id association. If that's possible then it may be the easiest fix.

I was originally following Issue #1117 which seemed like it may be the fix. So either it's still borked, or perhaps the btc.us app is not on the latest release?

@m-roll
Copy link

m-roll commented Feb 28, 2022

I am developing a web application with stacks.js and I've run into the same thing. #1199 . The popup/modal for selecting a wallet is able to correctly identify the usernames, but this is not passed in to the UserSession object when the user has finished going through the auth flow.

@ahsan-javaid
Copy link
Contributor

cc: @friedger or @kyranjamie for thoughts.

@friedger
Copy link
Contributor

@m-roll The current implementation does not return usernames.
Workaround is to look the name up via https://hirosystems.github.io/stacks-blockchain-api/client/classes/generated.namesapi.html#getnamesownedbyaddress

@m-roll
Copy link

m-roll commented Feb 28, 2022

@m-roll The current implementation does not return usernames.
Workaround is to look the name up via https://hirosystems.github.io/stacks-blockchain-api/client/classes/generated.namesapi.html#getnamesownedbyaddress

Thanks!

@kyranjamie
Copy link
Contributor

kyranjamie commented Apr 14, 2022

Just running through this issue with @janniks.

Can someone explain why it's important for the wallet to return a username in the first place? App can themselves query the registrar. Is anyone opposed to killing this functionality? Stacks.js could even expose helper methods to fetch the name.

cc/ @pradel @friedger @ahsan-javaid

@pradel
Copy link
Contributor Author

pradel commented Apr 14, 2022

As it's easy for apps to fetch the name, definitely not opposed to remove this functionality. This issue was more for consistency as the .btc names were not injected. Moving to the app level is good too.

@markmhendrickson
Copy link

I'd also support removing the username from the wallet's response data as part of our general intent to separate concerns better going forward.

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

Successfully merging a pull request may close this issue.

8 participants