From c805c8c02cce80ff9ae02562829581ea0b14f7ac Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Mar 2026 06:29:54 -0600 Subject: [PATCH] chore: setting time forward for expiration math (#22687) It was set backwards, which allowed invalid refresh tokens. Making things worse. --- .../provisionerdserver/provisionerdserver.go | 7 ++- .../provisionerdserver_internal_test.go | 49 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 9440cf88d4..c0ccee4145 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -3093,10 +3093,13 @@ func shouldRefreshOIDCToken(link database.UserLink) bool { // // If an OIDC provider issues short-lived tokens less than our defined period, // the token will always be refreshed on every workspace build. - assumeExpiredAt := dbtime.Now().Add(-1 * time.Minute * 10) + // + // By shifting the time forward, we are asking + // "Will this token be valid in 10 minutes" + expiryCheckTime := dbtime.Now().Add(time.Minute * 10) // Return if the token is assumed to be expired. - return link.OAuthExpiry.Before(assumeExpiredAt) + return link.OAuthExpiry.Before(expiryCheckTime) } // obtainOIDCAccessToken returns a valid OpenID Connect access token diff --git a/coderd/provisionerdserver/provisionerdserver_internal_test.go b/coderd/provisionerdserver/provisionerdserver_internal_test.go index 04320c0e59..cf18d502aa 100644 --- a/coderd/provisionerdserver/provisionerdserver_internal_test.go +++ b/coderd/provisionerdserver/provisionerdserver_internal_test.go @@ -36,18 +36,51 @@ func TestShouldRefreshOIDCToken(t *testing.T) { want: false, }, { - name: "ExpiredBeyondAssumedWindow", + name: "LongExpired", link: database.UserLink{ OAuthRefreshToken: "refresh", - OAuthExpiry: now.Add(-20 * time.Minute), + OAuthExpiry: now.Add(-1 * time.Hour), }, want: true, }, { - name: "ExpiredWithinAssumedWindow", + // Edge being "+/- 10 minutes" + name: "EdgeExpired", link: database.UserLink{ OAuthRefreshToken: "refresh", - OAuthExpiry: now.Add(-5 * time.Minute), + OAuthExpiry: now.Add(-1 * time.Minute * 10), + }, + want: true, + }, + { + name: "Expired", + link: database.UserLink{ + OAuthRefreshToken: "refresh", + OAuthExpiry: now.Add(-1 * time.Minute), + }, + want: true, + }, + { + name: "SoonToBeExpired", + link: database.UserLink{ + OAuthRefreshToken: "refresh", + OAuthExpiry: now.Add(5 * time.Minute), + }, + want: true, + }, + { + name: "SoonToBeExpiredEdge", + link: database.UserLink{ + OAuthRefreshToken: "refresh", + OAuthExpiry: now.Add(9 * time.Minute), + }, + want: true, + }, + { + name: "AfterEdge", + link: database.UserLink{ + OAuthRefreshToken: "refresh", + OAuthExpiry: now.Add(11 * time.Minute), }, want: false, }, @@ -59,6 +92,14 @@ func TestShouldRefreshOIDCToken(t *testing.T) { }, want: false, }, + { + name: "NotEvenCloseExpired", + link: database.UserLink{ + OAuthRefreshToken: "refresh", + OAuthExpiry: now.Add(time.Hour * 24), + }, + want: false, + }, } for _, tc := range testCases {