From 91a1fec5cf7f42b41634659efae00013827f54b4 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 8 Jul 2021 22:26:21 -0500 Subject: [PATCH] Add hidden `--skip-listen` flag for `pinniped login oidc`. This flag is (for now) meant only to facilitate end-to-end testing, allowing us to force the "manual" login flow. If it ends up being useful we can un-hide it, but this seemed like the safest option to start with. There is also a corresponding `--oidc-skip-listen` on the `pinniped get kubeconfig` command. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/kubeconfig.go | 8 ++++++++ cmd/pinniped/cmd/kubeconfig_test.go | 2 ++ cmd/pinniped/cmd/login_oidc.go | 9 +++++++++ cmd/pinniped/cmd/login_oidc_test.go | 3 ++- pkg/oidcclient/login.go | 8 ++++++++ pkg/oidcclient/login_test.go | 5 ++--- 6 files changed, 31 insertions(+), 4 deletions(-) diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 32d724a2..54042b34 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -61,6 +61,7 @@ type getKubeconfigOIDCParams struct { listenPort uint16 scopes []string skipBrowser bool + skipListen bool sessionCachePath string debugSessionCache bool caBundle caBundleFlag @@ -146,6 +147,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { f.Uint16Var(&flags.oidc.listenPort, "oidc-listen-port", 0, "TCP port for localhost listener (authorization code flow only)") f.StringSliceVar(&flags.oidc.scopes, "oidc-scopes", []string{oidc.ScopeOfflineAccess, oidc.ScopeOpenID, "pinniped:request-audience"}, "OpenID Connect scopes to request during login") f.BoolVar(&flags.oidc.skipBrowser, "oidc-skip-browser", false, "During OpenID Connect login, skip opening the browser (just print the URL)") + f.BoolVar(&flags.oidc.skipListen, "oidc-skip-listen", false, "During OpenID Connect login, skip starting a localhost callback listener (manual copy/paste flow only)") f.StringVar(&flags.oidc.sessionCachePath, "oidc-session-cache", "", "Path to OpenID Connect session cache file") f.Var(&flags.oidc.caBundle, "oidc-ca-bundle", "Path to TLS certificate authority bundle (PEM format, optional, can be repeated)") f.BoolVar(&flags.oidc.debugSessionCache, "oidc-debug-session-cache", false, "Print debug logs related to the OpenID Connect session cache") @@ -161,6 +163,9 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { f.StringVar(&flags.credentialCachePath, "credential-cache", "", "Path to cluster-specific credentials cache") mustMarkHidden(cmd, "oidc-debug-session-cache") + // --oidc-skip-listen is mainly needed for testing. We'll leave it hidden until we have a non-testing use case. + mustMarkHidden(cmd, "oidc-skip-listen") + mustMarkDeprecated(cmd, "concierge-namespace", "not needed anymore") mustMarkHidden(cmd, "concierge-namespace") @@ -317,6 +322,9 @@ func newExecConfig(deps kubeconfigDeps, flags getKubeconfigParams) (*clientcmdap if flags.oidc.skipBrowser { execConfig.Args = append(execConfig.Args, "--skip-browser") } + if flags.oidc.skipListen { + execConfig.Args = append(execConfig.Args, "--skip-listen") + } if flags.oidc.listenPort != 0 { execConfig.Args = append(execConfig.Args, "--listen-port="+strconv.Itoa(int(flags.oidc.listenPort))) } diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 2853b0e4..cb9f7b83 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -1352,6 +1352,7 @@ func TestGetKubeconfig(t *testing.T) { "--concierge-ca-bundle", testConciergeCABundlePath, "--oidc-issuer", issuerURL, "--oidc-skip-browser", + "--oidc-skip-listen", "--oidc-listen-port", "1234", "--oidc-ca-bundle", f.Name(), "--oidc-session-cache", "/path/to/cache/dir/sessions.yaml", @@ -1405,6 +1406,7 @@ func TestGetKubeconfig(t *testing.T) { - --client-id=pinniped-cli - --scopes=offline_access,openid,pinniped:request-audience - --skip-browser + - --skip-listen - --listen-port=1234 - --ca-bundle-data=%s - --session-cache=/path/to/cache/dir/sessions.yaml diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 1d6e02b0..9141d26a 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -59,6 +59,7 @@ type oidcLoginFlags struct { listenPort uint16 scopes []string skipBrowser bool + skipListen bool sessionCachePath string caBundlePaths []string caBundleData []string @@ -91,6 +92,7 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command { cmd.Flags().Uint16Var(&flags.listenPort, "listen-port", 0, "TCP port for localhost listener (authorization code flow only)") cmd.Flags().StringSliceVar(&flags.scopes, "scopes", []string{oidc.ScopeOfflineAccess, oidc.ScopeOpenID, "pinniped:request-audience"}, "OIDC scopes to request during login") cmd.Flags().BoolVar(&flags.skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL)") + cmd.Flags().BoolVar(&flags.skipListen, "skip-listen", false, "Skip starting a localhost callback listener (manual copy/paste flow only)") cmd.Flags().StringVar(&flags.sessionCachePath, "session-cache", filepath.Join(mustGetConfigDir(), "sessions.yaml"), "Path to session cache file") cmd.Flags().StringSliceVar(&flags.caBundlePaths, "ca-bundle", nil, "Path to TLS certificate authority bundle (PEM format, optional, can be repeated)") cmd.Flags().StringSliceVar(&flags.caBundleData, "ca-bundle-data", nil, "Base64 encoded TLS certificate authority bundle (base64 encoded PEM format, optional, can be repeated)") @@ -107,6 +109,8 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command { cmd.Flags().StringVar(&flags.upstreamIdentityProviderName, "upstream-identity-provider-name", "", "The name of the upstream identity provider used during login with a Supervisor") cmd.Flags().StringVar(&flags.upstreamIdentityProviderType, "upstream-identity-provider-type", "oidc", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap')") + // --skip-listen is mainly needed for testing. We'll leave it hidden until we have a non-testing use case. + mustMarkHidden(cmd, "skip-listen") mustMarkHidden(cmd, "debug-session-cache") mustMarkRequired(cmd, "issuer") cmd.RunE = func(cmd *cobra.Command, args []string) error { return runOIDCLogin(cmd, deps, flags) } @@ -187,6 +191,11 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin opts = append(opts, oidcclient.WithSkipBrowserOpen()) } + // --skip-listen skips starting the localhost callback listener. + if flags.skipListen { + opts = append(opts, oidcclient.WithSkipListen()) + } + if len(flags.caBundlePaths) > 0 || len(flags.caBundleData) > 0 { client, err := makeClient(flags.caBundlePaths, flags.caBundleData) if err != nil { diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index b31d7021..055dcec6 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -226,6 +226,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--client-id", "test-client-id", "--issuer", "test-issuer", "--skip-browser", + "--skip-listen", "--listen-port", "1234", "--debug-session-cache", "--request-audience", "cluster-1234", @@ -242,7 +243,7 @@ func TestLoginOIDCCommand(t *testing.T) { "--upstream-identity-provider-type", "ldap", }, env: map[string]string{"PINNIPED_DEBUG": "true"}, - wantOptionsCount: 10, + wantOptionsCount: 11, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{},"status":{"token":"exchanged-token"}}` + "\n", wantLogs: []string{ "\"level\"=0 \"msg\"=\"Pinniped login: Performing OIDC login\" \"client id\"=\"test-client-id\" \"issuer\"=\"test-issuer\"", diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 7bfec416..36b688e2 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -180,6 +180,14 @@ func WithSkipBrowserOpen() Option { } } +// WithSkipListen causes the login skip starting the localhost listener, forcing the manual copy/paste login flow. +func WithSkipListen() Option { + return func(h *handlerState) error { + h.listen = func(string, string) (net.Listener, error) { return nil, nil } + return nil + } +} + // SessionCacheKey contains the data used to select a valid session cache entry. type SessionCacheKey struct { Issuer string `json:"issuer"` diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index 87587ecf..358becd3 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -529,10 +529,10 @@ func TestLogin(t *testing.T) { // nolint:gocyclo wantErr: "login failed: must have either a localhost listener or stdin must be a TTY", }, { - name: "listen failure and manual prompt fails", + name: "listening disabled and manual prompt fails", opt: func(t *testing.T) Option { return func(h *handlerState) error { - h.listen = func(string, string) (net.Listener, error) { return nil, fmt.Errorf("some listen error") } + require.NoError(t, WithSkipListen()(h)) h.isTTY = func(fd int) bool { return true } h.openURL = func(authorizeURL string) error { parsed, err := url.Parse(authorizeURL) @@ -550,7 +550,6 @@ func TestLogin(t *testing.T) { // nolint:gocyclo issuer: formPostSuccessServer.URL, wantLogs: []string{ `"level"=4 "msg"="Pinniped: Performing OIDC discovery" "issuer"="` + formPostSuccessServer.URL + `"`, - `"msg"="could not open callback listener" "error"="some listen error"`, `"msg"="could not open browser" "error"="some browser open error"`, }, wantErr: "error handling callback: failed to prompt for manual authorization code: some prompt error",