mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix: use time.Equal() for external auth token expiry comparison
The listen loop in workspaceAgentsExternalAuthListen compared OAuthExpiry using == which compares time.Time internal struct fields including the *time.Location pointer. time.LoadLocation does not cache the returned *Location pointer, so each lib/pq connection gets a distinct pointer for the same timezone. When pq.ParseTimestamp applies the connection's location to a parsed timestamp, the resulting time.Time embeds that connection-specific pointer. If the sql.DB pool hands out different connections for the two GetExternalAuthLink reads, the identical timestamp produces time.Time values where == returns false despite representing the same instant. This is intermittent because the pool usually reuses the same connection for sequential queries. This change uses .Equal() to compare instants regardless of location. Also makes the test's validateCalls counter atomic to fix a possible data race between the HTTP server and test goroutines.
This commit is contained in:
@@ -2045,7 +2045,7 @@ func (api *API) workspaceAgentsExternalAuthListen(ctx context.Context, rw http.R
|
||||
// No point in trying to validate the same token over and over again.
|
||||
if previousToken.OAuthAccessToken == externalAuthLink.OAuthAccessToken &&
|
||||
previousToken.OAuthRefreshToken == externalAuthLink.OAuthRefreshToken &&
|
||||
previousToken.OAuthExpiry == externalAuthLink.OAuthExpiry {
|
||||
previousToken.OAuthExpiry.Equal(externalAuthLink.OAuthExpiry) {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
@@ -2784,12 +2784,12 @@ func TestWorkspaceAgentExternalAuthListen(t *testing.T) {
|
||||
const providerID = "fake-idp"
|
||||
|
||||
// Count all the times we call validate
|
||||
validateCalls := 0
|
||||
var validateCalls atomic.Int32
|
||||
fake := oidctest.NewFakeIDP(t, oidctest.WithServing(), oidctest.WithMiddlewares(func(handler http.Handler) http.Handler {
|
||||
return http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// Count all the validate calls
|
||||
if strings.Contains(r.URL.Path, "/external-auth-validate/") {
|
||||
validateCalls++
|
||||
validateCalls.Add(1)
|
||||
}
|
||||
handler.ServeHTTP(w, r)
|
||||
}))
|
||||
@@ -2852,7 +2852,7 @@ func TestWorkspaceAgentExternalAuthListen(t *testing.T) {
|
||||
// other should be skipped.
|
||||
// In a failed test, you will likely see 9, as the last one
|
||||
// gets canceled.
|
||||
require.Equal(t, 1, validateCalls, "validate calls duplicated on same token")
|
||||
require.EqualValues(t, 1, validateCalls.Load(), "validate calls duplicated on same token")
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user