Skip to content

Commit

Permalink
Add new --oidc-use-access-token flag to get-token (#1084)
Browse files Browse the repository at this point in the history
* Add new `--oidc-use-access-token` flag to `get-token`

Implements #1083. See
description there for context.

In its current form, this PR is bare bones functionality. I have not yet
added any tests to confirm this behavior. Additionally, we could
consider updtating some of the naming. It is confusing to return a
`TokenSet` where `IDToken` actually has an `accessToken`. I'm open to
feedback on how best to improve this.

However, this PR is functional. I have validated it locally. Without
adding `--oidc-use-access-token`, and `id_token` is successfully
returned. Adding `--oidc-use-access-token` results in an `access_token`
being successfully returned.

* Fix failing tests

Needed to plumb through our new parameter `UseAccessToken` to the mocks
as well.

* Add a test to make sure new flag is plumbed through

* Support Access Tokens whose audience differ from the client_id

As noted in the PR, there are some cases where the access token `aud`
field will not be the `client_id`. To allow for these, we use a
different token verifier that will not verify that claim.

---------

Co-authored-by: Adam kafka <[email protected]>
  • Loading branch information
adkafka and Adam kafka authored Aug 16, 2024
1 parent 70ce255 commit 905238c
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 13 deletions.
26 changes: 26 additions & 0 deletions pkg/cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func TestCmd_Run(t *testing.T) {
RedirectURLHostname: "localhost",
},
},
UseAccessToken: false,
},
},
"FullOptions": {
Expand Down Expand Up @@ -150,6 +151,30 @@ func TestCmd_Run(t *testing.T) {
RedirectURLHostname: "localhost",
},
},
UseAccessToken: false,
},
},
"AccessToken": {
args: []string{executable,
"get-token",
"--oidc-issuer-url", "https://issuer.example.com",
"--oidc-client-id", "YOUR_CLIENT_ID",
"--oidc-use-access-token=true",
},
in: credentialplugin.Input{
TokenCacheDir: filepath.Join(userHomeDir, ".kube/cache/oidc-login"),
Provider: oidc.Provider{
IssuerURL: "https://issuer.example.com",
ClientID: "YOUR_CLIENT_ID",
},
GrantOptionSet: authentication.GrantOptionSet{
AuthCodeBrowserOption: &authcode.BrowserOption{
BindAddress: defaultListenAddress,
AuthenticationTimeout: defaultAuthenticationTimeoutSec * time.Second,
RedirectURLHostname: "localhost",
},
},
UseAccessToken: true,
},
},
"HomedirExpansion": {
Expand Down Expand Up @@ -180,6 +205,7 @@ func TestCmd_Run(t *testing.T) {
TLSClientConfig: tlsclientconfig.Config{
CACertFilename: []string{filepath.Join(userHomeDir, ".kube/ca.crt")},
},
UseAccessToken: false,
},
},
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/get_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type getTokenOptions struct {
ClientSecret string
ExtraScopes []string
UsePKCE bool
UseAccessToken bool
TokenCacheDir string
tlsOptions tlsOptions
authenticationOptions authenticationOptions
Expand All @@ -30,6 +31,7 @@ func (o *getTokenOptions) addFlags(f *pflag.FlagSet) {
f.StringVar(&o.ClientSecret, "oidc-client-secret", "", "Client secret of the provider")
f.StringSliceVar(&o.ExtraScopes, "oidc-extra-scope", nil, "Scopes to request to the provider")
f.BoolVar(&o.UsePKCE, "oidc-use-pkce", false, "Force PKCE usage")
f.BoolVar(&o.UseAccessToken, "oidc-use-access-token", false, "Instead of using the id_token, use the access_token to authenticate to Kubernetes")
f.StringVar(&o.TokenCacheDir, "token-cache-dir", defaultTokenCacheDir, "Path to a directory for token cache")
f.BoolVar(&o.ForceRefresh, "force-refresh", false, "If set, refresh the ID token regardless of its expiration time")
o.tlsOptions.addFlags(f)
Expand Down Expand Up @@ -85,6 +87,7 @@ func (cmd *GetToken) New() *cobra.Command {
GrantOptionSet: grantOptionSet,
TLSClientConfig: o.tlsOptions.tlsClientConfig(),
ForceRefresh: o.ForceRefresh,
UseAccessToken: o.UseAccessToken,
}
if err := cmd.GetToken.Do(c.Context(), in); err != nil {
return fmt.Errorf("get-token: %w", err)
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type setupOptions struct {
ClientSecret string
ExtraScopes []string
UsePKCE bool
UseAccessToken bool
tlsOptions tlsOptions
authenticationOptions authenticationOptions
}
Expand All @@ -25,6 +26,7 @@ func (o *setupOptions) addFlags(f *pflag.FlagSet) {
f.StringVar(&o.ClientSecret, "oidc-client-secret", "", "Client secret of the provider")
f.StringSliceVar(&o.ExtraScopes, "oidc-extra-scope", nil, "Scopes to request to the provider")
f.BoolVar(&o.UsePKCE, "oidc-use-pkce", false, "Force PKCE usage")
f.BoolVar(&o.UseAccessToken, "oidc-use-access-token", false, "Instead of using the id_token, use the access_token to authenticate to Kubernetes")
o.tlsOptions.addFlags(f)
o.authenticationOptions.addFlags(f)
}
Expand All @@ -50,6 +52,7 @@ func (cmd *Setup) New() *cobra.Command {
ClientSecret: o.ClientSecret,
ExtraScopes: o.ExtraScopes,
UsePKCE: o.UsePKCE,
UseAccessToken: o.UseAccessToken,
GrantOptionSet: grantOptionSet,
TLSClientConfig: o.tlsOptions.tlsClientConfig(),
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/oidc/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type client struct {
logger logger.Interface
supportedPKCEMethods []string
deviceAuthorizationEndpoint string
useAccessToken bool
}

func (c *client) wrapContext(ctx context.Context) context.Context {
Expand Down Expand Up @@ -205,6 +206,32 @@ func (c *client) verifyToken(ctx context.Context, token *oauth2.Token, nonce str
if nonce != "" && nonce != verifiedIDToken.Nonce {
return nil, fmt.Errorf("nonce did not match (wants %s but got %s)", nonce, verifiedIDToken.Nonce)
}

if c.useAccessToken {
accessToken, ok := token.Extra("access_token").(string)
if !ok {
return nil, fmt.Errorf("access_token is missing in the token response: %#v", accessToken)
}

// We intentionally do not perform a ClientID check here because there
// are some use cases in access_tokens where we *expect* the audience
// to differ. For example, one can explicitly set
// `audience=CLUSTER_CLIENT_ID` as an extra auth parameter.
verifier = c.provider.Verifier(&gooidc.Config{ClientID: "", Now: c.clock.Now, SkipClientIDCheck: true})

_, err := verifier.Verify(ctx, accessToken)
if err != nil {
return nil, fmt.Errorf("could not verify the access token: %w", err)
}

// There is no `nonce` to check on the `access_token`. We rely on the
// above `nonce` check on the `id_token`.

return &oidc.TokenSet{
IDToken: accessToken,
RefreshToken: token.RefreshToken,
}, nil
}
return &oidc.TokenSet{
IDToken: idToken,
RefreshToken: token.RefreshToken,
Expand Down
5 changes: 3 additions & 2 deletions pkg/oidc/client/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var Set = wire.NewSet(
)

type FactoryInterface interface {
New(ctx context.Context, p oidc.Provider, tlsClientConfig tlsclientconfig.Config) (Interface, error)
New(ctx context.Context, p oidc.Provider, tlsClientConfig tlsclientconfig.Config, useAccessToken bool) (Interface, error)
}

type Factory struct {
Expand All @@ -34,7 +34,7 @@ type Factory struct {
}

// New returns an instance of infrastructure.Interface with the given configuration.
func (f *Factory) New(ctx context.Context, p oidc.Provider, tlsClientConfig tlsclientconfig.Config) (Interface, error) {
func (f *Factory) New(ctx context.Context, p oidc.Provider, tlsClientConfig tlsclientconfig.Config, useAccessToken bool) (Interface, error) {
rawTLSClientConfig, err := f.Loader.Load(tlsClientConfig)
if err != nil {
return nil, fmt.Errorf("could not load the TLS client config: %w", err)
Expand Down Expand Up @@ -80,6 +80,7 @@ func (f *Factory) New(ctx context.Context, p oidc.Provider, tlsClientConfig tlsc
logger: f.Logger,
supportedPKCEMethods: supportedPKCEMethods,
deviceAuthorizationEndpoint: deviceAuthorizationEndpoint,
useAccessToken: useAccessToken,
}, nil
}

Expand Down
15 changes: 8 additions & 7 deletions pkg/oidc/client/mock_FactoryInterface.go

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

3 changes: 2 additions & 1 deletion pkg/usecases/authentication/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type Input struct {
CachedTokenSet *oidc.TokenSet // optional
TLSClientConfig tlsclientconfig.Config
ForceRefresh bool
UseAccessToken bool
}

type GrantOptionSet struct {
Expand Down Expand Up @@ -98,7 +99,7 @@ func (u *Authentication) Do(ctx context.Context, in Input) (*Output, error) {
}

u.Logger.V(1).Infof("initializing an OpenID Connect client")
oidcClient, err := u.ClientFactory.New(ctx, in.Provider, in.TLSClientConfig)
oidcClient, err := u.ClientFactory.New(ctx, in.Provider, in.TLSClientConfig, in.UseAccessToken)
if err != nil {
return nil, fmt.Errorf("oidc error: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/usecases/authentication/authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestAuthentication_Do(t *testing.T) {
}, nil)
mockClientFactory := client.NewMockFactoryInterface(t)
mockClientFactory.EXPECT().
New(ctx, dummyProvider, dummyTLSClientConfig).
New(ctx, dummyProvider, dummyTLSClientConfig, false).
Return(mockClient, nil)
u := Authentication{
ClientFactory: mockClientFactory,
Expand Down Expand Up @@ -143,7 +143,7 @@ func TestAuthentication_Do(t *testing.T) {
}, nil)
mockClientFactory := client.NewMockFactoryInterface(t)
mockClientFactory.EXPECT().
New(ctx, dummyProvider, dummyTLSClientConfig).
New(ctx, dummyProvider, dummyTLSClientConfig, false).
Return(mockClient, nil)
u := Authentication{
ClientFactory: mockClientFactory,
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestAuthentication_Do(t *testing.T) {
}, nil)
mockClientFactory := client.NewMockFactoryInterface(t)
mockClientFactory.EXPECT().
New(ctx, dummyProvider, dummyTLSClientConfig).
New(ctx, dummyProvider, dummyTLSClientConfig, false).
Return(mockClient, nil)
u := Authentication{
ClientFactory: mockClientFactory,
Expand Down
2 changes: 2 additions & 0 deletions pkg/usecases/credentialplugin/get_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Input struct {
GrantOptionSet authentication.GrantOptionSet
TLSClientConfig tlsclientconfig.Config
ForceRefresh bool
UseAccessToken bool
}

type GetToken struct {
Expand Down Expand Up @@ -92,6 +93,7 @@ func (u *GetToken) Do(ctx context.Context, in Input) error {
CachedTokenSet: cachedTokenSet,
TLSClientConfig: in.TLSClientConfig,
ForceRefresh: in.ForceRefresh,
UseAccessToken: in.UseAccessToken,
}
authenticationOutput, err := u.Authentication.Do(ctx, authenticationInput)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions pkg/usecases/setup/stage2.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type Stage2Input struct {
ClientSecret string
ExtraScopes []string // optional
UsePKCE bool // optional
UseAccessToken bool // optional
ListenAddressArgs []string // non-nil if set by the command arg
GrantOptionSet authentication.GrantOptionSet
TLSClientConfig tlsclientconfig.Config
Expand All @@ -91,6 +92,7 @@ func (u *Setup) DoStage2(ctx context.Context, in Stage2Input) error {
},
GrantOptionSet: in.GrantOptionSet,
TLSClientConfig: in.TLSClientConfig,
UseAccessToken: in.UseAccessToken,
})
if err != nil {
return fmt.Errorf("authentication error: %w", err)
Expand Down Expand Up @@ -128,6 +130,9 @@ func makeCredentialPluginArgs(in Stage2Input) []string {
if in.UsePKCE {
args = append(args, "--oidc-use-pkce")
}
if in.UseAccessToken {
args = append(args, "--oidc-use-access-token")
}
for _, f := range in.TLSClientConfig.CACertFilename {
args = append(args, "--certificate-authority="+f)
}
Expand Down

0 comments on commit 905238c

Please sign in to comment.