diff --git a/coderd/gitauth/config.go b/coderd/gitauth/config.go index 29d4804dcd538..1387acee7ebf1 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,26 @@ 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, 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 { 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(retryCtx) { + 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{})