[Fix/1144] Populate username field in auth response

[ahsan-javaid]

Description

This PR fixes the issue of empty username field in the auth response.

For details refer to issue #1144

Type of Change

• New feature
• Bug fix
• API reference/documentation update
• Other

Does this introduce a breaking change?

No

Are documentation updates required?

No

Testing information

Provide context on how tests should be performed.

1. Test case added
2. npm run test passed successfully

Checklist

• Code is commented where needed
• Unit test coverage for new or modified code paths
• npm run test passes
• Changelog is updated
• Tag 1 of @yknl or @zone117x for review

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blockstack/stacks-js/F1Ltmo1LzEb9xjLzhFcD5ajip3sB
✅ Preview: https://stacks-js-git-fix-populate-username-field-in-862972-blockstack.vercel.app

[codecov]

Codecov Report

Merging #1154 (87483dc) into master (af65b91) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1154      +/-   ##
==========================================
+ Coverage   62.03%   62.07%   +0.04%     
==========================================
  Files         122      122              
  Lines        8315     8322       +7     
  Branches     1534     1537       +3     
==========================================
+ Hits         5158     5166       +8     
+ Misses       3052     3051       -1     
  Partials      105      105              

Impacted Files	Coverage Δ
packages/auth/src/userSession.ts	59.33% <100.00%> (+1.99%)	⬆️
packages/auth/src/messages.ts	82.85% <0.00%> (+1.42%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9962ea...87483dc. Read the comment docs.

[friedger]

how does it work with testnet?

[ahsan-javaid]

https://github.com/blockstack/stacks.js/blob/fe831cf28d71de9410c81cfabe256fd511e9aa4d/packages/auth/src/userSession.ts#L227-L233

The handlePendingSignIn function is already configured with mainnet. It does not have any testnet param. So the implementation is aligned with existing auth flow. User may also set coreNode url there.

Secondly

When we try to get bnsLookupUrl from testnet it will give you bnsLookupUrl of mainnet.
Here is code sample:

import { StacksTestnet } from '@stacks/network';
void (async () => {
  const n = new StacksTestnet();
  console.log(n.bnsLookupUrl);
})();

Output: https://stacks-node-api.mainnet.stacks.co

Reason: Refer to this commit: fix: use only mainnet URLs during auth BNS checks
1434262

This proves to use only mainet urls for auth BNS checks.
In case if we add network parameter in handlePendingSignIn then it will return mainet bnsLookupUrl on both testnet and mainnet as the reason is mentioned above.

Other than that if you want to purpose different thoughts then most welcome.

[friedger]

The hard coded URLs are only fallback. Here the user does not have a chance to use testnet.

[ahsan-javaid]

@kyranjamie @friedger Share your thoughts on this pr for further action items. Thanks 🙏

[friedger]

Shouldn't this issue be solved in wallet.js on the authenticator?
The app can always try to fetch more data than thrbuser/authenticator provided but the auth should not do it.

[ahsan-javaid]

@friedger Thanks for the review.
Can you please point out to wallet.js/authenticator repo that you mentioned above. 🙏

[friedger]

@ahsan-javaid Somewhere around here:

stacks.js/packages/wallet-sdk/src/models/wallet.ts

Line 67 in 3f8fa11

existingAccount = deriveAccount({

Newly created accounts are not populated, this should be changed, also it is never checked whether the user has registered a name by now.

The client should be able to trust the authenticator that the authenticator knows about registered usernames. If the auth response does not contain the username, the client can savely assume that the user does not have a username or that the user does not want to the app to use it. IMHO.

[friedger]

Another good place could be https://github.com/blockstack/stacks-wallet-web/blob/f1567b6812b2a2e7de1532fbe7a690fbdc9166d2/src/app/query/account/account.query.ts#L27

[ahsan-javaid]

Closing based on friedger comment to handle this on wallet repo.