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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fcce5f
fix: conceal sensitive domain information in auth error messages
ericpaulsen Mar 27, 2025
949ca4d
add unit tests for unauth'd email domains
ericpaulsen Mar 27, 2025
75bb4cd
make fmt
ericpaulsen Mar 27, 2025
bbdd048
test: add test cases for domain concealment in auth error messages
ericpaulsen Mar 27, 2025
4fe7e66
move tests to userauth_test.go & reduce comments
ericpaulsen Mar 27, 2025
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
View file
Open in desktop
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
View file
Open in desktop
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": "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"
Expand Down
Loading