AUTH-1188: UX Review and Changes for CLI SSH Access

cmd/cloudflared/access/carrier.go

@@ -1,9 +1,10 @@
-package tunnel
+package access
 
 import (
 	"net/url"
 
 	"github.com/cloudflare/cloudflared/carrier"
+	"github.com/cloudflare/cloudflared/cmd/cloudflared/config"
 	"github.com/cloudflare/cloudflared/validation"
 	"github.com/pkg/errors"
 	cli "gopkg.in/urfave/cli.v2"
@@ -15,13 +16,12 @@ import (
 // (which you can put Access in front of)
 func ssh(c *cli.Context) error {
 	hostname, err := validation.ValidateHostname(c.String("hostname"))
-	if err != nil {
-		logger.WithError(err).Error("Invalid hostname")
-		return errors.Wrap(err, "invalid hostname")
+	if err != nil || c.String("hostname") == "" {
+		return cli.ShowCommandHelp(c, "ssh")
 	}
 
 	if c.NArg() > 0 || c.IsSet("url") {
-		localForwarder, err := validateUrl(c)
+		localForwarder, err := config.ValidateUrl(c)
 		if err != nil {
 			logger.WithError(err).Error("Error validating origin URL")
 			return errors.Wrap(err, "error validating origin URL")

cmd/cloudflared/access/cmd.go

@@ -7,6 +7,7 @@ import (
 	"os"
 
 	"github.com/cloudflare/cloudflared/cmd/cloudflared/shell"
+	"github.com/cloudflare/cloudflared/cmd/cloudflared/token"
 	"golang.org/x/net/idna"
 
 	"github.com/cloudflare/cloudflared/log"
@@ -16,6 +17,17 @@ import (
 
 const sentryDSN = "https://56a9c9fa5c364ab28f34b14f35ea0f1b@sentry.io/189878"
 
+var (
+	logger         = log.CreateLogger()
+	shutdownC      chan struct{}
+	graceShutdownC chan struct{}
+)
+
+// Init will initialize and store vars from the main program
+func Init(s, g chan struct{}) {
+	shutdownC, graceShutdownC = s, g
+}
+
 // Flags return the global flags for Access related commands (hopefully none)
 func Flags() []cli.Flag {
 	return []cli.Flag{} // no flags yet.
@@ -61,7 +73,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:        "token",
-					Action:      token,
+					Action:      generateToken,
 					Usage:       "token -app=<url of access application>",
 					ArgsUsage:   "url of Access application",
 					Description: `The token subcommand produces a JWT which can be used to authenticate requests.`,
@@ -71,6 +83,30 @@ func Commands() []*cli.Command {
 						},
 					},
 				},
+				{
+					Name:        "ssh",
+					Action:      ssh,
+					Usage:       "",
+					ArgsUsage:   "",
+					Description: `The ssh subcommand sends data over a proxy to the Cloudflare edge.`,
+					Hidden:      true,
+					Flags: []cli.Flag{
+						&cli.StringFlag{
+							Name: "hostname",
+						},
+						&cli.StringFlag{
+							Name:   "url",
+							Hidden: true,
+						},
+					},
+				},
+				{
+					Name:        "ssh-config",
+					Action:      sshConfig,
+					Usage:       "ssh-config",
+					Description: `Prints an example configuration ~/.ssh/config`,
+					Hidden:      true,
+				},
 			},
 		},
 	}
@@ -86,7 +122,7 @@ func login(c *cli.Context) error {
 		logger.Errorf("Please provide the url of the Access application\n")
 		return err
 	}
-	token, err := FetchToken(appURL)
+	token, err := token.FetchToken(appURL)
 	if err != nil {
 		logger.Errorf("Failed to fetch token: %s\n", err)
 		return err
@@ -111,13 +147,13 @@ func curl(c *cli.Context) error {
 		return err
 	}
 
-	token, err := getTokenIfExists(appURL)
-	if err != nil || token == "" {
+	tok, err := token.GetTokenIfExists(appURL)
+	if err != nil || tok == "" {
 		if allowRequest {
 			logger.Warn("You don't have an Access token set. Please run access token <access application> to fetch one.")
 			return shell.Run("curl", cmdArgs...)
 		}
-		token, err = FetchToken(appURL)
+		tok, err = token.FetchToken(appURL)
 		if err != nil {
 			logger.Error("Failed to refresh token: ", err)
 			return err
@@ -125,31 +161,39 @@ func curl(c *cli.Context) error {
 	}
 
 	cmdArgs = append(cmdArgs, "-H")
-	cmdArgs = append(cmdArgs, fmt.Sprintf("cf-access-token: %s", token))
+	cmdArgs = append(cmdArgs, fmt.Sprintf("cf-access-token: %s", tok))
 	return shell.Run("curl", cmdArgs...)
 }
 
 // token dumps provided token to stdout
-func token(c *cli.Context) error {
+func generateToken(c *cli.Context) error {
 	raven.SetDSN(sentryDSN)
 	appURL, err := url.Parse(c.String("app"))
 	if err != nil || c.NumFlags() < 1 {
 		fmt.Fprintln(os.Stderr, "Please provide a url.")
 		return err
 	}
-	token, err := getTokenIfExists(appURL)
-	if err != nil || token == "" {
+	tok, err := token.GetTokenIfExists(appURL)
+	if err != nil || tok == "" {
 		fmt.Fprintln(os.Stderr, "Unable to find token for provided application. Please run token command to generate token.")
 		return err
 	}
 
-	if _, err := fmt.Fprint(os.Stdout, token); err != nil {
+	if _, err := fmt.Fprint(os.Stdout, tok); err != nil {
 		fmt.Fprintln(os.Stderr, "Failed to write token to stdout.")
 		return err
 	}
 	return nil
 }
 
+// sshConfig prints an example SSH config to stdout
+func sshConfig(c *cli.Context) error {
+	_, err := os.Stdout.Write([]byte(`Add this configuration block to your $HOME/.ssh/config
+		Host <your hostname>
+		    ProxyCommand cloudflared access ssh --hostname %h` + "\n"))
+	return err
+}
+
 // processURL will preprocess the string (parse to a url, convert to punycode, etc).
 func processURL(s string) (*url.URL, error) {
 	u, err := url.ParseRequestURI(s)

cmd/cloudflared/config/configuration.go

@@ -1,9 +1,11 @@
 package config
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 
+	"github.com/cloudflare/cloudflared/validation"
 	homedir "github.com/mitchellh/go-homedir"
 	"gopkg.in/urfave/cli.v2"
 	"gopkg.in/urfave/cli.v2/altsrc"
@@ -60,3 +62,16 @@ func FindDefaultConfigPath() string {
 	}
 	return ""
 }
+
+// ValidateUrl will validate url flag correctness. It can be either from --url or argument
+func ValidateUrl(c *cli.Context) (string, error) {
+	var url = c.String("url")
+	if c.NArg() > 0 {
+		if c.IsSet("url") {
+			return "", errors.New("Specified origin urls using both --url and argument. Decide which one you want, I can only support one.")
+		}
+		url = c.Args().Get(0)
+	}
+	validUrl, err := validation.ValidateUrl(url)
+	return validUrl, err
+}

cmd/cloudflared/main.go

@@ -55,6 +55,7 @@ func main() {
 	app.Commands = commands()
 
 	tunnel.Init(Version, shutdownC, graceShutdownC) // we need this to support the tunnel sub command...
+	access.Init(shutdownC, graceShutdownC)
 	runApp(app, shutdownC, graceShutdownC)
 }
 

cmd/cloudflared/token/token.go

@@ -1,4 +1,4 @@
-package access
+package token
 
 import (
 	"fmt"
@@ -21,7 +21,7 @@ var logger = log.CreateLogger()
 
 // FetchToken will either load a stored token or generate a new one
 func FetchToken(appURL *url.URL) (string, error) {
-	if token, err := getTokenIfExists(appURL); token != "" && err == nil {
+	if token, err := GetTokenIfExists(appURL); token != "" && err == nil {
 		return token, nil
 	}
 
@@ -42,8 +42,8 @@ func FetchToken(appURL *url.URL) (string, error) {
 	return string(token), nil
 }
 
-// getTokenIfExists will return the token from local storage if it exists
-func getTokenIfExists(url *url.URL) (string, error) {
+// GetTokenIfExists will return the token from local storage if it exists
+func GetTokenIfExists(url *url.URL) (string, error) {
 	path, err := generateFilePathForTokenURL(url)
 	if err != nil {
 		return "", err

cmd/cloudflared/tunnel/cmd.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"os"
 	"runtime/trace"
 	"sync"
@@ -139,21 +140,6 @@ func Commands() []*cli.Command {
 			},
 			Hidden: true,
 		},
-		{
-			Name:        "ssh",
-			Action:      ssh,
-			Usage:       `ssh -o ProxyCommand="cloudflared tunnel ssh --hostname %h" ssh.warptunnels.org`,
-			ArgsUsage:   "[origin-url]",
-			Description: `The ssh subcommand wraps sends data over a WebSocket proxy to the Cloudflare edge.`,
-			Flags: []cli.Flag{
-				&cli.StringFlag{
-					Name: "hostname",
-				},
-				&cli.StringFlag{
-					Name: "url",
-				},
-			},
-		},
 	}
 
 	var subcommands []*cli.Command
@@ -325,7 +311,11 @@ func StartServer(c *cli.Context, version string, shutdownC, graceShutdownC chan
 		c.Set("url", "https://"+helloListener.Addr().String())
 	}
 
-	if c.IsSet("ws-proxy-server") {
+	if uri, _ := url.Parse(c.String("url")); uri.Scheme == "ssh" {
+		host := uri.Host
+		if uri.Port() == "" { // default to 22
+			host = uri.Hostname() + ":22"
+		}
 		listener, err := net.Listen("tcp", "127.0.0.1:")
 		if err != nil {
 			logger.WithError(err).Error("Cannot start Websocket Proxy Server")
@@ -334,7 +324,7 @@ func StartServer(c *cli.Context, version string, shutdownC, graceShutdownC chan
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			errC <- websocket.StartProxyServer(logger, listener, c.String("remote"), shutdownC)
+			errC <- websocket.StartProxyServer(logger, listener, host, shutdownC)
 		}()
 		c.Set("url", "http://"+listener.Addr().String())
 	}
@@ -355,19 +345,19 @@ func StartServer(c *cli.Context, version string, shutdownC, graceShutdownC chan
 
 func Before(c *cli.Context) error {
 	if c.String("config") == "" {
-		logger.Warnf("Cannot determine default configuration path. No file %v in %v", config.DefaultConfigFiles, config.DefaultConfigDirs)
+		logger.Debugf("Cannot determine default configuration path. No file %v in %v", config.DefaultConfigFiles, config.DefaultConfigDirs)
 	}
 	inputSource, err := config.FindInputSourceContext(c)
 	if err != nil {
-		logger.WithError(err).Infof("Cannot load configuration from %s", c.String("config"))
+		logger.WithError(err).Debugf("Cannot load configuration from %s", c.String("config"))
 		return err
 	} else if inputSource != nil {
 		err := altsrc.ApplyInputSourceValues(c, inputSource, c.App.Flags)
 		if err != nil {
-			logger.WithError(err).Infof("Cannot apply configuration from %s", c.String("config"))
+			logger.WithError(err).Debugf("Cannot apply configuration from %s", c.String("config"))
 			return err
 		}
-		logger.Infof("Applied configuration from %s", c.String("config"))
+		logger.Debugf("Applied configuration from %s", c.String("config"))
 	}
 	return nil
 }
@@ -478,13 +468,6 @@ func tunnelFlags(shouldHide bool) []cli.Flag {
 			EnvVars: []string{"TUNNEL_URL"},
 			Hidden:  shouldHide,
 		}),
-		altsrc.NewStringFlag(&cli.StringFlag{
-			Name:    "remote",
-			Value:   "localhost:22",
-			Usage:   "Connect to the local server over tcp at `remote`.",
-			EnvVars: []string{"TUNNEL_REMOTE"},
-			Hidden:  shouldHide,
-		}),
 		altsrc.NewStringFlag(&cli.StringFlag{
 			Name:    "hostname",
 			Usage:   "Set a hostname on a Cloudflare zone to route traffic through this tunnel.",
@@ -587,13 +570,6 @@ func tunnelFlags(shouldHide bool) []cli.Flag {
 			EnvVars: []string{"TUNNEL_HELLO_WORLD"},
 			Hidden:  shouldHide,
 		}),
-		altsrc.NewBoolFlag(&cli.BoolFlag{
-			Name:    "ws-proxy-server",
-			Value:   false,
-			Usage:   "Run WS proxy Server",
-			EnvVars: []string{"TUNNEL_WS_PROXY"},
-			Hidden:  shouldHide,
-		}),
 		altsrc.NewStringFlag(&cli.StringFlag{
 			Name:    "pidfile",
 			Usage:   "Write the application's PID to this file after first successful connection.",

cmd/cloudflared/tunnel/configuration.go

@@ -64,19 +64,6 @@ func handleDeprecatedOptions(c *cli.Context) error {
 	return nil
 }
 
-// validate url. It can be either from --url or argument
-func validateUrl(c *cli.Context) (string, error) {
-	var url = c.String("url")
-	if c.NArg() > 0 {
-		if c.IsSet("url") {
-			return "", errors.New("Specified origin urls using both --url and argument. Decide which one you want, I can only support one.")
-		}
-		url = c.Args().Get(0)
-	}
-	validUrl, err := validation.ValidateUrl(url)
-	return validUrl, err
-}
-
 func logClientOptions(c *cli.Context) {
 	flags := make(map[string]interface{})
 	for _, flag := range c.LocalFlagNames() {
@@ -168,7 +155,7 @@ func prepareTunnelConfig(c *cli.Context, buildInfo *origin.BuildInfo, version st
 
 	tags = append(tags, tunnelpogs.Tag{Name: "ID", Value: clientID})
 
-	originURL, err := validateUrl(c)
+	originURL, err := config.ValidateUrl(c)
 	if err != nil {
 		logger.WithError(err).Error("Error validating origin URL")
 		return nil, errors.Wrap(err, "Error validating origin URL")