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

New signer token RPC & Initial signer connection without token. #2096

Merged
merged 6 commits into from
Sep 21, 2016

Conversation

tomusdrw
Copy link
Collaborator

  1. personal_generateAuthorizationToken RPC method available only over Signer's WebSocket connection.
  2. When there are no tokens generated (store is empty = i.e. first run) single WebSocket connection will be allowed to access using special token with value initial.

Closes #1999

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Sep 14, 2016
@rphmeier rphmeier added the M4-core ⛓ Core client code / Rust. label Sep 14, 2016
@jacogr
Copy link
Contributor

jacogr commented Sep 15, 2016

General question around functionality -

  1. User opens (new) UI is Chrome, takes him/her through the setup process, the token gets generated and stored in localStorage. Everything all ok, seamless.
  2. User opens UI in FireFox (or extension) - what happens? Browser store is empty since it doesn't have a token, but... Any issues?

@tomusdrw
Copy link
Collaborator Author

Yes, so the second signer should ask for authorization token, which can be generated:

  1. Via CLI (as currently)
  2. Via previously authorized Signer (using personal_generateAuthorizationToken RPC endpoint)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 85.206% when pulling 63d7ffb on signer-initial into c8533a3 on master.

@@ -43,10 +44,11 @@ pub fn read_block(lines: &mut Lines, all: bool) -> String {

pub fn request(address: &SocketAddr, request: &str) -> Response {
let mut req = TcpStream::connect(address).unwrap();
req.set_read_timeout(Some(Duration::from_secs(1))).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

is there an indication that this is in fact enough time for all requests to be handled? Spurious test failures caused by a too-short timeout would be annoying.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So there are no requests that are doing any heavy stuff, but of course there is no guarantee that all of them will be handled.
Unfortunatelly it's necessary for some other requests that are not closing the connection after replying with a handshake (i.e. websocket upgrade requests).

I didn't have any other idea how to handle those - read without timeout blocks idefinitely, setting non-blocking mode returns WouldBlock on every try, tbh I have no idea why I cannot just read a handshake and then close the connection (Same thing happens if http server is trying to keep the connection alive (i.e. Connection: close header is missing))

I'm open to any ideas.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 20, 2016
@gavofyork gavofyork merged commit b7e2afd into master Sep 21, 2016
@gavofyork gavofyork deleted the signer-initial branch September 21, 2016 10:44
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.

5 participants