Investigate session token establishment and user table verification #58
Conversation
Co-authored-by: chris.corsi <chris.corsi@proofgeist.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Documentation docs/redis-session-cache.md |
New documentation describing Redis session cache architecture, setup steps for Vercel KV and self-hosted Redis, cache behavior (keys, TTL, invalidation), performance impacts, and monitoring guidance. |
Dependencies package.json |
Added @vercel/kv (^3.0.0) as a runtime dependency. |
Configuration src/config/env.ts |
Added three optional environment variables: KV_URL, KV_REST_API_URL, and KV_REST_API_TOKEN for Redis/KV configuration. |
Redis Cache Utilities src/server/auth/utils/redis-cache.ts |
New module providing Redis cache functions: getCachedSession(), setCachedSession(), invalidateCachedSession(), invalidateUserCachedSessions(), and calculateCacheTTL(), with graceful error handling and availability checks. |
Session Integration src/server/auth/utils/session.ts |
Modified to integrate Redis cache checks into session validation and invalidation flows. validateSessionToken now checks Redis first before querying FileMaker. invalidateSession and invalidateUserSessions now clear cache entries. |
Sequence Diagram
sequenceDiagram
participant Client
participant App
participant Redis
participant FileMaker
rect rgb(240, 248, 255)
note over App: Cache Hit Path
Client->>App: Request with session token
App->>Redis: Check cache
Redis-->>App: Valid session found
App-->>Client: Return session (fast)
end
rect rgb(255, 245, 238)
note over App: Cache Miss Path
Client->>App: Request with session token
App->>Redis: Check cache
Redis-->>App: Not found or expired
App->>FileMaker: Query session
FileMaker-->>App: Session data
App->>Redis: Store in cache (async)
App-->>Client: Return session
end
rect rgb(245, 255, 250)
note over App: Invalidation
Client->>App: Logout
App->>Redis: Delete cache entry
App->>FileMaker: Delete session record
App-->>Client: Success
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20–30 minutes
- Key areas requiring attention:
src/server/auth/utils/redis-cache.ts: Verify TTL calculation logic, error handling patterns, and Redis availability checks for edge casessrc/server/auth/utils/session.ts: Trace the validateSessionToken flow to confirm cache consistency (hit/miss/expiry branches) and ensure race conditions between cache and FileMaker are handled correctly- Cache invalidation logic across
invalidateSessionandinvalidateUserSessionsto confirm all invalidation triggers are covered - Environment variable optionality and fallback behavior when Redis is unavailable
Poem
🐰 A cache was born from Redis dreams,
Where sessions fly in faster streams,
FileMaker still the trusted keeper,
While Redis makes the lookups cheaper!
With graceful falls when cache goes dim, 🌟
The session flow stays light and trim.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The title references 'session token establishment and user table verification,' but the changeset actually implements a Redis-based session cache layer to reduce FileMaker queries and improve authentication performance, which is unrelated to the title's stated focus. | Revise the title to accurately reflect the main change, such as 'Add Redis-based session cache to reduce FileMaker queries' or 'Implement session caching layer for improved authentication performance.' |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
cursor/investigate-session-token-establishment-and-user-table-verification-a947
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/server/auth/utils/session.ts (2)
211-216: Consider parallelizing cache invalidation for better performance.The current implementation invalidates cached sessions sequentially in a loop. For users with many sessions, this could be slow.
Apply this diff to parallelize the cache invalidation:
// Invalidate cache and delete from FileMaker for each session + const invalidationPromises = []; for (const session of sessions) { const sessionId = session.fieldData.id; - await invalidateCachedSession(sessionId); - await sessionsLayout.delete({ recordId: session.recordId }); + invalidationPromises.push( + invalidateCachedSession(sessionId).then(() => + sessionsLayout.delete({ recordId: session.recordId }) + ) + ); } + await Promise.all(invalidationPromises); }
70-73: Consider the order of cache invalidation.The cache is invalidated before checking if the session exists in FileMaker. If the session doesn't exist (line 77-79), the cache invalidation was unnecessary. While this doesn't cause correctness issues, you might consider checking existence first to avoid redundant cache operations.
src/server/auth/utils/redis-cache.ts (1)
99-110: Document the no-op behavior more clearly.The function
invalidateUserCachedSessionsis intentionally a no-op, which means bulk invalidation is not supported. This is clearly documented in the comments, but consider adding a console.warn to alert developers if this function is called, or simply remove it until the functionality is implemented.Consider one of these approaches:
export async function invalidateUserCachedSessions( userId: string ): Promise<void> { if (!isRedisAvailable()) { return; } // Warn developers that bulk invalidation is not yet supported console.warn( `invalidateUserCachedSessions called for user ${userId}, but bulk invalidation is not yet implemented. Use individual session invalidation instead.` ); }Or simply remove the function until it's implemented, since
invalidateUserSessionsinsession.tsalready handles individual invalidation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
docs/redis-session-cache.md(1 hunks)package.json(1 hunks)src/config/env.ts(1 hunks)src/server/auth/utils/redis-cache.ts(1 hunks)src/server/auth/utils/session.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{package.json,pnpm-lock.yaml,.npmrc}
📄 CodeRabbit inference engine (.cursor/rules/package-manager.mdc)
{package.json,pnpm-lock.yaml,.npmrc}: Always use pnpm instead of npm or yarn for installing dependencies, running scripts, and managing packages
Use 'pnpm install' to install dependencies instead of 'npm install' or 'yarn install'
Use 'pnpm run ' to run scripts instead of 'npm run ' or 'yarn '
Use 'pnpm add ' to add dependencies instead of 'npm install ' or 'yarn add '
Files:
package.json
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/filemaker-api.mdc)
src/**/*.{ts,tsx}: FileMaker is the ONLY data source for this application - no SQL or other databases should be used. All data operations must go through FileMaker to maintain data integrity and business logic.
ALL data operations MUST use FileMaker Data API. No direct database connections outside FileMaker, and no use of SQL, NoSQL, local storage, or direct file system storage for persistent data.
Use @proofgeist/fmdapi package version ^4.2.2 for all FileMaker Data API interactions.
Type generation for FileMaker schemas must be performed via the{package-manager} typegencommand after any schema changes or when troubleshooting data issues.
Validate all input and output data against Zod schemas for type safety and runtime validation.
Use layout.create({ fieldData: {...} }) for creating records, validate input with Zod, and handle duplicates via FileMaker business logic.
Use layout.get({ recordId }) for fetching a single record, layout.find({ query, limit, offset, sort }) for multiple records, and layout.maybeFindFirst({ query }) for optional single records.
Use layout.update({ recordId, fieldData }) for updating records, following FileMaker field naming and validation rules.
Use layout.delete({ recordId }) for deleting records, respecting FileMaker deletion rules and handling cascading deletes via FileMaker.
Support pagination (limit and offset), sorting by multiple fields, and complex query criteria with operators in FileMaker queries.
Centralized error handling for FileMaker Data API errors, including Zod validation errors.
Type definitions must be generated from the FileMaker schema and used in server actions and components.
Files:
src/server/auth/utils/redis-cache.tssrc/server/auth/utils/session.tssrc/config/env.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/mantine-components.mdc)
**/*.{js,jsx,ts,tsx}: Always prefer using Mantine components over custom HTML or other libraries when developing UI elements.
Prefer using Mantine's built-in styling props over custom CSS classes when possible.
Use Mantine layout components such as Container, Grid, SimpleGrid, Group, Stack, Center, and Box for layout and alignment.
Use Mantine UI components such as Card, Button, TextInput, Select, Modal, Drawer, Tabs, Accordion, and Image for UI elements.
Use Mantine's compound components for more complex UI patterns, such as composing Card with Card.Section and other Mantine components.
Use Mantine's responsive props for adaptive layouts, such as specifying breakpoints in SimpleGrid.
Leverage Mantine's theme system for consistent styling, such as using theme colors and spacing props.
**/*.{js,jsx,ts,tsx}: Always lint and fix errors after completing a feature or making significant changes to existing code before committing your changes
If you need to temporarily bypass a linting rule, use appropriate ESLint comments with justification
Files:
src/server/auth/utils/redis-cache.tssrc/server/auth/utils/session.tssrc/config/env.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/troubleshooting-patterns.mdc)
**/*.{ts,tsx}: All data operations MUST use FileMaker Data API exclusively; do not use SQL queries, direct database connections, local storage for persistent data, or alternative data stores.
Always run{package-manager} typegenfirst for any data-related issues to ensure TypeScript types match the FileMaker schema.
Use @proofgeist/fmdapi for all data operations and follow FileMaker layout and field conventions (use layout.create, layout.find, layout.get, layout.update, layout.delete, layout.maybeFindFirst).
Run{package-manager} tsc --noEmitbefore deployments and after schema changes to catch type errors before runtime.
Do not skip running typegen after FileMaker schema changes.
Do not implement alternative data stores or use local storage for persistent data in place of FileMaker Data API.
Do not add direct database connections; use only the FileMaker Data API via @proofgeist/fmdapi.
Files:
src/server/auth/utils/redis-cache.tssrc/server/auth/utils/session.tssrc/config/env.ts
🧬 Code graph analysis (2)
src/server/auth/utils/redis-cache.ts (1)
src/server/auth/utils/session.ts (1)
SessionValidationResult(256-258)
src/server/auth/utils/session.ts (1)
src/server/auth/utils/redis-cache.ts (4)
invalidateCachedSession(74-88)getCachedSession(32-48)calculateCacheTTL(115-122)setCachedSession(53-69)
🪛 markdownlint-cli2 (0.18.1)
docs/redis-session-cache.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
src/config/env.ts (1)
15-18: LGTM! Environment variables properly configured.The optional KV environment variables are well-designed with appropriate validation (
.url()for URLs) and enable graceful fallback to FileMaker when Redis is unavailable.src/server/auth/utils/redis-cache.ts (3)
17-27: LGTM! Graceful availability check.The
isRedisAvailable()function provides a simple but effective check for Redis availability. While it doesn't test the actual connection, this approach is appropriate for a quick guard clause, with actual connection errors handled gracefully in the try-catch blocks of each cache operation.
39-47: LGTM! Excellent error handling for graceful degradation.All cache operations properly catch errors and log them without throwing, allowing the system to gracefully fall back to FileMaker when Redis is unavailable. This is exactly the right approach for a performance optimization layer.
Also applies to: 62-68, 82-87
115-122: LGTM! Proper TTL calculation with safety cap.The
calculateCacheTTLfunction correctly calculates the TTL based on expiration time and caps it at the maximum to prevent extremely long cache times. The use ofMath.max(0, ...)prevents negative TTLs.docs/redis-session-cache.md (1)
1-124: LGTM! Comprehensive and well-structured documentation.The documentation clearly explains the Redis caching implementation, setup process, cache behavior, performance impact, and troubleshooting steps. It appropriately emphasizes that FileMaker remains the source of truth and that Redis is optional with graceful fallback.
package.json (1)
37-37: No action needed—@vercel/kv@^3.0.0 is the latest stable version with no known security vulnerabilities.The verification confirms that the specified version matches the latest release, and no security advisories exist for this package.
| ``` | ||
| Request → Check Redis Cache → | ||
| ├─ Cache Hit: Return cached session (fast path, ~0.1-1ms) | ||
| └─ Cache Miss: Query FileMaker → Store in Redis → Return session | ||
| ``` |
There was a problem hiding this comment.
Fix markdown linting issue: specify language for code block.
The fenced code block should have a language specified for proper rendering and linting compliance.
Apply this diff:
-```
+```text
Request → Check Redis Cache →
├─ Cache Hit: Return cached session (fast path, ~0.1-1ms)
└─ Cache Miss: Query FileMaker → Store in Redis → Return session
Alternatively, if you want to use a diagram syntax:
```diff
-```
+```mermaid
+flowchart LR
+ Request --> Cache[Check Redis Cache]
+ Cache -->|Hit| Fast[Return cached session<br/>~0.1-1ms]
+ Cache -->|Miss| FM[Query FileMaker]
+ FM --> Store[Store in Redis]
+ Store --> Return[Return session]
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In docs/redis-session-cache.md around lines 9 to 13, the fenced code block lacks
a language identifier which triggers markdown linting errors; update the opening
fence to specify a language (e.g., change totext) so the snippet is
treated as plain text, or replace the block with a mermaid flowchart (use
diagram; ensure you remove the extra plus signs and keep the fence markers
consistent.
| export async function getCachedSession( | ||
| sessionId: string | ||
| ): Promise<SessionValidationResult | null> { | ||
| if (!isRedisAvailable()) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const cacheKey = getCacheKey(sessionId); | ||
| const cached = await kv.get<SessionValidationResult>(cacheKey); | ||
| return cached || null; | ||
| } catch (error) { | ||
| // Log error but don't throw - fallback to FileMaker | ||
| console.error("Redis cache read error:", error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Date deserialization needed (see critical issue in session.ts).
As noted in the review of src/server/auth/utils/session.ts, this function needs to deserialize Date objects after retrieving from Redis. The fix should be applied here in lines 41-42.
🤖 Prompt for AI Agents
In src/server/auth/utils/redis-cache.ts around lines 32 to 48, the function
returns the cached SessionValidationResult as-is which leaves Date fields
serialized as strings; after retrieving `cached` from Redis, detect and convert
any ISO date strings back into Date instances (e.g., fields like expiresAt,
createdAt, issuedAt or other known date properties on SessionValidationResult),
ensuring you handle nested objects if necessary; do this conversion before
returning (and keep the existing null and error fallbacks).
| // Try to get from Redis cache first | ||
| const cachedResult = await getCachedSession(sessionId); | ||
| if (cachedResult !== null) { | ||
| // Validate cached result hasn't expired | ||
| if (cachedResult.session && Date.now() < cachedResult.session.expiresAt.getTime()) { | ||
| return cachedResult; | ||
| } | ||
| // Cache hit but expired - invalidate cache and continue to FileMaker | ||
| if (cachedResult.session) { | ||
| await invalidateCachedSession(sessionId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Date deserialization issue will cause runtime errors.
When SessionValidationResult is retrieved from Redis cache, the expiresAt Date object will be deserialized as a string (due to JSON serialization). Line 98 calls .getTime() on cachedResult.session.expiresAt, which will fail if expiresAt is a string.
You need to handle Date deserialization in src/server/auth/utils/redis-cache.ts. Apply this fix in the getCachedSession function:
export async function getCachedSession(
sessionId: string
): Promise<SessionValidationResult | null> {
if (!isRedisAvailable()) {
return null;
}
try {
const cacheKey = getCacheKey(sessionId);
const cached = await kv.get<SessionValidationResult>(cacheKey);
- return cached || null;
+ if (!cached) return null;
+
+ // Deserialize Date objects
+ if (cached.session) {
+ cached.session.expiresAt = new Date(cached.session.expiresAt);
+ }
+ return cached;
} catch (error) {
// Log error but don't throw - fallback to FileMaker
console.error("Redis cache read error:", error);
return null;
}
}📝 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.
| // Try to get from Redis cache first | |
| const cachedResult = await getCachedSession(sessionId); | |
| if (cachedResult !== null) { | |
| // Validate cached result hasn't expired | |
| if (cachedResult.session && Date.now() < cachedResult.session.expiresAt.getTime()) { | |
| return cachedResult; | |
| } | |
| // Cache hit but expired - invalidate cache and continue to FileMaker | |
| if (cachedResult.session) { | |
| await invalidateCachedSession(sessionId); | |
| } | |
| } | |
| export async function getCachedSession( | |
| sessionId: string | |
| ): Promise<SessionValidationResult | null> { | |
| if (!isRedisAvailable()) { | |
| return null; | |
| } | |
| try { | |
| const cacheKey = getCacheKey(sessionId); | |
| const cached = await kv.get<SessionValidationResult>(cacheKey); | |
| if (!cached) return null; | |
| // Deserialize Date objects | |
| if (cached.session) { | |
| cached.session.expiresAt = new Date(cached.session.expiresAt); | |
| } | |
| return cached; | |
| } catch (error) { | |
| // Log error but don't throw - fallback to FileMaker | |
| console.error("Redis cache read error:", error); | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/server/auth/utils/session.ts around lines 94 to 105, the cache-hit branch
assumes cachedResult.session.expiresAt is a Date but Redis JSON will return a
string; update getCachedSession in src/server/auth/utils/redis-cache.ts to
convert any string timestamp into a Date before returning
SessionValidationResult. Specifically, when reading the cached payload, detect
if payload.session?.expiresAt is a string (or numeric) and replace it with new
Date(payload.session.expiresAt), preserve null/undefined cases, and keep the
rest of the object shape so callers can safely call .getTime() without runtime
errors.
Implement Redis-based session caching to reduce FileMaker database load and improve response times for authenticated requests.
The previous session validation process queried FileMaker for every authenticated request, leading to high latency. This change introduces a Redis cache layer that serves session data much faster, falling back to FileMaker only on cache misses or unavailability, significantly reducing database hits.
Summary by CodeRabbit
Release Notes
New Features
Documentation