mirror of
https://github.com/AsamK/signal-cli.git
synced 2026-09-22 06:29:04 +00:00
Remove numberless flag in accounts file
This is an edge case and keeping the ACI only account discoverable is actually the better way to handle it.
This commit is contained in:
parent
d0724fb5b5
commit
3d7109eb01
@ -4,6 +4,5 @@ import java.util.List;
|
||||
|
||||
public record AccountsStorage(List<Account> accounts, Integer version) {
|
||||
|
||||
// Older indexes omit this flag; entries with only a UUID must then be checked against their account state.
|
||||
public record Account(String path, String environment, String number, String uuid, Boolean numberless) {}
|
||||
public record Account(String path, String environment, String number, String uuid) {}
|
||||
}
|
||||
|
||||
@ -63,7 +63,7 @@ public class AccountsStore {
|
||||
public synchronized Set<AccountsStorage.Account> getAllAccounts() throws IOException {
|
||||
return readAccounts().stream()
|
||||
.filter(a -> a.environment() == null || serviceEnvironment.equals(a.environment()))
|
||||
.filter(a -> a.number() != null || (a.uuid() != null && Boolean.TRUE.equals(a.numberless())))
|
||||
.filter(a -> a.number() != null || a.uuid() != null)
|
||||
.collect(Collectors.toSet());
|
||||
}
|
||||
|
||||
@ -98,15 +98,14 @@ public class AccountsStore {
|
||||
return new AccountsStorage.Account(a.path(),
|
||||
serviceEnvironment,
|
||||
number,
|
||||
aci == null ? null : aci.toString(),
|
||||
number == null && aci != null);
|
||||
aci == null ? null : aci.toString());
|
||||
}
|
||||
|
||||
if (number != null && number.equals(a.number())) {
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false);
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid());
|
||||
}
|
||||
if (aci != null && aci.toString().equals(a.uuid())) {
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false);
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null);
|
||||
}
|
||||
|
||||
return a;
|
||||
@ -118,8 +117,7 @@ public class AccountsStore {
|
||||
final var account = new AccountsStorage.Account(accountPath,
|
||||
serviceEnvironment,
|
||||
number,
|
||||
aci == null ? null : aci.toString(),
|
||||
number == null && aci != null);
|
||||
aci == null ? null : aci.toString());
|
||||
updateAccounts(accounts -> {
|
||||
final var existingAccounts = accounts.stream().map(a -> {
|
||||
if (a.environment() != null && !serviceEnvironment.equals(a.environment())) {
|
||||
@ -127,10 +125,10 @@ public class AccountsStore {
|
||||
}
|
||||
|
||||
if (number != null && number.equals(a.number())) {
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid(), false);
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), null, a.uuid());
|
||||
}
|
||||
if (aci != null && aci.toString().equals(a.uuid())) {
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null, false);
|
||||
return new AccountsStorage.Account(a.path(), a.environment(), a.number(), null);
|
||||
}
|
||||
|
||||
return a;
|
||||
@ -161,7 +159,7 @@ public class AccountsStore {
|
||||
private void createInitialAccounts() throws IOException {
|
||||
final var legacyAccountPaths = getLegacyAccountPaths();
|
||||
final var accountsStorage = new AccountsStorage(legacyAccountPaths.stream()
|
||||
.map(number -> new AccountsStorage.Account(number, null, number, null, false))
|
||||
.map(number -> new AccountsStorage.Account(number, null, number, null))
|
||||
.toList(), CURRENT_STORAGE_VERSION);
|
||||
|
||||
IOUtils.createPrivateDirectories(dataPath);
|
||||
@ -204,34 +202,10 @@ public class AccountsStore {
|
||||
} else if (accountsVersion < CURRENT_STORAGE_VERSION) {
|
||||
storage = upgradeAccountsFile(fileChannel, storage, accountsVersion);
|
||||
}
|
||||
final var accounts = storage.accounts().stream().map(this::resolveNumberlessAccount).toList();
|
||||
if (!accounts.equals(storage.accounts())) {
|
||||
saveAccountsLocked(fileChannel, new AccountsStorage(accounts, storage.version()));
|
||||
}
|
||||
return accounts;
|
||||
return storage.accounts();
|
||||
}
|
||||
}
|
||||
|
||||
private AccountsStorage.Account resolveNumberlessAccount(final AccountsStorage.Account entry) {
|
||||
if (entry.numberless() != null) {
|
||||
return entry;
|
||||
}
|
||||
|
||||
var numberless = false;
|
||||
if (entry.number() == null && entry.uuid() != null) {
|
||||
// Older entries also lose their number when another account takes it over.
|
||||
try (final var account = accountLoader.loadAccountOrNull(entry.path())) {
|
||||
if (account == null) {
|
||||
// Keep the entry unresolved so a locked or unavailable account can be checked later.
|
||||
return entry;
|
||||
}
|
||||
numberless = account.getNumber() == null && account.getAci() != null && entry.uuid()
|
||||
.equals(account.getAci().toString());
|
||||
}
|
||||
}
|
||||
return new AccountsStorage.Account(entry.path(), entry.environment(), entry.number(), entry.uuid(), numberless);
|
||||
}
|
||||
|
||||
private AccountsStorage upgradeAccountsFile(
|
||||
final FileChannel fileChannel,
|
||||
final AccountsStorage storage,
|
||||
@ -252,8 +226,7 @@ public class AccountsStore {
|
||||
return new AccountsStorage.Account(a.path(),
|
||||
getServiceEnvironmentString(account.getServiceEnvironment()),
|
||||
a.number(),
|
||||
a.uuid(),
|
||||
a.numberless());
|
||||
a.uuid());
|
||||
}
|
||||
}).toList();
|
||||
}
|
||||
|
||||
@ -1,10 +1,6 @@
|
||||
package org.asamk.signal.manager.storage.accounts;
|
||||
|
||||
import org.asamk.signal.manager.Settings;
|
||||
import org.asamk.signal.manager.api.ServiceEnvironment;
|
||||
import org.asamk.signal.manager.storage.SignalAccount;
|
||||
import org.asamk.signal.manager.storage.Utils;
|
||||
import org.asamk.signal.manager.util.KeyUtils;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
@ -12,7 +8,6 @@ import org.junit.jupiter.params.provider.ValueSource;
|
||||
import org.signal.core.models.ServiceId.ACI;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.channels.OverlappingFileLockException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Set;
|
||||
@ -61,7 +56,7 @@ class AccountsStoreTest {
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(booleans = {false, true})
|
||||
void replacingANumberDoesNotRediscoverTheOldAccountAsNumberless(final boolean update) throws Exception {
|
||||
void replacingANumberKeepsTheOldAccountAvailableByAci(final boolean update) throws Exception {
|
||||
final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> null);
|
||||
final var oldPath = store.addAccount("+12025550123", OLD_ACI);
|
||||
Files.createFile(directory.resolve(oldPath));
|
||||
@ -73,136 +68,14 @@ class AccountsStoreTest {
|
||||
newPath = store.addAccount("+12025550123", NEW_ACI);
|
||||
}
|
||||
|
||||
final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
|
||||
throw new AssertionError("New entries should not require loading account state for discovery");
|
||||
});
|
||||
assertEquals(Set.of(newPath), getAccountPaths(reopened));
|
||||
final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> null);
|
||||
assertEquals(Set.of(oldPath, newPath), getAccountPaths(reopened));
|
||||
assertEquals(newPath, reopened.getPathByNumber("+12025550123"));
|
||||
assertEquals(oldPath, reopened.getPathByAci(OLD_ACI));
|
||||
|
||||
// Only an explicit account update should turn the old entry into a numberless account.
|
||||
reopened.updateAccount(oldPath, null, OLD_ACI);
|
||||
assertEquals(Set.of(oldPath, newPath), getAccountPaths(reopened));
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(ints = {1, 2})
|
||||
void legacyIndexesDistinguishNumberlessAccountsFromSupersededNumberedAccounts(final int version) throws Exception {
|
||||
createSavedAccount("old", "+12025550123", OLD_ACI);
|
||||
createSavedAccount("numberless", null, NEW_ACI);
|
||||
final var legacyIndex = """
|
||||
{
|
||||
"version": %d,
|
||||
"accounts": [
|
||||
{"path": "old", "environment": %s, "number": null, "uuid": "%s"},
|
||||
{"path": "numberless", "environment": %s, "number": null, "uuid": "%s"},
|
||||
{"path": "current", "environment": "STAGING", "number": "+12025550123", "uuid": null}
|
||||
]
|
||||
}
|
||||
""".formatted(version,
|
||||
version == 1 ? "null" : "\"STAGING\"",
|
||||
OLD_ACI,
|
||||
version == 1 ? "null" : "\"STAGING\"",
|
||||
NEW_ACI);
|
||||
Files.writeString(directory.resolve("accounts.json"), legacyIndex);
|
||||
|
||||
final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
|
||||
assertEquals(Set.of("numberless", "current"), getAccountPaths(store));
|
||||
assertEquals("old", store.getPathByAci(OLD_ACI));
|
||||
assertEquals("numberless", store.getPathByAci(NEW_ACI));
|
||||
assertEquals(Set.of("+12025550123"), store.getAllNumbers());
|
||||
|
||||
final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
|
||||
throw new AssertionError("Resolved entries should not require loading account state again");
|
||||
});
|
||||
assertEquals(Set.of("numberless", "current"), getAccountPaths(reopened));
|
||||
assertTrue(new AccountsStore(directory.toFile(),
|
||||
ServiceEnvironment.LIVE,
|
||||
this::loadAccountOrNull).getAllAccounts().isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
void retriesLockedLegacyAccountsAndPersistsSuccessfulClassification() throws Exception {
|
||||
createSavedAccount("numberless", null, NEW_ACI);
|
||||
writeLegacyNumberlessEntry(NEW_ACI);
|
||||
final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
|
||||
|
||||
try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) {
|
||||
assertTrue(store.getAllAccounts().isEmpty());
|
||||
assertEquals("numberless", store.getPathByAci(NEW_ACI));
|
||||
final var storage = Utils.createStorageObjectMapper()
|
||||
.readValue(directory.resolve("accounts.json").toFile(), AccountsStorage.class);
|
||||
assertNull(storage.accounts().getFirst().numberless());
|
||||
}
|
||||
|
||||
assertEquals(Set.of("numberless"), getAccountPaths(store));
|
||||
try (final var account = SignalAccount.load(directory.toFile(), "numberless", false, Settings.DEFAULT)) {
|
||||
final var reopened = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, path -> {
|
||||
throw new AssertionError("A classified account should remain discoverable while locked");
|
||||
});
|
||||
assertEquals(Set.of("numberless"), getAccountPaths(reopened));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void retriesLegacyAccountsAfterMissingStateIsRestored() throws Exception {
|
||||
writeLegacyNumberlessEntry(NEW_ACI);
|
||||
final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
|
||||
assertTrue(store.getAllAccounts().isEmpty());
|
||||
assertEquals("numberless", store.getPathByAci(NEW_ACI));
|
||||
|
||||
createSavedAccount("numberless", null, NEW_ACI);
|
||||
assertEquals(Set.of("numberless"), getAccountPaths(store));
|
||||
}
|
||||
|
||||
@Test
|
||||
void doesNotDiscoverLegacyEntriesWithAMismatchedAci() throws Exception {
|
||||
createSavedAccount("numberless", null, NEW_ACI);
|
||||
writeLegacyNumberlessEntry(OLD_ACI);
|
||||
final var store = new AccountsStore(directory.toFile(), ServiceEnvironment.STAGING, this::loadAccountOrNull);
|
||||
assertTrue(store.getAllAccounts().isEmpty());
|
||||
assertEquals("numberless", store.getPathByAci(OLD_ACI));
|
||||
}
|
||||
|
||||
private Set<String> getAccountPaths(final AccountsStore store) throws IOException {
|
||||
return store.getAllAccounts().stream().map(AccountsStorage.Account::path).collect(Collectors.toSet());
|
||||
}
|
||||
|
||||
private void writeLegacyNumberlessEntry(final ACI aci) throws IOException {
|
||||
final var legacyIndex = """
|
||||
{
|
||||
"version": 2,
|
||||
"accounts": [
|
||||
{"path": "numberless", "environment": "STAGING", "number": null, "uuid": "%s"}
|
||||
]
|
||||
}
|
||||
""".formatted(aci);
|
||||
Files.writeString(directory.resolve("accounts.json"), legacyIndex);
|
||||
}
|
||||
|
||||
private void createSavedAccount(final String path, final String number, final ACI aci) throws IOException {
|
||||
// Account creation needs a number until the ACI has been assigned.
|
||||
try (final var account = SignalAccount.create(directory.toFile(),
|
||||
path,
|
||||
"+12025550123",
|
||||
ServiceEnvironment.STAGING,
|
||||
KeyUtils.generateIdentityKeyPair(),
|
||||
KeyUtils.generateIdentityKeyPair(),
|
||||
KeyUtils.createProfileKey(),
|
||||
Settings.DEFAULT)) {
|
||||
account.setAci(aci);
|
||||
account.setNumber(number);
|
||||
}
|
||||
}
|
||||
|
||||
private SignalAccount loadAccountOrNull(final String path) {
|
||||
if (!SignalAccount.accountFileExists(directory.toFile(), path)) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return SignalAccount.load(directory.toFile(), path, false, Settings.DEFAULT);
|
||||
} catch (IOException | OverlappingFileLockException e) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -4012,8 +4012,7 @@
|
||||
"java.lang.String",
|
||||
"java.lang.String",
|
||||
"java.lang.String",
|
||||
"java.lang.String",
|
||||
"java.lang.Boolean"
|
||||
"java.lang.String"
|
||||
]
|
||||
},
|
||||
{
|
||||
@ -4024,10 +4023,6 @@
|
||||
"name": "number",
|
||||
"parameterTypes": []
|
||||
},
|
||||
{
|
||||
"name": "numberless",
|
||||
"parameterTypes": []
|
||||
},
|
||||
{
|
||||
"name": "path",
|
||||
"parameterTypes": []
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user