Skip to content

Commit

Permalink
Handle PIN creation failure better.
Browse files Browse the repository at this point in the history
  • Loading branch information
greyson-signal committed May 9, 2020
1 parent 14858ad commit 618b1b5
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
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;
Expand Down Expand Up @@ -186,7 +185,7 @@ private int getApplicationState(boolean locked) {
}

private boolean userMustCreateSignalPin() {
return !SignalStore.registrationValues().isRegistrationComplete() && !SignalStore.kbsValues().hasPin();
return !SignalStore.registrationValues().isRegistrationComplete() && !SignalStore.kbsValues().hasPin() && !SignalStore.kbsValues().lastPinCreateFailed();
}

private boolean userMustSetProfileName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@

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;

Expand All @@ -36,6 +37,7 @@ public void clearRegistrationLockAndPin() {
.remove(V2_LOCK_ENABLED)
.remove(TOKEN_RESPONSE)
.remove(LOCK_LOCAL_PIN_HASH)
.remove(LAST_CREATE_FAILED_TIMESTAMP)
.commit();
}

Expand All @@ -53,6 +55,7 @@ public synchronized void setKbsMasterKey(@NonNull KbsPinData pinData, @NonNull S
.putString(TOKEN_RESPONSE, tokenResponse)
.putBlob(MASTER_KEY, masterKey.serialize())
.putString(LOCK_LOCAL_PIN_HASH, localPinHash)
.putLong(LAST_CREATE_FAILED_TIMESTAMP, -1)
.commit();
}

Expand All @@ -61,10 +64,25 @@ public synchronized void setV2RegistrationLockEnabled(boolean enabled) {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ void setPin(@NonNull KbsPin kbsPin, @NonNull PinKeyboardType keyboard, @NonNull
return PinSetResult.SUCCESS;
} catch (IOException | UnauthenticatedResponseException e) {
Log.w(TAG, e);
PinState.onPinCreateFailure();
return PinSetResult.FAILURE;
}
}, resultConsumer::accept);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@

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()) {
Expand All @@ -37,7 +41,9 @@ public boolean shouldDisplay(int seenCount, long lastSeen, long firstVisible, lo
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;
}
}

Expand Down
10 changes: 10 additions & 0 deletions app/src/main/java/org/thoughtcrime/securesms/pin/PinState.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -175,6 +176,15 @@ public static synchronized void onPinChangedOrCreated(@NonNull Context context,
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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
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;

Expand All @@ -26,19 +28,22 @@
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ private KbsPinData restorePin(HashedPin hashedPin, TokenResponse token)
// 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");
Expand Down

0 comments on commit 618b1b5

Please sign in to comment.