avoid accounts being connected multiple times

the pool should not be asked to connect a specific account
it should only be called to do a full reconfiguration
This commit is contained in:
Daniel Gultsch 2023-02-18 09:55:17 +01:00
parent bca253faa4
commit d6edea8ddf
No known key found for this signature in database
GPG key ID: F43D18AD2A0982C2
6 changed files with 105 additions and 41 deletions

View file

@ -20,11 +20,14 @@ public class Account {
@Override @Override
public String toString() { public String toString() {
return "Account{" + return "Account{"
"id=" + id + + "id="
", address=" + address + + id
", randomSeed=" + Arrays.toString(randomSeed) + + ", address="
'}'; + address
+ ", randomSeed="
+ Arrays.toString(randomSeed)
+ '}';
} }
public Account(final long id, @NonNull final BareJid address, @NonNull byte[] randomSeed) { public Account(final long id, @NonNull final BareJid address, @NonNull byte[] randomSeed) {

View file

@ -1,5 +1,7 @@
package im.conversations.android.database.model; package im.conversations.android.database.model;
import java.util.Objects;
public class Credential { public class Credential {
public final String password; public final String password;
@ -44,6 +46,12 @@ public class Credential {
this.privateKeyAlias = privateKeyAlias; this.privateKeyAlias = privateKeyAlias;
} }
public boolean isEmpty() {
return Objects.isNull(this.password)
&& Objects.isNull(this.fastToken)
&& Objects.isNull(this.privateKeyAlias);
}
public static Credential empty() { public static Credential empty() {
return new Credential(); return new Credential();
} }

View file

@ -39,13 +39,21 @@ public class AccountRepository extends AbstractRepository {
if (password != null) { if (password != null) {
CredentialStore.getInstance(context).setPassword(account, password); CredentialStore.getInstance(context).setPassword(account, password);
} }
ConnectionPool.getInstance(context).reconfigure();
return account; return account;
} }
public ListenableFuture<Account> createAccountAsync( public ListenableFuture<Account> createAccountAsync(
final @NonNull BareJid address, final String password, final boolean loginAndBind) { final @NonNull BareJid address, final String password, final boolean loginAndBind) {
return Futures.submit(() -> createAccount(address, password, loginAndBind), IO_EXECUTOR); final var creationFuture =
Futures.submit(() -> createAccount(address, password, loginAndBind), IO_EXECUTOR);
return Futures.transformAsync(
creationFuture,
account ->
Futures.transform(
ConnectionPool.getInstance(context).reconfigure(),
v -> account,
MoreExecutors.directExecutor()),
MoreExecutors.directExecutor());
} }
public ListenableFuture<Account> createAccountAsync( public ListenableFuture<Account> createAccountAsync(
@ -62,16 +70,25 @@ public class AccountRepository extends AbstractRepository {
MoreExecutors.directExecutor()); MoreExecutors.directExecutor());
} }
public ListenableFuture<Boolean> deleteAccountAsync(@NonNull Account account) { public ListenableFuture<Void> deleteAccountAsync(@NonNull Account account) {
return Futures.submit(() -> deleteAccount(account), IO_EXECUTOR); return Futures.submit(() -> deleteAccount(account), IO_EXECUTOR);
} }
private Boolean deleteAccount(@NonNull Account account) { private Void deleteAccount(@NonNull Account account) {
return database.accountDao().delete(account.id) > 0; database.accountDao().delete(account.id);
ConnectionPool.getInstance(context).reconfigure();
return null;
} }
public ListenableFuture<XmppConnection> getConnectedFuture(@NonNull final Account account) { public ListenableFuture<XmppConnection> getConnectedFuture(@NonNull final Account account) {
return ConnectionPool.getInstance(context).get(account).asConnectedFuture(); final var optional = ConnectionPool.getInstance(context).get(account);
if (optional.isPresent()) {
return optional.get().asConnectedFuture();
} else {
return Futures.immediateFailedFuture(
new IllegalStateException(
String.format("Account %s is not configured", account.address)));
}
} }
public ListenableFuture<Account> setPasswordAsync( public ListenableFuture<Account> setPasswordAsync(
@ -86,6 +103,10 @@ public class AccountRepository extends AbstractRepository {
return account; return account;
} }
public void reconnect(final Account account) {
ConnectionPool.getInstance(context).reconnect(account);
}
public static class AccountAlreadyExistsException extends IllegalStateException { public static class AccountAlreadyExistsException extends IllegalStateException {
public AccountAlreadyExistsException(BareJid address) { public AccountAlreadyExistsException(BareJid address) {
super(String.format("The account %s has already been setup", address)); super(String.format("The account %s has already been setup", address));

View file

@ -74,6 +74,7 @@ public class SetupViewModel extends AndroidViewModel {
} }
public boolean submitXmppAddress() { public boolean submitXmppAddress() {
final var account = this.account;
final var userInput = Strings.nullToEmpty(this.xmppAddress.getValue()).trim(); final var userInput = Strings.nullToEmpty(this.xmppAddress.getValue()).trim();
if (userInput.isEmpty()) { if (userInput.isEmpty()) {
this.xmppAddressError.postValue( this.xmppAddressError.postValue(
@ -88,14 +89,28 @@ public class SetupViewModel extends AndroidViewModel {
return true; return true;
} }
// TODO do we already have an account in this viewModel? is it the same? if so go to that if (account != null) {
// one with the next step if (account.address.equals(address)) {
this.accountRepository.reconnect(account);
decideNextStep(Target.ENTER_ADDRESS, account);
return true;
} else {
this.account = null;
this.accountRepository.deleteAccountAsync(account);
}
}
createAccount(address);
return true;
}
final String password = this.password.getValue(); private void createAccount(final BareJid address) {
// if the user hasn't entered anything we want this to be null so we don't store credentials
final String password = Strings.emptyToNull(this.password.getValue());
// post parsed/normalized jid back into UI // post parsed/normalized jid back into UI
this.xmppAddress.postValue(address.toString()); this.xmppAddress.postValue(address.toString());
this.loading.postValue(true);
final var creationFuture = this.accountRepository.createAccountAsync(address, password); final var creationFuture = this.accountRepository.createAccountAsync(address, password);
this.setCurrentOperation(creationFuture);
Futures.addCallback( Futures.addCallback(
creationFuture, creationFuture,
new FutureCallback<>() { new FutureCallback<>() {
@ -116,7 +131,11 @@ public class SetupViewModel extends AndroidViewModel {
} }
}, },
MoreExecutors.directExecutor()); MoreExecutors.directExecutor());
return true; }
private void setCurrentOperation(ListenableFuture<?> currentOperation) {
this.loading.postValue(true);
this.currentOperation = currentOperation;
} }
private void setAccount(@NonNull final Account account) { private void setAccount(@NonNull final Account account) {
@ -125,9 +144,9 @@ public class SetupViewModel extends AndroidViewModel {
} }
private void decideNextStep(final Target current, @NonNull final Account account) { private void decideNextStep(final Target current, @NonNull final Account account) {
LOGGER.info("Get connected future for {}", account.address);
final ListenableFuture<XmppConnection> connectedFuture = final ListenableFuture<XmppConnection> connectedFuture =
this.accountRepository.getConnectedFuture(account); this.accountRepository.getConnectedFuture(account);
this.setCurrentOperation(connectedFuture);
Futures.addCallback( Futures.addCallback(
connectedFuture, connectedFuture,
new FutureCallback<>() { new FutureCallback<>() {
@ -135,6 +154,8 @@ public class SetupViewModel extends AndroidViewModel {
public void onSuccess(final XmppConnection result) { public void onSuccess(final XmppConnection result) {
// TODO only when configured for loginAndBind // TODO only when configured for loginAndBind
LOGGER.info("Account setup successful"); LOGGER.info("Account setup successful");
SetupViewModel.this.account = null;
redirect(Target.DONE);
} }
@Override @Override
@ -197,19 +218,22 @@ public class SetupViewModel extends AndroidViewModel {
} }
final String password = Strings.nullToEmpty(this.password.getValue()); final String password = Strings.nullToEmpty(this.password.getValue());
final var setPasswordFuture = this.accountRepository.setPasswordAsync(account, password); final var setPasswordFuture = this.accountRepository.setPasswordAsync(account, password);
this.loading.postValue(true); this.setCurrentOperation(setPasswordFuture);
Futures.addCallback(setPasswordFuture, new FutureCallback<>() { Futures.addCallback(
@Override setPasswordFuture,
public void onSuccess(final Account account) { new FutureCallback<>() {
decideNextStep(Target.ENTER_PASSWORD, account); @Override
} public void onSuccess(final Account account) {
decideNextStep(Target.ENTER_PASSWORD, account);
}
@Override @Override
public void onFailure(@NonNull Throwable throwable) { public void onFailure(@NonNull Throwable throwable) {
// TODO show some sort of error message // TODO show some sort of error message
loading.postValue(false); loading.postValue(false);
} }
},MoreExecutors.directExecutor()); },
MoreExecutors.directExecutor());
return true; return true;
} }
@ -224,6 +248,7 @@ public class SetupViewModel extends AndroidViewModel {
public void cancelSetup() { public void cancelSetup() {
final var account = this.account; final var account = this.account;
if (account != null) { if (account != null) {
this.account = null;
this.accountRepository.deleteAccountAsync(account); this.accountRepository.deleteAccountAsync(account);
} }
} }

View file

@ -3,7 +3,6 @@ package im.conversations.android.xmpp;
import android.content.Context; import android.content.Context;
import android.os.SystemClock; import android.os.SystemClock;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
@ -62,7 +61,7 @@ public class ConnectionPool {
reconfigurationExecutor); reconfigurationExecutor);
} }
public synchronized XmppConnection get(final Account account) { private synchronized XmppConnection reconfigure(final Account account) {
final Optional<XmppConnection> xmppConnectionOptional = final Optional<XmppConnection> xmppConnectionOptional =
Iterables.tryFind(this.connections, c -> c.getAccount().equals(account)); Iterables.tryFind(this.connections, c -> c.getAccount().equals(account));
if (xmppConnectionOptional.isPresent()) { if (xmppConnectionOptional.isPresent()) {
@ -71,13 +70,19 @@ public class ConnectionPool {
return setupXmppConnection(context, account); return setupXmppConnection(context, account);
} }
public synchronized Optional<XmppConnection> get(final Account account) {
return Iterables.tryFind(this.connections, c -> c.getAccount().equals(account));
}
public synchronized void reconnect(final Account account) { public synchronized void reconnect(final Account account) {
final Optional<XmppConnection> xmppConnectionOptional = final Optional<XmppConnection> xmppConnectionOptional = get(account);
Iterables.tryFind(this.connections, c -> c.getAccount().equals(account));
if (xmppConnectionOptional.isPresent()) { if (xmppConnectionOptional.isPresent()) {
reconnectAccount(xmppConnectionOptional.get()); reconnectAccount(xmppConnectionOptional.get());
} else { } else {
setupXmppConnection(context, account); throw new IllegalStateException(
String.format(
"Attempted to reconnect %s but the account was not configured",
account.address));
} }
} }
@ -95,7 +100,7 @@ public class ConnectionPool {
String.format( String.format(
"No enabled account with address %s", address.toString())); "No enabled account with address %s", address.toString()));
} }
return get(account); return reconfigure(account);
}, },
reconfigurationExecutor); reconfigurationExecutor);
} }
@ -112,7 +117,7 @@ public class ConnectionPool {
throw new IllegalStateException( throw new IllegalStateException(
String.format("No enabled account with id %d", id)); String.format("No enabled account with id %d", id));
} }
return get(account); return reconfigure(account);
}, },
reconfigurationExecutor); reconfigurationExecutor);
} }
@ -121,7 +126,6 @@ public class ConnectionPool {
return Iterables.any(this.connections, c -> id == c.getAccount().id); return Iterables.any(this.connections, c -> id == c.getAccount().id);
} }
private synchronized Void reconfigure(final Set<Account> accounts) { private synchronized Void reconfigure(final Set<Account> accounts) {
final Set<Account> current = getAccounts(); final Set<Account> current = getAccounts();
final Set<Account> removed = Sets.difference(current, accounts); final Set<Account> removed = Sets.difference(current, accounts);
@ -356,7 +360,7 @@ public class ConnectionPool {
} }
private XmppConnection setupXmppConnection(final Context context, final Account account) { private XmppConnection setupXmppConnection(final Context context, final Account account) {
LOGGER.info("Setting up XMPP connection for {}",account.address); LOGGER.info("Setting up XMPP connection for {}", account.address);
final XmppConnection xmppConnection = new XmppConnection(context, account); final XmppConnection xmppConnection = new XmppConnection(context, account);
this.connections.add(xmppConnection); this.connections.add(xmppConnection);
xmppConnection.setOnStatusChangedListener(this::onStatusChanged); xmppConnection.setOnStatusChangedListener(this::onStatusChanged);

View file

@ -1238,9 +1238,12 @@ public class XmppConnection implements Runnable {
final Element cbElement = final Element cbElement =
this.streamFeatures.findChild("sasl-channel-binding", Namespace.CHANNEL_BINDING); this.streamFeatures.findChild("sasl-channel-binding", Namespace.CHANNEL_BINDING);
final Collection<ChannelBinding> channelBindings = ChannelBinding.of(cbElement); final Collection<ChannelBinding> channelBindings = ChannelBinding.of(cbElement);
final SaslMechanism.Factory saslFactory = final var credential = CredentialStore.getInstance(context).get(account);
new SaslMechanism.Factory( if (credential.isEmpty()) {
account, CredentialStore.getInstance(context).get(account)); LOGGER.warn("No credentials configured. Going to bail out before actual attempt.");
throw new StateChangingException(ConnectionState.UNAUTHORIZED);
}
final SaslMechanism.Factory saslFactory = new SaslMechanism.Factory(account, credential);
final SaslMechanism saslMechanism = final SaslMechanism saslMechanism =
saslFactory.of( saslFactory.of(
mechanisms, channelBindings, version, SSLSockets.version(this.socket)); mechanisms, channelBindings, version, SSLSockets.version(this.socket));