Improve handling of unlinked device during send.
This commit is contained in:
parent
5e8cebdc87
commit
8c2ff2f1c2
@ -241,5 +241,6 @@ class OtherClient(val serviceId: ServiceId, val e164: String, val identityKeyPai
|
||||
override fun deleteAllStaleOneTimeKyberPreKeys(threshold: Long, minCount: Int) = throw UnsupportedOperationException()
|
||||
override fun loadLastResortKyberPreKeys(): List<KyberPreKeyRecord> = throw UnsupportedOperationException()
|
||||
override fun isMultiDevice(): Boolean = throw UnsupportedOperationException()
|
||||
override fun setMultiDevice(isMultiDevice: Boolean) = throw UnsupportedOperationException()
|
||||
}
|
||||
}
|
||||
|
||||
@ -58,6 +58,11 @@ public class SignalServiceAccountDataStoreImpl implements SignalServiceAccountDa
|
||||
return SignalStore.account().isMultiDevice();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setMultiDevice(boolean isMultiDevice) {
|
||||
SignalStore.account().setMultiDevice(isMultiDevice);
|
||||
}
|
||||
|
||||
@Override
|
||||
public IdentityKeyPair getIdentityKeyPair() {
|
||||
return identityKeyStore.getIdentityKeyPair();
|
||||
|
||||
@ -201,6 +201,10 @@ class BufferedSignalServiceAccountDataStore(selfServiceId: ServiceId) : SignalSe
|
||||
error("Should not happen during the intended usage pattern of this class")
|
||||
}
|
||||
|
||||
override fun setMultiDevice(isMultiDevice: Boolean) {
|
||||
error("Should not happen during the intended usage pattern of this class")
|
||||
}
|
||||
|
||||
fun flushToDisk(persistentStore: SignalServiceAccountDataStore) {
|
||||
SignalDatabase.writableDatabase.withinTransaction {
|
||||
identityStore.flushToDisk(persistentStore)
|
||||
|
||||
@ -15,4 +15,9 @@ public interface SignalServiceAccountDataStore extends SignalProtocolStore,
|
||||
* @return True if the user has linked devices, otherwise false.
|
||||
*/
|
||||
boolean isMultiDevice();
|
||||
|
||||
/**
|
||||
* Update whether the user has linked devices.
|
||||
*/
|
||||
void setMultiDevice(boolean isMultiDevice);
|
||||
}
|
||||
|
||||
@ -1969,14 +1969,15 @@ public class SignalServiceMessageSender {
|
||||
return SendMessageResult.canceledFailure(recipient);
|
||||
}
|
||||
|
||||
OutgoingPushMessageList messages = null;
|
||||
try {
|
||||
OutgoingPushMessageList messages = getEncryptedMessages(recipient,
|
||||
sealedSenderAccess,
|
||||
timestamp,
|
||||
content,
|
||||
online,
|
||||
urgent,
|
||||
story);
|
||||
messages = getEncryptedMessages(recipient,
|
||||
sealedSenderAccess,
|
||||
timestamp,
|
||||
content,
|
||||
online,
|
||||
urgent,
|
||||
story);
|
||||
boolean isSentSyncTranscript = content.getContent().isPresent() && content.getContent().get().syncMessage != null && content.getContent().get().syncMessage.sent != null;
|
||||
|
||||
if (i == 0 && sendEvents != null) {
|
||||
@ -2060,8 +2061,18 @@ public class SignalServiceMessageSender {
|
||||
}
|
||||
} catch (MismatchedDevicesException mde) {
|
||||
Log.w(TAG, "[sendMessage][" + timestamp + "] Handling mismatched devices. (" + mde.getMessage() + ")");
|
||||
|
||||
MismatchedDevices mismatchedDevices = mde.getMismatchedDevices();
|
||||
boolean sentOnlyToSelf = recipient.matches(localAddress) && messages != null && messages.getDevices().equals(Collections.singletonList(localDeviceId));
|
||||
if (sentOnlyToSelf && mismatchedDevices.getMissingDevices().isEmpty()) {
|
||||
Log.w(TAG, "[sendMessage][" + timestamp + "] Sent only to our own device and the server reports no other devices. Marking as no longer multi-device and skipping send.");
|
||||
archiveSessions(recipient, mismatchedDevices.getExtraDevices());
|
||||
aciStore.setMultiDevice(false);
|
||||
return SendMessageResult.success(recipient, Collections.emptyList(), false, false, System.currentTimeMillis() - startTime, content.getContent());
|
||||
}
|
||||
|
||||
try {
|
||||
handleMismatchedDevices(recipient, mde.getMismatchedDevices());
|
||||
handleMismatchedDevices(recipient, mismatchedDevices);
|
||||
} catch (InvalidPreKeyException e) {
|
||||
return SendMessageResult.invalidPreKeyFailure(recipient);
|
||||
}
|
||||
|
||||
@ -172,6 +172,11 @@ open class MessageService(
|
||||
raise(SendError.ContentTooLarge(size = contentSize, maxAllowed = maxContentSizeBytes))
|
||||
}
|
||||
|
||||
if (!protocolStore.isMultiDevice) {
|
||||
Log.d(TAG, "We do not have any linked devices. Skipping sync message send.")
|
||||
return@either SendSuccess(envelopeContent = envelopeContent, sentSealedSender = false, devices = emptyList())
|
||||
}
|
||||
|
||||
var encryptedReported = false
|
||||
|
||||
// Certain errors self-resolve by mutating external state, like creating new sessions.
|
||||
@ -211,6 +216,12 @@ open class MessageService(
|
||||
when (val error = result.error) {
|
||||
is MismatchedDeviceException -> {
|
||||
handleMismatched(error, sealedSenderAccess = null)
|
||||
val sentOnlyToSelf = encryptedMessages.map { it.destinationDeviceId } == listOf(localDeviceId)
|
||||
if (sentOnlyToSelf && error.entries.all { it.missingDevices.isEmpty() }) {
|
||||
Log.w(TAG, "Sent only to our own device and the server reports no other devices. Marking as no longer multi-device and skipping send.")
|
||||
protocolStore.setMultiDevice(false)
|
||||
return@either SendSuccess(envelopeContent = envelopeContent, sentSealedSender = false, devices = emptyList())
|
||||
}
|
||||
}
|
||||
is RateLimitChallengeException -> {
|
||||
raise(SendError.ChallengeRequired(error.token, error.options, error.retryLater?.toKotlinDuration()))
|
||||
@ -331,7 +342,7 @@ open class MessageService(
|
||||
}
|
||||
|
||||
suspend fun Raise<SendError>.handleMismatched(error: MismatchedDeviceException, sealedSenderAccess: SealedSenderAccess?) {
|
||||
Log.w(TAG, "Handling mismatched devices: ${error.entries}")
|
||||
Log.w(TAG, "Handling mismatched devices: ${error.entries.contentToString()}")
|
||||
|
||||
for (entry in error.entries) {
|
||||
for (staleDeviceId in entry.staleDevices) {
|
||||
|
||||
@ -298,6 +298,54 @@ class MessageServiceTest {
|
||||
coVerify { keysApi.getPreKey(recipientAci.toString(), 2, null) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `sync send to self with no other devices stops instead of looping`() = runTest {
|
||||
val service = newService()
|
||||
every { protocolStore.isMultiDevice } returns true
|
||||
every { protocolStore.getSubDeviceSessions(localAci.toString()) } returns emptyList()
|
||||
every { cipher.encrypt(any(), any(), any()) } returns OutgoingPushMessage(1, 1, 100, validSerializedSignalMessageBase64())
|
||||
|
||||
coEvery { messageApi.sendSyncMessage(any(), any(), any()) } returns
|
||||
RequestResult.NonSuccess(mismatchedException(extra = intArrayOf(1), account = localAci))
|
||||
|
||||
val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null)
|
||||
|
||||
val success = (result as Either.Right).value
|
||||
assertThat(success.devices).isEqualTo(emptyList<Int>())
|
||||
verify { protocolStore.archiveSession(SignalProtocolAddress(localAci.libSignalServiceId, 1)) }
|
||||
verify(exactly = 1) { protocolStore.setMultiDevice(false) }
|
||||
coVerify(exactly = 1) { messageApi.sendSyncMessage(any(), any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `sync send that reached a real linked device is not treated as no other devices`() = runTest {
|
||||
val service = newService()
|
||||
every { protocolStore.isMultiDevice } returns true
|
||||
every { protocolStore.getSubDeviceSessions(localAci.toString()) } returns listOf(2)
|
||||
every { cipher.encrypt(any(), any(), any()) } returns OutgoingPushMessage(1, 2, 100, validSerializedSignalMessageBase64())
|
||||
|
||||
coEvery { messageApi.sendSyncMessage(any(), any(), any()) } returns
|
||||
RequestResult.NonSuccess(mismatchedException(extra = intArrayOf(2), account = localAci))
|
||||
|
||||
val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null)
|
||||
|
||||
assertThat(result).isInstanceOf(Either.Left::class)
|
||||
verify(exactly = 0) { protocolStore.setMultiDevice(any()) }
|
||||
coVerify(atLeast = 2) { messageApi.sendSyncMessage(any(), any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `sync send is skipped entirely when not multi-device`() = runTest {
|
||||
val service = newService()
|
||||
every { protocolStore.isMultiDevice } returns false
|
||||
|
||||
val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null)
|
||||
|
||||
val success = (result as Either.Right).value
|
||||
assertThat(success.devices).isEqualTo(emptyList<Int>())
|
||||
coVerify(exactly = 0) { messageApi.sendSyncMessage(any(), any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ServiceIdNotFoundException maps to NotRegistered`() = runTest {
|
||||
val service = newService()
|
||||
@ -449,10 +497,11 @@ class MessageServiceTest {
|
||||
private fun mismatchedException(
|
||||
missing: IntArray = intArrayOf(),
|
||||
extra: IntArray = intArrayOf(),
|
||||
stale: IntArray = intArrayOf()
|
||||
stale: IntArray = intArrayOf(),
|
||||
account: ServiceId = recipientAci
|
||||
): MismatchedDeviceException {
|
||||
val entry = MismatchedDeviceException.Entry(
|
||||
account = recipientAci.libSignalServiceId,
|
||||
account = account.libSignalServiceId,
|
||||
missingDevices = missing,
|
||||
extraDevices = extra,
|
||||
staleDevices = stale
|
||||
|
||||
Loading…
Reference in New Issue
Block a user