From 3a243c111b9abb5c38328169ff70064025bbe2fe Mon Sep 17 00:00:00 2001
From: Ethan <39577870+ethanndickson@users.noreply.github.com>
Date: Tue, 1 Apr 2025 22:28:05 +1100
Subject: [PATCH] fix: remove shared mutable state between oidc tests (#17179)

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
````
---
 coderd/userauth_test.go | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go
index ad8e126706..ddf3dceba2 100644
--- a/coderd/userauth_test.go
+++ b/coderd/userauth_test.go
@@ -1988,22 +1988,28 @@ 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",
@@ -2011,7 +2017,7 @@ func TestOIDCDomainErrorMessage(t *testing.T) {
 			"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,6 +2037,8 @@ 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",
@@ -2038,7 +2046,7 @@ func TestOIDCDomainErrorMessage(t *testing.T) {
 			"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)