feat: add configurable token storage #1266
Conversation
- Add storage abstraction with file and FalkorDB backends - Support API_TOKEN_FALKORDB_URL for production storage - Default to .data/api_tokens.json for local development - Consolidate token storage logic in lib/token-storage
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR replaces direct FalkorDB usage with a pluggable token storage abstraction (StorageFactory) and adds two storage implementations (file-based and FalkorDB-backed). API token endpoints and utilities were refactored to use the storage layer. Configuration and gitignore were updated for local file storage. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as Token API
participant Factory as StorageFactory
participant Store as Storage (File or FalkorDB)
note right of Store `#dff0d8`: implements ITokenStorage
Client->>API: GET /api/auth/tokens
API->>Factory: getStorage()
Factory->>Store: fetchTokens(options)
Store-->>Factory: TokenData[]
Factory-->>API: storage instance returns tokens
API-->>Client: 200 + mapped token list
Client->>API: POST /api/auth/tokens (create)
API->>Factory: getStorage()
API->>Store: createToken(tokenData)
Store-->>API: success
API-->>Client: 201 Created
Client->>API: DELETE /api/auth/tokens/{id}
API->>Factory: getStorage()
API->>Store: revokeToken(tokenId, revoker)
Store-->>API: boolean (success)
API-->>Client: 200 or 404
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
app/api/auth/tokens/[tokenId]/route.ts (1)
13-39: Storage-backedfetchTokenByIdmapping looks correct; consider extracting a shared mapperThe switch to
StorageFactory.getStorage().fetchTokenByIdand the unix‑seconds → ISO/date/null mapping look consistent with the list endpoint. To avoid duplication and keep future changes in sync, consider extracting a small helper (e.g.mapTokenDataToApiToken(token: TokenData)) shared between this route andapp/api/auth/tokens/route.ts.app/api/auth/tokens/route.ts (2)
24-48: fetchTokens’ use of the storage abstraction preserves existing API shapeDelegating to
storage.fetchTokens({ isAdmin, username, host, port })and then normalizing timestamps (unix seconds → ISO,-1→null) maintains the previous response contract while hiding backend details. This looks correct and matches the[tokenId]handler.If you want to tighten typings, you could:
- type
tokenDataasTokenData[]instead ofany[], and/or- extract a shared mapper helper to avoid duplicating the field list and date math in multiple routes.
197-232: Best-effort storage in POST is reasonable; consider a mode where storage failures fail the requestStoring via
StorageFactory.getStorage().createToken(...)inside atryand only logging on failure matches the comment ("token will still work but can't be managed via UI"). That’s a pragmatic default.If, in some environments, manageability is a hard requirement, consider a config flag or
StorageFactory.isFalkorDBStorage()check to turn storage failures into a 5xx instead of silently proceeding, e.g.:- } catch (storageError) { + } catch (storageError) { // eslint-disable-next-line no-console console.error('Failed to store token:', storageError); - // Continue - token will still work but can't be managed via UI + // Optionally, fail hard in environments where storage must succeed + // return NextResponse.json({ message: "Failed to store token" }, { status: 500 }); }As per coding guidelines for
**/api/**/*, this would give a clearer failure mode where needed.lib/token-storage/StorageFactory.ts (1)
16-47: Singleton selection logic is solid; consider passing the computed file path toFileTokenStorageThe env-driven selection between FalkorDB and file storage is clear and
reset()makes testing easier.You currently compute the default file path here and again inside
FileTokenStorage. To avoid divergence if the logic ever changes, you could pass the computed path into the constructor:- if (process.env.API_TOKEN_FALKORDB_URL) { + if (process.env.API_TOKEN_FALKORDB_URL) { // eslint-disable-next-line no-console console.log('Using FalkorDB storage for API tokens'); this.instance = new FalkorDBTokenStorage(); } else { - // eslint-disable-next-line no-console - console.log( - 'Using file storage for API tokens at:', - this.getDefaultFilePath() - ); - this.instance = new FileTokenStorage(); + const filePath = this.getDefaultFilePath(); + // eslint-disable-next-line no-console + console.log('Using file storage for API tokens at:', filePath); + this.instance = new FileTokenStorage(filePath); }lib/token-storage/ITokenStorage.ts (1)
4-64: Token storage interfaces are well-structured and align with usage
TokenData,TokenFetchOptions, andITokenStoragecover the full lifecycle (create/list/read/revoke/touch/cleanup) and match the fields used in your routes and storage implementations. It’s good thatencrypted_passwordlives only on the storage DTO and isn’t surfaced in API responses.If you find yourself repeating the same subset of fields at the API layer, you could introduce a separate
ApiTokentype and a small mapper, but that’s purely an ergonomics improvement.lib/token-storage/FalkorDBTokenStorage.ts (4)
10-37: Manual Cypher string building works but would benefit from parameterizationUsing
escapeStringto double single quotes mitigates the most common injection vector when interpolating into Cypher, and the fields here all come from server-sideTokenData. Still, as this becomes the central FalkorDB storage layer, it would be more robust (and easier to maintain) to use parameterized queries ifexecutePATQuery/ FalkorDB support them, instead of string concatenation.This is not a blocker, but worth considering for hardening.
39-82: fetchTokens logic matches the file backend; mapping toTokenDatalooks correctThe admin vs non-admin filter (
userFilter) mirrors the file-based implementation (username + host + port +is_active), and the mapping fromresult.dataintoTokenDatacopies all expected fields.If you want to tighten typings, you could define a small
DbTokenRowinterface and castresult.datato that instead ofany, but functionally this looks good.
84-141: revoker user requirement inrevokeTokenmay cause surprising failures
revokeTokenmatches both the token and aUsernode forrevokerUsername. If the revoker has never been created as aUsernode in FalkorDB, the MATCH won’t bind anything and the token will stay active, causing this method to returnfalse.If that’s not intentional, two options:
MERGE (revoker:User { username: ..., user_id: ... })before creatingREVOKED_BY, or- fall back to deactivating the token even if the revoker user node doesn’t exist (e.g., separate
MATCHfor the token and optionalMATCH/MERGEfor the revoker).That would make revocation more robust across different user graph states.
143-175: Confirm thatDELETE tremoves relationships for expired tokens in FalkorDB
cleanupExpiredTokensdeletest:Tokennodes withexpires_at > 0 AND expires_at < nowand returns the deleted count. Given tokens are connected to users and revokers via relationships, it’s worth confirming thatDELETE tin FalkorDB automatically removes attached relationships; if not, you may needDETACH DELETE tor to match and delete relationships explicitly.Functionally the logic is sound; this is just about ensuring compatibility with the underlying graph engine.
lib/token-storage/FileTokenStorage.ts (2)
21-55: File init and read behavior is pragmatic; consider surfacing hard parse errors
ensureFileExists+readTokensgive you a nice “it just works” experience (auto-create file/dir, empty array on read failure). For a dev/local-only backend this is reasonable.If you ever rely on this in a more persistent environment, you might want to distinguish between “file missing” (auto-initialize) and “corrupt JSON” (log + throw) so token data corruption doesn’t silently turn into an empty list.
109-147: File revoke/cleanup semantics are simple; consider optional revoker trackingFor the file backend,
revokeTokenmarksis_active = falseand returnsfalsewhen the token isn’t found, andcleanupExpiredTokensdrops expired entries. That’s sufficient for local/dev storage.If you ever need parity with the FalkorDB backend’s revocation audit (via
REVOKED_BY), you could extend the file format to recordrevoked_at/revoked_byfields before writing, but that’s a future enhancement rather than a requirement.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.env.local.template(1 hunks).gitignore(1 hunks)app/api/auth/tokenUtils.ts(1 hunks)app/api/auth/tokens/[tokenId]/route.ts(3 hunks)app/api/auth/tokens/credentials/route.ts(2 hunks)app/api/auth/tokens/route.ts(4 hunks)lib/token-storage/FalkorDBTokenStorage.ts(1 hunks)lib/token-storage/FileTokenStorage.ts(1 hunks)lib/token-storage/ITokenStorage.ts(1 hunks)lib/token-storage/StorageFactory.ts(1 hunks)lib/token-storage/falkordb-client.ts(2 hunks)lib/token-storage/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
⚙️ CodeRabbit configuration file
Review TypeScript code for type safety, proper error handling, and Next.js best practices.
Files:
app/api/auth/tokens/route.tslib/token-storage/falkordb-client.tslib/token-storage/index.tslib/token-storage/FalkorDBTokenStorage.tslib/token-storage/StorageFactory.tsapp/api/auth/tokens/credentials/route.tsapp/api/auth/tokens/[tokenId]/route.tsapp/api/auth/tokenUtils.tslib/token-storage/FileTokenStorage.tslib/token-storage/ITokenStorage.ts
**/api/**/*
⚙️ CodeRabbit configuration file
Review API routes for security, error handling, and proper HTTP status codes.
Files:
app/api/auth/tokens/route.tsapp/api/auth/tokens/credentials/route.tsapp/api/auth/tokens/[tokenId]/route.tsapp/api/auth/tokenUtils.ts
🧠 Learnings (2)
📚 Learning: 2025-11-13T09:41:15.717Z
Learnt from: Naseem77
Repo: FalkorDB/falkordb-browser PR: 1208
File: .github/workflows/playwright.yml:264-264
Timestamp: 2025-11-13T09:41:15.717Z
Learning: The hardcoded ENCRYPTION_KEY value "a3f5d8c2b7e4f1a0c9d6e8b2f5a7c3d1e9f2a4b8c6d3e7f1a5b9c2d8e4f0a6b3" in .github/workflows/playwright.yml at the run-token-tests job is a test-only encryption key used for CI token tests and is not a production secret.
Applied to files:
.env.local.template
📚 Learning: 2025-11-11T13:42:45.194Z
Learnt from: Naseem77
Repo: FalkorDB/falkordb-browser PR: 1208
File: app/api/auth/[...nextauth]/options.ts:9-10
Timestamp: 2025-11-11T13:42:45.194Z
Learning: In app/api/auth/[...nextauth]/options.ts for the FalkorDB browser application: The jwt() callback stores 'password' field for NextAuth session tokens (cookie-based auth), while Personal Access Tokens (PATs) created by /api/auth/login store 'pwd' field (application-encrypted). These are two separate JWT flows: session tokens are never used with Bearer auth and PATs are never used with session auth. The jwt() callback must keep 'password' field for the session() callback to work correctly.
Applied to files:
.env.local.templateapp/api/auth/tokens/route.tslib/token-storage/FalkorDBTokenStorage.tsapp/api/auth/tokens/credentials/route.tsapp/api/auth/tokens/[tokenId]/route.tsapp/api/auth/tokenUtils.ts
🧬 Code graph analysis (4)
lib/token-storage/StorageFactory.ts (1)
lib/token-storage/ITokenStorage.ts (1)
ITokenStorage(34-64)
app/api/auth/tokens/credentials/route.ts (1)
app/api/auth/encryption.ts (1)
encrypt(25-38)
app/api/auth/tokens/[tokenId]/route.ts (2)
lib/token-storage/FalkorDBTokenStorage.ts (1)
fetchTokenById(84-126)lib/token-storage/FileTokenStorage.ts (1)
fetchTokenById(103-106)
lib/token-storage/FileTokenStorage.ts (1)
lib/token-storage/ITokenStorage.ts (3)
ITokenStorage(34-64)TokenData(4-18)TokenFetchOptions(23-28)
🪛 dotenv-linter (4.0.0)
.env.local.template
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [UnorderedKey] The API_TOKEN_STORAGE_PATH key should go before the CHAT_URL key
(UnorderedKey)
⏰ 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: Setup and cache dependencies
🔇 Additional comments (6)
.gitignore (1)
33-35: LGTM!Good addition to exclude the local token storage directory, preventing accidental commits of sensitive token data during development.
lib/token-storage/falkordb-client.ts (2)
115-117:PAT_FALKORDB_GRAPH_NAMEremoved from template but still referenced.The
.env.local.templateno longer includesPAT_FALKORDB_GRAPH_NAME, but this function still references it. Either add it back to the template or consider adding graph name to the URL query parameters for consistency.
29-36: URL-encoded credentials need decoding.Credentials in URLs may be percent-encoded (e.g.,
%40for@). UsedecodeURIComponent()to properly decode username and password.const parsed = new URL(url); const host = parsed.hostname || "localhost"; const port = parsed.port ? parseInt(parsed.port, 10) : 6380; - const username = parsed.username || "default"; - const password = parsed.password || undefined; + const username = parsed.username ? decodeURIComponent(parsed.username) : "default"; + const password = parsed.password ? decodeURIComponent(parsed.password) : undefined; const tls = parsed.searchParams.get("tls") === "true";⛔ Skipped due to learnings
Learnt from: Naseem77 Repo: FalkorDB/falkordb-browser PR: 1208 File: app/api/auth/[...nextauth]/options.ts:9-10 Timestamp: 2025-11-11T13:42:45.194Z Learning: In app/api/auth/[...nextauth]/options.ts for the FalkorDB browser application: The jwt() callback stores 'password' field for NextAuth session tokens (cookie-based auth), while Personal Access Tokens (PATs) created by /api/auth/login store 'pwd' field (application-encrypted). These are two separate JWT flows: session tokens are never used with Bearer auth and PATs are never used with session auth. The jwt() callback must keep 'password' field for the session() callback to work correctly.app/api/auth/tokens/credentials/route.ts (1)
166-203: LGTM! Clean migration to storage abstraction.Good refactoring from direct Cypher queries to the storage abstraction. The error handling allows token generation to succeed even if storage fails—reasonable resilience for the auth flow, though users should be aware the token won't appear in management UI.
lib/token-storage/index.ts (1)
1-14: Module re‑exports look good and give a clear public surfaceCentralizing
ITokenStorage/TokenDatatypes, both storage backends,StorageFactory, and FalkorDB client helpers behind this index keeps consumers flexible without leaking too much structure. No issues from my side.Suggested PR titles (conventional commits):
feat: add configurable token storage backendsrefactor: abstract API token storage behind ITokenStoragelib/token-storage/FileTokenStorage.ts (1)
83-101: fetchTokens behavior matches FalkorDB storage and keeps semantics consistentFiltering to
is_activetokens, then applying username/host/port filters for non-admins, and sorting bycreated_atdescending all mirror the FalkorDB implementation. That consistency is great for swapping backends without changing API behavior.No changes needed here.
.env.local.template
Outdated
| PAT_FALKORDB_TLS=false | ||
| PAT_FALKORDB_CA= | ||
| PAT_FALKORDB_GRAPH_NAME=token_management | ||
| API_TOKEN_STORAGE_PATH=/data/tokens.json |
There was a problem hiding this comment.
Path mismatch with default and missing trailing newline.
The template specifies /data/tokens.json, but the .gitignore excludes /.data/ and the FileTokenStorage likely defaults to .data/api_tokens.json. Consider aligning these for consistency.
Also, add a trailing newline to the file.
-API_TOKEN_STORAGE_PATH=/data/tokens.json
+API_TOKEN_STORAGE_PATH=.data/api_tokens.json
+📝 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.
| API_TOKEN_STORAGE_PATH=/data/tokens.json | |
| API_TOKEN_STORAGE_PATH=.data/api_tokens.json | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [UnorderedKey] The API_TOKEN_STORAGE_PATH key should go before the CHAT_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.local.template around line 7, the API_TOKEN_STORAGE_PATH value
(/data/tokens.json) mismatches the repository .gitignore (which excludes
/.data/) and the FileTokenStorage default (.data/api_tokens.json); update the
value to .data/api_tokens.json to match the default and project ignore rules,
and ensure the file ends with a trailing newline.
app/api/auth/tokenUtils.ts
Outdated
| import { jwtVerify } from "jose"; | ||
| import crypto from "crypto"; | ||
| import { executePATQuery } from "@/lib/token-storage"; | ||
| import { executePATQuery } from "@/lib/token-storage/falkordb-client"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/api/auth/tokenUtils.tsRepository: FalkorDB/falkordb-browser
Length of output: 6375
🏁 Script executed:
# Search for StorageFactory usage in API routes to understand the abstraction pattern
rg "StorageFactory" app/api --type ts -A 2 -B 2Repository: FalkorDB/falkordb-browser
Length of output: 2920
🏁 Script executed:
# Find other API routes to compare the pattern
fd -e ts app/api | head -20Repository: FalkorDB/falkordb-browser
Length of output: 402
🏁 Script executed:
# Search for usage of isTokenActive and getPasswordFromTokenDB
rg "isTokenActive|getPasswordFromTokenDB" app/api --type ts -B 2 -A 2Repository: FalkorDB/falkordb-browser
Length of output: 1931
🏁 Script executed:
# Check StorageFactory interface to see what methods are available
cat -n lib/token-storage/StorageFactory.tsRepository: FalkorDB/falkordb-browser
Length of output: 2394
🏁 Script executed:
# Check if there's a storage interface definition
fd -type f "Storage.ts\|StorageInterface.ts\|IStorage.ts" lib/Repository: FalkorDB/falkordb-browser
Length of output: 239
🏁 Script executed:
cat -n lib/token-storage/ITokenStorage.tsRepository: FalkorDB/falkordb-browser
Length of output: 1921
🏁 Script executed:
cat -n lib/token-storage/FileTokenStorage.ts | head -100Repository: FalkorDB/falkordb-browser
Length of output: 3786
🏁 Script executed:
cat -n lib/token-storage/FalkorDBTokenStorage.ts | head -150Repository: FalkorDB/falkordb-browser
Length of output: 6344
Add isTokenActive and getPasswordFromTokenDB methods to ITokenStorage interface and refactor utility functions to use the storage abstraction.
The functions isTokenActive and getPasswordFromTokenDB use direct executePATQuery calls, bypassing the storage abstraction layer. Since ITokenStorage lacks these methods, they cannot be implemented in FileTokenStorage—token validation will fail if file-based storage is configured. Refactor these functions to use StorageFactory.getStorage() and add the corresponding methods to the storage interface.
lib/token-storage/falkordb-client.ts
Outdated
| const tls = parsed.searchParams.get("tls") === "true"; | ||
|
|
||
| return { host, port, username, password, tls }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd lib/token-storage && wc -l falkordb-client.tsRepository: FalkorDB/falkordb-browser
Length of output: 91
🏁 Script executed:
cat -n lib/token-storage/falkordb-client.tsRepository: FalkorDB/falkordb-browser
Length of output: 6871
🌐 Web query:
Redis connection URL CA certificate parameter syntax
💡 Result:
Short answer: there is no single Redis-standard URL query key for a CA; client libraries accept TLS/SSL options as URL query parameters (or separate options) — common names are ssl_ca_certs, ssl_certfile, ssl_keyfile and ssl_cert_reqs. Example (redis-py / redis-cli-style clients):
rediss://host:6380/?ssl_ca_certs=%2Fpath%2Fto%2Fca.pem&ssl_certfile=%2Fpath%2Fto%2Fclient.crt&ssl_keyfile=%2Fpath%2Fto%2Fclient.key&ssl_cert_reqs=CERT_REQUIRED
Notes:
- Use the rediss:// scheme to indicate TLS; many libraries reject SSL params when scheme is plain redis://. [1][2]
- Query values must be URL-encoded if they contain special characters or slashes. [1][2]
- Different client libraries may accept slightly different names or require passing an SSLContext instead — check your client docs (examples: redis-py, Lettuce). [1][3]
Sources:
[1] redis-py connection docs (URL/query handling).
[2] celery/redis client code showing ssl_param keys (ssl_ca_certs, ssl_certfile, ssl_keyfile, ssl_cert_reqs).
[3] Lettuce Redis-URI documentation (rediss scheme + TLS handling).
🌐 Web query:
FalkorDB TLS CA certificate connection options
💡 Result:
Summary — FalkorDB TLS / CA options
- FalkorDB supports TLS for client connections; clients/REST accept a tls flag when connecting (e.g., REST example includes "tls": "false" / "true"). [1]
- Client libraries rely on the underlying Redis/TLS implementation, so CA/trust configuration is done in the client TLS options (e.g., the Rust client uses the redis crate's TLS support and accepts rustls or native-tls features and their TlsOptions/ca-file settings). For Rust enable the "rustls" or "native-tls" feature and provide CA/certificate paths via the underlying TLS options. [2][3]
- Practical knobs you’ll use:
- Enable TLS (tls: true / use TLS-enabled connection URI).
- Supply a CA bundle / ca file path to the client TLS options so the client can validate the server certificate (client library-specific API). [2][3]
- If you need mutual TLS, supply client cert + key (client TLS options) and CA trust bundle on the server side. (FalkorDB clients inherit whatever mTLS options the underlying TLS implementation supports.) [2][3]
- Where to configure:
- REST/browser: pass tls in the connection payload (docs example). [1]
- Rust client: enable rustls/native-tls feature and set the TlsOptions/ca file paths via the redis/TLS API. [2][3]
- If you want exact code or the specific option names for Python/JS/Java/Rust clients, tell me which client and I’ll give the precise call/parameters.
Sources:
[1] FalkorDB REST / integration docs (connection example with "tls" flag).
[2] FalkorDB Rust client docs (SSL/TLS support; rustls/native-tls features).
[3] docs.rs / redis crate TLS guidance (underlying TLS options and CA/cert file usage).
CA certificate cannot be specified when using URL format.
The URL parser (lines 34–36) does not extract a ca parameter, while the fallback path (line 50) supports PAT_FALKORDB_CA. When using API_TOKEN_FALKORDB_URL with TLS enabled, custom CA certificates cannot be provided, causing TLS verification failures. Add support for a base64-encoded ca query parameter (e.g., ?ca=base64string) or document this limitation.
🤖 Prompt for AI Agents
In lib/token-storage/falkordb-client.ts around lines 34 to 36, the URL parser
currently extracts host, port, username, password and tls but omits the CA
parameter so clients using API_TOKEN_FALKORDB_URL with TLS cannot supply a
custom CA; update the parser to read a "ca" query parameter (e.g.,
?ca=base64string), base64-decode it to the PEM string (or validate/throw on
invalid base64), and include it on the returned object (e.g., ca: string |
undefined), ensuring callers that use TLS will receive the CA; if base64 is
missing or invalid, return undefined for ca (or surface a clear error) to
preserve existing behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
app/api/auth/tokens/[tokenId]/route.ts (1)
202-213: LGTM: Storage abstraction integrated cleanly.The migration to
StorageFactoryis well-executed. The error handling at line 210 now returns a generic "Failed to revoke token" message as requested in previous reviews, avoiding the misleading "token not found" message whenrevokeTokenreturns false.app/api/auth/tokenUtils.ts (1)
33-70: Storage abstraction integration verified.The refactor to use
storage.isTokenActive(tokenHash)is correct. The methodsisTokenActiveandgetEncryptedPasswordare now properly defined inITokenStorage(lines 60-72), addressing the previous review concern about missing interface methods.
🧹 Nitpick comments (3)
.env.local.template (1)
2-7: Add trailing newline and reorder keys alphabetically.The path was correctly updated to
.data/api_tokens.json, but two minor formatting issues remain:
- Missing trailing newline at end of file
API_TOKEN_STORAGE_PATHshould precedeCHAT_URLper alphabetical ordering conventionsApply this diff:
-CHAT_URL=http://localhost:8080/ -NEXTAUTH_URL=http://localhost:3000/ -NEXTAUTH_SECRET=SECRET -NEXT_PUBLIC_GOOGLE_ANALYTICS=ANALYTICS -PORT=3000 API_TOKEN_STORAGE_PATH=.data/api_tokens.json +CHAT_URL=http://localhost:8080/ +NEXTAUTH_URL=http://localhost:3000/ +NEXTAUTH_SECRET=SECRET +NEXT_PUBLIC_GOOGLE_ANALYTICS=ANALYTICS +PORT=3000 +e2e/tests/settingsTokens.spec.ts (1)
233-234: Consider using waitForSelector or retry logic instead of hardcoded timeouts.The
waitFor(1500)calls improve test stability when waiting for tokens to load from storage, but hardcoded delays can be brittle—especially with the new storage abstraction that may have variable latency between file and FalkorDB backends.Consider replacing with a more resilient pattern:
- // Wait for tokens to load in the UI - await settingsTokensPage.waitFor(1500); + // Wait for tokens to load in the UI + await settingsTokensPage.waitForTokenToAppear(tokenName);Or implement a retry-based verification helper that polls until the token appears or times out.
Also applies to: 269-270
app/api/auth/tokenUtils.ts (1)
58-62: Clean up or track technical debt comment.The comment about throttling and
getTokenByHashdescribes deferred work but lacks an actionable tracking mechanism (no TODO/FIXME tag, no issue reference).Either:
- Remove the comment if throttling is now handled in storage implementations, or
- Add a TODO tag and create a tracking issue:
- // Note: updateLastUsed in storage implementations should handle throttling - // For now, we'll skip the throttling logic here and just update - // This could be improved by adding a getTokenByHash method to get the token_id + // TODO(issue-XXXX): Add throttled updateLastUsed via getTokenByHash in storage implementations
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env.local.template(1 hunks)app/api/auth/tokenUtils.ts(4 hunks)app/api/auth/tokens/[tokenId]/route.ts(3 hunks)e2e/tests/settingsTokens.spec.ts(2 hunks)lib/token-storage/FalkorDBTokenStorage.ts(1 hunks)lib/token-storage/FileTokenStorage.ts(1 hunks)lib/token-storage/ITokenStorage.ts(1 hunks)lib/token-storage/falkordb-client.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/token-storage/FileTokenStorage.ts
- lib/token-storage/falkordb-client.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
⚙️ CodeRabbit configuration file
Review TypeScript code for type safety, proper error handling, and Next.js best practices.
Files:
lib/token-storage/ITokenStorage.tslib/token-storage/FalkorDBTokenStorage.tsapp/api/auth/tokens/[tokenId]/route.tse2e/tests/settingsTokens.spec.tsapp/api/auth/tokenUtils.ts
**/api/**/*
⚙️ CodeRabbit configuration file
Review API routes for security, error handling, and proper HTTP status codes.
Files:
app/api/auth/tokens/[tokenId]/route.tsapp/api/auth/tokenUtils.ts
e2e/**/*
⚙️ CodeRabbit configuration file
Review test code for proper assertions, test coverage, and maintainability.
Files:
e2e/tests/settingsTokens.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-11-11T13:42:45.194Z
Learnt from: Naseem77
Repo: FalkorDB/falkordb-browser PR: 1208
File: app/api/auth/[...nextauth]/options.ts:9-10
Timestamp: 2025-11-11T13:42:45.194Z
Learning: In app/api/auth/[...nextauth]/options.ts for the FalkorDB browser application: The jwt() callback stores 'password' field for NextAuth session tokens (cookie-based auth), while Personal Access Tokens (PATs) created by /api/auth/login store 'pwd' field (application-encrypted). These are two separate JWT flows: session tokens are never used with Bearer auth and PATs are never used with session auth. The jwt() callback must keep 'password' field for the session() callback to work correctly.
Applied to files:
lib/token-storage/FalkorDBTokenStorage.tsapp/api/auth/tokens/[tokenId]/route.ts.env.local.templateapp/api/auth/tokenUtils.ts
📚 Learning: 2025-11-13T09:41:15.717Z
Learnt from: Naseem77
Repo: FalkorDB/falkordb-browser PR: 1208
File: .github/workflows/playwright.yml:264-264
Timestamp: 2025-11-13T09:41:15.717Z
Learning: The hardcoded ENCRYPTION_KEY value "a3f5d8c2b7e4f1a0c9d6e8b2f5a7c3d1e9f2a4b8c6d3e7f1a5b9c2d8e4f0a6b3" in .github/workflows/playwright.yml at the run-token-tests job is a test-only encryption key used for CI token tests and is not a production secret.
Applied to files:
.env.local.template
🧬 Code graph analysis (2)
lib/token-storage/ITokenStorage.ts (1)
lib/token-storage/index.ts (3)
TokenData(2-2)TokenFetchOptions(2-2)ITokenStorage(2-2)
app/api/auth/tokens/[tokenId]/route.ts (2)
lib/token-storage/FalkorDBTokenStorage.ts (1)
fetchTokenById(84-126)lib/token-storage/FileTokenStorage.ts (1)
fetchTokenById(103-106)
🪛 dotenv-linter (4.0.0)
.env.local.template
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [UnorderedKey] The API_TOKEN_STORAGE_PATH key should go before the CHAT_URL key
(UnorderedKey)
⏰ 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: Setup and cache dependencies
- GitHub Check: build
🔇 Additional comments (1)
lib/token-storage/ITokenStorage.ts (1)
1-78: Well-designed storage abstraction interface.The interface is comprehensive and well-documented:
TokenDatacaptures all necessary token fieldsTokenFetchOptionssupports role-based filtering- Method signatures are clear with appropriate async returns
isTokenActiveandgetEncryptedPasswordmethods properly address previous review concerns about storage abstraction completeness
| private escapeString(str: string): string { | ||
| return str.replace(/'/g, "''"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/token-storage/FalkorDBTokenStorage.tsRepository: FalkorDB/falkordb-browser
Length of output: 8603
🏁 Script executed:
find lib -name "falkordb-client*" -type fRepository: FalkorDB/falkordb-browser
Length of output: 105
🏁 Script executed:
cat -n lib/token-storage/falkordb-client.tsRepository: FalkorDB/falkordb-browser
Length of output: 6251
🏁 Script executed:
# Search for any usage examples or tests showing parameterized queries
find . -type f \( -name "*.ts" -o -name "*.js" \) -exec grep -l "executePATQuery" {} \;Repository: FalkorDB/falkordb-browser
Length of output: 180
🏁 Script executed:
# Check package.json to see falkordb version
cat package.json | grep -A 2 -B 2 "falkordb"Repository: FalkorDB/falkordb-browser
Length of output: 310
🏁 Script executed:
# Check if there are any comments or documentation about query parameters in the codebase
rg -i "param|parameterized|injection" lib/token-storage/ --type tsRepository: FalkorDB/falkordb-browser
Length of output: 372
Use parameterized queries instead of manual string escaping.
The escapeString method only escapes single quotes but Cypher strings require comprehensive escaping for backslashes, newlines, and control characters. More importantly, executePATQuery already supports parameterized queries—the function signature accepts params: Record<string, unknown> which is passed directly to graph.query(query, params). All eight query methods (lines 14–209) should use parameterized queries instead of string interpolation:
// Before:
const query = `MATCH (t:Token {token_id: '${this.escapeString(tokenId)}'})...`;
await executePATQuery(query);
// After:
const query = `MATCH (t:Token {token_id: $tokenId})...`;
await executePATQuery(query, { tokenId });This eliminates the escaping burden entirely and is the standard secure approach for query construction.
🤖 Prompt for AI Agents
In lib/token-storage/FalkorDBTokenStorage.ts around lines 10–12 (and affecting
methods from ~14–209), the class uses a brittle escapeString and string
interpolation for Cypher queries; replace all string-interpolated queries with
parameterized queries using $placeholders and pass a params object to
executePATQuery (e.g. $tokenId -> { tokenId }), remove/stop using escapeString,
and ensure every query method supplies the corresponding params argument to
executePATQuery so values are sent as parameters rather than concatenated into
the query string.
| async cleanupExpiredTokens(): Promise<number> { | ||
| const nowUnix = Math.floor(Date.now() / 1000); | ||
|
|
||
| // Delete tokens that have expired (expires_at > 0 AND expires_at < now) | ||
| const query = ` | ||
| MATCH (t:Token) | ||
| WHERE t.expires_at > 0 AND t.expires_at < ${nowUnix} | ||
| DELETE t | ||
| RETURN count(t) as deleted_count | ||
| `; | ||
|
|
||
| const result = await executePATQuery(query); | ||
|
|
||
| if (result.data && result.data.length > 0) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return (result.data[0] as any).deleted_count || 0; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for relationships that reference Token nodes or patterns involving Token
rg -n "Token.*[\[\-]|[\-\]].*Token|\-\[.*Token|Token.*\-\[" lib/ app/ --type ts --type js -A 2 -B 2Repository: FalkorDB/falkordb-browser
Length of output: 9764
🏁 Script executed:
# Search for REVOKED_BY relationships and other Token relationships
rg -n "REVOKED_BY|Token.*NEXT_VERSION|hasToken|TOKEN_FOR" lib/ app/ --type ts --type jsRepository: FalkorDB/falkordb-browser
Length of output: 173
🏁 Script executed:
# Check if is_active property is already used in Token schema
rg -n "is_active" lib/token-storage/ --type tsRepository: FalkorDB/falkordb-browser
Length of output: 1229
🏁 Script executed:
# Look for audit logs or historical tracking of tokens
rg -n "audit|Audit|AuditLog|history|History" lib/token-storage/ app/ --type ts --type jsRepository: FalkorDB/falkordb-browser
Length of output: 14388
🏁 Script executed:
# Search for any queries that might join or reference deleted tokens
rg -n "DELETE.*Token|MATCH.*Token" lib/token-storage/ app/ --type ts --type js -A 3Repository: FalkorDB/falkordb-browser
Length of output: 2634
🏁 Script executed:
# Search for any code that might depend on tokens being deleted (not just is_active=false)
rg -n "cleanupExpiredTokens|DELETE.*Token" lib/ app/ --type ts --type jsRepository: FalkorDB/falkordb-browser
Length of output: 338
🏁 Script executed:
# Check the context around the cleanupExpiredTokens function to understand when it's called
rg -n "cleanupExpiredTokens" lib/ app/ --type ts --type js -B 2 -A 2Repository: FalkorDB/falkordb-browser
Length of output: 1077
🏁 Script executed:
# View FileTokenStorage cleanupExpiredTokens implementation
sed -n '157,171p' lib/token-storage/FileTokenStorage.tsRepository: FalkorDB/falkordb-browser
Length of output: 475
cleanupExpiredTokens should not physically delete tokens—preserve audit trail.
Line 197 uses DELETE t to remove expired tokens, which orphans REVOKED_BY relationships created when tokens are revoked (see revokeToken() at line 135). This destroys the audit trail of who revoked which token and when. Additionally, the cleanup query does not check is_active, so recently revoked tokens can still be deleted if expired.
Update to soft-delete pattern already established elsewhere:
MATCH (t:Token)
- WHERE t.expires_at > 0 AND t.expires_at < ${nowUnix}
- DELETE t
- RETURN count(t) as deleted_count
+ WHERE t.expires_at > 0 AND t.expires_at < ${nowUnix} AND t.is_active = true
+ SET t.is_active = false
+ RETURN count(t) as updated_countThis aligns with the existing revokeToken() pattern which uses soft-delete (SET t.is_active = false) and maintains audit trails through relationships.
📝 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.
| async cleanupExpiredTokens(): Promise<number> { | |
| const nowUnix = Math.floor(Date.now() / 1000); | |
| // Delete tokens that have expired (expires_at > 0 AND expires_at < now) | |
| const query = ` | |
| MATCH (t:Token) | |
| WHERE t.expires_at > 0 AND t.expires_at < ${nowUnix} | |
| DELETE t | |
| RETURN count(t) as deleted_count | |
| `; | |
| const result = await executePATQuery(query); | |
| if (result.data && result.data.length > 0) { | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| return (result.data[0] as any).deleted_count || 0; | |
| } | |
| return 0; | |
| } | |
| async cleanupExpiredTokens(): Promise<number> { | |
| const nowUnix = Math.floor(Date.now() / 1000); | |
| // Delete tokens that have expired (expires_at > 0 AND expires_at < now) | |
| const query = ` | |
| MATCH (t:Token) | |
| WHERE t.expires_at > 0 AND t.expires_at < ${nowUnix} AND t.is_active = true | |
| SET t.is_active = false | |
| RETURN count(t) as updated_count | |
| `; | |
| const result = await executePATQuery(query); | |
| if (result.data && result.data.length > 0) { | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| return (result.data[0] as any).updated_count || 0; | |
| } | |
| return 0; | |
| } |
🤖 Prompt for AI Agents
In lib/token-storage/FalkorDBTokenStorage.ts around lines 190 to 209, the
current cleanupExpiredTokens() physically deletes expired tokens (DELETE t)
which removes REVOKED_BY relationships and loses the audit trail; change it to
perform a soft-delete consistent with revokeToken(): only target tokens that are
expired AND currently active (t.expires_at > 0 AND t.expires_at < nowUnix AND
t.is_active = true), SET t.is_active = false and set an expiration marker (e.g.,
t.expired_at = nowUnix or update t.updated_at) rather than deleting, and then
RETURN count(t) as deleted_count so the method preserves relationships and only
marks expired active tokens as inactive.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.