Merge 30e1d4c into 7e7bd7a

server/server.go

@@ -1210,9 +1210,24 @@ func (s *StanServer) stanDisconnectedHandler(nc *nats.Conn) {
 	}
 }
 
+func obfuscatePasswordInURL(url string) string {
+	atPos := strings.Index(url, "@")
+	if atPos == -1 {
+		return url
+	}
+	start := strings.Index(url, "://")
+	if start == -1 {
+		return "invalid url"
+	}
+	newurl := url[:start+3]
+	newurl += "[REDACTED]"
+	newurl += url[atPos:]
+	return newurl
+}
+
 func (s *StanServer) stanReconnectedHandler(nc *nats.Conn) {
 	s.log.Noticef("connection %q reconnected to NATS Server at %q",
-		nc.Opts.Name, nc.ConnectedUrl())
+		nc.Opts.Name, obfuscatePasswordInURL(nc.ConnectedUrl()))
 }
 
 func (s *StanServer) stanClosedHandler(nc *nats.Conn) {
@@ -1244,8 +1259,6 @@ func (s *StanServer) buildServerURLs() ([]string, error) {
 			}
 			return urls, nil
 		}
-		// Otherwise, prepare the host and port and continue to see
-		// if user/pass needs to be added.
 
 		// First trim the protocol.
 		parts := strings.Split(natsURL, "://")
@@ -1272,15 +1285,6 @@ func (s *StanServer) buildServerURLs() ([]string, error) {
 			hostport = net.JoinHostPort(opts.Host, sport)
 		}
 	}
-	var userpart string
-	if opts.Authorization != "" {
-		userpart = opts.Authorization
-	} else if opts.Username != "" {
-		userpart = fmt.Sprintf("%s:%s", opts.Username, opts.Password)
-	}
-	if userpart != "" {
-		return []string{fmt.Sprintf("nats://%s@%s", userpart, hostport)}, nil
-	}
 	return []string{fmt.Sprintf("nats://%s", hostport)}, nil
 }
 
@@ -1295,6 +1299,10 @@ func (s *StanServer) createNatsClientConn(name string) (*nats.Conn, error) {
 	if err != nil {
 		return nil, err
 	}
+	ncOpts.User = s.natsOpts.Username
+	ncOpts.Password = s.natsOpts.Password
+	ncOpts.Token = s.natsOpts.Authorization
+
 	ncOpts.Name = fmt.Sprintf("_NSS-%s-%s", s.opts.ID, name)
 
 	if err = nats.ErrorHandler(s.stanErrorHandler)(&ncOpts); err != nil {
@@ -1334,8 +1342,6 @@ func (s *StanServer) createNatsClientConn(name string) (*nats.Conn, error) {
 	// on reconnect.
 	ncOpts.ReconnectBufSize = -1
 
-	s.log.Tracef(" NATS conn opts: %v", ncOpts)
-
 	var nc *nats.Conn
 	if nc, err = ncOpts.Connect(); err != nil {
 		return nil, err

server/server_run_test.go

@@ -110,6 +110,9 @@ func TestServerLoggerDebugAndTrace(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error running server: %v", err)
 	}
+	sc := NewDefaultConnection(t)
+	sc.Publish("foo", []byte("hello"))
+	sc.Close()
 	s.Shutdown()
 	// Signal that we are done (the channel is buffered)
 	done <- true
@@ -121,7 +124,7 @@ func TestServerLoggerDebugAndTrace(t *testing.T) {
 	// This is a bit dependent on what we currently print with
 	// trace and debug. May need to be adjusted.
 	str := string(out)
-	if !strings.Contains(str, "NATS conn opts") || !strings.Contains(str, "Publish subject") {
+	if !strings.Contains(str, "Received message from publisher") || !strings.Contains(str, "Publish subject") {
 		t.Fatalf("Expected tracing to include debug and trace, got %v", str)
 	}
 }
@@ -459,7 +462,7 @@ func TestDontEmbedNATSMultipleURLs(t *testing.T) {
 	}
 	for _, url := range notWorkingURLs {
 		sOpts.NATSServerURL = url
-		s, err := RunServerWithOpts(sOpts, &nOpts)
+		s, err := RunServerWithOpts(sOpts, nil)
 		if s != nil || err == nil {
 			s.Shutdown()
 			t.Fatalf("Expected streaming server to fail to start with url=%v", url)
@@ -995,3 +998,65 @@ func TestRunServerWithCrypto(t *testing.T) {
 		t.Fatalf("Did not receive message")
 	}
 }
+
+func TestDontExposeUserPassword(t *testing.T) {
+	ns := natsdTest.RunDefaultServer()
+	defer shutdownRestartedNATSServerOnTestExit(&ns)
+
+	l := &captureNoticesLogger{}
+	sOpts := GetDefaultOptions()
+	sOpts.CustomLogger = l
+	sOpts.NATSServerURL = "nats://localhost:4222"
+	nOpts := natsdTest.DefaultTestOptions
+	nOpts.Username = "ivan"
+	nOpts.Password = "password"
+	s := runServerWithOpts(t, sOpts, &nOpts)
+	defer s.Shutdown()
+
+	// Restart the NATS server that should cause streaming
+	// to reconnect and log the connected url.
+	ns.Shutdown()
+	ns = natsdTest.RunDefaultServer()
+
+	collectNotice := func(t *testing.T) string {
+		t.Helper()
+		var msg string
+		waitFor(t, 2*time.Second, 15*time.Millisecond, func() error {
+			l.Lock()
+			for _, n := range l.notices {
+				if strings.Contains(n, "reconnected to NATS Server at") {
+					msg = n
+					l.Unlock()
+					return nil
+				}
+			}
+			l.Unlock()
+			return fmt.Errorf("did not get proper notices")
+		})
+		return msg
+	}
+	// Now make sure that this string does not contain our user/password
+	msg := collectNotice(t)
+	if strings.Contains(msg, "ivan:password@") {
+		t.Fatalf("Password exposed in url: %v", msg)
+	}
+
+	// Now try again but with nats_server_url that contains user/pass
+	s.Shutdown()
+	l.Lock()
+	l.notices = l.notices[:0]
+	l.Unlock()
+	sOpts.NATSServerURL = "nats://ivan:password@localhost:4222"
+	s = runServerWithOpts(t, sOpts, nil)
+	defer s.Shutdown()
+
+	// Restart the NATS server that should cause streaming
+	// to reconnect and log the connected url.
+	ns.Shutdown()
+	ns = natsdTest.RunDefaultServer()
+
+	msg = collectNotice(t)
+	if !strings.Contains(msg, "nats://[REDACTED]@localhost:") {
+		t.Fatalf("Password exposed in url: %v", msg)
+	}
+}