Conversation
Signed-off-by: Simo <simo@6529.io>
WalkthroughAdds per-address role storage and a server-authoritative synchronization path; introduces exported validateJwtRole and syncWalletRoleWithServer functions; set/remove JWT now manage both global and per-address role localStorage keys and normalize wallet addresses to lowercase. Tests updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Client
participant Auth as auth.utils
participant LS as localStorage
participant Srv as Server
rect rgb(235,245,255)
note right of Auth: setAuthJwt / removeAuthJwt
User->>App: authenticate
App->>Auth: setAuthJwt(jwt, walletAddr, role?)
Auth->>Auth: addr := lowercase(walletAddr)
alt role provided
Auth->>LS: setItem("auth-role", role)
Auth->>LS: setItem("auth-role-"+addr, role)
else no role
Auth->>LS: removeItem("auth-role")
Auth->>LS: removeItem("auth-role-"+addr)
end
App->>Auth: removeAuthJwt()
Auth->>Auth: read stored addr? -> lowercase
Auth->>LS: removeItem("auth-role-"+addr)
Auth->>LS: removeItem("auth-role")
end
rect rgb(240,255,240)
note right of Auth: syncWalletRoleWithServer (server-authoritative)
App->>Srv: fetch wallet role for addr
Srv-->>App: serverRole (string|null)
App->>Auth: syncWalletRoleWithServer(serverRole, walletAddr)
Auth->>Auth: addr := lowercase(walletAddr)
alt serverRole != null
Auth->>LS: setItem("auth-role", serverRole)
Auth->>LS: setItem("auth-role-"+addr, serverRole)
else serverRole == null
Auth->>LS: removeItem("auth-role")
Auth->>LS: removeItem("auth-role-"+addr)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
🧬 Code graph analysis (1)services/auth/auth.utils.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 0
🧹 Nitpick comments (1)
services/auth/auth.utils.ts (1)
72-80: Extract per-address key construction to reduce duplication.The per-address storage key construction
auth-role-${normalizedAddress}is duplicated insetAuthJwt,removeAuthJwt, andsyncWalletRoleWithServer.Consider extracting a helper function:
+const getAddressRoleStorageKey = (address: string): string => { + return `auth-role-${address.toLowerCase()}`; +}; + export const setAuthJwt = ( address: string, jwt: string, refreshToken: string, role?: string ) => { const jwtExpiration = getJwtExpiration(jwt); const now = Math.floor(Date.now() / 1000); const expiresInSeconds = jwtExpiration - now; const expiresInDays = expiresInSeconds / 86400; Cookies.set(WALLET_AUTH_COOKIE, jwt, { ...COOKIE_OPTIONS, expires: expiresInDays, }); safeLocalStorage.setItem(WALLET_ADDRESS_STORAGE_KEY, address); safeLocalStorage.setItem(WALLET_REFRESH_TOKEN_STORAGE_KEY, refreshToken); - const normalizedAddress = address.toLowerCase(); - const addressRoleStorageKey = `auth-role-${normalizedAddress}`; + const addressRoleStorageKey = getAddressRoleStorageKey(address); if (role) { safeLocalStorage.setItem(WALLET_ROLE_STORAGE_KEY, role); safeLocalStorage.setItem(addressRoleStorageKey, role); } else { safeLocalStorage.removeItem(WALLET_ROLE_STORAGE_KEY); safeLocalStorage.removeItem(addressRoleStorageKey); } };Then apply similar changes to
removeAuthJwtandsyncWalletRoleWithServer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/services/auth.utils.test.ts(4 hunks)services/auth/auth.utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript for implementation code
Files:
__tests__/services/auth.utils.test.tsservices/auth/auth.utils.ts
**/{__tests__/**/*.{ts,tsx},*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.{ts,tsx},*.test.tsx}: Place tests in tests directories or alongside components as ComponentName.test.tsx
Mock external dependencies and APIs in tests
Files:
__tests__/services/auth.utils.test.ts
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/services/auth.utils.test.ts
🧬 Code graph analysis (2)
__tests__/services/auth.utils.test.ts (2)
helpers/safeLocalStorage.ts (1)
safeLocalStorage(3-11)services/auth/auth.utils.ts (2)
setAuthJwt(53-81)syncWalletRoleWithServer(124-138)
services/auth/auth.utils.ts (1)
helpers/safeLocalStorage.ts (1)
safeLocalStorage(3-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
__tests__/services/auth.utils.test.ts (7)
11-11: LGTM!The import correctly adds the new function to the test suite.
54-58: LGTM!The test correctly verifies that per-address role storage is created with the normalized (lowercase) address.
60-70: LGTM!The test correctly verifies that both global and per-address role keys are removed when no role is provided, and correctly expects the lowercase normalized address in the storage key despite passing a mixed-case address.
72-82: LGTM!The test correctly verifies the new
syncWalletRoleWithServerfunction, including address normalization to lowercase.
84-92: LGTM!The test correctly verifies role removal when server role is null, with proper address normalization.
147-147: LGTM!The mock return value update enables testing of address normalization in the removal logic.
162-164: LGTM!The test correctly verifies that per-address role storage is removed when clearing auth, with proper address normalization from the mocked "Addr" to "auth-role-addr".
services/auth/auth.utils.ts (3)
122-122: Review comment against coding guidelines.The coding guidelines state "Do not include any comments in the code" for TypeScript files. However, this is a JSDoc comment documenting a public API function.
Please clarify whether JSDoc comments for public APIs are exempt from the "no comments" guideline, or if this comment should be removed.
128-137: LGTM!The implementation correctly normalizes the address and manages both global and per-address role storage keys consistently with
setAuthJwt.
110-118: Per-address role keys are cleaned up in removeAuthJwt
removeAuthJwt is the only code path that removes WALLET_ADDRESS_STORAGE_KEY and it also deletes the matchingauth-role-${address}, so orphaned keys cannot occur.
|
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests