Handle PIN creation failure better.

This commit is contained in:
Greyson Parrelli 2020-05-09 12:02:18 -04:00
parent 14858adc88
commit 618b1b5ace
9 changed files with 52 additions and 14 deletions

View File

@ -20,7 +20,6 @@ import org.thoughtcrime.securesms.logging.Log;
import org.thoughtcrime.securesms.migrations.ApplicationMigrationActivity;
import org.thoughtcrime.securesms.migrations.ApplicationMigrations;
import org.thoughtcrime.securesms.pin.PinRestoreActivity;
import org.thoughtcrime.securesms.profiles.ProfileName;
import org.thoughtcrime.securesms.profiles.edit.EditProfileActivity;
import org.thoughtcrime.securesms.push.SignalServiceNetworkAccess;
import org.thoughtcrime.securesms.recipients.Recipient;
@ -186,7 +185,7 @@ public abstract class PassphraseRequiredActionBarActivity extends BaseActionBarA
}
private boolean userMustCreateSignalPin() {
return !SignalStore.registrationValues().isRegistrationComplete() && !SignalStore.kbsValues().hasPin();
return !SignalStore.registrationValues().isRegistrationComplete() && !SignalStore.kbsValues().hasPin() && !SignalStore.kbsValues().lastPinCreateFailed();
}
private boolean userMustSetProfileName() {

View File

@ -15,10 +15,11 @@ import java.security.SecureRandom;
public final class KbsValues {
public static final String V2_LOCK_ENABLED = "kbs.v2_lock_enabled";
private static final String MASTER_KEY = "kbs.registration_lock_master_key";
private static final String TOKEN_RESPONSE = "kbs.token_response";
private static final String LOCK_LOCAL_PIN_HASH = "kbs.registration_lock_local_pin_hash";
public static final String V2_LOCK_ENABLED = "kbs.v2_lock_enabled";
private static final String MASTER_KEY = "kbs.registration_lock_master_key";
private static final String TOKEN_RESPONSE = "kbs.token_response";
private static final String LOCK_LOCAL_PIN_HASH = "kbs.registration_lock_local_pin_hash";
private static final String LAST_CREATE_FAILED_TIMESTAMP = "kbs.last_create_failed_timestamp";
private final KeyValueStore store;
@ -36,6 +37,7 @@ public final class KbsValues {
.remove(V2_LOCK_ENABLED)
.remove(TOKEN_RESPONSE)
.remove(LOCK_LOCAL_PIN_HASH)
.remove(LAST_CREATE_FAILED_TIMESTAMP)
.commit();
}
@ -53,6 +55,7 @@ public final class KbsValues {
.putString(TOKEN_RESPONSE, tokenResponse)
.putBlob(MASTER_KEY, masterKey.serialize())
.putString(LOCK_LOCAL_PIN_HASH, localPinHash)
.putLong(LAST_CREATE_FAILED_TIMESTAMP, -1)
.commit();
}
@ -61,10 +64,25 @@ public final class KbsValues {
store.beginWrite().putBoolean(V2_LOCK_ENABLED, enabled).apply();
}
/**
* Whether or not registration lock V2 is enabled.
*/
public synchronized boolean isV2RegistrationLockEnabled() {
return store.getBoolean(V2_LOCK_ENABLED, false);
}
/** Should only be set by {@link org.thoughtcrime.securesms.pin.PinState}. */
public synchronized void onPinCreateFailure() {
store.beginWrite().putLong(LAST_CREATE_FAILED_TIMESTAMP, System.currentTimeMillis()).apply();
}
/**
* Whether or not the last time the user attempted to create a PIN, it failed.
*/
public synchronized boolean lastPinCreateFailed() {
return store.getLong(LAST_CREATE_FAILED_TIMESTAMP, -1) > 0;
}
/**
* Finds or creates the master key. Therefore this will always return a master key whether backed
* up or not.

View File

@ -31,6 +31,7 @@ final class ConfirmKbsPinRepository {
return PinSetResult.SUCCESS;
} catch (IOException | UnauthenticatedResponseException e) {
Log.w(TAG, e);
PinState.onPinCreateFailure();
return PinSetResult.FAILURE;
}
}, resultConsumer::accept);

View File

@ -22,6 +22,7 @@ public class LogSectionPin implements LogSection {
.append("ReglockV1: ").append(TextSecurePreferences.isV1RegistrationLockEnabled(context)).append("\n")
.append("ReglockV2: ").append(SignalStore.kbsValues().isV2RegistrationLockEnabled()).append("\n")
.append("Signal PIN: ").append(SignalStore.kbsValues().hasPin()).append("\n")
.append("Last Creation Failed: ").append(SignalStore.kbsValues().lastPinCreateFailed()).append("\n")
.append("Needs Account Restore: ").append(SignalStore.storageServiceValues().needsAccountRestore()).append("\n")
.append("PIN Required at Registration: ").append(SignalStore.registrationValues().pinWasRequiredAtRegistration()).append("\n")
.append("Registration Complete: ").append(SignalStore.registrationValues().isRegistrationComplete());

View File

@ -4,17 +4,21 @@ import androidx.annotation.VisibleForTesting;
import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
import org.thoughtcrime.securesms.keyvalue.SignalStore;
import org.thoughtcrime.securesms.logging.Log;
import org.thoughtcrime.securesms.util.FeatureFlags;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
import java.util.Locale;
import java.util.concurrent.TimeUnit;
class PinsForAllSchedule implements MegaphoneSchedule {
private static final String TAG = Log.tag(PinsForAllSchedule.class);
@VisibleForTesting
static final long DAYS_UNTIL_FULLSCREEN = 8L;
private final MegaphoneSchedule schedule = new RecurringSchedule(TimeUnit.DAYS.toMillis(2));
private final MegaphoneSchedule schedule = new RecurringSchedule(TimeUnit.HOURS.toMillis(2));
static boolean shouldDisplayFullScreen(long firstVisible, long currentTime) {
if (!FeatureFlags.pinsForAllMandatory()) {
@ -37,7 +41,9 @@ class PinsForAllSchedule implements MegaphoneSchedule {
if (shouldDisplayFullScreen(firstVisible, currentTime)) {
return true;
} else {
return schedule.shouldDisplay(seenCount, lastSeen, firstVisible, currentTime);
boolean shouldDisplay = schedule.shouldDisplay(seenCount, lastSeen, firstVisible, currentTime);
Log.i(TAG, String.format(Locale.ENGLISH, "seenCount: %d, lastSeen: %d, firstVisible: %d, currentTime: %d, result: %b", seenCount, lastSeen, firstVisible, currentTime, shouldDisplay));
return shouldDisplay;
}
}

View File

@ -27,6 +27,7 @@ import org.whispersystems.signalservice.api.kbs.HashedPin;
import org.whispersystems.signalservice.api.kbs.MasterKey;
import org.whispersystems.signalservice.internal.contacts.crypto.UnauthenticatedResponseException;
import org.whispersystems.signalservice.internal.contacts.entities.TokenResponse;
import org.whispersystems.signalservice.internal.storage.protos.SignalStorage;
import java.io.IOException;
import java.util.Arrays;
@ -175,6 +176,15 @@ public final class PinState {
updateState(buildInferredStateFromOtherFields());
}
/**
* Invoked when PIN creation fails.
*/
public static synchronized void onPinCreateFailure() {
if (getState() == State.NO_REGISTRATION_LOCK) {
SignalStore.kbsValues().onPinCreateFailure();
}
}
/**
* Invoked whenever a Signal PIN user enables registration lock.
*/

View File

@ -33,7 +33,7 @@ public final class RegistrationCodeRequest {
*/
@SuppressLint("StaticFieldLeak")
static void requestSmsVerificationCode(@NonNull Context context, @NonNull Credentials credentials, @Nullable String captchaToken, @NonNull Mode mode, @NonNull SmsVerificationCodeCallback callback) {
Log.d(TAG, String.format("SMS Verification requested for %s captcha %s", credentials.getE164number(), captchaToken));
Log.d(TAG, "SMS Verification requested");
new AsyncTask<Void, Void, VerificationRequestResult>() {
@Override

View File

@ -8,10 +8,12 @@ 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.BaseUnitTest;
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.logging.Log;
import org.thoughtcrime.securesms.util.FeatureFlags;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
@ -26,19 +28,22 @@ import static org.powermock.api.mockito.PowerMockito.mockStatic;
import static org.powermock.api.mockito.PowerMockito.when;
@RunWith(PowerMockRunner.class)
@PrepareForTest({ApplicationDependencies.class, SignalStore.class, FeatureFlags.class, RegistrationValues.class, KbsValues.class, TextSecurePreferences.class})
public class PinsForAllScheduleTest {
@PrepareForTest({ApplicationDependencies.class, SignalStore.class, FeatureFlags.class, RegistrationValues.class, KbsValues.class, TextSecurePreferences.class })
public class PinsForAllScheduleTest extends BaseUnitTest {
private final PinsForAllSchedule testSubject = new PinsForAllSchedule();
private final RegistrationValues registrationValues = mock(RegistrationValues.class);
private final KbsValues kbsValues = mock(KbsValues.class);
@Before
public void setUp() {
public void setUp() throws Exception {
super.setUp();
mockStatic(ApplicationDependencies.class);
mockStatic(SignalStore.class);
mockStatic(FeatureFlags.class);
mockStatic(TextSecurePreferences.class);
mockStatic(Log.class);
when(ApplicationDependencies.getApplication()).thenReturn(mock(Application.class));
when(SignalStore.registrationValues()).thenReturn(registrationValues);
when(SignalStore.kbsValues()).thenReturn(kbsValues);

View File

@ -180,8 +180,6 @@ public final class KeyBackupService {
// if the number of tries has not fallen, the pin is correct we're just using an out of date token
boolean canRetry = remainingTries == status.getTries();
Log.i(TAG, String.format(Locale.US, "Token MISMATCH %d %d", remainingTries, status.getTries()));
Log.i(TAG, String.format("Last token %s", Hex.toStringCondensed(token.getToken())));
Log.i(TAG, String.format("Next token %s", Hex.toStringCondensed(nextToken.getToken())));
throw new TokenException(nextToken, canRetry);
case MISSING:
Log.i(TAG, "Restore OK! No data though");