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

login with browser using nonce #4057

Merged
merged 28 commits into from
Jul 11, 2023
Merged

login with browser using nonce #4057

merged 28 commits into from
Jul 11, 2023

Conversation

dynamite-bud
Copy link
Contributor

Initially we would login from the wasmer-cli using the wasmer login command which would in turn ask for the token.

For this token a person would explicitly had to go to the wasmer registry make a token, then come back and save the token in the CLI. this is very unpleasant in terms of DX.

  1. So this PR makes the WASMER_CLI have an option to login using the browser by making a nonce, querying the GraphQL api from the registry with a callback URL.
  2. Then person authorizes himself on the frontend of the website and comes back to the CLI and he is authorized now and his token is saved in the config file.

@syrusakbary
Copy link
Member

Comments:

  • Logic needs to be in wasmer login, not in wasmer login-with-browser. Basically the old logic should work if people do wasmer login THE_TOKEN, but if people simply do wasmer login, the browser path should be chosen
  • We should not show in any way SERVER url, or the token. The output should be as follows:
$ wasmer login 
Opening auth link in your default browser: https://frontend-git-867-add-auth-flow-for-the-wasmer-cli-frontend-wapm.vercel.app/auth/cli?nonce_id=nnc_ZR4qIGtAiZA2&secret=kX2GjcvqlI6oR12mHaB3_HtqqS3go1EMRTBNJJIg-Rc
Waiting for session... Done
Login for Wasmer user "syrusakbary" saved

If the user cancels the auth in the browser, the output should be:

$ wasmer login 
Opening auth link in your default browser: https://frontend-git-867-add-auth-flow-for-the-wasmer-cli-frontend-wapm.vercel.app/auth/cli?nonce_id=nnc_ZR4qIGtAiZA2&secret=kX2GjcvqlI6oR12mHaB3_HtqqS3go1EMRTBNJJIg-Rc
Waiting for session... Cancelled by the user

If the user takes more than (10 mins? ask @ayys) to complete, the output should be:

$ wasmer login 
Opening auth link in your default browser: https://frontend-git-867-add-auth-flow-for-the-wasmer-cli-frontend-wapm.vercel.app/auth/cli?nonce_id=nnc_ZR4qIGtAiZA2&secret=kX2GjcvqlI6oR12mHaB3_HtqqS3go1EMRTBNJJIg-Rc
Waiting for session... Timed out (10 mins exceeded)

@ayys
Copy link
Member

ayys commented Jul 6, 2023

Yup, nonces expire after 1- minutes (configurable via Doppler)

@dynamite-bud
Copy link
Contributor Author

dynamite-bud commented Jul 6, 2023

@syrusakbary the behaviour exists on wasmer login

@syrusakbary
Copy link
Member

We should set it to 5 mins min? 1 min seems very quick

@syrusakbary
Copy link
Member

@syrusakbary the behaviour exists on wasmer login

What behavior are you referring to?

@dynamite-bud
Copy link
Contributor Author

@syrusakbary the behaviour exists on wasmer login

What behavior are you referring to?

$ wasmer login --registry https://registry.wasmer.wtf/graphql 
Server URL: http://localhost:64307
Opening browser at https://frontend-git-add-auth-flow-for-the-wasmer-cli-frontend-wapm.vercel.app/auth/cli?nonce_id=nnc_YIZteiOYR&secret=R2diNa5dcXzbJ9abCotp7-Eorag
Token: wap_f77d01da79caad37f7e1553850b271c2b61b***
Shutting down server
Login for Wasmer user "xorcist" saved

@syrusakbary
Copy link
Member

syrusakbary commented Jul 7, 2023

Missing:

  • Texts should be the same as prompted
  • Fix the frontend with the cancel user
  • 10 minutes in the CLI not working
  • Use hyper

@syrusakbary
Copy link
Member

syrusakbary commented Jul 7, 2023

Texts as suggestions means: text as I suggested in a previous comment: #4057 (comment)

@dynamite-bud dynamite-bud marked this pull request as ready for review July 8, 2023 08:38
lib/cli/Cargo.toml Outdated Show resolved Hide resolved
AuthorizationState::TokenSuccess(token) => {
assert_eq!(
token,
"Please paste the login token from https://wasmer.wtf/settings/access-tokens"
Copy link
Member

@syrusakbary syrusakbary Jul 10, 2023

Choose a reason for hiding this comment

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

This should be wasmer.io not wtf ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are tests @syrusakbary

@syrusakbary syrusakbary merged commit d83c8fa into master Jul 11, 2023
@syrusakbary syrusakbary deleted the cli-login-through-browser branch July 11, 2023 16:18
@dynamite-bud
Copy link
Contributor Author

closes #4034

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 this pull request may close these issues.

3 participants