diff --git a/api/client.go b/api/client.go index 3f6fcdfe..4377b81d 100644 --- a/api/client.go +++ b/api/client.go @@ -171,19 +171,31 @@ func (c *Client) GetConnectionSecret(ctx context.Context, mg resource.Managed) ( return secret, nil } +// Token returns a valid access token for the configured API, or an empty +// string if none could be obtained. Errors are reported on stderr but +// otherwise swallowed so existing callers keep working. Use TokenE when the +// caller needs to react to the error (e.g. to avoid emitting a stale token). func (c *Client) Token(ctx context.Context) string { - if c.Config == nil { - return "" - } - - token, err := GetTokenFromConfig(ctx, c.Config) + token, err := c.TokenE(ctx) if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not get a valid access token: %s\n", err) return "" } return token } +// TokenE returns a valid access token for the configured API. Unlike Token it +// returns the underlying error instead of swallowing it, so an expired token +// surfaces as a clear, actionable error rather than an empty string. +func (c *Client) TokenE(ctx context.Context) (string, error) { + if c.Config == nil { + return "", fmt.Errorf("client config is not set") + } + + return GetTokenFromConfig(ctx, c.Config) +} + func (c *Client) DeploioRuntimeClient(ctx context.Context, scheme *runtime.Scheme) (runtimeclient.Client, error) { cfg, err := c.DeploioRuntimeConfig(ctx) if err != nil { diff --git a/api/login.go b/api/login.go index e60ea1fa..d4771f6a 100644 --- a/api/login.go +++ b/api/login.go @@ -27,6 +27,7 @@ import ( "github.com/int128/kubelogin/pkg/usecases/authentication/authcode" "github.com/int128/kubelogin/pkg/usecases/authentication/ropc" "github.com/int128/kubelogin/pkg/usecases/credentialplugin" + "github.com/ninech/nctl/internal/format" "golang.org/x/oauth2/clientcredentials" "k8s.io/client-go/pkg/apis/clientauthentication" "k8s.io/client-go/rest" @@ -49,6 +50,11 @@ const ( var ( defaultBindAddresses = []string{"127.0.0.1:8000", "127.0.0.1:18000"} defaultAuthTimeout = 180 * time.Second + // tokenExpiryLeeway is added to the current time when checking the exp + // claim of an access token. This treats tokens that are about to expire as + // already expired so we refresh them before they are rejected in-flight by + // a resource server. + tokenExpiryLeeway = 10 * time.Second ) // GetTokenFromConfig takes a rest.Config and returns a valid OIDC access @@ -126,31 +132,114 @@ type TokenGetter interface { type DefaultTokenGetter struct{} func (t *DefaultTokenGetter) GetTokenString(ctx context.Context, issuerURL, clientID string, usePKCE bool) (string, error) { - buf := &bytes.Buffer{} - if err := GetToken(ctx, issuerURL, clientID, usePKCE, buf); err != nil { + return validToken(ctx, issuerURL, clientID, usePKCE, time.Now(), getTokenString) +} + +// tokenFetcher obtains a token (and the expiry reported by kubelogin) for the +// given OIDC parameters. forceRefresh makes kubelogin bypass its token cache. +// It is a separate type so the orchestration in validToken can be tested +// without driving the real kubelogin login flow. +type tokenFetcher func(ctx context.Context, issuerURL, clientID string, usePKCE, forceRefresh bool) (string, *time.Time, error) + +// validToken fetches a token and guarantees it is not expired. If the fetched +// token is already expired it forces a refresh, and if even the refreshed +// token is expired it returns an actionable error instead of a stale token. +func validToken(ctx context.Context, issuerURL, clientID string, usePKCE bool, now time.Time, fetch tokenFetcher) (string, error) { + token, kubeExpiry, err := fetch(ctx, issuerURL, clientID, usePKCE, false) + if err != nil { return "", err } + // kubelogin only refreshes the token when its own cache considers it + // expired (e.g. based on a stored ExpirationTimestamp). That can hand us a + // token whose JWT exp has already passed while the cache still thinks it's + // valid. Inspect the exp claim ourselves and, if the token is stale, force + // kubelogin to refresh instead of returning it. + if _, expired := accessTokenExpired(token, kubeExpiry, now); !expired { + return token, nil + } + + token, kubeExpiry, err = fetch(ctx, issuerURL, clientID, usePKCE, true) + if err != nil { + return "", err + } + + // If the forced refresh still yields an expired token, a non-interactive + // refresh was not possible (e.g. the refresh token is gone). Fail with an + // actionable error instead of writing a stale token to stdout. + if expiry, expired := accessTokenExpired(token, kubeExpiry, now); expired { + return "", fmt.Errorf( + "access token expired on %s, run %q to re-authenticate", + expiry.UTC().Format(time.RFC3339), format.Command().Login(), + ) + } + + return token, nil +} + +// getTokenString runs the OIDC login flow via kubelogin and returns the access +// token together with the expiry reported by kubelogin (nil if absent). When +// forceRefresh is true, kubelogin bypasses its token cache and refreshes. +func getTokenString(ctx context.Context, issuerURL, clientID string, usePKCE, forceRefresh bool) (string, *time.Time, error) { + buf := &bytes.Buffer{} + if err := GetToken(ctx, issuerURL, clientID, usePKCE, forceRefresh, buf); err != nil { + return "", nil, err + } + creds := &clientauthentication.ExecCredential{} if err := json.NewDecoder(buf).Decode(creds); err != nil { - return "", fmt.Errorf("unable to decode exec credentials: %w", err) + return "", nil, fmt.Errorf("unable to decode exec credentials: %w", err) } - if creds.Status.ExpirationTimestamp != nil && creds.Status.ExpirationTimestamp.Time.Before(time.Now()) { - return "", fmt.Errorf("token expired on %s", creds.Status.ExpirationTimestamp.Time) + var kubeExpiry *time.Time + if creds.Status.ExpirationTimestamp != nil { + kubeExpiry = &creds.Status.ExpirationTimestamp.Time } - return creds.Status.Token, nil + return creds.Status.Token, kubeExpiry, nil +} + +// accessTokenExpired reports whether the access token is expired as of now. It +// prefers the exp claim of the JWT itself and falls back to the expiry +// reported by kubelogin for non-JWT tokens. The returned time is the expiry +// the decision was based on (zero if no expiry information is available, in +// which case the token is treated as not expired). +func accessTokenExpired(token string, kubeExpiry *time.Time, now time.Time) (time.Time, bool) { + if exp, ok := tokenExpiry(token); ok { + return exp, !exp.After(now.Add(tokenExpiryLeeway)) + } + if kubeExpiry != nil { + return *kubeExpiry, !kubeExpiry.After(now.Add(tokenExpiryLeeway)) + } + return time.Time{}, false +} + +// tokenExpiry decodes the (unverified) JWT and returns the time encoded in its +// exp claim. The signature is intentionally not verified: we only need the +// expiry to decide whether to force a refresh. ok is false if the token is not +// a parseable JWT or carries no exp claim. +func tokenExpiry(token string) (expiry time.Time, ok bool) { + claims := jwt.RegisteredClaims{} + if _, _, err := jwt.NewParser().ParseUnverified(token, &claims); err != nil { + return time.Time{}, false + } + if claims.ExpiresAt == nil { + return time.Time{}, false + } + return claims.ExpiresAt.Time, true } // GetToken executes the OIDC login flow using the kubelogin with the provided -// OIDC parameters writes the raw JSON ExecCredential result to out. -func GetToken(ctx context.Context, issuerURL, clientID string, usePKCE bool, out io.Writer) error { +// OIDC parameters writes the raw JSON ExecCredential result to out. When +// forceRefresh is true, kubelogin bypasses its token cache and refreshes the +// token regardless of its cached expiration. +func GetToken(ctx context.Context, issuerURL, clientID string, usePKCE, forceRefresh bool, out io.Writer) error { in := credentialplugin.Input{ Provider: oidc.Provider{ IssuerURL: issuerURL, ClientID: clientID, }, + ForceRefresh: forceRefresh, TokenCacheConfig: tokencache.Config{ Directory: path.Join(homedir.HomeDir(), DefaultTokenCachePath), }, diff --git a/api/login_test.go b/api/login_test.go new file mode 100644 index 00000000..d4db783f --- /dev/null +++ b/api/login_test.go @@ -0,0 +1,185 @@ +package api + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" +) + +// makeJWT builds a signed JWT carrying the given exp claim. When exp is the +// zero value, no exp claim is set. The signature is irrelevant because the +// expiry check parses the token without verification. +func makeJWT(t *testing.T, exp time.Time) string { + t.Helper() + + claims := jwt.RegisteredClaims{} + if !exp.IsZero() { + claims.ExpiresAt = jwt.NewNumericDate(exp) + } + + token, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte("test-secret")) + require.NoError(t, err) + return token +} + +func TestTokenExpiry(t *testing.T) { + now := time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC) + + t.Run("reads exp claim", func(t *testing.T) { + exp := now.Add(time.Hour) + got, ok := tokenExpiry(makeJWT(t, exp)) + require.True(t, ok) + require.True(t, got.Equal(exp), "got %s, want %s", got, exp) + }) + + t.Run("missing exp claim", func(t *testing.T) { + _, ok := tokenExpiry(makeJWT(t, time.Time{})) + require.False(t, ok) + }) + + t.Run("not a JWT", func(t *testing.T) { + _, ok := tokenExpiry("not-a-jwt") + require.False(t, ok) + }) +} + +func TestAccessTokenExpired(t *testing.T) { + now := time.Date(2026, 5, 29, 12, 0, 0, 0, time.UTC) + + for name, tc := range map[string]struct { + token string + kubeExpiry *time.Time + wantExpired bool + wantExpiry time.Time + }{ + "valid JWT with future exp": { + token: makeJWT(t, now.Add(time.Hour)), + wantExpired: false, + wantExpiry: now.Add(time.Hour), + }, + "JWT expired hours ago": { + token: makeJWT(t, now.Add(-3*time.Hour)), + wantExpired: true, + wantExpiry: now.Add(-3 * time.Hour), + }, + "JWT within expiry leeway is treated as expired": { + token: makeJWT(t, now.Add(tokenExpiryLeeway/2)), + wantExpired: true, + wantExpiry: now.Add(tokenExpiryLeeway / 2), + }, + "JWT exp takes precedence over a valid kubelogin expiry": { + token: makeJWT(t, now.Add(-time.Hour)), + kubeExpiry: new(now.Add(time.Hour)), + wantExpired: true, + wantExpiry: now.Add(-time.Hour), + }, + "missing exp falls back to expired kubelogin expiry": { + token: makeJWT(t, time.Time{}), + kubeExpiry: new(now.Add(-time.Hour)), + wantExpired: true, + wantExpiry: now.Add(-time.Hour), + }, + "missing exp falls back to valid kubelogin expiry": { + token: makeJWT(t, time.Time{}), + kubeExpiry: new(now.Add(time.Hour)), + wantExpired: false, + wantExpiry: now.Add(time.Hour), + }, + "missing exp and no kubelogin expiry is treated as not expired": { + token: makeJWT(t, time.Time{}), + kubeExpiry: nil, + wantExpired: false, + }, + "opaque token falls back to expired kubelogin expiry": { + token: "opaque-token", + kubeExpiry: new(now.Add(-time.Hour)), + wantExpired: true, + wantExpiry: now.Add(-time.Hour), + }, + "opaque token with no kubelogin expiry is treated as not expired": { + token: "opaque-token", + kubeExpiry: nil, + wantExpired: false, + }, + } { + t.Run(name, func(t *testing.T) { + expiry, expired := accessTokenExpired(tc.token, tc.kubeExpiry, now) + require.Equal(t, tc.wantExpired, expired) + if !tc.wantExpiry.IsZero() { + require.True(t, expiry.Equal(tc.wantExpiry), "got %s, want %s", expiry, tc.wantExpiry) + } + }) + } +} + +// TestValidTokenOrchestration exercises the force-refresh / no-stale-token +// logic of validToken without driving the real kubelogin flow. The fetch stub +// records how it was called and returns scripted tokens per attempt. +func TestValidTokenOrchestration(t *testing.T) { + now := time.Date(2026, 6, 1, 12, 0, 0, 0, time.UTC) + valid := makeJWT(t, now.Add(time.Hour)) + expired := makeJWT(t, now.Add(-time.Hour)) + + for name, tc := range map[string]struct { + // first/second are the tokens returned for forceRefresh=false / true. + first, second string + secondErr error + wantToken string + wantErr string + wantForceSecond bool // expect a forced (forceRefresh=true) second fetch + }{ + "fresh token is returned without a refresh": { + first: valid, + wantToken: valid, + wantForceSecond: false, + }, + "expired token triggers a forced refresh that succeeds": { + first: expired, + second: valid, + wantToken: valid, + wantForceSecond: true, + }, + "still expired after forced refresh errors instead of emitting it": { + first: expired, + second: expired, + wantErr: "access token expired on 2026-06-01T11:00:00Z", + wantForceSecond: true, + }, + "error from the forced refresh is propagated": { + first: expired, + secondErr: fmt.Errorf("boom"), + wantErr: "boom", + wantForceSecond: true, + }, + } { + t.Run(name, func(t *testing.T) { + var calls []bool // forceRefresh per call + fetch := func(_ context.Context, _, _ string, _, forceRefresh bool) (string, *time.Time, error) { + calls = append(calls, forceRefresh) + if forceRefresh { + return tc.second, nil, tc.secondErr + } + return tc.first, nil, nil + } + + token, err := validToken(context.Background(), "https://issuer", "client", false, now, fetch) + + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantToken, token) + } + + if tc.wantForceSecond { + require.Equal(t, []bool{false, true}, calls, "expected a normal fetch followed by a forced refresh") + } else { + require.Equal(t, []bool{false}, calls, "expected a single non-forced fetch") + } + }) + } +} diff --git a/auth/oidc.go b/auth/oidc.go index 7219fd55..f3481f19 100644 --- a/auth/oidc.go +++ b/auth/oidc.go @@ -18,7 +18,7 @@ type OIDCCmd struct { const OIDCCmdName = api.OIDCCmdName func (o *OIDCCmd) Run(ctx context.Context) error { - return api.GetToken(ctx, o.IssuerURL, o.ClientID, o.UsePKCE, os.Stdout) + return api.GetToken(ctx, o.IssuerURL, o.ClientID, o.UsePKCE, false, os.Stdout) } // execConfig returns an *clientcmdapi.ExecConfig that can be used to login to diff --git a/auth/print_access_token.go b/auth/print_access_token.go index 7f7c8b10..0e83603f 100644 --- a/auth/print_access_token.go +++ b/auth/print_access_token.go @@ -12,6 +12,11 @@ type PrintAccessTokenCmd struct { } func (cmd *PrintAccessTokenCmd) Run(ctx context.Context, client *api.Client) error { - cmd.Println(client.Token(ctx)) + token, err := client.TokenE(ctx) + if err != nil { + return err + } + + cmd.Println(token) return nil }