synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-18 14:17:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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
````
This commit is contained in:
Ethan
2025-04-01 22:28:05 +11:00
committed by GitHub
parent 7d08bf0afe
commit 3a243c111b
1 changed files with 19 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
coderd/userauth_test.go
View File

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