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 <moyerm@vmware.com>
This commit is contained in:
Matt Moyer 2021-07-08 22:26:21 -05:00
parent d0b37a7c90
commit 91a1fec5cf
No known key found for this signature in database
GPG Key ID: EAE88AD172C5AE2D
6 changed files with 31 additions and 4 deletions

View File

@ -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)))
}

View File

@ -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

View File

@ -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 {

View File

@ -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\"",

View File

@ -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"`

View File

@ -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",