Only mark username corrupted after repeated failures.

2025-12-20 11:08:31 +00:00 · 2023-11-16 22:54:19 -05:00
parent 1aa7175006
commit 8023285b9d
5 changed files with 68 additions and 25 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/internal/InternalSettingsFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/internal/InternalSettingsFragment.kt
@@ -12,7 +12,6 @@ import androidx.navigation.fragment.findNavController
 import androidx.recyclerview.widget.LinearLayoutManager
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import org.signal.core.util.AppUtil
-import org.signal.core.util.Hex
 import org.signal.core.util.concurrent.SignalExecutors
 import org.signal.core.util.concurrent.SimpleTask
 import org.signal.core.util.logging.Log
@@ -717,7 +716,7 @@ class InternalSettingsFragment : DSLSettingsFragment(R.string.preferences__inter
            .setTitle("Corrupt your username?")
            .setMessage("Are you sure? You might not be able to get your original username back.")
            .setPositiveButton(android.R.string.ok) { _, _ ->
-              val random = "${Hex.toStringCondensed(Util.getSecretBytes(4))}.${Random.nextInt(1, 100)}"
+              val random = "${(1..5).map { ('a'..'z').random() }.joinToString(separator = "") }.${Random.nextInt(1, 100)}"

              SignalStore.account().username = random
              SignalDatabase.recipients.setUsername(Recipient.self().id, random)
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RefreshOwnProfileJob.java
@@ -23,6 +23,7 @@ import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint;
 import org.thoughtcrime.securesms.keyvalue.AccountValues;
 import org.thoughtcrime.securesms.keyvalue.SignalStore;
 import org.thoughtcrime.securesms.profiles.ProfileName;
+import org.thoughtcrime.securesms.profiles.manage.UsernameRepository;
 import org.thoughtcrime.securesms.recipients.Recipient;
 import org.thoughtcrime.securesms.storage.StorageSyncHelper;
 import org.thoughtcrime.securesms.subscription.Subscriber;
@@ -267,9 +268,10 @@ public class RefreshOwnProfileJob extends BaseJob {

      if (TextUtils.isEmpty(localUsernameHash) && TextUtils.isEmpty(remoteUsernameHash)) {
        Log.d(TAG, "Local and remote username hash are both empty. Considering validated.");
+        UsernameRepository.onUsernameConsistencyValidated();
      } else if (!Objects.equals(localUsernameHash, remoteUsernameHash)) {
        Log.w(TAG, "Local username hash does not match server username hash. Local hash: " + (TextUtils.isEmpty(localUsername) ? "empty" : "present") + ", Remote hash: " + (TextUtils.isEmpty(remoteUsernameHash) ? "empty" : "present"));
-        SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.USERNAME_AND_LINK_CORRUPTED);
+        UsernameRepository.onUsernameMismatchDetected();
        return;
      } else {
        Log.d(TAG, "Username validated.");
@@ -278,7 +280,8 @@ public class RefreshOwnProfileJob extends BaseJob {
      Log.w(TAG, "Failed perform synchronization check during username phase.", e);
    } catch (BaseUsernameException e) {
      Log.w(TAG, "Our local username data is invalid!", e);
-      SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.USERNAME_AND_LINK_CORRUPTED);
+      UsernameRepository.onUsernameMismatchDetected();
+      return;
    }

    try {
@@ -291,9 +294,7 @@ public class RefreshOwnProfileJob extends BaseJob {

        if (!remoteUsername.getUsername().equals(SignalStore.account().getUsername())) {
          Log.w(TAG, "The remote username decrypted ok, but the decrypted username did not match our local username!");
-          SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.LINK_CORRUPTED);
-          SignalStore.account().setUsernameLink(null);
-          StorageSyncHelper.scheduleSyncForDataChange();
+          UsernameRepository.onUsernameLinkMismatchDetected();
        } else {
          Log.d(TAG, "Username link validated.");
        }
@@ -304,13 +305,11 @@ public class RefreshOwnProfileJob extends BaseJob {
      Log.w(TAG, "Failed perform synchronization check during the username link phase.", e);
    } catch (BaseUsernameException e) {
      Log.w(TAG, "Failed to decrypt username link using the remote encrypted username and our local entropy!", e);
-      SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.LINK_CORRUPTED);
-      SignalStore.account().setUsernameLink(null);
-      StorageSyncHelper.scheduleSyncForDataChange();
+      UsernameRepository.onUsernameLinkMismatchDetected();
    }

    if (validated) {
-      SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.IN_SYNC);
+      UsernameRepository.onUsernameConsistencyValidated();
    }
  }

--- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/AccountValues.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/AccountValues.kt
@@ -6,7 +6,6 @@ import android.content.SharedPreferences
 import android.preference.PreferenceManager
 import androidx.annotation.VisibleForTesting
 import org.signal.core.util.Base64
-import org.signal.core.util.LongSerializer
 import org.signal.core.util.logging.Log
 import org.signal.libsignal.protocol.IdentityKey
 import org.signal.libsignal.protocol.IdentityKeyPair
@@ -72,6 +71,7 @@ internal class AccountValues internal constructor(store: KeyValueStore) : Signal
    private const val KEY_USERNAME_LINK_ENTROPY = "account.username_link_entropy"
    private const val KEY_USERNAME_LINK_SERVER_ID = "account.username_link_server_id"
    private const val KEY_USERNAME_SYNC_STATE = "phoneNumberPrivacy.usernameSyncState"
+    private const val KEY_USERNAME_SYNC_ERROR_COUNT = "phoneNumberPrivacy.usernameErrorCount"

    @VisibleForTesting
    const val KEY_E164 = "account.e164"
@@ -397,17 +397,14 @@ internal class AccountValues internal constructor(store: KeyValueStore) : Signal
   * There are some cases where our username may fall out of sync with the service. In particular, we may get a new value for our username from
   * storage service but then find that it doesn't match what's on the service.
   */
-  var usernameSyncState: UsernameSyncState by enumValue(
-    KEY_USERNAME_SYNC_STATE,
-    UsernameSyncState.IN_SYNC,
-    object : LongSerializer<UsernameSyncState> {
-      override fun serialize(data: UsernameSyncState): Long {
-        Log.i(TAG, "Marking username sync state as: $data")
-        return data.serialize()
-      }
-      override fun deserialize(data: Long): UsernameSyncState = UsernameSyncState.deserialize(data)
+  var usernameSyncState: UsernameSyncState
+    get() = UsernameSyncState.deserialize(getLong(KEY_USERNAME_SYNC_STATE, UsernameSyncState.IN_SYNC.serialize()))
+    set(value) {
+      Log.i(TAG, "Marking username sync state as: $value")
+      putLong(KEY_USERNAME_SYNC_STATE, value.serialize())
    }
-  )
+
+  var usernameSyncErrorCount: Int by integerValue(KEY_USERNAME_SYNC_ERROR_COUNT, 0)

  private fun clearLocalCredentials() {
    putString(KEY_SERVICE_PASSWORD, Util.getSecret(18))
@@ -520,7 +517,7 @@ internal class AccountValues internal constructor(store: KeyValueStore) : Signal

    companion object {
      fun deserialize(value: Long): UsernameSyncState {
-        return values().firstOrNull { it.value == value } ?: throw IllegalArgumentException("Invalud value: $value")
+        return values().firstOrNull { it.value == value } ?: throw IllegalArgumentException("Invalid value: $value")
      }
    }
  }
--- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameRepository.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/UsernameRepository.kt
@@ -83,7 +83,8 @@ object UsernameRepository {

  private val URL_REGEX = """(https://)?signal.me/?#eu/([a-zA-Z0-9+\-_/]+)""".toRegex()

-  val BASE_URL = "https://signal.me/#eu/"
+  private const val BASE_URL = "https://signal.me/#eu/"
+  private const val USERNAME_SYNC_ERROR_THRESHOLD = 3

  private val accountManager: SignalServiceAccountManager get() = ApplicationDependencies.getSignalServiceAccountManager()

@@ -153,6 +154,7 @@ object UsernameRepository {

          if (SignalStore.account().usernameSyncState == AccountValues.UsernameSyncState.LINK_CORRUPTED) {
            SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.IN_SYNC
+            SignalStore.account().usernameSyncErrorCount = 0
          }

          SignalDatabase.recipients.markNeedsSync(Recipient.self().id)
@@ -251,6 +253,44 @@ object UsernameRepository {
    return BASE_URL + base64
  }

+  @JvmStatic
+  fun onUsernameConsistencyValidated() {
+    SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.IN_SYNC
+
+    if (SignalStore.account().usernameSyncErrorCount > 0) {
+      Log.i(TAG, "Username consistency validated. There were previously ${SignalStore.account().usernameSyncErrorCount} error(s).")
+      SignalStore.account().usernameSyncErrorCount = 0
+    }
+  }
+
+  @JvmStatic
+  fun onUsernameMismatchDetected() {
+    SignalStore.account().usernameSyncErrorCount++
+
+    if (SignalStore.account().usernameSyncErrorCount >= USERNAME_SYNC_ERROR_THRESHOLD) {
+      Log.w(TAG, "We've now seen ${SignalStore.account().usernameSyncErrorCount} mismatches in a row. Marking username and link as corrupted.")
+      SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.USERNAME_AND_LINK_CORRUPTED
+      SignalStore.account().usernameSyncErrorCount = 0
+    } else {
+      Log.w(TAG, "Username mismatch reported. At ${SignalStore.account().usernameSyncErrorCount} / $USERNAME_SYNC_ERROR_THRESHOLD tries.")
+    }
+  }
+
+  @JvmStatic
+  fun onUsernameLinkMismatchDetected() {
+    SignalStore.account().usernameSyncErrorCount++
+
+    if (SignalStore.account().usernameSyncErrorCount >= USERNAME_SYNC_ERROR_THRESHOLD) {
+      Log.w(TAG, "We've now seen ${SignalStore.account().usernameSyncErrorCount} mismatches in a row. Marking link as corrupted.")
+      SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.LINK_CORRUPTED
+      SignalStore.account().usernameLink = null
+      SignalStore.account().usernameSyncErrorCount = 0
+      StorageSyncHelper.scheduleSyncForDataChange()
+    } else {
+      Log.w(TAG, "Link mismatch reported. At ${SignalStore.account().usernameSyncErrorCount} / $USERNAME_SYNC_ERROR_THRESHOLD tries.")
+    }
+  }
+
  @WorkerThread
  private fun reserveUsernameInternal(nickname: String): Result<UsernameState.Reserved, UsernameSetResult> {
    return try {
@@ -293,6 +333,7 @@ object UsernameRepository {
      SignalStore.account().usernameLink = null
      SignalDatabase.recipients.setUsername(Recipient.self().id, reserved.username)
      SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.IN_SYNC
+      SignalStore.account().usernameSyncErrorCount = 0

      SignalDatabase.recipients.markNeedsSync(Recipient.self().id)
      StorageSyncHelper.scheduleSyncForDataChange()
@@ -345,6 +386,7 @@ object UsernameRepository {
      SignalStore.account().username = null
      SignalStore.account().usernameLink = null
      SignalStore.account().usernameSyncState = AccountValues.UsernameSyncState.IN_SYNC
+      SignalStore.account().usernameSyncErrorCount = 0
      SignalDatabase.recipients.markNeedsSync(Recipient.self().id)
      StorageSyncHelper.scheduleSyncForDataChange()
      Log.i(TAG, "[deleteUsername] Successfully deleted the username.")
--- a/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncHelper.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncHelper.java
@@ -17,6 +17,7 @@ import org.thoughtcrime.securesms.database.model.RecipientRecord;
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
 import org.thoughtcrime.securesms.jobs.RetrieveProfileAvatarJob;
 import org.thoughtcrime.securesms.jobs.StorageSyncJob;
+import org.thoughtcrime.securesms.keyvalue.AccountValues;
 import org.thoughtcrime.securesms.keyvalue.PhoneNumberPrivacyValues;
 import org.thoughtcrime.securesms.keyvalue.SignalStore;
 import org.thoughtcrime.securesms.payments.Entropy;
@@ -207,7 +208,6 @@ public final class StorageSyncHelper {
    SignalStore.storyValues().setUserHasViewedOnboardingStory(update.getNew().hasViewedOnboardingStory());
    SignalStore.storyValues().setFeatureDisabled(update.getNew().isStoriesDisabled());
    SignalStore.storyValues().setUserHasSeenGroupStoryEducationSheet(update.getNew().hasSeenGroupStoryEducationSheet());
-    SignalStore.account().setUsername(update.getNew().getUsername());

    if (update.getNew().getStoryViewReceiptsState() == OptionalBool.UNSET) {
      SignalStore.storyValues().setViewedReceiptsEnabled(update.getNew().isReadReceiptsEnabled());
@@ -236,6 +236,12 @@ public final class StorageSyncHelper {
      ApplicationDependencies.getJobManager().add(new RetrieveProfileAvatarJob(self, update.getNew().getAvatarUrlPath().get()));
    }

+    if (!update.getNew().getUsername().equals(update.getOld().getUsername())) {
+      SignalStore.account().setUsername(update.getNew().getUsername());
+      SignalStore.account().setUsernameSyncState(AccountValues.UsernameSyncState.IN_SYNC);
+      SignalStore.account().setUsernameSyncErrorCount(0);
+    }
+
    if (update.getNew().getUsernameLink() != null) {
      SignalStore.account().setUsernameLink(
        new UsernameLinkComponents(