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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions otdfctl/cmd/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
5 changes: 4 additions & 1 deletion otdfctl/cmd/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
23 changes: 20 additions & 3 deletions otdfctl/pkg/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
Comment thread
elizabethhealy marked this conversation as resolved.
default:
return ErrInvalidAuthType
}
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions otdfctl/pkg/auth/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
21 changes: 20 additions & 1 deletion otdfctl/pkg/auth/refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment thread
elizabethhealy marked this conversation as resolved.
return fmt.Errorf("%w: %w", ErrRefreshFailed, err)
}

Expand All @@ -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,
},
}
Expand Down Expand Up @@ -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"
}
81 changes: 79 additions & 2 deletions otdfctl/pkg/auth/refresh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}))
Expand All @@ -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) {
Expand Down
157 changes: 157 additions & 0 deletions otdfctl/pkg/auth/token_source.go
Original file line number Diff line number Diff line change
@@ -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
}
Comment thread
elizabethhealy marked this conversation as resolved.

// 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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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
}
Loading
Loading