diff --git a/docs/usage.md b/docs/usage.md index fd38dae0..238eb2e1 100644 --- a/docs/usage.md +++ b/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 diff --git a/integration_test/httpdriver/http_driver.go b/integration_test/httpdriver/http_driver.go index ef19aa9c..d601bc62 100644 --- a/integration_test/httpdriver/http_driver.go +++ b/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) +} diff --git a/pkg/cmd/authentication.go b/pkg/cmd/authentication.go index a67f13b8..333be198 100644 --- a/pkg/cmd/authentication.go +++ b/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, diff --git a/pkg/cmd/authentication_test.go b/pkg/cmd/authentication_test.go index 1f9f8479..4bc19a5f 100644 --- a/pkg/cmd/authentication_test.go +++ b/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", diff --git a/pkg/infrastructure/browser/browser.go b/pkg/infrastructure/browser/browser.go index 93cf1846..c0df43bf 100644 --- a/pkg/infrastructure/browser/browser.go +++ b/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() +} diff --git a/pkg/infrastructure/browser/mock_browser/mock_browser.go b/pkg/infrastructure/browser/mock_browser/mock_browser.go index 87ca4707..f82073de 100644 --- a/pkg/infrastructure/browser/mock_browser/mock_browser.go +++ b/pkg/infrastructure/browser/mock_browser/mock_browser.go @@ -5,6 +5,7 @@ package mock_browser import ( + context "context" gomock "github.com/golang/mock/gomock" reflect "reflect" ) @@ -45,3 +46,17 @@ func (mr *MockInterfaceMockRecorder) Open(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Open", reflect.TypeOf((*MockInterface)(nil).Open), arg0) } + +// OpenCommand mocks base method +func (m *MockInterface) OpenCommand(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "OpenCommand", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// OpenCommand indicates an expected call of OpenCommand +func (mr *MockInterfaceMockRecorder) OpenCommand(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OpenCommand", reflect.TypeOf((*MockInterface)(nil).OpenCommand), arg0, arg1, arg2) +} diff --git a/pkg/usecases/authentication/authcode/browser.go b/pkg/usecases/authentication/authcode/browser.go index 29f71498..75fd2fe6 100644 --- a/pkg/usecases/authentication/authcode/browser.go +++ b/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) + } +} diff --git a/pkg/usecases/authentication/authcode/browser_test.go b/pkg/usecases/authentication/authcode/browser_test.go index 4f0d8233..bed43205 100644 --- a/pkg/usecases/authentication/authcode/browser_test.go +++ b/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) + } + }) } diff --git a/pkg/usecases/setup/stage2.go b/pkg/usecases/setup/stage2.go index 203cdef1..30d01b63 100644 --- a/pkg/usecases/setup/stage2.go +++ b/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. diff --git a/system_test/login/Makefile b/system_test/login/Makefile index fc87dfe0..1af198e7 100644 --- a/system_test/login/Makefile +++ b/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