feat/twitter-auth#149
Conversation
📝 WalkthroughWalkthroughThis PR adds Twitter OAuth2 authentication support to the Turnkey wallet integration. It introduces configuration constants for Twitter OAuth, extends wallet store state to track which provider (Email, Google, or Twitter) was used for authentication, implements new OAuth flow actions with PKCE and localStorage state management, and configures the Turnkey strategy metadata to include Twitter OAuth fields. ChangesTwitter OAuth2 Integration for Turnkey
Sequence DiagramsequenceDiagram
participant Client
participant Turnkey
participant Twitter
participant App as App Store
Client->>Turnkey: connectTurnkeyTwitter()
Turnkey->>Turnkey: initOAuth2 (PKCE)
Turnkey->>Turnkey: Save oauth/PKCE/expiry to localStorage
Turnkey->>Twitter: Redirect to authorization URL
Twitter->>Client: Redirect with authCode + state
Client->>Turnkey: initTurnkeyTwitter(authCode, state)
Turnkey->>Turnkey: Validate state, PKCE, expiry
Turnkey->>Turnkey: confirmOAuth2 with authCode
Turnkey->>Turnkey: Fetch wallet addresses
Turnkey->>App: Patch walletStore (session, address, provider=Twitter)
Turnkey->>App: onConnect callback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/store/wallet/index.tsOops! Something went wrong! :( ESLint: 9.39.2 Error: Cannot find module './.nuxt/eslint.config.mjs'
... [truncated 273 characters] ... 266:11) app/utils/constant/index.tsOops! Something went wrong! :( ESLint: 9.39.2 Error: Cannot find module './.nuxt/eslint.config.mjs'
... [truncated 273 characters] ... 266:11) app/store/wallet/turnkey.tsOops! Something went wrong! :( ESLint: 9.39.2 Error: Cannot find module './.nuxt/eslint.config.mjs'
... [truncated 273 characters] ... 266:11)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/store/wallet/turnkey.ts`:
- Around line 187-215: The expiresAt validation currently only triggers when
expiresAt > 0, allowing missing or unparsable values to bypass expiry; update
the check around the expiresAt variable so that missing/invalid values also fail
closed (e.g., treat 0/NaN as expired) and throw the same WalletException (with
UnspecifiedErrorCode and ErrorType.WalletError). Locate the expiresAt
declaration and the subsequent if block that throws on expiry and replace the
condition with one that checks for invalid/missing values (isNaN or <= 0) OR
Date.now() > expiresAt so the OAuth session is rejected when expiresAt is absent
or invalid.
In `@app/wallet/strategy.ts`:
- Around line 65-68: The apiServerEndpoint selector currently uses
IS_TRUE_CURRENT to point to 'http://localhost:3001/api/v1', which breaks
OAuth/session exchange outside local dev; update the conditional in
app/wallet/strategy.ts (the apiServerEndpoint assignment that references
IS_TRUE_CURRENT) to use the correct TrueCurrent remote API URL or derive the
host from an environment/config value instead of hardcoding localhost so
deployed environments never route TrueCurrent traffic to localhost.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40b77517-f37c-468a-8b1a-40e8ff6f84e7
📒 Files selected for processing (5)
app/store/wallet/index.tsapp/store/wallet/turnkey.tsapp/utils/constant/index.tsapp/utils/constant/setup.tsapp/wallet/strategy.ts
| const expiresAt = Number( | ||
| localStorage.getItem('twitter_oauth_expires_at') || '0' | ||
| ) | ||
|
|
||
| localStorage.removeItem('twitter_oauth_state') | ||
| localStorage.removeItem('twitter_oauth_expires_at') | ||
| localStorage.removeItem('twitter_oauth_code_verifier') | ||
| localStorage.removeItem('twitter_oauth_target_public_key') | ||
|
|
||
| if (state !== savedState) { | ||
| throw new WalletException( | ||
| new Error('Twitter sign-in failed — invalid state. Please try again.'), | ||
| { code: UnspecifiedErrorCode, type: ErrorType.WalletError } | ||
| ) | ||
| } | ||
|
|
||
| if (!codeVerifier || !targetPublicKey) { | ||
| throw new WalletException( | ||
| new Error('OAuth session not found — please try signing in again'), | ||
| { code: UnspecifiedErrorCode, type: ErrorType.WalletError } | ||
| ) | ||
| } | ||
|
|
||
| if (expiresAt > 0 && Date.now() > expiresAt) { | ||
| throw new WalletException( | ||
| new Error('OAuth session expired — please try signing in again'), | ||
| { code: UnspecifiedErrorCode, type: ErrorType.WalletError } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fail closed when OAuth expiry is missing or invalid.
Line 210 only checks expiry when expiresAt > 0, so missing or unparsable values (0/NaN) skip validation and continue the auth flow.
Proposed fix
- const expiresAt = Number(
- localStorage.getItem('twitter_oauth_expires_at') || '0'
- )
+ const expiresAtRaw = localStorage.getItem('twitter_oauth_expires_at')
+ const expiresAt = Number(expiresAtRaw)
@@
- if (expiresAt > 0 && Date.now() > expiresAt) {
+ if (!Number.isFinite(expiresAt) || expiresAt <= 0 || Date.now() > expiresAt) {
throw new WalletException(
new Error('OAuth session expired — please try signing in again'),
{ code: UnspecifiedErrorCode, type: ErrorType.WalletError }
)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/store/wallet/turnkey.ts` around lines 187 - 215, The expiresAt validation
currently only triggers when expiresAt > 0, allowing missing or unparsable
values to bypass expiry; update the check around the expiresAt variable so that
missing/invalid values also fail closed (e.g., treat 0/NaN as expired) and throw
the same WalletException (with UnspecifiedErrorCode and ErrorType.WalletError).
Locate the expiresAt declaration and the subsequent if block that throws on
expiry and replace the condition with one that checks for invalid/missing values
(isNaN or <= 0) OR Date.now() > expiresAt so the OAuth session is rejected when
expiresAt is absent or invalid.
| apiServerEndpoint: IS_TRUE_CURRENT | ||
| ? 'https://api.ui.staging.tc.xyz/api/v1' | ||
| : 'https://api.ui.injective.network/api/v1', | ||
| expirationSeconds: '86400' | ||
| ? 'http://localhost:3001/api/v1' | ||
| : // ? 'https://api.ui.staging.tc.xyz/api/v1' | ||
| 'https://api.ui.injective.network/api/v1', |
There was a problem hiding this comment.
Do not route TrueCurrent Turnkey API traffic to localhost.
Using http://localhost:3001/api/v1 for IS_TRUE_CURRENT will fail outside local development and break OAuth/session exchange in deployed environments.
Proposed fix
- apiServerEndpoint: IS_TRUE_CURRENT
- ? 'http://localhost:3001/api/v1'
- : // ? 'https://api.ui.staging.tc.xyz/api/v1'
- 'https://api.ui.injective.network/api/v1',
+ apiServerEndpoint: IS_TRUE_CURRENT
+ ? 'https://api.ui.staging.tc.xyz/api/v1'
+ : 'https://api.ui.injective.network/api/v1',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/wallet/strategy.ts` around lines 65 - 68, The apiServerEndpoint selector
currently uses IS_TRUE_CURRENT to point to 'http://localhost:3001/api/v1', which
breaks OAuth/session exchange outside local dev; update the conditional in
app/wallet/strategy.ts (the apiServerEndpoint assignment that references
IS_TRUE_CURRENT) to use the correct TrueCurrent remote API URL or derive the
host from an environment/config value instead of hardcoding localhost so
deployed environments never route TrueCurrent traffic to localhost.
1 participant
Summary by CodeRabbit
New Features
Configuration