feat: open Custom OAuth in external browser to support WebAuthn/passkeys #6987
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import React from 'react'; | ||
| import { AppState, Dimensions, type EmitterSubscription, Linking, type AppStateStatus } from 'react-native'; | ||
| import { GestureHandlerRootView } from 'react-native-gesture-handler'; | ||
| import { SafeAreaProvider } from 'react-native-safe-area-context'; | ||
| import { enableScreens } from 'react-native-screens'; | ||
|
|
@@ -61,6 +61,25 @@ interface IState { | |
|
|
||
| const parseDeepLinking = (url: string) => { | ||
| if (url) { | ||
| // Handle OAuth redirect from external browser (rocketchat://auth?...) | ||
| // The Rocket.Chat server redirects to 'rocketchat://auth' after OAuth login | ||
| // when using the external browser flow (required for WebAuthn/passkeys) | ||
| if (url.startsWith('rocketchat://auth')) { | ||
| const authUrl = url.replace(/rocketchat:\/\//, ''); | ||
| const authMatch = authUrl.match(/^auth\?(.+)/); | ||
| if (authMatch) { | ||
| const query = authMatch[1]; | ||
| const parsedQuery = parseQuery(query); | ||
| if (parsedQuery?.credentialToken) { | ||
| return { | ||
| ...parsedQuery, | ||
| type: 'oauth' | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Handle standard deep links (rocketchat:// and https://go.rocket.chat/) | ||
| url = url.replace(/rocketchat:\/\/|https:\/\/go.rocket.chat\//, ''); | ||
| const regex = /^(room|auth|invite|shareextension)\?/; | ||
| const match = url.match(regex); | ||
|
|
@@ -83,9 +102,10 @@ const parseDeepLinking = (url: string) => { | |
| }; | ||
|
|
||
| export default class Root extends React.Component<{}, IState> { | ||
| private dimensionsListener?: EmitterSubscription; | ||
| private videoConfActionCleanup?: () => void; | ||
| private appStateSubscription?: ReturnType<typeof AppState.addEventListener>; | ||
| private lastAppState: AppStateStatus = AppState.currentState; | ||
|
|
||
| constructor(props: any) { | ||
| super(props); | ||
|
|
@@ -108,28 +128,51 @@ export default class Root extends React.Component<{}, IState> { | |
| } | ||
|
|
||
| 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)); | ||
| } | ||
| }); | ||
|
Comment on lines
+133
to
+138
There was a problem hiding this comment. The event listener returned by |
||
|
|
||
| // 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); | ||
|
|
||
| // Set up video conf action listener for background accept/decline | ||
| this.videoConfActionCleanup = setupVideoConfActionListener(); | ||
| } | ||
|
|
||
| componentWillUnmount() { | ||
| this.dimensionsListener?.remove?.(); | ||
| this.appStateSubscription?.remove?.(); | ||
| this.videoConfActionCleanup?.(); | ||
|
|
||
| unsubscribeTheme(); | ||
| } | ||
|
|
||
| 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; | ||
| }; | ||
|
Comment on lines
+158
to
+174
There was a problem hiding this comment.
Comment on lines
+158
to
+174
Contributor
There was a problem hiding this comment.
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 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 |
||
|
|
||
| init = async () => { | ||
| store.dispatch(appInitLocalSettings()); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -96,9 +96,68 @@ const fallbackNavigation = function* fallbackNavigation() { | |||||
| }; | ||||||
|
|
||||||
| const handleOAuth = function* handleOAuth({ params }) { | ||||||
| const { credentialToken, credentialSecret, host } = params; | ||||||
| try { | ||||||
| // When OAuth completes via external browser redirect, the SDK connection | ||||||
| // may not be ready yet. We need to ensure the server is connected before | ||||||
| // attempting to complete the login. | ||||||
| let server = host; | ||||||
| if (server && server.endsWith('/')) { | ||||||
| server = server.slice(0, -1); | ||||||
| } | ||||||
| if (!server) { | ||||||
| server = UserPreferences.getString(CURRENT_SERVER); | ||||||
| } | ||||||
|
|
||||||
| if (!server) { | ||||||
| yield put(appInit()); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Check if SDK is connected to this server and the WebSocket is ready | ||||||
| const sdkHost = sdk.current?.client?.host; | ||||||
| const meteorConnected = yield select(state => state.meteor.connected); | ||||||
|
|
||||||
| if (!meteorConnected || !sdkHost || sdkHost !== server) { | ||||||
| const serverRecord = yield getServerById(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(selectServerRequest(server, result.version)); | ||||||
|
Comment on lines
+128
to
+131
Contributor
There was a problem hiding this comment. 🧩 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 -A5Repository: 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
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 |
||||||
| } else { | ||||||
| yield put(selectServerRequest(server, serverRecord.version)); | ||||||
| } | ||||||
| // Wait for the WebSocket connection to be fully ready | ||||||
| yield take(types.METEOR.SUCCESS); | ||||||
| } | ||||||
|
Comment on lines
+135
to
+137
Contributor
There was a problem hiding this comment. No timeout on If the WebSocket connection never succeeds (server unreachable, DNS failure, etc.), this 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 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| // Retry logic for OAuth login - the external browser flow can have timing | ||||||
| // issues where the SDK is not fully ready even after METEOR.SUCCESS | ||||||
| const maxRetries = 3; | ||||||
| let lastError; | ||||||
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | ||||||
| try { | ||||||
| const delayMs = attempt === 1 ? 500 : 1000 * attempt; | ||||||
|
There was a problem hiding this comment. 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
Suggested change
|
||||||
| yield delay(delayMs); | ||||||
| yield loginOAuthOrSso({ oauth: { credentialToken, credentialSecret } }, false); | ||||||
| return; | ||||||
| } catch (e) { | ||||||
| lastError = e; | ||||||
| const isNetworkError = e?.message === 'Network request failed' || e?.message?.includes('network'); | ||||||
| if (attempt < maxRetries && isNetworkError) { | ||||||
| continue; | ||||||
| } | ||||||
| throw e; | ||||||
| } | ||||||
|
Comment on lines
+149
to
+156
Contributor
There was a problem hiding this comment. Case-sensitive network error check may miss retries for valid errors.
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 |
||||||
| } | ||||||
| if (lastError) { | ||||||
| throw lastError; | ||||||
| } | ||||||
|
Comment on lines
+158
to
+160
There was a problem hiding this comment. 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. |
||||||
| } catch (e) { | ||||||
| log(e); | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
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://authbut lacks acredentialTokenparameter, it falls through to the standard deep link parsing at line 83, where it will match theauthpattern in the regex. This could lead to unexpected behavior where an invalid OAuth redirect is treated as a standard auth deep link. Consider returningnullexplicitly after line 79 if the URL starts withrocketchat://authbut doesn't have a valid credentialToken, or add additional validation to prevent this fallthrough scenario.