Skip to content

Commit e94de0b

Browse files
angrycubCopilot
angrycub
and
Copilot
authored
fix(coderd): render HTML error page for OIDC email validation failures (#23059)
## Summary When the email address returned from an OIDC provider doesn't match the configured allowed domain list (or isn't verified), users previously saw raw JSON dumped directly in the browser — an ugly and confusing experience during a browser-redirect flow. This PR replaces those JSON responses with the same styled static HTML error page already used for group allow-list errors, signups-disabled, and wrong-login-type errors. ## Changes ### `coderd/userauth.go` Replaced 3 `httpapi.Write` calls in `userOIDC` with `site.RenderStaticErrorPage`: | Error case | Title shown | |---|---| | Email domain not in allowed list | "Unauthorized email" | | Malformed email (no `@`) with domain restrictions | "Unauthorized email" | | `email_verified` is `false` | "Email not verified" | All render HTTP 403 with `HideStatus: true` and a "Back to login" action button. ### `coderd/userauth_test.go` - Updated `AssertResponse` callbacks on existing table-driven tests (`EmailNotVerified`, `NotInRequiredEmailDomain`, `EmailDomainForbiddenWithLeadingAt`) to verify HTML Content-Type and page content. - Extended `TestOIDCDomainErrorMessage` to additionally assert HTML rendering. - Added new `TestOIDCErrorPageRendering` with 3 subtests covering all error scenarios, verifying: HTML doctype, expected title/description, "Back to login" link, and absence of JSON markers. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent fa86936 commit e94de0b

2 files changed

Lines changed: 84 additions & 11 deletions

File tree

‎coderd/userauth.go‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"github.com/coder/coder/v2/coderd/util/ptr"
4545
"github.com/coder/coder/v2/codersdk"
4646
"github.com/coder/coder/v2/cryptorand"
47+
"github.com/coder/coder/v2/site"
4748
)
4849

4950
type MergedClaimsSource string
@@ -1343,12 +1344,21 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
13431344
verified, ok := verifiedRaw.(bool)
13441345
if ok && !verified {
13451346
if !api.OIDCConfig.IgnoreEmailVerified {
1346-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
1347-
Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", email),
1347+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1348+
Status: http.StatusForbidden,
1349+
HideStatus: true,
1350+
Title: "Email not verified",
1351+
Description: fmt.Sprintf(
1352+
"Verify the %q email address on your OIDC provider to authenticate!",
1353+
email,
1354+
),
1355+
Actions: []site.Action{
1356+
{URL: "/login", Text: "Back to login"},
1357+
},
13481358
})
13491359
return
13501360
}
1351-
logger.Warn(ctx, "allowing unverified oidc email %q")
1361+
logger.Warn(ctx, "allowing unverified oidc email", slog.F("email", email))
13521362
}
13531363
}
13541364

@@ -1370,8 +1380,17 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
13701380
ok = false
13711381
emailSp := strings.Split(email, "@")
13721382
if len(emailSp) == 1 {
1373-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
1374-
Message: fmt.Sprintf("Your email %q is not from an authorized domain! Please contact your administrator.", email),
1383+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1384+
Status: http.StatusForbidden,
1385+
HideStatus: true,
1386+
Title: "Unauthorized email",
1387+
Description: fmt.Sprintf(
1388+
"Your email %q is not from an authorized domain! Please contact your administrator.",
1389+
email,
1390+
),
1391+
Actions: []site.Action{
1392+
{URL: "/login", Text: "Back to login"},
1393+
},
13751394
})
13761395
return
13771396
}
@@ -1385,8 +1404,17 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
13851404
}
13861405
}
13871406
if !ok {
1388-
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
1389-
Message: fmt.Sprintf("Your email %q is not from an authorized domain! Please contact your administrator.", email),
1407+
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1408+
Status: http.StatusForbidden,
1409+
HideStatus: true,
1410+
Title: "Unauthorized email",
1411+
Description: fmt.Sprintf(
1412+
"Your email %q is not from an authorized domain! Please contact your administrator.",
1413+
email,
1414+
),
1415+
Actions: []site.Action{
1416+
{URL: "/login", Text: "Back to login"},
1417+
},
13901418
})
13911419
return
13921420
}
@@ -1406,7 +1434,6 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
14061434
if ok {
14071435
picture, _ = pictureRaw.(string)
14081436
}
1409-
14101437
ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name))
14111438

14121439
user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), email)

‎coderd/userauth_test.go‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,10 +1107,21 @@ func TestUserOIDC(t *testing.T) {
11071107
},
11081108
AllowSignups: true,
11091109
StatusCode: http.StatusForbidden,
1110+
AssertResponse: func(t testing.TB, resp *http.Response) {
1111+
data, err := io.ReadAll(resp.Body)
1112+
require.NoError(t, err)
1113+
body := string(data)
1114+
// Should be an HTML error page, not JSON.
1115+
require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
1116+
require.Contains(t, body, "<!doctype html>")
1117+
require.Contains(t, body, "Email not verified")
1118+
require.Contains(t, body, "Verify the")
1119+
require.Contains(t, body, "Back to login")
1120+
require.NotContains(t, body, `"message"`)
1121+
},
11101122
},
11111123
{
1112-
Name: "EmailNotAString",
1113-
IDTokenClaims: jwt.MapClaims{
1124+
Name: "EmailNotAString", IDTokenClaims: jwt.MapClaims{
11141125
"email": 3.14159,
11151126
"email_verified": false,
11161127
"sub": uuid.NewString(),
@@ -1144,6 +1155,18 @@ func TestUserOIDC(t *testing.T) {
11441155
"coder.com",
11451156
},
11461157
StatusCode: http.StatusForbidden,
1158+
AssertResponse: func(t testing.TB, resp *http.Response) {
1159+
data, err := io.ReadAll(resp.Body)
1160+
require.NoError(t, err)
1161+
body := string(data)
1162+
// Should be an HTML error page, not JSON.
1163+
require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
1164+
require.Contains(t, body, "<!doctype html>")
1165+
require.Contains(t, body, "Unauthorized email")
1166+
require.Contains(t, body, "is not from an authorized domain")
1167+
require.Contains(t, body, "Back to login")
1168+
require.NotContains(t, body, `"message"`)
1169+
},
11471170
},
11481171
{
11491172
Name: "EmailDomainWithLeadingAt",
@@ -1170,6 +1193,18 @@ func TestUserOIDC(t *testing.T) {
11701193
"@coder.com",
11711194
},
11721195
StatusCode: http.StatusForbidden,
1196+
AssertResponse: func(t testing.TB, resp *http.Response) {
1197+
data, err := io.ReadAll(resp.Body)
1198+
require.NoError(t, err)
1199+
body := string(data)
1200+
// Should be an HTML error page, not JSON.
1201+
require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
1202+
require.Contains(t, body, "<!doctype html>")
1203+
require.Contains(t, body, "Unauthorized email")
1204+
require.Contains(t, body, "is not from an authorized domain")
1205+
require.Contains(t, body, "Back to login")
1206+
require.NotContains(t, body, `"message"`)
1207+
},
11731208
},
11741209
{
11751210
Name: "EmailDomainCaseInsensitive",
@@ -2062,6 +2097,12 @@ func TestOIDCDomainErrorMessage(t *testing.T) {
20622097

20632098
require.Contains(t, string(data), "is not from an authorized domain")
20642099
require.Contains(t, string(data), "Please contact your administrator")
2100+
// Verify the response is a rendered HTML error page, not raw JSON.
2101+
require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
2102+
require.Contains(t, string(data), "<!doctype html>")
2103+
require.Contains(t, string(data), "Unauthorized email")
2104+
require.Contains(t, string(data), "Back to login")
2105+
require.NotContains(t, string(data), `"message"`)
20652106

20662107
for _, domain := range allowedDomains {
20672108
require.NotContains(t, string(data), domain)
@@ -2091,7 +2132,12 @@ func TestOIDCDomainErrorMessage(t *testing.T) {
20912132

20922133
require.Contains(t, string(data), "is not from an authorized domain")
20932134
require.Contains(t, string(data), "Please contact your administrator")
2094-
2135+
// Verify the response is a rendered HTML error page, not raw JSON.
2136+
require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
2137+
require.Contains(t, string(data), "<!doctype html>")
2138+
require.Contains(t, string(data), "Unauthorized email")
2139+
require.Contains(t, string(data), "Back to login")
2140+
require.NotContains(t, string(data), `"message"`)
20952141
for _, domain := range allowedDomains {
20962142
require.NotContains(t, string(data), domain)
20972143
}

0 commit comments

Comments
 (0)