feat: keep original token refresh error in external auth

External auth refresh errors lose the original error thrown on the first refresh. This PR saves that error to the database to be raised on subsequent refresh attempts
coder · Emyrk · Aug 14, 2025 · Aug 13, 2025 · Aug 13, 2025 · Aug 13, 2025
commit aec8e95c9f7facd489c014956f604a6cbe2e569c
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000358_failed_ext_auth_error.down.sql b/coderd/database/migrations/000358_failed_ext_auth_error.down.sql
@@ -0,0 +1,3 @@
+ALTER TABLE external_auth_links
+	DROP COLUMN oauth_refresh_failure_reason
+;
diff --git a/coderd/database/migrations/000358_failed_ext_auth_error.up.sql b/coderd/database/migrations/000358_failed_ext_auth_error.up.sql
@@ -0,0 +1,7 @@
+ALTER TABLE external_auth_links
+	ADD COLUMN oauth_refresh_failure_reason TEXT NOT NULL DEFAULT ''
+;
+
+COMMENT ON COLUMN external_auth_links.oauth_refresh_failure_reason IS
+	'This error means the refresh token is invalid. Cached so we can avoid calling the external provider again for the same error.'
+;
diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/externalauth.sql b/coderd/database/queries/externalauth.sql
@@ -40,13 +40,16 @@ UPDATE external_auth_links SET
     oauth_refresh_token = $6,
     oauth_refresh_token_key_id = $7,
     oauth_expiry = $8,
-	oauth_extra = $9
+	oauth_extra = $9,
+	-- Only 'UpdateExternalAuthLinkRefreshToken' supports updating the oauth_refresh_failure_reason
+	oauth_refresh_failure_reason = ''
 WHERE provider_id = $1 AND user_id = $2 RETURNING *;
 
 -- name: UpdateExternalAuthLinkRefreshToken :exec
 UPDATE
 	external_auth_links
 SET
+	oauth_refresh_failure_reason = @oauth_refresh_failure_reason,
 	oauth_refresh_token = @oauth_refresh_token,
 	updated_at = @updated_at
 WHERE

diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go
@@ -28,6 +28,10 @@ import (
 	"github.com/coder/retry"
 )
 
+const (
+	failureReasonLimit = 200
+)
+
 // Config is used for authentication for Git operations.
 type Config struct {
 	promoauth.InstrumentedOAuth2Config
@@ -130,6 +134,12 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, externalAu
 		refreshToken = ""
 	}
 
+	if externalAuthLink.OauthRefreshFailureReason != "" {
+		// If the refresh token is invalid, do not try to keep using it. This will
+		// prevent spamming the IdP with refresh attempts that will fail.
+		refreshToken = ""
+	}
+
 	existingToken := &oauth2.Token{
 		AccessToken:  externalAuthLink.OAuthAccessToken,
 		RefreshToken: refreshToken,
@@ -143,12 +153,19 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, externalAu
 		// get rid of it. Keeping it around will cause additional refresh
 		// attempts that will fail and cost us api rate limits.
 		if isFailedRefresh(existingToken, err) {
+			reason := err.Error()
+			if len(reason) > failureReasonLimit {
+				// Limit the length of the error message to prevent
+				// spamming the database with long error messages.
+				reason = reason[:failureReasonLimit]
+			}
 			dbExecErr := db.UpdateExternalAuthLinkRefreshToken(ctx, database.UpdateExternalAuthLinkRefreshTokenParams{
-				OAuthRefreshToken:      "", // It is better to clear the refresh token than to keep retrying.
-				OAuthRefreshTokenKeyID: externalAuthLink.OAuthRefreshTokenKeyID.String,
-				UpdatedAt:              dbtime.Now(),
-				ProviderID:             externalAuthLink.ProviderID,
-				UserID:                 externalAuthLink.UserID,
+				// Adding a reason will prevent further attempts to try and refresh the token.
+				OauthRefreshFailureReason: reason,
+				OAuthRefreshTokenKeyID:    externalAuthLink.OAuthRefreshTokenKeyID.String,
+				UpdatedAt:                 dbtime.Now(),
+				ProviderID:                externalAuthLink.ProviderID,
+				UserID:                    externalAuthLink.UserID,
 			})
 			if dbExecErr != nil {
 				// This error should be rare.

diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go
@@ -177,37 +177,44 @@ func TestRefreshToken(t *testing.T) {
 		link.OAuthExpiry = expired
 
 		// Make the failure a server internal error. Not related to the token
+		// This should be retried since this error is temporary.
 		refreshErr = &oauth2.RetrieveError{
 			Response: &http.Response{
 				StatusCode: http.StatusInternalServerError,
 			},
 			ErrorCode: "internal_error",
 		}
-		_, err := config.RefreshToken(ctx, mDB, link)
-		require.Error(t, err)
-		require.True(t, externalauth.IsInvalidTokenError(err))
-		require.Equal(t, refreshCount, 1)
+		totalRefreshes := 0
+		for i := 0; i < 3; i++ {
+			// Each loop will hit the temporary error and retry.
+			_, err := config.RefreshToken(ctx, mDB, link)
+			require.Error(t, err)
+			totalRefreshes++
+			require.True(t, externalauth.IsInvalidTokenError(err))
+			require.Equal(t, refreshCount, totalRefreshes)
+		}
 
-		// Try again with a bad refresh token error
-		// Expect DB call to remove the refresh token
+		// Try again with a bad refresh token error. This will invalidate the
+		// refresh token, and not retry again. Expect DB call to remove the refresh token
 		mDB.EXPECT().UpdateExternalAuthLinkRefreshToken(gomock.Any(), gomock.Any()).Return(nil).Times(1)
 		refreshErr = &oauth2.RetrieveError{ // github error
 			Response: &http.Response{
 				StatusCode: http.StatusOK,
 			},
 			ErrorCode: "bad_refresh_token",
 		}
-		_, err = config.RefreshToken(ctx, mDB, link)
+		_, err := config.RefreshToken(ctx, mDB, link)
 		require.Error(t, err)
+		totalRefreshes++
 		require.True(t, externalauth.IsInvalidTokenError(err))
-		require.Equal(t, refreshCount, 2)
+		require.Equal(t, refreshCount, totalRefreshes)
 
 		// When the refresh token is empty, no api calls should be made
 		link.OAuthRefreshToken = "" // mock'd db, so manually set the token to ''
 		_, err = config.RefreshToken(ctx, mDB, link)
 		require.Error(t, err)
 		require.True(t, externalauth.IsInvalidTokenError(err))
-		require.Equal(t, refreshCount, 2)
+		require.Equal(t, refreshCount, totalRefreshes)
 	})
 
 	// ValidateFailure tests if the token is no longer valid with a 401 response.

diff --git a/scripts/testidp/main.go b/scripts/testidp/main.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"flag"
 	"log"
+	"net/http"
 	"os"
 	"os/signal"
 	"strings"
@@ -13,6 +14,7 @@ import (
 	"github.com/golang-jwt/jwt/v4"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2"
 
 	"cdr.dev/slog"
 	"cdr.dev/slog/sloggers/sloghuman"
@@ -96,6 +98,17 @@ func RunIDP() func(t *testing.T) {
 				"groups":             []string{"testidp", "qa", "engineering"},
 				"roles":              []string{"testidp", "admin", "higher_power"},
 			}),
+			oidctest.WithRefresh(func(email string) error {
+				return &oauth2.RetrieveError{
+					Response: &http.Response{
+						StatusCode: http.StatusBadRequest,
+					},
+					Body:             nil,
+					ErrorCode:        "bad_refresh_token",
+					ErrorDescription: "refreshing is set to fail for testing purposes",
+					ErrorURI:         "",
+				}
+			}),
 			oidctest.WithDefaultIDClaims(jwt.MapClaims{}),
 			oidctest.WithDefaultExpire(*expiry),
 			oidctest.WithStaticCredentials(*clientID, *clientSecret),