Switch login flags to use --concierge-mode flag instead of boolean flag.

The login commands now expect either `--concierge-mode ImpersonationProxy` or `--concierge-mode TokenCredentialRequestAPI` (the default).

This is partly a style choice, but I also think it helps in case we need to add a third major mode of operation at some point.

I also cleaned up some other minor style items in the help text.

Signed-off-by: Matt Moyer <moyerm@vmware.com>

cmd/pinniped/cmd/flag_types.go

@@ -0,0 +1,46 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package cmd
+
+import (
+	"flag"
+	"fmt"
+	"strings"
+)
+
+// conciergeMode represents the method by which we should connect to the Concierge on a cluster during login.
+// this is meant to be a valid flag.Value implementation.
+type conciergeMode int
+
+var _ flag.Value = new(conciergeMode)
+
+const (
+	modeTokenCredentialRequestAPI conciergeMode = iota
+	modeImpersonationProxy        conciergeMode = iota
+)
+
+func (c *conciergeMode) String() string {
+	switch *c {
+	case modeImpersonationProxy:
+		return "ImpersonationProxy"
+	default:
+		return "TokenCredentialRequestAPI"
+	}
+}
+
+func (c *conciergeMode) Set(s string) error {
+	if strings.EqualFold(s, "TokenCredentialRequestAPI") {
+		*c = modeTokenCredentialRequestAPI
+		return nil
+	}
+	if strings.EqualFold(s, "ImpersonationProxy") {
+		*c = modeImpersonationProxy
+		return nil
+	}
+	return fmt.Errorf("invalid mode %q, valid modes are TokenCredentialRequestAPI and ImpersonationProxy", s)
+}
+
+func (c *conciergeMode) Type() string {
+	return "mode"
+}

cmd/pinniped/cmd/flag_types_test.go

@@ -0,0 +1,30 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestConciergeModeFlag(t *testing.T) {
+	var m conciergeMode
+	require.Equal(t, "mode", m.Type())
+	require.Equal(t, modeTokenCredentialRequestAPI, m)
+	require.EqualError(t, m.Set("foo"), `invalid mode "foo", valid modes are TokenCredentialRequestAPI and ImpersonationProxy`)
+
+	require.NoError(t, m.Set("TokenCredentialRequestAPI"))
+	require.Equal(t, modeTokenCredentialRequestAPI, m)
+	require.Equal(t, "TokenCredentialRequestAPI", m.String())
+
+	require.NoError(t, m.Set("tokencredentialrequestapi"))
+	require.Equal(t, modeTokenCredentialRequestAPI, m)
+	require.Equal(t, "TokenCredentialRequestAPI", m.String())
+
+	require.NoError(t, m.Set("ImpersonationProxy"))
+	require.Equal(t, modeImpersonationProxy, m)
+	require.Equal(t, "ImpersonationProxy", m.String())
+
+	require.NoError(t, m.Set("impersonationproxy"))
+	require.Equal(t, modeImpersonationProxy, m)
+	require.Equal(t, "ImpersonationProxy", m.String())
+}

cmd/pinniped/cmd/login_oidc.go

@@ -71,7 +71,7 @@ type oidcLoginFlags struct {
 	conciergeEndpoint          string
 	conciergeCABundle          string
 	conciergeAPIGroupSuffix    string
-	useImpersonationProxy      bool
+	conciergeMode              conciergeMode
 }
 
 func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
@@ -92,17 +92,17 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
 	cmd.Flags().BoolVar(&flags.skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL)")
 	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 endcoded TLS certificate authority bundle (base64 encoded 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)")
 	cmd.Flags().BoolVar(&flags.debugSessionCache, "debug-session-cache", false, "Print debug logs related to the session cache")
 	cmd.Flags().StringVar(&flags.requestAudience, "request-audience", "", "Request a token with an alternate audience using RFC8693 token exchange")
-	cmd.Flags().BoolVar(&flags.conciergeEnabled, "enable-concierge", false, "Exchange the OIDC ID token with the Pinniped concierge during login")
+	cmd.Flags().BoolVar(&flags.conciergeEnabled, "enable-concierge", false, "Use the Concierge to login")
-	cmd.Flags().StringVar(&conciergeNamespace, "concierge-namespace", "pinniped-concierge", "Namespace in which the concierge was installed")
+	cmd.Flags().StringVar(&conciergeNamespace, "concierge-namespace", "pinniped-concierge", "Namespace in which the Concierge was installed")
 	cmd.Flags().StringVar(&flags.conciergeAuthenticatorType, "concierge-authenticator-type", "", "Concierge authenticator type (e.g., 'webhook', 'jwt')")
 	cmd.Flags().StringVar(&flags.conciergeAuthenticatorName, "concierge-authenticator-name", "", "Concierge authenticator name")
-	cmd.Flags().StringVar(&flags.conciergeEndpoint, "concierge-endpoint", "", "API base for the Pinniped concierge endpoint")
+	cmd.Flags().StringVar(&flags.conciergeEndpoint, "concierge-endpoint", "", "API base for the Concierge endpoint")
-	cmd.Flags().StringVar(&flags.conciergeCABundle, "concierge-ca-bundle-data", "", "CA bundle to use when connecting to the concierge")
+	cmd.Flags().StringVar(&flags.conciergeCABundle, "concierge-ca-bundle-data", "", "CA bundle to use when connecting to the Concierge")
 	cmd.Flags().StringVar(&flags.conciergeAPIGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
-	cmd.Flags().BoolVar(&flags.useImpersonationProxy, "concierge-use-impersonation-proxy", false, "Whether the concierge cluster uses an impersonation proxy")
+	cmd.Flags().Var(&flags.conciergeMode, "concierge-mode", "Concierge mode of operation")
 
 	mustMarkHidden(cmd, "debug-session-cache")
 	mustMarkRequired(cmd, "issuer")
@@ -179,18 +179,27 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
 	}
 	cred := tokenCredential(token)
 
-	// If the concierge was configured, exchange the credential for a separate short-lived, cluster-specific credential.
+	// If there is no concierge configuration, return the credential directly.
+	if concierge == nil {
+		return json.NewEncoder(cmd.OutOrStdout()).Encode(cred)
+	}
+
+	// If the concierge was configured, we need to do extra steps to make the credential usable.
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
-	// do a credential exchange request, unless impersonation proxy is configured
+	// The exact behavior depends on in which mode the Concierge is operating.
-	if concierge != nil && !flags.useImpersonationProxy {
+	switch flags.conciergeMode {
-		cred, err = deps.exchangeToken(ctx, concierge, token.IDToken.Token)
+
+	case modeTokenCredentialRequestAPI:
+		// do a credential exchange request
+		cred, err := deps.exchangeToken(ctx, concierge, token.IDToken.Token)
 		if err != nil {
 			return fmt.Errorf("could not complete concierge credential exchange: %w", err)
 		}
-	}
+		return json.NewEncoder(cmd.OutOrStdout()).Encode(cred)
-	if concierge != nil && flags.useImpersonationProxy {
+
+	case modeImpersonationProxy:
 		// Put the token into a TokenCredentialRequest
 		// put the TokenCredentialRequest in an ExecCredential
 		req, err := execCredentialForImpersonationProxy(token.IDToken.Token, flags.conciergeAuthenticatorType, flags.conciergeAuthenticatorName, &token.IDToken.Expiry)
@@ -198,9 +207,12 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
 			return err
 		}
 		return json.NewEncoder(cmd.OutOrStdout()).Encode(req)
+
+	default:
+		return fmt.Errorf("unsupported Concierge mode %q", flags.conciergeMode.String())
 	}
-	return json.NewEncoder(cmd.OutOrStdout()).Encode(cred)
 }
+
 func makeClient(caBundlePaths []string, caBundleData []string) (*http.Client, error) {
 	pool := x509.NewCertPool()
 	for _, p := range caBundlePaths {

cmd/pinniped/cmd/login_oidc_test.go

@@ -15,15 +15,13 @@ import (
 	"testing"
 	"time"
 
-	corev1 "k8s.io/api/core/v1"
-
-	authenticationv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
-	loginv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1"
-
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientauthv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1"
 
+	authenticationv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
+	loginv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1"
 	"go.pinniped.dev/internal/certauthority"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/testutil"
@@ -64,15 +62,15 @@ func TestLoginOIDCCommand(t *testing.T) {
 
 				Flags:
 				      --ca-bundle strings                     Path to TLS certificate authority bundle (PEM format, optional, can be repeated)
-				      --ca-bundle-data strings                Base64 endcoded TLS certificate authority bundle (base64 encoded PEM format, optional, can be repeated)
+				      --ca-bundle-data strings                Base64 encoded TLS certificate authority bundle (base64 encoded PEM format, optional, can be repeated)
 				      --client-id string                      OpenID Connect client ID (default "pinniped-cli")
 				      --concierge-api-group-suffix string     Concierge API group suffix (default "pinniped.dev")
 				      --concierge-authenticator-name string   Concierge authenticator name
 				      --concierge-authenticator-type string   Concierge authenticator type (e.g., 'webhook', 'jwt')
-				      --concierge-ca-bundle-data string       CA bundle to use when connecting to the concierge
+				      --concierge-ca-bundle-data string       CA bundle to use when connecting to the Concierge
-				      --concierge-endpoint string             API base for the Pinniped concierge endpoint
+				      --concierge-endpoint string             API base for the Concierge endpoint
-				      --concierge-use-impersonation-proxy     Whether the concierge cluster uses an impersonation proxy
+				      --concierge-mode mode                   Concierge mode of operation (default TokenCredentialRequestAPI)
-				      --enable-concierge                      Exchange the OIDC ID token with the Pinniped concierge during login
+				      --enable-concierge                      Use the Concierge to login
 				  -h, --help                                  help for oidc
 				      --issuer string                         OpenID Connect issuer URL
 				      --listen-port uint16                    TCP port for localhost listener (authorization code flow only)
@@ -207,7 +205,7 @@ func TestLoginOIDCCommand(t *testing.T) {
 				"--client-id", "test-client-id",
 				"--issuer", "test-issuer",
 				"--enable-concierge",
-				"--concierge-use-impersonation-proxy",
+				"--concierge-mode", "ImpersonationProxy",
 				"--concierge-authenticator-type", "webhook",
 				"--concierge-authenticator-name", "test-authenticator",
 				"--concierge-endpoint", "https://127.0.0.1:1234/",

cmd/pinniped/cmd/login_static.go

@@ -47,7 +47,7 @@ type staticLoginParams struct {
 	conciergeEndpoint          string
 	conciergeCABundle          string
 	conciergeAPIGroupSuffix    string
-	useImpersonationProxy      bool
+	conciergeMode              conciergeMode
 }
 
 func staticLoginCommand(deps staticLoginDeps) *cobra.Command {
@@ -63,14 +63,14 @@ func staticLoginCommand(deps staticLoginDeps) *cobra.Command {
 	)
 	cmd.Flags().StringVar(&flags.staticToken, "token", "", "Static token to present during login")
 	cmd.Flags().StringVar(&flags.staticTokenEnvName, "token-env", "", "Environment variable containing a static token")
-	cmd.Flags().BoolVar(&flags.conciergeEnabled, "enable-concierge", false, "Exchange the token with the Pinniped concierge during login")
+	cmd.Flags().BoolVar(&flags.conciergeEnabled, "enable-concierge", false, "Use the Concierge to login")
-	cmd.Flags().StringVar(&conciergeNamespace, "concierge-namespace", "pinniped-concierge", "Namespace in which the concierge was installed")
+	cmd.Flags().StringVar(&conciergeNamespace, "concierge-namespace", "pinniped-concierge", "Namespace in which the Concierge was installed")
 	cmd.Flags().StringVar(&flags.conciergeAuthenticatorType, "concierge-authenticator-type", "", "Concierge authenticator type (e.g., 'webhook', 'jwt')")
 	cmd.Flags().StringVar(&flags.conciergeAuthenticatorName, "concierge-authenticator-name", "", "Concierge authenticator name")
-	cmd.Flags().StringVar(&flags.conciergeEndpoint, "concierge-endpoint", "", "API base for the Pinniped concierge endpoint")
+	cmd.Flags().StringVar(&flags.conciergeEndpoint, "concierge-endpoint", "", "API base for the Concierge endpoint")
-	cmd.Flags().StringVar(&flags.conciergeCABundle, "concierge-ca-bundle-data", "", "CA bundle to use when connecting to the concierge")
+	cmd.Flags().StringVar(&flags.conciergeCABundle, "concierge-ca-bundle-data", "", "CA bundle to use when connecting to the Concierge")
 	cmd.Flags().StringVar(&flags.conciergeAPIGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
-	cmd.Flags().BoolVar(&flags.useImpersonationProxy, "concierge-use-impersonation-proxy", false, "Whether the concierge cluster uses an impersonation proxy")
+	cmd.Flags().Var(&flags.conciergeMode, "concierge-mode", "Concierge mode of operation")
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error { return runStaticLogin(cmd.OutOrStdout(), deps, flags) }
 
@@ -115,18 +115,26 @@ func runStaticLogin(out io.Writer, deps staticLoginDeps, flags staticLoginParams
 	}
 	cred := tokenCredential(&oidctypes.Token{IDToken: &oidctypes.IDToken{Token: token}})
 
-	// Exchange that token with the concierge, if configured.
+	// If there is no concierge configuration, return the credential directly.
-	if concierge != nil && !flags.useImpersonationProxy {
+	if concierge == nil {
+		return json.NewEncoder(out).Encode(cred)
+	}
+
+	// If the concierge is enabled, we need to do extra steps.
+	switch flags.conciergeMode {
+
+	case modeTokenCredentialRequestAPI:
+		// do a credential exchange request
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 		defer cancel()
 
-		var err error
+		cred, err := deps.exchangeToken(ctx, concierge, token)
-		cred, err = deps.exchangeToken(ctx, concierge, token)
 		if err != nil {
 			return fmt.Errorf("could not complete concierge credential exchange: %w", err)
 		}
-	}
+		return json.NewEncoder(out).Encode(cred)
-	if concierge != nil && flags.useImpersonationProxy {
+
+	case modeImpersonationProxy:
 		// Put the token into a TokenCredentialRequest
 		// put the TokenCredentialRequest in an ExecCredential
 		req, err := execCredentialForImpersonationProxy(token, flags.conciergeAuthenticatorType, flags.conciergeAuthenticatorName, nil)
@@ -134,6 +142,9 @@ func runStaticLogin(out io.Writer, deps staticLoginDeps, flags staticLoginParams
 			return err
 		}
 		return json.NewEncoder(out).Encode(req)
+
+	default:
+		return fmt.Errorf("unsupported Concierge mode %q", flags.conciergeMode.String())
 	}
-	return json.NewEncoder(out).Encode(cred)
+
 }

cmd/pinniped/cmd/login_static_test.go

@@ -54,10 +54,10 @@ func TestLoginStaticCommand(t *testing.T) {
 				      --concierge-api-group-suffix string     Concierge API group suffix (default "pinniped.dev")
 				      --concierge-authenticator-name string   Concierge authenticator name
 				      --concierge-authenticator-type string   Concierge authenticator type (e.g., 'webhook', 'jwt')
-				      --concierge-ca-bundle-data string       CA bundle to use when connecting to the concierge
+				      --concierge-ca-bundle-data string       CA bundle to use when connecting to the Concierge
-				      --concierge-endpoint string             API base for the Pinniped concierge endpoint
+				      --concierge-endpoint string             API base for the Concierge endpoint
-				      --concierge-use-impersonation-proxy     Whether the concierge cluster uses an impersonation proxy
+				      --concierge-mode mode                   Concierge mode of operation (default TokenCredentialRequestAPI)
-				      --enable-concierge                      Exchange the token with the Pinniped concierge during login
+				      --enable-concierge                      Use the Concierge to login
 				  -h, --help                                  help for static
 				      --token string                          Static token to present during login
 				      --token-env string                      Environment variable containing a static token
@@ -156,7 +156,7 @@ func TestLoginStaticCommand(t *testing.T) {
 			name: "impersonation proxy success",
 			args: []string{
 				"--enable-concierge",
-				"--concierge-use-impersonation-proxy",
+				"--concierge-mode", "ImpersonationProxy",
 				"--token", "test-token",
 				"--concierge-endpoint", "https://127.0.0.1/",
 				"--concierge-authenticator-type", "webhook",