ockam icon indicating copy to clipboard operation
ockam copied to clipboard

`ockam enroll` displays accurate output depending on which identity codepath is taken

Open nazmulidris opened this issue 1 year ago • 4 comments

Current behavior

Currently running ockam enroll handles identity re-use and creation differently depending on what the local state is. Here are the 3 states that are possible.

  1. --identity option not used. And no prior enrollment, ~/.ockam is empty. Running ockam enroll creates a new identity, marks it as default, and saves it to a vault, and then uses that w/ Orchestrator.

  2. --identity option not used. Default identity already exists in ~/.ockam and is used. A new identity is not created. This is the result of a previous successful enrollment, or, someone creating a new identity and marking it as the default using ockam identity create, etc.

  3. —-identity my_id option is used. Where my_id has already been created and is on disk. Eg using ockam identity create my_id.

The UI should provide information about which one of these 3 paths is taken when ockam enroll or ockam enroll --identity ... is run.

When you run ockam enroll (no --identity option)

Currently: Image

Desired:

  • It should produce output to inform the user that a new identity has been created or a pre-existing default has been used.
  • There are currently some sections for device activation (one-time code), space provisioning, and project provisioning.
  • Add a section on creation of identity (first time enroll or subsequent enroll).

When you run ockam enroll --identity <ID>

Happy path

Currently: Image

Desired:

  • After running ockam identity create my_id && ockam enroll —-identity my_id, the output should show that the passed in identity is being used (rather than a new one being created).
  • Add text like this to the output to inform the user what is going on in the stdout display output: "We create a default identity and vault for you. This is what is used if you don’t pass in —-identity".

Error path - identity can't be found

Currently: Image The above is an example of the output if the passed in identity can't be found, eg: if you run ockam enroll --identity default you might get the error message above.

Desired:

  • Instead, it should display the following error message: "You typed in ockam enroll —-identity default but it does not exist; instead try ockam identity list to see what identities already exist. If you don’t specify an identity, we will create a new one for you."

References and code

Here are links to some related files in the source code related to "identity":

  • https://github.com/build-trust/ockam/blob/develop/implementations/rust/ockam/ockam_api/src/cli_state/identities.rs#L126
  • https://github.com/build-trust/ockam/blob/develop/implementations/rust/ockam/ockam_command/src/enroll/command.rs#L120
  • https://github.com/build-trust/ockam/blob/develop/implementations/rust/ockam/ockam_api/src/cli_state/identities.rs#L217
  • https://github.com/build-trust/ockam/blob/0f827e7d207c01f6015880a51982d0ea4d1021d6/implementations/rust/ockam/ockam_api/src/cli_state/identities.rs#L113 ( get_named_identity_by_identifier(), get_named_identity() )

nazmulidris avatar Jan 29 '24 03:01 nazmulidris

Hi @nazmulidris

I can take a look at this one later today. Just to understand, I created a sample format of when a default identity doesn't or does exist. Please let me know if that is what you had in mind, such that i know what to work on.

image

YorickdeJong avatar Jan 30 '24 11:01 YorickdeJong

@YorickdeJong Thank you for doing the mocks. I really appreciate it. This is a great direction to head in 👏🏽 .

I think your mocks are a great starting point and we might try to create a PR that we can collaborate on as you get further in it. I can help provide feedback on the UX strings and the organization as you go. This way you won't have to iterate very far before you get feedback and I can engage with you along the way.

The identity section could reflect the following states:

  1. No identity exists on the machine. A new one is created for the user. Saved to the vault locally. Marked as default. The user can be apprised of this information.

  2. Default identity was found (eg in ~/.ockam folder or whatever $OCKAM_HOME is set to). The user can be apprised of this information - that a pre-existing default identity on the local machine is used.

  3. A given identity is used that is passed in via the --identity flag. The user can be apprised of this information that the given identity is used.

nazmulidris avatar Jan 31 '24 03:01 nazmulidris

Hi @nazmulidris,

I'm glad the direction we're heading in is to your liking. I'll start working on the PR based on our discussion and the initial mocks I've created. I'll set up the PR (#7497), such that you can see its progression, thanks!

YorickdeJong avatar Jan 31 '24 08:01 YorickdeJong

Hi @nazmulidris,

I've made progress on the issue. I found that name_identity includes name, vault_name, and is_default fields. Based on this, I've updated the terminal messages to reflect whether a default identity exists and if a specific identity is being used.

I've also added a function check_default_named_identity in the CliState struct to verify the existence of a default identity. This function returns a boolean indicating the presence of a default identity.

Here are the key changes in the code:

        // Print the identity name if it exists.
        if let Ok(named_identity) = opts
            .state
            .get_named_identity_by_identifier(&identifier)
            .await
        {

            // Notify the user that a new identity was created for them.
            if !default_identity_exist {
                opts.terminal.write_line(&fmt_log!(
                    "\nNo Identity name exists. A new identity was created for you: '{}'.", 
                    color_primary(named_identity.name())
                ))?;
            }
            // Notify the user that the default identity will be used.
            else if named_identity.is_default() && self.identity.is_none() {
                opts.terminal
                    .write_line(&fmt_log!("\nExisting default identity: '{}' will be used for enrollment. \
                    To use a different identity, run `ockam enroll --identity <IDENTITY_NAME>`.", 
                        color_primary(named_identity.name()))
                    )?;
            }
            // Notify the user that the chosen identity will be used.
            else {
                opts.terminal
                    .write_line(&fmt_log!("\nChosen identity: '{}' will be used for enrollment. \
                        To use the default identity, run `ockam enroll` instead.", 
                        color_primary(named_identity.name()))
                    )?;
            }
            
        }

This update will show different messages based on whether a default identity exists, and if a specific identity is chosen. For example:

  • ockam enroll with no default identity will now indicate that a new identity is created:
No Identity name exists. A new identity was created for you: '[name]'.
  • ockam enroll with a default identity will inform the user about using the default identity:
Existing default identity: '[name]' will be used for enrollment. To use a different identity, run `ockam enroll --identity <IDENTITY_NAME>`.
  • ockam enroll --identity [name] will confirm the chosen identity's usage:
Chosen identity: '[name]' will be used for enrollment. To use the default identity, run `ockam enroll` instead.

Do you think additional details, like vault information, should be included?

Best, Yorick

YorickdeJong avatar Jan 31 '24 13:01 YorickdeJong

This PR https://github.com/build-trust/ockam/pull/7616 closes this issue

nazmulidris avatar Feb 27 '24 14:02 nazmulidris