fix(security): harden Android notification receivers, token storage, and deep-link login#7447
fix(security): harden Android notification receivers, token storage, and deep-link login#7447OtavioStasiak wants to merge 13 commits into
Conversation
…otification receivers, confirm deep-link login
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx,json}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
app/sagas/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-04-30T17:07:51.020ZApplied to files:
📚 Learning: 2026-06-25T18:37:25.526ZApplied to files:
🔇 Additional comments (1)
WalkthroughTokens now use server-scoped keys across JS, Android, and iOS, with migration and legacy fallback support. Deep-link resume login now shows a confirmation prompt before proceeding, and Android notification receivers are no longer exported. ChangesToken scoping and migration
Deep-link login confirmation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
temporarily deployed
to
android_build
GitHub Actions
Inactive
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/methods/logout.ts (1)
76-80: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winKeep the legacy fallback before server removal.
If migration has not run or failed,
resumeisundefined,sdk.login({ resume })fails, and the server-side push token cleanup is skipped before local data is deleted. Fall back to${TOKEN_KEY}-${userId}here as well.Proposed fix
- const resume = UserPreferences.getString(getUserTokenKey(server, userId)); + const resume = UserPreferences.getString(getUserTokenKey(server, userId)) ?? UserPreferences.getString(`${TOKEN_KEY}-${userId}`); + if (!resume) { + throw new Error('Missing resume token for server removal'); + }🤖 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/lib/methods/logout.ts` around lines 76 - 80, The logout flow in `logout` uses only `getUserTokenKey(server, userId)` for `resume`, so when migration hasn’t run the `RocketchatClient.login` call fails and token cleanup is skipped. Update the `resume` lookup to fall back to the legacy `${TOKEN_KEY}-${userId}` key before calling `sdk.login({ resume })`, keeping the existing cleanup flow in `app/lib/methods/logout.ts` intact.
🤖 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/sagas/init.js`:
- Around line 31-60: The migration in migrateTokenKeysToServerScoped is
incorrectly copying a shared legacy token into every server-scoped slot when
multiple servers reuse the same userId. Update the first pass to detect
ambiguous userId mappings and skip migrating those legacy tokens into new slots,
while still removing the legacy token afterward so affected sessions
re-authenticate safely. Keep the fix localized to migrateTokenKeysToServerScoped
and its use of UserPreferences, TOKEN_KEY_SERVER_SCOPED_MIGRATED, and
getUserTokenKey.
In `@app/sagas/selectServer.ts`:
- Line 174: The server token lookup in selectServerRequest/selectServer should
handle legacy sessions before migration has completed. Update the token read
around UserPreferences.getString(getUserTokenKey(server, userId)) to fall back
to the legacy token key when the server-scoped value is missing, or otherwise
gate the deep-link/share-extension callers until appInit() finishes migration.
Keep the change localized to the token resolution path so direct dispatches
still succeed for legacy users.
---
Outside diff comments:
In `@app/lib/methods/logout.ts`:
- Around line 76-80: The logout flow in `logout` uses only
`getUserTokenKey(server, userId)` for `resume`, so when migration hasn’t run the
`RocketchatClient.login` call fails and token cleanup is skipped. Update the
`resume` lookup to fall back to the legacy `${TOKEN_KEY}-${userId}` key before
calling `sdk.login({ resume })`, keeping the existing cleanup flow in
`app/lib/methods/logout.ts` intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 124eb5ef-1af7-4051-a141-833d7f08c334
⛔ Files ignored due to path filters (1)
app/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
android/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/chat/rocket/reactnative/notification/Ejson.javaapp/i18n/locales/en.jsonapp/lib/constants/keys.tsapp/lib/methods/logout.tsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/deepLinking.jsapp/sagas/init.jsapp/sagas/login.jsapp/sagas/selectServer.tsios/Shared/RocketChat/MMKV.swiftios/Shared/RocketChat/Storage.swiftios/Watch/WatchConnection.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: E2E Build Android / android-build
- GitHub Check: E2E Build iOS / ios-build
- GitHub Check: Build Android / Build
- GitHub Check: Build iOS / Build
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/i18n/locales/en.jsonapp/lib/constants/keys.tsapp/sagas/selectServer.tsapp/sagas/init.jsapp/lib/methods/logout.tsapp/sagas/deepLinking.jsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/login.js
app/i18n/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Place internationalization (i18n) configuration in 'app/i18n/' directory with support for 40+ locales and RTL
Files:
app/i18n/locales/en.json
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/constants/keys.tsapp/sagas/selectServer.tsapp/sagas/init.jsapp/lib/methods/logout.tsapp/sagas/deepLinking.jsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/login.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/constants/keys.tsapp/sagas/selectServer.tsapp/lib/methods/logout.tsapp/sagas/__tests__/deepLinking.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/constants/keys.tsapp/sagas/selectServer.tsapp/sagas/init.jsapp/lib/methods/logout.tsapp/sagas/deepLinking.jsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/login.js
app/sagas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place sagas in 'app/sagas/' directory for handling side effects
Files:
app/sagas/selectServer.tsapp/sagas/__tests__/deepLinking.test.ts
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/constants/keys.tsapp/sagas/selectServer.tsapp/lib/methods/logout.tsapp/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/init.jsapp/sagas/deepLinking.jsapp/sagas/login.js
📚 Learning: 2026-06-15T16:17:10.741Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7398
File: android/app/src/main/java/chat/rocket/reactnative/notification/Encryption.java:211-211
Timestamp: 2026-06-15T16:17:10.741Z
Learning: When querying the Android SQLCipher WatermelonDB `subscriptions` table, treat the table’s primary-key column `id` as the Rocket.Chat room id (`rid`). In other words, a clause like `WHERE id = ?` should bind `ejson.rid` (this is equivalent to the iOS-side `WHERE rid = ?`). Do not flag this as a column-mismatch issue, because SQLite `subscriptions.id` stores the same value as the Rocket.Chat `rid` field.
Applied to files:
android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/sagas/__tests__/deepLinking.test.ts
🔇 Additional comments (13)
app/i18n/locales/en.json (1)
233-234: LGTM!app/sagas/deepLinking.js (1)
24-24: LGTM!Also applies to: 41-64, 270-276
app/sagas/__tests__/deepLinking.test.ts (1)
85-90: LGTM!Also applies to: 109-114, 316-335
app/lib/constants/keys.ts (1)
26-36: LGTM!app/lib/methods/logout.ts (1)
11-18: LGTM!Also applies to: 27-29
app/sagas/init.js (1)
5-5: LGTM!Also applies to: 67-70
android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java (1)
138-178: LGTM!ios/Watch/WatchConnection.swift (1)
44-44: LGTM!app/sagas/login.js (1)
25-25: LGTM!Also applies to: 344-344
app/sagas/selectServer.ts (1)
32-32: LGTM!android/app/src/main/AndroidManifest.xml (1)
105-117: LGTM!ios/Shared/RocketChat/MMKV.swift (1)
21-31: LGTM!ios/Shared/RocketChat/Storage.swift (1)
15-15: LGTM!
…t before direct server selects
|
iOS Build Available Rocket.Chat 4.75.0.109259 |
OtavioStasiak
had a problem deploying
to
upload_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.75.0.109258 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQeFdGa37dcYES0nomT6vqYCR3_JSDuS8adBZQpBOs9PLUesMFb2HE24l75n-6m2xNuwKzoM0qtktXM9Ug_ |
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
temporarily deployed
to
android_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
upload_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.75.0.109262 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNS4Eg8yH-E9IJrDRU9IucfqUsEIqRBD6L09EQsLlfVgVZ_O1cT4AV1Sah2cTMaj4wKFtWpMqhpy6mC7LGSz |
|
iOS Build Available Rocket.Chat 4.75.0.109263 |
julio-rocketchat
left a comment
julio-rocketchat
left a comment
There was a problem hiding this comment.
It looks good to me. It seems like all the gaps that could lead to security issues have been closed by this PR. Tks.
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/lib/methods/migrateTokenKeysToServerScoped.test.ts (1)
77-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a legacy-slot assertion here.
This test verifies the scoped key isn't overwritten but doesn't assert the fate of
${TOKEN_KEY}-${userId}. Given the removal gap flagged inmigrateTokenKeysToServerScoped.ts, addingexpect(UserPreferences.getString(${TOKEN_KEY}-${userId})).toBeNull()would pin the intended behavior once the source fix lands.🤖 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/lib/methods/migrateTokenKeysToServerScoped.test.ts` around lines 77 - 89, The migration test currently only checks that the existing server-scoped token is preserved, but it does not verify what happens to the legacy user-scoped slot. In migrateTokenKeysToServerScoped.test.ts within the “does not overwrite an existing server-scoped token” case, add an assertion for the `${TOKEN_KEY}-${userId}` entry so the test pins the intended cleanup behavior after migrateTokenKeysToServerScoped runs, using UserPreferences.getString on that legacy key and expecting it to be null.
🤖 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/lib/methods/migrateTokenKeysToServerScoped.ts`:
- Around line 42-53: The migration in migrateTokenKeysToServerScoped leaves the
legacy user token slot behind when getUserTokenKey(server, userId) already
exists, because legacyKey is only tracked when a copy happens. Update the
single-server branch so the legacy ${TOKEN_KEY}-${userId} key is always added to
legacyKeys (or otherwise removed) whenever that legacy slot is processed, even
if the server-scoped key already has a value, and keep the final
legacyKeys.forEach cleanup in place. Consider tightening the existing “does not
overwrite an existing server-scoped token” test to assert the legacy key is
removed too.
---
Nitpick comments:
In `@app/lib/methods/migrateTokenKeysToServerScoped.test.ts`:
- Around line 77-89: The migration test currently only checks that the existing
server-scoped token is preserved, but it does not verify what happens to the
legacy user-scoped slot. In migrateTokenKeysToServerScoped.test.ts within the
“does not overwrite an existing server-scoped token” case, add an assertion for
the `${TOKEN_KEY}-${userId}` entry so the test pins the intended cleanup
behavior after migrateTokenKeysToServerScoped runs, using
UserPreferences.getString on that legacy key and expecting it to be null.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5552ba1c-0d7a-47ae-8a5b-09b197d37a07
📒 Files selected for processing (34)
android/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/chat/rocket/reactnative/notification/Ejson.javaapp/i18n/locales/ar.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/cs.jsonapp/i18n/locales/de.jsonapp/i18n/locales/es.jsonapp/i18n/locales/fi.jsonapp/i18n/locales/fr.jsonapp/i18n/locales/hi-IN.jsonapp/i18n/locales/hu.jsonapp/i18n/locales/it.jsonapp/i18n/locales/ja.jsonapp/i18n/locales/nl.jsonapp/i18n/locales/nn.jsonapp/i18n/locales/no.jsonapp/i18n/locales/pt-BR.jsonapp/i18n/locales/pt-PT.jsonapp/i18n/locales/ru.jsonapp/i18n/locales/sl-SI.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/ta-IN.jsonapp/i18n/locales/te-IN.jsonapp/i18n/locales/tr.jsonapp/i18n/locales/zh-CN.jsonapp/i18n/locales/zh-TW.jsonapp/lib/constants/keys.tsapp/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/lib/methods/migrateTokenKeysToServerScoped.tsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/deepLinking.jsapp/sagas/init.jsapp/sagas/selectServer.tsios/Shared/RocketChat/MMKV.swift
💤 Files with no reviewable changes (3)
- app/sagas/tests/deepLinking.test.ts
- android/app/src/main/AndroidManifest.xml
- app/sagas/deepLinking.js
✅ Files skipped from review due to trivial changes (19)
- app/i18n/locales/zh-TW.json
- app/i18n/locales/hu.json
- app/i18n/locales/zh-CN.json
- app/i18n/locales/it.json
- app/i18n/locales/de.json
- app/i18n/locales/ta-IN.json
- app/i18n/locales/es.json
- app/i18n/locales/cs.json
- app/i18n/locales/hi-IN.json
- app/i18n/locales/tr.json
- app/i18n/locales/fr.json
- app/i18n/locales/ar.json
- app/i18n/locales/ja.json
- app/i18n/locales/nl.json
- app/i18n/locales/pt-BR.json
- app/i18n/locales/nn.json
- app/i18n/locales/pt-PT.json
- app/i18n/locales/ru.json
- app/i18n/locales/fi.json
🚧 Files skipped from review as they are similar to previous changes (4)
- app/lib/constants/keys.ts
- app/sagas/selectServer.ts
- ios/Shared/RocketChat/MMKV.swift
- android/app/src/main/java/chat/rocket/reactnative/notification/Ejson.java
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/i18n/locales/te-IN.jsonapp/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/i18n/locales/no.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/sl-SI.jsonapp/sagas/init.jsapp/lib/methods/migrateTokenKeysToServerScoped.ts
app/i18n/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Place internationalization (i18n) configuration in 'app/i18n/' directory with support for 40+ locales and RTL
Files:
app/i18n/locales/te-IN.jsonapp/i18n/locales/no.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/sl-SI.json
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/sagas/init.jsapp/lib/methods/migrateTokenKeysToServerScoped.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/lib/methods/migrateTokenKeysToServerScoped.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/sagas/init.jsapp/lib/methods/migrateTokenKeysToServerScoped.ts
🧠 Learnings (3)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/migrateTokenKeysToServerScoped.test.tsapp/lib/methods/migrateTokenKeysToServerScoped.ts
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/lib/methods/migrateTokenKeysToServerScoped.test.ts
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/init.js
🔇 Additional comments (6)
app/i18n/locales/bn-IN.json (1)
216-217: LGTM!app/i18n/locales/no.json (1)
220-221: LGTM!app/i18n/locales/sl-SI.json (1)
181-182: LGTM!app/i18n/locales/sv.json (1)
199-200: LGTM!app/i18n/locales/te-IN.json (1)
215-216: LGTM!app/sagas/init.js (1)
27-27: LGTM!
…nd simplify comments
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
android_build
GitHub Actions
Error
OtavioStasiak
requested a deployment
to
android_build
GitHub Actions
Waiting
3 participants
Proposed changes
This PR hardens authentication and deep-link handling against a set of related token-confusion /
silent-authentication issues, and stops exporting Android notification receivers.
The resume token was stored under reactnativemeteor_usertoken-${userId}, with no server component.
Because two servers can legitimately share the same userId — and a malicious server can deliberately
force a collision — that storage slot was ambiguous: the last writer won, and a lookup by a different
server could resolve a token belonging to another server, enabling token confusion / exfiltration.
The token is now keyed by reactnativemeteor_usertoken-${server}-${userId} via a shared
getUserTokenKey(server, userId) helper, used consistently across the login, server-select, and logout
paths, and mirrored on the native side (MMKV.swift on iOS, Ejson.java on Android).
A one-time, two-pass migration in the init saga moves existing tokens from the legacy slot to the new
server-scoped slot (copy every server first, then remove legacy slots, so a userId shared by multiple
servers isn't dropped mid-migration). Logout removes both the new and the legacy slots.
A rocketchat://auth?host=…&token=… deep link could silently authenticate the user into an arbitrary
server, and any app or web page can fire it. Before consuming a resume token for a server the user
isn't already signed in to, the app now asks for explicit confirmation and lands on the login screen if
declined. (Auto-confirmed under RUNNING_E2E_TESTS so the E2E harness, which bootstraps via deep link,
isn't blocked by the native alert.)
The notification receivers in AndroidManifest.xml are no longer exported, closing them off from other
apps on the device.
Notes:
they're added in app/i18n/locales/en.json.
token-scoping and the deep-link-confirmation behaviors.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1342
How to test or reproduce
Deep-link login confirmation
"rocketchat://auth?host=your.server.com&token=" (or the equivalent adb shell am
start).
leaves you on the login/outside screen with no session. Before this PR the login happened silently.
Token scoped to (server, userId)
session bleed between servers.
this build over it and relaunch. Expected: you stay logged in (token migrated from the legacy slot to
the server-scoped one), and logging out clears it fully.
Affected screens: deep-link entry (confirmation alert), Servers list / server switching, and the
login/logout flow. No visual changes beyond the new confirmation alert.
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Security