From e94de0bdabcaeed735d468bb6d279d06f8b8b2d1 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:56:59 -0400 Subject: [PATCH] fix(coderd): render HTML error page for OIDC email validation failures (#23059) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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> --- coderd/userauth.go | 43 +++++++++++++++++++++++++++------- coderd/userauth_test.go | 52 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 42a8f3871b..6b2aab6c53 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -44,6 +44,7 @@ import ( "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" + "github.com/coder/coder/v2/site" ) type MergedClaimsSource string @@ -1343,12 +1344,21 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { verified, ok := verifiedRaw.(bool) if ok && !verified { if !api.OIDCConfig.IgnoreEmailVerified { - httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", email), + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusForbidden, + HideStatus: true, + Title: "Email not verified", + Description: fmt.Sprintf( + "Verify the %q email address on your OIDC provider to authenticate!", + email, + ), + Actions: []site.Action{ + {URL: "/login", Text: "Back to login"}, + }, }) return } - logger.Warn(ctx, "allowing unverified oidc email %q") + logger.Warn(ctx, "allowing unverified oidc email", slog.F("email", email)) } } @@ -1370,8 +1380,17 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { ok = false emailSp := strings.Split(email, "@") if len(emailSp) == 1 { - httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: fmt.Sprintf("Your email %q is not from an authorized domain! Please contact your administrator.", email), + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusForbidden, + HideStatus: true, + Title: "Unauthorized email", + Description: fmt.Sprintf( + "Your email %q is not from an authorized domain! Please contact your administrator.", + email, + ), + Actions: []site.Action{ + {URL: "/login", Text: "Back to login"}, + }, }) return } @@ -1385,8 +1404,17 @@ 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 from an authorized domain! Please contact your administrator.", email), + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusForbidden, + HideStatus: true, + Title: "Unauthorized email", + Description: fmt.Sprintf( + "Your email %q is not from an authorized domain! Please contact your administrator.", + email, + ), + Actions: []site.Action{ + {URL: "/login", Text: "Back to login"}, + }, }) return } @@ -1406,7 +1434,6 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { if ok { picture, _ = pictureRaw.(string) } - ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name)) user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), email) diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index d78c77f45e..b5df59ae82 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -1107,10 +1107,21 @@ func TestUserOIDC(t *testing.T) { }, AllowSignups: true, StatusCode: http.StatusForbidden, + AssertResponse: func(t testing.TB, resp *http.Response) { + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + body := string(data) + // Should be an HTML error page, not JSON. + require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) + require.Contains(t, body, "") + require.Contains(t, body, "Email not verified") + require.Contains(t, body, "Verify the") + require.Contains(t, body, "Back to login") + require.NotContains(t, body, `"message"`) + }, }, { - Name: "EmailNotAString", - IDTokenClaims: jwt.MapClaims{ + Name: "EmailNotAString", IDTokenClaims: jwt.MapClaims{ "email": 3.14159, "email_verified": false, "sub": uuid.NewString(), @@ -1144,6 +1155,18 @@ func TestUserOIDC(t *testing.T) { "coder.com", }, StatusCode: http.StatusForbidden, + AssertResponse: func(t testing.TB, resp *http.Response) { + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + body := string(data) + // Should be an HTML error page, not JSON. + require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) + require.Contains(t, body, "") + require.Contains(t, body, "Unauthorized email") + require.Contains(t, body, "is not from an authorized domain") + require.Contains(t, body, "Back to login") + require.NotContains(t, body, `"message"`) + }, }, { Name: "EmailDomainWithLeadingAt", @@ -1170,6 +1193,18 @@ func TestUserOIDC(t *testing.T) { "@coder.com", }, StatusCode: http.StatusForbidden, + AssertResponse: func(t testing.TB, resp *http.Response) { + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + body := string(data) + // Should be an HTML error page, not JSON. + require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) + require.Contains(t, body, "") + require.Contains(t, body, "Unauthorized email") + require.Contains(t, body, "is not from an authorized domain") + require.Contains(t, body, "Back to login") + require.NotContains(t, body, `"message"`) + }, }, { Name: "EmailDomainCaseInsensitive", @@ -2062,6 +2097,12 @@ func TestOIDCDomainErrorMessage(t *testing.T) { require.Contains(t, string(data), "is not from an authorized domain") require.Contains(t, string(data), "Please contact your administrator") + // Verify the response is a rendered HTML error page, not raw JSON. + require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) + require.Contains(t, string(data), "") + require.Contains(t, string(data), "Unauthorized email") + require.Contains(t, string(data), "Back to login") + require.NotContains(t, string(data), `"message"`) for _, domain := range allowedDomains { require.NotContains(t, string(data), domain) @@ -2091,7 +2132,12 @@ func TestOIDCDomainErrorMessage(t *testing.T) { require.Contains(t, string(data), "is not from an authorized domain") require.Contains(t, string(data), "Please contact your administrator") - + // Verify the response is a rendered HTML error page, not raw JSON. + require.Equal(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type")) + require.Contains(t, string(data), "") + require.Contains(t, string(data), "Unauthorized email") + require.Contains(t, string(data), "Back to login") + require.NotContains(t, string(data), `"message"`) for _, domain := range allowedDomains { require.NotContains(t, string(data), domain) }