Skip to content

fix: conceal sensitive domain information in auth error messages #17132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions coderd/userauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
73 changes: 73 additions & 0 deletions coderd/userauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]",
"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"
Expand Down
Loading