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
20 changes: 13 additions & 7 deletions cmd/grounds/commands/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,14 @@ func checkAuth(ctx context.Context) checkResult {
return checkResult{name: "Auth", status: statusError, summary: "You are not logged in", details: []string{"Run `grounds login` to authenticate."}}
}
// The access_token Keycloak issues is short-lived (≈5 min by default)
// but the refresh_token lives much longer (≈30 d). Real commands go
// through FileTokenSource.Token which transparently refreshes; doctor
// must do the same or it reports "expired" while everything else
// works fine. Drop down to refresh + persist if needed.
// but the refresh_token is an offline token that doesn't expire
// (Keycloak signals that with refresh_expires_in: 0, which we map
// to a zero `time.Time` — see auth.RefreshExpiryFromSeconds).
// Real commands go through FileTokenSource.Token which transparently
// refreshes; doctor must do the same or it reports "expired" while
// everything else works fine. Drop down to refresh + persist if needed.
if time.Now().After(c.ExpiresAt.Add(-30 * time.Second)) {
if time.Now().After(c.RefreshExpiresAt) {
if !c.IsRefreshAlive() {
return checkResult{name: "Auth", status: statusError, summary: "Your login session has expired", details: []string{"Run `grounds login` to authenticate again."}}
}
device := &auth.DeviceClient{
Expand All @@ -259,13 +261,17 @@ func checkAuth(ctx context.Context) checkResult {
c.AccessToken = fresh.AccessToken
c.RefreshToken = fresh.RefreshToken
c.ExpiresAt = time.Now().Add(time.Duration(fresh.ExpiresIn) * time.Second)
c.RefreshExpiresAt = time.Now().Add(time.Duration(fresh.RefreshExpiresIn) * time.Second)
c.RefreshExpiresAt = auth.RefreshExpiryFromSeconds(fresh.RefreshExpiresIn)
if err := store.Save(c); err != nil {
return checkResult{name: "Auth", status: statusError, summary: "Refreshed your session but could not save it", details: []string{err.Error()}}
}
return checkResult{name: "Auth", status: statusOK, summary: c.PreferredUser + " is logged in (session refreshed, valid for " + time.Until(c.ExpiresAt).Round(time.Minute).String() + ")"}
}
return checkResult{name: "Auth", status: statusOK, summary: c.PreferredUser + " is logged in (session valid for " + time.Until(c.ExpiresAt).Round(time.Minute).String() + ", refresh token valid for " + time.Until(c.RefreshExpiresAt).Round(time.Hour).String() + ")"}
refreshSummary := "no expiry (offline token)"
if !c.RefreshExpiresAt.IsZero() {
refreshSummary = "refresh token valid for " + time.Until(c.RefreshExpiresAt).Round(time.Hour).String()
}
return checkResult{name: "Auth", status: statusOK, summary: c.PreferredUser + " is logged in (session valid for " + time.Until(c.ExpiresAt).Round(time.Minute).String() + ", " + refreshSummary + ")"}
}

func checkAPI(ctx context.Context) checkResult {
Expand Down
26 changes: 26 additions & 0 deletions internal/auth/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,29 @@ func ParseCredentials(b []byte) (*Credentials, error) {
}
return c, nil
}

// RefreshExpiryFromSeconds turns the OAuth `refresh_expires_in` field
// into the absolute timestamp we persist. Keycloak returns `0` for
// offline tokens (the canonical "no expiry" indicator), so we map that
// to the zero `time.Time` value and let the IsRefreshAlive predicate
// treat it as "lives forever". Naively doing `time.Now().Add(0)` would
// stamp the token as expired the instant after login, which is exactly
// the regression that bit grounds-cli — see IsRefreshAlive for the
// matching read path.
func RefreshExpiryFromSeconds(seconds int) time.Time {
if seconds <= 0 {
return time.Time{}
}
return time.Now().Add(time.Duration(seconds) * time.Second)
}

// IsRefreshAlive reports whether the stored refresh token is still
// usable. A zero RefreshExpiresAt means the token is an OIDC offline
// token (no expiry) and is always alive; otherwise compare to wall
// clock.
func (c *Credentials) IsRefreshAlive() bool {
if c.RefreshExpiresAt.IsZero() {
return true
}
return time.Now().Before(c.RefreshExpiresAt)
}
42 changes: 42 additions & 0 deletions internal/auth/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,45 @@ func TestParseLegacyFileWithoutVersion(t *testing.T) {
t.Errorf("legacy file should be upgraded to v%d, got v%d", CredentialsVersion, c.Version)
}
}

// TestRefreshExpiryFromSeconds_offline maps Keycloak's `0` for offline
// tokens to a zero `time.Time` so the CLI doesn't immediately decide
// the refresh token is dead the instant after login.
func TestRefreshExpiryFromSeconds_offline(t *testing.T) {
if !RefreshExpiryFromSeconds(0).IsZero() {
t.Error("seconds=0 should map to zero time.Time")
}
if !RefreshExpiryFromSeconds(-5).IsZero() {
t.Error("negative seconds should also map to zero time.Time")
}
}

func TestRefreshExpiryFromSeconds_finite(t *testing.T) {
got := RefreshExpiryFromSeconds(60)
if got.IsZero() {
t.Fatal("seconds=60 should produce a non-zero expiry")
}
if got.Before(time.Now()) {
t.Error("seconds=60 should produce an expiry in the future")
}
}

func TestIsRefreshAlive(t *testing.T) {
now := time.Now()
cases := []struct {
name string
c Credentials
want bool
}{
{"offline (zero time)", Credentials{}, true},
{"future", Credentials{RefreshExpiresAt: now.Add(time.Hour)}, true},
{"past", Credentials{RefreshExpiresAt: now.Add(-time.Hour)}, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := tc.c.IsRefreshAlive(); got != tc.want {
t.Errorf("got %v, want %v", got, tc.want)
}
})
}
}
2 changes: 1 addition & 1 deletion internal/auth/refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func CredentialsFromToken(t *TokenResponse, email, preferred string) *Credential
AccessToken: t.AccessToken,
RefreshToken: t.RefreshToken,
ExpiresAt: time.Now().Add(time.Duration(t.ExpiresIn) * time.Second),
RefreshExpiresAt: time.Now().Add(time.Duration(t.RefreshExpiresIn) * time.Second),
RefreshExpiresAt: RefreshExpiryFromSeconds(t.RefreshExpiresIn),
Email: email,
PreferredUser: preferred,
}
Expand Down
9 changes: 8 additions & 1 deletion internal/auth/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ func (f *FileTokenSource) Token(ctx context.Context) (string, error) {
return "", err
}
if time.Now().After(c.ExpiresAt.Add(-30 * time.Second)) {
// Refuse to even attempt refresh against a known-dead token —
// the only outcome is a useless network round-trip + the
// `_ = Store.Delete()` below would unhelpfully nuke the
// already-stale credentials.
if !c.IsRefreshAlive() {
return "", fmt.Errorf("session expired: run 'grounds login' again")
}
fresh, err := f.Device.Refresh(ctx, c.RefreshToken)
if err != nil {
_ = f.Store.Delete()
Expand All @@ -27,7 +34,7 @@ func (f *FileTokenSource) Token(ctx context.Context) (string, error) {
c.AccessToken = fresh.AccessToken
c.RefreshToken = fresh.RefreshToken
c.ExpiresAt = time.Now().Add(time.Duration(fresh.ExpiresIn) * time.Second)
c.RefreshExpiresAt = time.Now().Add(time.Duration(fresh.RefreshExpiresIn) * time.Second)
c.RefreshExpiresAt = RefreshExpiryFromSeconds(fresh.RefreshExpiresIn)
if err := f.Store.Save(c); err != nil {
return "", err
}
Expand Down
Loading