Draft
Conversation
📝 WalkthroughWalkthroughAdds Google Sign-In integration: Expo config plugin with iOS URL scheme, Metro SVG transformer, new Google auth and backend token exchange services, secure token storage, package deps, and app initialization calling Google sign-in setup before redirecting. Changes
Sequence DiagramsequenceDiagram
participant App as App/Index
participant Google as Google SDK
participant Backend as Backend API
participant Store as Secure Store
App->>App: configureGoogleSignIn()
App->>Google: googleSignIn()
Google->>Google: verifyPlayServices / initiate sign-in
Google-->>App: idToken / accessToken
App->>Backend: POST /auth/oauth/token (provider: GOOGLE, idToken)
Backend->>Backend: validate idToken, create access/refresh tokens
Backend-->>App: TokenResponse { accessToken, refreshToken, redirectUrl? }
App->>Store: saveTokens(accessToken, refreshToken)
Store-->>App: success
App->>App: Redirect to authenticated flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (1)
app/index.tsx (1)
1-15: Move Google Sign-In setup intouseEffectto avoid render-time side effects.Calling
configureGoogleSignIn()during render executes on every re-render (and twice in Strict Mode). Use a mount-only effect instead.✅ Proposed change
-import { StyleSheet, Text, View, StatusBar, Pressable } from 'react-native'; +import { StyleSheet, Text, View, StatusBar, Pressable } from 'react-native'; +import { useEffect } from 'react'; @@ export default function Index() { - configureGoogleSignIn(); + useEffect(() => { + configureGoogleSignIn(); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/index.tsx` around lines 1 - 15, The call to configureGoogleSignIn() is done during render in the Index component causing render-time side effects; move that setup into a mount-only effect by importing useEffect from 'react' and invoking configureGoogleSignIn() inside a useEffect with an empty dependency array in the Index component, and remove the direct call from the render body so configuration only runs once on mount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app.config.ts`:
- Around line 33-39: Add a guard in app.config.ts to validate
process.env.EXPO_PUBLIC_IOS_URL_SCHEME before configuring the
'@react-native-google-signin/google-signin' plugin: check that
EXPO_PUBLIC_IOS_URL_SCHEME is defined and non-empty, and if not either throw a
clear build-time error or skip/disable adding the plugin to the plugins array so
the misconfiguration is caught early; reference the plugins array entry for
'@react-native-google-signin/google-signin' and the iosUrlScheme property when
implementing the check.
In `@package.json`:
- Line 23: The PR added "@react-native-google-signin/google-signin" but missed
the required Expo config plugin and Metro transformer setup; add the Google
Sign-In Expo config plugin entry in app.json (providing iosUrlScheme or Firebase
config files) so prebuild recognizes native config for the package, and update
project Metro configuration to extend Expo's transformer to include
react-native-svg-transformer; ensure developers know this package cannot run in
Expo Go and must use a dev build (npx expo prebuild --clean) after adding the
plugin and metro.config.js changes.
In `@services/auth/google.ts`:
- Around line 9-15: The configureGoogleSignIn function should validate
EXPO_PUBLIC_WEB_CLIENT_ID and EXPO_PUBLIC_IOS_CLIENT_ID before calling
GoogleSignin.configure: check that process.env.EXPO_PUBLIC_WEB_CLIENT_ID (and
process.env.EXPO_PUBLIC_IOS_CLIENT_ID when running on iOS) are non-empty strings
and if not, throw or call processLogger.error (or throw new Error) with a clear
message indicating which client ID is missing; only then call
GoogleSignin.configure(...) so misconfiguration fails fast instead of causing
opaque signIn() errors. Ensure the validation references the
configureGoogleSignIn function and the GoogleSignin.configure call so the guard
is colocated with the configuration step.
- Around line 25-32: After calling GoogleSignin.signIn() and checking
isSuccessResponse(response), add an explicit guard that verifies
response.data.idToken exists before calling getTokens('google', ...); if idToken
is missing, call console.error (or process logger) with a clear message
including the full response object (e.g., "Missing idToken in Google sign-in
response") and return null instead of passing an empty string to getTokens;
update the code paths around GoogleSignin.signIn, isSuccessResponse, and
getTokens to reflect this early-guard and improved logging.
In `@services/tokenstore.ts`:
- Around line 6-9: The saveTokens function currently calls
SecureStore.setItemAsync twice (ACCESS_TOKEN_KEY and REFRESH_TOKEN_KEY) which
can leave state inconsistent if the second write fails; change saveTokens to
perform an atomic save by writing both tokens as one JSON string under a single
key (e.g., TOKENS_KEY) or implement a rollback strategy: write the second token,
and if any write rejects delete the first key to restore clean state; update
references to ACCESS_TOKEN_KEY/REFRESH_TOKEN_KEY usage accordingly (or add a
TOKENS_KEY) and ensure SecureStore.setItemAsync and SecureStore.deleteItemAsync
are used to implement the chosen approach in the saveTokens function.
---
Nitpick comments:
In `@app/index.tsx`:
- Around line 1-15: The call to configureGoogleSignIn() is done during render in
the Index component causing render-time side effects; move that setup into a
mount-only effect by importing useEffect from 'react' and invoking
configureGoogleSignIn() inside a useEffect with an empty dependency array in the
Index component, and remove the direct call from the render body so
configuration only runs once on mount.
Comment on lines
25
to
32
| const response = await GoogleSignin.signIn(); | ||
|
|
||
| if (isSuccessResponse(response)) { | ||
| return await getTokens('google', response.data.idToken ?? ''); | ||
| } else { | ||
| console.log('Google Sign-In Cancelled'); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n services/auth/google.tsRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 1816
🏁 Script executed:
rg -A 10 "function getTokens|const getTokens|export.*getTokens" services/auth/Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 707
🏁 Script executed:
rg "GoogleSignin|isSuccessResponse" --type ts -A 3Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 1211
🏁 Script executed:
sed -n '1,50p' services/auth/konect.tsRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 1205
🏁 Script executed:
rg "isSuccessResponse" -B 5 -A 15 --type tsRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 2116
🏁 Script executed:
rg "getTokens\(" services/auth/ -B 2 -A 2 --type tsRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 359
🏁 Script executed:
ls -la services/auth/ && cat -n services/auth/*.ts | head -100Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 3488
Add explicit guard for missing idToken and improve error logging.
After isSuccessResponse(response), the code passes an empty string to getTokens if idToken is absent. Instead of silently failing with a request to the backend, guard early and log the error explicitly:
Suggested fix
if (isSuccessResponse(response)) {
- return await getTokens('google', response.data.idToken ?? '');
+ const idToken = response.data.idToken;
+ if (!idToken) {
+ console.error('Google Sign-In missing idToken');
+ return null;
+ }
+ return await getTokens('google', idToken);
} else {📝 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.
Suggested change
| const response = await GoogleSignin.signIn(); | |
| if (isSuccessResponse(response)) { | |
| return await getTokens('google', response.data.idToken ?? ''); | |
| } else { | |
| console.log('Google Sign-In Cancelled'); | |
| return null; | |
| } | |
| const response = await GoogleSignin.signIn(); | |
| if (isSuccessResponse(response)) { | |
| const idToken = response.data.idToken; | |
| if (!idToken) { | |
| console.error('Google Sign-In missing idToken'); | |
| return null; | |
| } | |
| return await getTokens('google', idToken); | |
| } else { | |
| console.log('Google Sign-In Cancelled'); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/auth/google.ts` around lines 25 - 32, After calling
GoogleSignin.signIn() and checking isSuccessResponse(response), add an explicit
guard that verifies response.data.idToken exists before calling
getTokens('google', ...); if idToken is missing, call console.error (or process
logger) with a clear message including the full response object (e.g., "Missing
idToken in Google sign-in response") and return null instead of passing an empty
string to getTokens; update the code paths around GoogleSignin.signIn,
isSuccessResponse, and getTokens to reflect this early-guard and improved
logging.
Comment on lines
+6
to
+9
| export async function saveTokens(accessToken: string, refreshToken: string) { | ||
| await SecureStore.setItemAsync(ACCESS_TOKEN_KEY, accessToken); | ||
| await SecureStore.setItemAsync(REFRESH_TOKEN_KEY, refreshToken); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the tokenstore.ts file
find . -name "tokenstore.ts" -type fRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 95
🏁 Script executed:
cat -n ./services/tokenstore.tsRepository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 933
🏁 Script executed:
# Search for all usages of saveTokens
rg "saveTokens" -A 3 -B 1Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 931
🏁 Script executed:
cat -n ./services/auth/konect.ts | head -50Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 1513
🏁 Script executed:
# Search for usage of getAccessToken and getRefreshToken to see how tokens are used
rg "getAccessToken|getRefreshToken" -A 2 -B 1 | head -60Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 462
🏁 Script executed:
# Search for imports of these token getters
rg "from.*tokenstore" | head -20Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 138
🏁 Script executed:
# Search for SecureStore usage to see if tokens are accessed elsewhere
rg "SecureStore\." -A 1 | grep -v "^--$"Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 670
🏁 Script executed:
# Search for context, hooks, or auth state management
rg "useAuth|useToken|AuthContext|TokenContext" -l | head -20Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 53
🏁 Script executed:
# List all files in services directory
ls -la ./services/Repository: BCSDLab/KONECT_REACT_NATIVE
Length of output: 435
🌐 Web query:
expo-secure-store setItemAsync atomicity transaction behavior
💡 Result:
expo-secure-store does not provide transactions across multiple keys. Each setItemAsync() call is an independent write of one key/value, backed by the platform’s storage primitives. (docs.expo.dev)
What “atomic” means here
Per-key write (single call)
- iOS:
setItemAsync()maps to a Keychain add-or-update: it callsSecItemAdd(...), and on duplicate it callsSecItemUpdate(...)for that key. This makes the operation effectively a single-item replace (either the new value is stored for that key or an error is thrown). (raw.githubusercontent.com) - Android: values are stored in a SecureStore-specific
SharedPreferencesfile, and Expo writes usingSharedPreferences.Editor.putString(...).commit()(a synchronous commit that returns success/failure). (raw.githubusercontent.com)
Multi-key “transaction”
- There is no API to group multiple
setItemAsync()calls into an all-or-nothing commit, and no isolation guarantees (another write can interleave between two of your writes). This follows from the underlying implementations: Keychain item operations andSharedPreferencesedits are not exposed as a cross-item transaction byexpo-secure-store. (raw.githubusercontent.com)
Concurrency / “last write wins”
- If you call
setItemAsync()concurrently for the same key, whichever write reaches the native store last will be the value you read afterward (iOS updates on duplicate; Android overwrites the stored string). (raw.githubusercontent.com)
If you need transactional behavior
- Put the related fields into one JSON blob under a single key (so the “transaction” becomes a single per-key write), or implement an app-level scheme (e.g., write new state under a versioned key, then atomically flip a “currentVersion” pointer key). (No built-in support in SecureStore.) (docs.expo.dev)
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/securestore?utm_source=openai
- 2: https://raw.githubusercontent.com/expo/expo/main/packages/expo-secure-store/ios/SecureStoreModule.swift
- 3: https://raw.githubusercontent.com/expo/expo/main/packages/expo-secure-store/android/src/main/java/expo/modules/securestore/SecureStoreModule.kt
- 4: https://raw.githubusercontent.com/expo/expo/main/packages/expo-secure-store/android/src/main/java/expo/modules/securestore/SecureStoreModule.kt
- 5: https://raw.githubusercontent.com/expo/expo/main/packages/expo-secure-store/android/src/main/java/expo/modules/securestore/SecureStoreModule.kt
- 6: https://docs.expo.dev/versions/latest/sdk/securestore?utm_source=openai
Implement atomic storage for access and refresh tokens.
expo-secure-store does not support transactions across multiple keys—each setItemAsync() call is independent. If the second write fails, the app ends up with only one token stored, creating an inconsistent auth state that can cause API failures on refresh.
Wrap the writes to rollback on failure:
🛡️ Suggested rollback on failure
export async function saveTokens(accessToken: string, refreshToken: string) {
- await SecureStore.setItemAsync(ACCESS_TOKEN_KEY, accessToken);
- await SecureStore.setItemAsync(REFRESH_TOKEN_KEY, refreshToken);
+ try {
+ await SecureStore.setItemAsync(ACCESS_TOKEN_KEY, accessToken);
+ await SecureStore.setItemAsync(REFRESH_TOKEN_KEY, refreshToken);
+ } catch (error) {
+ await SecureStore.deleteItemAsync(ACCESS_TOKEN_KEY);
+ await SecureStore.deleteItemAsync(REFRESH_TOKEN_KEY);
+ throw error;
+ }
}Alternatively, store both tokens as a single JSON object under one key for true atomic writes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/tokenstore.ts` around lines 6 - 9, The saveTokens function currently
calls SecureStore.setItemAsync twice (ACCESS_TOKEN_KEY and REFRESH_TOKEN_KEY)
which can leave state inconsistent if the second write fails; change saveTokens
to perform an atomic save by writing both tokens as one JSON string under a
single key (e.g., TOKENS_KEY) or implement a rollback strategy: write the second
token, and if any write rejects delete the first key to restore clean state;
update references to ACCESS_TOKEN_KEY/REFRESH_TOKEN_KEY usage accordingly (or
add a TOKENS_KEY) and ensure SecureStore.setItemAsync and
SecureStore.deleteItemAsync are used to implement the chosen approach in the
saveTokens function.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
services/auth/google.ts (2)
50-52: Log unknown errors before returning null.When an error is caught but lacks a recognized code, the function silently returns
null. Logging the error here would aid debugging unexpected failures.🔧 Proposed fix
} else { + console.error('Google Sign-In Unknown Error:', error); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth/google.ts` around lines 50 - 52, The catch branch in services/auth/google.ts currently returns null silently in the else block; update that branch to log the unexpected error before returning null (use the project's logger if available, e.g., processLogger.error or console.error) and include the error object and contextual message (e.g., "Unexpected error in [function name or Google auth handler]") so unexpected failures are visible in logs.
28-32: Remove redundant nullish coalescing.Line 32 uses
response.data.idToken ?? ''but lines 28-31 already guaranteeidTokenis truthy when reaching line 32. The fallback is dead code.🧹 Suggested cleanup
if (!response.data.idToken) { console.log('Google Sign-In Failed: No ID Token'); return null; } - return await getTokens('google', response.data.idToken ?? ''); + return await getTokens('google', response.data.idToken); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/auth/google.ts` around lines 28 - 32, The code is passing a redundant fallback to getTokens after already checking idToken; in services/auth/google.ts locate the call to getTokens('google', response.data.idToken ?? '') and remove the nullish coalescing so you pass the guaranteed token directly (use response.data.idToken or response.data.idToken! if TypeScript needs a non-null assertion) so dead-code fallback '' is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@services/auth/google.ts`:
- Around line 9-15: The configureGoogleSignIn function should validate
EXPO_PUBLIC_WEB_CLIENT_ID and EXPO_PUBLIC_IOS_CLIENT_ID before calling
GoogleSignin.configure; update configureGoogleSignIn to read
process.env.EXPO_PUBLIC_WEB_CLIENT_ID and process.env.EXPO_PUBLIC_IOS_CLIENT_ID,
verify they are non-empty, and if either is missing log/throw a clear error (or
return early) describing which env var is absent instead of calling
GoogleSignin.configure with undefined values so sign-in failures become
actionable.
---
Nitpick comments:
In `@services/auth/google.ts`:
- Around line 50-52: The catch branch in services/auth/google.ts currently
returns null silently in the else block; update that branch to log the
unexpected error before returning null (use the project's logger if available,
e.g., processLogger.error or console.error) and include the error object and
contextual message (e.g., "Unexpected error in [function name or Google auth
handler]") so unexpected failures are visible in logs.
- Around line 28-32: The code is passing a redundant fallback to getTokens after
already checking idToken; in services/auth/google.ts locate the call to
getTokens('google', response.data.idToken ?? '') and remove the nullish
coalescing so you pass the guaranteed token directly (use response.data.idToken
or response.data.idToken! if TypeScript needs a non-null assertion) so dead-code
fallback '' is eliminated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 요약
😎 해결한 이슈
Summary by CodeRabbit