diff --git a/crates/openshell-bootstrap/src/oidc_token.rs b/crates/openshell-bootstrap/src/oidc_token.rs index 16988a5f9d..85dd29d791 100644 --- a/crates/openshell-bootstrap/src/oidc_token.rs +++ b/crates/openshell-bootstrap/src/oidc_token.rs @@ -193,6 +193,17 @@ mod tests { assert!(is_token_actually_expired(&expired)); } + #[test] + fn conformance_fixture_matches_cli_serialization() { + const FIXTURE: &str = include_str!("../../../sdk/conformance/oidc-token-bundle.json"); + let bundle: OidcTokenBundle = serde_json::from_str(FIXTURE).unwrap(); + assert!(bundle.refresh_token.is_some() && bundle.expires_at.is_some()); + assert_eq!( + serde_json::to_value(&bundle).unwrap(), + serde_json::from_str::(FIXTURE).unwrap() + ); + } + #[test] fn oidc_login_prompt_marker_is_per_gateway() { let tmp = tempfile::tempdir().unwrap(); diff --git a/scripts/update_license_headers.py b/scripts/update_license_headers.py index 48d7810171..bb00feaa4f 100755 --- a/scripts/update_license_headers.py +++ b/scripts/update_license_headers.py @@ -66,6 +66,7 @@ "deploy/deb/control.in", "scripts/keycloak-realm.json", "sdk/conformance/oauth-client-credentials.json", + "sdk/conformance/oidc-token-bundle.json", "sdk/typescript/biome.json", "sdk/typescript/tsconfig.build.json", "sdk/typescript/tsconfig.json", diff --git a/sdk/conformance/oidc-token-bundle.json b/sdk/conformance/oidc-token-bundle.json new file mode 100644 index 0000000000..19c71745f0 --- /dev/null +++ b/sdk/conformance/oidc-token-bundle.json @@ -0,0 +1,7 @@ +{ + "access_token": "conformance-access-token", + "refresh_token": "conformance-refresh-token", + "expires_at": 4102444800, + "issuer": "https://issuer.example.com", + "client_id": "openshell-cli" +} diff --git a/sdk/conformance/oidc-token-bundle.json.license b/sdk/conformance/oidc-token-bundle.json.license new file mode 100644 index 0000000000..817df24424 --- /dev/null +++ b/sdk/conformance/oidc-token-bundle.json.license @@ -0,0 +1,2 @@ +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 diff --git a/sdk/go/README.md b/sdk/go/README.md index 80e71aebee..dceabfadc7 100644 --- a/sdk/go/README.md +++ b/sdk/go/README.md @@ -127,7 +127,8 @@ defer client.Close() ``` Concurrent callers share a single refresh call. If the token source fails, the -SDK falls back to the cached token with a logged warning. See the +SDK falls back to the cached token with a logged warning until that token +expires; after that, RPCs fail with the refresh error. See the [Auth](https://ro14nd.de/openshell-sdk-go/api/auth.html) docs for details. ### With edge proxy headers diff --git a/sdk/go/docs/src/api/gateway.md b/sdk/go/docs/src/api/gateway.md index f61e19a85d..e1d8006992 100644 --- a/sdk/go/docs/src/api/gateway.md +++ b/sdk/go/docs/src/api/gateway.md @@ -55,7 +55,7 @@ client, err := gateway.NewClient("staging", ) ``` -**Errors**: `ErrGatewayNotFound`, `ErrConfigParse`, `ErrTokenLoad`, +**Errors**: `ErrGatewayNotFound`, `ErrConfigParse`, `ErrUnsupportedAuthMode`, `ErrInvalidGatewayName`, `ErrNoActiveGateway` ### LoadConfig @@ -124,7 +124,7 @@ type Info struct { | `""` or `"none"` | `AuthModeNone` | `v1.NoAuth()` | | `"plaintext"` | `AuthModePlaintext` | `v1.NoAuth()` + insecure TLS | | `"cloudflare_jwt"` | `AuthModeCloudflareJWT` | Lazy edge token auth | -| `"oidc"` | `AuthModeOIDC` | `v1.RefreshableToken` with disk source | +| `"oidc"` | `AuthModeOIDC` | `v1.RefreshableToken` that reads `oidc_token.json` near expiry and fails once the token has expired | | `"mtls"` | `AuthModeMTLS` | Unsupported (use `WithAuth`) | ### ClientOption @@ -146,11 +146,13 @@ client, err := gateway.NewClient("my-gateway") if errors.Is(err, gateway.ErrGatewayNotFound) { fmt.Println("Gateway not configured. Run: openshell gateway add my-gateway") } -if errors.Is(err, gateway.ErrTokenLoad) { - fmt.Println("Token expired or missing. Run: openshell gateway login my-gateway") -} ``` +Tokens load lazily, so `NewClient` does not return `ErrTokenLoad`. For `oidc` +gateways, a missing, malformed, or expired `oidc_token.json` surfaces on RPCs as +an error for which `v1.IsUnauthenticated(err)` is true, and its message includes +``run `openshell gateway login ` ``. + | Error | Meaning | |-------|---------| | `ErrGatewayNotFound` | No gateway directory in user or system paths | diff --git a/sdk/go/docs/src/api/oidc.md b/sdk/go/docs/src/api/oidc.md index 110015ae60..1f3c1bef27 100644 --- a/sdk/go/docs/src/api/oidc.md +++ b/sdk/go/docs/src/api/oidc.md @@ -39,7 +39,10 @@ PKCE. If the provider does not support PKCE (S256), the flow proceeds without it. Tokens are persisted to the gateway's config directory as -`oidc_token.json`. Subsequent calls reuse a valid cached token. +`oidc_token.json`, in the same format as `openshell gateway login`, so CLI +commands against that gateway can use them. Subsequent calls reuse the stored +token only when it is valid and was issued for the gateway's issuer and client +ID; otherwise `Login` runs the flow again and rewrites the file. For standalone use (no gateway), pass an empty `gatewayName` with `WithIssuer` and `WithClientID`. Combine with `WithInMemory` to skip diff --git a/sdk/go/openshell/v1/auth_refresh.go b/sdk/go/openshell/v1/auth_refresh.go index 86c475bf12..5cc9fbf2fd 100644 --- a/sdk/go/openshell/v1/auth_refresh.go +++ b/sdk/go/openshell/v1/auth_refresh.go @@ -66,6 +66,13 @@ type refreshableAuth struct { logger types.Logger nextRetry time.Time backoff time.Duration + lastErr error +} + +// usableCached reports whether the cached token has not yet expired. +// Callers must hold r.mu. +func (r *refreshableAuth) usableCached() bool { + return r.tok != nil && (r.tok.Expiry.IsZero() || time.Now().Before(r.tok.Expiry)) } func (r *refreshableAuth) isTokenValid() bool { @@ -87,13 +94,14 @@ func (r *refreshableAuth) GetRequestMetadata(ctx context.Context, _ ...string) ( } if !r.nextRetry.IsZero() && time.Now().Before(r.nextRetry) { - if r.tok != nil { + if r.usableCached() { tok := r.tok.AccessToken r.mu.Unlock() return map[string]string{"authorization": "Bearer " + tok}, nil } + err := r.lastErr r.mu.Unlock() - return nil, errors.New("openshell: token refresh failed and backoff is active") + return nil, err } r.mu.Unlock() @@ -127,8 +135,9 @@ func (r *refreshableAuth) GetRequestMetadata(ctx context.Context, _ ...string) ( r.backoff = bo r.nextRetry = time.Now().Add(bo) } + r.lastErr = err - if r.tok != nil { + if r.usableCached() { if r.logger != nil { r.logger.Error(err, "token refresh failed, using cached token") } @@ -144,6 +153,7 @@ func (r *refreshableAuth) GetRequestMetadata(ctx context.Context, _ ...string) ( r.tok = tok r.backoff = 0 r.nextRetry = time.Time{} + r.lastErr = nil return map[string]string{"authorization": "Bearer " + r.tok.AccessToken}, nil } @@ -155,6 +165,8 @@ func (r *refreshableAuth) RequireTransportSecurity() bool { // and refreshes them before expiry. Concurrent callers share a single // in-flight refresh via singleflight. Failed refreshes trigger exponential // backoff (1s, 2s, 4s, ..., 30s cap) to avoid amplifying token endpoint outages. +// After a failed refresh, the cached token is used only until it expires; +// after that, requests fail with the last refresh error. func RefreshableToken(src oauth2.TokenSource, opts ...RefreshOption) (AuthProvider, error) { if src == nil { return nil, errNilTokenSource diff --git a/sdk/go/openshell/v1/auth_refresh_test.go b/sdk/go/openshell/v1/auth_refresh_test.go index dc0b1ec16a..69fde8b118 100644 --- a/sdk/go/openshell/v1/auth_refresh_test.go +++ b/sdk/go/openshell/v1/auth_refresh_test.go @@ -5,6 +5,7 @@ package v1 import ( "context" + "errors" "fmt" "sync" "sync/atomic" @@ -194,38 +195,69 @@ func TestGetRequestMetadata_RefreshFailureReturnsStaleCachedToken(t *testing.T) tokenFunc: func() (*oauth2.Token, error) { n := callNum.Add(1) if n == 1 { - return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(-time.Minute)}, nil + return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(5 * time.Second)}, nil } return nil, fmt.Errorf("idp unavailable") }, } - provider, err := RefreshableToken(src, WithLeeway(0)) + provider, err := RefreshableToken(src) // default 10s leeway require.NoError(t, err) - // First call succeeds but returns already-expired token. + // First call succeeds with a token inside the leeway window. _, err = provider.GetRequestMetadata(context.Background()) require.NoError(t, err) - // Second call: refresh fails, should return stale token. + // Second call: refresh fails, should return the unexpired stale token. md, err := provider.GetRequestMetadata(context.Background()) require.NoError(t, err) assert.Equal(t, "Bearer stale", md["authorization"]) } +func TestGetRequestMetadata_RefreshFailureWithExpiredCachedTokenReturnsError(t *testing.T) { + errIDP := errors.New("idp unavailable") + var callNum atomic.Int32 + src := &mockTokenSource{ + tokenFunc: func() (*oauth2.Token, error) { + n := callNum.Add(1) + if n == 1 { + return &oauth2.Token{AccessToken: "expired", Expiry: time.Now().Add(-time.Minute)}, nil + } + return nil, errIDP + }, + } + logger := &captureLogger{} + provider, err := RefreshableToken(src, WithLeeway(0), WithLogger(logger)) + require.NoError(t, err) + + _, err = provider.GetRequestMetadata(context.Background()) + require.NoError(t, err) + + md, err := provider.GetRequestMetadata(context.Background()) + require.ErrorIs(t, err, errIDP) + assert.Nil(t, md) + assert.Empty(t, logger.errors) + + // During backoff, the same error is returned without calling the source. + md, err = provider.GetRequestMetadata(context.Background()) + require.ErrorIs(t, err, errIDP) + assert.Nil(t, md) + assert.Equal(t, 2, src.calls()) +} + func TestGetRequestMetadata_RefreshFailureLogsWarning(t *testing.T) { var callNum atomic.Int32 src := &mockTokenSource{ tokenFunc: func() (*oauth2.Token, error) { n := callNum.Add(1) if n == 1 { - return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(-time.Minute)}, nil + return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(5 * time.Second)}, nil } return nil, fmt.Errorf("idp unavailable") }, } logger := &captureLogger{} - provider, err := RefreshableToken(src, WithLeeway(0), WithLogger(logger)) + provider, err := RefreshableToken(src, WithLogger(logger)) require.NoError(t, err) _, _ = provider.GetRequestMetadata(context.Background()) @@ -255,12 +287,12 @@ func TestGetRequestMetadata_RefreshFailureNoLoggerNoPanic(t *testing.T) { tokenFunc: func() (*oauth2.Token, error) { n := callNum.Add(1) if n == 1 { - return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(-time.Minute)}, nil + return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(5 * time.Second)}, nil } return nil, fmt.Errorf("idp unavailable") }, } - provider, err := RefreshableToken(src, WithLeeway(0)) + provider, err := RefreshableToken(src) require.NoError(t, err) _, _ = provider.GetRequestMetadata(context.Background()) @@ -348,12 +380,12 @@ func TestGetRequestMetadata_BackoffSkipsRefreshDuringWindow(t *testing.T) { tokenFunc: func() (*oauth2.Token, error) { n := callNum.Add(1) if n == 1 { - return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(-time.Minute)}, nil + return &oauth2.Token{AccessToken: "stale", Expiry: time.Now().Add(5 * time.Second)}, nil } return nil, fmt.Errorf("idp down") }, } - provider, err := RefreshableToken(src, WithLeeway(0)) + provider, err := RefreshableToken(src) require.NoError(t, err) _, err = provider.GetRequestMetadata(context.Background()) @@ -374,6 +406,24 @@ func TestGetRequestMetadata_BackoffSkipsRefreshDuringWindow(t *testing.T) { assert.Equal(t, beforeCount, src.calls(), "should not call Token() during backoff window") } +func TestGetRequestMetadata_BackoffWithoutCachedTokenReturnsLastError(t *testing.T) { + errIDP := errors.New("idp down") + src := &mockTokenSource{ + tokenFunc: func() (*oauth2.Token, error) { + return nil, errIDP + }, + } + provider, err := RefreshableToken(src) + require.NoError(t, err) + + _, err = provider.GetRequestMetadata(context.Background()) + require.ErrorIs(t, err, errIDP) + + _, err = provider.GetRequestMetadata(context.Background()) + require.ErrorIs(t, err, errIDP) + assert.Equal(t, 1, src.calls(), "should not call Token() during backoff window") +} + func TestGetRequestMetadata_BackoffResetsOnSuccess(t *testing.T) { var callNum atomic.Int32 src := &mockTokenSource{ diff --git a/sdk/go/openshell/v1/gateway/gateway.go b/sdk/go/openshell/v1/gateway/gateway.go index dde3545664..60245f2d93 100644 --- a/sdk/go/openshell/v1/gateway/gateway.go +++ b/sdk/go/openshell/v1/gateway/gateway.go @@ -169,7 +169,7 @@ func resolveAuthProvider(cfg *Config) (types.AuthProvider, error) { return &lazyEdgeAuth{loader: &edgeTokenLoader{dir: cfg.Dir}}, nil case AuthModeOIDC: - src := newDiskTokenSource(cfg.Dir) + src := newDiskTokenSource(cfg.Dir, cfg.Name) return v1.RefreshableToken(src) case AuthModeMTLS: diff --git a/sdk/go/openshell/v1/gateway/gateway_test.go b/sdk/go/openshell/v1/gateway/gateway_test.go index e22a0a7873..407ee6da8d 100644 --- a/sdk/go/openshell/v1/gateway/gateway_test.go +++ b/sdk/go/openshell/v1/gateway/gateway_test.go @@ -5,13 +5,16 @@ package gateway import ( "context" + "fmt" "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -87,7 +90,7 @@ func TestNewClient_AuthModeCloudflareJWT(t *testing.T) { func TestNewClient_AuthModeOIDC(t *testing.T) { setupGatewayWithTokens(t, "oidc-gw", `{"gateway_endpoint":"localhost:50051","auth_mode":"oidc"}`, - map[string]string{oidcTokenFile: `{"access_token":"test-oidc-token"}`}, + map[string]string{oidctoken.FileName: `{"access_token":"test-oidc-token"}`}, ) // RefreshableToken requires transport security, so use WithAuth @@ -341,9 +344,9 @@ func TestResolveAuthProvider_CloudflareJWT(t *testing.T) { func TestResolveAuthProvider_OIDC(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{"access_token":"oidc-access-token"}`) + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("oidc-access-token", "")) - cfg := &Config{AuthMode: AuthModeOIDC, Dir: dir} + cfg := &Config{AuthMode: AuthModeOIDC, Dir: dir, Name: "prod"} auth, err := resolveAuthProvider(cfg) require.NoError(t, err) require.NotNil(t, auth) @@ -353,6 +356,25 @@ func TestResolveAuthProvider_OIDC(t *testing.T) { assert.Equal(t, "Bearer oidc-access-token", md["authorization"]) } +func TestResolveAuthProvider_OIDCPicksUpReplacedTokenNearExpiry(t *testing.T) { + dir := t.TempDir() + // Within the default 10s refresh leeway, so the next call re-reads the file. + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("token-a", fmt.Sprint(time.Now().Add(5*time.Second).Unix()))) + + auth, err := resolveAuthProvider(&Config{AuthMode: AuthModeOIDC, Dir: dir, Name: "prod"}) + require.NoError(t, err) + + md, err := auth.GetRequestMetadata(context.Background()) + require.NoError(t, err) + assert.Equal(t, "Bearer token-a", md["authorization"]) + + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("token-b", fmt.Sprint(time.Now().Add(time.Hour).Unix()))) + + md, err = auth.GetRequestMetadata(context.Background()) + require.NoError(t, err) + assert.Equal(t, "Bearer token-b", md["authorization"]) +} + func TestResolveAuthProvider_MTLS(t *testing.T) { cfg := &Config{AuthMode: AuthModeMTLS} _, err := resolveAuthProvider(cfg) diff --git a/sdk/go/openshell/v1/gateway/token.go b/sdk/go/openshell/v1/gateway/token.go index fb19726e29..8ec003dda0 100644 --- a/sdk/go/openshell/v1/gateway/token.go +++ b/sdk/go/openshell/v1/gateway/token.go @@ -14,6 +14,8 @@ import ( "time" "golang.org/x/oauth2" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" ) const ( @@ -23,9 +25,6 @@ const ( // cfTokenFile is the legacy Cloudflare token filename, used as // fallback when edge_token does not exist. cfTokenFile = "cf_token" - - // oidcTokenFile is the OIDC token bundle filename. - oidcTokenFile = "oidc_token.json" ) // edgeTokenLoader provides lazy, thread-safe loading of the edge token @@ -87,72 +86,57 @@ func readEdgeToken(dir string) (string, error) { return token, nil } -// oidcBundle is the on-disk representation of oidc_token.json. -type oidcBundle struct { - AccessToken string `json:"access_token"` - RefreshToken string `json:"refresh_token"` - Expiry string `json:"expiry"` - ExpiresIn int64 `json:"expires_in"` -} - // diskTokenSource implements oauth2.TokenSource by reading oidc_token.json // from disk on every Token() call. This allows the source to pick up // tokens refreshed by the Rust CLI without process restart. type diskTokenSource struct { - dir string + dir string + name string } // newDiskTokenSource returns an oauth2.TokenSource that reads -// oidc_token.json from the given gateway directory on each Token() call. -func newDiskTokenSource(dir string) oauth2.TokenSource { - return &diskTokenSource{dir: dir} +// oidc_token.json from the directory of the named gateway on each +// Token() call. +func newDiskTokenSource(dir, name string) oauth2.TokenSource { + return &diskTokenSource{dir: dir, name: name} } // Token reads and parses oidc_token.json, returning an oauth2.Token. // The file is read on every call to pick up CLI-refreshed tokens. func (d *diskTokenSource) Token() (*oauth2.Token, error) { - path := filepath.Join(d.dir, oidcTokenFile) - - data, err := os.ReadFile(path) + data, err := os.ReadFile(filepath.Join(d.dir, oidctoken.FileName)) if err != nil { - return nil, fmt.Errorf("%w: cannot read %s: %v", ErrTokenLoad, oidcTokenFile, err) + return nil, d.loadError("cannot read %s: %v", oidctoken.FileName, err) } - var bundle oidcBundle + var bundle oidctoken.Bundle if err := json.Unmarshal(data, &bundle); err != nil { - return nil, fmt.Errorf("%w: invalid JSON in %s", ErrTokenLoad, oidcTokenFile) + return nil, d.loadError("invalid JSON in %s", oidctoken.FileName) } if bundle.AccessToken == "" { - return nil, fmt.Errorf("%w: missing access_token in %s", ErrTokenLoad, oidcTokenFile) + return nil, d.loadError("missing access_token in %s", oidctoken.FileName) } - - tok := &oauth2.Token{ - AccessToken: bundle.AccessToken, - RefreshToken: bundle.RefreshToken, - TokenType: "Bearer", + if bundle.Issuer == "" || bundle.ClientID == "" { + return nil, d.loadError("OIDC token for gateway %q is missing issuer or client_id", d.name) } - // Parse expiry from the absolute "expiry" field only. The "expires_in" - // field (seconds-until-expiry) cannot be used reliably because there is - // no "written_at" timestamp: interpreting it at read time would make - // stale tokens appear perpetually valid. - if bundle.Expiry != "" { - expiry, parseErr := time.Parse(time.RFC3339, bundle.Expiry) - if parseErr != nil { - return nil, fmt.Errorf("%w: invalid expiry in %s: %v", ErrTokenLoad, oidcTokenFile, parseErr) - } - tok.Expiry = expiry - - if time.Now().After(expiry) { - return nil, fmt.Errorf("%w: token in %s expired at %s; re-authenticate with the CLI", - ErrTokenLoad, oidcTokenFile, expiry.Format(time.RFC3339)) - } + tok := bundle.Token() + if !tok.Expiry.IsZero() && !time.Now().Before(tok.Expiry) { + return nil, d.loadError("OIDC token for gateway %q expired at %s", + d.name, tok.Expiry.UTC().Format(time.RFC3339)) } return tok, nil } +// loadError wraps ErrTokenLoad with the CLI login command that repairs +// the token file. +func (d *diskTokenSource) loadError(format string, args ...any) error { + return fmt.Errorf("%w: %s; run `openshell gateway login %s`", + ErrTokenLoad, fmt.Sprintf(format, args...), d.name) +} + // lazyEdgeAuth implements types.AuthProvider for the cloudflare_jwt auth // mode. Token loading is deferred to GetRequestMetadata so that // NewClient succeeds even when the token file is missing on disk (FR-007). diff --git a/sdk/go/openshell/v1/gateway/token_test.go b/sdk/go/openshell/v1/gateway/token_test.go index e9c01b4aed..19a8f1febc 100644 --- a/sdk/go/openshell/v1/gateway/token_test.go +++ b/sdk/go/openshell/v1/gateway/token_test.go @@ -4,6 +4,7 @@ package gateway import ( + "fmt" "os" "path/filepath" "testing" @@ -11,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" ) // --- T014: Edge token loading tests --- @@ -122,108 +125,99 @@ func TestEdgeTokenLoader_LazyError(t *testing.T) { // --- T015: diskTokenSource tests --- -func TestDiskTokenSource_ValidBundle(t *testing.T) { - dir := t.TempDir() - expiry := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) - writeFile(t, dir, oidcTokenFile, `{ - "access_token": "access-123", - "refresh_token": "refresh-456", - "expiry": "`+expiry+`" - }`) - - src := newDiskTokenSource(dir) - tok, err := src.Token() - require.NoError(t, err) - assert.Equal(t, "access-123", tok.AccessToken) - assert.Equal(t, "refresh-456", tok.RefreshToken) - assert.Equal(t, "Bearer", tok.TokenType) - assert.False(t, tok.Expiry.IsZero()) -} +const loginHint = "run `openshell gateway login prod`" -func TestDiskTokenSource_ExpiresInIgnored(t *testing.T) { +func TestDiskTokenSource_ValidBundle(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{ - "access_token": "access-789", - "expires_in": 3600 - }`) - - src := newDiskTokenSource(dir) - tok, err := src.Token() + fixture, err := os.ReadFile(filepath.Join("..", "..", "..", "..", "conformance", "oidc-token-bundle.json")) require.NoError(t, err) - assert.Equal(t, "access-789", tok.AccessToken) - // expires_in without an absolute expiry field is ignored because - // there is no written_at timestamp to compute absolute expiry from. - assert.True(t, tok.Expiry.IsZero(), "expiry should be zero when only expires_in is present") -} + writeFile(t, dir, oidctoken.FileName, string(fixture)) -func TestDiskTokenSource_ExpiryPrecedence(t *testing.T) { - dir := t.TempDir() - expiry := time.Now().Add(2 * time.Hour).UTC().Format(time.RFC3339) - writeFile(t, dir, oidcTokenFile, `{ - "access_token": "access-abc", - "expiry": "`+expiry+`", - "expires_in": 60 - }`) - - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") tok, err := src.Token() require.NoError(t, err) - // "expiry" field takes precedence over "expires_in". - assert.WithinDuration(t, time.Now().Add(2*time.Hour), tok.Expiry, 5*time.Second) + assert.Equal(t, "conformance-access-token", tok.AccessToken) + assert.Equal(t, "conformance-refresh-token", tok.RefreshToken) + assert.Equal(t, "Bearer", tok.TokenType) + assert.Equal(t, time.Unix(4102444800, 0), tok.Expiry) } func TestDiskTokenSource_NoExpiry(t *testing.T) { - dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{"access_token": "no-expiry-token"}`) - - src := newDiskTokenSource(dir) - tok, err := src.Token() - require.NoError(t, err) - assert.Equal(t, "no-expiry-token", tok.AccessToken) - assert.True(t, tok.Expiry.IsZero(), "should have zero expiry when not set") + for _, expiresAt := range []string{"", "null"} { + dir := t.TempDir() + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("no-expiry-token", expiresAt)) + + src := newDiskTokenSource(dir, "prod") + tok, err := src.Token() + require.NoError(t, err, "expires_at=%q", expiresAt) + assert.Equal(t, "no-expiry-token", tok.AccessToken) + assert.True(t, tok.Expiry.IsZero(), "should have zero expiry for expires_at=%q", expiresAt) + } } func TestDiskTokenSource_ExpiredToken(t *testing.T) { - dir := t.TempDir() - expiry := time.Now().Add(-time.Hour).UTC().Format(time.RFC3339) - writeFile(t, dir, oidcTokenFile, `{ - "access_token": "expired-token", - "expiry": "`+expiry+`" - }`) + for _, expiresAt := range []string{fmt.Sprint(time.Now().Add(-time.Hour).Unix()), "0"} { + dir := t.TempDir() + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("expired-token", expiresAt)) + + src := newDiskTokenSource(dir, "prod") + _, err := src.Token() + require.ErrorIs(t, err, ErrTokenLoad, "expires_at=%s", expiresAt) + assert.Contains(t, err.Error(), "expired at") + assert.Contains(t, err.Error(), loginHint) + } +} - src := newDiskTokenSource(dir) - _, err := src.Token() - require.Error(t, err) - assert.ErrorIs(t, err, ErrTokenLoad) - assert.Contains(t, err.Error(), "expired") - assert.Contains(t, err.Error(), "re-authenticate") +func TestDiskTokenSource_MissingIssuerOrClientID(t *testing.T) { + expiry := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) + bundles := map[string]string{ + "earlier Go SDK format": `{"access_token": "a", "refresh_token": "r", "expiry": "` + expiry + `", "expires_in": 3600}`, + "no issuer": `{"access_token": "a", "client_id": "openshell-cli"}`, + "no client_id": `{"access_token": "a", "issuer": "https://issuer.example.com"}`, + "empty issuer": `{"access_token": "a", "issuer": "", "client_id": "openshell-cli"}`, + } + for name, content := range bundles { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, oidctoken.FileName, content) + + src := newDiskTokenSource(dir, "prod") + _, err := src.Token() + require.Error(t, err) + assert.ErrorIs(t, err, ErrTokenLoad) + assert.Contains(t, err.Error(), `OIDC token for gateway "prod" is missing issuer or client_id`) + assert.Contains(t, err.Error(), loginHint) + }) + } } func TestDiskTokenSource_MissingFile(t *testing.T) { dir := t.TempDir() - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") _, err := src.Token() require.Error(t, err) assert.ErrorIs(t, err, ErrTokenLoad) + assert.Contains(t, err.Error(), loginHint) } func TestDiskTokenSource_MalformedJSON(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{not valid json}`) + writeFile(t, dir, oidctoken.FileName, `{not valid json}`) - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") _, err := src.Token() require.Error(t, err) assert.ErrorIs(t, err, ErrTokenLoad) assert.Contains(t, err.Error(), "invalid JSON") + assert.Contains(t, err.Error(), loginHint) } func TestDiskTokenSource_MissingAccessToken(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{"refresh_token": "only-refresh"}`) + writeFile(t, dir, oidctoken.FileName, `{"refresh_token": "only-refresh"}`) - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") _, err := src.Token() require.Error(t, err) assert.ErrorIs(t, err, ErrTokenLoad) @@ -232,15 +226,15 @@ func TestDiskTokenSource_MissingAccessToken(t *testing.T) { func TestDiskTokenSource_ReReadsOnEachCall(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{"access_token": "token-v1"}`) + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("token-v1", "")) - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") tok1, err := src.Token() require.NoError(t, err) assert.Equal(t, "token-v1", tok1.AccessToken) // Update the file. Next call should read the new value. - writeFile(t, dir, oidcTokenFile, `{"access_token": "token-v2"}`) + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("token-v2", "")) tok2, err := src.Token() require.NoError(t, err) @@ -249,15 +243,13 @@ func TestDiskTokenSource_ReReadsOnEachCall(t *testing.T) { func TestDiskTokenSource_InvalidExpiryFormat(t *testing.T) { dir := t.TempDir() - writeFile(t, dir, oidcTokenFile, `{ - "access_token": "token-xyz", - "expiry": "not-a-date" - }`) + writeFile(t, dir, oidctoken.FileName, oidcBundleJSON("token-xyz", "-1")) - src := newDiskTokenSource(dir) + src := newDiskTokenSource(dir, "prod") _, err := src.Token() require.Error(t, err) assert.ErrorIs(t, err, ErrTokenLoad) + assert.Contains(t, err.Error(), "invalid JSON") } // writeFile is a test helper that writes a file with the given content. @@ -266,3 +258,14 @@ func writeFile(t *testing.T, dir, name, content string) { err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644) require.NoError(t, err) } + +// oidcBundleJSON returns a CLI-format oidc_token.json body. expiresAt is +// the raw JSON value of expires_at; an empty string omits the field. +func oidcBundleJSON(accessToken, expiresAt string) string { + expiry := "" + if expiresAt != "" { + expiry = `"expires_at": ` + expiresAt + `, ` + } + return `{"access_token": "` + accessToken + `", ` + expiry + + `"issuer": "https://issuer.example.com", "client_id": "openshell-cli"}` +} diff --git a/sdk/go/openshell/v1/internal/oidctoken/oidctoken.go b/sdk/go/openshell/v1/internal/oidctoken/oidctoken.go new file mode 100644 index 0000000000..79e7035958 --- /dev/null +++ b/sdk/go/openshell/v1/internal/oidctoken/oidctoken.go @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package oidctoken defines the on-disk OIDC token bundle shared with the +// OpenShell CLI. +package oidctoken + +import ( + "time" + + "golang.org/x/oauth2" +) + +// FileName is the OIDC token bundle in a gateway directory. +const FileName = "oidc_token.json" + +// Bundle is the CLI's on-disk OIDC token bundle (openshell-bootstrap OidcTokenBundle). +type Bundle struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token,omitempty"` + ExpiresAt *uint64 `json:"expires_at,omitempty"` // unix seconds + Issuer string `json:"issuer"` + ClientID string `json:"client_id"` +} + +// FromToken converts tok into the bundle stored for issuer and clientID. +func FromToken(tok *oauth2.Token, issuer, clientID string) Bundle { + b := Bundle{ + AccessToken: tok.AccessToken, + RefreshToken: tok.RefreshToken, + Issuer: issuer, + ClientID: clientID, + } + if !tok.Expiry.IsZero() { + expiresAt := uint64(max(tok.Expiry.Unix(), 0)) + b.ExpiresAt = &expiresAt + } + return b +} + +// Token converts b into a Bearer token. A nil ExpiresAt yields a zero +// Expiry, which oauth2 treats as never expiring. +func (b Bundle) Token() *oauth2.Token { + tok := &oauth2.Token{ + AccessToken: b.AccessToken, + RefreshToken: b.RefreshToken, + TokenType: "Bearer", + } + if b.ExpiresAt != nil { + tok.Expiry = time.Unix(int64(*b.ExpiresAt), 0) + } + return tok +} diff --git a/sdk/go/openshell/v1/oidc/doc.go b/sdk/go/openshell/v1/oidc/doc.go index 1e3c0ac5a4..a89d4f2206 100644 --- a/sdk/go/openshell/v1/oidc/doc.go +++ b/sdk/go/openshell/v1/oidc/doc.go @@ -21,8 +21,10 @@ // } // // After successful authentication, tokens are persisted to disk in the -// gateway directory as oidc_token.json, compatible with -// [gateway.NewClient] and the existing [gateway.diskTokenSource]. +// gateway directory as oidc_token.json, in the same format as +// `openshell gateway login`, so both [gateway.NewClient] and the CLI can +// use them. A later Login reuses the stored token only when it is valid +// and was issued for the gateway's issuer and client ID. // // # Standalone Login // diff --git a/sdk/go/openshell/v1/oidc/oidc.go b/sdk/go/openshell/v1/oidc/oidc.go index b80ae1ca8a..d16aafc4d6 100644 --- a/sdk/go/openshell/v1/oidc/oidc.go +++ b/sdk/go/openshell/v1/oidc/oidc.go @@ -13,6 +13,7 @@ import ( "golang.org/x/oauth2" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" ) @@ -20,14 +21,16 @@ import ( // // When gatewayName is non-empty, Login resolves OIDC configuration // (issuer URL and client ID) from the gateway's metadata.json file and -// persists tokens to the gateway directory. +// persists tokens to the gateway directory as oidc_token.json, in the +// same format as `openshell gateway login`. // // When gatewayName is empty, the caller must provide [WithIssuer] and // [WithClientID] options explicitly. // // Before starting an interactive flow, Login checks for an existing -// valid token on disk (FR-019). If a valid, non-expired token is found, -// it is returned immediately without user interaction. +// token on disk (FR-019). If the stored token is valid and was issued +// for the same issuer and client ID, it is returned immediately without +// user interaction. // // The flow attempts to open a browser for authorization. If the browser // cannot be opened, or if [WithKeyboardFlow] is set, the keyboard @@ -53,13 +56,16 @@ func Login(ctx context.Context, gatewayName string, opts ...LoginOption) (*oauth // FR-019: Check for existing valid token on disk before starting // an interactive flow. if tokenDir != "" { - tok, readErr := readToken(tokenDir) - if readErr == nil && tok != nil && tok.Valid() { - return tok, nil + stored, readErr := readToken(tokenDir) + if readErr == nil && stored != nil && + normalizeIssuer(stored.Issuer) == normalizeIssuer(cfg.issuer) && + stored.ClientID == cfg.clientID { + if tok := stored.Token(); tok.Valid() { + return tok, nil + } } - // If readErr is a non-NotExist error, we log and proceed. - // Stale/expired tokens or missing files are not errors; we - // simply proceed to the interactive flow. + // Unreadable, mismatched, or expired tokens and missing files + // are not errors; we simply proceed to the interactive flow. } // Run OIDC discovery to get provider endpoints. @@ -106,7 +112,7 @@ func Login(ctx context.Context, gatewayName string, opts ...LoginOption) (*oauth // Persist token to disk unless in-memory mode is requested. if !cfg.inMemory && tokenDir != "" { - if writeErr := writeToken(tokenDir, tok); writeErr != nil { + if writeErr := writeToken(tokenDir, oidctoken.FromToken(tok, cfg.issuer, cfg.clientID)); writeErr != nil { return nil, writeErr } } diff --git a/sdk/go/openshell/v1/oidc/oidc_test.go b/sdk/go/openshell/v1/oidc/oidc_test.go index 5e27ab9e9b..9bf733032a 100644 --- a/sdk/go/openshell/v1/oidc/oidc_test.go +++ b/sdk/go/openshell/v1/oidc/oidc_test.go @@ -22,6 +22,7 @@ import ( "golang.org/x/oauth2" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" ) // --- T021: Login entry point tests --- @@ -88,9 +89,9 @@ func TestLogin_MissingClientID(t *testing.T) { assert.True(t, errors.Is(err, ErrOIDCConfig), "expected ErrOIDCConfig, got: %v", err) } -// TestLogin_ReusesValidToken verifies FR-019: when a valid token exists -// on disk in the token directory, Login returns it without starting an -// interactive flow. +// TestLogin_ReusesValidToken verifies FR-019: when a valid token for the +// same issuer (ignoring a trailing slash) and client ID exists on disk, +// Login returns it without starting an interactive flow. func TestLogin_ReusesValidToken(t *testing.T) { resetDiscoveryCache() @@ -102,7 +103,8 @@ func TestLogin_ReusesValidToken(t *testing.T) { TokenType: "Bearer", Expiry: time.Now().Add(1 * time.Hour), } - err := writeToken(tokenDir, existingToken) + err := writeToken(tokenDir, oidctoken.FromToken(existingToken, + "https://issuer-should-not-be-called.example.com/", "test-client")) require.NoError(t, err) // Login with explicit issuer/clientID and WithTokenDir (internal) @@ -135,7 +137,7 @@ func TestLogin_ExpiredTokenTriggersFlow(t *testing.T) { TokenType: "Bearer", Expiry: time.Now().Add(-1 * time.Hour), // expired } - err := writeToken(tokenDir, expiredToken) + err := writeToken(tokenDir, oidctoken.FromToken(expiredToken, provider.URL, "test-client")) require.NoError(t, err) // Start a callback server ourselves to simulate the auth code callback. @@ -159,6 +161,51 @@ func TestLogin_ExpiredTokenTriggersFlow(t *testing.T) { assert.Equal(t, "login-refresh-token", tok.RefreshToken) } +// TestLogin_ReplacesMismatchedStoredToken verifies that a valid stored +// token issued for another client or issuer, or written by an earlier Go +// SDK build, is not reused: Login runs the flow and rewrites the file in +// the CLI format. +func TestLogin_ReplacesMismatchedStoredToken(t *testing.T) { + resetDiscoveryCache() + + provider := setupMockProvider(t) + expiresAt := fmt.Sprint(time.Now().Add(time.Hour).Unix()) + expiry := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) + stored := map[string]string{ + "different client ID": `{"access_token": "stored-token", "expires_at": ` + expiresAt + + `, "issuer": "` + provider.URL + `", "client_id": "other-client"}`, + "different issuer": `{"access_token": "stored-token", "expires_at": ` + expiresAt + + `, "issuer": "https://other-issuer.example.com", "client_id": "test-client"}`, + "earlier Go SDK format": `{"access_token": "stored-token", "refresh_token": "stored-refresh", "expiry": "` + + expiry + `", "expires_in": 3600}`, + } + + for name, content := range stored { + t.Run(name, func(t *testing.T) { + tokenDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tokenDir, oidctoken.FileName), []byte(content), 0o600)) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + tok, err := Login(ctx, "", + WithIssuer(provider.URL), + WithClientID("test-client"), + withTokenDir(tokenDir), + WithKeyboardFlow(), + withInput(strings.NewReader("auth-code\n")), + ) + require.NoError(t, err) + assert.Equal(t, "login-access-token", tok.AccessToken) + + diskTok, err := readToken(tokenDir) + require.NoError(t, err) + require.NotNil(t, diskTok) + assert.Equal(t, "login-access-token", diskTok.AccessToken) + }) + } +} + // TestLogin_KeyboardFlow verifies that Login completes using the // keyboard flow when WithKeyboardFlow() is set. The test provides a // mock OIDC provider and feeds an auth code through a reader. @@ -215,7 +262,7 @@ func TestLogin_InMemorySkipsPersistence(t *testing.T) { assert.Equal(t, "login-access-token", tok.AccessToken) // Verify NO token file on disk. - _, err = os.Stat(filepath.Join(tokenDir, oidcTokenFile)) + _, err = os.Stat(filepath.Join(tokenDir, oidctoken.FileName)) assert.True(t, os.IsNotExist(err), "token file should not exist in in-memory mode") } @@ -275,6 +322,8 @@ func TestLogin_GatewayResolution(t *testing.T) { require.NoError(t, err) require.NotNil(t, diskTok) assert.Equal(t, "login-access-token", diskTok.AccessToken) + assert.Equal(t, provider.URL, diskTok.Issuer) + assert.Equal(t, "gateway-client-id", diskTok.ClientID) } // TestLogin_GatewayMissingOIDCFields verifies that Login returns diff --git a/sdk/go/openshell/v1/oidc/token.go b/sdk/go/openshell/v1/oidc/token.go index 7ae368c6d3..7dd71cba6a 100644 --- a/sdk/go/openshell/v1/oidc/token.go +++ b/sdk/go/openshell/v1/oidc/token.go @@ -8,54 +8,20 @@ import ( "fmt" "os" "path/filepath" - "time" - "golang.org/x/oauth2" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" ) -// oidcTokenFile is the filename for persisted OIDC tokens. This must -// match the constant in gateway/token.go for interop. -const oidcTokenFile = "oidc_token.json" - -// tokenExpiryLeeway is the grace period subtracted from the token -// expiry when checking validity. Tokens expiring within this window -// are treated as expired to avoid using a token that expires during -// an in-flight request. -const tokenExpiryLeeway = 10 * time.Second - -// oidcBundle is the on-disk JSON representation of an OIDC token. -// The format is shared with the Rust CLI and the gateway package's -// diskTokenSource for interop. -type oidcBundle struct { - AccessToken string `json:"access_token"` - RefreshToken string `json:"refresh_token"` - Expiry string `json:"expiry"` - ExpiresIn int64 `json:"expires_in"` -} - -// writeToken persists an oauth2.Token to disk as oidc_token.json in -// the given directory. The file is written with 0600 permissions -// (owner-only) to protect credentials. -func writeToken(dir string, tok *oauth2.Token) error { - bundle := oidcBundle{ - AccessToken: tok.AccessToken, - RefreshToken: tok.RefreshToken, - } - - if !tok.Expiry.IsZero() { - bundle.Expiry = tok.Expiry.UTC().Format(time.RFC3339) - remaining := time.Until(tok.Expiry) - if remaining > 0 { - bundle.ExpiresIn = int64(remaining.Seconds()) - } - } - - data, err := json.Marshal(bundle) +// writeToken persists an OIDC token bundle to disk as oidc_token.json in +// the given directory, in the CLI's format. The file is written with 0600 +// permissions (owner-only) to protect credentials. +func writeToken(dir string, b oidctoken.Bundle) error { + data, err := json.Marshal(b) if err != nil { return fmt.Errorf("%w: failed to marshal token: %v", ErrTokenPersist, err) } - path := filepath.Join(dir, oidcTokenFile) + path := filepath.Join(dir, oidctoken.FileName) tmp, err := os.CreateTemp(dir, ".oidc-token-*") if err != nil { return fmt.Errorf("%w: failed to create temporary token file: %v", ErrTokenPersist, err) @@ -86,13 +52,11 @@ func writeToken(dir string, tok *oauth2.Token) error { // readToken reads an existing oidc_token.json from the given // directory. It returns: -// - (token, nil) if the file exists, is valid, and the token has not -// expired (with leeway) -// - (nil, nil) if the file does not exist, the token is expired, or -// the access token is empty (not an error, just no reusable token) -// - (nil, error) if the file exists but cannot be parsed -func readToken(dir string) (*oauth2.Token, error) { - path := filepath.Join(dir, oidcTokenFile) +// - (bundle, nil) if the file exists and parses +// - (nil, nil) if the file does not exist +// - (nil, error) if the file exists but cannot be read or parsed +func readToken(dir string) (*oidctoken.Bundle, error) { + path := filepath.Join(dir, oidctoken.FileName) data, err := os.ReadFile(path) if err != nil { @@ -102,37 +66,10 @@ func readToken(dir string) (*oauth2.Token, error) { return nil, fmt.Errorf("%w: cannot read %s: %v", ErrTokenPersist, path, err) } - var bundle oidcBundle + var bundle oidctoken.Bundle if err := json.Unmarshal(data, &bundle); err != nil { - return nil, fmt.Errorf("%w: invalid JSON in %s: %v", ErrTokenPersist, oidcTokenFile, err) - } - - if bundle.AccessToken == "" { - return nil, nil - } - - tok := &oauth2.Token{ - AccessToken: bundle.AccessToken, - RefreshToken: bundle.RefreshToken, - TokenType: "Bearer", - } - - // Parse expiry from the "expiry" field (RFC 3339). Without an - // explicit expiry, the token is treated as non-expiring (always - // valid); "expires_in" alone cannot reconstruct an absolute time - // without a write timestamp. - if bundle.Expiry != "" { - expiry, parseErr := time.Parse(time.RFC3339, bundle.Expiry) - if parseErr != nil { - return nil, fmt.Errorf("%w: invalid expiry format in %s: %v", ErrTokenPersist, oidcTokenFile, parseErr) - } - tok.Expiry = expiry - } - - // Check if the token has expired (with leeway). - if !tok.Expiry.IsZero() && time.Now().After(tok.Expiry.Add(-tokenExpiryLeeway)) { - return nil, nil // Expired; caller should re-authenticate. + return nil, fmt.Errorf("%w: invalid JSON in %s: %v", ErrTokenPersist, oidctoken.FileName, err) } - return tok, nil + return &bundle, nil } diff --git a/sdk/go/openshell/v1/oidc/token_test.go b/sdk/go/openshell/v1/oidc/token_test.go index cff24260af..6558a038bf 100644 --- a/sdk/go/openshell/v1/oidc/token_test.go +++ b/sdk/go/openshell/v1/oidc/token_test.go @@ -14,81 +14,76 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/oidctoken" ) -func TestWriteToken_Success(t *testing.T) { +func loadOIDCTokenFixture(t *testing.T) []byte { + t.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "..", "..", "conformance", "oidc-token-bundle.json")) + require.NoError(t, err) + return data +} + +func TestReadToken_ConformanceFixture(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, oidctoken.FileName), loadOIDCTokenFixture(t), 0o600)) + + b, err := readToken(dir) + require.NoError(t, err) + require.NotNil(t, b) + assert.Equal(t, "conformance-access-token", b.AccessToken) + assert.Equal(t, "conformance-refresh-token", b.RefreshToken) + require.NotNil(t, b.ExpiresAt) + assert.Equal(t, uint64(4102444800), *b.ExpiresAt) + assert.Equal(t, "https://issuer.example.com", b.Issuer) + assert.Equal(t, "openshell-cli", b.ClientID) +} + +func TestWriteToken_MatchesConformanceFixture(t *testing.T) { dir := t.TempDir() tok := &oauth2.Token{ - AccessToken: "access-123", - RefreshToken: "refresh-456", - Expiry: time.Date(2026, 7, 3, 12, 0, 0, 0, time.UTC), + AccessToken: "conformance-access-token", + RefreshToken: "conformance-refresh-token", + Expiry: time.Unix(4102444800, 0), } - err := writeToken(dir, tok) - require.NoError(t, err) + require.NoError(t, writeToken(dir, oidctoken.FromToken(tok, "https://issuer.example.com", "openshell-cli"))) - // Verify the file was written. - data, err := os.ReadFile(filepath.Join(dir, "oidc_token.json")) + data, err := os.ReadFile(filepath.Join(dir, oidctoken.FileName)) require.NoError(t, err) - assert.Contains(t, string(data), `"access_token":"access-123"`) - assert.Contains(t, string(data), `"refresh_token":"refresh-456"`) - assert.Contains(t, string(data), `"expiry":"2026-07-03T12:00:00Z"`) + assert.JSONEq(t, string(loadOIDCTokenFixture(t)), string(data)) } -func TestWriteToken_ReplacesInsecureExistingFileWithOwnerOnlyFile(t *testing.T) { +func TestWriteToken_OmitsEmptyOptionalFields(t *testing.T) { dir := t.TempDir() - path := filepath.Join(dir, oidcTokenFile) - require.NoError(t, os.WriteFile(path, []byte("old"), 0o644)) + tok := &oauth2.Token{AccessToken: "access-only"} - require.NoError(t, writeToken(dir, &oauth2.Token{AccessToken: "secret"})) + require.NoError(t, writeToken(dir, oidctoken.FromToken(tok, "https://issuer.example.com", "openshell-cli"))) - assertOwnerOnlyFilePermissions(t, path) + data, err := os.ReadFile(filepath.Join(dir, oidctoken.FileName)) + require.NoError(t, err) + assert.JSONEq(t, + `{"access_token": "access-only", "issuer": "https://issuer.example.com", "client_id": "openshell-cli"}`, + string(data)) } -func TestWriteToken_ExpiresInCalculated(t *testing.T) { +func TestWriteToken_ReplacesInsecureExistingFileWithOwnerOnlyFile(t *testing.T) { dir := t.TempDir() - expiry := time.Now().Add(3600 * time.Second) - tok := &oauth2.Token{ - AccessToken: "access-123", - Expiry: expiry, - } + path := filepath.Join(dir, oidctoken.FileName) + require.NoError(t, os.WriteFile(path, []byte("old"), 0o644)) - err := writeToken(dir, tok) - require.NoError(t, err) + require.NoError(t, writeToken(dir, oidctoken.Bundle{AccessToken: "secret"})) - data, err := os.ReadFile(filepath.Join(dir, "oidc_token.json")) - require.NoError(t, err) - // expires_in should be roughly 3600 (within a few seconds). - assert.Contains(t, string(data), `"expires_in":`) + assertOwnerOnlyFilePermissions(t, path) } func TestWriteToken_InvalidDirectory(t *testing.T) { - err := writeToken("/nonexistent/path/that/does/not/exist", &oauth2.Token{ - AccessToken: "test", - Expiry: time.Now().Add(time.Hour), - }) + err := writeToken("/nonexistent/path/that/does/not/exist", oidctoken.Bundle{AccessToken: "test"}) require.Error(t, err) assert.True(t, errors.Is(err, ErrTokenPersist)) } -func TestReadToken_Success(t *testing.T) { - dir := t.TempDir() - content := `{ - "access_token": "access-123", - "refresh_token": "refresh-456", - "expiry": "2099-07-03T12:00:00Z", - "expires_in": 3600 - }` - err := os.WriteFile(filepath.Join(dir, "oidc_token.json"), []byte(content), 0o600) - require.NoError(t, err) - - tok, err := readToken(dir) - require.NoError(t, err) - assert.Equal(t, "access-123", tok.AccessToken) - assert.Equal(t, "refresh-456", tok.RefreshToken) - assert.False(t, tok.Expiry.IsZero()) -} - func TestReadToken_MissingFile(t *testing.T) { dir := t.TempDir() @@ -107,83 +102,10 @@ func TestReadToken_InvalidJSON(t *testing.T) { assert.True(t, errors.Is(err, ErrTokenPersist)) } -func TestReadToken_ExpiredToken(t *testing.T) { - dir := t.TempDir() - content := `{ - "access_token": "expired-access", - "expiry": "2020-01-01T00:00:00Z" - }` - err := os.WriteFile(filepath.Join(dir, "oidc_token.json"), []byte(content), 0o600) - require.NoError(t, err) - - tok, err := readToken(dir) - assert.Nil(t, tok, "expired token should return nil") - assert.NoError(t, err, "expired token is not an error, just nil") -} - -func TestReadToken_ValidWithExpiresInFallback(t *testing.T) { - dir := t.TempDir() - // No expiry field, only expires_in. Since we wrote it "now", - // a large expires_in should make the token valid. - content := `{ - "access_token": "access-via-expires-in", - "expires_in": 99999 - }` - err := os.WriteFile(filepath.Join(dir, "oidc_token.json"), []byte(content), 0o600) - require.NoError(t, err) - - tok, err := readToken(dir) - require.NoError(t, err) - // Token with only expires_in cannot reconstruct a valid Expiry - // without knowing when the file was written. readToken should - // treat it as potentially valid and return it. - assert.NotNil(t, tok) - assert.Equal(t, "access-via-expires-in", tok.AccessToken) -} - -func TestReadToken_EmptyAccessToken(t *testing.T) { - dir := t.TempDir() - content := `{ - "access_token": "", - "expiry": "2099-01-01T00:00:00Z" - }` - err := os.WriteFile(filepath.Join(dir, "oidc_token.json"), []byte(content), 0o600) - require.NoError(t, err) - - tok, err := readToken(dir) - assert.Nil(t, tok, "empty access token should return nil") - assert.NoError(t, err) -} - -func TestWriteAndReadToken_Roundtrip(t *testing.T) { - dir := t.TempDir() - original := &oauth2.Token{ - AccessToken: "roundtrip-access", - RefreshToken: "roundtrip-refresh", - Expiry: time.Now().Add(time.Hour).Truncate(time.Second), - } - - err := writeToken(dir, original) - require.NoError(t, err) - - loaded, err := readToken(dir) - require.NoError(t, err) - require.NotNil(t, loaded) - - assert.Equal(t, original.AccessToken, loaded.AccessToken) - assert.Equal(t, original.RefreshToken, loaded.RefreshToken) - // Expiry should be close (within a second due to serialization). - assert.WithinDuration(t, original.Expiry, loaded.Expiry, time.Second) -} - func TestWriteToken_FilePermissions(t *testing.T) { dir := t.TempDir() - tok := &oauth2.Token{ - AccessToken: "perm-check", - Expiry: time.Now().Add(time.Hour), - } - err := writeToken(dir, tok) + err := writeToken(dir, oidctoken.Bundle{AccessToken: "perm-check"}) require.NoError(t, err) assertOwnerOnlyFilePermissions(t, filepath.Join(dir, "oidc_token.json"))