fix: remove shared mutable state between oidc tests (#17179)

ethanndickson · web-flow · commit 3a243c111b9a · 2025-04-01T22:28:05.000+11:00
Spotted on main: https://github.com/coder/coder/actions/runs/14179449567/job/39721999486 ``` === FAIL: coderd TestOIDCDomainErrorMessage/MalformedEmailErrorOmitsDomains (0.01s) ================== WARNING: DATA RACE Read at 0x00c060b54e68 by goroutine 296485: golang.org/x/oauth2.(*Config).Exchange() /home/runner/go/pkg/mod/golang.org/x/oauth2@v0.28.0/oauth2.go:228 +0x1d8 github.com/coder/coder/v2/coderd.(*OIDCConfig).Exchange() <autogenerated>:1 +0xb7 github.com/coder/coder/v2/coderd.New.func11.12.1.2.ExtractOAuth2.1.1() /home/runner/work/coder/coder/coderd/httpmw/oauth2.go:168 +0x7b5 net/http.HandlerFunc.ServeHTTP() /opt/hostedtoolcache/go/1.24.1/x64/src/net/http/server.go:2294 +0x47 [...] Previous write at 0x00c060b54e68 by goroutine 55730: github.com/coder/coder/v2/coderd/coderdtest/oidctest.(*FakeIDP).SetRedirect() /home/runner/work/coder/coder/coderd/coderdtest/oidctest/idp.go:1280 +0x1e6 github.com/coder/coder/v2/coderd/coderdtest/oidctest.(*FakeIDP).LoginWithClient() /home/runner/work/coder/coder/coderd/coderdtest/oidctest/idp.go:494 +0x170 github.com/coder/coder/v2/coderd/coderdtest/oidctest.(*FakeIDP).AttemptLogin() /home/runner/work/coder/coder/coderd/coderdtest/oidctest/idp.go:479 +0x624 github.com/coder/coder/v2/coderd_test.TestOIDCDomainErrorMessage.func3() /home/runner/work/coder/coder/coderd/userauth_test.go:2041 +0x1f2 ``` As seen, this race was caused by sharing a `*oidctest.FakeIDP` between test cases. The fix is to simply do the setup twice. ``` $ go test -race -run "TestOIDCDomainErrorMessage" github.com/coder/coder/v2/coderd -count=100 ok github.com/coder/coder/v2/coderd 7.551s ````
diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go
@@ -1988,30 +1988,36 @@ func TestUserLogout(t *testing.T) {
 func TestOIDCDomainErrorMessage(t *testing.T) {
 	t.Parallel()
 
-	fake := oidctest.NewFakeIDP(t, oidctest.WithServing())
-
 	allowedDomains := []string{"allowed1.com", "allowed2.org", "company.internal"}
-	cfg := fake.OIDCConfig(t, nil, func(cfg *coderd.OIDCConfig) {
-		cfg.EmailDomain = allowedDomains
-		cfg.AllowSignups = true
-	})
 
-	server := coderdtest.New(t, &coderdtest.Options{
-		OIDCConfig: cfg,
-	})
+	setup := func() (*oidctest.FakeIDP, *codersdk.Client) {
+		fake := oidctest.NewFakeIDP(t, oidctest.WithServing())
+
+		cfg := fake.OIDCConfig(t, nil, func(cfg *coderd.OIDCConfig) {
+			cfg.EmailDomain = allowedDomains
+			cfg.AllowSignups = true
+		})
+
+		client := coderdtest.New(t, &coderdtest.Options{
+			OIDCConfig: cfg,
+		})
+		return fake, client
+	}
 
 	// Test case 1: Email domain not in allowed list
 	t.Run("ErrorMessageOmitsDomains", func(t *testing.T) {
 		t.Parallel()
 
+		fake, client := setup()
+
 		// Prepare claims with email from unauthorized domain
 		claims := jwt.MapClaims{
 			"email":          "user@unauthorized.com",
 			"email_verified": true,
 			"sub":            uuid.NewString(),
 		}
 
-		_, resp := fake.AttemptLogin(t, server, claims)
+		_, resp := fake.AttemptLogin(t, client, claims)
 		defer resp.Body.Close()
 
 		require.Equal(t, http.StatusForbidden, resp.StatusCode)
@@ -2031,14 +2037,16 @@ func TestOIDCDomainErrorMessage(t *testing.T) {
 	t.Run("MalformedEmailErrorOmitsDomains", func(t *testing.T) {
 		t.Parallel()
 
+		fake, client := setup()
+
 		// Prepare claims with an invalid email format (no @ symbol)
 		claims := jwt.MapClaims{
 			"email":          "invalid-email-without-domain",
 			"email_verified": true,
 			"sub":            uuid.NewString(),
 		}
 
-		_, resp := fake.AttemptLogin(t, server, claims)
+		_, resp := fake.AttemptLogin(t, client, claims)
 		defer resp.Body.Close()
 
 		require.Equal(t, http.StatusForbidden, resp.StatusCode)
