Batteries-included Account / Connection management

[TobiasFella]

Problem

Currently, handling connections in libquotient-based clients is complicated.

To log in a client, the flow is roughly:

• Create a Connection object
• run Connection::resolveServer(mxId) to get the server's url and query the login flows
• wait until the login flows in the connection change
• call Connection::loginWithPassword(mxId, password, ...)
• wait until Connection::connected
• Save the account to local state config (AccountSettings)
• Save the access token to the keychain
• Add the connection to the client's list of connections (e.g., Quotient::AccountRegistry)
• Start syncing
• Save the state cache after each sync iteration

To load an existing connection:

• Load list of connections from state config; for each one do:
• Load settings through AccountSettings
• Load access token from keychain
• Create a Connection object
• Run Connection::assumeIdentity(mxId, accessToken)
• Wait until Connection::connected
• Run Connection::loadState
• Add the connection to the client's list of connections (e.g., Quotient::AccountRegistry)
• start syncing
• Save state cache after each sync

(I'm skipping over SSO login and account registration here. From libQuotient's perspective, account registration doesn't really exist; the relevant API here is the same as for login. For SSO login, SsoSession exists)

There are some further complications here, e.g., that a client might want to wait until after the initial sync / load from cache before showing the connection (or switching away from a loading screen) to make the UX nicer.

With sliding sync, OIDC login, and my work on making libquotient work offline (#777), this would not get simpler.

Overall, there are too many things that are hard to understand, making the developer expierence not amazing.

There's also the - somewhat failed - attempt to improve this by providing the account loading functionality in Quotient::AccountRegistry.

Goals

• There's easy to use API for logging in accounts through password, traditional sso, and oidc, registering accounts, and loading existing accounts
• The Connection type is always ready to be used by a client, in the sense that it has the necessary userid, deviceid, access-/refreshtoken, crypto state, etc. that is required for operation. Notably, this does not mean that the homeserver must be reachable / was already reached (see Prepare libQuotient for operating without being able to reach the server #777)
• Connections are loaded in a "batteries-included" way, e.g., the automatically load/save the state cache, run a sync loop, are saved to state config to be restored when the client starts next time, have their access token saved to keychain, are added to an accountregistry, etc. These behaviors can be overwritten as needed.

Proposal

• Connection uses its public Constructors

• The following functions are removed (at least from public API, might be kept as internal API as needed. we should look into removing functions that clients don't need from public API, but that's a different topic):

  • Connection::isLoggedIn (needs some further thought. should connections be automatically be destroyed when logging out? At least, it should not be relevant for checking whether a connection is "loaded" yet)
  • Connection::loadState, Connection::saveState: Handled automatically, depending on configuration
  • Connection::prepareForSso, Connection::resolveServer, Connection::setHomeserver, Connection::assumeIdentity: Implementation details that the client shouldn't need to care about
  • Connection::sync: maybe, might be useful for situations where we want to selectively sync,
  • Connection::syncLoop, Connection::stopSync: Handled automatically depending on configuration

• Connection signals are removed / changed as needed:

  • Connection::resolveError needs is moved to PendingConnection
  • Connection::homeserverChanged is moved (or removed entirely, as needed)
  • Connection::loginFlowsChanged is removed
  • Connection::connected is removed (probably. See Prepare libQuotient for operating without being able to reach the server #777)
  • Connection::loggedOut is removed if we end up deleting connections automatically on logout
  • Connection::stateChanged is removed
  • Connection::loginError is moved to PendingConnection

• A new Homeserver class is added (name up for debate). It's intended usage is:

  • Resolving the server's url from an mxid or url
  • Querying the server's login flows
  • Exposing well-known info to client
  • Exposing password, sso, oidc login capability to client

• AccountRegistry is kept roughly as-is:

• AccountRegistry::add and AccountRegistry::drop are dropped from public API (maybe?)

• new functions

  • PendingConnection AccountRegistry::loginWithPassword(mxId, password)
  • PendingConnection AccountRegistry::restoreConnection(mxId)
  • PendingConnection AccountRegistry::loginWithSso(homeserverUrl)
  • PendingConnection AccountRegistry::loginWithOidc(url)
  • PendingConnection AccountRegistry::registerAccount() (maybe we need a different name for AccountRegistry to make it clear that this is not about adding a name to the registry)

• A function that lists the mxIDs of all connections that the are stored in state cache, i.e., that can be loaded

• Maybe a convenience function to automatically load all connections

• PendingConnection

  • Future/Promise to get a Connection
  • Subclasses for the situations that need specific user interaction to get a connection (SSO login, OIDC login, registration)
  • PendingSsoConnection replaces SsoSession

• The functions for getting connections (AccountRegistry::loginWithPassword, etc.) take an optional ConnectionSettings parameter, which configures

  • Whether a sync loop is started automatically (default: yes)
  • Whether cache is saved / restored (default: yes)
  • Whether the connection is added to the account registry (default: yes)
  • Whether the connection is remembered (i.e., stored to state config) (default: yes)

• Semi-related:

  • There's some kind of mechanism by which the client can provide the backend for the state config used to store accounts. This would clean reduce the number of individual config files that e.g. NeoChat produces

[vkrause]

As a user of libquotient in a an app that isn't primarily a Matrix (chat) client, having the library handle more of this out of the box would indeed be much appreciated :)

[KitsuneRal]

Wow, this is comprehensive :) I wasn't particularly happy with this part of the library for a long time, and my ideas overlap with a lot of this. A few considerations below:

• I have long wanted to apply RAII rules to the Connection lifecycle - specifically, to produce it fully set up at the moment of a successful login (around the moment currently in Connection::Private::completeSetup() and destroy it at the moment of a (hard) logout completion. This answers your question with regards to things like isLoggedIn() and loggedOut() - I'd like to get rid of them.
• Big yes for PendingConnection (or should we call it LoginFlow?); we'll probably need an abstract base class for it and implement specific flows in derived classes
• I like the ConnectionSettings idea, looks like a good way to decouple the settings from actual connection objects. Not sure if we should allow skipping the account registry though.
• Aye for AccountRegistry changes.

I'm not sure if Connection should still be a base class and provide a protected interface; I see NeoChat using it quite successfully, but we should probably give a bit more thought to what (else) Connection should provide for derived classes.

[KitsuneRal]

Looking once again at it, I wonder if it would make more sense to have login functions in Homeserver, rather than AccountRegistry. We could even make Homeserver a base class so that client authors could add their own fancy login methods.

[TobiasFella]

Not sure if we should allow skipping the account registry though.

Sure, that's not something I want to push for anyway

I wonder if it would make more sense to have login functions in Homeserver, rather than AccountRegistry

That would also make sense from an API perspective. I've put it in AccountRegistry for my proposal since then we automatically have the accountregistry object that is needed during creation of the connection