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

@@ -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:
 

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(),
 	}

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)
 	}

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 {

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)