From 13b7f8f3493c015a8789252a2101fe5818c3cd9a Mon Sep 17 00:00:00 2001
From: Kyle Carberry <kyle@carberry.com>
Date: Tue, 8 Aug 2023 04:08:47 +0000
Subject: [PATCH 1/2] fix: add exp backoff to validate fresh git auth tokens

A customer using GitHub in Australia reported that validating immediately
after refreshing the token would intermittently fail with a 401. Waiting
a few milliseconds with the exact same token on the exact same request
would resolve the issue. It seems likely that the write is not propagating
to the read replica in time.
---
 coderd/gitauth/config.go      | 15 ++++++++++++++-
 coderd/gitauth/config_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go
index 29d4804dcd538..a30eaf967005d 100644
--- a/coderd/gitauth/config.go
+++ b/coderd/gitauth/config.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/url"
 	"regexp"
+	"time"
 
 	"golang.org/x/oauth2"
 	"golang.org/x/xerrors"
@@ -17,6 +18,7 @@ import (
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/codersdk"
+	"github.com/coder/retry"
 )
 
 type OAuth2Config interface {
@@ -75,12 +77,23 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, gitAuthLin
 		// we aren't trying to surface an error, we're just trying to obtain a valid token.
 		return gitAuthLink, false, nil
 	}
-
+	r := retry.New(50*time.Millisecond, time.Second)
+validate:
 	valid, _, err := c.ValidateToken(ctx, token.AccessToken)
 	if err != nil {
 		return gitAuthLink, false, xerrors.Errorf("validate git auth token: %w", err)
 	}
 	if !valid {
+		// A customer using GitHub in Australia reported that validating immediately
+		// after refreshing the token would intermittently fail with a 401. Waiting
+		// a few milliseconds with the exact same token on the exact same request
+		// would resolve the issue. It seems likely that the write is not propagating
+		// to the read replica in time.
+		//
+		// We do an exponential backoff here to give the write time to propagate.
+		if c.Type == codersdk.GitProviderGitHub && r.Wait(ctx) {
+			goto validate
+		}
 		// The token is no longer valid!
 		return gitAuthLink, false, nil
 	}
diff --git a/coderd/gitauth/config_test.go b/coderd/gitauth/config_test.go
index 31d6392341426..f58531fdf773f 100644
--- a/coderd/gitauth/config_test.go
+++ b/coderd/gitauth/config_test.go
@@ -73,6 +73,39 @@ func TestRefreshToken(t *testing.T) {
 		require.NoError(t, err)
 		require.False(t, refreshed)
 	})
+	t.Run("ValidateRetryGitHub", func(t *testing.T) {
+		t.Parallel()
+		hit := false
+		// We need to ensure that the exponential backoff kicks in properly.
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if !hit {
+				hit = true
+				w.WriteHeader(http.StatusUnauthorized)
+				w.Write([]byte("Not permitted"))
+				return
+			}
+			w.WriteHeader(http.StatusOK)
+		}))
+		config := &gitauth.Config{
+			ID: "test",
+			OAuth2Config: &testutil.OAuth2Config{
+				Token: &oauth2.Token{
+					AccessToken: "updated",
+				},
+			},
+			ValidateURL: srv.URL,
+			Type:        codersdk.GitProviderGitHub,
+		}
+		db := dbfake.New()
+		link := dbgen.GitAuthLink(t, db, database.GitAuthLink{
+			ProviderID:       config.ID,
+			OAuthAccessToken: "initial",
+		})
+		_, refreshed, err := config.RefreshToken(context.Background(), db, link)
+		require.NoError(t, err)
+		require.True(t, refreshed)
+		require.True(t, hit)
+	})
 	t.Run("ValidateNoUpdate", func(t *testing.T) {
 		t.Parallel()
 		validated := make(chan struct{})

From d37f6eab24e05dd616ebecae663fa4529e932cbb Mon Sep 17 00:00:00 2001
From: Kyle Carberry <kyle@carberry.com>
Date: Tue, 8 Aug 2023 04:18:51 +0000
Subject: [PATCH 2/2] Fix infinite backoff

---
 coderd/gitauth/config.go | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go
index a30eaf967005d..1387acee7ebf1 100644
--- a/coderd/gitauth/config.go
+++ b/coderd/gitauth/config.go
@@ -77,7 +77,10 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, gitAuthLin
 		// we aren't trying to surface an error, we're just trying to obtain a valid token.
 		return gitAuthLink, false, nil
 	}
-	r := retry.New(50*time.Millisecond, time.Second)
+	r := retry.New(50*time.Millisecond, 200*time.Millisecond)
+	// See the comment below why the retry and cancel is required.
+	retryCtx, retryCtxCancel := context.WithTimeout(ctx, time.Second)
+	defer retryCtxCancel()
 validate:
 	valid, _, err := c.ValidateToken(ctx, token.AccessToken)
 	if err != nil {
@@ -91,7 +94,7 @@ validate:
 		// to the read replica in time.
 		//
 		// We do an exponential backoff here to give the write time to propagate.
-		if c.Type == codersdk.GitProviderGitHub && r.Wait(ctx) {
+		if c.Type == codersdk.GitProviderGitHub && r.Wait(retryCtx) {
 			goto validate
 		}
 		// The token is no longer valid!