feat(keychain): add TikTok OAuth connections UI #2313
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new "Connected Accounts" section to the Settings page, enabling users to manage TikTok OAuth connections. The feature allows users to connect their TikTok accounts via OAuth flow, view connection status (including expiration), and disconnect accounts when needed. The implementation is controlled by a feature flag for gradual rollout.
Key changes:
- New GraphQL schema definitions and queries for OAuth connection management
- ConnectionsSection component that displays connected accounts and handles OAuth flow
- ConnectionCard component for displaying individual connections with disconnect functionality
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/keychain/src/utils/api/oauth-connections.ts |
Defines TypeScript types and GraphQL queries/mutations for OAuth connection operations |
packages/keychain/src/utils/api/index.ts |
Exports OAuth connections API for use throughout the application |
packages/keychain/src/components/settings/index.tsx |
Integrates ConnectionsSection into Settings page with feature flag control |
packages/keychain/src/components/settings/connections/index.ts |
Exports connection components from the module |
packages/keychain/src/components/settings/connections/connections-section.tsx |
Main component for managing OAuth connections list and initiating new connections |
packages/keychain/src/components/settings/connections/connection-card.tsx |
Card component displaying individual OAuth connection with disconnect action |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| window.open( | ||
| authUrl, | ||
| "tiktok-oauth", | ||
| `width=${width},height=${height},left=${left},top=${top}`, | ||
| ); |
There was a problem hiding this comment.
The OAuth popup window is opened but there's no mechanism to detect when the OAuth flow completes and refresh the connection list. After the user successfully authenticates in the popup, the UI won't automatically update to show the connected account. Consider adding a window message listener to detect when the OAuth callback completes and then invalidate the query to refresh the connections list.
| window.open( | |
| authUrl, | |
| "tiktok-oauth", | |
| `width=${width},height=${height},left=${left},top=${top}`, | |
| ); | |
| const popup = window.open( | |
| authUrl, | |
| "tiktok-oauth", | |
| `width=${width},height=${height},left=${left},top=${top}`, | |
| ); | |
| // Listen for OAuth completion message from the popup and refresh connections | |
| const handleMessage = (event: MessageEvent) => { | |
| const data = event.data; | |
| if (!data || typeof data !== "object") { | |
| return; | |
| } | |
| const { type, provider } = data as { type?: string; provider?: string }; | |
| if (type === "oauth-complete" && provider === "TIKTOK") { | |
| queryClient.invalidateQueries("oauthConnections"); | |
| window.removeEventListener("message", handleMessage); | |
| if (popup && !popup.closed) { | |
| popup.close(); | |
| } | |
| } | |
| }; | |
| window.addEventListener("message", handleMessage); |
| onClick={async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| await onDisconnect?.(); | ||
| setIsOpen(false); | ||
| } catch (error) { | ||
| console.error(error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } |
There was a problem hiding this comment.
The disconnect error is silently swallowed with only a console.error. If the disconnection fails (e.g., due to network error or backend issue), the user won't see any error feedback and the sheet will close, giving the false impression that it succeeded. Consider displaying an error message to the user and keeping the sheet open when the disconnect operation fails.
| <div className="text-destructive-100 text-sm"> | ||
| Failed to load connections |
There was a problem hiding this comment.
The error message "Failed to load connections" is too generic and doesn't help the user understand what went wrong or how to fix it. Consider providing more specific error information, such as whether it's a network error, authentication issue, or backend error, along with suggested actions like "Try refreshing the page" or "Check your internet connection".
| className="border-background-100 p-6 gap-6 rounded-t-xl" | ||
| showClose={false} | ||
| > | ||
| <SheetTitle className="hidden"></SheetTitle> |
There was a problem hiding this comment.
The SheetTitle has an empty string and is hidden with className="hidden". This is an accessibility issue as it leaves the sheet without a proper accessible name for screen readers. While the visual heading exists in the content, the SheetTitle should contain meaningful text that describes the sheet's purpose, such as "Disconnect OAuth Connection Confirmation".
| <SheetTitle className="hidden"></SheetTitle> | |
| <SheetTitle className="hidden"> | |
| Disconnect {providerName} connection | |
| </SheetTitle> |
| const disconnectMutation = useMutation<boolean, Error, string>( | ||
| async (provider: string) => { | ||
| const result = await request<DisconnectOAuthData>(DISCONNECT_OAUTH, { | ||
| provider, | ||
| }); |
There was a problem hiding this comment.
The provider parameter is typed as string but should be typed as OAuthProvider. This creates a type safety issue where any string could be passed instead of the valid enum values. The mutation should accept OAuthProvider type to match the GraphQL schema and prevent runtime errors from invalid provider values.
| "tiktok-oauth", | ||
| `width=${width},height=${height},left=${left},top=${top}`, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
There's no error handling for the connectTikTok mutation failure. If the INITIATE_TIKTOK_OAUTH mutation fails (e.g., due to network error or backend issue), the user won't see any error message. Consider adding an onError callback to the mutation to display an error message to the user.
| }, | |
| }, | |
| onError: () => { | |
| window.alert( | |
| "Failed to initiate TikTok connection. Please try again.", | |
| ); | |
| }, |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/keychain/src/components/settings/connections/connections-section.tsx
Show resolved
Hide resolved
packages/keychain/src/components/settings/connections/connections-section.tsx
Outdated
Show resolved
Hide resolved
| inProgress: false, | ||
| }); | ||
| } | ||
| }, 500); |
There was a problem hiding this comment.
OAuth popup closure falsely indicates successful connection
High Severity
The OAuth flow logic assumes success whenever the popup closes or is null. The polling logic checks !popup || popup.closed and then displays "TikTok Connected" with a check icon and navigates away. However, the popup can close for many reasons: user manually closed it, user denied authorization, popup was blocked by the browser (returns null), or actual success. Without message passing or verifying the connection status via API, users see a false success message when the OAuth flow actually failed.
Additional Locations (1)
| Connect Socials | ||
| </span> | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Connect button shows during loading and error states
Low Severity
The hasConnections variable is data && data.length > 0, which evaluates to falsy when data is undefined during loading or error states. This causes the "Connect Socials" button to appear alongside the loading skeleton or error message, creating a confusing UI where users see both a loading/error indicator and a call-to-action button simultaneously.
| queryClient.invalidateQueries("oauthConnections"); | ||
| }, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Disconnect mutation ignores server failure response value
Medium Severity
The disconnectOAuth GraphQL mutation returns a boolean indicating success, but this value is never validated. The onSuccess callback in the mutation fires regardless of whether disconnectOAuth is true or false, and the connection card closes the sheet after awaiting onDisconnect() without checking the return value. If the server returns false (disconnect failed), the UI incorrectly indicates success by closing the sheet and invalidating the cache, even though the connection wasn't actually removed.
Additional Locations (1)
| inProgress: false, | ||
| }); | ||
| } | ||
| }, 500); |
There was a problem hiding this comment.
Interval not cleaned up on component unmount
Medium Severity
The pollTimer interval created in handleTikTokConnect is not cleaned up when the component unmounts. Unlike the established codebase pattern (e.g., header.tsx and quests/index.tsx) where intervals are created in useEffect with a cleanup return function, this interval lives inside a callback with no unmount cleanup. If the user navigates away before the popup closes, the interval continues running, potentially causing memory leaks and attempting state updates on an unmounted component.
| navigate("/settings"); | ||
| }, 2000); | ||
| } | ||
| }, [connectionPending, navigate]); |
There was a problem hiding this comment.
useEffect setTimeout lacks cleanup on unmount
Medium Severity
The useEffect that schedules navigation after a successful connection creates a setTimeout without returning a cleanup function. If the user navigates away during the 2-second delay (e.g., browser back button), the timeout will still fire and trigger navigate("/settings"), causing unexpected navigation. Unlike the interval issue in the callback, this useEffect has a well-established pattern for cleanup (return () => clearTimeout(id)) that isn't being followed.
Add UI for managing TikTok OAuth connections in settings: - ConnectionCard component for displaying connected accounts - ConnectionsSection for listing and managing connections - Integration with initiateTikTokOAuth and disconnectOAuth mutations - Feature flag for gradual rollout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add @icons-pack/react-simple-icons dependency for TikTok icon - Fix onDisconnect callback return type (Promise<void> instead of Promise<boolean>) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use useFetchData hook instead of raw request function to ensure auth headers are included when querying OAuth connections. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The keychain iframe doesn't have session cookies for the API, so the `me` query fails with unauthenticated error. Changed to use `account(username: $username)` query which doesn't require auth, similar to how sessions are queried. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated mutations to pass username parameter: - initiateTikTokOAuth now accepts username - disconnectOAuth now accepts username and provider 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add AddConnectionRoute and add-connection component - Update connections-section with "+ Connect Socials" button - Use direct URL for OAuth init instead of GraphQL mutation - Add /settings/add-connection route Flow matches existing signer pattern with pending state while OAuth completes in popup window. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The connections section can now be toggled per-session via URL: - /feature/connections/enable - /feature/connections/disable This allows testing the feature without affecting all users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
steebchen
force-pushed
the
feat/tiktok-oauth-connections
branch
from
3df2757 to
47bb9fd
Compare
| export function AddConnectionRoute() { | ||
| const { controller } = useConnection(); | ||
|
|
||
| return <AddConnection username={controller?.username()} />; |
There was a problem hiding this comment.
Feature flag bypassed via direct route navigation
Medium Severity
The connections feature flag controls visibility of ConnectionsSection in the settings page, but the /settings/add-connection route is always registered and accessible. AddConnectionRoute doesn't check isFeatureEnabled("connections"), so users can directly navigate to the URL and access the OAuth connection flow even when the feature flag is disabled. This defeats the purpose of the gradual rollout mechanism described in the PR.
Additional Locations (1)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2313 +/- ##
==========================================
- Coverage 20.89% 20.68% -0.21%
==========================================
Files 307 313 +6
Lines 31079 31489 +410
Branches 880 883 +3
==========================================
+ Hits 6493 6514 +21
- Misses 24574 24963 +389
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Claude finished @steebchen's task —— View job Documentation analysis and updates completedI've analyzed the TikTok OAuth connections feature from PR #2313 and updated the documentation accordingly. Todo List:
What I Added:I successfully added comprehensive documentation for the new TikTok OAuth connections feature to 📝 Social Connections (OAuth) Section:
👤 User Guide:
🔒 Security & Privacy:
💻 Developer Integration:
Key Documentation Points:
Files Modified:
The documentation is ready and staged for commit. Due to git configuration permissions, the changes are prepared but not yet committed to the repository. The documentation comprehensively covers the new TikTok OAuth connections feature and maintains consistency with the existing documentation structure and style. |
Summary
connections: true)Changes
connection-card.tsx,connections-section.tsx,oauth-connections.ts(GraphQL queries)settings/index.tsx(add ConnectionsSection)Dependencies
Requires backend PR in cartridge-internal to be merged first: https://github.com/cartridge-gg/internal/pull/4003
Test plan
🤖 Generated with Claude Code
Note
Adds a feature-flagged UI to manage social connections in Settings.
ConnectionsSection,ConnectionCard, andAddConnectionflow with popup OAuth; route'/settings/add-connection'GET_OAUTH_CONNECTIONSandDISCONNECT_OAUTHplusgetTikTokAuthUrl(); integratesreact-queryfor fetch/invalidateuseFeaturesto toggleconnections; Settings updated to render section when enabled@icons-pack/react-simple-iconsfor TikTok iconWritten by Cursor Bugbot for commit 47bb9fd. This will update automatically on new commits. Configure here.