Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design: Support for Multiple Identities in DFX #883

Closed
hansl opened this issue Jul 28, 2020 · 8 comments · Fixed by #972
Closed

Design: Support for Multiple Identities in DFX #883

hansl opened this issue Jul 28, 2020 · 8 comments · Fixed by #972
Assignees

Comments

@hansl
Copy link
Contributor

hansl commented Jul 28, 2020

Currently, DFX supports only 1 identity, stored at $HOME/.dfinity/identity/creds.pem. This poses multiple problems:

  1. the location is not standard. A configuration file for dfx should probably live in $HOME/.config/dfx/identity.
  2. the file is hardcoded and there's no argument flag to change it. In order to change identity a backup must be made and the file must be deleted.

This proposal does three separate things to fix this;

Technical Design

Anynomous Identity

There were 2 solutions proposed for dealing with anonymous identities;

Anonymous being a reserved identity name

Any strings are valid identities, except anonymous which is reserved identity name. Using anonymous as an identity name would result in using the anonymous principal.

Example usage:

dfx identity new anonymous  # Error out!
dfx identity use anonymous
dfx canister call ...

# This is an actual anonymous call.
dfx --identity=anonymous canister call ...

# Same as:
dfx identity use anonymous
dfx canister call ...

Pros:

  1. Single flag. dfx --identity is easy to understand and will not have any mutually exclusive flag.
  2. Flag can be overwritten by environment variable (e.g. DFX_IDENTITY).

Cons:

  1. More edge cases for us, since there are a few more if .. else ...

Use a separate flag for anonymous calls

Any strings are valid identities. There is an --anonymous flag that is mutually exclusive with the --identity flag

Example usage:

# Create a PEM file with name "anonymous"
dfx identity new anonymous  # @ericswanson: errors out?
dfx identity use anonymous  # fails with identity not found.
# The following 2 calls are different:
dfx --identity=anonymous canister call ...  # fails with identity not found.
dfx --anonymous canister call ...

Pros:

  1. Semantic of dfx identity is straightforward and doesn't have edge cases.
  2. Slightly smaller codebase to maintain.

Cons:

  1. No way to use environment variable or set the current whoami identity to anonymous.
  2. Mutually exclusive flags are hard to keep track.

Because it is easy enough for users to keep in mind a reserved name for the anonymous identity, and easier in general than using mutually exclusive flags, and because it allows us to set a default identity and use environment variables for anonymous identity, the first solution was picked. Having a small increase in edge cases was not considered a significant problem.

Identity Files

Create a file per identity under $HOME/.config/dfx/identity/<name>/identity.pem. This file will contain the private key. For now, no other mechanism than creating a private key will exist, but there could be other files in there for other token types, if and when necessary.

A single file $HOME/.config/dfx/identity.json will have metadata associated with general identities. It will look like the following:

{
  "default": "some_name"
}

All these files could be created under the project's .dfx/ folder, but for now only global identities are supported.

Identity subcommand

Following partially on the design here, an identity subcommand will be added. It will look like this:

dfx identity new <name>  # Creates a new identity. If this identity already exists, return an error.
dfx identity use <name>  # Changes the default identity being used.
dfx identity remove <name>  # Delete any files and folders related to this identity.
dfx identity rename <from> <to>  # Change the name of an identity to a different name.
dfx identity whoami  # Show the name of the current identity.
dfx identity list  # List all available identities.

Default Identity

By default, without a JSON file, the default identity "default" will be used. The first time dfx is used, if there is no identity "default" created, a "default" identity automatically will be created.

For migration purposes, if a file $HOME/.dfinity/identity/creds.pem is found, copy that file to the new identity (instead of generating one randomly).

Log INFO to the terminal that a "default" user was created.

Flag Argument

Adding a --identity argument to pick a name from the list. If the name is not part of the list, dfx should look if it's pointing to a file that contains a valid identity. If the file does not exist, there are two approaches:

  1. Creating a new identity. This approach is the simplest for the user, but might result in some unexpected errors since the principal would be a new one. With a typo, for example.
  2. Error. This makes it clear to the user that the identity does not exist, and we can tell the user what to do (call to action).

The risk to reward of the two options points to the second one being the less risky, while not inconveniencing the user much. In case of typo, the user at least does not have unexpected side effects.

Adding a --anonymous flag that is mutually exclusive with --identity and will use an anonymous principal.

@enzoh
Copy link
Contributor

enzoh commented Jul 28, 2020

I think this makes a lot of sense! My only suggestion would be to keep identities global. Many people, certainly myself included, blow away the .dfx directory all the time. If identities are stored there and they have cycles tied to them, then they could easily get lost.

@enzoh
Copy link
Contributor

enzoh commented Jul 28, 2020

Just to reiterate a point I already made, we would not need any commands to manage identity if we used BIP32. Though that is conditioned upon https://github.com/dfinity-lab/ic-ref/pull/115 due to a lack of available libraries for our desired signature scheme and language.

@enzoh
Copy link
Contributor

enzoh commented Jul 28, 2020

Basically, I would like something as simple as this:

https://github.com/enzoh/cdk/blob/master/go/src/ic/cdk/command/whoami/whoami.go

..but if we can't negotiate for secp256k1, then I support this design.

@hansl
Copy link
Contributor Author

hansl commented Jul 28, 2020 via email

@enzoh
Copy link
Contributor

enzoh commented Jul 28, 2020

No, no command needed; BIP32, batteries included!

@hansl
Copy link
Contributor Author

hansl commented Jul 28, 2020

How do you specify which seed file? How do you build it? I'm against using identities that haven't been "created", for reason mentioned above (e.g. typos). In the future, how do I link multiple keys to the same identity, but also have multiple identities locally? How do I use hardware tokens? Just saying "no command needed" is not a good answer. Can you give me an example of an SDK that doesn't require commands to manage identities?

@hansl
Copy link
Contributor Author

hansl commented Jul 28, 2020

Corrected a few pieces about the proposal after comments. Thanks both of you!

@enzoh
Copy link
Contributor

enzoh commented Jul 28, 2020

To summarize some offline conversations with @hansl. Our aim to support these commands extends beyond referencing multiple private key files. Even if we can do away with multiple private key files, which we can for the reasons I mention above, we want to provide the best possible user experience, and that means (has Hans has defined) restricting identities to only those that were defined by the user. Specifically, if a user creates an identity called qwerty, and then ties to run dfx --identity qwert5 ..., then the user should receive an error message indicating that no such identity exists. We recognize that we are trading correctness for convenience here. Obviously it would be much more convenient for the user to avoid having to define an identity in the first place, but that isn't what we're optimizing for.

ericswanson-dfinity added a commit that referenced this issue Aug 28, 2020
Implements #883

dfx identity new <name>  # Creates a new identity. If this identity already exists, return an error.
dfx identity use <name>  # Changes the default identity being used.
dfx identity remove <name>  # Delete any files and folders related to this identity.
dfx identity rename <from> <to>  # Change the name of an identity to a different name.
dfx identity whoami  # Show the name of the current identity.
dfx identity list  # List all available identities.
ericswanson-dfinity added a commit that referenced this issue Aug 28, 2020
Implements #883

dfx identity new <name>  # Creates a new identity. If this identity already exists, return an error.
dfx identity use <name>  # Changes the default identity being used.
dfx identity remove <name>  # Delete any files and folders related to this identity.
dfx identity rename <from> <to>  # Change the name of an identity to a different name.
dfx identity whoami  # Show the name of the current identity.
dfx identity list  # List all available identities.
@ghost ghost linked a pull request Aug 31, 2020 that will close this issue
@mergify mergify bot closed this as completed in #972 Sep 3, 2020
mergify bot pushed a commit that referenced this issue Sep 3, 2020
Implements #883


Adds new commands:
dfx identity new <name>  # Creates a new identity. If this identity already exists, return an error.
dfx identity use <name>  # Changes the default identity being used.
dfx identity remove <name>  # Delete any files and folders related to this identity.
dfx identity rename <from> <to>  # Change the name of an identity to a different name.
dfx identity whoami  # Show the name of the current identity.
dfx identity list  # List all available identities.

Also adds top-level `--identity <identity>` argument, which uses the specified identity for the current command only.
dfinity-bot added a commit that referenced this issue May 1, 2021
## Changelog for advisory-db:
Branch: master
Commits: [rustsec/advisory-db@db03320a...c8f399d8](rustsec/advisory-db@db03320...c8f399d)

* [`3dcdf93d`](rustsec/advisory-db@3dcdf93) Bump `rustsec-admin` to v0.4.1 ([RustSec/advisory-db⁠#881](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/881))
* [`43778319`](rustsec/advisory-db@4377831) Add CVE-2021-3449 for openssl-src ([RustSec/advisory-db⁠#882](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/882))
* [`e4e343b7`](rustsec/advisory-db@e4e343b) Assigned RUSTSEC-2021-0055 to openssl-src ([RustSec/advisory-db⁠#884](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/884))
* [`ee38ef50`](rustsec/advisory-db@ee38ef5) Add CVE-2021-3450 for openssl-src ([RustSec/advisory-db⁠#883](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/883))
* [`d824e5d5`](rustsec/advisory-db@d824e5d) Assigned RUSTSEC-2021-0056 to openssl-src ([RustSec/advisory-db⁠#886](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/886))
* [`35792564`](rustsec/advisory-db@3579256) Add CVE-2021-23840 for openssl-src ([RustSec/advisory-db⁠#887](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/887))
* [`d2a673c6`](rustsec/advisory-db@d2a673c) Assigned RUSTSEC-2021-0057 to openssl-src ([RustSec/advisory-db⁠#889](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/889))
* [`eed48b9a`](rustsec/advisory-db@eed48b9) Add CVE-2021-23841 for openssl-src ([RustSec/advisory-db⁠#888](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/888))
* [`c8f399d8`](rustsec/advisory-db@c8f399d) Assigned RUSTSEC-2021-0058 to openssl-src ([RustSec/advisory-db⁠#890](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/890))
mergify bot pushed a commit that referenced this issue May 1, 2021
## Changelog for advisory-db:
Branch: master
Commits: [rustsec/advisory-db@db03320a...c8f399d8](rustsec/advisory-db@db03320...c8f399d)

* [`3dcdf93d`](rustsec/advisory-db@3dcdf93) Bump `rustsec-admin` to v0.4.1 ([RustSec/advisory-db⁠#881](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/881))
* [`43778319`](rustsec/advisory-db@4377831) Add CVE-2021-3449 for openssl-src ([RustSec/advisory-db⁠#882](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/882))
* [`e4e343b7`](rustsec/advisory-db@e4e343b) Assigned RUSTSEC-2021-0055 to openssl-src ([RustSec/advisory-db⁠#884](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/884))
* [`ee38ef50`](rustsec/advisory-db@ee38ef5) Add CVE-2021-3450 for openssl-src ([RustSec/advisory-db⁠#883](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/883))
* [`d824e5d5`](rustsec/advisory-db@d824e5d) Assigned RUSTSEC-2021-0056 to openssl-src ([RustSec/advisory-db⁠#886](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/886))
* [`35792564`](rustsec/advisory-db@3579256) Add CVE-2021-23840 for openssl-src ([RustSec/advisory-db⁠#887](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/887))
* [`d2a673c6`](rustsec/advisory-db@d2a673c) Assigned RUSTSEC-2021-0057 to openssl-src ([RustSec/advisory-db⁠#889](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/889))
* [`eed48b9a`](rustsec/advisory-db@eed48b9) Add CVE-2021-23841 for openssl-src ([RustSec/advisory-db⁠#888](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/888))
* [`c8f399d8`](rustsec/advisory-db@c8f399d) Assigned RUSTSEC-2021-0058 to openssl-src ([RustSec/advisory-db⁠#890](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/890))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants