feat: Centralizes password caching with utility functions#21
Conversation
Replaces direct localStorage access with dedicated password-cache utilities to improve consistency and maintainability across the codebase. Introduces getCachedPassword, setCachedPassword, removeCachedPassword, and verifyAndCachePassword functions that encapsulate password caching logic. Updates all components and hooks to use these utilities instead of manually managing localStorage. Simplifies password verification flow by automatically caching valid passwords and removing invalid ones. Reduces code duplication and makes password handling more robust throughout the application. Updates documentation to reflect the new approach and provide clear usage examples for developers.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces a centralized password caching and verification module and replaces scattered localStorage password handling across UI components, hooks, and the main page with the new password-cache utilities; also removes the Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (5)app/**/*.tsx📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
app/page.tsx📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
components/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
📚 Learning: 2025-12-03T20:28:35.609ZApplied to files:
🧬 Code graph analysis (1)app/page.tsx (1)
🪛 ast-grep (0.40.0)lib/password-cache.ts[warning] 18-18: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side. (local-storage-sensitive-information) ⏰ 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)
🔇 Additional comments (8)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.tsx (1)
783-813: Inconsistent: inline form bypasses centralized password utilities.This form handler uses direct
fetchandlocalStorage.setIteminstead ofverifyAndCachePassword, defeating the PR's centralization goal. Update the import to includesetCachedPasswordor refactor to useverifyAndCachePassword.+import { + getCachedPassword, + verifyAndCachePassword, + removeCachedPassword, + setCachedPassword, +} from "@/lib/password-cache";Then refactor the form handler:
<form onSubmit={(e) => { e.preventDefault(); const formData = new FormData(e.currentTarget); const password = formData.get("password") as string; if (password && selectedCalendar) { - fetch( - `/api/calendars/${selectedCalendar}/verify-password`, - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ password }), - } - ) - .then((response) => response.json()) - .then((data) => { - if (data.valid) { - localStorage.setItem( - `calendar_password_${selectedCalendar}`, - password - ); - setIsCalendarUnlocked(true); - } else { - toast.error(t("password.errorIncorrect")); - } - }) - .catch(() => { - toast.error(t("password.errorIncorrect")); - }); + verifyAndCachePassword(selectedCalendar, password) + .then((result) => { + if (result.valid) { + setIsCalendarUnlocked(true); + } else { + toast.error(t("password.errorIncorrect")); + } + }); } }}
🧹 Nitpick comments (3)
components/password-dialog.tsx (1)
15-18: Remove unused importsetCachedPassword.
setCachedPasswordis imported but not used in this file. TheverifyAndCachePasswordfunction already handles caching valid passwords internally, so this import is unnecessary.import { - setCachedPassword, verifyAndCachePassword, } from "@/lib/password-cache";lib/password-cache.ts (2)
48-50: Error response handling may misclassify server failures as protected calendars.When
!response.ok, returning{ valid: false, protected: true }assumes the calendar is password-protected. However, this could be a 500 server error or network issue. Consider checking for specific status codes (e.g., 401/403) before assuming protection status.if (!response.ok) { + // Only treat 401/403 as protected; other errors should propagate differently + if (response.status === 401 || response.status === 403) { + return { valid: false, protected: true }; + } + // For other errors, we can't determine protection status return { valid: false, protected: true }; }
17-20: Acknowledge cleartext password storage trade-off.The static analysis correctly flags this as storing sensitive data in localStorage (CWE-312). Since this is the established pattern in the codebase (per project guidelines), document this intentional trade-off in the module's JSDoc header to inform future maintainers of the security consideration.
/** * Centralized password caching and verification utility * Ensures consistent password handling across all components + * + * Security note: Passwords are stored in localStorage in cleartext for + * client-side caching. This is a convenience trade-off; for higher security + * requirements, consider session-based or encrypted storage alternatives. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/copilot-instructions.md(1 hunks)app/page.tsx(7 hunks)components/manage-password-dialog.tsx(2 hunks)components/password-dialog.tsx(2 hunks)hooks/useCalendars.ts(3 hunks)hooks/useNotes.ts(4 hunks)hooks/usePasswordProtection.ts(3 hunks)hooks/useShifts.ts(3 hunks)lib/password-cache.ts(1 hunks)lib/password-utils.ts(0 hunks)
💤 Files with no reviewable changes (1)
- lib/password-utils.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: UseuseTranslations()hook for all user-facing text; maintain translations inmessages/de.jsonandmessages/en.json
UseformatDateToLocal()utility function to format dates as YYYY-MM-DD for consistent date representation
Only add comments for complex logic or non-obvious behavior; favor self-documenting code with clear variable and function names
Use locale-specific date formatters:delocale for German,enUSfor English when formatting dates for display
All errors in API routes and client components should be logged withconsole.error()for debugging purposes
Files:
components/password-dialog.tsxhooks/useCalendars.tshooks/useShifts.tsapp/page.tsxlib/password-cache.tshooks/usePasswordProtection.tshooks/useNotes.tscomponents/manage-password-dialog.tsx
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.{ts,tsx}: Store calendar colors in hex format (e.g.,#3b82f6) and use 20% opacity for backgrounds by appending20to the hex value (e.g.,${color}20)
UsePRESET_COLORSarray for available color options in preset configuration
Files:
components/password-dialog.tsxcomponents/manage-password-dialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All code, comments, variable names, and messages must be in English
Files:
components/password-dialog.tsxhooks/useCalendars.tshooks/useShifts.tsapp/page.tsxlib/password-cache.tshooks/usePasswordProtection.tshooks/useNotes.tscomponents/manage-password-dialog.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/**/*.tsx: Check localStorage for stored password (calendar_password_${calendarId}) before verifying via/api/calendars/[id]/verify-passwordPOST endpoint on password-protected calendars
UsependingActionstate pattern to store operation details before showing PasswordDialog, and retry operation after authentication
UseuseRouter().replace()to keep URL state synchronized with application state
Use Client-side state management with React hooks; organize custom hooks inhooks/directory
UseuseEffectto fetch data when calendar or date changes; prevent unnecessary API calls with proper dependency arrays
Files:
app/page.tsx
app/page.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
statsRefreshTriggercounter to track state mutations and trigger data refetches in main page component
Files:
app/page.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Check localStorage for stored password (`calendar_password_${calendarId}`) before verifying via `/api/calendars/[id]/verify-password` POST endpoint on password-protected calendars
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/api/**/*.ts : Passwords must be SHA-256 hashed via `lib/password-utils.ts` utility before database storage or comparison
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Use `pendingAction` state pattern to store operation details before showing PasswordDialog, and retry operation after authentication
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Check localStorage for stored password (`calendar_password_${calendarId}`) before verifying via `/api/calendars/[id]/verify-password` POST endpoint on password-protected calendars
Applied to files:
components/password-dialog.tsxhooks/useCalendars.ts.github/copilot-instructions.mdhooks/useShifts.tsapp/page.tsxlib/password-cache.tshooks/usePasswordProtection.tshooks/useNotes.tscomponents/manage-password-dialog.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Use `pendingAction` state pattern to store operation details before showing PasswordDialog, and retry operation after authentication
Applied to files:
components/password-dialog.tsx.github/copilot-instructions.mdhooks/useShifts.tsapp/page.tsxhooks/usePasswordProtection.tshooks/useNotes.tscomponents/manage-password-dialog.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to components/**/*Calendar*.tsx : Use separate mobile calendar dialog with `showMobileCalendarDialog` state for responsive mobile UI experience
Applied to files:
components/password-dialog.tsxapp/page.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to components/**/*Dialog.tsx : Dialog components should accept `open`, `onOpenChange`, `onSubmit`, and optional `onDelete` props; reset local state when `open` changes to false
Applied to files:
components/password-dialog.tsxcomponents/manage-password-dialog.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to components/**/*ShiftDialog*.tsx : Shift dialog has `saveAsPreset` enabled by default for convenience when creating shifts
Applied to files:
components/password-dialog.tsxhooks/useShifts.tsapp/page.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to components/**/*Calendar*.tsx : Calendar left-click should toggle shift with selected preset (delete if exists, create if not); right-click should open note dialog and prevent default context menu
Applied to files:
hooks/useCalendars.tshooks/useShifts.tsapp/page.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Use `useEffect` to fetch data when calendar or date changes; prevent unnecessary API calls with proper dependency arrays
Applied to files:
hooks/useCalendars.tsapp/page.tsxhooks/usePasswordProtection.tshooks/useNotes.ts
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to components/**/*Calendar*.tsx : Add `<StickyNote>` icon indicator to calendar days that have notes
Applied to files:
hooks/useCalendars.tsapp/page.tsxhooks/useNotes.ts
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/api/**/*.ts : Passwords must be SHA-256 hashed via `lib/password-utils.ts` utility before database storage or comparison
Applied to files:
hooks/useShifts.tsapp/page.tsxlib/password-cache.tshooks/usePasswordProtection.ts
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to app/**/*.tsx : Use Client-side state management with React hooks; organize custom hooks in `hooks/` directory
Applied to files:
app/page.tsx
📚 Learning: 2025-12-03T20:28:35.609Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-03T20:28:35.609Z
Learning: Applies to **/*.{ts,tsx} : Use `useTranslations()` hook for all user-facing text; maintain translations in `messages/de.json` and `messages/en.json`
Applied to files:
app/page.tsx
🧬 Code graph analysis (7)
components/password-dialog.tsx (1)
lib/password-cache.ts (1)
verifyAndCachePassword(34-73)
hooks/useCalendars.ts (1)
lib/password-cache.ts (2)
setCachedPassword(17-20)removeCachedPassword(25-28)
hooks/useShifts.ts (1)
lib/password-cache.ts (1)
getCachedPassword(9-12)
app/page.tsx (1)
lib/password-cache.ts (2)
getCachedPassword(9-12)verifyAndCachePassword(34-73)
hooks/usePasswordProtection.ts (1)
lib/password-cache.ts (2)
getCachedPassword(9-12)verifyAndCachePassword(34-73)
hooks/useNotes.ts (1)
lib/password-cache.ts (1)
getCachedPassword(9-12)
components/manage-password-dialog.tsx (1)
lib/password-cache.ts (2)
removeCachedPassword(25-28)setCachedPassword(17-20)
🪛 ast-grep (0.40.0)
lib/password-cache.ts
[warning] 18-18: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(calendar_password_${calendarId}, password)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: build-dev
🔇 Additional comments (19)
components/password-dialog.tsx (1)
47-67: LGTM!The password verification flow is correctly simplified using
verifyAndCachePassword. Error handling covers both API failures (via the utility's internal handling) and network errors (via the catch block with properconsole.errorlogging per guidelines).hooks/useShifts.ts (3)
6-6: LGTM!Import correctly added for the centralized password cache utility.
85-85: LGTM!Correctly migrated from direct localStorage access to
getCachedPassword. The ternary guard handles the case whencalendarIdmight be undefined.
121-121: LGTM!Consistent usage with
updateShift- properly usesgetCachedPasswordwith the same guard pattern.hooks/useCalendars.ts (2)
67-71: LGTM!Good practice to cache the password immediately after calendar creation when a password is provided. This ensures the user doesn't need to re-enter the password right after creating a protected calendar.
109-109: LGTM!Correct cleanup - removing the cached password when the calendar is deleted prevents stale data in localStorage.
hooks/useNotes.ts (3)
38-38: LGTM!The direct call to
getCachedPassword(calendarId)is safe here because line 35 already returns early ifcalendarIdis undefined.
83-83: LGTM!Correct usage with ternary guard since
updateNotedoesn't have an early return for undefinedcalendarId.
122-122: LGTM!Consistent pattern with
updateNote- ternary guard handles potential undefinedcalendarId.components/manage-password-dialog.tsx (1)
116-126: LGTM - Cache handling logic is comprehensive.The three-way conditional correctly handles all scenarios:
- Password removed → clear cache
- New password set → cache new password
- Only lock setting changed → preserve current password
The third case (lines 123-125) is defensive but reasonable - it ensures the cache is refreshed after a successful settings update, even if only
isLockedchanged.hooks/usePasswordProtection.ts (2)
16-20: LGTM!The verification logic is cleanly simplified using the centralized utilities.
verifyAndCachePasswordhandles the API call, cache invalidation on failure, and caching on success - the hook just needs to return the validity.
38-40: LGTM!Simple delegation to
getCachedPasswordmaintains the hook's API while using the centralized utility..github/copilot-instructions.md (1)
69-91: Documentation is accurate and complete.The updated documentation for password-cache utilities is well-structured and the example usage correctly demonstrates the authentication flow. All five utility functions including
hasValidCachedPasswordare properly exported fromlib/password-cache.tsand accurately documented.lib/password-cache.ts (1)
1-87: Well-structured centralization of password caching logic.The module cleanly encapsulates localStorage access with proper SSR guards, consolidates verification logic, and maintains the established key pattern. This improves maintainability over scattered direct localStorage calls.
app/page.tsx (5)
47-51: Good adoption of the centralized password-cache utilities.The imports align with the PR's goal. However,
setCachedPasswordis not imported here but is needed for the inline form handler below.
219-254: Unlock flow correctly uses centralized password utilities.The pattern of checking cached password first, then verifying with the server aligns with the established guidelines and properly manages loading/unlocked states.
261-296: iCloud sync handler correctly uses centralized utilities.The flow properly checks cached password, verifies with server, and falls back to the password dialog using the
pendingActionpattern as per coding guidelines.
494-514: Manual shift creation correctly uses password verification flow.The implementation properly verifies before allowing shift creation and prompts for password when needed.
538-575: Shift toggle handler correctly integrates password verification.The optimistic UI update pattern with proper rollback on error, combined with the centralized password verification, maintains good UX while ensuring security.
There was a problem hiding this comment.
Pull request overview
This PR introduces centralized password caching utilities to improve code consistency and maintainability across the BetterShift application. It creates a new lib/password-cache.ts module that encapsulates all password-related localStorage operations, replacing scattered direct localStorage access throughout the codebase.
Key changes:
- Introduces five utility functions (
getCachedPassword,setCachedPassword,removeCachedPassword,verifyAndCachePassword,hasValidCachedPassword) that handle password caching logic - Migrates all hooks (
useShifts,usePasswordProtection,useNotes,useCalendars) and components (password-dialog,manage-password-dialog) to use the new utilities - Simplifies password verification by automatically caching valid passwords and removing invalid ones within the utility functions
- Removes unused
generateSalt()function fromlib/password-utils.ts - Updates developer documentation with clear usage examples
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/password-cache.ts | New file introducing centralized password caching utilities with server verification |
| lib/password-utils.ts | Removes unused generateSalt() function |
| hooks/useShifts.ts | Migrates from direct localStorage access to getCachedPassword() utility |
| hooks/usePasswordProtection.ts | Simplifies password verification logic using verifyAndCachePassword() utility |
| hooks/useNotes.ts | Replaces localStorage calls with getCachedPassword() utility |
| hooks/useCalendars.ts | Uses setCachedPassword() and removeCachedPassword() for cache management |
| components/password-dialog.tsx | Adopts verifyAndCachePassword() to consolidate verification and caching |
| components/manage-password-dialog.tsx | Refines cache management logic with improved conditional handling |
| app/page.tsx | Migrates multiple password verification flows to use centralized utilities |
| .github/copilot-instructions.md | Documents the new password cache utilities and provides usage examples |
Comments suppressed due to low confidence (1)
components/password-dialog.tsx:18
- Unused import setCachedPassword.
import {
setCachedPassword,
verifyAndCachePassword,
} from "@/lib/password-cache";
…remove cached password on unauthorized access
1 participant
Replaces direct localStorage access with dedicated password-cache utilities to improve consistency and maintainability across the codebase.
Introduces getCachedPassword, setCachedPassword, removeCachedPassword, and verifyAndCachePassword functions that encapsulate password caching logic. Updates all components and hooks to use these utilities instead of manually managing localStorage.
Simplifies password verification flow by automatically caching valid passwords and removing invalid ones. Reduces code duplication and makes password handling more robust throughout the application.
Updates documentation to reflect the new approach and provide clear usage examples for developers.
Summary by CodeRabbit
Refactor
User Experience
Chores
✏️ Tip: You can customize this high-level summary in your review settings.