diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dbb23d57..0d7634504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Bug Fixes: - fix(compute/deploy): remove compute trial activation code because trials no longer exist ([#1730](https://github.com/fastly/cli/pull/1730)) +- fix(auth): SSO token expiration status now reflects the actual API token lifetime (~12 hours) instead of the internal JWT refresh token (~30 minutes), preventing spurious warnings and premature re-authentication [#1728](https://github.com/fastly/cli/pull/1728) ### Enhancements: diff --git a/pkg/app/expiry_warning_test.go b/pkg/app/expiry_warning_test.go index 3dc80f3a7..6c86cd6ea 100644 --- a/pkg/app/expiry_warning_test.go +++ b/pkg/app/expiry_warning_test.go @@ -16,7 +16,7 @@ import ( // expiringTokenData returns a global.Data configured with a stored token that // expires soon. Callers can override Flags and commandName to test suppression. func expiringTokenData(out *bytes.Buffer) *global.Data { - soon := time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + soon := time.Now().Add(20 * time.Minute).Format(time.RFC3339) return &global.Data{ Output: out, ErrOutput: out, @@ -38,7 +38,7 @@ func expiringTokenData(out *bytes.Buffer) *global.Data { } func TestCheckTokenExpirationWarning(t *testing.T) { - soon := time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + soon := time.Now().Add(20 * time.Minute).Format(time.RFC3339) farFuture := time.Now().Add(60 * 24 * time.Hour).Format(time.RFC3339) tests := []struct { diff --git a/pkg/app/run.go b/pkg/app/run.go index 8d140a58e..0707b0662 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -457,7 +457,13 @@ func checkAndRefreshAuthSSOToken(name string, at *config.AuthToken, data *global return false, fmt.Errorf("invalid refresh_expires_at %q: %w", at.RefreshExpiresAt, err) } if time.Now().After(refreshExpires) { - return true, nil // both expired, needs full re-auth + if at.APITokenExpiresAt != "" { + apiExpires, err := time.Parse(time.RFC3339, at.APITokenExpiresAt) + if err == nil && time.Now().Before(apiExpires) { + return false, nil + } + } + return true, nil } } diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index bfcd06c57..f9e63400f 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -147,7 +147,7 @@ func TestExecQuietSuppressesExpiryWarning(t *testing.T) { app.Init = func(_ []string, _ io.Reader) (*global.Data, error) { data := testutil.MockGlobalData(args, &stdout) // Set the default token to expire soon so a warning would fire without --quiet. - data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(20 * time.Minute).Format(time.RFC3339) return data, nil } err := app.Run(args, nil) @@ -169,7 +169,7 @@ func TestExecConfigShowsExpiryWarning(t *testing.T) { args := testutil.SplitArgs("config -l") app.Init = func(_ []string, _ io.Reader) (*global.Data, error) { data := testutil.MockGlobalData(args, &stdout) - data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(20 * time.Minute).Format(time.RFC3339) return data, nil } err := app.Run(args, nil) @@ -199,7 +199,7 @@ func TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr(t *testing.T) { data := testutil.MockGlobalData(args, &stdout) data.ErrOutput = &stderr data.Flags.JSON = true - data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(20 * time.Minute).Format(time.RFC3339) return data, nil } err := app.Run(args, nil) diff --git a/pkg/commands/auth/expiry.go b/pkg/commands/auth/expiry.go index 0a95d3699..06e19190d 100644 --- a/pkg/commands/auth/expiry.go +++ b/pkg/commands/auth/expiry.go @@ -27,7 +27,7 @@ const ( const ( // expiryWarningThreshold is the warning window for API tokens and SSO refresh tokens. - expiryWarningThreshold = 7 * 24 * time.Hour + expiryWarningThreshold = 30 * time.Minute // accessOnlyWarningThreshold is the warning window when only an access token // expiry is available (no refresh token). This is shorter because access // tokens are short-lived and auto-refresh. @@ -70,10 +70,15 @@ func staticExpirationStatus(at *config.AuthToken, now time.Time) (ExpirationStat return classifyExpiry(expires, now, expiryWarningThreshold), expires, nil } -// ssoExpirationStatus handles expiration for SSO tokens. It prefers -// RefreshExpiresAt (the user-actionable deadline) and falls back to -// AccessExpiresAt when refresh info is missing or malformed. +// ssoExpirationStatus handles expiration for SSO tokens. func ssoExpirationStatus(at *config.AuthToken, now time.Time) (ExpirationStatus, time.Time, error) { + if at.APITokenExpiresAt != "" { + expires, err := time.Parse(time.RFC3339, at.APITokenExpiresAt) + if err == nil { + return classifyExpiry(expires, now, expiryWarningThreshold), expires, nil + } + } + if at.RefreshToken != "" && at.RefreshExpiresAt == "" { return StatusNoExpiry, time.Time{}, nil } @@ -83,7 +88,6 @@ func ssoExpirationStatus(at *config.AuthToken, now time.Time) (ExpirationStatus, if err == nil { return classifyExpiry(expires, now, expiryWarningThreshold), expires, nil } - // Malformed refresh_expires_at; fall through to access token. } if at.AccessExpiresAt != "" { @@ -94,11 +98,6 @@ func ssoExpirationStatus(at *config.AuthToken, now time.Time) (ExpirationStatus, return StatusNoExpiry, time.Time{}, fmt.Errorf("invalid access_expires_at %q: %w", at.AccessExpiresAt, err) } - // Neither field set. - if at.RefreshExpiresAt != "" { - // RefreshExpiresAt was set but malformed, and no AccessExpiresAt to fall back to. - return StatusNoExpiry, time.Time{}, fmt.Errorf("invalid refresh_expires_at %q", at.RefreshExpiresAt) - } return StatusNoExpiry, time.Time{}, nil } diff --git a/pkg/commands/auth/expiry_test.go b/pkg/commands/auth/expiry_test.go index 4bb5f150a..809851b10 100644 --- a/pkg/commands/auth/expiry_test.go +++ b/pkg/commands/auth/expiry_test.go @@ -62,18 +62,26 @@ func TestGetExpirationStatus(t *testing.T) { wantStatus: authcmd.StatusOK, }, { - name: "static expiring soon (within 7 days)", + name: "static not expiring soon (3 days out)", token: &config.AuthToken{ Type: config.AuthTokenTypeStatic, APITokenExpiresAt: now.Add(3 * 24 * time.Hour).Format(time.RFC3339), }, + wantStatus: authcmd.StatusOK, + }, + { + name: "static expiring soon (within 30 minutes)", + token: &config.AuthToken{ + Type: config.AuthTokenTypeStatic, + APITokenExpiresAt: now.Add(20 * time.Minute).Format(time.RFC3339), + }, wantStatus: authcmd.StatusExpiringSoon, }, { - name: "static expiring soon (exactly 7 days)", + name: "static expiring soon (exactly 30 minutes)", token: &config.AuthToken{ Type: config.AuthTokenTypeStatic, - APITokenExpiresAt: now.Add(7 * 24 * time.Hour).Format(time.RFC3339), + APITokenExpiresAt: now.Add(30 * time.Minute).Format(time.RFC3339), }, wantStatus: authcmd.StatusExpiringSoon, }, @@ -96,6 +104,42 @@ func TestGetExpirationStatus(t *testing.T) { }, // SSO tokens: RefreshExpiresAt primary. + { + name: "sso api_token_expires_at preferred over refresh", + token: &config.AuthToken{ + Type: config.AuthTokenTypeSSO, + APITokenExpiresAt: now.Add(30 * 24 * time.Hour).Format(time.RFC3339), + RefreshExpiresAt: now.Add(25 * time.Minute).Format(time.RFC3339), + }, + wantStatus: authcmd.StatusOK, + }, + { + name: "sso api_token_expires_at not expiring soon (3 days out)", + token: &config.AuthToken{ + Type: config.AuthTokenTypeSSO, + APITokenExpiresAt: now.Add(3 * 24 * time.Hour).Format(time.RFC3339), + RefreshExpiresAt: now.Add(25 * time.Minute).Format(time.RFC3339), + }, + wantStatus: authcmd.StatusOK, + }, + { + name: "sso api_token_expires_at expiring soon (within 30 minutes)", + token: &config.AuthToken{ + Type: config.AuthTokenTypeSSO, + APITokenExpiresAt: now.Add(15 * time.Minute).Format(time.RFC3339), + RefreshExpiresAt: now.Add(25 * time.Minute).Format(time.RFC3339), + }, + wantStatus: authcmd.StatusExpiringSoon, + }, + { + name: "sso api_token_expires_at expired", + token: &config.AuthToken{ + Type: config.AuthTokenTypeSSO, + APITokenExpiresAt: now.Add(-1 * time.Hour).Format(time.RFC3339), + RefreshExpiresAt: now.Add(-2 * time.Hour).Format(time.RFC3339), + }, + wantStatus: authcmd.StatusExpired, + }, { name: "sso refresh OK", token: &config.AuthToken{ @@ -105,11 +149,19 @@ func TestGetExpirationStatus(t *testing.T) { wantStatus: authcmd.StatusOK, }, { - name: "sso refresh expiring soon", + name: "sso refresh not expiring soon (3 days out)", token: &config.AuthToken{ Type: config.AuthTokenTypeSSO, RefreshExpiresAt: now.Add(3 * 24 * time.Hour).Format(time.RFC3339), }, + wantStatus: authcmd.StatusOK, + }, + { + name: "sso refresh expiring soon (within 30 minutes)", + token: &config.AuthToken{ + Type: config.AuthTokenTypeSSO, + RefreshExpiresAt: now.Add(20 * time.Minute).Format(time.RFC3339), + }, wantStatus: authcmd.StatusExpiringSoon, }, { @@ -164,7 +216,7 @@ func TestGetExpirationStatus(t *testing.T) { RefreshExpiresAt: "garbage", }, wantStatus: authcmd.StatusNoExpiry, - wantErr: true, + wantErr: false, }, { name: "sso no refresh, malformed access", @@ -195,11 +247,19 @@ func TestGetExpirationStatus(t *testing.T) { // Unknown type falls through to static-style check. { - name: "unknown type with expiry", + name: "unknown type with expiry (not soon)", token: &config.AuthToken{ Type: "unknown", APITokenExpiresAt: now.Add(3 * 24 * time.Hour).Format(time.RFC3339), }, + wantStatus: authcmd.StatusOK, + }, + { + name: "unknown type with expiry (within 30 minutes)", + token: &config.AuthToken{ + Type: "unknown", + APITokenExpiresAt: now.Add(10 * time.Minute).Format(time.RFC3339), + }, wantStatus: authcmd.StatusExpiringSoon, },