control plane: add request id to all error pages (#2149)

* controlplane: add request id to all error pages

- use a single http error handler for both envoy and go control plane
- add http lib style status text for our custom statuses.

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>

authorize/check_response.go

@@ -1,12 +1,13 @@
 package authorize
 
 import (
-	"bytes"
 	"context"
+	"errors"
+	"io"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"sort"
-	"strings"
 
 	envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_service_auth_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
@@ -18,6 +19,7 @@ import (
 	"github.com/pomerium/pomerium/authorize/evaluator"
 	"github.com/pomerium/pomerium/internal/httputil"
 	"github.com/pomerium/pomerium/internal/log"
+	"github.com/pomerium/pomerium/internal/telemetry/requestid"
 	"github.com/pomerium/pomerium/internal/urlutil"
 )
 
@@ -41,79 +43,51 @@ func (a *Authorize) okResponse(reply *evaluator.Result) *envoy_service_auth_v3.C
 }
 
 func (a *Authorize) deniedResponse(
+	ctx context.Context,
 	in *envoy_service_auth_v3.CheckRequest,
 	code int32, reason string, headers map[string]string,
 ) (*envoy_service_auth_v3.CheckResponse, error) {
-	returnHTMLError := true
-	inHeaders := in.GetAttributes().GetRequest().GetHttp().GetHeaders()
-	if inHeaders != nil {
-		returnHTMLError = strings.Contains(inHeaders["accept"], "text/html")
-	}
-
-	if returnHTMLError {
-		return a.htmlDeniedResponse(in, code, reason, headers)
-	}
-	return a.plainTextDeniedResponse(code, reason, headers), nil
-}
-
-func (a *Authorize) htmlDeniedResponse(
-	in *envoy_service_auth_v3.CheckRequest,
-	code int32, reason string, headers map[string]string,
-) (*envoy_service_auth_v3.CheckResponse, error) {
-	opts := a.currentOptions.Load()
-	authenticateURL, err := opts.GetAuthenticateURL()
-	if err != nil {
-		return nil, err
-	}
-	debugEndpoint := authenticateURL.ResolveReference(&url.URL{Path: "/.pomerium/"})
-
-	// create go-style http request
-	r := getHTTPRequestFromCheckRequest(in)
-	redirectURL := urlutil.GetAbsoluteURL(r).String()
-	if ref := r.Header.Get(httputil.HeaderReferrer); ref != "" {
-		redirectURL = ref
-	}
-
-	debugEndpoint = debugEndpoint.ResolveReference(&url.URL{
-		RawQuery: url.Values{
-			urlutil.QueryRedirectURI: {redirectURL},
-		}.Encode(),
-	})
-
-	debugEndpoint = urlutil.NewSignedURL(a.state.Load().sharedKey, debugEndpoint).Sign()
 
 	var details string
 	switch code {
 	case httputil.StatusInvalidClientCertificate:
-		details = "a valid client certificate is required to access this page"
+		details = httputil.StatusText(httputil.StatusInvalidClientCertificate)
 	case http.StatusForbidden:
-		details = "access to this page is forbidden"
+		details = http.StatusText(http.StatusForbidden)
 	default:
 		details = reason
 	}
 
-	if reason == "" {
-		reason = http.StatusText(int(code))
-	}
+	// create a http response writer recorder
+	w := httptest.NewRecorder()
+	r := getHTTPRequestFromCheckRequest(in)
 
-	var buf bytes.Buffer
-	err = a.templates.ExecuteTemplate(&buf, "error.html", map[string]interface{}{
-		"Status":     code,
-		"StatusText": reason,
-		"CanDebug":   code/100 == 4,
-		"DebugURL":   debugEndpoint,
-		"Error":      details,
-	})
+	// build the user info / debug endpoint
+	debugEndpoint, _ := a.userInfoEndpointURL(in) // if there's an error, we just wont display it
+
+	// run the request through our go error handler
+	httpErr := httputil.HTTPError{
+		Status:    int(code),
+		Err:       errors.New(details),
+		DebugURL:  debugEndpoint,
+		RequestID: requestid.FromContext(ctx),
+	}
+	httpErr.ErrorResponse(w, r)
+
+	// transpose the go http response writer into a envoy response
+	resp := w.Result()
+	defer resp.Body.Close()
+	respBody, err := io.ReadAll(resp.Body)
 	if err != nil {
-		buf.WriteString(reason)
-		log.Error(context.TODO()).Err(err).Msg("error executing error template")
+		log.Error(ctx).Err(err).Msg("error executing error template")
+		return nil, err
 	}
+	// convert go headers to envoy headers
+	respHeader := toEnvoyHeaders(resp.Header)
 
-	envoyHeaders := []*envoy_config_core_v3.HeaderValueOption{
-		mkHeader("Content-Type", "text/html", false),
-	}
+	// add any additional headers
 	for k, v := range headers {
-		envoyHeaders = append(envoyHeaders, mkHeader(k, v, false))
+		respHeader = append(respHeader, mkHeader(k, v, false))
 	}
 
 	return &envoy_service_auth_v3.CheckResponse{
@@ -123,36 +97,14 @@ func (a *Authorize) htmlDeniedResponse(
 				Status: &envoy_type_v3.HttpStatus{
 					Code: envoy_type_v3.StatusCode(code),
 				},
-				Headers: envoyHeaders,
-				Body:    buf.String(),
+				Headers: respHeader,
+				Body:    string(respBody),
 			},
 		},
 	}, nil
 }
 
-func (a *Authorize) plainTextDeniedResponse(code int32, reason string, headers map[string]string) *envoy_service_auth_v3.CheckResponse {
-	envoyHeaders := []*envoy_config_core_v3.HeaderValueOption{
-		mkHeader("Content-Type", "text/plain", false),
-	}
-	for k, v := range headers {
-		envoyHeaders = append(envoyHeaders, mkHeader(k, v, false))
-	}
-
-	return &envoy_service_auth_v3.CheckResponse{
-		Status: &status.Status{Code: int32(codes.PermissionDenied), Message: "Access Denied"},
-		HttpResponse: &envoy_service_auth_v3.CheckResponse_DeniedResponse{
-			DeniedResponse: &envoy_service_auth_v3.DeniedHttpResponse{
-				Status: &envoy_type_v3.HttpStatus{
-					Code: envoy_type_v3.StatusCode(code),
-				},
-				Headers: envoyHeaders,
-				Body:    reason,
-			},
-		},
-	}
-}
-
-func (a *Authorize) redirectResponse(in *envoy_service_auth_v3.CheckRequest) (*envoy_service_auth_v3.CheckResponse, error) {
+func (a *Authorize) redirectResponse(ctx context.Context, in *envoy_service_auth_v3.CheckRequest) (*envoy_service_auth_v3.CheckResponse, error) {
 	opts := a.currentOptions.Load()
 	state := a.state.Load()
 	authenticateURL, err := opts.GetAuthenticateURL()
@@ -173,7 +125,7 @@ func (a *Authorize) redirectResponse(in *envoy_service_auth_v3.CheckRequest) (*e
 	signinURL.RawQuery = q.Encode()
 	redirectTo := urlutil.NewSignedURL(state.sharedKey, signinURL).String()
 
-	return a.deniedResponse(in, http.StatusFound, "Login", map[string]string{
+	return a.deniedResponse(ctx, in, http.StatusFound, "Login", map[string]string{
 		"Location": redirectTo,
 	})
 }
@@ -189,3 +141,42 @@ func mkHeader(k, v string, shouldAppend bool) *envoy_config_core_v3.HeaderValueO
 		},
 	}
 }
+
+func toEnvoyHeaders(headers http.Header) []*envoy_config_core_v3.HeaderValueOption {
+	var ks []string
+	for k := range headers {
+		ks = append(ks, k)
+	}
+	sort.Strings(ks)
+
+	envoyHeaders := make([]*envoy_config_core_v3.HeaderValueOption, 0, len(headers))
+	for _, k := range ks {
+		envoyHeaders = append(envoyHeaders, mkHeader(k, headers.Get(k), false))
+	}
+	return envoyHeaders
+}
+
+// userInfoEndpointURL returns the user info endpoint url which can be used to debug the user's
+// session that lives on the authenticate service.
+func (a *Authorize) userInfoEndpointURL(in *envoy_service_auth_v3.CheckRequest) (*url.URL, error) {
+	opts := a.currentOptions.Load()
+	authenticateURL, err := opts.GetAuthenticateURL()
+	if err != nil {
+		return nil, err
+	}
+	debugEndpoint := authenticateURL.ResolveReference(&url.URL{Path: "/.pomerium/"})
+
+	r := getHTTPRequestFromCheckRequest(in)
+	redirectURL := urlutil.GetAbsoluteURL(r).String()
+	if ref := r.Header.Get(httputil.HeaderReferrer); ref != "" {
+		redirectURL = ref
+	}
+
+	debugEndpoint = debugEndpoint.ResolveReference(&url.URL{
+		RawQuery: url.Values{
+			urlutil.QueryRedirectURI: {redirectURL},
+		}.Encode(),
+	})
+
+	return urlutil.NewSignedURL(a.state.Load().sharedKey, debugEndpoint).Sign(), nil
+}

authorize/check_response_test.go

@@ -1,6 +1,7 @@
 package authorize
 
 import (
+	"context"
 	"html/template"
 	"net/http"
 	"net/url"
@@ -151,36 +152,8 @@ func TestAuthorize_deniedResponse(t *testing.T) {
 							Code: envoy_type_v3.StatusCode(codes.InvalidArgument),
 						},
 						Headers: []*envoy_config_core_v3.HeaderValueOption{
-							mkHeader("Content-Type", "text/html", false),
-						},
-						Body: "Access Denied",
-					},
-				},
-			},
-		},
-		{
-			"plain text denied",
-			&envoy_service_auth_v3.CheckRequest{
-				Attributes: &envoy_service_auth_v3.AttributeContext{
-					Request: &envoy_service_auth_v3.AttributeContext_Request{
-						Http: &envoy_service_auth_v3.AttributeContext_HttpRequest{
-							Headers: map[string]string{},
-						},
-					},
-				},
-			},
-			http.StatusBadRequest,
-			"Access Denied",
-			map[string]string{},
-			&envoy_service_auth_v3.CheckResponse{
-				Status: &status.Status{Code: int32(codes.PermissionDenied), Message: "Access Denied"},
-				HttpResponse: &envoy_service_auth_v3.CheckResponse_DeniedResponse{
-					DeniedResponse: &envoy_service_auth_v3.DeniedHttpResponse{
-						Status: &envoy_type_v3.HttpStatus{
-							Code: envoy_type_v3.StatusCode(codes.InvalidArgument),
-						},
-						Headers: []*envoy_config_core_v3.HeaderValueOption{
-							mkHeader("Content-Type", "text/plain", false),
+							mkHeader("Content-Type", "text/html; charset=UTF-8", false),
+							mkHeader("X-Pomerium-Intercepted-Response", "true", false),
 						},
 						Body: "Access Denied",
 					},
@@ -192,7 +165,7 @@ func TestAuthorize_deniedResponse(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			got, err := a.deniedResponse(tc.in, tc.code, tc.reason, tc.headers)
+			got, err := a.deniedResponse(context.TODO(), tc.in, tc.code, tc.reason, tc.headers)
 			require.NoError(t, err)
 			assert.Equal(t, tc.want.Status.Code, got.Status.Code)
 			assert.Equal(t, tc.want.Status.Message, got.Status.Message)

authorize/grpc.go

@@ -70,11 +70,11 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe
 		return a.okResponse(reply), nil
 	case reply.Status == http.StatusUnauthorized:
 		if isForwardAuth && hreq.URL.Path == "/verify" {
-			return a.deniedResponse(in, http.StatusUnauthorized, "Unauthenticated", nil)
+			return a.deniedResponse(ctx, in, http.StatusUnauthorized, "Unauthenticated", nil)
 		}
-		return a.redirectResponse(in)
+		return a.redirectResponse(ctx, in)
 	}
-	return a.deniedResponse(in, int32(reply.Status), reply.Message, nil)
+	return a.deniedResponse(ctx, in, int32(reply.Status), reply.Message, nil)
 }
 
 func getForwardAuthURL(r *http.Request) *url.URL {

internal/frontend/assets/html/error.go.html

@@ -24,8 +24,17 @@
           </div>
         </div>
         <div class="category-link">
-          {{if and .CanDebug .DebugURL }}
-            If you should have access, contact your administrator with your <a href="{{.DebugURL}}">session details</a>.
+          {{if .CanDebug}}
+            If you should have access, contact your administrator with your
+            {{if .RequestID }}
+            request id {{.RequestID}}
+            {{end}}
+            {{if and .RequestID .DebugURL }}
+            and
+            {{end}}
+            {{if .DebugURL }}
+            <a href="{{.DebugURL}}">session details</a>.
+            {{end}}
           {{end}}
         </div>
       </div>

internal/httputil/errors.go

@@ -3,9 +3,9 @@ package httputil
 import (
 	"html/template"
 	"net/http"
+	"net/url"
 
 	"github.com/pomerium/pomerium/internal/frontend"
-	"github.com/pomerium/pomerium/internal/log"
 	"github.com/pomerium/pomerium/internal/telemetry/requestid"
 )
 
@@ -15,8 +15,12 @@ var errorTemplate = template.Must(frontend.NewTemplates())
 type HTTPError struct {
 	// HTTP status codes as registered with IANA.
 	Status int
-	// Err is the wrapped error
+	// Err is the wrapped error.
 	Err error
+	// DebugURL is the URL to the debug endpoint.
+	DebugURL *url.URL
+	// The request ID.
+	RequestID string
 }
 
 // NewError returns an error that contains a HTTP status and error.
@@ -36,32 +40,35 @@ func (e *HTTPError) Unwrap() error { return e.Err }
 // It does not otherwise end the request; the caller should ensure no further
 // writes are done to w.
 func (e *HTTPError) ErrorResponse(w http.ResponseWriter, r *http.Request) {
-	// indicate to clients that the error originates from Pomerium, not the app
-	w.Header().Set(HeaderPomeriumResponse, "true")
-	w.WriteHeader(e.Status)
-
-	log.FromRequest(r).Info().Err(e).Msg("httputil: ErrorResponse")
-	requestID := requestid.FromContext(r.Context())
-
+	reqID := e.RequestID
+	if e.RequestID == "" {
+		// if empty, try to grab from the request id from the request context
+		reqID = requestid.FromContext(r.Context())
+	}
 	response := struct {
 		Status     int
 		Error      string
-		StatusText string `json:"-"`
-		RequestID  string `json:",omitempty"`
-		CanDebug   bool   `json:"-"`
-		Version    string `json:"-"`
+		StatusText string   `json:"-"`
+		RequestID  string   `json:",omitempty"`
+		CanDebug   bool     `json:"-"`
+		Version    string   `json:"-"`
+		DebugURL   *url.URL `json:",omitempty"`
 	}{
 		Status:     e.Status,
 		StatusText: http.StatusText(e.Status),
 		Error:      e.Error(),
-		RequestID:  requestID,
-		CanDebug:   e.Status/100 == 4,
+		RequestID:  reqID,
+		CanDebug:   e.Status/100 == 4 && (e.DebugURL != nil || reqID != ""),
+		DebugURL:   e.DebugURL,
 	}
+	// indicate to clients that the error originates from Pomerium, not the app
+	w.Header().Set(HeaderPomeriumResponse, "true")
 
 	if r.Header.Get("Accept") == "application/json" {
 		RenderJSON(w, e.Status, response)
 		return
 	}
 	w.Header().Set("Content-Type", "text/html; charset=UTF-8")
+	w.WriteHeader(e.Status)
 	errorTemplate.ExecuteTemplate(w, "error.html", response)
 }

internal/httputil/handlers.go

@@ -59,7 +59,7 @@ func (f HandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if err := f(w, r); err != nil {
 		var e *HTTPError
 		if !errors.As(err, &e) {
-			e = &HTTPError{http.StatusInternalServerError, err}
+			e = &HTTPError{Status: http.StatusInternalServerError, Err: err}
 		}
 		e.ErrorResponse(w, r)
 	}

internal/httputil/httputil.go

@@ -4,3 +4,13 @@ package httputil
 // client's certificate is invalid. This is the same status code used
 // by nginx for this purpose.
 const StatusInvalidClientCertificate = 495
+
+var statusText = map[int]string{
+	StatusInvalidClientCertificate: "a valid client certificate is required to access this page",
+}
+
+// StatusText returns a text for the HTTP status code. It returns the empty
+// string if the code is unknown.
+func StatusText(code int) string {
+	return statusText[code]
+}