Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Signer new-token generates a link and opens browser #3379

Merged
merged 10 commits into from
Nov 16, 2016
Merged

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Nov 11, 2016

@tomusdrw tomusdrw added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Nov 11, 2016
r#"
Open: {}
to authorize your browser.
Or use the code: {}"#,
Copy link
Contributor

Choose a reason for hiding this comment

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

Have also had a few requests to put the code on its own line to make it easier to copy

@gavofyork
Copy link
Contributor

moving in the right direction here.

since it now opens a window, let's just have the user run parity open rather than parity signer new-token?

@tomusdrw
Copy link
Collaborator Author

So I think we should rather:

  1. Remove parity signer new-token
  2. Have parity ui open UI with newly generated token each time
  3. But we also need to keep track of token creation date and number of times it was used
  4. Remove old tokens that have never been used.

@jacogr
Copy link
Contributor

jacogr commented Nov 12, 2016

UI code to handle the auth token as passed forthcoming in this PR.

EDIT: UI handles #/auth?token=valid-token-here path correctly - if token setting fails, the same usual connections fallbacks are still in-place (in this case we expect to never get there)

@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Nov 14, 2016

  1. parity ui runs parity, generates new token and opens up a browser with UI (in case parity is running it just opens a browser)
  2. parity signer new-token generates token, opens browser and quits.
  3. We track creation time and usage of tokens.
  4. Tokens which were never used and are more then a day old are removed.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 14, 2016
@gavofyork
Copy link
Contributor

parity signer new-token generates token, opens browser and quits

so a user doesn't care about authorisation tokens - they only care about using parity's UI. if this opens a browser window, it should be called parity open or some such. keeping it with the present name:
a) confuses any power-users that expect it just to create a new token; but way more importantly:
b) is a really obscure command for actually opening a new window.

@gavofyork gavofyork added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 16, 2016
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Nov 16, 2016
@tomusdrw
Copy link
Collaborator Author

parity ui - opens a browser with new token in URL (starts parity if not running already)
parity signer new-token - displays the new token (and link for convenience) and quits

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 16, 2016
@gavofyork gavofyork merged commit 2595034 into master Nov 16, 2016
@gavofyork gavofyork deleted the new-token branch November 16, 2016 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants