From be771f225a102373d52e9571dfc26ab6ffb569c6 Mon Sep 17 00:00:00 2001 From: Greg Szabo <16846635+greg-szabo@users.noreply.github.com> Date: Wed, 8 Jan 2020 02:41:14 -0500 Subject: [PATCH] rpc/lib: RPC client basic authentication with URL parsing refactored (#4285) Enable basic authentication in the RPC client using the https://username:password@node:port format. Issue #4248 has details about what was refactored/enhanced (it's not as bad as it looks.) I'm open to suggestions on where/how the documentation should be updated. Please note that PR #4284 is superseded with this PR. The reason for this is because both PR makes changes to the same code. --- CHANGELOG_PENDING.md | 1 + rpc/lib/client/http_client.go | 142 +++++++++++++++++------------ rpc/lib/client/http_client_test.go | 6 +- rpc/lib/client/ws_client.go | 10 +- rpc/lib/client/ws_client_test.go | 11 ++- 5 files changed, 102 insertions(+), 68 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 3b8eaf48c..359c952e3 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -16,6 +16,7 @@ program](https://hackerone.com/tendermint). - Go API ### FEATURES: +- [rpc/lib] [\#4248](https://github.com/tendermint/tendermint/issues/4248) RPC client basic authentication support (@greg-szabo) ### IMPROVEMENTS: diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index 0673177c3..fc31de504 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -28,61 +28,60 @@ const ( protoTCP = "tcp" ) -// HTTPClient is a common interface for JSONRPCClient and URIClient. -type HTTPClient interface { - Call(method string, params map[string]interface{}, result interface{}) (interface{}, error) - Codec() *amino.Codec - SetCodec(*amino.Codec) +// Parsed URL structure +type parsedURL struct { + url.URL } -// protocol - client's protocol (for example, "http", "https", "wss", "ws", "tcp") -// trimmedS - rest of the address (for example, "192.0.2.1:25", "[2001:db8::1]:80") with "/" replaced with "." -func toClientAddrAndParse(remoteAddr string) (network string, trimmedS string, err error) { - protocol, address, err := parseRemoteAddr(remoteAddr) +// Parse URL and set defaults +func newParsedURL(remoteAddr string) (*parsedURL, error) { + u, err := url.Parse(remoteAddr) if err != nil { - return "", "", err + return nil, err + } + + // default to tcp if nothing specified + if u.Scheme == "" { + u.Scheme = protoTCP } + return &parsedURL{*u}, nil +} + +// Change protocol to HTTP for unknown protocols and TCP protocol - useful for RPC connections +func (u *parsedURL) SetDefaultSchemeHTTP() { // protocol to use for http operations, to support both http and https - var clientProtocol string - // default to http for unknown protocols (ex. tcp) - switch protocol { + switch u.Scheme { case protoHTTP, protoHTTPS, protoWS, protoWSS: - clientProtocol = protocol + // known protocols not changed default: - clientProtocol = protoHTTP + // default to http for unknown protocols (ex. tcp) + u.Scheme = protoHTTP } +} +// Get full address without the protocol - useful for Dialer connections +func (u parsedURL) GetHostWithPath() string { + // Remove protocol, userinfo and # fragment, assume opaque is empty + return u.Host + u.EscapedPath() +} + +// Get a trimmed address - useful for WS connections +func (u parsedURL) GetTrimmedHostWithPath() string { // replace / with . for http requests (kvstore domain) - trimmedAddress := strings.Replace(address, "/", ".", -1) - return clientProtocol, trimmedAddress, nil + return strings.Replace(u.GetHostWithPath(), "/", ".", -1) } -func toClientAddress(remoteAddr string) (string, error) { - clientProtocol, trimmedAddress, err := toClientAddrAndParse(remoteAddr) - if err != nil { - return "", err - } - return clientProtocol + "://" + trimmedAddress, nil -} - -// network - name of the network (for example, "tcp", "unix") -// s - rest of the address (for example, "192.0.2.1:25", "[2001:db8::1]:80") -// TODO: Deprecate support for IP:PORT or /path/to/socket -func parseRemoteAddr(remoteAddr string) (network string, s string, err error) { - parts := strings.SplitN(remoteAddr, "://", 2) - var protocol, address string - switch { - case len(parts) == 1: - // default to tcp if nothing specified - protocol, address = protoTCP, remoteAddr - case len(parts) == 2: - protocol, address = parts[0], parts[1] - default: - return "", "", fmt.Errorf("invalid addr: %s", remoteAddr) - } +// Get a trimmed address with protocol - useful as address in RPC connections +func (u parsedURL) GetTrimmedURL() string { + return u.Scheme + "://" + u.GetTrimmedHostWithPath() +} - return protocol, address, nil +// HTTPClient is a common interface for JSONRPCClient and URIClient. +type HTTPClient interface { + Call(method string, params map[string]interface{}, result interface{}) (interface{}, error) + Codec() *amino.Codec + SetCodec(*amino.Codec) } func makeErrorDialer(err error) func(string, string) (net.Conn, error) { @@ -92,11 +91,13 @@ func makeErrorDialer(err error) func(string, string) (net.Conn, error) { } func makeHTTPDialer(remoteAddr string) func(string, string) (net.Conn, error) { - protocol, address, err := parseRemoteAddr(remoteAddr) + u, err := newParsedURL(remoteAddr) if err != nil { return makeErrorDialer(err) } + protocol := u.Scheme + // accept http(s) as an alias for tcp switch protocol { case protoHTTP, protoHTTPS: @@ -104,7 +105,7 @@ func makeHTTPDialer(remoteAddr string) func(string, string) (net.Conn, error) { } return func(proto, addr string) (net.Conn, error) { - return net.Dial(protocol, address) + return net.Dial(protocol, u.GetHostWithPath()) } } @@ -142,10 +143,12 @@ type JSONRPCRequestBatch struct { // JSONRPCClient takes params as a slice type JSONRPCClient struct { - address string - client *http.Client - id types.JSONRPCStringID - cdc *amino.Codec + address string + username string + password string + client *http.Client + id types.JSONRPCStringID + cdc *amino.Codec } // JSONRPCCaller implementers can facilitate calling the JSON RPC endpoint. @@ -170,16 +173,24 @@ func NewJSONRPCClientWithHTTPClient(remote string, client *http.Client) *JSONRPC panic("nil http.Client provided") } - clientAddress, err := toClientAddress(remote) + parsedURL, err := newParsedURL(remote) if err != nil { panic(fmt.Sprintf("invalid remote %s: %s", remote, err)) } + parsedURL.SetDefaultSchemeHTTP() + + address := parsedURL.GetTrimmedURL() + username := parsedURL.User.Username() + password, _ := parsedURL.User.Password() + return &JSONRPCClient{ - address: clientAddress, - client: client, - id: types.JSONRPCStringID("jsonrpc-client-" + cmn.RandStr(8)), - cdc: amino.NewCodec(), + address: address, + username: username, + password: password, + client: client, + id: types.JSONRPCStringID("jsonrpc-client-" + cmn.RandStr(8)), + cdc: amino.NewCodec(), } } @@ -195,7 +206,15 @@ func (c *JSONRPCClient) Call(method string, params map[string]interface{}, resul return nil, err } requestBuf := bytes.NewBuffer(requestBytes) - httpResponse, err := c.client.Post(c.address, "text/json", requestBuf) + httpRequest, err := http.NewRequest(http.MethodPost, c.address, requestBuf) + if err != nil { + return nil, err + } + httpRequest.Header.Set("Content-Type", "text/json") + if c.username != "" || c.password != "" { + httpRequest.SetBasicAuth(c.username, c.password) + } + httpResponse, err := c.client.Do(httpRequest) if err != nil { return nil, err } @@ -228,7 +247,15 @@ func (c *JSONRPCClient) sendBatch(requests []*jsonRPCBufferedRequest) ([]interfa if err != nil { return nil, err } - httpResponse, err := c.client.Post(c.address, "text/json", bytes.NewBuffer(requestBytes)) + httpRequest, err := http.NewRequest(http.MethodPost, c.address, bytes.NewBuffer(requestBytes)) + if err != nil { + return nil, err + } + httpRequest.Header.Set("Content-Type", "text/json") + if c.username != "" || c.password != "" { + httpRequest.SetBasicAuth(c.username, c.password) + } + httpResponse, err := c.client.Do(httpRequest) if err != nil { return nil, err } @@ -315,12 +342,15 @@ type URIClient struct { // The function panics if the provided remote is invalid. func NewURIClient(remote string) *URIClient { - clientAddress, err := toClientAddress(remote) + parsedURL, err := newParsedURL(remote) if err != nil { panic(fmt.Sprintf("invalid remote %s: %s", remote, err)) } + + parsedURL.SetDefaultSchemeHTTP() + return &URIClient{ - address: clientAddress, + address: parsedURL.GetTrimmedURL(), client: DefaultHTTPClient(remote), cdc: amino.NewCodec(), } diff --git a/rpc/lib/client/http_client_test.go b/rpc/lib/client/http_client_test.go index b74425a54..fd8d52817 100644 --- a/rpc/lib/client/http_client_test.go +++ b/rpc/lib/client/http_client_test.go @@ -7,14 +7,14 @@ import ( ) func TestHTTPClientMakeHTTPDialer(t *testing.T) { - remote := []string{"https://foo-bar.com:80", "http://foo-bar.net:80"} + remote := []string{"https://foo-bar.com:80", "http://foo-bar.net:80", "https://user:pass@foo-bar.net:80"} for _, f := range remote { - protocol, address, err := parseRemoteAddr(f) + u, err := newParsedURL(f) require.NoError(t, err) dialFn := makeHTTPDialer(f) - addr, err := dialFn(protocol, address) + addr, err := dialFn(u.Scheme, u.GetHostWithPath()) require.NoError(t, err) require.NotNil(t, addr) } diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index d343a3019..b0d09ca97 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -80,18 +80,18 @@ type WSClient struct { // pong wait time. The endpoint argument must begin with a `/`. // The function panics if the provided address is invalid. func NewWSClient(remoteAddr, endpoint string, options ...func(*WSClient)) *WSClient { - protocol, addr, err := toClientAddrAndParse(remoteAddr) + parsedURL, err := newParsedURL(remoteAddr) if err != nil { panic(fmt.Sprintf("invalid remote %s: %s", remoteAddr, err)) } // default to ws protocol, unless wss is explicitly specified - if protocol != "wss" { - protocol = "ws" + if parsedURL.Scheme != protoWSS { + parsedURL.Scheme = protoWS } c := &WSClient{ cdc: amino.NewCodec(), - Address: addr, + Address: parsedURL.GetTrimmedHostWithPath(), Dialer: makeHTTPDialer(remoteAddr), Endpoint: endpoint, PingPongLatencyTimer: metrics.NewTimer(), @@ -100,7 +100,7 @@ func NewWSClient(remoteAddr, endpoint string, options ...func(*WSClient)) *WSCli readWait: defaultReadWait, writeWait: defaultWriteWait, pingPeriod: defaultPingPeriod, - protocol: protocol, + protocol: parsedURL.Scheme, } c.BaseService = *cmn.NewBaseService(nil, "WSClient", c) for _, option := range options { diff --git a/rpc/lib/client/ws_client_test.go b/rpc/lib/client/ws_client_test.go index 1babdae92..07200e152 100644 --- a/rpc/lib/client/ws_client_test.go +++ b/rpc/lib/client/ws_client_test.go @@ -64,7 +64,8 @@ func TestWSClientReconnectsAfterReadFailure(t *testing.T) { s := httptest.NewServer(h) defer s.Close() - c := startClient(t, s.Listener.Addr().String()) + // https://github.com/golang/go/issues/19297#issuecomment-282651469 + c := startClient(t, "//" + s.Listener.Addr().String()) defer c.Stop() wg.Add(1) @@ -96,7 +97,8 @@ func TestWSClientReconnectsAfterWriteFailure(t *testing.T) { h := &myHandler{} s := httptest.NewServer(h) - c := startClient(t, s.Listener.Addr().String()) + // https://github.com/golang/go/issues/19297#issuecomment-282651469 + c := startClient(t, "//" + s.Listener.Addr().String()) defer c.Stop() wg.Add(2) @@ -124,7 +126,8 @@ func TestWSClientReconnectFailure(t *testing.T) { h := &myHandler{} s := httptest.NewServer(h) - c := startClient(t, s.Listener.Addr().String()) + // https://github.com/golang/go/issues/19297#issuecomment-282651469 + c := startClient(t, "//" + s.Listener.Addr().String()) defer c.Stop() go func() { @@ -173,7 +176,7 @@ func TestWSClientReconnectFailure(t *testing.T) { func TestNotBlockingOnStop(t *testing.T) { timeout := 2 * time.Second s := httptest.NewServer(&myHandler{}) - c := startClient(t, s.Listener.Addr().String()) + c := startClient(t, "//" + s.Listener.Addr().String()) c.Call(context.Background(), "a", make(map[string]interface{})) // Let the readRoutine get around to blocking time.Sleep(time.Second)