Compare commits

...

2 Commits

Author SHA1 Message Date
Kyle Carberry
3ab1f6845c fix: add exp backoff to validate fresh git auth tokens (#8956)
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.
2023-08-08 05:09:28 +00:00
Colin Adler
ad513fa8b9 chore: fix release and security pipelines 2023-08-03 23:36:22 +00:00
4 changed files with 54 additions and 21 deletions

View File

@@ -98,16 +98,8 @@ jobs:
- name: Setup Go
uses: ./.github/actions/setup-go
- name: Cache Node
id: cache-node
uses: buildjet/cache@v3
with:
path: |
**/node_modules
.eslintcache
key: js-${{ runner.os }}-test-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
js-${{ runner.os }}-
- name: Setup Node
uses: ./.github/actions/setup-node
- name: Install nsis and zstd
run: sudo apt-get install -y nsis zstd

View File

@@ -69,16 +69,8 @@ jobs:
- name: Setup Go
uses: ./.github/actions/setup-go
- name: Cache Node
id: cache-node
uses: buildjet/cache@v3
with:
path: |
**/node_modules
.eslintcache
key: js-${{ runner.os }}-test-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
js-${{ runner.os }}-
- name: Setup Node
uses: ./.github/actions/setup-node
- name: Setup sqlc
uses: ./.github/actions/setup-sqlc

View File

@@ -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
}

View File

@@ -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{})