Conversation
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces a complete wallet-based authentication system comprising: a context provider managing authentication state and token persistence; a modal component for wallet signature verification; protected content wrapper and utility hook; API methods for sign-in; updated layout integration; enhanced disconnect handling; example component; and comprehensive documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App (Frontend)
participant Wallet as Wallet (StarkNet)
participant API as Backend API
participant Storage as localStorage
User->>Wallet: Connect Wallet
Wallet->>App: Connection Success Event
App->>App: Set isAuthModalOpen = true
App->>User: Display WalletSignInModal
User->>App: Click "Sign Message"
App->>App: Construct TypedData Message
App->>Wallet: account.signMessage(typedData)
Wallet->>User: Prompt for Signature
User->>Wallet: Sign Message
Wallet->>App: Return Signature
App->>App: Validate & Convert Signature Format
App->>API: POST /api/v1/auth/signin { public_key, signatures }
API->>API: Derive Address from Public Key
API->>API: Validate Signature
API->>API: Create/Find Member
API->>API: Issue JWT Tokens
API->>App: Return { token, refresh_token, member }
App->>Storage: Save token, refresh_token, member_address
App->>App: apiClient.setAuthToken(token)
App->>App: Set isAuthenticated = true
App->>User: Show Success State
App->>App: Close Modal
Note over App: On Subsequent Requests
App->>Storage: Retrieve & Restore Tokens
App->>App: setAuthToken from localStorage
App->>API: API Calls with Auth Header
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/shared/WalletConnected.tsx (1)
7-20: Resolve unused disconnect hook (lint failure)After switching to the auth-context
logout()path we still importuseDisconnectand destructuredisconnect, so the lint job fails withno-unused-vars. Drop the hook and its destructuring; you can also remove theasynckeyword fromhandleDisconnectbecause nothing is awaited anymore.-import { useAccount, useDisconnect } from '@starknet-react/core' +import { useAccount } from '@starknet-react/core' … - const { disconnect } = useDisconnect()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/app/components/modals/WalletSignInModal.tsx(1 hunks)frontend/app/context/wallet-auth-context.tsx(1 hunks)frontend/app/layout.tsx(2 hunks)frontend/components/examples/AuthenticationExample.tsx(1 hunks)frontend/components/shared/ProtectedContent.tsx(1 hunks)frontend/components/shared/WalletConnected.tsx(3 hunks)frontend/lib/api/spherre-api.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
frontend/components/shared/WalletConnected.tsx (1)
frontend/app/context/wallet-auth-context.tsx (1)
useWalletAuth(105-111)
frontend/components/examples/AuthenticationExample.tsx (3)
frontend/app/context/wallet-auth-context.tsx (1)
useWalletAuth(105-111)frontend/app/context/theme-context-provider.tsx (1)
useTheme(144-150)frontend/components/shared/ProtectedContent.tsx (1)
useRequireAuth(67-82)
frontend/app/layout.tsx (2)
frontend/app/context/wallet-auth-context.tsx (1)
WalletAuthProvider(29-103)frontend/context/OnboardingContext.tsx (1)
OnboardingProvider(22-57)
frontend/app/components/modals/WalletSignInModal.tsx (3)
frontend/app/context/wallet-auth-context.tsx (1)
useWalletAuth(105-111)frontend/lib/api/spherre-api.ts (1)
SpherreApi(52-177)frontend/lib/api/client.ts (1)
apiClient(181-181)
frontend/components/shared/ProtectedContent.tsx (1)
frontend/app/context/wallet-auth-context.tsx (1)
useWalletAuth(105-111)
frontend/app/context/wallet-auth-context.tsx (1)
frontend/lib/api/client.ts (1)
apiClient(181-181)
frontend/lib/api/spherre-api.ts (1)
frontend/lib/api/client.ts (2)
ApiUtils(184-241)apiClient(181-181)
🪛 GitHub Actions: Frontend Lint and Code Formatting Check
frontend/components/shared/WalletConnected.tsx
[error] 16-16: ESLint: 'disconnect' is assigned a value but never used. (no-unused-vars)
frontend/components/examples/AuthenticationExample.tsx
[error] 231-231: React no-unescaped-entities: ''' can be escaped with ' in JSX.
frontend/app/components/modals/WalletSignInModal.tsx
[error] 34-34: ESLint/TypeScript: Use "@ts-expect-error" instead of "@ts-ignore".
[error] 100-100: ESLint/TypeScript: Unexpected any. Specify a different type.
| const handleClose = () => { | ||
| if (!isLoading) { | ||
| setIsAuthModalOpen(false) | ||
| setError(null) | ||
| } |
There was a problem hiding this comment.
Reset success when closing or retrying.
After one successful sign-in, success stays true, the modal re-opens showing the success banner, and the Sign button remains disabled (disabled={isLoading || success}), so users cannot re-authenticate after a disconnect. Clear success in handleClose and before starting a new signing attempt.
const handleClose = () => {
if (!isLoading) {
setIsAuthModalOpen(false)
setError(null)
+ setSuccess(false)
}
}
const handleSignIn = async () => {
if (!account || !address) {
setError('Wallet not connected')
return
}
+ setSuccess(false)
setIsLoading(true)Also applies to: 29-31, 93-101, 200-201
🤖 Prompt for AI Agents
In frontend/app/components/modals/WalletSignInModal.tsx around lines 16-20,
29-31, 93-101 and 200-201, the boolean state success is never cleared when
closing the modal or starting a new signing attempt, leaving the success banner
visible and the Sign button permanently disabled; update the close handler(s) to
call setSuccess(false) (e.g., alongside setIsAuthModalOpen(false) and
setError(null)), and clear success right before any new sign/retry flow begins
(add setSuccess(false) at the start of the sign/submit/retry handlers) so the
modal resets correctly for a fresh authentication attempt.
| // Get the public key from the account | ||
| // @ts-ignore - Accessing signer from account | ||
| const publicKey = await account.signer.getPubKey() | ||
|
|
There was a problem hiding this comment.
Remove the @ts-ignore and type the signer access.
@ts-ignore silences real type gaps and violates the lint rule the pipeline flagged. Guard the account value or widen its type so you only call getPubKey when the signer exists, rather than suppressing the error.
- // @ts-ignore - Accessing signer from account
- const publicKey = await account.signer.getPubKey()
+ if (!('signer' in account) || typeof account.signer?.getPubKey !== 'function') {
+ throw new Error('Connected account does not expose a signer')
+ }
+ const publicKey = await account.signer.getPubKey()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Get the public key from the account | |
| // @ts-ignore - Accessing signer from account | |
| const publicKey = await account.signer.getPubKey() | |
| // Get the public key from the account | |
| if (!('signer' in account) || typeof account.signer?.getPubKey !== 'function') { | |
| throw new Error('Connected account does not expose a signer') | |
| } | |
| const publicKey = await account.signer.getPubKey() | |
🧰 Tools
🪛 GitHub Actions: Frontend Lint and Code Formatting Check
[error] 34-34: ESLint/TypeScript: Use "@ts-expect-error" instead of "@ts-ignore".
🤖 Prompt for AI Agents
In frontend/app/components/modals/WalletSignInModal.tsx around lines 33–36,
remove the @ts-ignore and properly type and guard access to account.signer so we
only call getPubKey when a signer exists. Update the account prop/type to
reflect signer?: { getPubKey: () => Promise<string> } (or import the proper
Signer interface), then replace the ignored call with a runtime guard like if
(!account?.signer) { handle missing signer (throw/return) } and only await
account.signer.getPubKey() after the type/guard narrows; this eliminates the
ts-ignore and satisfies the linter/type-checker.
| // Convert signature to numbers for backend | ||
| const signatures = signature.map((sig: string | bigint) => | ||
| typeof sig === 'string' ? parseInt(sig, 16) : Number(sig), | ||
| ) | ||
|
|
||
| // Send to backend for authentication | ||
| const response = await SpherreApi.signIn({ | ||
| signatures, | ||
| public_key: publicKey.toString(), | ||
| }) | ||
|
|
There was a problem hiding this comment.
Stop serializing signatures as JS numbers.
StarkNet signatures are 252-bit values; forcing them through parseInt(..., 16) / Number(...) truncates them to IEEE-754 doubles, so the backend will verify a different signature than the wallet actually signed. Authentication will fail (or worse, succeed with corrupted data). Emit the signatures as exact strings/BigInts and update the request type accordingly so nothing goes through JS number coercion.
- const signatures = signature.map((sig: string | bigint) =>
- typeof sig === 'string' ? parseInt(sig, 16) : Number(sig),
- )
+ const signatures = signature.map((sig: string | bigint) => {
+ const value = typeof sig === 'string' ? BigInt(sig) : sig
+ return value.toString()
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Convert signature to numbers for backend | |
| const signatures = signature.map((sig: string | bigint) => | |
| typeof sig === 'string' ? parseInt(sig, 16) : Number(sig), | |
| ) | |
| // Send to backend for authentication | |
| const response = await SpherreApi.signIn({ | |
| signatures, | |
| public_key: publicKey.toString(), | |
| }) | |
| // Convert signature to numbers for backend | |
| const signatures = signature.map((sig: string | bigint) => { | |
| const value = typeof sig === 'string' ? BigInt(sig) : sig | |
| return value.toString() | |
| }) | |
| // Send to backend for authentication | |
| const response = await SpherreApi.signIn({ | |
| signatures, | |
| public_key: publicKey.toString(), | |
| }) |
| if (typeof window !== 'undefined' && address) { | ||
| const authToken = localStorage.getItem('auth_token') | ||
| const memberAddress = localStorage.getItem('member_address') | ||
|
|
||
| // Check if user has valid token | ||
| const isAuth = authToken && memberAddress | ||
|
|
||
| if (isAuth) { | ||
| // Restore the auth token in API client | ||
| apiClient.setAuthToken(authToken) | ||
| setIsAuthenticated(true) | ||
| setHasCheckedAuth(true) | ||
| } else { | ||
| setIsAuthenticated(false) | ||
| setHasCheckedAuth(true) | ||
|
|
||
| // If wallet is connected but not authenticated, show modal | ||
| // Small delay to ensure smooth transition after wallet connect modal closes | ||
| setTimeout(() => { | ||
| setIsAuthModalOpen(true) | ||
| }, 500) | ||
| } | ||
| } else if (!address) { | ||
| // Reset auth state if wallet disconnects | ||
| setIsAuthenticated(false) |
There was a problem hiding this comment.
Validate stored tokens against the active wallet
The restoration path treats any leftover auth_token + member_address pair as valid without verifying that the stored address matches the wallet that just connected. If someone signs in with wallet A, then switches to wallet B on the same device, B inherits A’s session and skips the signing challenge—critical auth bypass. Compare the stored member address to the connected wallet, and clear the tokens when they don’t match before prompting for re-authentication.
- const memberAddress = localStorage.getItem('member_address')
-
- // Check if user has valid token
- const isAuth = authToken && memberAddress
-
- if (isAuth) {
+ const memberAddress = localStorage.getItem('member_address')
+ const storedMatchesWallet =
+ memberAddress?.toLowerCase() === address.toLowerCase()
+
+ if (authToken && storedMatchesWallet) {
// Restore the auth token in API client
apiClient.setAuthToken(authToken)
setIsAuthenticated(true)
setHasCheckedAuth(true)
} else {
+ if (authToken || memberAddress) {
+ localStorage.removeItem('auth_token')
+ localStorage.removeItem('refresh_token')
+ localStorage.removeItem('member_address')
+ apiClient.clearAuthToken()
+ }
setIsAuthenticated(false)
setHasCheckedAuth(true)🤖 Prompt for AI Agents
In frontend/app/context/wallet-auth-context.tsx around lines 38 to 62, the
current logic treats any stored auth_token + member_address as valid without
ensuring the stored member_address matches the currently connected wallet;
update the flow to compare localStorage.getItem('member_address') with the
current address and if they differ, clear the stored auth_token and
member_address (and call apiClient.clearAuthToken or equivalent), then treat the
user as unauthenticated and open the auth modal after the small delay; if they
match, proceed to restore the token into apiClient and setIsAuthenticated(true)
and setHasCheckedAuth(true).
- Add automatic sign-in modal after wallet connection - Integrate with backend /api/v1/auth/signin endpoint - Implement JWT token management with localStorage - Add wallet signature verification flow - Create ProtectedContent component for sensitive data - Update API client to handle JWT tokens automatically - Fix linter errors in authentication components
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
frontend/app/components/modals/WalletSignInModal.tsx (3)
16-20: Resetsuccesswhen closing or retrying.After successful sign-in,
successremains true, so if the modal reopens (e.g., after disconnect), the success banner still shows and the Sign button stays disabled (disabled={isLoading || success}on line 202), preventing re-authentication. ClearsuccessinhandleCloseand at the start ofhandleSignIn.Apply this diff:
const handleClose = () => { if (!isLoading) { setIsAuthModalOpen(false) setError(null) + setSuccess(false) } } const handleSignIn = async () => { if (!account || !address) { setError('Wallet not connected') return } + setSuccess(false) setIsLoading(true) setError(null)
32-39: Add runtime guard instead of suppressing the type error.
@ts-expect-errorstill silences the type gap. Add a runtime check that verifiesaccount.signerexists before callinggetPubKey(), then throw a clear error if the signer is unavailable.Apply this diff:
try { // Get the public key from the account - // @ts-expect-error - Accessing signer from account + if (!('signer' in account) || typeof account.signer?.getPubKey !== 'function') { + throw new Error('Connected account does not expose a signer') + } const publicKey = await account.signer.getPubKey() if (!publicKey) { throw new Error('Could not retrieve public key from wallet') }
69-72: Stop converting signatures to JS numbers—they will be truncated.StarkNet signatures are 252-bit values. Converting them with
parseInt(sig, 16)orNumber(sig)truncates them to 53-bit IEEE-754 doubles, corrupting the signature data. The backend will receive incorrect signatures, causing authentication failures or security vulnerabilities.Keep signatures as strings or convert to BigInt, then serialize for the backend:
- // Convert signature to numbers for backend - const signatures = signature.map((sig: string | bigint) => - typeof sig === 'string' ? parseInt(sig, 16) : Number(sig), - ) + // Convert signature to string representation for backend + const signatures = signature.map((sig: string | bigint) => { + const value = typeof sig === 'string' ? BigInt(sig) : sig + return value.toString() + })Important: Verify that the backend accepts string-formatted signatures. If it requires exact integer arrays, you may need to adjust the backend serializer or use a different transport format that preserves large integers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/WALLET_AUTH_BACKEND_INTEGRATION.md(1 hunks)frontend/WALLET_AUTH_GUIDE.md(1 hunks)frontend/WALLET_AUTH_IMPLEMENTATION.md(1 hunks)frontend/app/components/modals/WalletSignInModal.tsx(1 hunks)frontend/components/shared/WalletConnected.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/shared/WalletConnected.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/components/modals/WalletSignInModal.tsx (3)
frontend/app/context/wallet-auth-context.tsx (1)
useWalletAuth(105-111)frontend/lib/api/spherre-api.ts (1)
SpherreApi(52-177)frontend/lib/api/client.ts (1)
apiClient(181-181)
🪛 Gitleaks (8.28.0)
frontend/WALLET_AUTH_BACKEND_INTEGRATION.md
[high] 128-128: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 129-129: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
frontend/WALLET_AUTH_GUIDE.md
[grammar] ~169-~169: Ensure spelling is correct
Context: ...yped data structure with: - Domain: Spherre app identification - Message: Sign-i...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
frontend/WALLET_AUTH_BACKEND_INTEGRATION.md
[grammar] ~168-~168: Use a hyphen to join words.
Context: ...ecord - Generates JWT access token (1 hour expiry) - Generates JWT refresh ...
(QB_NEW_EN_HYPHEN)
[grammar] ~432-~432: Use a hyphen to join words.
Context: ...g Checklist - [ ] Connect wallet → Sign in prompt appears - [ ] Sign message → B...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (4)
frontend/WALLET_AUTH_BACKEND_INTEGRATION.md (1)
1-488: LGTM! Comprehensive backend integration documentation.The documentation clearly describes the end-to-end authentication flow, backend validation process, and frontend integration points. The static analysis warnings about generic API keys on lines 128-129 are false positives (example JWT tokens in documentation).
frontend/app/components/modals/WalletSignInModal.tsx (2)
100-112: LGTM! Error handling properly narrows the error type.The catch block now correctly uses
err instanceof Errorto narrow the type before accessingerr.message, addressing the previous type safety concern.
118-230: LGTM! Well-structured UI with proper accessibility.The modal implementation includes proper backdrop handling, loading states, error/success feedback, and accessibility attributes. The theme-aware styling integrates well with the app's design system.
frontend/WALLET_AUTH_IMPLEMENTATION.md (1)
1-331: LGTM! Comprehensive implementation summary.The document provides a clear overview of all implemented features, files changed, testing checklists, and usage examples. Well-structured and helpful for onboarding developers to the new authentication system.
| - [ ] Backend signature verification | ||
| - [ ] JWT token generation for API authentication | ||
| - [ ] Remember device option (localStorage with expiry) | ||
| - [ ] Biometric authentication fallback | ||
| - [ ] Multi-signature support for shared accounts | ||
| - [ ] Rate limiting for signature requests |
There was a problem hiding this comment.
Clarify backend verification status.
Line 236 lists "Backend signature verification" and "JWT token generation" as future enhancements, but WALLET_AUTH_BACKEND_INTEGRATION.md documents that the backend already validates signatures via /api/v1/auth/signin and issues JWT tokens. Update this section to reflect the current implementation state or move fully-implemented features to a separate "Implemented Features" section.
🤖 Prompt for AI Agents
In frontend/WALLET_AUTH_GUIDE.md around lines 236–241, the checklist incorrectly
marks "Backend signature verification" and "JWT token generation for API
authentication" as future enhancements though WALLET_AUTH_BACKEND_INTEGRATION.md
shows the backend already validates signatures at /api/v1/auth/signin and issues
JWTs; update this section by moving those two items into a new "Implemented
Features" or "Completed" section (or change their checklist state to checked),
add a brief note and link referencing WALLET_AUTH_BACKEND_INTEGRATION.md and
remove them from the future/enhancements list so the guide accurately reflects
the current implementation.
| ## 🚨 Important Notes | ||
|
|
||
| 1. **Session Storage**: Authentication clears when browser tab closes (by design for security) | ||
| 2. **No Backend Verification**: Currently client-side only. For production, consider backend verification |
There was a problem hiding this comment.
Update backend verification note.
Line 282 states "No Backend Verification: Currently client-side only," but WALLET_AUTH_BACKEND_INTEGRATION.md documents full backend integration with signature verification and JWT token issuance at /api/v1/auth/signin. Update this note to reflect the current implementation or clarify what additional backend verification is being referred to.
🤖 Prompt for AI Agents
In frontend/WALLET_AUTH_IMPLEMENTATION.md around line 282, the note saying "No
Backend Verification: Currently client-side only" is outdated or misleading
because WALLET_AUTH_BACKEND_INTEGRATION.md documents a full backend flow
(signature verification and JWT issuance at /api/v1/auth/signin); update the
line to either state that backend verification is implemented and point readers
to WALLET_AUTH_BACKEND_INTEGRATION.md for details, or clarify what additional
backend verification (if any) is still required in production, including
endpoint and behavior differences.
2 participants
🧩 Pull Request Draft:
feat(auth): auto wallet sign-in popup on connectTitle: feat(auth): automatic sign-in popup after wallet connection
Description:
Add an automatic popup that appears immediately after a wallet is successfully connected. The popup prompts the user to complete a sign-in with wallet verification (e.g., message signing). On successful verification the application marks the session as authenticated and enables access to protected user areas (settings, backend/smart-contract updates, private data). The popup will only fire post-connection and will not replace the wallet connection flow.
What changed
Verification / Testing Steps
Notes for Reviewers
Related issue: closes #188
Summary by CodeRabbit
New Features
Documentation