From 2f70b05c64d853f18f58a299af8937f38b7d7ea7 Mon Sep 17 00:00:00 2001
From: Dalton <dalton@cloudflare.com>
Date: Fri, 5 Jun 2020 09:26:06 -0500
Subject: [PATCH] AUTH-2169 make access login page more generic

---
 carrier/carrier.go                |  9 ++-------
 cmd/cloudflared/access/carrier.go |  2 --
 cmd/cloudflared/main.go           |  7 ++++++-
 overwatch/app_manager.go          | 18 ++++++++++++------
 overwatch/manager_test.go         |  5 ++++-
 websocket/notice_page.go          |  2 +-
 6 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/carrier/carrier.go b/carrier/carrier.go
index 1ef5a032..6240b8e4 100644
--- a/carrier/carrier.go
+++ b/carrier/carrier.go
@@ -66,7 +66,7 @@ func StartForwarder(conn Connection, address string, shutdownC <-chan struct{},
 // StartClient will copy the data from stdin/stdout over a WebSocket connection
 // to the edge (originURL)
 func StartClient(conn Connection, stream io.ReadWriter, options *StartOptions) error {
-	return serveStream(conn, stream, options)
+	return conn.ServeStream(options, stream)
 }
 
 // Serve accepts incoming connections on the specified net.Listener.
@@ -103,12 +103,7 @@ func Serve(remoteConn Connection, listener net.Listener, shutdownC <-chan struct
 // serveConnection handles connections for the Serve() call
 func serveConnection(remoteConn Connection, c net.Conn, options *StartOptions) {
 	defer c.Close()
-	serveStream(remoteConn, c, options)
-}
-
-// serveStream will serve the data over the WebSocket stream
-func serveStream(remoteConn Connection, conn io.ReadWriter, options *StartOptions) error {
-	return remoteConn.ServeStream(options, conn)
+	remoteConn.ServeStream(options, c)
 }
 
 // IsAccessResponse checks the http Response to see if the url location
diff --git a/cmd/cloudflared/access/carrier.go b/cmd/cloudflared/access/carrier.go
index 337792ab..173ced25 100644
--- a/cmd/cloudflared/access/carrier.go
+++ b/cmd/cloudflared/access/carrier.go
@@ -18,13 +18,11 @@ import (
 func StartForwarder(forwarder config.Forwarder, shutdown <-chan struct{}, logger logger.Service) error {
 	validURLString, err := validation.ValidateUrl(forwarder.Listener)
 	if err != nil {
-		logger.Errorf("Error validating origin URL: %s", err)
 		return errors.Wrap(err, "error validating origin URL")
 	}
 
 	validURL, err := url.Parse(validURLString)
 	if err != nil {
-		logger.Errorf("Error parsing origin URL: %s", err)
 		return errors.Wrap(err, "error parsing origin URL")
 	}
 
diff --git a/cmd/cloudflared/main.go b/cmd/cloudflared/main.go
index 20fd0743..4160027d 100644
--- a/cmd/cloudflared/main.go
+++ b/cmd/cloudflared/main.go
@@ -187,7 +187,12 @@ func handleServiceMode(shutdownC chan struct{}) error {
 		return err
 	}
 
-	serviceManager := overwatch.NewAppManager(nil)
+	serviceCallback := func(t string, name string, err error) {
+		if err != nil {
+			logger.Errorf("%s service: %s encountered an error: %s", t, name, err)
+		}
+	}
+	serviceManager := overwatch.NewAppManager(serviceCallback)
 
 	appService := NewAppService(configManager, serviceManager, shutdownC, logger)
 	if err := appService.Run(); err != nil {
diff --git a/overwatch/app_manager.go b/overwatch/app_manager.go
index bd341551..9bbf6baf 100644
--- a/overwatch/app_manager.go
+++ b/overwatch/app_manager.go
@@ -1,14 +1,20 @@
 package overwatch
 
+// ServiceCallback is a service notify it's runloop finished.
+// the first parameter is the service type
+// the second parameter is the service name
+// the third parameter is an optional error if the service failed
+type ServiceCallback func(string, string, error)
+
 // AppManager is the default implementation of overwatch service management
 type AppManager struct {
-	services  map[string]Service
-	errorChan chan error
+	services map[string]Service
+	callback ServiceCallback
 }
 
 // NewAppManager creates a new overwatch manager
-func NewAppManager(errorChan chan error) Manager {
-	return &AppManager{services: make(map[string]Service), errorChan: errorChan}
+func NewAppManager(callback ServiceCallback) Manager {
+	return &AppManager{services: make(map[string]Service), callback: callback}
 }
 
 // Add takes in a new service to manage.
@@ -47,7 +53,7 @@ func (m *AppManager) Services() []Service {
 
 func (m *AppManager) serviceRun(service Service) {
 	err := service.Run()
-	if err != nil && m.errorChan != nil {
-		m.errorChan <- err
+	if m.callback != nil {
+		m.callback(service.Type(), service.Name(), err)
 	}
 }
diff --git a/overwatch/manager_test.go b/overwatch/manager_test.go
index d07e6cf0..988072f0 100644
--- a/overwatch/manager_test.go
+++ b/overwatch/manager_test.go
@@ -64,7 +64,10 @@ func TestManagerDuplicate(t *testing.T) {
 
 func TestManagerErrorChannel(t *testing.T) {
 	errChan := make(chan error)
-	m := NewAppManager(errChan)
+	serviceCallback := func(t string, name string, err error) {
+		errChan <- err
+	}
+	m := NewAppManager(serviceCallback)
 
 	err := errors.New("test error")
 	first := &mockService{serviceName: "first", serviceType: "mock", runError: err}
diff --git a/websocket/notice_page.go b/websocket/notice_page.go
index 8023f2f3..fc3fb4a7 100644
--- a/websocket/notice_page.go
+++ b/websocket/notice_page.go
@@ -253,7 +253,7 @@ func nonWebSocketRequestPage() []byte {
 					<div class="main-message">
 						<div class="title">  Success </div>
 						<div class="sub-title">
-						You are now logged in and can reach this application over SSH from your command line. 
+						You are now logged in and can reach this application. 
 							You can close this browser window.
 						</div>
 					</div>