mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: use time.Equal() for external auth token expiry comparison (#22295)
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 validation call 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