diff --git a/otdfctl/cmd/auth/login.go b/otdfctl/cmd/auth/login.go index d1618ebba8..e37ba35b93 100644 --- a/otdfctl/cmd/auth/login.go +++ b/otdfctl/cmd/auth/login.go @@ -39,6 +39,7 @@ func codeLogin(cmd *cobra.Command, args []string) { }); err != nil { c.ExitWithError("failed to set auth credentials", err) } + c.ExitWithMessage(fmt.Sprintf("Code login complete for profile: [%s]", cp.Name()), cli.ExitCodeSuccess) } diff --git a/otdfctl/cmd/common/common.go b/otdfctl/cmd/common/common.go index c6a94594cc..075d0155e9 100644 --- a/otdfctl/cmd/common/common.go +++ b/otdfctl/cmd/common/common.go @@ -207,8 +207,11 @@ func NewHandler(c *cli.Cli, hooks ...handlers.Hook) handlers.Handler { cli.ExitWithWarning("Profile missing credentials. Please login or add client credentials.") } + if errors.Is(err, auth.ErrRefreshTokenInvalid) { + cli.ExitWithWarning("Your session has expired. Please login again.") + } if errors.Is(err, auth.ErrAccessTokenExpired) { - cli.ExitWithWarning("Access token expired. Please login or add flag-provided credentials.") + cli.ExitWithWarning("Access token expired and could not be refreshed. Please login or add flag-provided credentials.") } if errors.Is(err, auth.ErrAccessTokenNotFound) { cli.ExitWithWarning("No access token found. Please login or add flag-provided credentials.") diff --git a/otdfctl/pkg/auth/auth.go b/otdfctl/pkg/auth/auth.go index bd505d88ab..d055afe243 100644 --- a/otdfctl/pkg/auth/auth.go +++ b/otdfctl/pkg/auth/auth.go @@ -170,8 +170,7 @@ func GetSDKAuthOptionFromProfile(profile *profiles.OtdfctlProfileStore) (sdk.Opt case profiles.AuthTypeClientCredentials: return sdk.WithClientCredentials(c.ClientID, c.ClientSecret, NormalizeScopes(c.Scopes)), nil case profiles.AuthTypeAccessToken: - tokenSource := oauth2.StaticTokenSource(buildToken(&c)) - return sdk.WithOAuthAccessTokenSource(tokenSource), nil + return sdk.WithOAuthAccessTokenSource(newProfileTokenSource(profile)), nil default: return nil, ErrInvalidAuthType } @@ -190,9 +189,18 @@ func ValidateProfileAuthCredentials(ctx context.Context, profile *profiles.Otdfc } return nil case profiles.AuthTypeAccessToken: - if !buildToken(&c).Valid() { + if buildToken(&c).Valid() { + return nil + } + if !HasRefreshToken(profile) { return ErrAccessTokenExpired } + if err := RefreshAccessToken(ctx, profile); err != nil { + if errors.Is(err, ErrRefreshTokenInvalid) { + return err + } + return errors.Join(ErrAccessTokenExpired, err) + } default: return ErrInvalidAuthType } @@ -206,6 +214,15 @@ func GetTokenWithProfile(ctx context.Context, profile *profiles.OtdfctlProfileSt case profiles.AuthTypeClientCredentials: return GetTokenWithClientCreds(ctx, profile.GetEndpoint(), c.ClientID, c.ClientSecret, profile.GetTLSNoVerify(), c.Scopes) case profiles.AuthTypeAccessToken: + if !buildToken(&c).Valid() { + if !HasRefreshToken(profile) { + return nil, ErrAccessTokenExpired + } + if err := RefreshAccessToken(ctx, profile); err != nil { + return nil, err + } + c = profile.GetAuthCredentials() + } return buildToken(&c), nil default: return nil, ErrInvalidAuthType diff --git a/otdfctl/pkg/auth/errors.go b/otdfctl/pkg/auth/errors.go index b074779795..760be84b87 100644 --- a/otdfctl/pkg/auth/errors.go +++ b/otdfctl/pkg/auth/errors.go @@ -12,4 +12,5 @@ var ( ErrProfileCredentialsNotFound = errors.New("profile missing credentials") ErrNoRefreshToken = errors.New("no refresh token available") ErrRefreshFailed = errors.New("token refresh failed") + ErrRefreshTokenInvalid = errors.New("refresh token is invalid or expired; please re-login") ) diff --git a/otdfctl/pkg/auth/refresh.go b/otdfctl/pkg/auth/refresh.go index 8965dbcb4a..46df71600b 100644 --- a/otdfctl/pkg/auth/refresh.go +++ b/otdfctl/pkg/auth/refresh.go @@ -82,6 +82,12 @@ func refreshAccessToken(ctx context.Context, profile *profiles.OtdfctlProfileSto tokenSource := oauth2Config.TokenSource(ctx, oldToken) newToken, err := tokenSource.Token() if err != nil { + if isInvalidGrant(err) { + // Refresh token is dead server-side; wipe stored creds so the next + // command hits the login prompt cleanly. + _ = profile.SetAuthCredentials(profiles.AuthCredentials{}) + return errors.Join(ErrRefreshTokenInvalid, err) + } return fmt.Errorf("%w: %w", ErrRefreshFailed, err) } @@ -93,12 +99,18 @@ func refreshAccessToken(ctx context.Context, profile *profiles.OtdfctlProfileSto slog.Warn("token response missing expires_in, assuming 1 hour") } + // Some IdPs omit refresh_token on refresh (no-rotation); keep the old one. + refreshToken := newToken.RefreshToken + if refreshToken == "" { + refreshToken = creds.AccessToken.RefreshToken + } + newCreds := profiles.AuthCredentials{ AuthType: profiles.AuthTypeAccessToken, AccessToken: profiles.AuthCredentialsAccessToken{ ClientID: clientID, AccessToken: newToken.AccessToken, - RefreshToken: newToken.RefreshToken, + RefreshToken: refreshToken, Expiration: expiration, }, } @@ -143,3 +155,10 @@ func getTokenEndpoint(endpoint string, tlsNoVerify bool) (string, error) { } return pc.tokenEndpoint, nil } + +// isInvalidGrant reports whether err is an oauth2 token-endpoint error carrying +// RFC 6749 §5.2 code "invalid_grant" — i.e. the refresh token is expired or revoked. +func isInvalidGrant(err error) bool { + var re *oauth2.RetrieveError + return errors.As(err, &re) && re.ErrorCode == "invalid_grant" +} diff --git a/otdfctl/pkg/auth/refresh_test.go b/otdfctl/pkg/auth/refresh_test.go index b7e4c9c0af..0666ed05ab 100644 --- a/otdfctl/pkg/auth/refresh_test.go +++ b/otdfctl/pkg/auth/refresh_test.go @@ -189,9 +189,10 @@ func TestRefreshAccessTokenEndpointError(t *testing.T) { require.ErrorContains(t, err, "failed to get token endpoint") } -func TestRefreshAccessTokenRefreshFails(t *testing.T) { +func TestRefreshAccessTokenInvalidGrant(t *testing.T) { tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusUnauthorized) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) err := json.NewEncoder(w).Encode(map[string]string{"error": "invalid_grant"}) assert.NoError(t, err) })) @@ -201,10 +202,86 @@ func TestRefreshAccessTokenRefreshFails(t *testing.T) { return tokenServer.URL, nil } + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "tok", "refresh", time.Now().Add(-time.Hour).Unix()) + err := refreshAccessToken(t.Context(), profile, resolver) + require.Error(t, err) + require.ErrorIs(t, err, ErrRefreshTokenInvalid) + + // Creds should be wiped so the next command re-prompts login. + creds := profile.GetAuthCredentials() + assert.Empty(t, creds.AuthType) + assert.Empty(t, creds.AccessToken.RefreshToken) +} + +func TestRefreshAccessTokenGenericFailure(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer tokenServer.Close() + + resolver := func(string, bool) (string, error) { + return tokenServer.URL, nil + } + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "tok", "refresh", time.Now().Add(-time.Hour).Unix()) err := refreshAccessToken(t.Context(), profile, resolver) require.Error(t, err) require.ErrorIs(t, err, ErrRefreshFailed) + // Creds should NOT be wiped on transient failures. + creds := profile.GetAuthCredentials() + assert.Equal(t, "refresh", creds.AccessToken.RefreshToken) +} + +func TestRefreshAccessTokenNoRotationKeepsOldRefreshToken(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(map[string]any{ + "access_token": "new-access", + "token_type": "Bearer", + "expires_in": 3600, + }) + assert.NoError(t, err) + })) + defer tokenServer.Close() + + resolver := func(string, bool) (string, error) { + return tokenServer.URL, nil + } + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "old-access", "original-refresh", time.Now().Add(-time.Hour).Unix()) + err := refreshAccessToken(t.Context(), profile, resolver) + require.NoError(t, err) + + creds := profile.GetAuthCredentials() + assert.Equal(t, "new-access", creds.AccessToken.AccessToken) + assert.Equal(t, "original-refresh", creds.AccessToken.RefreshToken, + "when IdP omits refresh_token, the existing one must be preserved") +} + +func TestRefreshAccessTokenRotationStoresNewRefreshToken(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(map[string]any{ + "access_token": "new-access", + "refresh_token": "rotated-refresh", + "token_type": "Bearer", + "expires_in": 3600, + }) + assert.NoError(t, err) + })) + defer tokenServer.Close() + + resolver := func(string, bool) (string, error) { + return tokenServer.URL, nil + } + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "old-access", "original-refresh", time.Now().Add(-time.Hour).Unix()) + err := refreshAccessToken(t.Context(), profile, resolver) + require.NoError(t, err) + + creds := profile.GetAuthCredentials() + assert.Equal(t, "rotated-refresh", creds.AccessToken.RefreshToken, + "when IdP returns a new refresh_token, it must replace the old one") } func TestRefreshAccessTokenEmptyClientID(t *testing.T) { diff --git a/otdfctl/pkg/auth/token_source.go b/otdfctl/pkg/auth/token_source.go new file mode 100644 index 0000000000..1d76a1948a --- /dev/null +++ b/otdfctl/pkg/auth/token_source.go @@ -0,0 +1,157 @@ +package auth + +import ( + "context" + "errors" + "log/slog" + "sync" + "time" + + "github.com/opentdf/platform/otdfctl/pkg/profiles" + "github.com/opentdf/platform/otdfctl/pkg/utils" + "golang.org/x/oauth2" +) + +// profileTokenSource is an oauth2.TokenSource that transparently refreshes +// expired access tokens via the stored refresh token and persists the rotated +// credentials back to the profile. It is safe for concurrent use. +type profileTokenSource struct { + profile *profiles.OtdfctlProfileStore + resolve tokenEndpointResolver + + mu sync.Mutex + inner oauth2.TokenSource + cachedAccess string + forceRefresh bool +} + +func newProfileTokenSource(profile *profiles.OtdfctlProfileStore) *profileTokenSource { + return &profileTokenSource{ + profile: profile, + resolve: getTokenEndpoint, + } +} + +// Token returns a valid access token, refreshing via the stored refresh token +// when needed and persisting rotated credentials to the profile. +func (p *profileTokenSource) Token() (*oauth2.Token, error) { + p.mu.Lock() + defer p.mu.Unlock() + + creds := p.profile.GetAuthCredentials() + if creds.AuthType != profiles.AuthTypeAccessToken { + return nil, ErrInvalidAuthType + } + + if p.inner == nil { + if err := p.rebuild(creds); err != nil { + return nil, err + } + } + + tok, err := p.inner.Token() + if err != nil { + if isInvalidGrant(err) { + _ = p.profile.SetAuthCredentials(profiles.AuthCredentials{}) + p.inner = nil + return nil, errors.Join(ErrRefreshTokenInvalid, err) + } + slog.Warn("token source refresh failed", slog.Any("error", err)) + return nil, err + } + + if tok.AccessToken != p.cachedAccess { + if p.persist(creds, tok) { + p.cachedAccess = tok.AccessToken + } + } + return tok, nil +} + +// Invalidate drops the cached inner source so the next Token() call rebuilds +// from the current profile creds and forces a refresh. Used by callers that +// know the server has rejected the current access token (e.g. a 401 retry). +func (p *profileTokenSource) Invalidate() { + p.mu.Lock() + defer p.mu.Unlock() + p.inner = nil + p.forceRefresh = true +} + +// rebuild constructs the inner oauth2.TokenSource from the profile's creds. +// Caller must hold p.mu. +func (p *profileTokenSource) rebuild(creds profiles.AuthCredentials) error { + endpoint := p.profile.GetEndpoint() + tlsNoVerify := p.profile.GetTLSNoVerify() + + normalized, err := utils.NormalizeEndpoint(endpoint) + if err != nil { + return err + } + tokenEndpoint, err := p.resolve(normalized.String(), tlsNoVerify) + if err != nil { + return err + } + + clientID := creds.AccessToken.ClientID + if clientID == "" { + clientID = DefaultPublicClientID + } + + cfg := &oauth2.Config{ + ClientID: clientID, + Endpoint: oauth2.Endpoint{TokenURL: tokenEndpoint}, + } + + ctx := context.Background() + if tlsNoVerify { + ctx = context.WithValue(ctx, oauth2.HTTPClient, utils.NewHTTPClient(tlsNoVerify)) + } + + oldToken := buildToken(&creds) + if p.forceRefresh { + oldToken.AccessToken = "" + oldToken.Expiry = time.Time{} + p.forceRefresh = false + } + p.inner = cfg.TokenSource(ctx, oldToken) + p.cachedAccess = oldToken.AccessToken + return nil +} + +// persist writes the rotated token back to the profile. Some IdPs omit +// refresh_token on refresh (RFC-compliant no-rotation); keep the old one in +// that case. A write-back failure is logged but not returned — the fresh +// token is still valid for the in-flight call. +func (p *profileTokenSource) persist(oldCreds profiles.AuthCredentials, tok *oauth2.Token) bool { + refresh := tok.RefreshToken + if refresh == "" { + refresh = oldCreds.AccessToken.RefreshToken + } + expiry := tok.Expiry.Unix() + if tok.Expiry.IsZero() { + expiry = time.Now().Add(time.Hour).Unix() + slog.Warn("token response missing expires_in, assuming 1 hour") + } + + clientID := oldCreds.AccessToken.ClientID + if clientID == "" { + clientID = DefaultPublicClientID + } + + newCreds := profiles.AuthCredentials{ + AuthType: profiles.AuthTypeAccessToken, + AccessToken: profiles.AuthCredentialsAccessToken{ + ClientID: clientID, + AccessToken: tok.AccessToken, + RefreshToken: refresh, + Expiration: expiry, + }, + } + if err := p.profile.SetAuthCredentials(newCreds); err != nil { + slog.Warn("failed to persist refreshed credentials", slog.Any("error", err)) + return false + } + slog.Info("access token refreshed", slog.String("profile", p.profile.Name())) + return true +} diff --git a/otdfctl/pkg/auth/token_source_test.go b/otdfctl/pkg/auth/token_source_test.go new file mode 100644 index 0000000000..59bfa0f2a4 --- /dev/null +++ b/otdfctl/pkg/auth/token_source_test.go @@ -0,0 +1,203 @@ +package auth + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/opentdf/platform/otdfctl/pkg/profiles" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newTokenSourceForTest builds a profileTokenSource with the resolver pointed +// at the given httptest server URL. +func newTokenSourceForTest(profile *profiles.OtdfctlProfileStore, tokenURL string) *profileTokenSource { + ts := newProfileTokenSource(profile) + ts.resolve = func(string, bool) (string, error) { return tokenURL, nil } + return ts +} + +func TestProfileTokenSource_ValidTokenNoRefresh(t *testing.T) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "current-access", "refresh", time.Now().Add(time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + tok, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "current-access", tok.AccessToken) + assert.Equal(t, int32(0), hits.Load(), "no network call expected for a valid token") +} + +func TestProfileTokenSource_ExpiredRefreshesAndPersists(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "fresh-access", + "refresh_token": "rotated-refresh", + "token_type": "Bearer", + "expires_in": 3600, + })) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "stale", "old-refresh", time.Now().Add(-time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + tok, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "fresh-access", tok.AccessToken) + + creds := profile.GetAuthCredentials() + assert.Equal(t, "fresh-access", creds.AccessToken.AccessToken) + assert.Equal(t, "rotated-refresh", creds.AccessToken.RefreshToken) + assert.Greater(t, creds.AccessToken.Expiration, time.Now().Unix()) +} + +func TestProfileTokenSource_NoRotationKeepsOldRefresh(t *testing.T) { + // RFC-compliant IdP that does not rotate refresh tokens omits the field. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "fresh-access", + "token_type": "Bearer", + "expires_in": 3600, + })) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "stale", "keep-me", time.Now().Add(-time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + _, err := ts.Token() + require.NoError(t, err) + + creds := profile.GetAuthCredentials() + assert.Equal(t, "keep-me", creds.AccessToken.RefreshToken) +} + +func TestProfileTokenSource_ConcurrentRefreshSingleFlights(t *testing.T) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "shared-fresh", + "refresh_token": "shared-refresh", + "token_type": "Bearer", + "expires_in": 3600, + })) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "stale", "refresh", time.Now().Add(-time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + const workers = 20 + var wg sync.WaitGroup + wg.Add(workers) + for range workers { + go func() { + defer wg.Done() + tok, err := ts.Token() + assert.NoError(t, err) + assert.Equal(t, "shared-fresh", tok.AccessToken) + }() + } + wg.Wait() + + assert.Equal(t, int32(1), hits.Load(), "concurrent Token() calls must single-flight the refresh") +} + +func TestProfileTokenSource_InvalidGrantClearsCreds(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + assert.NoError(t, json.NewEncoder(w).Encode(map[string]string{"error": "invalid_grant"})) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "stale", "refresh", time.Now().Add(-time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + _, err := ts.Token() + require.Error(t, err) + require.ErrorIs(t, err, ErrRefreshTokenInvalid) + + creds := profile.GetAuthCredentials() + assert.Empty(t, creds.AuthType) + assert.Empty(t, creds.AccessToken.RefreshToken) +} + +func TestProfileTokenSource_InvalidateForcesRefresh(t *testing.T) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "post-invalidate", + "refresh_token": "post-invalidate-refresh", + "token_type": "Bearer", + "expires_in": 3600, + })) + })) + defer server.Close() + + // Start with a valid, non-expired token so the first Token() does not refresh. + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "current", "refresh", time.Now().Add(time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + tok, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "current", tok.AccessToken) + require.Equal(t, int32(0), hits.Load()) + + // Server has revoked the token; Invalidate() forces the next call to refresh + // even though the stored token hasn't expired yet. + ts.Invalidate() + + tok, err = ts.Token() + require.NoError(t, err) + assert.Equal(t, "post-invalidate", tok.AccessToken) + assert.Equal(t, int32(1), hits.Load()) +} + +func TestProfileTokenSource_ZeroExpiryFallback(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "access_token": "fresh", + "refresh_token": "fresh-refresh", + "token_type": "Bearer", + })) + })) + defer server.Close() + + profile := newTestProfile(t, profiles.AuthTypeAccessToken, "stale", "refresh", time.Now().Add(-time.Hour).Unix()) + ts := newTokenSourceForTest(profile, server.URL) + + _, err := ts.Token() + require.NoError(t, err) + + creds := profile.GetAuthCredentials() + assert.Greater(t, creds.AccessToken.Expiration, time.Now().Unix(), + "zero expiry in response should fall back to ~1 hour") +} + +func TestProfileTokenSource_WrongAuthType(t *testing.T) { + profile := newTestProfile(t, profiles.AuthTypeClientCredentials, "tok", "refresh", time.Now().Add(time.Hour).Unix()) + ts := newTokenSourceForTest(profile, "http://ignored") + + _, err := ts.Token() + require.ErrorIs(t, err, ErrInvalidAuthType) +}