From 2d24c8c525b269b2751560dd3466b6f2ed08fb9d Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 3 Feb 2020 12:35:39 -0500 Subject: [PATCH] Update conditions for PIN megaphone. Handles additional corner cases. - Shows megaphone when you register with a v1 pin. - Show fullscreen when you fail to set a PIN during registration. --- .../megaphone/PinsForAllSchedule.java | 31 ++++++++++++++++++- .../service/CodeVerificationRequest.java | 4 --- .../megaphone/PinsForAllScheduleTest.java | 13 +++++++- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/megaphone/PinsForAllSchedule.java b/app/src/main/java/org/thoughtcrime/securesms/megaphone/PinsForAllSchedule.java index bce2442dea..dd165fc857 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/megaphone/PinsForAllSchedule.java +++ b/app/src/main/java/org/thoughtcrime/securesms/megaphone/PinsForAllSchedule.java @@ -2,8 +2,10 @@ package org.thoughtcrime.securesms.megaphone; import androidx.annotation.VisibleForTesting; +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.util.FeatureFlags; +import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; import java.util.concurrent.TimeUnit; @@ -19,6 +21,10 @@ class PinsForAllSchedule implements MegaphoneSchedule { private final MegaphoneSchedule schedule = new RecurringSchedule(TimeUnit.DAYS.toMillis(2)); static boolean shouldDisplayFullScreen(long firstVisible, long currentTime) { + if (pinCreationFailedDuringRegistration()) { + return true; + } + if (firstVisible == 0L) { return false; } else { @@ -46,10 +52,33 @@ class PinsForAllSchedule implements MegaphoneSchedule { } private static boolean isEnabled() { - if (FeatureFlags.pinsForAllMegaphoneKillSwitch() || SignalStore.registrationValues().pinWasRequiredAtRegistration()) { + if (FeatureFlags.pinsForAllMegaphoneKillSwitch()) { + return false; + } + + if (pinCreationFailedDuringRegistration()) { + return true; + } + + if (newlyRegisteredV1PinUser()) { + return true; + } + + if (SignalStore.registrationValues().pinWasRequiredAtRegistration()) { return false; } return FeatureFlags.pinsForAll(); } + + private static boolean pinCreationFailedDuringRegistration() { + return SignalStore.registrationValues().pinWasRequiredAtRegistration() && + !SignalStore.kbsValues().isV2RegistrationLockEnabled() && + !TextSecurePreferences.isV1RegistrationLockEnabled(ApplicationDependencies.getApplication()); + } + + private static final boolean newlyRegisteredV1PinUser() { + return SignalStore.registrationValues().pinWasRequiredAtRegistration() && TextSecurePreferences.isV1RegistrationLockEnabled(ApplicationDependencies.getApplication()); + } + } diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java b/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java index 9ce60e3769..ed1621ef9f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/service/CodeVerificationRequest.java @@ -250,10 +250,6 @@ public final class CodeVerificationRequest { TextSecurePreferences.setDeprecatedRegistrationLockPin(context, pin); //noinspection deprecation Only acceptable place to write the old pin enabled state. TextSecurePreferences.setV1RegistrationLockEnabled(context, pin != null); - if (pin != null) { - Log.i(TAG, "Pin V1 successfully entered during registration, scheduling a migration to Pin V2"); - ApplicationDependencies.getJobManager().add(new RegistrationPinV2MigrationJob()); - } } else { SignalStore.kbsValues().setRegistrationLockMasterKey(kbsData, PinHashing.localPinHash(pin)); repostPinToResetTries(context, pin, kbsData); diff --git a/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java b/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java index b40c026dbf..0312f0ba18 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/megaphone/PinsForAllScheduleTest.java @@ -5,31 +5,40 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.keyvalue.KbsValues; import org.thoughtcrime.securesms.keyvalue.RegistrationValues; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.util.FeatureFlags; +import org.thoughtcrime.securesms.util.TextSecurePreferences; import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.mockStatic; import static org.powermock.api.mockito.PowerMockito.when; @RunWith(PowerMockRunner.class) -@PrepareForTest({SignalStore.class, FeatureFlags.class, RegistrationValues.class}) +@PrepareForTest({ApplicationDependencies.class, SignalStore.class, FeatureFlags.class, RegistrationValues.class, KbsValues.class, TextSecurePreferences.class}) public class PinsForAllScheduleTest { private final PinsForAllSchedule testSubject = new PinsForAllSchedule(); private final RegistrationValues registrationValues = mock(RegistrationValues.class); + private final KbsValues kbsValues = mock(KbsValues.class); @Before public void setUp() { + mockStatic(ApplicationDependencies.class); mockStatic(SignalStore.class); mockStatic(FeatureFlags.class); + mockStatic(TextSecurePreferences.class); when(SignalStore.registrationValues()).thenReturn(registrationValues); + when(SignalStore.kbsValues()).thenReturn(kbsValues); + when(TextSecurePreferences.isV1RegistrationLockEnabled(any())).thenReturn(false); } @Test @@ -155,6 +164,7 @@ public class PinsForAllScheduleTest { public void whenUserIsANewInstallAndFlagIsDisabled_whenIShouldDisplay_thenIExpectFalse() { // GIVEN when(registrationValues.pinWasRequiredAtRegistration()).thenReturn(true); + when(kbsValues.isV2RegistrationLockEnabled()).thenReturn(true); when(FeatureFlags.pinsForAll()).thenReturn(false); // WHEN @@ -168,6 +178,7 @@ public class PinsForAllScheduleTest { public void whenUserIsANewInstallAndFlagIsEnabled_whenIShouldDisplay_thenIExpectFalse() { // GIVEN when(registrationValues.pinWasRequiredAtRegistration()).thenReturn(true); + when(kbsValues.isV2RegistrationLockEnabled()).thenReturn(true); when(FeatureFlags.pinsForAll()).thenReturn(true); // WHEN