SELF-1204: Enable Turnkey for iOS #1401
Conversation
WalkthroughTwo dependency patches: one filters Turnkey publicKeys to only process 66- or 130‑char hex keys before deletion; the other replaces the iOS RNInAppBrowser implementation to add auth flows, Safari presentation, scene-aware startup, and full session lifecycle management. package.json script tweak updates reinstall cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript
participant ObjC as RNInAppBrowser (Obj-C)
participant Scene as UIScene (iOS13+)
participant Auth as AuthSession (AS/SFAuth)
participant Safari as SFSafariVC
JS->>ObjC: openAuth(url, redirectURL, options)
ObjC->>ObjC: initializeWebBrowserWithResolver()
alt iOS13+ and no active scene
ObjC->>Scene: waitForActiveSceneAndStartSession() (retry/backoff)
Scene-->>ObjC: scene becomes active
end
ObjC->>Auth: startAuthSession(url, callback)
Auth->>Safari: present web auth UI
Safari-->>Auth: user completes or cancels
Auth-->>ObjC: completion (success/cancel/error)
ObjC->>ObjC: flowDidFinish() (cleanup)
ObjC-->>JS: resolve({type:"success"}) or resolve({type:"cancel"/"dismiss"})
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 1
🧹 Nitpick comments (2)
patches/react-native-inappbrowser-reborn+3.7.0.patch (2)
191-206: Consider more robust redirect URL parsing.The current logic attempts to extract the scheme from a full URL but could fail silently if the URL is malformed. Consider validating the extracted scheme format.
NSString *escapedRedirectURL = nil; if (redirectURL) { // Try to parse as a full URL first NSURL *redirectUrl = [[NSURL alloc] initWithString:redirectURL]; if (redirectUrl && redirectUrl.scheme) { // It's a full URL, extract the scheme escapedRedirectURL = redirectUrl.scheme; NSLog(@"[RNInAppBrowser] Extracted callback URL scheme from full URL: %@", escapedRedirectURL); } else { // It's probably just a scheme already (e.g., "com.warroom.proofofpassport") escapedRedirectURL = redirectURL; NSLog(@"[RNInAppBrowser] Using redirect URL as scheme directly: %@", escapedRedirectURL); } + + // Validate scheme format + if (escapedRedirectURL && ![escapedRedirectURL containsString:@"://"]) { + NSLog(@"[RNInAppBrowser] Validated callback URL scheme: %@", escapedRedirectURL); + } } else { NSLog(@"[RNInAppBrowser] WARNING: No redirect URL provided!"); }
793-811: Replace deprecated keyWindow with proper fallback.The implementation correctly searches for active scenes, but the fallback to
keyWindowis deprecated in iOS 13+.// Fallback to keyWindow NSLog(@"[RNInAppBrowser] No active scene found, using keyWindow"); - return [UIApplication sharedApplication].keyWindow; + // Use the first window from connected scenes as fallback + for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) { + if ([scene isKindOfClass:[UIWindowScene class]]) { + UIWindowScene *windowScene = (UIWindowScene *)scene; + if (windowScene.windows.firstObject) { + return windowScene.windows.firstObject; + } + } + } + // Last resort: keyWindow (deprecated but handles edge cases) + return [UIApplication sharedApplication].keyWindow; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
patches/@turnkey+core+1.7.0.patch(4 hunks)patches/react-native-inappbrowser-reborn+3.7.0.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-10T14:47:40.945Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1041
File: app/src/providers/passportDataProvider.tsx:297-301
Timestamp: 2025-09-10T14:47:40.945Z
Learning: In app/src/providers/passportDataProvider.tsx: The deleteDocumentDirectlyFromKeychain function is a low-level utility used by the DocumentsAdapter and should not include error handling since callers like deleteDocument() already implement appropriate try/catch with logging for Keychain operations.
Applied to files:
patches/@turnkey+core+1.7.0.patch
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: type-check
🔇 Additional comments (4)
patches/@Turnkey+core+1.7.0.patch (2)
30-31: Correct implementation of the filter.The loop now iterates over
filteredPublicKeysinstead ofpublicKeys, ensuring only validated Turnkey keys are deleted.
9-22: Approve with caution — verify stored public‑key encoding (128‑char / 0x cases)Patch correctly limits deletions to likely Turnkey P‑256 keys (66 = compressed, 130 = uncompressed with 0x04). However code/tests also use raw x/y coords (64+64 = 128 hex) and keys may appear with a "0x" prefix. Confirm protocol_store.public_keys / Turnkey API always returns 66 or 130‑char hex (no 128‑char or "0x" variants). If not, either normalize (strip "0x") or extend the filter to accept 128‑char hex before deleting keys.
Patch location: patches/@Turnkey+core+1.7.0.patch -> node_modules/@turnkey/core/dist/clients/core.js(.mjs).
patches/react-native-inappbrowser-reborn+3.7.0.patch (2)
510-575: App may start auth session unexpectedly when backgrounded during scene wait.The 5-second retry loop uses
dispatch_afterto poll for an active scene, but lacks any lifecycle listeners to detect when the app enters the background. If the user switches apps during the wait period, the auth session could still start when the app returns to foreground, triggering an unexpected modal or session resumption.Add a cancel mechanism that detects
applicationDidEnterBackgroundand stops the retry attempts, or move the scene-wait logic intoapplicationDidBecomeActiveto ensure it only runs when appropriate.
167-189: Verify error handling strategy for auth completion handler simplification.The completion handler treats all errors (user cancellation, network failures, authentication failures) as a single "cancel" type, sacrificing error granularity. While the extensive logging captures details for debugging, the JavaScript layer cannot distinguish between different failure modes.
Confirm whether this simplified "success"/"cancel" response contract is intentional and sufficient for your auth flow requirements. If the caller code needs to differentiate between user cancellations and actual authentication failures, you may want to preserve error type specificity instead of collapsing all errors into one category.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
patches/react-native-inappbrowser-reborn+3.7.0.patch (2)
184-196: Clean up redirect URL parsing logic.The redirect URL parsing has an empty else block and lacks validation. Consider:
- Remove the empty else block
- Add validation for the extracted/provided scheme
- Log a warning if scheme extraction fails
NSString *escapedRedirectURL = nil; if (redirectURL) { // Try to parse as a full URL first NSURL *redirectUrl = [[NSURL alloc] initWithString:redirectURL]; if (redirectUrl && redirectUrl.scheme) { // It's a full URL, extract the scheme escapedRedirectURL = redirectUrl.scheme; } else { // It's probably just a scheme already (e.g., "com.warroom.proofofpassport") escapedRedirectURL = redirectURL; } - } else { }
494-529: Consider using scene lifecycle notifications instead of polling.The current polling approach (0.1s intervals, 50 attempts) works but could be more efficient and reliable. Using
UISceneDidActivateNotificationwould eliminate the need for polling and provide immediate response when the scene becomes active.If the polling timeout expires without finding an active scene, the auth session still starts (line 524), which may lead to the same timing issues this code attempts to solve. Consider either:
- Rejecting the promise if no active scene is found after timeout
- Using
NSNotificationCenterto observeUISceneDidActivateNotification+ // Add observer in init or when needed + [[NSNotificationCenter defaultCenter] addObserverForName:UISceneDidActivateNotification + object:nil + queue:nil + usingBlock:^(NSNotification *note) { + [self startAuthSession]; + }];
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.cursorignore(0 hunks)patches/react-native-inappbrowser-reborn+3.7.0.patch(1 hunks)
💤 Files with no reviewable changes (1)
- .cursorignore
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Test, build, and deploy scripts (`yarn test`, `yarn ios`, `yarn test:e2e:ios`, Fastlane, etc.) must be used for automation.
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/**/*.{ios,android,web}.{ts,tsx,js,jsx} : Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g., `***-***-1234` for passport numbers, `J*** D***` for names).
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Address CodeRabbitAI feedback and resolve security warnings during review
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
Applied to files:
patches/react-native-inappbrowser-reborn+3.7.0.patch
| + - (UIWindow *)presentationAnchorForWebAuthenticationSession:(ASWebAuthenticationSession *)session API_AVAILABLE(ios(13.0)) | ||
| + { | ||
| + // Search for an active foreground scene | ||
| + for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) { | ||
| + if (scene.activationState == UISceneActivationStateForegroundActive && | ||
| + [scene isKindOfClass:[UIWindowScene class]]) { | ||
| + UIWindowScene *windowScene = (UIWindowScene *)scene; | ||
| + UIWindow *window = windowScene.keyWindow ?: windowScene.windows.firstObject; | ||
| + if (window) { | ||
| + return window; | ||
| + } | ||
| + } | ||
| + } | ||
| + | ||
| + // Fallback to keyWindow | ||
| + return [UIApplication sharedApplication].keyWindow; | ||
| } |
There was a problem hiding this comment.
Replace deprecated keyWindow fallback.
The fallback to keyWindow at line 785 uses a deprecated API (iOS 13+). Since this method is only called on iOS 13+, the fallback should use scene-based window access.
- (UIWindow *)presentationAnchorForWebAuthenticationSession:(ASWebAuthenticationSession *)session API_AVAILABLE(ios(13.0))
{
// Search for an active foreground scene
for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) {
if (scene.activationState == UISceneActivationStateForegroundActive &&
[scene isKindOfClass:[UIWindowScene class]]) {
UIWindowScene *windowScene = (UIWindowScene *)scene;
UIWindow *window = windowScene.keyWindow ?: windowScene.windows.firstObject;
if (window) {
return window;
}
}
}
- // Fallback to keyWindow
- return [UIApplication sharedApplication].keyWindow;
+ // Fallback: return first available window from any scene
+ for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) {
+ if ([scene isKindOfClass:[UIWindowScene class]]) {
+ UIWindowScene *windowScene = (UIWindowScene *)scene;
+ if (windowScene.windows.firstObject) {
+ return windowScene.windows.firstObject;
+ }
+ }
+ }
+ return nil;
}📝 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.
| + - (UIWindow *)presentationAnchorForWebAuthenticationSession:(ASWebAuthenticationSession *)session API_AVAILABLE(ios(13.0)) | |
| + { | |
| + // Search for an active foreground scene | |
| + for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) { | |
| + if (scene.activationState == UISceneActivationStateForegroundActive && | |
| + [scene isKindOfClass:[UIWindowScene class]]) { | |
| + UIWindowScene *windowScene = (UIWindowScene *)scene; | |
| + UIWindow *window = windowScene.keyWindow ?: windowScene.windows.firstObject; | |
| + if (window) { | |
| + return window; | |
| + } | |
| + } | |
| + } | |
| + | |
| + // Fallback to keyWindow | |
| + return [UIApplication sharedApplication].keyWindow; | |
| } | |
| - (UIWindow *)presentationAnchorForWebAuthenticationSession:(ASWebAuthenticationSession *)session API_AVAILABLE(ios(13.0)) | |
| { | |
| // Search for an active foreground scene | |
| for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) { | |
| if (scene.activationState == UISceneActivationStateForegroundActive && | |
| [scene isKindOfClass:[UIWindowScene class]]) { | |
| UIWindowScene *windowScene = (UIWindowScene *)scene; | |
| UIWindow *window = windowScene.keyWindow ?: windowScene.windows.firstObject; | |
| if (window) { | |
| return window; | |
| } | |
| } | |
| } | |
| // Fallback: return first available window from any scene | |
| for (UIScene *scene in [UIApplication sharedApplication].connectedScenes) { | |
| if ([scene isKindOfClass:[UIWindowScene class]]) { | |
| UIWindowScene *windowScene = (UIWindowScene *)scene; | |
| if (windowScene.windows.firstObject) { | |
| return windowScene.windows.firstObject; | |
| } | |
| } | |
| } | |
| return nil; | |
| } |
🤖 Prompt for AI Agents
In patches/react-native-inappbrowser-reborn+3.7.0.patch around lines 770 to 786,
the fallback uses the deprecated [UIApplication sharedApplication].keyWindow;
replace that with scene-based window discovery. After the existing loop for a
foregroundActive UIWindowScene, add a second pass that finds any UIWindowScene
in connectedScenes and returns its windows.firstObject (or keyWindow equivalent
from that scene), and only return nil (or handle absence) if no scene window is
found — do not call the deprecated keyWindow API.
transphorm
changed the title
transphorm
changed the title
Summary by CodeRabbit
Bug Fixes
New Features
Chores