feat: add RECOUP_ORG_ID constant for accountId override#109
Conversation
Add the Recoup admin organization UUID constant to support the accountId override feature. API keys from this organization have universal access and can specify any accountId when creating chats. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces account ID override functionality for organization API keys. It adds API key validation, organization access checking, and modifies chat creation to support accountId parameter overrides when authorized. A new Recoup admin organization constant grants universal access permissions. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as createChatHandler
participant Validator as validateOverrideAccountId
participant KeySvc as getApiKeyDetails
participant OrgSvc as canAccessAccount
participant DB as getAccountOrganizations
participant ChatDB as insertRoom
Client->>Handler: POST /chat (with optional accountId)
Handler->>Handler: Extract API key from auth
alt bodyAccountId provided
Handler->>Validator: validateOverrideAccountId(apiKey, targetAccountId)
Validator->>KeySvc: getApiKeyDetails(apiKey)
KeySvc->>DB: getAccountOrganizations()
DB-->>KeySvc: org membership details
KeySvc-->>Validator: { accountId, orgId }
Validator->>OrgSvc: canAccessAccount(orgId, targetAccountId)
alt orgId equals RECOUP_ORG_ID
OrgSvc-->>Validator: true (universal access)
else organization member access
OrgSvc->>DB: getAccountOrganizations(accountId, orgId)
DB-->>OrgSvc: membership check
OrgSvc-->>Validator: boolean result
end
alt Access granted
Validator-->>Handler: { accountId: targetAccountId }
Handler->>Handler: Override accountId
else Access denied
Validator-->>Handler: 403 Forbidden
Handler-->>Client: Error response
end
end
Handler->>ChatDB: insertRoom(with resolved accountId)
ChatDB-->>Handler: room created
Handler-->>Client: 200 Success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)lib/**/validate*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-01-09T15:08:44.122ZApplied to files:
📚 Learning: 2026-01-09T15:08:44.122ZApplied to files:
📚 Learning: 2026-01-09T15:08:44.122ZApplied to files:
🧬 Code graph analysis (5)lib/organizations/__tests__/canAccessAccount.test.ts (2)
lib/keys/__tests__/getApiKeyDetails.test.ts (3)
lib/accounts/validateOverrideAccountId.ts (3)
lib/chats/createChatHandler.ts (1)
lib/organizations/canAccessAccount.ts (2)
⏰ 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 (9)
✏️ Tip: You can disable this entire section by setting 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 |
Adds a new authentication utility that extracts both accountId and orgId from an API key. This is needed for the accountId override feature where org API keys can specify a target accountId. - Returns accountId from the API key's account field - Returns orgId if the account is an organization (exists in account_organization_ids) - Returns null orgId for personal API keys - Includes 7 unit tests covering all scenarios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/auth/getApiKeyDetails.ts`:
- Line 28: getApiKeyDetails currently calls hashApiKey(apiKey,
PRIVY_PROJECT_SECRET) but PRIVY_PROJECT_SECRET is typed string | undefined; add
an explicit guard in getApiKeyDetails to handle undefined PRIVY_PROJECT_SECRET
before calling hashApiKey (e.g., log an error and return null or throw a clear
Error) so hashApiKey always receives a string; reference PRIVY_PROJECT_SECRET,
hashApiKey, and getApiKeyDetails when updating the code and ensure any callers
handle the null/error return.
🧹 Nitpick comments (3)
lib/auth/getApiKeyDetails.ts (1)
42-48: Consider logging supabase query errors.The error from the
account_organization_idsquery is destructured away. While returningnullfororgIdon failure is acceptable, silently ignoring database errors makes debugging harder.♻️ Suggested improvement
// Check if this account is an organization by looking it up in account_organization_ids - const { data: orgRecord } = await supabase + const { data: orgRecord, error: orgError } = await supabase .from("account_organization_ids") .select("organization_id") .eq("organization_id", accountId) .maybeSingle(); + if (orgError) { + console.error("[ERROR] getApiKeyDetails: Failed to check organization:", orgError); + } + const orgId = orgRecord?.organization_id ?? null;lib/auth/__tests__/getApiKeyDetails.test.ts (2)
38-64: Consider verifying mock invocations.The tests verify the return value but don't assert that dependencies were called with the expected arguments. Adding these checks would improve confidence that the function integrates correctly.
♻️ Example verification additions
import { hashApiKey } from "@/lib/keys/hashApiKey"; // Inside the test after calling getApiKeyDetails: expect(hashApiKey).toHaveBeenCalledWith("test_api_key", "test_secret"); expect(selectAccountApiKeys).toHaveBeenCalledWith({ keyHash: "hashed_test_api_key" }); expect(supabase.from).toHaveBeenCalledWith("account_organization_ids");
17-27: Consider adding a test for supabase query failure.The implementation's try-catch block would handle a thrown error from the supabase query, but there's no test verifying this path or that the error is logged correctly.
♻️ Suggested additional test
it("returns result with null orgId when supabase org lookup fails", async () => { vi.mocked(selectAccountApiKeys).mockResolvedValue([ { id: "key-1", account: "account-123", name: "Test Key", key_hash: "hashed_test_key", created_at: "2024-01-01T00:00:00Z", last_used: null, }, ]); // Mock supabase to return an error const mockMaybeSingle = vi.fn().mockResolvedValue({ data: null, error: { message: "Database error" } }); const mockEq = vi.fn().mockReturnValue({ maybeSingle: mockMaybeSingle }); const mockSelect = vi.fn().mockReturnValue({ eq: mockEq }); vi.mocked(supabase.from).mockReturnValue({ select: mockSelect } as any); const result = await getApiKeyDetails("test_key"); // Currently implementation ignores error and returns null orgId expect(result).toEqual({ accountId: "account-123", orgId: null, }); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/auth/__tests__/getApiKeyDetails.test.tslib/auth/getApiKeyDetails.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/auth/getApiKeyDetails.ts (3)
lib/keys/hashApiKey.ts (1)
hashApiKey(10-12)lib/const.ts (1)
PRIVY_PROJECT_SECRET(14-14)lib/supabase/account_api_keys/selectAccountApiKeys.ts (1)
selectAccountApiKeys(13-32)
lib/auth/__tests__/getApiKeyDetails.test.ts (2)
lib/supabase/account_api_keys/selectAccountApiKeys.ts (1)
selectAccountApiKeys(13-32)lib/auth/getApiKeyDetails.ts (1)
getApiKeyDetails(20-58)
⏰ 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: Vercel Agent Review
🔇 Additional comments (3)
lib/auth/getApiKeyDetails.ts (1)
6-9: Well-structured implementation with good defensive coding.The function properly handles null/empty inputs, wraps database operations in try-catch, and provides clear JSDoc documentation. The interface is cleanly defined with appropriate nullability for
orgId.Also applies to: 20-57
lib/auth/__tests__/getApiKeyDetails.test.ts (2)
66-95: Organization key test correctly validates orgId derivation.The test properly mocks the scenario where
account_organization_idsreturns a matching record, and verifies that bothaccountIdandorgIdare set to the same organization ID. This aligns with the implementation logic.
98-143: Good coverage of edge cases and error paths.The invalid input tests comprehensively cover: empty string, undefined coercion, not found in DB,
selectAccountApiKeysreturning null (error case), and API key record with null account. This provides solid defensive coverage.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document that all Supabase calls must be in lib/supabase/[table_name]/[function].ts - Add directory structure example - Add naming conventions (select, insert, update, delete, get) - Add code pattern template Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add accountId field to POST /api/chats validation schema to enable account override for organization API keys. Includes 9 unit tests for the validation logic. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Allow org API keys to create chat rooms for accounts within their org. - When accountId provided, validates access via getApiKeyDetails + canAccessAccount - Returns 403 if access denied, 500 if API key validation fails - Recoup admin org has universal access to all accounts - Includes 6 unit tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add CRITICAL marker: NEVER import serverClient outside lib/supabase/ - Add 3-step process for database access in domain code - Add WRONG vs CORRECT code examples - Make rule more prominent and actionable Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace direct Supabase query with existing getAccountOrganizations() lib function, following the architecture pattern documented in CLAUDE.md. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extracted direct Supabase call from getApiKeyDetails.ts into a new isOrganization() function in lib/supabase/account_organization_ids/. This follows the architecture pattern of using supabase lib functions instead of direct queries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
canAccessAccount is about organization access control, not authentication. Moving it to lib/organizations for better code organization. - Moved lib/auth/canAccessAccount.ts to lib/organizations/canAccessAccount.ts - Moved tests to lib/organizations/__tests__/canAccessAccount.test.ts - Updated imports in createChatHandler.ts and its tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move getApiKeyDetails to lib/keys since it's about API key operations, not authentication. Updated imports in createChatHandler and its tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create lib/accounts/validateOverrideAccountId.ts for SRP - Update createChatHandler to use validateOverrideAccountId - Replace isOrganization with getAccountOrganizations in getApiKeyDetails - Extend getAccountOrganizations to support organizationId-only queries - Delete isOrganization.ts and its tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
RECOUP_ORG_IDconstant tolib/const.tswith the Recoup admin organization UUIDTest plan
pnpm test- all 64 unit tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.