Add flag to set custom browser command (#622)

* Add flag to set custom browser command

* Use --browser-command in system_test

* Add --browser-command= to setup message

docs/usage.md

@@ -11,6 +11,7 @@ Flags:
       --oidc-client-id string                           Client ID of the provider (mandatory)
       --oidc-client-secret string                       Client secret of the provider
       --oidc-extra-scope strings                        Scopes to request to the provider
+      --oidc-use-pkce                                   Force PKCE usage
       --token-cache-dir string                          Path to a directory for token cache (default "~/.kube/cache/oidc-login")
       --certificate-authority stringArray               Path to a cert file for the certificate authority
       --certificate-authority-data stringArray          Base64 encoded cert for the certificate authority
@@ -20,6 +21,7 @@ Flags:
       --grant-type string                               Authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto")
       --listen-address strings                          [authcode] Address to bind to the local server. If multiple addresses are set, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000])
       --skip-open-browser                               [authcode] Do not open the browser automatically
+      --browser-command string                          [authcode] Command to open the browser
       --authentication-timeout-sec int                  [authcode] Timeout of authentication in seconds (default 180)
       --local-server-cert string                        [authcode] Certificate path for the local server
       --local-server-key string                         [authcode] Certificate key path for the local server

integration_test/httpdriver/http_driver.go

@@ -60,6 +60,10 @@ func (c *client) Open(url string) error {
 	return nil
 }
 
+func (c *client) OpenCommand(_ context.Context, url, _ string) error {
+	return c.Open(url)
+}
+
 type zeroClient struct {
 	t *testing.T
 }
@@ -68,3 +72,7 @@ func (c *zeroClient) Open(url string) error {
 	c.t.Errorf("unexpected function call Open(%s)", url)
 	return nil
 }
+
+func (c *zeroClient) OpenCommand(_ context.Context, url, _ string) error {
+	return c.Open(url)
+}

pkg/cmd/authentication.go

@@ -17,6 +17,7 @@ type authenticationOptions struct {
 	ListenPort                 []int // deprecated
 	AuthenticationTimeoutSec   int
 	SkipOpenBrowser            bool
+	BrowserCommand             string
 	LocalServerCertFile        string
 	LocalServerKeyFile         string
 	OpenURLAfterAuthentication string
@@ -57,6 +58,7 @@ func (o *authenticationOptions) addFlags(f *pflag.FlagSet) {
 		panic(err)
 	}
 	f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "[authcode] Do not open the browser automatically")
+	f.StringVar(&o.BrowserCommand, "browser-command", "", "[authcode] Command to open the browser")
 	f.IntVar(&o.AuthenticationTimeoutSec, "authentication-timeout-sec", defaultAuthenticationTimeoutSec, "[authcode] Timeout of authentication in seconds")
 	f.StringVar(&o.LocalServerCertFile, "local-server-cert", "", "[authcode] Certificate path for the local server")
 	f.StringVar(&o.LocalServerKeyFile, "local-server-key", "", "[authcode] Certificate key path for the local server")
@@ -78,6 +80,7 @@ func (o *authenticationOptions) grantOptionSet() (s authentication.GrantOptionSe
 		s.AuthCodeBrowserOption = &authcode.BrowserOption{
 			BindAddress:                o.determineListenAddress(),
 			SkipOpenBrowser:            o.SkipOpenBrowser,
+			BrowserCommand:             o.BrowserCommand,
 			AuthenticationTimeout:      time.Duration(o.AuthenticationTimeoutSec) * time.Second,
 			LocalServerCertFile:        o.LocalServerCertFile,
 			LocalServerKeyFile:         o.LocalServerKeyFile,

pkg/cmd/authentication_test.go

@@ -31,6 +31,7 @@ func Test_authenticationOptions_grantOptionSet(t *testing.T) {
 				"--listen-address", "127.0.0.1:10080",
 				"--listen-address", "127.0.0.1:20080",
 				"--skip-open-browser",
+				"--browser-command", "firefox",
 				"--authentication-timeout-sec", "10",
 				"--local-server-cert", "/path/to/local-server-cert",
 				"--local-server-key", "/path/to/local-server-key",
@@ -45,6 +46,7 @@ func Test_authenticationOptions_grantOptionSet(t *testing.T) {
 				AuthCodeBrowserOption: &authcode.BrowserOption{
 					BindAddress:                []string{"127.0.0.1:10080", "127.0.0.1:20080"},
 					SkipOpenBrowser:            true,
+					BrowserCommand:             "firefox",
 					AuthenticationTimeout:      10 * time.Second,
 					LocalServerCertFile:        "/path/to/local-server-cert",
 					LocalServerKeyFile:         "/path/to/local-server-key",

pkg/infrastructure/browser/browser.go

@@ -1,7 +1,9 @@
 package browser
 
 import (
+	"context"
 	"os"
+	"os/exec"
 
 	"github.com/google/wire"
 	"github.com/pkg/browser"
@@ -23,6 +25,7 @@ var Set = wire.NewSet(
 
 type Interface interface {
 	Open(url string) error
+	OpenCommand(ctx context.Context, url, command string) error
 }
 
 type Browser struct{}
@@ -31,3 +34,11 @@ type Browser struct{}
 func (*Browser) Open(url string) error {
 	return browser.OpenURL(url)
 }
+
+// OpenCommand opens the browser using the command.
+func (*Browser) OpenCommand(ctx context.Context, url, command string) error {
+	c := exec.CommandContext(ctx, command, url)
+	c.Stdout = os.Stderr // see above
+	c.Stderr = os.Stderr
+	return c.Run()
+}

pkg/usecases/authentication/authcode/browser.go

@@ -15,6 +15,7 @@ import (
 
 type BrowserOption struct {
 	SkipOpenBrowser            bool
+	BrowserCommand             string
 	BindAddress                []string
 	AuthenticationTimeout      time.Duration
 	OpenURLAfterAuthentication string
@@ -71,17 +72,7 @@ func (u *Browser) Do(ctx context.Context, o *BrowserOption, oidcClient client.In
 			if !ok {
 				return nil
 			}
-			if o.SkipOpenBrowser {
-				u.Logger.Printf("Please visit the following URL in your browser: %s", url)
-				return nil
-			}
-			u.Logger.V(1).Infof("opening %s in the browser", url)
-			if err := u.Browser.Open(url); err != nil {
-				u.Logger.Printf(`error: could not open the browser: %s
-
-Please visit the following URL in your browser manually: %s`, err, url)
-				return nil
-			}
+			u.openURL(ctx, o, url)
 			return nil
 		case <-ctx.Done():
 			return fmt.Errorf("context cancelled while waiting for the local server: %w", ctx.Err())
@@ -103,3 +94,25 @@ Please visit the following URL in your browser manually: %s`, err, url)
 	u.Logger.V(1).Infof("finished the authorization code flow via the browser")
 	return out, nil
 }
+
+func (u *Browser) openURL(ctx context.Context, o *BrowserOption, url string) {
+	if o.SkipOpenBrowser {
+		u.Logger.Printf("Please visit the following URL in your browser: %s", url)
+		return
+	}
+
+	u.Logger.V(1).Infof("opening %s in the browser", url)
+	if o.BrowserCommand != "" {
+		if err := u.Browser.OpenCommand(ctx, url, o.BrowserCommand); err != nil {
+			u.Logger.Printf(`error: could not open the browser: %s
+
+Please visit the following URL in your browser manually: %s`, err, url)
+		}
+		return
+	}
+	if err := u.Browser.Open(url); err != nil {
+		u.Logger.Printf(`error: could not open the browser: %s
+
+Please visit the following URL in your browser manually: %s`, err, url)
+	}
+}

pkg/usecases/authentication/authcode/browser_test.go

@@ -116,4 +116,45 @@ func TestBrowser_Do(t *testing.T) {
 			t.Errorf("mismatch (-want +got):\n%s", diff)
 		}
 	})
+
+	t.Run("OpenBrowserCommand", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		ctx, cancel := context.WithTimeout(context.TODO(), timeout)
+		defer cancel()
+		o := &BrowserOption{
+			BindAddress:           []string{"127.0.0.1:8000"},
+			BrowserCommand:        "firefox",
+			AuthenticationTimeout: 10 * time.Second,
+		}
+		mockClient := mock_client.NewMockInterface(ctrl)
+		mockClient.EXPECT().SupportedPKCEMethods()
+		mockClient.EXPECT().
+			GetTokenByAuthCode(gomock.Any(), gomock.Any(), gomock.Any()).
+			Do(func(_ context.Context, _ client.GetTokenByAuthCodeInput, readyChan chan<- string) {
+				readyChan <- "LOCAL_SERVER_URL"
+			}).
+			Return(&oidc.TokenSet{
+				IDToken:      "YOUR_ID_TOKEN",
+				RefreshToken: "YOUR_REFRESH_TOKEN",
+			}, nil)
+		mockBrowser := mock_browser.NewMockInterface(ctrl)
+		mockBrowser.EXPECT().
+			OpenCommand(gomock.Any(), "LOCAL_SERVER_URL", "firefox")
+		u := Browser{
+			Logger:  logger.New(t),
+			Browser: mockBrowser,
+		}
+		got, err := u.Do(ctx, o, mockClient)
+		if err != nil {
+			t.Errorf("Do returned error: %+v", err)
+		}
+		want := &oidc.TokenSet{
+			IDToken:      "YOUR_ID_TOKEN",
+			RefreshToken: "YOUR_REFRESH_TOKEN",
+		}
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Errorf("mismatch (-want +got):\n%s", diff)
+		}
+	})
 }

pkg/usecases/setup/stage2.go

@@ -142,6 +142,9 @@ func makeCredentialPluginArgs(in Stage2Input) []string {
 		if in.GrantOptionSet.AuthCodeBrowserOption.SkipOpenBrowser {
 			args = append(args, "--skip-open-browser")
 		}
+		if in.GrantOptionSet.AuthCodeBrowserOption.BrowserCommand != "" {
+			args = append(args, "--browser-command="+in.GrantOptionSet.AuthCodeBrowserOption.BrowserCommand)
+		}
 		if in.GrantOptionSet.AuthCodeBrowserOption.LocalServerCertFile != "" {
 			// Resolve the absolute path for the cert files so the user doesn't have to know
 			// to use one when running setup.

system_test/login/Makefile

@@ -7,10 +7,6 @@ export PATH
 KUBECONFIG := ../cluster/kubeconfig.yaml
 export KUBECONFIG
 
-# run the login script instead of opening chrome
-BROWSER := $(BIN_DIR)/chromelogin
-export BROWSER
-
 .PHONY: test
 test: build
 	# see the setup instruction
@@ -19,7 +15,8 @@ test: build
 		--oidc-client-id=YOUR_CLIENT_ID \
 		--oidc-client-secret=YOUR_CLIENT_SECRET \
 		--oidc-extra-scope=email \
-		--certificate-authority=$(CERT_DIR)/ca.crt
+		--certificate-authority=$(CERT_DIR)/ca.crt \
+		--browser-command=$(BIN_DIR)/chromelogin
 	# set up the kubeconfig
 	kubectl config set-credentials oidc \
 		--exec-api-version=client.authentication.k8s.io/v1beta1 \
@@ -30,7 +27,8 @@ test: build
 		--exec-arg=--oidc-client-id=YOUR_CLIENT_ID \
 		--exec-arg=--oidc-client-secret=YOUR_CLIENT_SECRET \
 		--exec-arg=--oidc-extra-scope=email \
-		--exec-arg=--certificate-authority=$(CERT_DIR)/ca.crt
+		--exec-arg=--certificate-authority=$(CERT_DIR)/ca.crt \
+		--exec-arg=--browser-command=$(BIN_DIR)/chromelogin
 	# make sure we can access the cluster
 	kubectl --user=oidc cluster-info
 	# switch the current context