Skip to content

feat: open Custom OAuth in external browser to support WebAuthn/passkeys#6987

Open
machadolucas wants to merge 3 commits intoRocketChat:developfrom
machadolucas:feat/external-browser-oauth-webauthn
Open

feat: open Custom OAuth in external browser to support WebAuthn/passkeys#6987
machadolucas wants to merge 3 commits intoRocketChat:developfrom
machadolucas:feat/external-browser-oauth-webauthn

Conversation

@machadolucas
Copy link

@machadolucas machadolucas commented Feb 15, 2026

Proposed changes

This PR enables WebAuthn/passkey authentication for Custom OAuth providers (e.g., Keycloak) by opening the OAuth flow in the device's native browser instead of the in-app WebView.

The problem: Android WebView (and in some cases iOS WKWebView) does not support the WebAuthn API. When users attempt to authenticate via an OAuth provider that requires WebAuthn (security keys, passkeys, biometric authenticators), the authentication fails with errors like "Failed to authenticate by the Security Key." This affects any Rocket.Chat deployment using an identity provider with WebAuthn/passkey enforcement.

The solution: This PR makes three coordinated changes:

  1. app/containers/LoginServices/serviceLogin.ts — Changes onPressCustomOAuth to open the OAuth URL in the device's native browser via Linking.openURL() instead of the in-app WebView (AuthenticationWebView). The login style is changed from 'popup' to 'redirect' so the Rocket.Chat server generates a rocketchat://auth redirect URL that brings the user back to the app after authentication. This mirrors the existing pattern already used by onPressGoogle, which already uses Linking.openURL() with the 'redirect' login style.

  2. app/index.tsx — Adds early handling in parseDeepLinking for rocketchat://auth?credentialToken=...&credentialSecret=... URLs that arrive from the external browser redirect, correctly identifying them as OAuth callbacks (setting type: 'oauth'). Also removes the 5-second setTimeout delay on the deep link listener so OAuth redirects from the external browser are processed immediately, and adds an AppState foreground listener to catch pending OAuth deep links on iOS (where Safari redirects may arrive while the app is transitioning from background to foreground).

  3. app/sagas/deepLinking.js — Enhances handleOAuth to be resilient to the external browser flow where the app may have been backgrounded and lost its WebSocket connection. Before attempting to complete the OAuth login, it now verifies that the SDK is connected to the correct server, re-establishes the connection if needed (looking up the server record, calling selectServerRequest, and waiting for METEOR.SUCCESS), and retries the login call up to 3 times with backoff for transient network errors.

Why this approach: The native browser has full access to the operating system's security APIs (WebAuthn, FIDO2, platform authenticators), while WebViews run in a sandboxed context without these capabilities. This is the same approach used by most OAuth-integrated mobile applications (including how Rocket.Chat already handles Google OAuth). The rocketchat://auth redirect scheme is already defined in the Rocket.Chat server codebase (see apps/meteor/app/google-oauth/server/index.js) and is already registered as a URL scheme in both the iOS and Android app configurations.

Issue(s)

Closes #5681

How to test or reproduce

Prerequisites

  • A Rocket.Chat server configured with a Custom OAuth provider (e.g., Keycloak, Authentik, or similar) that has WebAuthn/passkey authentication enabled
  • The OAuth provider must be configured to allow the Rocket.Chat server's /_oauth/<service> redirect URI
  • A mobile device (Android or iOS) with a passkey/security key registered in the OAuth provider

Steps to reproduce the original bug (before this PR)

  1. Open the Rocket.Chat mobile app
  2. Tap the Custom OAuth login button (e.g., "Login with Keycloak")
  3. The in-app WebView opens the OAuth provider's login page
  4. Attempt to authenticate using a passkey or security key
  5. Expected: Authentication succeeds
  6. Actual: Authentication fails with "Failed to authenticate by the Security Key" because the WebView does not support WebAuthn

Steps to test the fix (after this PR)

  1. Open the Rocket.Chat mobile app
  2. Tap the Custom OAuth login button (e.g., "Login with Keycloak")
  3. The device's native browser (Chrome on Android, Safari on iOS) opens the OAuth provider's login page
  4. Authenticate using a passkey, security key, or any other WebAuthn method
  5. After successful authentication, the OAuth provider redirects through the Rocket.Chat server, which generates a rocketchat://auth redirect
  6. The app opens automatically and the user is logged in
  7. Verify the app navigates to the home screen with the user fully authenticated

Additional test scenarios

  • Cold start: Kill the app, then tap the OAuth login from a deep link — verify the app establishes the server connection before completing OAuth
  • Network instability: Test with a flaky network connection — verify the retry logic handles transient failures gracefully
  • iOS background: After tapping the OAuth button and authenticating in Safari, verify the app correctly handles the redirect even if there is a brief period where the app is in the background state
  • Existing OAuth flows: Verify that Google OAuth (which already uses Linking.openURL) continues to work correctly
  • Non-OAuth deep links: Verify that standard deep links (rocketchat://room?..., https://go.rocket.chat/...) continue to work as before

Screenshots

No visual UI changes — the only difference is that the OAuth page opens in the native browser instead of the in-app WebView.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Design decisions

Why only Custom OAuth and not all OAuth providers?
This PR specifically targets onPressCustomOAuth because that is the code path used by identity providers like Keycloak, Authentik, and other Custom OAuth services where WebAuthn is commonly enforced. The built-in providers (Facebook, GitHub, GitLab, LinkedIn, etc.) continue to use the in-app WebView because they don't typically require WebAuthn and the popup flow provides a smoother UX. Google OAuth already uses Linking.openURL with the redirect flow. If the maintainers prefer, this approach could be extended to all OAuth providers in a follow-up PR.

Why the retry logic in handleOAuth?
When the OAuth flow completes in the external browser and the app is reopened via the rocketchat://auth deep link, there is a race condition: the app may still be re-establishing its WebSocket connection to the Rocket.Chat server. The original handleOAuth assumed the SDK was already connected (which was true for the in-app WebView flow, since the WebView was a child view and the app stayed in the foreground). With the external browser flow, we need to wait for the connection and handle transient network errors during the transition.

Why remove the 5-second deep link listener delay?
The original 5-second setTimeout delay before registering the deep link listener was likely intended to avoid processing deep links before the app was fully initialized. However, this delay means OAuth redirects from the external browser that arrive within the first 5 seconds are silently dropped. Since the deep linking saga already handles initialization states gracefully (checking server connection, waiting for login, etc.), the delay is unnecessary and harmful for the external browser OAuth flow.

Backward compatibility:
This change is fully backward compatible. The rocketchat://auth redirect URL and URL scheme are already part of the existing codebase. The enhanced handleOAuth saga gracefully falls back to the stored current server if no host is provided in the OAuth callback params. Non-OAuth deep links are unaffected as the new rocketchat://auth handling in parseDeepLinking is checked first and only matches URLs with a credentialToken parameter.

Summary by CodeRabbit

  • New Features

    • OAuth authentication now opens in your default browser instead of within the app
    • Added support for WebAuthn/passkeys authentication
  • Bug Fixes

    • Enhanced retry logic for OAuth sign-in with automatic recovery on network failures
    • Improved app state handling to properly process OAuth redirects when returning from background

machadolucas and others added 3 commits February 15, 2026 16:50
Android WebView does not support WebAuthn, preventing users from
authenticating with security keys/passkeys through OAuth providers
like Keycloak. This changes Custom OAuth to use the device's native
browser (via Linking.openURL) instead of the in-app WebView, matching
the existing behavior for Google OAuth.

The login style is changed from 'popup' to 'redirect' so the
Rocket.Chat server generates a rocketchat://auth redirect URL that
brings the user back to the app after authentication.

Closes RocketChat#5681

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When OAuth authentication completes in the external browser, the
Rocket.Chat server redirects to rocketchat://auth with credential
tokens. This adds handling for that redirect URL in parseDeepLinking,
setting type to 'oauth' so the deep linking saga can complete the
login flow.

Also removes the 5-second delay on the deep link listener so OAuth
redirects are processed immediately, and adds an AppState listener
to catch pending OAuth deep links when the app returns to foreground
(needed on iOS where Safari redirects may arrive while app is
backgrounded).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the app receives an OAuth callback from the external browser,
the SDK WebSocket connection may not be ready yet. This updates
handleOAuth to verify the server connection, establish it if needed
(waiting for METEOR.SUCCESS), and retry the login with backoff on
transient network errors.

This is necessary because unlike the in-app WebView flow where the
SDK stays connected, the external browser flow may cause the app to
be backgrounded and lose its WebSocket connection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 15, 2026 15:03
@CLAassistant
Copy link

CLAassistant commented Feb 15, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

Walkthrough

These changes refactor the OAuth authentication flow to use an external browser instead of an in-app WebView, enabling WebAuthn/passkey support. The implementation includes deep linking to capture OAuth redirects via custom URL schemes, app state lifecycle management to handle pending authentication when the app returns to foreground, and server synchronization with retry logic to ensure the client-server connection is established before proceeding with login.

Changes

Cohort / File(s) Summary
OAuth Browser Integration
app/containers/LoginServices/serviceLogin.ts
Changed OAuth flow to open external browser via Linking.openURL instead of in-app WebView; updated OAuth state generation to use 'redirect' login style; added comments clarifying WebAuthn/passkey support rationale.
Deep Linking & App State Management
app/index.tsx
Extended parseDeepLinking to extract credentialToken from rocketchat://auth OAuth redirects; introduced AppState lifecycle tracking to handle pending OAuth callbacks when app returns to foreground; moved deep link listener initialization from delayed timeout to immediate setup in componentDidMount.
Server Synchronization & Retry Logic
app/sagas/deepLinking.js
Expanded handleOAuth to verify server readiness and SDK connectivity before login; added Meteor WebSocket synchronization checks with fallback server initialization; introduced retry loop for loginOAuthOrSso with up to 3 attempts and exponential-like delays (500ms, increasing) to handle transient network failures.

Sequence Diagram

sequenceDiagram
    participant App as App (Foreground)
    participant Browser as External Browser
    participant DeepLink as Deep Linking Handler
    participant Server as Rocket.Chat Server
    participant SDK as SDK/Meteor
    participant Auth as OAuth Saga
    
    App->>Browser: Open OAuth URL (Linking.openURL)<br/>with state parameter
    Note over Browser: User authenticates &<br/>completes WebAuthn
    Browser->>App: Redirect via rocketchat://auth<br/>with credentialToken
    
    App->>DeepLink: Handle deep link (background or<br/>returning to foreground)
    DeepLink->>Auth: Dispatch deepLinkingOpen<br/>with credentialToken
    
    Auth->>SDK: Check Meteor WebSocket<br/>connection status
    alt WebSocket Not Ready
        Auth->>Server: Fetch/initialize server record
        Auth->>SDK: Wait for METEOR.SUCCESS
    end
    
    Auth->>Auth: Retry loop (up to 3 times)<br/>with exponential delays
    Auth->>Server: loginOAuthOrSso(credentialToken)
    alt Login Success
        Server->>Auth: Return session token
        Auth->>App: Update state, complete login
    else Network/Timing Error
        Auth->>Auth: Retry with increased delay
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 OAuth flows now leap through external skies,
WebAuthn passkeys dance where browsers thrive,
Deep links catch the redirects' gentle calls,
Server states align before we stall,
Retry loops ensure our walls stay strong! 🔐

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: open Custom OAuth in external browser to support WebAuthn/passkeys' clearly and concisely summarizes the main change: opening OAuth in external browser instead of WebView to enable WebAuthn support.
Linked Issues check ✅ Passed The changes comprehensively address issue #5681 by opening OAuth in external browser, implementing redirect-based flow with rocketchat://auth callbacks, handling app lifecycle/connectivity edge cases, and maintaining backward compatibility.
Out of Scope Changes check ✅ Passed All changes are directly related to the PR objective: serviceLogin.ts switches to Linking.openURL with redirect style, index.tsx adds deep link parsing for OAuth and AppState handling, and deepLinking.js adds server verification and retry logic for OAuth completion.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into develop

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables WebAuthn/passkey authentication for Custom OAuth providers by opening the OAuth flow in the device's native browser instead of an in-app WebView. The change addresses a critical limitation where Android WebView and some iOS WKWebView implementations don't support the WebAuthn API, preventing users from authenticating with security keys, passkeys, or biometric authenticators when using Custom OAuth providers like Keycloak.

Changes:

  • Modified Custom OAuth login to use external browser with redirect flow (matching existing Google OAuth pattern)
  • Added immediate deep link handling for OAuth redirects from external browser
  • Enhanced OAuth saga with connection verification and retry logic for external browser flow

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
app/containers/LoginServices/serviceLogin.ts Changed onPressCustomOAuth to use Linking.openURL() with 'redirect' login style instead of in-app WebView
app/index.tsx Added early parsing for rocketchat://auth OAuth callbacks, removed setTimeout delay on deep link listener, added AppState listener for iOS Safari redirects
app/sagas/deepLinking.js Enhanced handleOAuth with SDK connection verification, server re-connection logic, and retry mechanism for external browser OAuth flow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

type: 'oauth'
};
}
}
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a URL matches rocketchat://auth but lacks a credentialToken parameter, it falls through to the standard deep link parsing at line 83, where it will match the auth pattern in the regex. This could lead to unexpected behavior where an invalid OAuth redirect is treated as a standard auth deep link. Consider returning null explicitly after line 79 if the URL starts with rocketchat://auth but doesn't have a valid credentialToken, or add additional validation to prevent this fallthrough scenario.

Suggested change
}
}
// If the URL is an OAuth redirect but lacks a valid credentialToken,
// treat it as invalid and do not fall through to the standard deep link parsing.
return null;

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +138
Linking.addEventListener('url', ({ url }) => {
const parsedDeepLinkingURL = parseDeepLinking(url);
if (parsedDeepLinkingURL) {
store.dispatch(deepLinkingOpen(parsedDeepLinkingURL));
}
});
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event listener returned by Linking.addEventListener is not stored or cleaned up. This creates a memory leak because the listener will persist even after the component unmounts. Store the listener in a class property (similar to dimensionsListener and appStateSubscription) and remove it in componentWillUnmount.

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +174
handleAppStateChange = async (nextAppState: AppStateStatus) => {
// When app comes to foreground from background, check for pending deep links
if (this.lastAppState.match(/inactive|background/) && nextAppState === 'active') {
try {
const url = await Linking.getInitialURL();
if (url && url.startsWith('rocketchat://auth')) {
const parsedDeepLinkingURL = parseDeepLinking(url);
if (parsedDeepLinkingURL) {
store.dispatch(deepLinkingOpen(parsedDeepLinkingURL));
}
}
} catch (e) {
// Ignore errors checking for pending deep links
}
}
this.lastAppState = nextAppState;
};
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking.getInitialURL() returns the URL that launched the app on cold start, not the URL from a redirect that occurred while the app was backgrounded. When the app is brought to the foreground from a redirect, the URL event listener (line 133) should already receive the deep link. This logic will always return either null or the original launch URL, making it ineffective for capturing OAuth redirects from Safari. Consider removing this AppState listener entirely, as the URL event listener should handle all incoming deep links including OAuth redirects.

Copilot uses AI. Check for mistakes.
Comment on lines +158 to +160
if (lastError) {
throw lastError;
}
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is unreachable. The for loop at line 143 will either return successfully (line 148), throw an error for non-network errors or the final retry (line 155), or continue to the next iteration. Since the loop always either returns or throws by the time it completes all iterations (attempt 3 throws at line 155), execution can never reach lines 158-160. Remove this dead code.

Copilot uses AI. Check for mistakes.
let lastError;
for (let attempt = 1; attempt <= maxRetries; attempt++) {
try {
const delayMs = attempt === 1 ? 500 : 1000 * attempt;
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry delays are inconsistent: attempt 1 delays 500ms, attempt 2 delays 2000ms (1000 * 2), and attempt 3 delays 3000ms (1000 * 3). This creates an unexpected pattern where the first retry has a much shorter delay than subsequent ones. Consider using a consistent exponential backoff pattern, such as 500 * attempt or 500 * Math.pow(2, attempt - 1), to provide more predictable behavior.

Suggested change
const delayMs = attempt === 1 ? 500 : 1000 * attempt;
const delayMs = 500 * Math.pow(2, attempt - 1);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@app/index.tsx`:
- Around line 158-174: The AppState handler handleAppStateChange incorrectly
uses Linking.getInitialURL (which returns the cold-start URL) and causes
duplicate deep link dispatches via deepLinkingOpen; remove the
handleAppStateChange logic (or at minimum the getInitialURL call and its
dispatch) and rely on the existing 'url' event listener that already handles
background-to-foreground deep links, or implement a consumed flag to ensure
OAuth links processed by the 'url' listener are not re-dispatched — update
references to this.lastAppState accordingly and remove any unused handler
registration/unregistration for handleAppStateChange.

In `@app/sagas/deepLinking.js`:
- Around line 149-156: The network-error detection in the catch block sets
isNetworkError using a case-sensitive includes check; change it to perform a
case-insensitive check by normalizing the error message (e.g., toLowerCase())
before calling includes so messages like "Network timeout" or "Network Error"
are detected; update the isNetworkError expression that references e?.message,
leaving the surrounding retry logic (attempt, maxRetries, continue/throw) and
lastError assignment unchanged.
- Around line 135-137: The yield take(types.METEOR.SUCCESS) in the deepLinking
saga can block forever; change it to a race between take(types.METEOR.SUCCESS)
and a timeout so the saga can continue or surface an error if the connection
never succeeds. Import race (and delay or use a configurable TIMEOUT constant)
from redux-saga/effects, wrap the existing take(types.METEOR.SUCCESS) in race,
handle the timeout branch by logging/dispatching a failure or fallback, and keep
the success branch behavior unchanged (refer to the METEOR.SUCCESS wait in
deepLinking).
- Around line 128-131: Read and store the current/previous server value before
attempting the OAuth connection and pass that stored previous server into
serverInitAdd rather than the new OAuth host; specifically, ensure you capture
the currently-active server (the value used as the CURRENT_SERVER fallback) into
a variable (e.g., previousServer) before the connection attempt and call yield
put(serverInitAdd(previousServer)) while still calling yield
put(selectServerRequest(server, result.version)) with the new OAuth server/host.
🧹 Nitpick comments (2)
app/sagas/deepLinking.js (1)

141-160: Lines 158–160 are unreachable dead code.

The for loop either returns on success (line 148) or throws on the last attempt / non-network error (line 155). It can never exit normally, so the if (lastError) throw lastError block is dead code.

Proposed cleanup
 		}
-		if (lastError) {
-			throw lastError;
-		}
 	} catch (e) {
app/index.tsx (1)

130-138: Missing cleanup for the Linking 'url' event listener.

Linking.addEventListener returns a subscription object, but it's not stored or removed in componentWillUnmount. Since Root is the top-level component, this is unlikely to cause issues in practice, but for correctness it should be cleaned up alongside the other subscriptions.

Proposed fix

Add a class field:

+	private linkingSubscription?: EmitterSubscription;

Store and clean up the subscription:

-		Linking.addEventListener('url', ({ url }) => {
+		this.linkingSubscription = Linking.addEventListener('url', ({ url }) => {
 	componentWillUnmount() {
+		this.linkingSubscription?.remove?.();
 		this.dimensionsListener?.remove?.();
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b93f25 and 3a79e75.

📒 Files selected for processing (3)
  • app/containers/LoginServices/serviceLogin.ts
  • app/index.tsx
  • app/sagas/deepLinking.js
🧰 Additional context used
🧬 Code graph analysis (2)
app/index.tsx (1)
app/actions/deepLinking.ts (1)
  • deepLinkingOpen (19-24)
app/sagas/deepLinking.js (7)
app/sagas/login.js (9)
  • params (432-432)
  • server (86-86)
  • server (247-247)
  • server (317-317)
  • server (393-393)
  • select (228-228)
  • serverRecord (53-53)
  • serverRecord (101-101)
  • result (75-75)
app/sagas/init.js (2)
  • server (26-26)
  • serverRecord (49-49)
app/sagas/encryption.js (2)
  • server (20-20)
  • server (93-93)
app/lib/constants/keys.ts (1)
  • CURRENT_SERVER (26-26)
app/actions/app.ts (1)
  • appInit (45-49)
app/lib/methods/getServerInfo.ts (1)
  • getServerInfo (53-137)
app/actions/server.ts (1)
  • serverInitAdd (104-109)
🔇 Additional comments (3)
app/containers/LoginServices/serviceLogin.ts (1)

97-112: LGTM! Consistent with the existing onPressGoogle redirect pattern.

The switch to getOAuthState('redirect') + Linking.openURL mirrors the Google OAuth flow (lines 53–55) and correctly leverages the rocketchat://auth redirect URI encoded in the state. This enables WebAuthn/passkey support by moving out of the WebView.

One minor note: Linking.openURL returns a Promise that can reject if no app can handle the URL. The same gap exists in onPressGoogle, so it's a pre-existing concern, but worth considering a .catch() for user feedback.

app/index.tsx (1)

62-80: LGTM! Early interception of rocketchat://auth OAuth redirects.

The early return correctly prevents the URL from falling through to the generic auth? matcher (line 84), which would set type to undefined (from parsedQuery?.type) instead of 'oauth'. The guard on parsedQuery?.credentialToken ensures only valid OAuth callbacks are routed.

app/sagas/deepLinking.js (1)

98-99: Verify that host is not actually passed through the OAuth deep link params.

The rocketchat://auth?credentialToken=...&credentialSecret=... redirect URL does not include a host parameter. The OAuth flow always falls back to CURRENT_SERVER (line 109). However, the server-readiness logic (lines 121–137) is not dead code—it's actively necessary. When OAuth completes via external browser redirect, the SDK may not be connected to the correct server yet, so the code checks if the meteor connection matches the required server and establishes it if needed before attempting login. This logic is essential for the OAuth external browser flow.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +158 to +174
handleAppStateChange = async (nextAppState: AppStateStatus) => {
// When app comes to foreground from background, check for pending deep links
if (this.lastAppState.match(/inactive|background/) && nextAppState === 'active') {
try {
const url = await Linking.getInitialURL();
if (url && url.startsWith('rocketchat://auth')) {
const parsedDeepLinkingURL = parseDeepLinking(url);
if (parsedDeepLinkingURL) {
store.dispatch(deepLinkingOpen(parsedDeepLinkingURL));
}
}
} catch (e) {
// Ignore errors checking for pending deep links
}
}
this.lastAppState = nextAppState;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Linking.getInitialURL() returns the cold-start URL, not pending background deep links — causes duplicate OAuth dispatches.

getInitialURL() returns the URL that launched the app (cold start). It does not return deep links received while the app is already running in the background — those arrive via the 'url' event listener (line 133). This means:

  1. If the app was cold-started by rocketchat://auth?..., getInitialURL() will return that same URL every foreground transition, dispatching deepLinkingOpen repeatedly.
  2. If the app was started normally, getInitialURL() returns null, so this handler does nothing — the actual OAuth redirect is already handled by the 'url' event listener.

Either way, this handler is either harmful (case 1) or a no-op (case 2).

On iOS, when a custom-scheme URL brings a backgrounded app to foreground, it fires the 'url' event, which is already handled at line 133. Consider removing this AppState listener entirely, or if there's a specific iOS edge case you've observed, track whether the OAuth link has already been processed (e.g., with a "consumed" flag).

Proposed fix — simplest approach: remove the AppState handler
-	private appStateSubscription?: ReturnType<typeof AppState.addEventListener>;
-	private lastAppState: AppStateStatus = AppState.currentState;

 	componentDidMount() {
 		// Set up deep link listener immediately (no delay) so OAuth redirects
 		// from external browser are handled promptly
 		Linking.addEventListener('url', ({ url }) => {
 			const parsedDeepLinkingURL = parseDeepLinking(url);
 			if (parsedDeepLinkingURL) {
 				store.dispatch(deepLinkingOpen(parsedDeepLinkingURL));
 			}
 		});
-
-		// Handle app returning to foreground - check for pending OAuth deep links
-		// This is needed on iOS where Safari redirects may arrive while app is backgrounded
-		this.appStateSubscription = AppState.addEventListener('change', this.handleAppStateChange);
-
 		this.dimensionsListener = Dimensions.addEventListener('change', this.onDimensionsChange);
 	componentWillUnmount() {
 		this.dimensionsListener?.remove?.();
-		this.appStateSubscription?.remove?.();
 		this.videoConfActionCleanup?.();
-	handleAppStateChange = async (nextAppState: AppStateStatus) => {
-		// When app comes to foreground from background, check for pending deep links
-		if (this.lastAppState.match(/inactive|background/) && nextAppState === 'active') {
-			try {
-				const url = await Linking.getInitialURL();
-				if (url && url.startsWith('rocketchat://auth')) {
-					const parsedDeepLinkingURL = parseDeepLinking(url);
-					if (parsedDeepLinkingURL) {
-						store.dispatch(deepLinkingOpen(parsedDeepLinkingURL));
-					}
-				}
-			} catch (e) {
-				// Ignore errors checking for pending deep links
-			}
-		}
-		this.lastAppState = nextAppState;
-	};
🤖 Prompt for AI Agents
In `@app/index.tsx` around lines 158 - 174, The AppState handler
handleAppStateChange incorrectly uses Linking.getInitialURL (which returns the
cold-start URL) and causes duplicate deep link dispatches via deepLinkingOpen;
remove the handleAppStateChange logic (or at minimum the getInitialURL call and
its dispatch) and rely on the existing 'url' event listener that already handles
background-to-foreground deep links, or implement a consumed flag to ensure
OAuth links processed by the 'url' listener are not re-dispatched — update
references to this.lastAppState accordingly and remove any unused handler
registration/unregistration for handleAppStateChange.

Comment on lines +128 to +131
return;
}
yield put(serverInitAdd(server));
yield put(selectServerRequest(server, result.version));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the serverInitAdd function definition and its usages
echo "=== Finding serverInitAdd definition and usages ==="
rg -n 'serverInitAdd' --type=js --type=ts -B2 -A5

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 5172


🏁 Script executed:

#!/bin/bash
# Check the specific file mentioned - app/actions/server.ts around line 103-108
echo "=== Checking app/actions/server.ts ==="
fd -t f 'server.ts' -x cat -n {}

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 23935


🏁 Script executed:

#!/bin/bash
# Check the deepLinking.js file context around lines 100-135
echo "=== Checking app/sagas/deepLinking.js context ==="
fd -t f 'deepLinking.js' -x cat -n {} | sed -n '100,135p'

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1555


Pass the current/previous server to serverInitAdd, not the new OAuth server.

serverInitAdd expects the previously-active server (see app/actions/server.ts line 104–108: serverInitAdd(previousServer: string)). At line 130, server (the OAuth host being connected to) is incorrectly passed. When server comes from the host parameter rather than the CURRENT_SERVER fallback, this stores the wrong value in Redux state. Read the current server separately before the connection attempt:

Proposed fix
+			const previousServer = UserPreferences.getString(CURRENT_SERVER) || '';
 			if (!serverRecord) {
 				// Server not in database yet, need to add it first
 				const result = yield getServerInfo(server);
 				if (!result.success) {
 					yield put(appInit());
 					return;
 				}
-				yield put(serverInitAdd(server));
+				yield put(serverInitAdd(previousServer));
 				yield put(selectServerRequest(server, result.version));
🤖 Prompt for AI Agents
In `@app/sagas/deepLinking.js` around lines 128 - 131, Read and store the
current/previous server value before attempting the OAuth connection and pass
that stored previous server into serverInitAdd rather than the new OAuth host;
specifically, ensure you capture the currently-active server (the value used as
the CURRENT_SERVER fallback) into a variable (e.g., previousServer) before the
connection attempt and call yield put(serverInitAdd(previousServer)) while still
calling yield put(selectServerRequest(server, result.version)) with the new
OAuth server/host.

Comment on lines +135 to +137
// Wait for the WebSocket connection to be fully ready
yield take(types.METEOR.SUCCESS);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

No timeout on yield take(METEOR.SUCCESS) — saga can hang indefinitely.

If the WebSocket connection never succeeds (server unreachable, DNS failure, etc.), this take will block forever, leaving the user stuck with no feedback. Consider using a race with a timeout:

Proposed fix
-			// Wait for the WebSocket connection to be fully ready
-			yield take(types.METEOR.SUCCESS);
+			// Wait for the WebSocket connection to be fully ready (with timeout)
+			const { timeout } = yield race({
+				success: take(types.METEOR.SUCCESS),
+				timeout: delay(15000)
+			});
+			if (timeout) {
+				log(new Error('Timeout waiting for Meteor connection during OAuth'));
+				yield put(appInit());
+				return;
+			}

You'll need to import race from redux-saga/effects (line 1).

🤖 Prompt for AI Agents
In `@app/sagas/deepLinking.js` around lines 135 - 137, The yield
take(types.METEOR.SUCCESS) in the deepLinking saga can block forever; change it
to a race between take(types.METEOR.SUCCESS) and a timeout so the saga can
continue or surface an error if the connection never succeeds. Import race (and
delay or use a configurable TIMEOUT constant) from redux-saga/effects, wrap the
existing take(types.METEOR.SUCCESS) in race, handle the timeout branch by
logging/dispatching a failure or fallback, and keep the success branch behavior
unchanged (refer to the METEOR.SUCCESS wait in deepLinking).

Comment on lines +149 to +156
} catch (e) {
lastError = e;
const isNetworkError = e?.message === 'Network request failed' || e?.message?.includes('network');
if (attempt < maxRetries && isNetworkError) {
continue;
}
throw e;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Case-sensitive network error check may miss retries for valid errors.

e?.message?.includes('network') is case-sensitive and won't match messages like "Network timeout" or "Network error". The first check (=== 'Network request failed') covers the most common case, but the fallback .includes should use a case-insensitive comparison.

Proposed fix
-				const isNetworkError = e?.message === 'Network request failed' || e?.message?.includes('network');
+				const isNetworkError = e?.message === 'Network request failed' || e?.message?.toLowerCase?.()?.includes?.('network');
🤖 Prompt for AI Agents
In `@app/sagas/deepLinking.js` around lines 149 - 156, The network-error detection
in the catch block sets isNetworkError using a case-sensitive includes check;
change it to perform a case-insensitive check by normalizing the error message
(e.g., toLowerCase()) before calling includes so messages like "Network timeout"
or "Network Error" are detected; update the isNetworkError expression that
references e?.message, leaving the surrounding retry logic (attempt, maxRetries,
continue/throw) and lastError assignment unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth login with Webauthn not working in Webview

2 participants