feat: add wallet authentication and permissions#3
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 32 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis pull request implements a complete wallet authentication system for the accounting app. It adds SIWE-based sign-in, iron-session server-side session management, wagmi wallet integration, permission-gated access based on on-chain shares and database cleric roles, and a toast notification system for user feedback. ChangesWallet Authentication & Authorization
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletConnect as WalletConnect UI
participant GetNonce as /api/auth/nonce
participant SignMessage as Sign via wagmi
participant VerifyRoute as /api/auth/verify
participant Permissions as getWalletPermissions
participant SessionAPI as /api/auth/session
User->>WalletConnect: Click "Connect Wallet"
WalletConnect->>GetNonce: Fetch nonce
GetNonce->>WalletConnect: Return nonce
WalletConnect->>SignMessage: Sign SIWE message
SignMessage->>WalletConnect: Return signature
WalletConnect->>VerifyRoute: POST message + signature
VerifyRoute->>VerifyRoute: Verify SIWE signature
VerifyRoute->>Permissions: Check wallet roles
Permissions->>VerifyRoute: Return canAccess, roles
VerifyRoute->>VerifyRoute: Save to session
VerifyRoute->>WalletConnect: Return authenticated session
WalletConnect->>SessionAPI: Confirm session state
SessionAPI->>WalletConnect: Return current auth state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR adds wallet-based authentication/authorization to the RaidGuild Accounting dashboard using SIWE + cookie-backed sessions, and wires the UI to support connect/verify/logout flows.
Changes:
- Introduces SIWE auth API endpoints (nonce, verify, session, logout) backed by
iron-session, plus server-side permission checks (DAO shares, Hats, DB cleric role). - Adds a
WalletConnectclient component and app-wide client providers (Wagmi + React Query + toast UI feedback). - Updates runtime DB initialization to use node-postgres for localhost DB URLs, and updates docs/env examples for new required secrets/vars.
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/wagmi.ts | Wagmi client config for supported chains/connectors. |
| src/lib/auth/types.ts | Shared types for session/permissions/roles. |
| src/lib/auth/session.ts | Iron-session setup + session serialization helper. |
| src/lib/auth/permissions.ts | Server-side permission evaluation (shares/hat/DB role). |
| src/db/index.ts | Runtime DB driver selection for local vs Neon URLs. |
| src/components/ui/toast.tsx | Toast provider for lightweight UI feedback. |
| src/components/providers.tsx | App-level client providers (Wagmi/React Query/Toast). |
| src/components/auth/wallet-connect.tsx | End-to-end wallet connect + SIWE sign/verify + logout UI. |
| src/app/page.tsx | Public vs member home rendering based on session state. |
| src/app/layout.tsx | Wraps app in Providers to enable client auth/UX infra. |
| src/app/globals.css | Toast enter/exit animations. |
| src/app/api/auth/verify/route.ts | SIWE verification + permission checks + session persistence. |
| src/app/api/auth/session/route.ts | Returns serialized session state. |
| src/app/api/auth/nonce/route.ts | Issues and stores SIWE nonce in session. |
| src/app/api/auth/logout/route.ts | Destroys session on logout. |
| README.md | Documents new auth/env requirements and secret generation. |
| public/raidguild-full-logo.svg | Adds logo used on public home. |
| package.json | Adds wagmi/viem/siwe/iron-session/react-query deps. |
| pnpm-lock.yaml | Lockfile updates for added deps. |
| .env.example | Adds required auth/permission env vars (incl. SESSION_SECRET). |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/app/api/auth/logout/route.ts (1)
11-18: 💤 Low valueError message may be misleading.
The catch block returns
"Session configuration is missing"for any error, but the actual failure could be unrelated to session configuration (e.g., network issues, unexpected exceptions). Consider returning a more generic message like"Logout failed"or implementing error mapping similar togetPublicErrorMessagein the verify route (lines 16-49 ofsrc/app/api/auth/verify/route.ts) to provide more accurate feedback while still avoiding information leakage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/auth/logout/route.ts` around lines 11 - 18, The catch block in the logout route currently always returns the specific string "Session configuration is missing" which can be misleading; update the handler in src/app/api/auth/logout/route.ts so that the catch uses a generic public message (e.g. "Logout failed") or reuse the same error-mapping pattern as verify route's getPublicErrorMessage (from src/app/api/auth/verify/route.ts) to translate the caught error before returning it via NextResponse.json, and keep logging the full error with console.error for diagnostics.src/app/api/auth/nonce/route.ts (1)
13-20: 💤 Low valueError message may be misleading.
The catch block returns
"Session configuration is missing"for any error, but the actual failure could be unrelated to session configuration. Consider using a more generic message like"Nonce generation failed"or implementing error mapping similar to the verify route'sgetPublicErrorMessageapproach.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/auth/nonce/route.ts` around lines 13 - 20, The catch in src/app/api/auth/nonce/route.ts currently returns a misleading fixed message ("Session configuration is missing") for all errors; update the handler in the nonce route's try/catch to return a generic public message like "Nonce generation failed" (or reuse the existing getPublicErrorMessage mapping used by the verify route) and keep the original error logged via console.error or processLogger so internal details are preserved. Locate the catch block in the nonce route handler and replace the hardcoded session message with the generic/publicized message and/or call getPublicErrorMessage(error) before returning the JSON response with status 500.src/app/api/auth/session/route.ts (1)
10-17: 💤 Low valueError message may be misleading.
The catch block returns
"Session configuration is missing"for any error, but the actual failure could be unrelated to session configuration. Consider using a more generic message like"Session lookup failed"or implementing error mapping similar to the verify route'sgetPublicErrorMessageapproach.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/auth/session/route.ts` around lines 10 - 17, The catch block in the session route currently always returns the specific message "Session configuration is missing" which can be misleading; update the error handling in the route (the catch surrounding the session lookup) to return a generic public error (for example "Session lookup failed") or reuse the existing getPublicErrorMessage mapping used by the verify route to map internal errors to safe client messages, and ensure the NextResponse.json call uses that mapped message while preserving logging of the original error via console.error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 59-60: Add documentation for HATS_CONTRACT_ADDRESS alongside
DAO_SHARE_TOKEN_ADDRESS, DAO_SHARE_THRESHOLD, and ANGRY_DWARF_HAT_ID in the
README: state that HATS_CONTRACT_ADDRESS is the Ethereum address of the Hats
protocol contract used for hats-based permission checks, show the expected
format (0x-prefixed hex address), give a short example value, and note which
network(s) to use (matching the deployed Hats contract or local test address);
ensure this aligns with the .env.example requirement and mention that it is
required for hats-based permissions to function.
In `@src/app/page.tsx`:
- Around line 87-88: Home() currently fetches the auth session but discards it,
causing WalletConnect to hydrate with an empty session and flicker; thread the
server-fetched session into WalletConnect by passing a serialized session prop
(for example serialize with JSON.stringify/parse or use a safe serializer) so
WalletConnect receives initial state and skips the extra fetch; update all
places where <WalletConnect /> is rendered (including the other occurrences
noted) to accept and forward that prop and adjust WalletConnect’s
props/signature to use the incoming initialSession instead of assuming
emptySession.
In `@src/components/auth/wallet-connect.tsx`:
- Around line 232-236: The signOut function currently waits for both the server
logout (fetch("/api/auth/logout")) and disconnectAsync to succeed before
clearing UI state, so a failed disconnectAsync can leave stale authenticated UI;
change the flow so the local session is cleared and router.refresh is called
regardless of disconnectAsync failure: perform the server logout first, then
attempt disconnectAsync inside a try/catch (log or ignore errors) and ensure
setSession(emptySession) and router.refresh() run in a finally block or
immediately after the server logout success so a rejected disconnectAsync cannot
block clearing the client state (update function signOut, referencing
disconnectAsync, setSession, and router.refresh).
In `@src/components/ui/toast.tsx`:
- Around line 27-44: Replace the collision-prone Date.now() ID generation in
showToast with a collision-safe ID (e.g., use crypto.randomUUID() or an internal
monotonic counter) so each toast gets a unique key; update the creation site
where setToasts adds { id, message, state: "entering" } and ensure the later
timeout comparisons (the toast.id checks in the leaving and filter callbacks)
use that new unique ID generation method to avoid duplicate keys and incorrect
toast mutations.
In `@src/db/index.ts`:
- Around line 11-18: The isLocalDatabaseUrl function misdetects IPv6 localhost
because URL().hostname returns bracketed addresses like "[::1]"; update the
check in isLocalDatabaseUrl to handle bracketed IPv6 by normalizing hostname
(strip surrounding square brackets if present) or by checking both bracketed and
unbracketed forms before comparing to "::1" (also keep existing checks for
"localhost" and "127.0.0.1" intact) so local IPv6 DSNs are correctly identified.
In `@src/lib/auth/permissions.ts`:
- Line 84: The code currently defaults DAO_SHARE_THRESHOLD to "100"; instead
require an explicit env var and throw when missing: check
process.env.DAO_SHARE_THRESHOLD and if undefined throw an Error (similar to the
DAO_SHARE_TOKEN_ADDRESS handling), then call parseUnits on the provided string
with the existing decimals to set threshold; reference the parseUnits call, the
threshold const, and the DAO_SHARE_THRESHOLD env var so the change is applied
where threshold is created.
---
Nitpick comments:
In `@src/app/api/auth/logout/route.ts`:
- Around line 11-18: The catch block in the logout route currently always
returns the specific string "Session configuration is missing" which can be
misleading; update the handler in src/app/api/auth/logout/route.ts so that the
catch uses a generic public message (e.g. "Logout failed") or reuse the same
error-mapping pattern as verify route's getPublicErrorMessage (from
src/app/api/auth/verify/route.ts) to translate the caught error before returning
it via NextResponse.json, and keep logging the full error with console.error for
diagnostics.
In `@src/app/api/auth/nonce/route.ts`:
- Around line 13-20: The catch in src/app/api/auth/nonce/route.ts currently
returns a misleading fixed message ("Session configuration is missing") for all
errors; update the handler in the nonce route's try/catch to return a generic
public message like "Nonce generation failed" (or reuse the existing
getPublicErrorMessage mapping used by the verify route) and keep the original
error logged via console.error or processLogger so internal details are
preserved. Locate the catch block in the nonce route handler and replace the
hardcoded session message with the generic/publicized message and/or call
getPublicErrorMessage(error) before returning the JSON response with status 500.
In `@src/app/api/auth/session/route.ts`:
- Around line 10-17: The catch block in the session route currently always
returns the specific message "Session configuration is missing" which can be
misleading; update the error handling in the route (the catch surrounding the
session lookup) to return a generic public error (for example "Session lookup
failed") or reuse the existing getPublicErrorMessage mapping used by the verify
route to map internal errors to safe client messages, and ensure the
NextResponse.json call uses that mapped message while preserving logging of the
original error via console.error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06b670be-6828-4ae4-b608-a84fbcb1a350
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/raidguild-full-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (18)
.env.exampleREADME.mdpackage.jsonsrc/app/api/auth/logout/route.tssrc/app/api/auth/nonce/route.tssrc/app/api/auth/session/route.tssrc/app/api/auth/verify/route.tssrc/app/globals.csssrc/app/layout.tsxsrc/app/page.tsxsrc/components/auth/wallet-connect.tsxsrc/components/providers.tsxsrc/components/ui/toast.tsxsrc/db/index.tssrc/lib/auth/permissions.tssrc/lib/auth/session.tssrc/lib/auth/types.tssrc/lib/wagmi.ts
This pull request introduces a full implementation of wallet-based authentication and authorization for the RaidGuild Accounting dashboard, using Sign-In with Ethereum (SIWE), session management, and permission checks. It includes new API endpoints, a
WalletConnectReact component for the UI, and supporting infrastructure for session and permission handling. Additionally, it updates environment variables and documentation to reflect the new authentication requirements, and improves UI feedback with toast animations.Authentication & Session Management
/api/auth/nonce(generates a SIWE nonce),/api/auth/verify(verifies wallet signature and checks permissions),/api/auth/session(returns session state), and/api/auth/logout(destroys session). These endpoints handle all aspects of SIWE-based authentication and session lifecycle. [1] [2] [3] [4]WalletConnectReact component, which manages the full wallet sign-in flow (connect, sign SIWE message, verify, show session state, and sign out), and provides user-friendly error handling and UI feedback.Environment & Documentation Updates
.env.exampleto include new required variables for authentication and permission checks (DAO_SHARE_TOKEN_ADDRESS,DAO_SHARE_THRESHOLD,HATS_CONTRACT_ADDRESS, etc.), and clarified comments for public and server-only configuration. [1] [2]README.mdwith documentation for the new environment variables and authentication requirements, and added instructions for generating required secrets.UI/UX Improvements
globals.cssfor smoother feedback on authentication actions.Dependencies
siwe,wagmi,viem,iron-session, and@tanstack/react-query.Authentication and Session Infrastructure
WalletConnectcomponent for end-to-end wallet connection, authentication, and session management in the UI.Environment and Documentation
.env.examplewith new required variables for DAO shares, Hats, and authentication, and improved comments for clarity. [1] [2]README.mdwith documentation on authentication, environment variables, and secret generation.UI/UX Enhancements
Dependencies
siwe,wagmi,viem,iron-session, and@tanstack/react-query.Summary by CodeRabbit
New Features
Documentation