Skip to content

[Fix] Handle account switching inside connected wallet for SIWE auth #3963

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

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

jnsdls
Copy link
Member

@jnsdls jnsdls commented Aug 6, 2024

FIXES: #3912

TL;DR

This PR fixes the handling of account switching within a connected wallet in Sign-In with Ethereum (SIWE) authentication states.

What changed?

  • Updated the isLoggedIn function to accept an address argument and added more checks to validate the JWT and the associated address.
  • Pass account as a new parameter to useSiweAuth to ensure the correct account is handled.
  • Removed experimental server external packages from next.config.mjs.
  • Removed @shikijs/twoslash dependency from package.json.

How to test?

  1. Switch accounts within a connected wallet and ensure the SIWE authentication state updates correctly.
  2. Verify that the application runs without errors related to the removed experimental packages and dependencies.

Why make this change?

To ensure better account management and state consistency within the SIWE authentication framework, and to remove unused or experimental configurations for better stability and performance.



PR-Codex overview

This PR enhances SIWE auth handling in connected wallets.

Detailed summary

  • Updated useSiweAuth to include activeAccount
  • Modified auth functions to use address parameter for verification
  • Improved logic for checking authentication status

The following files were skipped due to too many changes: pnpm-lock.yaml

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

vercel bot commented Aug 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 7:16am
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 7:16am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 7:16am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 7:16am

Copy link

changeset-bot bot commented Aug 6, 2024

🦋 Changeset detected

Latest commit: 2e3c5e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
thirdweb Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

graphite-app bot commented Aug 6, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added Playground Changes involving the Playground codebase. TS SDK Involves changes to the v5 TypeScript SDK. labels Aug 6, 2024
@jnsdls jnsdls marked this pull request as ready for review August 6, 2024 06:31
Copy link
Member Author

jnsdls commented Aug 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jnsdls and the rest of your teammates on Graphite Graphite

Copy link
Contributor

github-actions bot commented Aug 6, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 43.16 KB (0%) 864 ms (0%) 629 ms (+9.66% 🔺) 1.5 s
thirdweb (cjs) 91.35 KB (0%) 1.9 s (0%) 1.1 s (-7.17% 🔽) 2.9 s
thirdweb (minimal + tree-shaking) 4.81 KB (0%) 97 ms (0%) 103 ms (+186.41% 🔺) 199 ms
thirdweb/chains (tree-shaking) 492 B (0%) 10 ms (0%) 31 ms (+338.1% 🔺) 41 ms
thirdweb/react (minimal + tree-shaking) 13.67 KB (0%) 274 ms (0%) 89 ms (+69.57% 🔺) 363 ms

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.05%. Comparing base (206b825) to head (2e3c5e9).

Files Patch % Lines
.../react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx 0.00% 2 Missing ⚠️
...web/ui/ConnectWallet/Modal/ConnectModalContent.tsx 33.33% 2 Missing ⚠️
...t/web/ui/ConnectWallet/screens/SignatureScreen.tsx 33.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3963   +/-   ##
=======================================
  Coverage   60.05%   60.05%           
=======================================
  Files         962      962           
  Lines       77543    77545    +2     
  Branches     3623     3624    +1     
=======================================
+ Hits        46569    46571    +2     
  Misses      30293    30293           
  Partials      681      681           
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from 206b825
packages 59.13% <45.45%> (+<0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
.../thirdweb/src/react/core/hooks/auth/useSiweAuth.ts 63.88% <100.00%> (+0.44%) ⬆️
...b/src/react/web/ui/ConnectWallet/ConnectButton.tsx 73.71% <100.00%> (ø)
.../react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx 48.39% <0.00%> (ø)
...web/ui/ConnectWallet/Modal/ConnectModalContent.tsx 13.60% <33.33%> (+0.31%) ⬆️
...t/web/ui/ConnectWallet/screens/SignatureScreen.tsx 16.01% <33.33%> (+0.20%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Contributor

graphite-app bot commented Aug 6, 2024

Merge activity

…3963)

FIXES: #3912

### TL;DR
This PR fixes the handling of account switching within a connected wallet in Sign-In with Ethereum (SIWE) authentication states.

### What changed?
- Updated the `isLoggedIn` function to accept an `address` argument and added more checks to validate the JWT and the associated address.
- Pass `account` as a new parameter to `useSiweAuth` to ensure the correct account is handled.
- Removed experimental server external packages from `next.config.mjs`.
- Removed `@shikijs/twoslash` dependency from `package.json`.

### How to test?
1. Switch accounts within a connected wallet and ensure the SIWE authentication state updates correctly.
2. Verify that the application runs without errors related to the removed experimental packages and dependencies.

### Why make this change?
To ensure better account management and state consistency within the SIWE authentication framework, and to remove unused or experimental configurations for better stability and performance.

---

<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on enhancing account switching in SIWE authentication states.

### Detailed summary
- Updated functions to include `account` parameter for SIWE authentication
- Changed login and authentication logic
- Added `getAuthResult` function for server-side auth
- Updated dependencies and imports

> The following files were skipped due to too many changes: `pnpm-lock.yaml`

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
@jnsdls jnsdls force-pushed the fix/authentication-account-switch-handling branch from 2982b2c to 2e3c5e9 Compare August 6, 2024 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Playground Changes involving the Playground codebase. TS SDK Involves changes to the v5 TypeScript SDK.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[React] [Thirdweb v5] Account Switch Listener and useSiweAuth Hook Causing Broken State
3 participants