diff --git a/coderd/userauth.go b/coderd/userauth.go index ba57a1dff4580..f08e126208d3c 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1358,7 +1358,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { emailSp := strings.Split(email, "@") if len(emailSp) == 1 { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: fmt.Sprintf("Your email %q is not in domains %q!", email, api.OIDCConfig.EmailDomain), + Message: fmt.Sprintf("Your email %q is not from an authorized domain! Please contact your administrator.", email), }) return } @@ -1373,7 +1373,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } if !ok { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: fmt.Sprintf("Your email %q is not in domains %q!", email, api.OIDCConfig.EmailDomain), + Message: fmt.Sprintf("Your email %q is not from an authorized domain! Please contact your administrator.", email), }) return } diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 38356eb19fdd6..ad8e126706dd1 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -1982,6 +1982,79 @@ func TestUserLogout(t *testing.T) { // - JWT with issuer https://secondary.com // // Without this security check disabled, all three above would have to match. + +// TestOIDCDomainErrorMessage ensures that when a user with an unauthorized domain +// attempts to login, the error message doesn't expose the list of authorized domains. +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, + }) + + // Test case 1: Email domain not in allowed list + t.Run("ErrorMessageOmitsDomains", func(t *testing.T) { + t.Parallel() + + // 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) + defer resp.Body.Close() + + require.Equal(t, http.StatusForbidden, resp.StatusCode) + + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Contains(t, string(data), "is not from an authorized domain") + require.Contains(t, string(data), "Please contact your administrator") + + for _, domain := range allowedDomains { + require.NotContains(t, string(data), domain) + } + }) + + // Test case 2: Malformed email without @ symbol + t.Run("MalformedEmailErrorOmitsDomains", func(t *testing.T) { + t.Parallel() + + // 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) + defer resp.Body.Close() + + require.Equal(t, http.StatusForbidden, resp.StatusCode) + + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Contains(t, string(data), "is not from an authorized domain") + require.Contains(t, string(data), "Please contact your administrator") + + for _, domain := range allowedDomains { + require.NotContains(t, string(data), domain) + } + }) +} + func TestOIDCSkipIssuer(t *testing.T) { t.Parallel() const primaryURLString = "https://primary.com"