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

feat: match authResponse username with 2.0 bns lookup #984

Merged
merged 6 commits into from
Apr 29, 2021

Conversation

hstove
Copy link
Contributor

@hstove hstove commented Mar 23, 2021

Fixes #953

This is the start of the work that will allow apps to handle an authResponse properly with 2.0 BNS.

After authentication, @stacks/auth calls verifyAuthResponse. This function runs a few checks, and also runs a check to ensure that the auth response is signed by the owner of the username provided in the payload.

Right now, you need to use a specific AppConfig to have a custom coreNodeUrl. When provided, it uses a different BNS lookup URL. For testing, I'm using our testnet registrar.

@vercel
Copy link

vercel bot commented Mar 23, 2021

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/FKzXe7eB3DRUNsgYPzoH6UVcakMC
✅ Preview: https://stacks-js-git-feat-stacks-2-bns-verification-blockstack.vercel.app

@markmhendrickson
Copy link

@hstove should someone review these code changes now?

Copy link
Contributor

@reedrosenbluth reedrosenbluth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! left one small style comment

packages/auth/src/verification.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #984 (4dd9e67) into master (dbab627) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #984      +/-   ##
==========================================
+ Coverage   60.69%   60.79%   +0.09%     
==========================================
  Files         101      101              
  Lines        7585     7596      +11     
  Branches     1412     1414       +2     
==========================================
+ Hits         4604     4618      +14     
+ Misses       2976     2973       -3     
  Partials        5        5              
Impacted Files Coverage Δ
packages/auth/src/userSession.ts 57.74% <100.00%> (+0.60%) ⬆️
packages/auth/src/verification.ts 81.25% <100.00%> (+4.55%) ⬆️

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 dbab627...4dd9e67. Read the comment docs.

@andresgalante
Copy link
Member

I this ready to merge?

@hstove
Copy link
Contributor Author

hstove commented Apr 29, 2021

Yes, ready for merge + release, cc @reedrosenbluth

@reedrosenbluth reedrosenbluth merged commit e0ab244 into master Apr 29, 2021
Splat Team automation moved this from 💻 In development to 🚀 Done Apr 29, 2021
@kyranjamie kyranjamie deleted the feat/stacks-2-bns-verification branch June 21, 2021 13:38
@CharlieC3 CharlieC3 removed this from 🚀 Done in Splat Team Jan 12, 2022
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 this pull request may close these issues.

Update @stacks/auth to work with new BNS APIs Integrate Stacks Blockchain API for querying username data
4 participants