Skip to content

New user handling - #6787

Open
mahibi wants to merge 76 commits into
masterfrom
fixUserManagement
Open

mahibi wants to merge 76 commits into
masterfrom
fixUserManagement

Conversation

@mahibi

@mahibi mahibi commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Bind screens and background work to their account

Problem

Screens, view models, workers and receivers read a global "current user" (CurrentUserProviderOld / CurrentUserProvider). Anything that changed it switched open screens to another account, or
mixed the data of account A with the credentials of account B: an incoming call, a notification tap, the account switcher, share-to, a deep link. The providers also had their own bugs:

  • non thread-safe caching
  • runBlocking on the main thread
  • writes during reads (self-heal in getActiveUser())
  • stale values

The HTTP layer had the same problem one level lower: one shared cookie store kept server session cookies per server, so with several accounts on the same server, requests of one account were
authenticated with another account's session.

Solution

  • Every screen, view model, worker and receiver is bound to an explicit account, passed as BundleKeys.KEY_INTERNAL_USER_ID (a Long). An open screen stays on its account.
  • The global user is now a "default account": the last used one, stored as the current flag. It is read only by entry points: app start, share-to, deep links without an account, the
    account switcher and the phone book integration. It changes on user actions: the account switcher, a notification tap, a deep link, opening the conversation list of an account, and joining a
    call. The conversation list follows it when it comes back from the background.
  • Theming is per account: screens use the server colors of their own account. The color scheme cache is keyed by the theming capability, so accounts on the same server with different colors
    don't share one entry, and changed server colors apply after the next capabilities sync.
  • Requests are authenticated by their account's credentials only:
    • Requests with an Authorization header neither send nor store cookies (CredentialsCookieInterceptor).
    • Images of the account's own server (chat previews, quote and link previews, the media viewer, conversation avatars of open conversations) send the account's credentials.
    • Credentials are only sent after an origin check (scheme, host, port, base path) via UriUtils.isOnServer(), never after a URL prefix check.
    • RemoteWipeInterceptor only reacts to a 401 for a request with credentials, and removes exactly the account with those credentials. Before, it removed the first account on the server for
      any 401.
    • The TLS client certificate is chosen by the server of the connection, not by the default account.
  • Clean-up and data layer:
    • The current user providers are replaced by DefaultAccountProvider.
    • Reading the active user no longer writes. A one-time repair at startup, a single SQL statement, fixes several accounts marked as active.
    • Background jobs and Settings only write the columns they change (Room partial entities). They can no longer reset the default account or the token by saving an old copy of the account.
    • A reauthorization only stores the new token in the account that logged in (by login name and server), and only if it's the account being reauthorized. Otherwise the login screen asks to log
      in with the right account.

Other related fixes found along the way

  • Switching accounts no longer opens a second app instance, and no longer briefly shows the previous account's conversations.
  • A created conversation opens only once, and only while the contacts screen is visible. Links to the own server open for the screen's account.
  • An edited status message survives rotation. The account switcher's status belongs to the current default account.
  • After picture in picture during a call for another account, the list shows that account.
  • The share-to chooser keeps the chosen account, uses the list's account, and passes on the shared files. The list no longer relaunches an intent received from other apps as-is (lint
    UnsafeIntentLaunch).
  • Sharing to Talk without an account opens the login instead of closing silently.
  • Renaming a conversation from the conversation list works again.
  • The current room (for joining and calls) is matched by token and account, as accounts on the same server share the tokens of their common conversations.
  • The login screen doesn't start a second login when it's recreated.
  • The file browser and the text viewer are BaseActivitys, so they get the screen lock, FLAG_SECURE and their account's theme; the sorting dialog uses the host's theme.
  • The media viewer and shared items authenticate with the login name instead of the user id.
  • Profile and Settings close when their account was removed, instead of crashing.
  • Crashes: CallActivity finishing in onCreate(); DialogBanListFragment on rotation; restored fragments accessing the chat view model.
  • isDialing is reset when a call screen closes before setup.
  • An upload whose account was removed fails cleanly.
  • A camera or picker result for the conversation avatar is no longer lost after process death.
  • The address search uses the chat's account theme.
  • Injected view models are real view models, so they survive rotation and are cleared.
  • MessageSearchActivity (unused) is removed.

How to get the account

Where How
Start an activity Always put KEY_INTERNAL_USER_ID (the user.id, a Long). For chats, use ChatActivity.createIntent(context, userId, roomToken, extras). A missing id fails in debug builds (check) and is logged as an error in release builds.
Activity (BaseActivity) In onCreate(): super.onCreate() → inject(this) → user = setUpBoundUserOrFinish() ?: return → the rest. This loads the account, applies its theme, and finishes the activity if the account doesn't exist. Only entry points set override val allowsDefaultAccount = true, and then fall back to the default account.
View model @AssistedInject constructor(…, @Assisted user: User) with an @AssistedFactory interface Factory { fun build(user: User): X }. In the activity: val viewModel by assistedViewModels { factory.build(user) }, used only after setUpBoundUserOrFinish(). In Compose: viewModel(key = "x-${user.id}", factory = ViewModelFactoryWithParams(X::class.java) { factory.build(user) }). Never read the default account in a view model.
Observe account changes (capabilities, token, …) userManager.userFlow(id). For a one-time read: userManager.getUserWithId(id).
Fragment / dialog Pass the User (Parcelable) or its id as fragment arguments via newInstance(…), never as constructor parameters (they break recreation). Theme: viewThemeUtils = hostViewThemeUtils(activity, viewThemeUtils).
Custom view / adapter Take the account from the host screen. Theme: hostViewThemeUtils(context, fallback).
Worker Put the id into the input data (putLong(KEY_INTERNAL_USER_ID, user.id)). In the worker: userManager.getUserWithId(inputData.getLong(KEY_INTERNAL_USER_ID,0L)), and fail the work if it's null.
Receiver / notification action / service Put the id into the (pending) intent. Read it with getLongExtra(KEY_INTERNAL_USER_ID, 0L), and stop if no account is found. There's no fallback to the default account.
Entry point without an account context DefaultAccountProvider.getDefaultUser() (suspend) or getDefaultUserBlocking(). Make an account the default only on user actions, with userManager.setUserAsActive(user).
Theme outside BaseActivity ViewThemeUtilsFactory.forUser(user).
Save account data Never save a whole User copy that was read earlier: it overwrites current and token. Use the partial updates (userManager.updateCapabilities, updateExternalSignalingServer, updateDisplayName, updateClientCertificate, updateCredentials). For new columns, add a partial entity class in data/user/model/UserPartialUpdates.kt and an @Update(entity = UserEntity::class) method.

Testing

  • Unit tests:
    • UserManager, DefaultAccountProvider, ChatViewModel, ContactsViewModel, CapabilitiesFetcher
    • LoginRepository (reauthorization with a different or unknown account), BrowserLoginActivityViewModel (login started once)
    • CredentialsCookieInterceptor (MockWebServer), RemoteWipeInterceptor (401s without matching credentials remove no account)
    • UriUtils.isOnServer (origin check), ClientCertificateSelector
    • Room tests for the partial updates and the startup repair (in-memory database)
  • Manual, with two or three accounts on different servers:
    • An open chat of A stays on A during an incoming call for B; the call uses B.
    • A notification tap for B opens B, and back goes to B's list.
    • The account switcher: one task, no leftover screens of A.
    • Rotation keeps the account.
    • The launcher opens the last used account.
    • Share-to with a switch to another account (text and files).
    • Picture in picture during a call for another account.
  • Manual, with two accounts on the same server:
    • Each account shows its own conversations while both are used in parallel (list, chat, catch-ups).
    • Image previews, the media viewer and shared items load (also with a login name that differs from the user id).
    • A reauthorization while the browser is logged in as another account asks to log in with the right account and changes nothing.
    • A shared 1:1 conversation: a call of one account doesn't take over the other account's chat session.
  • Remote wipe: wiping the device on the web still removes the account; a 401 for an image does not.

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@mahibi mahibi added this to the 25.1.0 milestone Sep 28, 2026
@mahibi mahibi self-assigned this Sep 28, 2026
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

📱 QA build

Download app-qa-debug.apk
QR code Open the QR code for this download
Commit d79bf07
Version 6787
Available until 7 days after this build

The QA build installs alongside a released Nextcloud app, so you can keep
using your existing install while testing.

Downloading the file requires a GitHub account, so open this link on the
device you want to test on, or transfer the APK to it.

@mahibi mahibi mentioned this pull request Sep 29, 2026
1 of 6 tasks
@mahibi mahibi added the 3. to review Waiting for reviews label Sep 29, 2026
@mahibi
mahibi marked this pull request as ready for review September 29, 2026 12:47
@mahibi mahibi changed the title Fix user management New user handling Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change makes account identity explicit across activities, view models, conversation flows, and background work. It adds default-account lookup, per-user data updates, and repair for multiple active accounts. Account IDs are passed through navigation and worker inputs. Feature view models receive their account user directly. Theme utilities can resolve the host activity’s theme. The message-search activity and related UI resources are removed. Login reauthorization now targets a specified account, and credentialed HTTP requests do not send or store session cookies.

Priority: ⬆️ High

Merge Risk: 🟡 Moderate · up to 06ace

Resolve image credential isolation, certificate endpoint matching, and stale credentials in restored poll dialogs before merging. These gaps can expose account-specific images or credentials and cause authentication failures. The historical rename, profile, theme, sharing, and login-recreation concerns are addressed.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 06ace

Explicit account binding improves isolation, but shared image caching has not been aligned with account-bound authentication. Authenticated previews also inherit existing unencrypted-transport and account-removal edge cases.

Retained concerns

  • Medium · security · inferred: New account-authenticated image consumers retain the application-wide cache without representing account identity in their cache configuration. For an identical URL whose response depends on account credentials, one account can potentially receive another account's cached image without a fresh authorization check. The shared cache existed before this PR, but newly credentialed consumers expand the content entering it. This concern is limited to colliding URLs within the same installation; account-specific URLs naturally reduce exposure.
Security review details

Security Blast Radius

  • inferred — Image-cache disclosure is bounded to accounts using the same application installation and colliding image URLs. Network authentication paths carry the selected account's application credentials; their downstream authority depends on that account's server permissions. The inspected paths do not establish administrative, cross-installation, or infrastructure-wide authority.

Security Findings and Attack Paths

  • inferred — The retained image-cache finding concerns a request authenticated as account A populating shared image state that account B can subsequently consume for the same URL. Newly credentialed consumers are confirmed, but no runtime cache-collision demonstration was performed. URLs embedding distinct account identifiers reduce this attack path.
  • observed — Matching thumbnails for an HTTP-configured account receive Authorization over cleartext transport. Thumbnail authentication is new, but HTTP account support and network policy already allowed this transport, so this is an additional credentialed consumer of an existing account exposure rather than a newly introduced TLS bypass. An HTTPS account's initial HTTP thumbnail fails the scheme check.
  • inferred — The retained denial-of-service path attributes a terminal 401 after redirection to the original request's account and schedules local account removal, even when the wipe check returns false. Original-request attribution and unconditional removal predate this PR; the head narrows candidate selection with exact credentials and server matching. No worsened maximum deletion scope was established.
  • inferred — The retained redirect-exposure finding applies to newly authenticated link thumbnails. This pass confirms initial-origin filtering and use of the shared HTTP client, but did not establish redirect-time Authorization handling in the pinned dependencies. Redirect following alone is insufficient evidence that credentials cross origins or survive an HTTPS-to-HTTP redirect, so that consequence is not independently asserted here.

Trust Boundaries and Controls

  • observed — Credentialed requests omit session cookies and ignore response Set-Cookie, preventing another account's server session from overriding explicit authentication. Reauthorization also binds the browser-authenticated username and server to the requested internal account ID. Neither control partitions image-cache ownership.

Resilience and Maintainability Implications

  • inferred — Credential matching is a snapshot check, not an atomic fence through account deletion. A credential replacement after candidate resolution can still be followed by deletion scheduling using the same account ID. Concurrent duplicate suppression protects the successful path but can suppress recovery after failure before the durable marker. These mechanisms existed at the base; the PR improves identity selection without resolving these lifecycle weaknesses.

Hardening Proposals

  • proposed — Partition authenticated image caches by stable account identity, with appropriate invalidation on removal or credential changes. Avoid using raw credentials as cache identifiers. Validate isolation using identical URLs that return different images for two accounts.
  • proposed — Bind destructive 401 processing to the effective response origin and a current credential generation at the durable mutation boundary. Make duplicate suppression recoverable when processing fails before deletion ownership is persisted, and verify redirect and interruption behavior against the pinned HTTP dependencies.
🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.21% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 421 functions across 90 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title relates to account handling, but "New user handling" is too broad and does not identify the primary change: binding screens and background work to explicit accounts. Use a specific title such as "Bind screens and background work to explicit accounts".
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, solution, related fixes, testing scope, and checklist. It omits the template's screenshots and TODO sections, but the required change details are otherwis…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Key the theme cache by the theme inputs, not only baseUrl. · MaterialSchemesProviderImpl.kt:32-33

app/src/main/java/com/nextcloud/talk/ui/theme/MaterialSchemesProviderImpl.kt:32-33
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Key the theme cache by the theme inputs, not only baseUrl.

When two accounts share a baseUrl but have different themingCapability values, the first account populates this cache entry. The new ViewThemeUtilsFactory.forUser then gives the second account the first account’s colors. Include the account and relevant capability state in the cache key, or calculate schemes without this cache. The new per-user factory makes this existing cache behavior affect account-scoped views.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ccc46ac3-e2bd-4831-a031-4f07e432752e

📥 Commits

Reviewing files that changed from the base of the PR and between f704b95 and 90f3bd5.

📒 Files selected for processing (143)
  • app/src/androidTest/java/com/nextcloud/talk/ui/LoginIT.java
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt
  • app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt
  • app/src/main/java/com/nextcloud/talk/account/ServerSelectionActivity.kt
  • app/src/main/java/com/nextcloud/talk/account/SwitchAccountActivity.kt
  • app/src/main/java/com/nextcloud/talk/account/data/io/LocalLoginDataSource.kt
  • app/src/main/java/com/nextcloud/talk/activities/BaseActivity.kt
  • app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt
  • app/src/main/java/com/nextcloud/talk/activities/MainActivity.kt
  • app/src/main/java/com/nextcloud/talk/adapters/items/LoadMoreResultsItem.kt
  • app/src/main/java/com/nextcloud/talk/adapters/items/MessageResultItem.kt
  • app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewFragment.kt
  • app/src/main/java/com/nextcloud/talk/callnotification/CallNotificationActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt
  • app/src/main/java/com/nextcloud/talk/chat/MessageInputVoiceRecordingFragment.kt
  • app/src/main/java/com/nextcloud/talk/chat/ScheduledMessagesActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt
  • app/src/main/java/com/nextcloud/talk/chat/viewmodels/ScheduledMessagesViewModel.kt
  • app/src/main/java/com/nextcloud/talk/chooseaccount/ChooseAccountDialogCompose.kt
  • app/src/main/java/com/nextcloud/talk/chooseaccount/ui/StatusMessageSheet.kt
  • app/src/main/java/com/nextcloud/talk/chooseaccount/viewmodel/StatusMessageViewModel.kt
  • app/src/main/java/com/nextcloud/talk/chooseaccount/viewmodel/StatusViewModel.kt
  • app/src/main/java/com/nextcloud/talk/contacts/ContactsActivity.kt
  • app/src/main/java/com/nextcloud/talk/contacts/ContactsScreen.kt
  • app/src/main/java/com/nextcloud/talk/contacts/ContactsViewModel.kt
  • app/src/main/java/com/nextcloud/talk/contacts/components/ContactItemRow.kt
  • app/src/main/java/com/nextcloud/talk/contacts/components/ConversationCreationOptions.kt
  • app/src/main/java/com/nextcloud/talk/contextchat/ContextChatViewModel.kt
  • app/src/main/java/com/nextcloud/talk/conversation/RenameConversationDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/conversationcreation/ConversationCreationActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationcreation/ui/CreatedConversation.kt
  • app/src/main/java/com/nextcloud/talk/conversationcreation/viewmodel/ConversationCreationViewModel.kt
  • app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationinfoedit/ConversationInfoEditActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationinfoedit/viewmodel/ConversationInfoEditViewModel.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt
  • app/src/main/java/com/nextcloud/talk/conversationtags/viewmodels/ConversationTagsViewModel.kt
  • app/src/main/java/com/nextcloud/talk/dagger/modules/ViewModelModule.kt
  • app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt
  • app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt
  • app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt
  • app/src/main/java/com/nextcloud/talk/data/user/model/UserPartialUpdates.kt
  • app/src/main/java/com/nextcloud/talk/diagnosis/DiagnosisActivity.kt
  • app/src/main/java/com/nextcloud/talk/diagnosis/DiagnosisElement.kt
  • app/src/main/java/com/nextcloud/talk/diagnosis/DiagnosisViewModel.kt
  • app/src/main/java/com/nextcloud/talk/invitation/InvitationsActivity.kt
  • app/src/main/java/com/nextcloud/talk/invitation/adapters/InvitationsAdapter.kt
  • app/src/main/java/com/nextcloud/talk/jobs/CapabilitiesFetcher.kt
  • app/src/main/java/com/nextcloud/talk/jobs/ContactAddressBookWorker.kt
  • app/src/main/java/com/nextcloud/talk/jobs/DownloadFileToCacheWorker.kt
  • app/src/main/java/com/nextcloud/talk/jobs/LeaveConversationWorker.kt
  • app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt
  • app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java
  • app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt
  • app/src/main/java/com/nextcloud/talk/location/GeocodingActivity.kt
  • app/src/main/java/com/nextcloud/talk/location/LocationPickerActivity.kt
  • app/src/main/java/com/nextcloud/talk/location/viewmodels/LocationPickerViewModel.kt
  • app/src/main/java/com/nextcloud/talk/logger/ui/LogsActivity.kt
  • app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerActivity.kt
  • app/src/main/java/com/nextcloud/talk/mediaviewer/viewmodels/MediaViewerViewModel.kt
  • app/src/main/java/com/nextcloud/talk/messagesearch/MessageSearchActivity.kt
  • app/src/main/java/com/nextcloud/talk/messagesearch/MessageSearchViewModel.kt
  • app/src/main/java/com/nextcloud/talk/openconversations/ListOpenConversationsActivity.kt
  • app/src/main/java/com/nextcloud/talk/openconversations/viewmodels/OpenConversationsViewModel.kt
  • app/src/main/java/com/nextcloud/talk/polls/ui/PollCreateDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/polls/ui/PollLoadingFragment.kt
  • app/src/main/java/com/nextcloud/talk/polls/ui/PollMainDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/polls/ui/PollResultsFragment.kt
  • app/src/main/java/com/nextcloud/talk/polls/ui/PollVoteFragment.kt
  • app/src/main/java/com/nextcloud/talk/polls/viewmodels/PollCreateViewModel.kt
  • app/src/main/java/com/nextcloud/talk/polls/viewmodels/PollMainViewModel.kt
  • app/src/main/java/com/nextcloud/talk/polls/viewmodels/PollVoteViewModel.kt
  • app/src/main/java/com/nextcloud/talk/presenters/MentionAutocompletePresenter.java
  • app/src/main/java/com/nextcloud/talk/profile/ProfileActivity.kt
  • app/src/main/java/com/nextcloud/talk/raisehand/viewmodel/RaiseHandViewModel.kt
  • app/src/main/java/com/nextcloud/talk/receivers/DirectReplyReceiver.kt
  • app/src/main/java/com/nextcloud/talk/receivers/DismissRecordingAvailableReceiver.kt
  • app/src/main/java/com/nextcloud/talk/receivers/MarkAsReadReceiver.kt
  • app/src/main/java/com/nextcloud/talk/receivers/ShareRecordingToChatReceiver.kt
  • app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt
  • app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt
  • app/src/main/java/com/nextcloud/talk/settings/SettingsActivity.kt
  • app/src/main/java/com/nextcloud/talk/shareditems/activities/SharedItemsActivity.kt
  • app/src/main/java/com/nextcloud/talk/shareditems/adapters/SharedItemsAdapter.kt
  • app/src/main/java/com/nextcloud/talk/threadsoverview/ThreadsOverviewActivity.kt
  • app/src/main/java/com/nextcloud/talk/threadsoverview/viewmodels/ThreadsOverviewViewModel.kt
  • app/src/main/java/com/nextcloud/talk/translate/ui/TranslateActivity.kt
  • app/src/main/java/com/nextcloud/talk/translate/viewmodels/TranslateViewModel.kt
  • app/src/main/java/com/nextcloud/talk/ui/PlaybackSpeedControl.kt
  • app/src/main/java/com/nextcloud/talk/ui/chooseaccount/ChooseAccountShareToDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/chooseaccount/ChooseAccountShareToViewModel.kt
  • app/src/main/java/com/nextcloud/talk/ui/chooseaccount/model/ChooseAccountShareToViewState.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/AudioOutputDialog.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/DateTimeCompose.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/DialogBanListFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/FilterConversationFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/MoreCallActionsDialog.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/SaveToStorageDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/SetPhoneNumberDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/HostViewThemeUtils.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/MaterialSchemesProvider.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/MaterialSchemesProviderImpl.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/ThemeModule.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/ViewThemeUtilsFactory.kt
  • app/src/main/java/com/nextcloud/talk/users/DefaultAccountProvider.kt
  • app/src/main/java/com/nextcloud/talk/users/UserManager.kt
  • app/src/main/java/com/nextcloud/talk/utils/FileViewerUtils.kt
  • app/src/main/java/com/nextcloud/talk/utils/PickImage.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProvider.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderOld.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderOldImpl.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/UserModule.kt
  • app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt
  • app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt
  • app/src/main/java/com/nextcloud/talk/utils/rx/SearchViewObservable.kt
  • app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java
  • app/src/main/java/com/nextcloud/talk/viewmodels/CallRecordingViewModel.kt
  • app/src/main/res/layout/activity_message_search.xml
  • app/src/main/res/layout/rv_item_load_more.xml
  • app/src/main/res/layout/rv_item_search_message.xml
  • app/src/main/res/menu/menu_search.xml
  • app/src/main/res/values/dimens.xml
  • app/src/main/res/values/strings.xml
  • app/src/test/java/com/nextcloud/talk/contacts/ContactsViewModelTest.kt
  • app/src/test/java/com/nextcloud/talk/conversationcreation/ConversationCreationViewModelTest.kt
  • app/src/test/java/com/nextcloud/talk/conversationlist/data/network/ConversationListFreshnessIntegrationTest.kt
  • app/src/test/java/com/nextcloud/talk/data/user/UsersDaoPartialUpdateTest.kt
  • app/src/test/java/com/nextcloud/talk/data/user/UsersDaoRepairTest.kt
  • app/src/test/java/com/nextcloud/talk/data/user/UsersRepositoryImplTest.kt
  • app/src/test/java/com/nextcloud/talk/jobs/CapabilitiesFetcherTest.kt
  • app/src/test/java/com/nextcloud/talk/location/viewmodels/LocationPickerViewModelTest.kt
  • app/src/test/java/com/nextcloud/talk/messagesearch/MessageSearchHelperTest.kt
  • app/src/test/java/com/nextcloud/talk/users/DefaultAccountProviderTest.kt
  • app/src/test/java/com/nextcloud/talk/users/UserManagerTest.kt
  • app/src/test/java/com/nextcloud/talk/viewmodels/CallRecordingViewModelTest.kt
💤 Files with no reviewable changes (19)
  • app/src/main/res/values/dimens.xml
  • app/src/main/java/com/nextcloud/talk/account/ServerSelectionActivity.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderOld.kt
  • app/src/main/res/layout/rv_item_search_message.xml
  • app/src/main/res/menu/menu_search.xml
  • app/src/main/res/layout/activity_message_search.xml
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/com/nextcloud/talk/logger/ui/LogsActivity.kt
  • app/src/main/java/com/nextcloud/talk/adapters/items/LoadMoreResultsItem.kt
  • app/src/main/java/com/nextcloud/talk/adapters/items/MessageResultItem.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderOldImpl.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProvider.kt
  • app/src/main/res/layout/rv_item_load_more.xml
  • app/src/main/res/values/strings.xml
  • app/src/main/java/com/nextcloud/talk/messagesearch/MessageSearchActivity.kt
  • app/src/main/java/com/nextcloud/talk/messagesearch/MessageSearchViewModel.kt
  • app/src/main/java/com/nextcloud/talk/utils/rx/SearchViewObservable.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt
  • app/src/main/java/com/nextcloud/talk/utils/database/user/UserModule.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread app/src/main/java/com/nextcloud/talk/profile/ProfileActivity.kt Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 37aaaa81-c164-4722-b2f6-2c5f88147b9d

📥 Commits

Reviewing files that changed from the base of the PR and between 90f3bd5 and 55bd468.

📒 Files selected for processing (7)
  • app/src/main/java/com/nextcloud/talk/conversation/RenameConversationDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/conversationinfoedit/viewmodel/ConversationInfoEditViewModel.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt
  • app/src/main/java/com/nextcloud/talk/profile/ProfileActivity.kt
  • app/src/main/java/com/nextcloud/talk/settings/SettingsActivity.kt
  • app/src/main/java/com/nextcloud/talk/ui/theme/MaterialSchemesProviderImpl.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

@github-actions

Copy link
Copy Markdown
Contributor

Codacy

Lint

TypemasterPR
Warnings138138
Errors1817

SpotBugs

CategoryBaseNew
Bad practice77
Correctness1111
Dodgy code4545
Internationalization33
Malicious code vulnerability33
Performance88
Security1111
Total8888

Chat and call screens read the global active user instead of the
account they were opened for. Any setUserAsActive() while such a screen
was open (e.g. an incoming call for another account) silently switched
it to the other account or mixed data and credentials of both.

- ChatActivity/ChatViewModel are bound to the user id passed in the
  intent (KEY_INTERNAL_USER_ID); all chat launches go through
  ChatActivity.createIntent() and pass the user id
- CallActivity, RaiseHandViewModel and CallRecordingViewModel use the
  user of the call
- NotificationWorker no longer switches the active account on incoming
  calls
- UserManager.userFlow(id) observes a single user
- getActiveUser() no longer writes to the database on every read; the
  "multiple active users" self-heal runs once at app start
- setUserAsActive() publishes the stored row
- CurrentUserProviderOldImpl: fix racy cache initialization

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
- Switching accounts opens the conversation list of the new account in
  a cleared task, so no screen of the previous account stays in the
  back stack (account dialog, SwitchAccountActivity, ecosystem account
  handoff)
- ConversationsListActivity keeps its account across new intents and
  opens a fresh instance for another account
- Notification receivers require the user id instead of falling back
  to the active account; the recording actions now pass it
- LeaveConversationWorker, UploadAndShareFilesWorker and
  DownloadFileToCacheWorker use the user id from their input data

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
ConversationsListViewModel, ConversationTagsViewModel and
ThreadsOverviewViewModel get the user of their screen via assisted
injection instead of reading the active account at construction time.
FilterConversationFragment receives the user as argument.

Opening the conversation list of an account makes it the last used
(default) account, which the account switcher and status views use.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
ConversationInfoActivity and ConversationInfoEditActivity use the user
id they were started with, and pass it on to the screens they open.
ConversationInfoEditViewModel and DialogBanListFragment get the user
from their screen.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
…account

SharedItemsActivity, MediaViewerActivity and RemoteFileBrowserActivity
use the user id they were started with; all launchers pass it.
RemoteFileBrowserItemsViewModel gets the user via assisted injection.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
- MessageSearchViewModel gets the user of its screen
- The poll dialogs use the user they are created with (the main poll
  dialog already received it but ignored it) and pass it to their
  view models

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Scheduled messages, reminders, mention autocomplete, location sharing,
translation and context chat use the user of the chat they belong to
instead of the active account.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
…eir account

ContactsViewModel, ConversationCreationViewModel and
OpenConversationsViewModel get the user of their screen via assisted
injection. Contacts, conversation creation, open conversations and
invitations use the user id they were started with; all launchers pass
it.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
SettingsActivity, ProfileActivity and DiagnosisActivity use the user id
they were started with; the conversation list and settings pass it.
DiagnosisViewModel gets the user via assisted injection and the
diagnosis account section shows that user.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Screens were themed with the server colors of the active account, so a
screen of another account (e.g. an incoming call for it) used the wrong
colors.

- ViewThemeUtilsFactory creates ViewThemeUtils for a given account
- Activities bound to an account apply its theme right after injection
  (BaseActivity.applyUserTheme)
- Fragments, dialogs and views use the ViewThemeUtils of their hosting
  activity (hostViewThemeUtils)

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
All screens, workers and receivers now use the account they were
started for, so the "current user" providers are only needed where
there is no account context (entry points, share-to, account switcher,
default theme). Replace CurrentUserProviderOld and CurrentUserProvider
by DefaultAccountProvider, which makes that meaning explicit, and
rename UserManager.getCurrentUser()/currentUserFlow to
getDefaultUser()/defaultUserFlow.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
…s after switching

The singleton conversations repository kept the observed account in a
global state that getRooms() only updated asynchronously, so the room
list of a newly opened account first emitted the conversations of the
previously shown account. roomListFlow() now takes the account id.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
FLAG_ACTIVITY_NEW_TASK never matches an existing task because the app's
activities have an empty task affinity, so FLAG_ACTIVITY_NEW_TASK |
FLAG_ACTIVITY_CLEAR_TASK started a second app instance. Use
FLAG_ACTIVITY_CLEAR_TOP instead, which recreates the conversation list
of the current task for the new account and closes all screens above
it.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
mahibi requested a balanced review from Copilot September 30, 2026 10:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b46df4ea-80d6-4dbf-bef1-490008cd4ec1

📥 Commits

Reviewing files that changed from the base of the PR and between 55bd468 and a428c33.

📒 Files selected for processing (18)
  • app/src/main/java/com/nextcloud/talk/account/BrowserLoginActivity.kt
  • app/src/main/java/com/nextcloud/talk/account/data/LoginRepository.kt
  • app/src/main/java/com/nextcloud/talk/account/data/io/LocalLoginDataSource.kt
  • app/src/main/java/com/nextcloud/talk/account/viewmodels/BrowserLoginActivityViewModel.kt
  • app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
  • app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
  • app/src/main/java/com/nextcloud/talk/dagger/modules/ViewModelModule.kt
  • app/src/main/java/com/nextcloud/talk/fullscreenfile/FullScreenTextViewerActivity.kt
  • app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt
  • app/src/main/java/com/nextcloud/talk/ui/chooseaccount/ChooseAccountShareToDialogFragment.kt
  • app/src/main/java/com/nextcloud/talk/ui/chooseaccount/ChooseAccountShareToViewModel.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/SortingOrderDialogFragment.java
  • app/src/main/java/com/nextcloud/talk/users/UserManager.kt
  • app/src/main/java/com/nextcloud/talk/utils/FileViewerUtils.kt
  • app/src/main/java/com/nextcloud/talk/utils/singletons/ApplicationWideCurrentRoomHolder.java
  • app/src/main/res/values/strings.xml
  • app/src/test/java/com/nextcloud/talk/login/data/LoginRepositoryTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/res/values/strings.xml

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

This PR eliminates reliance on a mutable global “current user” by binding screens/workers/receivers to an explicit account (internal user id), introduces a dedicated DefaultAccountProvider for true entry points, and updates theming + persistence patterns accordingly.

Changes:

  • Replace CurrentUserProvider* usage with explicit User/KEY_INTERNAL_USER_ID plumbing across activities, view models, workers, receivers, and UI components.
  • Introduce DefaultAccountProvider and per-account theming helpers (HostViewThemeUtils, ViewThemeUtilsFactory, updated MaterialSchemesProvider* APIs).
  • Add DB “repair” + Room partial update paths (and tests), and remove legacy message search resources/classes.
File Description
app/​src/​test/​java/​com/​nextcloud/​talk/​viewmodels/​CallRecordingViewModelTest.kt Adapts call recording VM tests to explicit User binding.
app/​src/​test/​java/​com/​nextcloud/​talk/​users/​DefaultAccountProviderTest.kt Adds unit tests for default-account behavior.
app/​src/​test/​java/​com/​nextcloud/​talk/​messagesearch/​MessageSearchHelperTest.kt Updates message search helper tests to use default user retrieval.
app/​src/​test/​java/​com/​nextcloud/​talk/​login/​data/​LoginRepositoryTest.kt Updates login tests for new LoginResult and reauth flows.
app/​src/​test/​java/​com/​nextcloud/​talk/​location/​viewmodels/​LocationPickerViewModelTest.kt Updates location picker VM tests for explicit User in init params.
app/​src/​test/​java/​com/​nextcloud/​talk/​jobs/​CapabilitiesFetcherTest.kt Updates tests for capabilities partial update API.
app/​src/​test/​java/​com/​nextcloud/​talk/​data/​user/​UsersRepositoryImplTest.kt Adds test ensuring active-user read is read-only.
app/​src/​test/​java/​com/​nextcloud/​talk/​data/​user/​UsersDaoRepairTest.kt Adds Robolectric test for “multiple active users” repair query.
app/​src/​test/​java/​com/​nextcloud/​talk/​data/​user/​UsersDaoPartialUpdateTest.kt Adds Robolectric test validating Room partial updates don’t clobber current.
app/​src/​test/​java/​com/​nextcloud/​talk/​conversationlist/​data/​network/​ConversationListFreshnessIntegrationTest.kt Updates/contributes integration coverage for account-scoped conversation flows.
app/​src/​test/​java/​com/​nextcloud/​talk/​conversationcreation/​ConversationCreationViewModelTest.kt Updates tests for VM now bound to a specific User.
app/​src/​test/​java/​com/​nextcloud/​talk/​contacts/​ContactsViewModelTest.kt Updates tests for assisted-bound contacts VM + adds room state clearing test.
app/​src/​main/​res/​values/​strings.xml Adds reauth “different account” user-facing message; removes message-search strings.
app/​src/​main/​res/​values/​dimens.xml Removes unused dimension tied to deleted message search layouts.
app/​src/​main/​res/​menu/​menu_search.xml Deletes legacy message search menu.
app/​src/​main/​res/​layout/​rv_item_search_message.xml Deletes legacy message search result row layout.
app/​src/​main/​res/​layout/​rv_item_load_more.xml Deletes legacy “load more” row layout.
app/​src/​main/​res/​layout/​activity_message_search.xml Deletes legacy message search activity layout.
app/​src/​main/​java/​com/​nextcloud/​talk/​viewmodels/​CallRecordingViewModel.kt Removes global current-user dependency; requires User via setData.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​ssl/​KeyManager.java Switches client cert alias selection to default user lookup.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​singletons/​ApplicationWideCurrentRoomHolder.java Adds account-aware current-room check.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​rx/​SearchViewObservable.kt Removes obsolete search view Rx helper used by deleted message search UI.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​preview/​ComposePreviewUtilsDaos.kt Extends preview DAO stubs for new DAO APIs and partial updates.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​preview/​ComposePreviewUtils.kt Updates preview wiring for default account + initial user injection.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​database/​user/​UserModule.kt Removes bindings for deprecated current-user providers.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​database/​user/​CurrentUserProviderOldImpl.kt Deletes deprecated provider implementation.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​database/​user/​CurrentUserProviderOld.kt Deletes deprecated provider interface.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​database/​user/​CurrentUserProviderImpl.kt Deletes provider implementation in favor of explicit account binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​database/​user/​CurrentUserProvider.kt Deletes provider interface in favor of explicit account binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​PickImage.kt Passes internal user id to remote file browser for account binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​FileViewerUtils.kt Updates intents/work data to carry internal user id instead of baseUrl/userId strings.
app/​src/​main/​java/​com/​nextcloud/​talk/​users/​DefaultAccountProvider.kt Introduces a dedicated default-account accessor (suspend + blocking).
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​theme/​ViewThemeUtilsFactory.kt Adds per-user factory for ViewThemeUtils.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​theme/​ThemeModule.kt Switches DI-provided schemes to default-account schemes.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​theme/​MaterialSchemesProviderImpl.kt Uses default account provider + theming capability-based caching.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​theme/​MaterialSchemesProvider.kt Renames “current user” schemes API to “default user” schemes API.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​theme/​HostViewThemeUtils.kt Adds helper to reuse host activity’s per-account theme from nested contexts.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​SortingOrderDialogFragment.java Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​SetPhoneNumberDialogFragment.kt Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​SaveToStorageDialogFragment.kt Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​MoreCallActionsDialog.kt Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​FilterConversationFragment.kt Binds fragment to explicit User argument; avoids global current user.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​DialogBanListFragment.kt Fixes fragment recreation by moving params to arguments; binds to explicit User.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​DateTimeCompose.kt Binds reminder UI to explicit User + host theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​AudioOutputDialog.kt Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​dialog/​AttachmentDialog.kt Ensures dialog themes match the host activity account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​chooseaccount/​model/​ChooseAccountShareToViewState.kt Includes selected user in “switch success” state.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​chooseaccount/​ChooseAccountShareToViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​chooseaccount/​ChooseAccountShareToDialogFragment.kt Passes account via args; relaunches list for selected account.
app/​src/​main/​java/​com/​nextcloud/​talk/​ui/​PlaybackSpeedControl.kt Ensures custom view uses host activity theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​translate/​viewmodels/​TranslateViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​translate/​ui/​TranslateActivity.kt Binds activity + VM to explicit account via setUpBoundUserOrFinish().
app/​src/​main/​java/​com/​nextcloud/​talk/​threadsoverview/​viewmodels/​ThreadsOverviewViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​threadsoverview/​ThreadsOverviewActivity.kt Binds activity + VM to explicit account; uses ChatActivity.createIntent.
app/​src/​main/​java/​com/​nextcloud/​talk/​shareditems/​adapters/​SharedItemsAdapter.kt Uses account-bound intents + passes User to context chat.
app/​src/​main/​java/​com/​nextcloud/​talk/​shareditems/​activities/​SharedItemsActivity.kt Binds to explicit account + updates view model wiring.
app/​src/​main/​java/​com/​nextcloud/​talk/​remotefilebrowser/​viewmodels/​RemoteFileBrowserItemsViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​remotefilebrowser/​activities/​RemoteFileBrowserActivity.kt Binds activity to explicit account; removes legacy system-bar code.
app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​ShareRecordingToChatReceiver.kt Requires explicit account id; fails cleanly when missing/removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​MarkAsReadReceiver.kt Requires explicit account id; fails cleanly when missing/removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​DismissRecordingAvailableReceiver.kt Requires explicit account id; fails cleanly when missing/removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​DirectReplyReceiver.kt Requires explicit account id; fails cleanly when missing/removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​raisehand/​viewmodel/​RaiseHandViewModel.kt Removes global current user dependency; requires User via setData.
app/​src/​main/​java/​com/​nextcloud/​talk/​profile/​ProfileActivity.kt Binds profile screen to explicit account and finishes if removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​presenters/​MentionAutocompletePresenter.java Requires explicit User and reuses host activity theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​viewmodels/​PollVoteViewModel.kt Removes global current user dependency; requires User for voting call.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​viewmodels/​PollMainViewModel.kt Reads credentials/baseUrl from bound user instead of global current user.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​viewmodels/​PollCreateViewModel.kt Removes global current user dependency; requires User via setData.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​ui/​PollVoteFragment.kt Applies host theme + passes correct User into vote requests.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​ui/​PollResultsFragment.kt Applies host theme for correct account.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​ui/​PollMainDialogFragment.kt Binds dialog to explicit User args + applies host theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​ui/​PollLoadingFragment.kt Applies host theme for correct account.
app/​src/​main/​java/​com/​nextcloud/​talk/​polls/​ui/​PollCreateDialogFragment.kt Binds dialog to explicit User args + applies host theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​openconversations/​viewmodels/​OpenConversationsViewModel.kt Converts to assisted injection with explicit user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​openconversations/​ListOpenConversationsActivity.kt Binds activity to explicit account + creates account-bound chat intents.
app/​src/​main/​java/​com/​nextcloud/​talk/​messagesearch/​MessageSearchViewModel.kt Removes unused legacy message search view model.
app/​src/​main/​java/​com/​nextcloud/​talk/​mediaviewer/​viewmodels/​MediaViewerViewModel.kt Passes internal user id into download worker input data.
app/​src/​main/​java/​com/​nextcloud/​talk/​mediaviewer/​activities/​MediaViewerActivity.kt Makes media viewer an account-bound activity; updates intent builder.
app/​src/​main/​java/​com/​nextcloud/​talk/​logger/​ui/​LogsActivity.kt Removes unused user-manager injection after account binding changes.
app/​src/​main/​java/​com/​nextcloud/​talk/​location/​viewmodels/​LocationPickerViewModel.kt Makes location picker VM use explicit User from init params.
app/​src/​main/​java/​com/​nextcloud/​talk/​location/​LocationPickerActivity.kt Binds activity to explicit account and propagates user id to sub-activities.
app/​src/​main/​java/​com/​nextcloud/​talk/​location/​GeocodingActivity.kt Binds activity to explicit account.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​UploadAndShareFilesWorker.kt Binds worker to account id and fails cleanly when account is removed.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​SignalingSettingsWorker.java Converts to partial user update for external signaling server.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​NotificationWorker.kt Ensures notification actions include internal user id; avoids default-user fallback.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​LeaveConversationWorker.kt Reads bound account from input data instead of global current user.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​DownloadFileToCacheWorker.kt Uses internal user id input; derives baseUrl/userId from retrieved user.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​ContactAddressBookWorker.kt Uses default account provider for phone book integration.
app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​CapabilitiesFetcher.kt Switches from full user writes to partial capabilities update.
app/​src/​main/​java/​com/​nextcloud/​talk/​invitation/​adapters/​InvitationsAdapter.kt Adapts adapter theming to host activity account theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​invitation/​InvitationsActivity.kt Binds activity to explicit account and creates account-bound chat intents.
app/​src/​main/​java/​com/​nextcloud/​talk/​fullscreenfile/​FullScreenTextViewerActivity.kt Converts to account-bound activity and stops passing username/baseUrl via intent.
app/​src/​main/​java/​com/​nextcloud/​talk/​diagnosis/​DiagnosisViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​diagnosis/​DiagnosisElement.kt Uses default-account or provided account user for diagnosis output.
app/​src/​main/​java/​com/​nextcloud/​talk/​diagnosis/​DiagnosisActivity.kt Binds activity + VM to explicit account; passes user into diagnosis element builder.
app/​src/​main/​java/​com/​nextcloud/​talk/​data/​user/​model/​UserPartialUpdates.kt Introduces Room partial entity models for safe column updates.
app/​src/​main/​java/​com/​nextcloud/​talk/​data/​user/​UsersRepositoryImpl.kt Makes active-user read read-only; adds flow + partial-update APIs.
app/​src/​main/​java/​com/​nextcloud/​talk/​data/​user/​UsersRepository.kt Adds per-user flow, repair method, and column-specific update APIs.
app/​src/​main/​java/​com/​nextcloud/​talk/​data/​user/​UsersDao.kt Adds per-user flow, repair query, and Room @Update(entity=...) partial updates.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationtags/​viewmodels/​ConversationTagsViewModel.kt Converts to assisted injection with explicit user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationlist/​viewmodels/​ConversationsListViewModel.kt Converts to assisted injection; scopes flows to account id.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationlist/​data/​network/​OfflineFirstConversationsRepository.kt Changes room list stream to be account-id parameterized instead of global observed state.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationlist/​data/​OfflineConversationsRepository.kt Updates repository interface to parameterize room list by account id.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationinfoedit/​viewmodel/​ConversationInfoEditViewModel.kt Removes current-user provider dependency; requires user at initialization.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationinfoedit/​ConversationInfoEditActivity.kt Binds to explicit account and initializes picker immediately for process death.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationcreation/​viewmodel/​ConversationCreationViewModel.kt Converts to assisted injection with explicit user binding; returns user id on success.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationcreation/​ui/​CreatedConversation.kt Creates account-bound chat intent when opening created conversation.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversationcreation/​ConversationCreationActivity.kt Binds to explicit account; removes loading for unknown user state.
app/​src/​main/​java/​com/​nextcloud/​talk/​conversation/​RenameConversationDialogFragment.kt Binds dialog to explicit User args and uses host theme.
app/​src/​main/​java/​com/​nextcloud/​talk/​contextchat/​ContextChatViewModel.kt Removes current-user provider dependency; takes User for context fetch.
app/​src/​main/​java/​com/​nextcloud/​talk/​contacts/​components/​ConversationCreationOptions.kt Adds userId parameter and propagates internal user id into intents.
app/​src/​main/​java/​com/​nextcloud/​talk/​contacts/​components/​ContactItemRow.kt Avoids per-row navigation; defers handling to activity once.
app/​src/​main/​java/​com/​nextcloud/​talk/​contacts/​ContactsViewModel.kt Converts to assisted injection; removes “unknown user” states and includes user id on success.
app/​src/​main/​java/​com/​nextcloud/​talk/​contacts/​ContactsScreen.kt Passes bound user id into creation options UI.
app/​src/​main/​java/​com/​nextcloud/​talk/​contacts/​ContactsActivity.kt Binds to explicit account; observes room creation and navigates once.
app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​viewmodel/​StatusViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​viewmodel/​StatusMessageViewModel.kt Converts to assisted injection with explicit current user binding.
app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​ui/​StatusMessageSheet.kt Prevents re-initializing persisted edits on recreation.
app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​ChooseAccountDialogCompose.kt Binds status VMs to default account and scopes them to activity lifecycle.
app/​src/​main/​java/​com/​nextcloud/​talk/​chat/​viewmodels/​ScheduledMessagesViewModel.kt Converts to assisted injection; removes current-user provider dependency.
app/​src/​main/​java/​com/​nextcloud/​talk/​chat/​MessageInputVoiceRecordingFragment.kt Applies host theme for correct account.
app/​src/​main/​java/​com/​nextcloud/​talk/​chat/​MessageInputFragment.kt Avoids accessing chat VM before it exists; uses account-bound mention presenter.
app/​src/​main/​java/​com/​nextcloud/​talk/​callnotification/​CallNotificationActivity.kt Binds activity to explicit account and removes extra user lookups.
app/​src/​main/​java/​com/​nextcloud/​talk/​attachmentpreview/​FileAttachmentPreviewFragment.kt Applies host theme for correct account.
app/​src/​main/​java/​com/​nextcloud/​talk/​application/​NextcloudTalkApplication.kt Runs one-time “multiple active users” repair at startup on app scope.
app/​src/​main/​java/​com/​nextcloud/​talk/​adapters/​items/​MessageResultItem.kt Deletes legacy message search adapter item.
app/​src/​main/​java/​com/​nextcloud/​talk/​adapters/​items/​LoadMoreResultsItem.kt Deletes legacy message search “load more” adapter item.
app/​src/​main/​java/​com/​nextcloud/​talk/​activities/​MainActivity.kt Uses default account provider for routing; passes internal user id in intents.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​viewmodels/​BrowserLoginActivityViewModel.kt Adds support for reauth-specific account id and “different account” outcome.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​data/​io/​LocalLoginDataSource.kt Makes reauth update account-id aware and switch to partial credential updates.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​data/​LoginRepository.kt Introduces LoginResult + account-to-reauthorize plumbing.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​SwitchAccountActivity.kt Ensures account switch relaunches correctly with explicit account id.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​ServerSelectionActivity.kt Removes unused user-manager injection after refactor.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​BrowserLoginActivity.kt Uses injected VM factory; shows “different account” snackbar; passes account-to-reauth.
app/​src/​main/​java/​com/​nextcloud/​talk/​account/​AccountVerificationActivity.kt Passes internal user id when opening conversation list post-verification.
app/​src/​main/​AndroidManifest.xml Removes legacy MessageSearchActivity declaration.
app/​src/​androidTest/​java/​com/​nextcloud/​talk/​ui/​LoginIT.java Updates instrumentation assertion to use DefaultAccountProvider.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/src/main/java/com/nextcloud/talk/account/data/io/LocalLoginDataSource.kt Outdated
Comment thread app/src/main/java/com/nextcloud/talk/contacts/ContactsActivity.kt
…ated

The view model of BrowserLoginActivity survives configuration changes, but
onCreate() starts the login on every recreation. The second login request
replaced the pending one, so the app polled a login session the browser was
not authorizing.

The view model now starts a login only once, so a recreated activity continues
the pending login. After process death, the login is started again.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
When another account than the one to reauthorize logged in and it was not set
up in the app yet, the reauthorization started the account verification and
added it as a new account, leaving the account to reauthorize unauthorized.

During a reauthorization, an account that is not set up is now reported as a
different account as well.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
…session cookie

All accounts share one HTTP client with one cookie store, which keeps cookies
per server only. A session cookie set by a request of one account was sent
along with the requests of another account on the same server, and the server
treated them as the first account: e.g. marcel2's conversation list showed
marcel's conversations and its own conversations returned 404. Clearing the
cookies on account switches didn't help once several accounts send requests
at the same time.

Requests with an Authorization header now neither send nor store cookies, so
they are authenticated by their own credentials only.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
LocalLoginDataSource.updateUser() reported success even if no row was updated,
e.g. when the account was removed between looking it up and storing the new
credentials. It now only reports success if the credentials were stored.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
…isible

The contacts screen collected the created room state as long as the activity
existed, so it could start the chat while in the background. It now collects
it only while started; a conversation created meanwhile is opened when the
screen is started again.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
mahibi marked this pull request as ready for review September 30, 2026 11:54
@mahibi
mahibi requested a balanced review from Copilot September 30, 2026 11:55
@mahibi
mahibi marked this pull request as draft September 30, 2026 12:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

TLS certificate selection remains global, status restoration can skip initialization after process death, and profile resume still blocks the main thread.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Persist initialization state with the ViewModel's edited state

app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​ui/​StatusMessageSheet.kt:68

rememberSaveable can restore isInitialized = true after process death, but the activity-scoped ViewModel is then a new instance with empty fields. If this sheet is restored open, initialization is skipped and the status editor shows blank/default state. Keep the initialization flag in the ViewModel (or its SavedStateHandle) so the flag and edited state share the same lifetime.

Medium severity Avoid synchronous Room lookup on the main thread during resume

app/​src/​main/​java/​com/​nextcloud/​talk/​profile/​ProfileActivity.kt:177

This executes a Room lookup synchronously on the main thread every time the activity resumes. A slow or contended database blocks rendering and can cause an ANR, reintroducing the main-thread runBlocking problem this account migration is intended to remove. Load/observe the bound account from a lifecycle coroutine and continue the profile refresh after it is available.

Comment thread app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java
RemoteWipeInterceptor removed the first account on the server of any request
answered with 401, even if the request carried no credentials or those of
another account, and even if the server hadn't requested a wipe. A 401 for an
image loaded without credentials therefore removed an account whose
credentials were still valid, and with several accounts on the same server it
could remove the wrong one.

It now only reacts to a 401 for a request with credentials, and resolves the
account by exactly those credentials.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Chat media previews, quote thumbnails, link previews of the own server, the
media viewer and conversation avatars of open conversations were loaded
without credentials. They only worked when the shared cookie store happened to
hold a server session, which could belong to another account on the same
server, and failed since authenticated requests no longer store session
cookies.

They now send the credentials of the screen's account, only for URLs of its
own server.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
KeyManager always offered the client certificate of the default account. Since
screens and background work are bound to their account, requests of other
accounts are common, and they offered the wrong certificate or none, so mutual
TLS failed for them.

The certificate is now chosen by the server of the connection: the one used by
the accounts on it. Only if accounts on the same server use different
certificates, the default account decides, as the TLS connection is shared.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

URL-prefix authentication checks can expose credentials or misidentify remote wipes, and new synchronous database lookups block Android main-thread entry points.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (1)
Previously missed (7)

In code that hasn't changed since last review

Medium severity Restored initialization state skips ViewModel reinitialization

app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​ui/​StatusMessageSheet.kt:71

rememberSaveable restores isInitialized = true after process death, but the activity-scoped ViewModel is newly created with empty fields. When the sheet is restored, initialization and predefined-status loading are skipped, leaving stale/blank state. Keep the initialization flag in the ViewModel (which survives rotation but resets after process death), or persist the edited state with SavedStateHandle.

Medium severity runBlocking in worker blocks the main thread

app/​src/​main/​java/​com/​nextcloud/​talk/​jobs/​LeaveConversationWorker.kt:54

WorkManager invokes ListenableWorker.startWork() on the main thread, so this runBlocking waits there for the Room lookup before the Rx work is scheduled. Make this a CoroutineWorker or compose the lookup asynchronously into the returned future.

Medium severity runBlocking on resume blocks the main thread

app/​src/​main/​java/​com/​nextcloud/​talk/​profile/​ProfileActivity.kt:177

onResume() runs on the main thread, so this runBlocking waits synchronously for a Room query on every resume and can stall rendering. Load the bound account in a lifecycle coroutine (or observe userFlow(id)) and continue the profile refresh after the suspend call completes.

Medium severity runBlocking in receiver blocks main-thread reply handling

app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​DirectReplyReceiver.kt:77

BroadcastReceiver.onReceive() executes on the main thread, and this runBlocking now waits there for Room I/O before handling the reply. Use goAsync() and perform the account lookup and reply work in an application coroutine, finishing the pending result afterward.

Medium severity runBlocking in receiver blocks main-thread action handling

app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​DismissRecordingAvailableReceiver.kt:61

BroadcastReceiver.onReceive() executes on the main thread, so this runBlocking synchronously waits for Room I/O. Use goAsync() and perform the account lookup and action in an application coroutine, then finish the pending result.

Medium severity runBlocking in receiver blocks main-thread action handling

app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​MarkAsReadReceiver.kt:65

BroadcastReceiver.onReceive() executes on the main thread, so this runBlocking synchronously waits for Room I/O. Use goAsync() and perform the account lookup and action in an application coroutine, then finish the pending result.

Medium severity runBlocking in receiver blocks main-thread action handling

app/​src/​main/​java/​com/​nextcloud/​talk/​receivers/​ShareRecordingToChatReceiver.kt:61

BroadcastReceiver.onReceive() executes on the main thread, so this runBlocking synchronously waits for Room I/O. Use goAsync() and perform the account lookup and action in an application coroutine, then finish the pending result.

Comment thread app/src/main/java/com/nextcloud/talk/ui/chat/LinkMessage.kt
Comment thread app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt Outdated
Splits the lookup of the account whose credentials a request carries from
building the wipe candidate, and replaces the chain of early returns.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Whether an image request got the account's credentials was decided by
checking whether its URL starts with the server's base URL. That also matches
other hosts like https://cloud.example.com.attacker.test for
https://cloud.example.com, so e.g. a link preview image could send the
account's credentials to a foreign host.

UriUtils.isOnServer() now compares scheme, host, port and base path, and is
used wherever credentials are attached based on the URL.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
The media viewer and the shared items screen built their credentials from the
user id instead of the login name. For accounts whose login name differs from
the user id, e.g. an email address, the requests were only authenticated by a
shared session cookie and got 401 since authenticated requests no longer use
cookies. They now use the login name.

A failed response is now reported as an HttpException instead of a
NullPointerException on the missing body.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Account restoration, client-certificate endpoint matching, and authenticated image cache isolation have unresolved correctness and security issues.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Process death skips ViewModel initialization

app/​src/​main/​java/​com/​nextcloud/​talk/​chooseaccount/​ui/​StatusMessageSheet.kt:70

rememberSaveable also survives process death, while StatusMessageViewModel is recreated with empty flows. If the process dies with this sheet open, isInitialized is restored as true, so init, backup detection, and predefined-status loading are skipped and the restored sheet shows blank/default state. Keep the initialization guard in the ViewModel (or a SavedStateHandle) so rotation preserves edits but a newly created ViewModel initializes normally.

Medium severity Hostname-only matching merges distinct TLS endpoints

app/​src/​main/​java/​com/​nextcloud/​talk/​utils/​ssl/​ClientCertificateSelector.kt:29

Matching only the hostname merges distinct TLS endpoints. Accounts for https://cloud.example.com and https://cloud.example.com:8443 are treated as the same server, so this can offer the certificate configured for one endpoint to the other (or choose the default account's alias when both differ). Include the peer port and compare it with each base URL's effective port.

Comment thread app/src/main/java/com/nextcloud/talk/ui/ImageAuthHeader.kt
@mahibi
mahibi marked this pull request as ready for review September 30, 2026 15:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 847160e6-8948-43d4-afe7-f98e813acbbf

📥 Commits

Reviewing files that changed from the base of the PR and between abbe099 and 06acebb.

📒 Files selected for processing (22)
  • app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt
  • app/src/main/java/com/nextcloud/talk/extensions/ImageViewExtensions.kt
  • app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerActivity.kt
  • app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt
  • app/src/main/java/com/nextcloud/talk/mediaviewer/viewmodels/MediaViewerViewModel.kt
  • app/src/main/java/com/nextcloud/talk/openconversations/ListOpenConversationsActivity.kt
  • app/src/main/java/com/nextcloud/talk/openconversations/OpenConversationsScreen.kt
  • app/src/main/java/com/nextcloud/talk/shareditems/repositories/SharedItemsRepository.kt
  • app/src/main/java/com/nextcloud/talk/shareditems/repositories/SharedItemsRepositoryImpl.kt
  • app/src/main/java/com/nextcloud/talk/shareditems/viewmodels/SharedItemsViewModel.kt
  • app/src/main/java/com/nextcloud/talk/ui/ImageAuthHeader.kt
  • app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageScaffold.kt
  • app/src/main/java/com/nextcloud/talk/ui/chat/LinkMessage.kt
  • app/src/main/java/com/nextcloud/talk/ui/chat/MediaGroupMessage.kt
  • app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt
  • app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt
  • app/src/main/java/com/nextcloud/talk/utils/UriUtils.kt
  • app/src/main/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelector.kt
  • app/src/main/java/com/nextcloud/talk/utils/ssl/KeyManager.java
  • app/src/test/java/com/nextcloud/talk/utils/RemoteWipeInterceptorTest.kt
  • app/src/test/java/com/nextcloud/talk/utils/UriUtilsTest.kt
  • app/src/test/java/com/nextcloud/talk/utils/ssl/ClientCertificateSelectorTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread app/src/main/java/com/nextcloud/talk/ui/chat/LinkMessage.kt
Comment thread app/src/main/java/com/nextcloud/talk/ui/ImageAuthHeader.kt
Comment thread app/src/main/java/com/nextcloud/talk/utils/RemoteWipeInterceptor.kt Outdated
RemoteWipeInterceptor is an application interceptor, so it sees the response
after redirects. It resolved the account from the original request, which
still carries the credentials after a redirect to another host, although
OkHttp dropped them for the redirected request that got the 401. It now uses
the request of the response.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants