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

CLI - Just-in-time login flow #2158

Merged
merged 26 commits into from
Jan 31, 2025
Merged

CLI - Just-in-time login flow #2158

merged 26 commits into from
Jan 31, 2025

Conversation

bfops
Copy link
Collaborator

@bfops bfops commented Jan 22, 2025

Description of Changes

When a user goes to perform an action that requires logging in, we prompt them whether they want to log in.

If they decline to log in, we auth them directly with the server that they're trying to access.

Note: We are still using the endpoint-based auth flow, and not doing any kind of scan for a key on the local machine! I have never gotten an answer for what code I'm supposed to use to encrypt a token "manually" once I have the key in hand. IIRC when we last discussed, we said it was fine to keep using this endpoint, but if not then I would still need to know where that code lives / how to use it.

The following commands now have the potential to prompt the user to log in:

  • call
  • delete
  • describe
  • rename
  • energy
  • list
  • logs
  • publish
  • sql
  • subscribe

All of these commands have been updated to have a --yes parameter (if they didn't already). Somewhat unintuitively, this will actually answer "no" to the "log in with spacetimedb.com?" prompt so that it can obey its deeper mission of proceeding non-interactively.

The login show command has not been updated to this "just in time" login behavior. It seems unintuitive to prompt the user to log in when they're trying to login show. Instead, it tells them they're not logged in.

API and ABI breaking changes

API breaking in the sense that it adds a confirmation prompt to many CLI actions.

Expected complexity level and risk

2

Testing

  • Logging out and then publishing prompts to log in
$ spacetimedb-cli logout
Saving config to /home/lead/.config/spacetime/cli.toml.

$ spacetimedb-cli publish -s local 
You are not logged in. Would you like to log in with spacetimedb.com? [y/N]
  • Staying logged in and then publishing does not prompt
$ spacetimedb-cli publish -s local 
checking crate with spacetimedb's clippy configuration
    Checking spacetime-module v0.1.0 (/mnt/nutera/work/my_new_project)
    Finished `release` profile [optimized] target(s) in 0.14s
    Finished `release` profile [optimized] target(s) in 0.07s
Optimising module with wasm-opt...
Could not find wasm-opt to optimise the module.
For best performance install wasm-opt from https://github.com/WebAssembly/binaryen/releases.
Continuing with unoptimised module.
Build finished successfully.
Uploading to local => http://127.0.0.1:3000
Publishing module...
Created new database with identity: c2004ae02e7a187daeb619cc2e21739ac62ee18b9dec7576720585c1ea8a09ee
  • Logging out and then declining to log in causes a direct server log in, and successfully publishes
$ spacetimedb-cli logout
Saving config to /home/lead/.config/spacetime/cli.toml.

$ spacetimedb-cli publish -s local 
You are not logged in. Would you like to log in with spacetimedb.com? [y/N]N
Saving config to /home/lead/.config/spacetime/cli.toml.
    Finished `release` profile [optimized] target(s) in 0.07s
Optimising module with wasm-opt...
Could not find wasm-opt to optimise the module.
For best performance install wasm-opt from https://github.com/WebAssembly/binaryen/releases.
Continuing with unoptimised module.
Build finished successfully.
Uploading to local => http://127.0.0.1:3000
Publishing module...
Created new database with identity: c200fe686e34bd022c3d64bac8227b979ff1a714e57bc540fe106e69c96d7775
  • ...and that login does not work for other servers
$ spacetimedb-cli publish -s testnet
    Finished `release` profile [optimized] target(s) in 0.04s
Optimising module with wasm-opt...
Could not find wasm-opt to optimise the module.
For best performance install wasm-opt from https://github.com/WebAssembly/binaryen/releases.
Continuing with unoptimised module.
Build finished successfully.
You are about to publish to a non-local server: testnet.spacetimedb.com
Are you sure you want to proceed? [y/N]y
Uploading to testnet => https://testnet.spacetimedb.com
Publishing module...
Error: Identity c200115452f755e4bc722b7b32cb440b338e50899049b89b6d22e856cbfdb070 is not valid for server testnet.
Please log back in with `spacetime logout` and then `spacetime login`.

Caused by:
    TokenError(Error(InvalidSignature))
  • Logging out and then agreeing to log in opens a browser window, then successfully publishes
$ spacetimedb-cli logout
Saving config to /home/lead/.config/spacetime/cli.toml.

$ spacetimedb-cli publish -s local
You are not logged in. Would you like to log in with spacetimedb.com? [y/N]y
Opening https://spacetimedb.com/login/cli?token=eyJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3Mzc2NTc5OTMwODQsInN1YiI6IjAxSkpBNzc3V0s0WlYwMFcwV0gwSEVXSDlGIn0.aqFlrhqGW0kbfqYr7ZW2vluCPu4zR6z5-UJeYRRlfiQ in your browser.
Waiting to hear response from the server...
Login successful!
Saving config to /home/lead/.config/spacetime/cli.toml.
Saving config to /home/lead/.config/spacetime/cli.toml.
    Finished `release` profile [optimized] target(s) in 0.05s
Optimising module with wasm-opt...
Could not find wasm-opt to optimise the module.
For best performance install wasm-opt from https://github.com/WebAssembly/binaryen/releases.
Continuing with unoptimised module.
Build finished successfully.
Uploading to local => http://127.0.0.1:3000
Publishing module...
Created new database with identity: c2004444682cf6d87407940e88592e0c6c51c8ca0d73f74a00578a567a52673e
  • ...and that login does work for other servers
$ spacetimedb-cli publish -s testnet
    Finished `release` profile [optimized] target(s) in 0.07s
Optimising module with wasm-opt...
Could not find wasm-opt to optimise the module.
For best performance install wasm-opt from https://github.com/WebAssembly/binaryen/releases.
Continuing with unoptimised module.
Build finished successfully.
You are about to publish to a non-local server: testnet.spacetimedb.com
Are you sure you want to proceed? [y/N]y
Uploading to testnet => https://testnet.spacetimedb.com
Publishing module...
Created new database with identity: c2002493cea901e0ac0888a4a68df2fcc99991700456b0961d5a594d38433cd3
  • login show works properly when logged in
$ spacetimedb-cli login show
You are logged in as c20058f5a5bbcbfd65110416110492e91d5ab64054640deb456bd5fa8a2ab9d5
  • login show gives an error when not logged in
$ spacetimedb-cli login show
You are not logged in. Run `spacetime login` to log in.
  • Existing automated smoketests still pass (so any CLI coverage in the smoketests still works as expected)

crates/cli/src/common_args.rs Outdated Show resolved Hide resolved
@bfops bfops added CLI only This change only affects the CLI behavior release-1.0 api-break A PR that makes an API breaking change labels Jan 22, 2025
@bfops bfops changed the title (WIP) Just-in-time login flow CLI - Just-in-time login flow Jan 22, 2025
@bfops bfops marked this pull request as ready for review January 23, 2025 18:42
@bfops bfops requested a review from cloutiertyler as a code owner January 23, 2025 18:42
@cloutiertyler
Copy link
Contributor

Logging out and then declining to log in causes a direct server log in, and successfully publishes

We should probably print something explaining what happens. e.g.

Logging in with a server issued identity: 0xc200deadbeef
WARNING: This identity is only valid on the server that issued it.

@bfops
Copy link
Collaborator Author

bfops commented Jan 30, 2025

Logging out and then declining to log in causes a direct server log in, and successfully publishes

We should probably print something explaining what happens. e.g.

Logging in with a server issued identity: 0xc200deadbeef
WARNING: This identity is only valid on the server that issued it.

Done. I just did a simple message though. Feel free to iterate on it more (inline in the diff).

Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good to me. Thanks for addressing the feedback and also for the detailed testing section and comments. It made review very straightforward.

@bfops
Copy link
Collaborator Author

bfops commented Jan 31, 2025

I'm going to merge this given how much trouble I'm having triggering the PermissionDenied or Unauthorized flows for testing. We can iterate on these cases later if something's not working properly.

@bfops bfops enabled auto-merge January 31, 2025 17:40
@bfops bfops added this pull request to the merge queue Jan 31, 2025
Merged via the queue into master with commit 382db47 Jan 31, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break A PR that makes an API breaking change CLI only This change only affects the CLI behavior release-1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants