diff --git a/authenticate/handlers_test.go b/authenticate/handlers_test.go index e2903eaf2..d8aa493df 100644 --- a/authenticate/handlers_test.go +++ b/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 { diff --git a/internal/httputil/errors.go b/internal/httputil/errors.go index 6ad70099b..980664c24 100644 --- a/internal/httputil/errors.go +++ b/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, diff --git a/internal/httputil/errors_test.go b/internal/httputil/errors_test.go index 020f8d192..41018bf40 100644 --- a/internal/httputil/errors_test.go +++ b/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 { diff --git a/internal/httputil/handlers_test.go b/internal/httputil/handlers_test.go index 2786bae9c..3ccd8b973 100644 --- a/internal/httputil/handlers_test.go +++ b/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 std error", func(w http.ResponseWriter, r *http.Request) error { return errors.New("404") }, "{\"Status\":500,\"Error\":\"Internal Server Error: 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}\n"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go index 26d80125b..86ef88f16 100644 --- a/internal/middleware/middleware_test.go +++ b/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) { diff --git a/proxy/forward_auth_test.go b/proxy/forward_auth_test.go index a732a9856..e17571977 100644 --- a/proxy/forward_auth_test.go +++ b/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 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 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 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}\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, ""}, diff --git a/proxy/handlers_test.go b/proxy/handlers_test.go index f842e7615..3e6b1e517 100644 --- a/proxy/handlers_test.go +++ b/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", diff --git a/ui/src/components/ErrorPage.tsx b/ui/src/components/ErrorPage.tsx index 77fe07a89..6e4b33200 100644 --- a/ui/src/components/ErrorPage.tsx +++ b/ui/src/components/ErrorPage.tsx @@ -1,30 +1,31 @@ -import {ErrorPageData, PolicyEvaluationTrace} from "../types"; -import SectionFooter from "./SectionFooter"; +import { ListItemProps, TableCell } from "@mui/material"; 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 = ({ - trace, - ...props + trace, + ...props }) => { return ( - + {trace.deny ? ( ) : trace.allow ? ( @@ -34,9 +35,7 @@ const PolicyEvaluationTraceDetails: FC = ({ )} - - {trace.explanation || trace.id} - + {trace.explanation || trace.id} @@ -63,14 +62,11 @@ export const ErrorPage: FC = ({ data }) => { {data?.status || 500}{" "} {data?.statusText || "Internal Server Error"} - {data?.error || "Internal Server Error"} {!!data?.errorMessageFirstParagraph && ( - - - {data.errorMessageFirstParagraph} - + + {data.errorMessageFirstParagraph} )} {traces?.length > 0 && ( diff --git a/ui/src/types/index.ts b/ui/src/types/index.ts index 55c1ee6e0..1d5df7a46 100644 --- a/ui/src/types/index.ts +++ b/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;