httputil: remove error details (#3703)

authenticate/handlers_test.go

@@ -294,7 +294,7 @@ func TestAuthenticate_SignOut(t *testing.T) {
 			identity.MockProvider{LogOutResponse: (*uriParseHelper("https://microsoft.com"))},
 			&mstore.Store{Encrypted: true, Session: &sessions.State{}},
 			http.StatusOK,
-			"{\"Status\":200,\"Error\":\"OK: user logged out\"}\n",
+			"{\"Status\":200}\n",
 		},
 	}
 	for _, tt := range tests {

internal/httputil/errors.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/url"
 
+	"github.com/pomerium/pomerium/internal/log"
 	"github.com/pomerium/pomerium/internal/telemetry/requestid"
 	"github.com/pomerium/pomerium/pkg/contextutil"
 	"github.com/pomerium/pomerium/ui"
@@ -52,7 +53,6 @@ func (e *HTTPError) ErrorResponse(ctx context.Context, w http.ResponseWriter, r
 	}
 	response := struct {
 		Status                 int
-		Error                  string
 		StatusText             string                              `json:"-"`
 		RequestID              string                              `json:",omitempty"`
 		CanDebug               bool                                `json:"-"`
@@ -61,7 +61,6 @@ func (e *HTTPError) ErrorResponse(ctx context.Context, w http.ResponseWriter, r
 	}{
 		Status:                 e.Status,
 		StatusText:             StatusText(e.Status),
-		Error:                  e.Error(),
 		RequestID:              reqID,
 		CanDebug:               e.Status/100 == 4 && (e.DebugURL != nil || reqID != ""),
 		DebugURL:               e.DebugURL,
@@ -70,6 +69,13 @@ func (e *HTTPError) ErrorResponse(ctx context.Context, w http.ResponseWriter, r
 	// indicate to clients that the error originates from Pomerium, not the app
 	w.Header().Set(HeaderPomeriumResponse, "true")
 
+	log.Error(ctx).
+		Err(e.Err).
+		Int("status", e.Status).
+		Str("status-text", StatusText(e.Status)).
+		Str("request-id", reqID).
+		Msg("httputil: error")
+
 	if r.Header.Get("Accept") == "application/json" {
 		RenderJSON(w, e.Status, response)
 		return
@@ -77,7 +83,6 @@ func (e *HTTPError) ErrorResponse(ctx context.Context, w http.ResponseWriter, r
 
 	m := map[string]any{
 		"canDebug":               response.CanDebug,
-		"error":                  response.Error,
 		"requestId":              response.RequestID,
 		"status":                 response.Status,
 		"statusText":             response.StatusText,

internal/httputil/errors_test.go

@@ -19,7 +19,7 @@ func TestHTTPError_ErrorResponse(t *testing.T) {
 		wantStatus int
 		wantBody   string
 	}{
-		{"404 json", http.StatusNotFound, errors.New("route not known"), "application/json", http.StatusNotFound, "{\"Status\":404,\"Error\":\"Not Found: route not known\"}\n"},
+		{"404 json", http.StatusNotFound, errors.New("route not known"), "application/json", http.StatusNotFound, "{\"Status\":404}\n"},
 		{"404 html", http.StatusNotFound, errors.New("route not known"), "", http.StatusNotFound, ""},
 	}
 	for _, tt := range tests {

internal/httputil/handlers_test.go

@@ -75,8 +75,8 @@ func TestHandlerFunc_ServeHTTP(t *testing.T) {
 		f        HandlerFunc
 		wantBody string
 	}{
-		{"good http error", func(w http.ResponseWriter, r *http.Request) error { return NewError(404, errors.New("404")) }, "{\"Status\":404,\"Error\":\"Not Found: 404\"}\n"},
+		{"good http error", func(w http.ResponseWriter, r *http.Request) error { return NewError(404, errors.New("404")) }, "{\"Status\":404}\n"},
-		{"good std error", func(w http.ResponseWriter, r *http.Request) error { return errors.New("404") }, "{\"Status\":500,\"Error\":\"Internal Server Error: 404\"}\n"},
+		{"good std error", func(w http.ResponseWriter, r *http.Request) error { return errors.New("404") }, "{\"Status\":500}\n"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/middleware/middleware_test.go

@@ -55,7 +55,7 @@ func TestValidateSignature(t *testing.T) {
 		wantBody   string
 	}{
 		{"good", []byte("secret"), []byte("secret"), http.StatusOK, http.StatusText(http.StatusOK)},
-		{"secret mistmatch", []byte("secret"), []byte("hunter42"), http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: internal/urlutil: hmac failed\"}\n"},
+		{"secret mistmatch", []byte("secret"), []byte("hunter42"), http.StatusBadRequest, "{\"Status\":400}\n"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

proxy/forward_auth_test.go

@@ -63,9 +63,9 @@ func TestProxy_ForwardAuth(t *testing.T) {
 		wantBody     string
 	}{
 		{"good verify only, no redirect", opts, nil, http.MethodGet, nil, nil, "https://some.domain.example/verify", "https://some.domain.example", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusOK, ""},
-		{"bad empty domain uri", opts, nil, http.MethodGet, nil, map[string]string{"uri": ""}, "https://some.domain.example/", "", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: https: url does contain a valid hostname\"}\n"},
+		{"bad empty domain uri", opts, nil, http.MethodGet, nil, map[string]string{"uri": ""}, "https://some.domain.example/", "", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400}\n"},
-		{"bad naked domain uri", opts, nil, http.MethodGet, nil, nil, "https://some.domain.example/", "a.naked.domain", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: a.naked.domain url does contain a valid scheme\"}\n"},
+		{"bad naked domain uri", opts, nil, http.MethodGet, nil, nil, "https://some.domain.example/", "a.naked.domain", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400}\n"},
-		{"bad empty verification uri", opts, nil, http.MethodGet, nil, nil, "https://some.domain.example/", " ", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400,\"Error\":\"Bad Request: %20 url does contain a valid scheme\"}\n"},
+		{"bad empty verification uri", opts, nil, http.MethodGet, nil, nil, "https://some.domain.example/", " ", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, "{\"Status\":400}\n"},
 		// traefik
 		{"good traefik callback", opts, nil, http.MethodGet, map[string]string{httputil.HeaderForwardedURI: "https://some.domain.example?" + urlutil.QuerySessionEncrypted + "=" + goodEncryptionString}, nil, "https://some.domain.example/", "https://some.domain.example", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusFound, ""},
 		{"bad traefik callback bad session", opts, nil, http.MethodGet, map[string]string{httputil.HeaderForwardedURI: "https://some.domain.example?" + urlutil.QuerySessionEncrypted + "=" + goodEncryptionString + "garbage"}, nil, "https://some.domain.example/", "https://some.domain.example", &mock.Encoder{}, &mstore.Store{Session: &sessions.State{}}, allowClient, http.StatusBadRequest, ""},

proxy/handlers_test.go

@@ -331,14 +331,14 @@ func TestProxy_ProgrammaticLogin(t *testing.T) {
 			opts, http.MethodGet, "https", "corp.example.example", "/.pomerium/api/v1/login", nil,
 			map[string]string{urlutil.QueryRedirectURI: "localhost"},
 			http.StatusBadRequest,
-			"{\"Status\":400,\"Error\":\"Bad Request: localhost url does contain a valid scheme\"}\n",
+			"{\"Status\":400}\n",
 		},
 		{
 			"bad redirect_uri not whitelisted",
 			opts, http.MethodGet, "https", "corp.example.example", "/.pomerium/api/v1/login", nil,
 			map[string]string{urlutil.QueryRedirectURI: "https://example.com"},
 			http.StatusBadRequest,
-			"{\"Status\":400,\"Error\":\"Bad Request: invalid redirect uri\"}\n",
+			"{\"Status\":400}\n",
 		},
 		{
 			"bad http method",

ui/src/components/ErrorPage.tsx

@@ -1,30 +1,31 @@
-import {ErrorPageData, PolicyEvaluationTrace} from "../types";
+import { ListItemProps, TableCell } from "@mui/material";
-import SectionFooter from "./SectionFooter";
 import Alert from "@mui/material/Alert";
 import AlertTitle from "@mui/material/AlertTitle";
 import Box from "@mui/material/Box";
 import Container from "@mui/material/Container";
 import Paper from "@mui/material/Paper";
 import Stack from "@mui/material/Stack";
-import Typography from "@mui/material/Typography";
-import React, { FC } from "react";
-import Markdown from "markdown-to-jsx";
-import {ListItemProps, TableCell} from "@mui/material";
-import {CheckCircle, MinusCircle, XCircle} from "react-feather";
 import Table from "@mui/material/Table";
-import TableRow from "@mui/material/TableRow";
 import TableHead from "@mui/material/TableHead";
+import TableRow from "@mui/material/TableRow";
+import Typography from "@mui/material/Typography";
+import Markdown from "markdown-to-jsx";
+import React, { FC } from "react";
+import { CheckCircle, MinusCircle, XCircle } from "react-feather";
+
+import { ErrorPageData, PolicyEvaluationTrace } from "../types";
+import SectionFooter from "./SectionFooter";
 
 type PolicyEvaluationTraceDetailsProps = {
   trace: PolicyEvaluationTrace;
 } & ListItemProps;
 const PolicyEvaluationTraceDetails: FC<PolicyEvaluationTraceDetailsProps> = ({
- trace,
+  trace,
- ...props
+  ...props
 }) => {
   return (
     <TableRow>
-      <TableCell align={'center'}>
+      <TableCell align={"center"}>
         {trace.deny ? (
           <XCircle color="red" />
         ) : trace.allow ? (
@@ -34,9 +35,7 @@ const PolicyEvaluationTraceDetails: FC<PolicyEvaluationTraceDetailsProps> = ({
         )}
       </TableCell>
       <TableCell>
-        <Markdown>
+        <Markdown>{trace.explanation || trace.id}</Markdown>
-          {trace.explanation || trace.id}
-        </Markdown>
       </TableCell>
       <TableCell>
         <Markdown>
@@ -63,14 +62,11 @@ export const ErrorPage: FC<ErrorPageProps> = ({ data }) => {
                 {data?.status || 500}{" "}
                 {data?.statusText || "Internal Server Error"}
               </AlertTitle>
-              {data?.error || "Internal Server Error"}
             </Alert>
           </Box>
           {!!data?.errorMessageFirstParagraph && (
-            <Box sx={{p: 4}}>
+            <Box sx={{ p: 4 }}>
-              <Markdown>
+              <Markdown>{data.errorMessageFirstParagraph}</Markdown>
-                {data.errorMessageFirstParagraph}
-              </Markdown>
             </Box>
           )}
           {traces?.length > 0 && (

ui/src/types/index.ts

@@ -92,7 +92,6 @@ export type ErrorPageData = BasePageData & {
 
   canDebug?: boolean;
   debugUrl?: string;
-  error?: string;
   requestId?: string;
   status?: number;
   statusText?: string;