From 1da4c0302ee1c781caad62bfd48438ddebf81d4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl?= Date: Fri, 17 Jul 2026 17:11:51 +0200 Subject: [PATCH] Point the classic app identifiers at HealthChat rather than Element (#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ClassicAppManager migrates an existing user's session and encryption keys by reading the legacy app's app-group container and keychain. Those identifiers defaulted to Element's own, which this team cannot provision, so loadAccounts() threw invalidAppGroupIdentifier — caught and logged, never surfaced. Since both apps share a bundle id, the result would be every existing user silently logged out and losing local history when they update. The legacy identifiers belong to HealthChat and are provisionable under the same team, so repoint them rather than dropping them. Also stop sending accounts without key backup to a dead end. Upstream shows those users a sheet asking them to open Element Classic and set up backup first, but this update replaces that app and nothing registers its URL scheme, so the sheet's only button does nothing. Sign in directly instead: the user re-verifies the device and loses only history that by definition has no backup. --- ElementX.xcodeproj/project.pbxproj | 12 +++---- .../AuthenticationStartScreenViewModel.swift | 36 +++++++++---------- .../SupportingFiles/ElementX.entitlements | 1 + ElementX/SupportingFiles/target.yml | 9 +++-- ...henticationStartScreenViewModelTests.swift | 28 +++++---------- Variants/HealthChat/healthchat.yml | 8 +++++ 6 files changed, 47 insertions(+), 47 deletions(-) diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 0e97c11ec..cf4cfcecd 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -10303,9 +10303,9 @@ CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CLASSIC_APP_DEEP_LINK_URL = "element://open"; - CLASSIC_APP_GROUP_IDENTIFIER = group.im.vector; - CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER = "$(DEVELOPMENT_TEAM).im.vector.app.keychain.shared"; - CLASSIC_APP_KEYCHAIN_SERVICE_IDENTIFIER = "im.vector.app.encryption-manager-service"; + CLASSIC_APP_GROUP_IDENTIFIER = group.ch.healthchat; + CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER = "$(DEVELOPMENT_TEAM).ch.healthchat.element.keychain.shared"; + CLASSIC_APP_KEYCHAIN_SERVICE_IDENTIFIER = "ch.healthchat.element.encryption-manager-service"; COPY_PHASE_STRIP = NO; CURRENT_PROJECT_VERSION = 1; DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym"; @@ -10378,9 +10378,9 @@ CLANG_WARN_UNREACHABLE_CODE = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CLASSIC_APP_DEEP_LINK_URL = "element://open"; - CLASSIC_APP_GROUP_IDENTIFIER = group.im.vector; - CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER = "$(DEVELOPMENT_TEAM).im.vector.app.keychain.shared"; - CLASSIC_APP_KEYCHAIN_SERVICE_IDENTIFIER = "im.vector.app.encryption-manager-service"; + CLASSIC_APP_GROUP_IDENTIFIER = group.ch.healthchat; + CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER = "$(DEVELOPMENT_TEAM).ch.healthchat.element.keychain.shared"; + CLASSIC_APP_KEYCHAIN_SERVICE_IDENTIFIER = "ch.healthchat.element.encryption-manager-service"; COPY_PHASE_STRIP = NO; CURRENT_PROJECT_VERSION = 1; DEBUG_INFORMATION_FORMAT = dwarf; diff --git a/ElementX/Sources/Screens/Authentication/StartScreen/AuthenticationStartScreenViewModel.swift b/ElementX/Sources/Screens/Authentication/StartScreen/AuthenticationStartScreenViewModel.swift index 31a30d796..c366b94bc 100644 --- a/ElementX/Sources/Screens/Authentication/StartScreen/AuthenticationStartScreenViewModel.swift +++ b/ElementX/Sources/Screens/Authentication/StartScreen/AuthenticationStartScreenViewModel.swift @@ -40,30 +40,27 @@ class AuthenticationStartScreenViewModel: AuthenticationStartScreenViewModelType canReportProblem = isBugReportServiceEnabled let isQRCodeScanningSupported = !ProcessInfo.processInfo.isiOSAppOnMac - let isClassicAppAccountAllowed = if let classicAppAccount = authenticationService.classicAppAccount { - appSettings.accountProviders.contains(serverName: classicAppAccount.serverName, orBaseURL: classicAppAccount.homeserverURL) - } else { - false - } + let classicAppAccountProvider = authenticationService.classicAppAccount?.serverName + let isClassicAppAccountAllowed = classicAppAccountProvider.map { appSettings.accountProviders.contains($0) } ?? false let initialViewState = if !appSettings.allowOtherAccountProviders { // We don't show the create account button when custom providers are disallowed. // The assumption here being that if you're running a custom app, your users will already be created. - AuthenticationStartScreenViewState(serverNameOrBaseURL: appSettings.accountProviders.count == 1 ? appSettings.accountProviders[0].serverNameOrBaseURL : nil, + AuthenticationStartScreenViewState(serverName: appSettings.accountProviders.count == 1 ? appSettings.accountProviders[0] : nil, showCreateAccountButton: false, showQRCodeLoginButton: isQRCodeScanningSupported, classicAppMode: isClassicAppAccountAllowed ? authenticationService.classicAppAccount.map { .welcomeBack($0) } : nil, hideBrandChrome: appSettings.hideBrandChrome) } else if let provisioningParameters { // We only show the "Sign in to …" button when using a provisioning link. - AuthenticationStartScreenViewState(serverNameOrBaseURL: provisioningParameters.accountProvider, + AuthenticationStartScreenViewState(serverName: provisioningParameters.accountProvider, showCreateAccountButton: false, showQRCodeLoginButton: false, classicAppMode: nil, hideBrandChrome: appSettings.hideBrandChrome) } else { // The default configuration. - AuthenticationStartScreenViewState(serverNameOrBaseURL: nil, + AuthenticationStartScreenViewState(serverName: nil, showCreateAccountButton: appSettings.showCreateAccountButton, showQRCodeLoginButton: isQRCodeScanningSupported, classicAppMode: authenticationService.classicAppAccount.map { .welcomeBack($0) }, @@ -114,25 +111,26 @@ class AuthenticationStartScreenViewModel: AuthenticationStartScreenViewModelType private func login(classicAppAccount: ClassicAppAccount? = nil) async { if let classicAppAccount { - if classicAppAccount.state.availableSecrets == .requiresBackup { - state.bindings.showClassicAppBackupInstructions = true - } else { - await loginDirectly(using: classicAppAccount.serverName, - loginHint: "mxid:\(classicAppAccount.userID)", - fallbackHomeserverURL: classicAppAccount.homeserverURL) - } - } else if let serverNameOrBaseURL = state.serverNameOrBaseURL { - await loginDirectly(using: serverNameOrBaseURL, loginHint: provisioningParameters?.loginHint) + // HealthChat diverges from upstream here. Upstream sends `.requiresBackup` accounts to a sheet + // asking the user to set up key backup in Element Classic first. HealthChat ships with the same + // bundle id as the legacy app, so this update *replaces* it — there is no Classic app left to + // open, and the sheet's only button resolves to nothing. Fall through to a normal sign-in + // instead: the user re-verifies and loses only history that by definition has no backup. + await configureAccountProvider(classicAppAccount.serverName, + loginHint: "mxid:\(classicAppAccount.userID)", + fallbackHomeserverURL: classicAppAccount.homeserverURL) + } else if let serverName = state.serverName { + await configureAccountProvider(serverName, loginHint: provisioningParameters?.loginHint) } else { actionsSubject.send(.login) // No need to configure anything here, continue the flow. } } - private func loginDirectly(using serverNameOrBaseURL: String, loginHint: String? = nil, fallbackHomeserverURL: URL? = nil) async { + private func configureAccountProvider(_ accountProvider: String, loginHint: String? = nil, fallbackHomeserverURL: URL? = nil) async { startLoading() defer { stopLoading() } - if case .failure = await authenticationService.configure(for: serverNameOrBaseURL, flow: .login) { + if case .failure = await authenticationService.configure(for: accountProvider, flow: .login) { // Try the fallback URL before showing an error. if let fallbackHomeserverURL, case .success = await authenticationService.configure(for: fallbackHomeserverURL.absoluteString, flow: .login) { diff --git a/ElementX/SupportingFiles/ElementX.entitlements b/ElementX/SupportingFiles/ElementX.entitlements index 6bb0cab4f..8758015f2 100644 --- a/ElementX/SupportingFiles/ElementX.entitlements +++ b/ElementX/SupportingFiles/ElementX.entitlements @@ -23,6 +23,7 @@ keychain-access-groups $(KEYCHAIN_ACCESS_GROUP_IDENTIFIER) + $(CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER) diff --git a/ElementX/SupportingFiles/target.yml b/ElementX/SupportingFiles/target.yml index 85dc30454..2f903f83a 100644 --- a/ElementX/SupportingFiles/target.yml +++ b/ElementX/SupportingFiles/target.yml @@ -129,12 +129,17 @@ targets: com.apple.developer.usernotifications.communication: true com.apple.security.app-sandbox: true com.apple.security.application-groups: - # HealthChat: dropped $(CLASSIC_APP_GROUP_IDENTIFIER) (group.im.vector) — owned by Element's team, not provisionable here. + # HealthChat: dropped $(CLASSIC_APP_GROUP_IDENTIFIER) — not because it is un-provisionable, + # but because it resolves to group.ch.healthchat, the same value as $(APP_GROUP_IDENTIFIER) + # below. The legacy container is already entitled; listing it twice would duplicate the entry. - $(APP_GROUP_IDENTIFIER) com.apple.security.network.client: true keychain-access-groups: - # HealthChat: dropped $(CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER) (im.vector.app.*) — not provisionable here. - $(KEYCHAIN_ACCESS_GROUP_IDENTIFIER) + # HealthChat: the legacy app's keychain group. ClassicAppManager reads the account and + # crypto-store keys from here to migrate existing users. Same team prefix, so it is + # provisionable — unlike Element's im.vector.app.* default in app.yml. + - $(CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER) settings: base: diff --git a/UnitTests/Sources/AuthenticationStartScreenViewModelTests.swift b/UnitTests/Sources/AuthenticationStartScreenViewModelTests.swift index 000d560cb..ea5c2d9bd 100644 --- a/UnitTests/Sources/AuthenticationStartScreenViewModelTests.swift +++ b/UnitTests/Sources/AuthenticationStartScreenViewModelTests.swift @@ -238,8 +238,8 @@ final class AuthenticationStartScreenViewModelTests { } @Test - func classicAppAccountRequiresBackup() async throws { - // Given a view model with a Classic app account that requires backup before signing in. + func classicAppAccountRequiresBackupFallsThroughToLogin() async throws { + // Given a view model with a Classic app account that has no usable key backup. let classicAppAccount = makeClassicAppAccount() await setupViewModel(classicAppAccount: classicAppAccount, availableSecrets: .requiresBackup) guard case .welcomeBack(let account) = context.viewState.classicAppMode else { @@ -248,27 +248,15 @@ final class AuthenticationStartScreenViewModelTests { } #expect(account.state.availableSecrets == .requiresBackup) - // When continuing with the Classic account while backup is required. - var deferred = deferFulfillment(context.observe(\.viewState.bindings.showClassicAppBackupInstructions)) { $0 } + // When continuing with the Classic account. + let deferred = deferFulfillment(viewModel.actions) { $0.isLoginDirectlyWithOAuth } context.send(viewAction: .continueWithClassic(classicAppAccount)) - // Then the backup instructions should be shown. + // Then the flow continues to a normal sign-in rather than the backup instructions sheet. + // HealthChat replaces the Classic app in place, so those instructions could never be followed. try await deferred.fulfill() - - // When the user completes the backup in the Classic app and the app returns to the foreground. - classicAppManager?.availableSecretsForReturnValue = .complete - deferred = deferFulfillment(context.observe(\.viewState.bindings.showClassicAppBackupInstructions)) { !$0 } - notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) - - // Then the backup instructions sheet should be dismissed. - try await deferred.fulfill() - - // When the user continues with the Classic account again. - let deferredAction = deferFulfillment(viewModel.actions) { $0.isLoginDirectlyWithOAuth } - context.send(viewAction: .continueWithClassic(classicAppAccount)) - - // Then the flow should continue the login process. - try await deferredAction.fulfill() + #expect(context.viewState.bindings.showClassicAppBackupInstructions == false) + #expect(client.urlForOauthOauthConfigurationPromptLoginHintDeviceIdAdditionalScopesReceivedArguments?.loginHint == "mxid:\(classicAppAccount.userID)") } // MARK: - Helpers diff --git a/Variants/HealthChat/healthchat.yml b/Variants/HealthChat/healthchat.yml index c03356763..aabf7145d 100644 --- a/Variants/HealthChat/healthchat.yml +++ b/Variants/HealthChat/healthchat.yml @@ -11,3 +11,11 @@ settings: APP_GROUP_IDENTIFIER: group.ch.healthchat BASE_BUNDLE_IDENTIFIER: ch.healthchat.element DEVELOPMENT_TEAM: 6G37K72W82 + + # The legacy element-ios HealthChat's identifiers. ClassicAppManager reads the account and + # crypto-store keys from these to migrate existing users on an in-place update. Upstream's + # defaults in app.yml point at Element's own (group.im.vector / im.vector.app.*), which this + # team cannot provision — repoint, never remove. Failure here is silent. + CLASSIC_APP_GROUP_IDENTIFIER: group.ch.healthchat + CLASSIC_APP_KEYCHAIN_SERVICE_IDENTIFIER: ch.healthchat.element.encryption-manager-service + CLASSIC_APP_KEYCHAIN_ACCESS_GROUP_IDENTIFIER: "$(DEVELOPMENT_TEAM).ch.healthchat.element.keychain.shared"