Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cmd/pinniped/cmd/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ type getKubeconfigOIDCParams struct {
clientID string
listenPort uint16
scopes []string
skipBrowser bool
skipListen bool
skipBrowser bool
skipListen bool
skipRequireIDTokenOnRefresh bool
sessionCachePath string
debugSessionCache bool
caBundle caBundleFlag
Expand Down Expand Up @@ -142,6 +143,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
f.StringSliceVar(&flags.oidc.scopes, "oidc-scopes", []string{oidcapi.ScopeOfflineAccess, oidcapi.ScopeOpenID, oidcapi.ScopeRequestAudience, oidcapi.ScopeUsername, oidcapi.ScopeGroups}, "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.BoolVar(&flags.oidc.skipRequireIDTokenOnRefresh, "oidc-skip-require-id-token-on-refresh", false, "During OpenID Connect login, skip requiring an ID token in the refresh response (use for OIDC providers that omit id_token on refresh)")
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")
Expand Down Expand Up @@ -352,6 +354,9 @@ func newExecConfig(deps kubeconfigDeps, flags getKubeconfigParams) (*clientcmdap
if flags.oidc.skipListen {
execConfig.Args = append(execConfig.Args, "--skip-listen")
}
if flags.oidc.skipRequireIDTokenOnRefresh {
execConfig.Args = append(execConfig.Args, "--skip-require-id-token-on-refresh")
}
if flags.oidc.listenPort != 0 {
execConfig.Args = append(execConfig.Args, "--listen-port="+strconv.Itoa(int(flags.oidc.listenPort)))
}
Expand Down
1 change: 1 addition & 0 deletions cmd/pinniped/cmd/kubeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func TestGetKubeconfig(t *testing.T) {
--oidc-scopes strings OpenID Connect scopes to request during login (default [offline_access,openid,pinniped:request-audience,username,groups])
--oidc-session-cache string Path to OpenID Connect session cache file
--oidc-skip-browser During OpenID Connect login, skip opening the browser (just print the URL)
--oidc-skip-require-id-token-on-refresh During OpenID Connect login, skip requiring an ID token in the refresh response (use for OIDC providers that omit id_token on refresh)
-o, --output string Output file path (default: stdout)
--pinniped-cli-path string Full path or executable name for the Pinniped CLI binary to be embedded in the resulting kubeconfig output (e.g. 'pinniped') (default: full path of the binary used to execute this command)
--skip-validation Skip final validation of the kubeconfig (default: false)
Expand Down
31 changes: 24 additions & 7 deletions cmd/pinniped/cmd/login_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type oidcLoginFlags struct {
upstreamIdentityProviderName string
upstreamIdentityProviderType string
upstreamIdentityProviderFlow string
skipRequireIDTokenOnRefresh bool
}

func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
Expand Down Expand Up @@ -152,6 +153,7 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
idpdiscoveryv1alpha1.IDPTypeGitHub,
))
cmd.Flags().StringVar(&flags.upstreamIdentityProviderFlow, "upstream-identity-provider-flow", "", fmt.Sprintf("The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. '%s', '%s')", idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode, idpdiscoveryv1alpha1.IDPFlowCLIPassword))
cmd.Flags().BoolVar(&flags.skipRequireIDTokenOnRefresh, "skip-require-id-token-on-refresh", false, "Skip requiring an ID token in the refresh response (use for OIDC providers that omit id_token on refresh)")

// --skip-listen is mainly needed for testing. We'll leave it hidden until we have a non-testing use case.
mustMarkHidden(cmd, "skip-listen")
Expand Down Expand Up @@ -241,6 +243,10 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
opts = append(opts, deps.optionsFactory.WithSkipListen())
}

if flags.skipRequireIDTokenOnRefresh {
opts = append(opts, deps.optionsFactory.WithRequireIDTokenOnRefresh(false))
}

if len(flags.caBundlePaths) > 0 || len(flags.caBundleData) > 0 {
client, err := makeClient(flags.caBundlePaths, flags.caBundleData)
if err != nil {
Expand Down Expand Up @@ -271,14 +277,17 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin
if err != nil {
return fmt.Errorf("could not complete Pinniped login: %w", err)
}
cred := tokenCredential(token.IDToken)
cred := tokenCredential(token)

// If the concierge was configured, exchange the credential for a separate short-lived, cluster-specific credential.
if concierge != nil {
pLogger.Debug("Exchanging token for cluster credential", "endpoint", flags.conciergeEndpoint, "authenticator type", flags.conciergeAuthenticatorType, "authenticator name", flags.conciergeAuthenticatorName)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

if token.IDToken == nil {
return fmt.Errorf("the OIDC provider did not return an ID token during login; cannot exchange token with Concierge (--skip-require-id-token-on-refresh is not compatible with --enable-concierge)")
}
cred, err = deps.exchangeToken(ctx, concierge, token.IDToken.Token)
if err != nil {
return fmt.Errorf("could not complete Concierge credential exchange: %w", err)
Expand Down Expand Up @@ -315,22 +324,30 @@ func makeClient(caBundlePaths []string, caBundleData []string) (*http.Client, er
return phttp.Default(pool), nil
}

func tokenCredential(idToken *oidctypes.IDToken) *clientauthv1beta1.ExecCredential {
func tokenCredential(token *oidctypes.Token) *clientauthv1beta1.ExecCredential {
cred := clientauthv1beta1.ExecCredential{
TypeMeta: metav1.TypeMeta{
Kind: "ExecCredential",
APIVersion: "client.authentication.k8s.io/v1beta1",
},
Status: &clientauthv1beta1.ExecCredentialStatus{
Token: idToken.Token,
},
Status: &clientauthv1beta1.ExecCredentialStatus{},
}
if !idToken.Expiry.IsZero() {
cred.Status.ExpirationTimestamp = &idToken.Expiry
switch {
case token.IDToken != nil:
applyTokenToStatus(cred.Status, token.IDToken.Token, token.IDToken.Expiry)
case token.AccessToken != nil:
applyTokenToStatus(cred.Status, token.AccessToken.Token, token.AccessToken.Expiry)
}
return &cred
}

func applyTokenToStatus(s *clientauthv1beta1.ExecCredentialStatus, tok string, expiry metav1.Time) {
s.Token = tok
if !expiry.IsZero() {
s.ExpirationTimestamp = &expiry
}
}

func SetLogLevel(ctx context.Context, lookupEnv func(string) (string, bool)) (plog.Logger, error) {
debug, _ := lookupEnv(debugEnvVarName)
if debug == envVarTruthyValue {
Expand Down
47 changes: 41 additions & 6 deletions cmd/pinniped/cmd/login_oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func TestLoginOIDCCommand(t *testing.T) {
--scopes strings OIDC scopes to request during login (default [offline_access,openid,pinniped:request-audience,username,groups])
--session-cache string Path to session cache file (default "` + cfgDir + `/sessions.yaml")
--skip-browser Skip opening the browser (just print the URL)
--skip-require-id-token-on-refresh Skip requiring an ID token in the refresh response (use for OIDC providers that omit id_token on refresh)
--upstream-identity-provider-flow string The type of client flow to use with the upstream identity provider during login with a Supervisor (e.g. 'browser_authcode', 'cli_password')
--upstream-identity-provider-name string The name of the upstream identity provider used during login with a Supervisor
--upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory', 'github') (default "oidc")
Expand Down Expand Up @@ -274,8 +275,8 @@ func TestLoginOIDCCommand(t *testing.T) {
wantOptionsCount: 4,
wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n",
wantLogs: []string{
nowStr + ` cmd/login_oidc.go:268 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`,
nowStr + ` cmd/login_oidc.go:288 No concierge configured, skipping token credential exchange`,
nowStr + ` cmd/login_oidc.go:274 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`,
nowStr + ` cmd/login_oidc.go:297 No concierge configured, skipping token credential exchange`,
},
},
{
Expand Down Expand Up @@ -319,10 +320,10 @@ func TestLoginOIDCCommand(t *testing.T) {
wantOptionsCount: 12,
wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"token":"exchanged-token"}}` + "\n",
wantLogs: []string{
nowStr + ` cmd/login_oidc.go:268 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`,
nowStr + ` cmd/login_oidc.go:278 Exchanging token for cluster credential {"endpoint": "https://127.0.0.1:1234/", "authenticator type": "webhook", "authenticator name": "test-authenticator"}`,
nowStr + ` cmd/login_oidc.go:286 Successfully exchanged token for cluster credential.`,
nowStr + ` cmd/login_oidc.go:293 caching cluster credential for future use.`,
nowStr + ` cmd/login_oidc.go:274 Performing OIDC login {"issuer": "test-issuer", "client id": "test-client-id"}`,
nowStr + ` cmd/login_oidc.go:284 Exchanging token for cluster credential {"endpoint": "https://127.0.0.1:1234/", "authenticator type": "webhook", "authenticator name": "test-authenticator"}`,
nowStr + ` cmd/login_oidc.go:295 Successfully exchanged token for cluster credential.`,
nowStr + ` cmd/login_oidc.go:302 caching cluster credential for future use.`,
},
},
}
Expand Down Expand Up @@ -395,3 +396,37 @@ func TestLoginOIDCCommand(t *testing.T) {
})
}
}

func TestTokenCredential(t *testing.T) {
expiry := metav1.NewTime(time.Date(3020, 10, 12, 13, 14, 15, 0, time.UTC))

t.Run("IDToken present uses IDToken", func(t *testing.T) {
token := &oidctypes.Token{
IDToken: &oidctypes.IDToken{Token: "id-tok", Expiry: expiry},
AccessToken: &oidctypes.AccessToken{Token: "access-tok", Expiry: expiry},
}
cred := tokenCredential(token)
require.Equal(t, "id-tok", cred.Status.Token)
require.Equal(t, &expiry, cred.Status.ExpirationTimestamp)
})

t.Run("IDToken nil falls back to AccessToken", func(t *testing.T) {
token := &oidctypes.Token{
IDToken: nil,
AccessToken: &oidctypes.AccessToken{Token: "access-tok", Expiry: expiry},
}
cred := tokenCredential(token)
require.Equal(t, "access-tok", cred.Status.Token)
require.Equal(t, &expiry, cred.Status.ExpirationTimestamp)
})

t.Run("IDToken nil and AccessToken with zero expiry omits ExpirationTimestamp", func(t *testing.T) {
token := &oidctypes.Token{
IDToken: nil,
AccessToken: &oidctypes.AccessToken{Token: "access-tok"},
}
cred := tokenCredential(token)
require.Equal(t, "access-tok", cred.Status.Token)
require.Nil(t, cred.Status.ExpirationTimestamp)
})
}
2 changes: 1 addition & 1 deletion cmd/pinniped/cmd/login_static.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func runStaticLogin(cmd *cobra.Command, deps staticLoginDeps, flags staticLoginP
return fmt.Errorf("--token-env variable %q is empty", flags.staticTokenEnvName)
}
}
cred := tokenCredential(&oidctypes.IDToken{Token: token})
cred := tokenCredential(&oidctypes.Token{IDToken: &oidctypes.IDToken{Token: token}})

// Look up cached credentials based on a hash of all the CLI arguments, the current token value, and the cluster info.
cacheKey := struct {
Expand Down
5 changes: 5 additions & 0 deletions cmd/pinniped/cmd/oidc_client_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type OIDCClientOptions interface {
WithRequestAudience(audience string) oidcclient.Option
WithLoginFlow(loginFlow v1alpha1.IDPFlow, flowSource string) oidcclient.Option
WithUpstreamIdentityProvider(upstreamName, upstreamType string) oidcclient.Option
WithRequireIDTokenOnRefresh(require bool) oidcclient.Option
}

// clientOptions implements OIDCClientOptions for production use.
Expand Down Expand Up @@ -82,3 +83,7 @@ func (o *clientOptions) WithLoginFlow(loginFlow v1alpha1.IDPFlow, flowSource str
func (o *clientOptions) WithUpstreamIdentityProvider(upstreamName, upstreamType string) oidcclient.Option {
return oidcclient.WithUpstreamIdentityProvider(upstreamName, upstreamType)
}

func (o *clientOptions) WithRequireIDTokenOnRefresh(require bool) oidcclient.Option {
return oidcclient.WithRequireIDTokenOnRefresh(require)
}
14 changes: 14 additions & 0 deletions internal/mocks/mockoidcclientoptions/mockoidcclientoptions.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 14 additions & 2 deletions pkg/oidcclient/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type handlerState struct {
skipPrintLoginURL bool
requestedAudience string
httpClient *http.Client
requireIDTokenOnRefresh bool

// Parameters of the localhost listener.
listenAddr string
Expand Down Expand Up @@ -291,6 +292,16 @@ func WithRequestAudience(audience string) Option {
}
}

// WithRequireIDTokenOnRefresh controls whether the refresh token flow requires the provider to return an id_token.
// Set to false for providers that omit id_token in their refresh response.
// Defaults to true for backward compatibility.
func WithRequireIDTokenOnRefresh(require bool) Option {
return func(h *handlerState) error {
h.requireIDTokenOnRefresh = require
return nil
}
}

// WithCLISendingCredentials causes the login flow to use CLI-based prompts for username and password and causes the
// call to the Issuer's authorize endpoint to be made directly (no web browser) with the username and password on custom
// HTTP headers. This is only intended to be used when the issuer is a Pinniped Supervisor and the upstream identity
Expand Down Expand Up @@ -421,7 +432,8 @@ func Login(issuer string, clientID string, opts ...Option) (*oidctypes.Token, er
ctx: context.Background(),
logger: &emptyLogger{},
callbacks: make(chan callbackResult, 2),
httpClient: phttp.Default(nil),
httpClient: phttp.Default(nil),
requireIDTokenOnRefresh: true,

// Default implementations of external dependencies (to be mocked in tests).
generateState: state.Generate,
Expand Down Expand Up @@ -1224,7 +1236,7 @@ func (h *handlerState) handleRefresh(ctx context.Context, refreshToken *oidctype

// The spec is not 100% clear about whether an ID token from the refresh flow should include a nonce, and at least
// some providers do not include one, so we skip the nonce validation here (but not other validations).
return upstreamOIDCIdentityProvider.ValidateTokenAndMergeWithUserInfo(ctx, refreshed, "", true, false)
return upstreamOIDCIdentityProvider.ValidateTokenAndMergeWithUserInfo(ctx, refreshed, "", h.requireIDTokenOnRefresh, false)
}

// handleAuthCodeCallback is used as an http handler, so it does not run in the CLI's main goroutine.
Expand Down
72 changes: 72 additions & 0 deletions pkg/oidcclient/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4065,3 +4065,75 @@ func TestMaybePrintAuditID(t *testing.T) {
})
}
}

func TestHandleRefreshWithRequireIDTokenOnRefresh(t *testing.T) {
accessTokenOnly := &oauth2.Token{
AccessToken: "access-tok",
RefreshToken: "new-refresh-tok",
TokenType: "Bearer",
}

t.Run("flag enabled, provider omits id_token, refresh succeeds with access token", func(t *testing.T) {
mock := mockUpstream(t)
mock.EXPECT().
PerformRefresh(gomock.Any(), "old-refresh-tok").
Return(accessTokenOnly, nil)
mock.EXPECT().
ValidateTokenAndMergeWithUserInfo(gomock.Any(), accessTokenOnly, nonce.Nonce(""), false, false).
Return(&oidctypes.Token{
AccessToken: &oidctypes.AccessToken{Token: "access-tok"},
}, nil)

h := &handlerState{
requireIDTokenOnRefresh: false,
logger: &emptyLogger{},
getProvider: func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI {
return mock
},
}
token, err := h.handleRefresh(context.Background(), &oidctypes.RefreshToken{Token: "old-refresh-tok"})
require.NoError(t, err)
require.NotNil(t, token)
require.Nil(t, token.IDToken)
require.Equal(t, "access-tok", token.AccessToken.Token)
})

t.Run("flag disabled (default), ValidateTokenAndMergeWithUserInfo called with requireIDToken=true", func(t *testing.T) {
mock := mockUpstream(t)
mock.EXPECT().
PerformRefresh(gomock.Any(), "old-refresh-tok").
Return(accessTokenOnly, nil)
mock.EXPECT().
ValidateTokenAndMergeWithUserInfo(gomock.Any(), accessTokenOnly, nonce.Nonce(""), true, false).
Return(nil, nil) // simulate provider returning nil (triggers browser login)

h := &handlerState{
requireIDTokenOnRefresh: true,
logger: &emptyLogger{},
getProvider: func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI {
return mock
},
}
token, err := h.handleRefresh(context.Background(), &oidctypes.RefreshToken{Token: "old-refresh-tok"})
require.NoError(t, err)
require.Nil(t, token)
})

t.Run("flag enabled, PerformRefresh itself fails, returns nil to trigger browser login", func(t *testing.T) {
mock := mockUpstream(t)
mock.EXPECT().
PerformRefresh(gomock.Any(), "old-refresh-tok").
Return(nil, fmt.Errorf("network error"))

h := &handlerState{
requireIDTokenOnRefresh: false,
logger: &emptyLogger{},
getProvider: func(_ *oauth2.Config, _ *coreosoidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI {
return mock
},
}
token, err := h.handleRefresh(context.Background(), &oidctypes.RefreshToken{Token: "old-refresh-tok"})
require.NoError(t, err)
require.Nil(t, token)
})
}