From a3987457402e70f1b64e3ebc0dabfcda7678f93f Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Fri, 17 Feb 2023 09:27:31 -0400 Subject: [PATCH] Implement username is out of sync banner. --- ...wnProfileJob__checkUsernameIsInSyncTest.kt | 175 ++++++++++++++++++ .../reminder/UsernameOutOfSyncReminder.kt | 36 ++++ .../ConversationListFragment.java | 6 + .../securesms/database/RecipientTable.kt | 10 + .../securesms/jobs/RefreshOwnProfileJob.java | 48 +++++ .../keyvalue/PhoneNumberPrivacyValues.java | 19 +- .../manage/UsernameEditRepository.java | 3 + app/src/main/res/values/ids.xml | 2 + app/src/main/res/values/strings.xml | 6 + 9 files changed, 302 insertions(+), 3 deletions(-) create mode 100644 app/src/androidTest/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob__checkUsernameIsInSyncTest.kt create mode 100644 app/src/main/java/org/thoughtcrime/securesms/components/reminder/UsernameOutOfSyncReminder.kt diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob__checkUsernameIsInSyncTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob__checkUsernameIsInSyncTest.kt new file mode 100644 index 000000000..e79f80471 --- /dev/null +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob__checkUsernameIsInSyncTest.kt @@ -0,0 +1,175 @@ +package org.thoughtcrime.securesms.jobs + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import okhttp3.mockwebserver.MockResponse +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.signal.libsignal.usernames.Username +import org.thoughtcrime.securesms.database.SignalDatabase +import org.thoughtcrime.securesms.dependencies.InstrumentationApplicationDependencyProvider +import org.thoughtcrime.securesms.keyvalue.SignalStore +import org.thoughtcrime.securesms.testing.Get +import org.thoughtcrime.securesms.testing.Put +import org.thoughtcrime.securesms.testing.SignalActivityRule +import org.thoughtcrime.securesms.testing.failure +import org.thoughtcrime.securesms.testing.success +import org.whispersystems.signalservice.internal.push.ReserveUsernameResponse +import org.whispersystems.signalservice.internal.push.WhoAmIResponse +import org.whispersystems.util.Base64UrlSafe + +@Suppress("ClassName") +@RunWith(AndroidJUnit4::class) +class RefreshOwnProfileJob__checkUsernameIsInSyncTest { + + @get:Rule + val harness = SignalActivityRule() + + @After + fun tearDown() { + InstrumentationApplicationDependencyProvider.clearHandlers() + SignalStore.phoneNumberPrivacy().clearUsernameOutOfSync() + } + + @Test + fun givenNoLocalUsername_whenICheckUsernameIsInSync_thenIExpectNoFailures() { + // WHEN + RefreshOwnProfileJob.checkUsernameIsInSync() + } + + @Test + fun givenLocalUsernameDoesNotMatchServerUsername_whenICheckUsernameIsInSync_thenIExpectRetry() { + // GIVEN + var didReserve = false + var didConfirm = false + val username = "hello.32" + val serverUsername = "hello.3232" + SignalDatabase.recipients.setUsername(harness.self.id, username) + InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers( + Get("/v1/accounts/whoami") { r -> + MockResponse().success( + WhoAmIResponse().apply { + usernameHash = Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(serverUsername)) + } + ) + }, + Put("/v1/accounts/username_hash/reserve") { r -> + didReserve = true + MockResponse().success(ReserveUsernameResponse(Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(username)))) + }, + Put("/v1/accounts/username_hash/confirm") { r -> + didConfirm = true + MockResponse().success() + } + ) + + // WHEN + RefreshOwnProfileJob.checkUsernameIsInSync() + + // THEN + assertTrue(didReserve) + assertTrue(didConfirm) + assertFalse(SignalStore.phoneNumberPrivacy().isUsernameOutOfSync) + } + + @Test + fun givenLocalAndNoServer_whenICheckUsernameIsInSync_thenIExpectRetry() { + // GIVEN + var didReserve = false + var didConfirm = false + val username = "hello.32" + SignalDatabase.recipients.setUsername(harness.self.id, username) + InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers( + Get("/v1/accounts/whoami") { r -> + MockResponse().success(WhoAmIResponse()) + }, + Put("/v1/accounts/username_hash/reserve") { r -> + didReserve = true + MockResponse().success(ReserveUsernameResponse(Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(username)))) + }, + Put("/v1/accounts/username_hash/confirm") { r -> + didConfirm = true + MockResponse().success() + } + ) + + // WHEN + RefreshOwnProfileJob.checkUsernameIsInSync() + + // THEN + assertTrue(didReserve) + assertTrue(didConfirm) + assertFalse(SignalStore.phoneNumberPrivacy().isUsernameOutOfSync) + } + + @Test + fun givenLocalAndServerMatch_whenICheckUsernameIsInSync_thenIExpectNoRetry() { + // GIVEN + var didReserve = false + var didConfirm = false + val username = "hello.32" + SignalDatabase.recipients.setUsername(harness.self.id, username) + InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers( + Get("/v1/accounts/whoami") { r -> + MockResponse().success( + WhoAmIResponse().apply { + usernameHash = Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(username)) + } + ) + }, + Put("/v1/accounts/username_hash/reserve") { r -> + didReserve = true + MockResponse().success(ReserveUsernameResponse(Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(username)))) + }, + Put("/v1/accounts/username_hash/confirm") { r -> + didConfirm = true + MockResponse().success() + } + ) + + // WHEN + RefreshOwnProfileJob.checkUsernameIsInSync() + + // THEN + assertFalse(didReserve) + assertFalse(didConfirm) + assertFalse(SignalStore.phoneNumberPrivacy().isUsernameOutOfSync) + } + + @Test + fun givenMismatchAndReservationFails_whenICheckUsernameIsInSync_thenIExpectNoConfirm() { + // GIVEN + var didReserve = false + var didConfirm = false + val username = "hello.32" + SignalDatabase.recipients.setUsername(harness.self.id, username) + InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers( + Get("/v1/accounts/whoami") { r -> + MockResponse().success( + WhoAmIResponse().apply { + usernameHash = Base64UrlSafe.encodeBytesWithoutPadding(Username.hash("${username}23")) + } + ) + }, + Put("/v1/accounts/username_hash/reserve") { r -> + didReserve = true + MockResponse().failure(418) + }, + Put("/v1/accounts/username_hash/confirm") { r -> + didConfirm = true + MockResponse().success() + } + ) + + // WHEN + RefreshOwnProfileJob.checkUsernameIsInSync() + + // THEN + assertTrue(didReserve) + assertFalse(didConfirm) + assertTrue(SignalStore.phoneNumberPrivacy().isUsernameOutOfSync) + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/reminder/UsernameOutOfSyncReminder.kt b/app/src/main/java/org/thoughtcrime/securesms/components/reminder/UsernameOutOfSyncReminder.kt new file mode 100644 index 000000000..fa7b36825 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/components/reminder/UsernameOutOfSyncReminder.kt @@ -0,0 +1,36 @@ +package org.thoughtcrime.securesms.components.reminder + +import android.content.Context +import org.thoughtcrime.securesms.R +import org.thoughtcrime.securesms.keyvalue.SignalStore +import org.thoughtcrime.securesms.util.FeatureFlags + +/** + * Displays a reminder message when the local username gets out of sync with + * what the server thinks our username is. + */ +class UsernameOutOfSyncReminder(context: Context) : Reminder( + null, + context.getString(R.string.UsernameOutOfSyncReminder__something_went_wrong) +) { + + init { + addAction( + Action( + context.getString(R.string.UsernameOutOfSyncReminder__fix_now), + R.id.reminder_action_fix_username + ) + ) + } + + override fun isDismissable(): Boolean { + return false + } + + companion object { + @JvmStatic + fun isEligible(): Boolean { + return FeatureFlags.usernames() && SignalStore.phoneNumberPrivacy().isUsernameOutOfSync + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java index d523b2904..b5c735176 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversationlist/ConversationListFragment.java @@ -108,6 +108,7 @@ import org.thoughtcrime.securesms.components.reminder.Reminder; import org.thoughtcrime.securesms.components.reminder.ReminderView; import org.thoughtcrime.securesms.components.reminder.ServiceOutageReminder; import org.thoughtcrime.securesms.components.reminder.UnauthorizedReminder; +import org.thoughtcrime.securesms.components.reminder.UsernameOutOfSyncReminder; import org.thoughtcrime.securesms.components.settings.app.notifications.manual.NotificationProfileSelectionFragment; import org.thoughtcrime.securesms.components.settings.app.subscription.errors.UnexpectedSubscriptionCancellation; import org.thoughtcrime.securesms.components.voice.VoiceNoteMediaControllerOwner; @@ -153,6 +154,7 @@ import org.thoughtcrime.securesms.notifications.MarkReadReceiver; import org.thoughtcrime.securesms.notifications.profiles.NotificationProfile; import org.thoughtcrime.securesms.payments.preferences.PaymentsActivity; import org.thoughtcrime.securesms.permissions.Permissions; +import org.thoughtcrime.securesms.profiles.manage.ManageProfileActivity; import org.thoughtcrime.securesms.ratelimit.RecaptchaProofBottomSheetFragment; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; @@ -784,6 +786,8 @@ public class ConversationListFragment extends MainFragment implements ActionMode CdsTemporaryErrorBottomSheet.show(getChildFragmentManager()); } else if (reminderActionId == R.id.reminder_action_cds_permanent_error_learn_more) { CdsPermanentErrorBottomSheet.show(getChildFragmentManager()); + } else if (reminderActionId == R.id.reminder_action_fix_username) { + startActivity(ManageProfileActivity.getIntentForUsernameEdit(requireContext())); } } @@ -1043,6 +1047,8 @@ public class ConversationListFragment extends MainFragment implements ActionMode return Optional.of(new CdsTemporyErrorReminder(context)); } else if (CdsPermanentErrorReminder.isEligible()) { return Optional.of(new CdsPermanentErrorReminder(context)); + } else if (UsernameOutOfSyncReminder.isEligible()) { + return Optional.of(new UsernameOutOfSyncReminder(context)); } else { return Optional.empty(); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index d8c9b9549..4b639a324 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -2085,6 +2085,16 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } } + fun getUsername(id: RecipientId): String? { + return writableDatabase.query(TABLE_NAME, arrayOf(USERNAME), "$ID = ?", SqlUtil.buildArgs(id), null, null, null).use { + if (it.moveToFirst()) { + it.requireString(USERNAME) + } else { + null + } + } + } + fun setUsername(id: RecipientId, username: String?) { writableDatabase.withinTransaction { if (username != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java index aef468bba..e646146ed 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java @@ -4,8 +4,11 @@ import android.text.TextUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import org.signal.core.util.logging.Log; +import org.signal.libsignal.usernames.BaseUsernameException; +import org.signal.libsignal.usernames.Username; import org.signal.libsignal.zkgroup.profiles.ExpiringProfileKeyCredential; import org.signal.libsignal.zkgroup.profiles.ProfileKey; import org.thoughtcrime.securesms.badges.BadgeRepository; @@ -23,6 +26,7 @@ import org.thoughtcrime.securesms.profiles.ProfileName; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.subscription.Subscriber; import org.thoughtcrime.securesms.util.Base64; +import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.ProfileUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; @@ -34,8 +38,12 @@ import org.whispersystems.signalservice.api.push.exceptions.PushNetworkException import org.whispersystems.signalservice.api.subscriptions.ActiveSubscription; import org.whispersystems.signalservice.api.util.ExpiringProfileCredentialUtil; import org.whispersystems.signalservice.internal.ServiceResponse; +import org.whispersystems.signalservice.internal.push.ReserveUsernameResponse; +import org.whispersystems.signalservice.internal.push.WhoAmIResponse; +import org.whispersystems.util.Base64UrlSafe; import java.io.IOException; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Objects; @@ -142,6 +150,10 @@ public class RefreshOwnProfileJob extends BaseJob { .ifPresent(expiringProfileKeyCredential -> setExpiringProfileKeyCredential(self, ProfileKeyUtil.getSelfProfileKey(), expiringProfileKeyCredential)); StoryOnboardingDownloadJob.Companion.enqueueIfNeeded(); + + if (FeatureFlags.usernames()) { + checkUsernameIsInSync(); + } } private void setExpiringProfileKeyCredential(@NonNull Recipient recipient, @@ -241,6 +253,42 @@ public class RefreshOwnProfileJob extends BaseJob { } } + @VisibleForTesting + static void checkUsernameIsInSync() { + try { + String localUsername = SignalDatabase.recipients().getUsername(Recipient.self().getId()); + boolean hasLocalUsername = !TextUtils.isEmpty(localUsername); + + if (!hasLocalUsername) { + return; + } + + WhoAmIResponse whoAmIResponse = ApplicationDependencies.getSignalServiceAccountManager().getWhoAmI(); + boolean hasServerUsername = !TextUtils.isEmpty(whoAmIResponse.getUsernameHash()); + String serverUsernameHash = whoAmIResponse.getUsernameHash(); + String localUsernameHash = Base64UrlSafe.encodeBytesWithoutPadding(Username.hash(localUsername)); + + if (!hasServerUsername || !Objects.equals(localUsernameHash, serverUsernameHash)) { + tryToReserveAndConfirmLocalUsername(localUsername, localUsernameHash); + } + } catch (IOException | BaseUsernameException e) { + Log.w(TAG, "Failed perform synchronization check", e); + } + } + + private static void tryToReserveAndConfirmLocalUsername(@NonNull String localUsername, @NonNull String localUsernameHash) { + try { + ReserveUsernameResponse response = ApplicationDependencies.getSignalServiceAccountManager() + .reserveUsername(Collections.singletonList(localUsernameHash)); + + ApplicationDependencies.getSignalServiceAccountManager() + .confirmUsername(localUsername, response); + } catch (IOException e) { + Log.d(TAG, "Failed to synchronize username.", e); + SignalStore.phoneNumberPrivacy().markUsernameOutOfSync(); + } + } + private void setProfileBadges(@Nullable List badges) throws IOException { if (badges == null) { return; diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PhoneNumberPrivacyValues.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PhoneNumberPrivacyValues.java index 7837158a1..94765d05f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PhoneNumberPrivacyValues.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PhoneNumberPrivacyValues.java @@ -11,9 +11,10 @@ import java.util.List; public final class PhoneNumberPrivacyValues extends SignalStoreValues { - public static final String SHARING_MODE = "phoneNumberPrivacy.sharingMode"; - public static final String LISTING_MODE = "phoneNumberPrivacy.listingMode"; - public static final String LISTING_TIMESTAMP = "phoneNumberPrivacy.listingMode.timestamp"; + public static final String SHARING_MODE = "phoneNumberPrivacy.sharingMode"; + public static final String LISTING_MODE = "phoneNumberPrivacy.listingMode"; + public static final String LISTING_TIMESTAMP = "phoneNumberPrivacy.listingMode.timestamp"; + public static final String USERNAME_OUT_OF_SYNC = "phoneNumberPrivacy.usernameOutOfSync"; private static final Collection REGULAR_CERTIFICATE = Collections.singletonList(CertificateType.UUID_AND_E164); private static final Collection PRIVACY_CERTIFICATE = Collections.singletonList(CertificateType.UUID_ONLY); @@ -68,6 +69,18 @@ public final class PhoneNumberPrivacyValues extends SignalStoreValues { return getLong(LISTING_TIMESTAMP, 0); } + public void markUsernameOutOfSync() { + putBoolean(USERNAME_OUT_OF_SYNC, true); + } + + public void clearUsernameOutOfSync() { + putBoolean(USERNAME_OUT_OF_SYNC, false); + } + + public boolean isUsernameOutOfSync() { + return getBoolean(USERNAME_OUT_OF_SYNC, false); + } + /** * If you respect {@link #getPhoneNumberSharingMode}, then you will only ever need to fetch and store * these certificates types. diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameEditRepository.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameEditRepository.java index ef5aaeb18..7feb848e8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameEditRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameEditRepository.java @@ -9,6 +9,7 @@ import org.signal.libsignal.usernames.BaseUsernameException; import org.signal.libsignal.usernames.Username; import org.thoughtcrime.securesms.database.SignalDatabase; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.util.UsernameUtil; import org.whispersystems.signalservice.api.SignalServiceAccountManager; @@ -87,6 +88,7 @@ class UsernameEditRepository { try { accountManager.confirmUsername(reserved.getUsername(), reserved.getReserveUsernameResponse()); SignalDatabase.recipients().setUsername(Recipient.self().getId(), reserved.getUsername()); + SignalStore.phoneNumberPrivacy().clearUsernameOutOfSync(); Log.i(TAG, "[confirmUsername] Successfully reserved username."); return UsernameSetResult.SUCCESS; } catch (UsernameTakenException e) { @@ -106,6 +108,7 @@ class UsernameEditRepository { try { accountManager.deleteUsername(); SignalDatabase.recipients().setUsername(Recipient.self().getId(), null); + SignalStore.phoneNumberPrivacy().clearUsernameOutOfSync(); Log.i(TAG, "[deleteUsername] Successfully deleted the username."); return UsernameDeleteResult.SUCCESS; } catch (IOException e) { diff --git a/app/src/main/res/values/ids.xml b/app/src/main/res/values/ids.xml index 0e9fe7ea5..82269d217 100644 --- a/app/src/main/res/values/ids.xml +++ b/app/src/main/res/values/ids.xml @@ -18,6 +18,8 @@ + + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 99f94f32f..d97d6705f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -959,6 +959,12 @@ Username deleted + + + Something went wrong with your username, it\'s no longer assigned to your account. You can try and set it again or choose a new one. + + Fix now + No groups in common