Preparing Polykey for Local Demo

[tegefaulkes]

Description

Working software trumps diagrams/presentations/talks. So we want a working demo with associated diagrams running right off staging branch atm.

The relevant commands that we need to see working are below.

For each of the commands we need:

1. Asciinema recording of the CLI execution
2. Diagram on excalidraw showing the control-flow and data-flow from the user's perspective having the components of: User, CLI, Agent, FileSystem, Identity Provider (any thing else that is relevant)
3. Fix up any bugs or minor usability issues here

Agent Start

pk agent start

This command needs:

1. Bootstrap the ~/.local/share/polykey if it doesn't exist.
2. Start the agent if the agent state already exists.
3. Ask you to setup a new password and provide you a recovery code if it doesn't exist.
4. Ask you enter the existing password or recovery code if it does exist.
5. Give you the current agent status once booted up.

Agent Stop

pk agent stop

This command needs:

1. Shutdown the agent gracefully if it is running.
2. Do nothing if the agent is not running.

Agent Status

pk agent status

This command needs:

1. Show the agent status when the agent is running.

Vaults Create

pk vaults create myvault

This command needs:

1. Create the vault called myvault.

Secrets Create

pk secrets create ./the-secret-file.txt myvault:the-secret-file.txt

This command needs:

1. Upload the secret file from ./the-secret-file.txt into myvault.

Secrets Get

pk secrets get myvault:the-secret-file.txt

This command needs:

1. Show the contents of the-secret-file.txt on the STDOUT of the CLI.

Secrets Delete

pk secrets delete myvault:the-secret-file.txt

The command needs:

1. Delete the the file from myvault.

Secrets Env

pk secrets env myvault:the-secret-file.txt bash

This command needs:

1. This needs to inject the value of the secret into the environment of the bash shell.
2. This is optional, it's a stretch goal.
3. Addresses The pk secrets env command for meeting Development Environment Usecase Polykey-CLI#31.

Identities Authenticate

pk identities authenticate github.com cmcdragonkai

This command needs:

1. Authenticates with github, and store the provider token

Identities Claim

pk identities claim github.com cmcdragonkai

This command needs:

1. Claim the identity on the cmcdragonkai gists: https://gist.github.com/CMCDragonkai, and post a cryptolink there

Tasks

• 1. pk agent start
• 2. pk agent stop
• 3. pk agent status
• 4. pk vaults create
• 5. pk secrets create
• 6. pk secrets get
• 7. pk secrets delete
• 8. pk secrets env
• 9. pk identities authenticate
• 10. pk identities claim

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[tegefaulkes]

• PK agent start is not prompting for the password twice when starting from clean state. It is however prompting twice when starting with existing state but specifying the --fresh flag.
• when starting fresh and specifying the recovery code we do get two prompts as expected.
• when starting with state and specifying the recovery code we get two prompts as expected.

So there is a bug with starting from no state where the password is only prompted once, it should be twice.

• State is maintained when starting with a recovery code from existing state.
• The password is changed when recovering
• The existing session token is still valid after recovering

I think current sessions should be locked if we change the root password of the agent. That doesn't seem to be happening right now.

[amydevs]

Asciinemas

1. pk agent start
   • First Run
   • Subsequent Runs
2. pk agent stop
3. pk agent status
4. pk vaults create
5. pk secrets create
6. pk secrets get
7. pk secrets delete
8. pk secrets env - not yet ready
9. pk identities authenticate
10. pk identities claim

[tegefaulkes]

Starting the Polykey agent with an invalid password gives a valid but ugly response.

[nix-shell:~/matixWorkspace/gitRepos/Polykey]$ npm run polykey -- agent start --node-path /tmp/pkt1

> [email protected] polykey
> ts-node src/bin/polykey.ts "agent" "start" "--node-path" "/tmp/pkt1"

✔ Please enter the password … ********
WARN:polykey.PolykeyAgent:Failed Creating PolykeyAgent
ErrorKeyPairParse: Unable to parse root keypair - Private key path /tmp/pkt1/state/keys/private.jwk is not a valid encrypted JWK
  exitCode      74
  timestamp     Fri Feb 03 2023 14:23:23 GMT+1100 (Australian Eastern Daylight Time)

This should be a cleaner output with just the message along the lines of invalid password and prompt to try again. This would feel broken to an end-user.

We should catch and re-throw the error here as a wrong password error since that makes more sense in this context. Also we should look at less verbose formatting of the errors.

[amydevs]

Starting the Polykey agent with an invalid password gives a valid but ugly response.

[nix-shell:~/matixWorkspace/gitRepos/Polykey]$ npm run polykey -- agent start --node-path /tmp/pkt1

> [email protected] polykey
> ts-node src/bin/polykey.ts "agent" "start" "--node-path" "/tmp/pkt1"

✔ Please enter the password … ********
WARN:polykey.PolykeyAgent:Failed Creating PolykeyAgent
ErrorKeyPairParse: Unable to parse root keypair - Private key path /tmp/pkt1/state/keys/private.jwk is not a valid encrypted JWK
  exitCode      74
  timestamp     Fri Feb 03 2023 14:23:23 GMT+1100 (Australian Eastern Daylight Time)

This should be a cleaner output with just the message along the lines of invalid password and prompt to try again. This would feel broken to an end-user.

I think also it might be more intuitive to say "Please enter a password:" when the user is initiating PK cos "the" kinda makes it sound like it's something the user should already know.

Also for No. 5 and 6 of #504 (comment), am I doing this correctly? Cos pk secrets get returns the secret name without the contents 4 me (Asciinemas are hyperlinked for reference) @tegefaulkes

[tegefaulkes]

I think you're doing it correct. It's likely a bug, I'll have a look at the command.

[tegefaulkes]

I've fixed polykey secrets get command. Give it another go.

It was reading the secreName field of the protobuf message for the secrets contents. We don't return anything in the name field since we provide the name when getting the secret. I just had to use the contents of the secretContents field of the protobuf message.

@CMCDragonkai suggested we return the secret name along side the contents but I'm going to defer that for now since it's not a super quick change and it's low priority.

[CMCDragonkai]

I forgot that PR #265 already exists for pk secrets env, but it's quite outdated. I started prototyping the command, and I think the only thing missing is to have a kexec, but we can do something simpler for now, which is just use run a subprocess.

[tegefaulkes]

I should check that the output for formatter is used for the output for all of these commands, even for single line outputs.

[tegefaulkes]

I think pretty much everything is covered except for the ENV command.

Just one other thing though. when changing the password on the agent via starting the agent with recovery. any active client session is still valid. I think if the password changes then we should have to unlock it again with the new password.

What behaviour do we want here?

[CMCDragonkai]

Yea if the password changes, the session tokens should be reset.

[CMCDragonkai]

Asciinemas

1. pk agent start

   • First Run
   • Subsequent Runs

2. pk agent stop

3. pk agent status

4. pk vaults create

5. pk secrets create

6. pk secrets get

7. pk secrets delete

8. pk secrets env - not yet ready

9. pk identities authenticate

10. pk identities claim

Awesome!

In the future we should have an org account under [email protected] and publish our asciicasts there (which can be embedded in our blog). Although, it may be preferable to use https://github.com/asciinema/agg to convert to gifs as that's more easily sharable.

[CMCDragonkai]

The pk secrets get command should have roundtrip tests that test verbatim file content being saved and printed later that preserves all the newlines of the original content.

Also binary data too. If the secret upload has binary data, we output the binary as is, as if the user were to run cat binarydatafile. No special consideration for binary files.

[CMCDragonkai]

I'm noticing another error while the agent is running:

WARN:polykey.PolykeyAgent.task v0ousk901hlo01ecevdv93usj80:Failed - Reason: TypeError: Reduce of empty array with no initial value

It occurs after some time while running the foreground agent.

@tegefaulkes please investigate.

It should:

> arr = []
[]
> arr.reduce(() => {})
Uncaught TypeError: Reduce of empty array with no initial value
    at Array.reduce (<anonymous>)

Any time we are reducing, we should always have an initial value.

[CMCDragonkai]

Another thing we should have:

1. Nested dictionary output formatting, this just means recalling the same output formatter in a recursive way (while incrementing an indentation level...), however this may be complicated due to types that we haven't supported yet
2. When using nested dictionary, if the value is a string, we should avoid printing out the double quote.

url	"https://github.com/login/device"
userCode	"04D6-9B53"

Better to do:

url	https://github.com/login/device
userCode	04D6-9B53

Similar to how numbers are printed out now.

I think the output formatter may require some refactoring to make it recursive capable.

[CMCDragonkai]

The claim command isn't working for me, it fails after I do authenticate:

[nix-shell:~/Projects/Polykey]$ node ./dist/bin/polykey.js identities claim github.com cmcdragonkai
ErrorPolykeyRemote: Remote error from RPC call
  command	identitiesClaim
  nodeId	vve3f83v1qog578bgav365bmmgtt71prpdogs2f2dcqcf59i60br0
  host	127.0.0.1
  port	42123
  timestamp	Fri Feb 03 2023 17:15:59 GMT+1100 (Australian Eastern Daylight Time)
  cause: ErrorProviderUnauthenticated: Provider has not been authenticated or access token is expired or invalid

[amydevs]

User Flow Diagram

[CMCDragonkai]

The claim command isn't working for me, it fails after I do authenticate:

[nix-shell:~/Projects/Polykey]$ node ./dist/bin/polykey.js identities claim github.com cmcdragonkai
ErrorPolykeyRemote: Remote error from RPC call
  command	identitiesClaim
  nodeId	vve3f83v1qog578bgav365bmmgtt71prpdogs2f2dcqcf59i60br0
  host	127.0.0.1
  port	42123
  timestamp	Fri Feb 03 2023 17:15:59 GMT+1100 (Australian Eastern Daylight Time)
  cause: ErrorProviderUnauthenticated: Provider has not been authenticated or access token is expired or invalid

The reason for this is because of case insensitivity of the login (possible). But later when in the identitiesManager this happens:

    if (!identities.includes(identityId)) {
      throw new identitiesErrors.ErrorProviderUnauthenticated();
    }

Which is a case sensitive check.

GitHub returns my username as CMCDragonkai.

This confusion stems from the fact that identities authenticate github.com cmcdragonkai actually does not require my username "identity ID" at all. We should remove it from the authenticate subcommand and also remove it from the protobuf message, it isn't used. Maybe in the future other providers will require it, but most providers will just expect the user the login to their portal.

The command ends up telling us the username which is CMCDragonkai which I can later use with pk identities claim github.com CMCDragonkai.

@tegefaulkes please fix this up by removing it from the command and GRPC message, we can still do a case sensitive check, but our error message can be better instead of saying the provider is not authenticated, it might be better to say something like the identity does not match the list of existing identities... which we could show too.

[CMCDragonkai]

Cool so @amydevs can refactor the diagrams to reduce the complexity, label the arrows, remove filesystem where it's not important.

Then re-run the asciinema with the new changes and fixes we've applied.

[CMCDragonkai]

Last thing for me is the env command.

[CMCDragonkai]

I'm doing pk secrets env in a separate branch, going to merge this now.

[CMCDragonkai]

User Flow Diagram

This will be great for Polykey Docs. Can you start incorporating this content into the docs too (keep it somewhere in the files for now so later we can properly form the content).

[CMCDragonkai]

@amydevs can you get this into Polykey docs too?