feat: store account authentication information separate from client information

[tomyrd]

Currently all the authentication information is stored in the client's Store and accessed via get_account_auth_by_pub_key.

Stemming from #631 (comment) we should try to remove the responsibility of tracking authentication information from the Store and move it directly to the authenticator. Auth information may be stored in a separate database or file.

[bobbinth]

cc @dagarcia7 - this is something we chatted briefly about today.

[tomyrd]

Regarding this issue, I think the most straightforward solution would be to decouple the authentication data from the Store, meaning that the StoreAuthenticator would be removed and a new ClientAuthenticator can be added that handles the authentication itself. In this case, should this new authenticator use a new/separate sqlite database? Maybe we want this information to be encrypted in some way before being persisted.

Additionally, I would remove the secret key requirement from add_accont and add a new authenticate_account that lets users add authentication only for the accounts that need it.

[bobbinth]

Largely agree. I think I would start with removing authentication related methods from the Store trait. I believe these are get_account_auth_by_pub_key() and get_account_auth(). This will imply that something like StoreAuthenticator will no longer be possible and that we'd need to remove auth_info from Store::insert_account() (which would in turn imply that we'd need to remove secret key parameter from Client::add_account().

a new ClientAuthenticator can be added that handles the authentication itself. In this case, should this new authenticator use a new/separate sqlite database? Maybe we want this information to be encrypted in some way before being persisted.

I would probably start with something very simple - more like a PoC rather than a robust authenticator (e.g., I wouldn't worry about encrypting things just yet). I would think about this more like an authenticator that could be used by the CLI rather than something more general. It could even be file system-based. For example, we could store public-private key pairs in some directory and the authenticator would just read them from there.

I would remove the secret key requirement from add_accont and add a new authenticate_account that lets users add authentication only for the accounts that need it.

As mentioned above, I agree with removing secrete key parameter from add_account() - but I'm not sure we need to add authenticate_account(). Basically, all authentication-related tasks would be handled by structs implementing TransactionAuthenticator - and so, the client shouldn't need to have any other authentication-specific functionality (by the client here i mean the Rust client, the CLI would have to instantiate a specific authenticator).

[tomyrd]

Yes, I agree that it should be as simple as possible at first.

I would think about this more like an authenticator that could be used by the CLI rather than something more general. It could even be file system-based.

I started working on this and realized that this would only work for the CLI and other std uses of the client but not for the web client. The web client will probably need a separate implementation that's compatible with wasm.

the client shouldn't need to have any other authentication-specific functionality

Oh ok, I think I understand. So the client should only use the authenticator for signatures. The responsibility of storing the private keys for every new account will fall on the user of the client (for example, the CLI binary).

[bobbinth]

Oh ok, I think I understand. So the client should only use the authenticator for signatures. The responsibility of storing the private keys for every new account will fall on the user of the client (for example, the CLI binary).

Yes, exactly. We can even provide some default implementations that work in std and wasm contexts (kind of like with do with different Store implementations).

I started working on this and realized that this would only work for the CLI and other std uses of the client but not for the web client. The web client will probably need a separate implementation that's compatible with wasm.

Correct, but I think WebClient may be using some custom implementation of the authenticator already (which uses the store as initial key repository).

[tomyrd]

Apart from using it to sign transactions, some of the other places where we currently use the AuthSecretKey inside the Client are:

• When exporting an account: The .mac files are serialized AccountData objects (now renamed to AccountFile). This object includes the AuthSecretKey for the account. Two possible alternatives for this are:
  • Add it as a parameter for Client::export_account, so the caller is responsible for providing the secret key.
  • Add a get_secret_key to TransactionAuthenticator (or to a separate wrapper trait we can define in miden-client). So the client itself can ask the authenticator for the secret key.
• When defining an account's capability (i.e. what are its components). When building scripts for transactions we want to know what are the components of the account so we know how to build the transaction. Specifically for this issue, we want to know what type of AuthSecretKey the account uses so we know what function to call for authentication (right now there's only one but that may change in the future). We don't need to know what the secret key is, just what type.
  • Would it be reasonable to check the type by iterating over the account's procedure and checking if one of the roots matches with the known root for the auth_tx_rpo_falcon512 procedure?
  • In a separate note, with the changes in account components, this AccountCapabilities part of the client may be reworked a little bit in the future. It would be useful if we could somehow check what components were used to build an Account, I'm not sure if this would be too much added metadata. If we don't want to change it in the base object, maybe we can add it to AccountRecord and add it to the client's store so it tracks this information.

[bobbinth]

When exporting an account: The .mac files are serialized AccountData objects (now renamed to AccountFile). This object includes the AuthSecretKey for the account. Two possible alternatives for this are:

• Add it as a parameter for Client::export_account, so the caller is responsible for providing the secret key.
• Add a get_secret_key to TransactionAuthenticator (or to a separate wrapper trait we can define in miden-client). So the client itself can ask the authenticator for the secret key.

Another option could be to move the responsibility of building the AccountFile to the CLI/WebClient. This way, the Client does not need to be exposed to the authentication-related structs at all.

Would it be reasonable to check the type by iterating over the account's procedure and checking if one of the roots matches with the known root for the auth_tx_rpo_falcon512 procedure?

Yes, I think this should be fine.

In a separate note, with the changes in account components, this AccountCapabilities part of the client may be reworked a little bit in the future. It would be useful if we could somehow check what components were used to build an Account, I'm not sure if this would be too much added metadata. If we don't want to change it in the base object, maybe we can add it to AccountRecord and add it to the client's store so it tracks this information.

Agreed - that's how I think it should work long-term (or maybe even relatively soon). I actually think it would be great if we could move most (all?) of this logic to miden-base so that the code for determining account capabilities is encapsulated there. There is already a kind of related issue to this: 0xPolygonMiden/miden-base#831.