Allow users to choose detailed error messages on error pages

oauthproxy.go

@@ -128,6 +128,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		ProxyPrefix: opts.ProxyPrefix,
 		Footer:      opts.Templates.Footer,
 		Version:     VERSION,
+		Debug:       opts.Templates.Debug,
 	}
 
 	upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), errorPage.ProxyErrorHandler)
@@ -523,7 +524,7 @@ func (p *OAuthProxy) RobotsTxt(rw http.ResponseWriter, req *http.Request) {
 }
 
 // ErrorPage writes an error response
-func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, appError string) {
+func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, appError string, messages ...interface{}) {
 	redirectURL, err := p.getAppRedirect(req)
 	if err != nil {
 		logger.Errorf("Error obtaining redirect: %v", err)
@@ -532,7 +533,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code i
 		redirectURL = "/"
 	}
 
-	p.errorPage.Render(rw, code, redirectURL, appError)
+	p.errorPage.Render(rw, code, redirectURL, appError, messages...)
 }
 
 // IsAllowedRequest is used to check if auth should be skipped for this request
@@ -751,28 +752,30 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 	errorString := req.Form.Get("error")
 	if errorString != "" {
 		logger.Errorf("Error while parsing OAuth2 callback: %s", errorString)
-		p.ErrorPage(rw, req, http.StatusForbidden, errorString)
+		message := fmt.Sprintf("Login Failed: The upstream identity provider returned an error: %s", errorString)
+		// Set the debug message and override the non debug message to be the same for this case
+		p.ErrorPage(rw, req, http.StatusForbidden, message, message)
 		return
 	}
 
 	session, err := p.redeemCode(req)
 	if err != nil {
 		logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)
-		p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Error")
+		p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
 		return
 	}
 
 	err = p.enrichSessionState(req.Context(), session)
 	if err != nil {
 		logger.Errorf("Error creating session during OAuth2 callback: %v", err)
-		p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Error")
+		p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
 		return
 	}
 
 	state := strings.SplitN(req.Form.Get("state"), ":", 2)
 	if len(state) != 2 {
 		logger.Error("Error while parsing OAuth2 state: invalid length")
-		p.ErrorPage(rw, req, http.StatusInternalServerError, "Invalid State")
+		p.ErrorPage(rw, req, http.StatusInternalServerError, "State paremeter did not have expected length", "Login Failed: Invalid State after login.")
 		return
 	}
 	nonce := state[0]
@@ -780,13 +783,13 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 	c, err := req.Cookie(p.CSRFCookieName)
 	if err != nil {
 		logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie")
-		p.ErrorPage(rw, req, http.StatusForbidden, err.Error())
+		p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")
 		return
 	}
 	p.ClearCSRFCookie(rw, req)
 	if c.Value != nonce {
 		logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: CSRF token mismatch, potential attack")
-		p.ErrorPage(rw, req, http.StatusForbidden, "CSRF Failed")
+		p.ErrorPage(rw, req, http.StatusForbidden, "CSRF token mismatch, potential attack", "Login Failed: Unable to find a valid CSRF token. Please try again.")
 		return
 	}
 
@@ -810,7 +813,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 		http.Redirect(rw, req, redirect, http.StatusFound)
 	} else {
 		logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unauthorized")
-		p.ErrorPage(rw, req, http.StatusForbidden, "Invalid Account")
+		p.ErrorPage(rw, req, http.StatusForbidden, "Invalid session: unauthorized")
 	}
 }
 
@@ -892,12 +895,12 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
 		}
 
 	case ErrAccessDenied:
-		p.ErrorPage(rw, req, http.StatusUnauthorized, "Unauthorized")
+		p.ErrorPage(rw, req, http.StatusForbidden, "The session failed authorization checks")
 
 	default:
 		// unknown error
 		logger.Errorf("Unexpected internal error: %v", err)
-		p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Error")
+		p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
 	}
 }
 

oauthproxy_test.go

@@ -2815,7 +2815,7 @@ func TestProxyAllowedGroups(t *testing.T) {
 			test.proxy.ServeHTTP(test.rw, test.req)
 
 			if tt.expectUnauthorized {
-				assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
+				assert.Equal(t, http.StatusForbidden, test.rw.Code)
 			} else {
 				assert.Equal(t, http.StatusOK, test.rw.Code)
 			}

pkg/apis/options/app.go

@@ -22,6 +22,12 @@ type Templates struct {
 	// password form if a static passwords file (htpasswd file) has been
 	// configured.
 	DisplayLoginForm bool `flag:"display-htpasswd-form" cfg:"display_htpasswd_form"`
+
+	// Debug renders detailed errors when an error page is shown.
+	// It is not advised to use this in production as errors may contain sensitive
+	// information.
+	// Use only for diagnosing backend errors.
+	Debug bool `flag:"show-debug-on-error" cfg:"show-debug-on-error"`
 }
 
 func templatesFlagSet() *pflag.FlagSet {
@@ -31,6 +37,7 @@ func templatesFlagSet() *pflag.FlagSet {
 	flagSet.String("banner", "", "custom banner string. Use \"-\" to disable default banner.")
 	flagSet.String("footer", "", "custom footer string. Use \"-\" to disable default footer.")
 	flagSet.Bool("display-htpasswd-form", true, "display username / password login form if an htpasswd file is provided")
+	flagSet.Bool("show-debug-on-error", false, "show detailed error information on error pages (WARNING: this may contain sensitive information - do not use in production)")
 
 	return flagSet
 }

pkg/app/error_page.go

@@ -1,12 +1,22 @@
 package app
 
 import (
+	"fmt"
 	"html/template"
 	"net/http"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
 )
 
+// errorMessages are default error messages for each of the the different
+// http status codes expected to be rendered in the error page.
+var errorMessages = map[int]string{
+	http.StatusInternalServerError: "Oops! Something went wrong. For more information contact your server administrator.",
+	http.StatusNotFound:            "We could not find the resource you were looking for.",
+	http.StatusForbidden:           "You do not have permission to access this resource.",
+	http.StatusUnauthorized:        "You need to be logged in to access this resource.",
+}
+
 // ErrorPage is used to render error pages.
 type ErrorPage struct {
 	// Template is the error page HTML template.
@@ -21,12 +31,16 @@ type ErrorPage struct {
 
 	// Version is the OAuth2 Proxy version to be used in the default footer.
 	Version string
+
+	// Debug determines whether errors pages should be rendered with detailed
+	// errors.
+	Debug bool
 }
 
 // Render writes an error page to the given response writer.
 // It uses the passed redirectURL to give users the option to go back to where
 // they originally came from or try signing in again.
-func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL string, appError string) {
+func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL string, appError string, messages ...interface{}) {
 	rw.WriteHeader(status)
 
 	// We allow unescaped template.HTML since it is user configured options
@@ -41,7 +55,7 @@ func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL strin
 		Version     string
 	}{
 		Title:       http.StatusText(status),
-		Message:     appError,
+		Message:     e.getMessage(status, appError, messages...),
 		ProxyPrefix: e.ProxyPrefix,
 		StatusCode:  status,
 		Redirect:    redirectURL,
@@ -60,5 +74,24 @@ func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL strin
 // It is expected to always render a bad gateway error.
 func (e *ErrorPage) ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error) {
 	logger.Errorf("Error proxying to upstream server: %v", proxyErr)
-	e.Render(rw, http.StatusBadGateway, "", "Error proxying to upstream server")
+	e.Render(rw, http.StatusBadGateway, "", proxyErr.Error(), "There was a problem connecting to the upstream server.")
+}
+
+// getMessage creates the message for the template parameters.
+// If the ErrorPage.Debug is enabled, the application error takes precedence.
+// Otherwise, any messages will be used.
+// The first message is expected to be a format string.
+// If no messages are supplied, a default error message will be used.
+func (e *ErrorPage) getMessage(status int, appError string, messages ...interface{}) string {
+	if e.Debug {
+		return appError
+	}
+	if len(messages) > 0 {
+		format := fmt.Sprintf("%v", messages[0])
+		return fmt.Sprintf(format, messages[1:]...)
+	}
+	if msg, ok := errorMessages[status]; ok {
+		return msg
+	}
+	return "Unknown error"
 }

pkg/app/error_page_test.go

@@ -32,7 +32,25 @@ var _ = Describe("Error Page", func() {
 
 			body, err := ioutil.ReadAll(recorder.Result().Body)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(string(body)).To(Equal("Forbidden Access Denied /prefix/ 403 /redirect Custom Footer Text v0.0.0-test"))
+			Expect(string(body)).To(Equal("Forbidden You do not have permission to access this resource. /prefix/ 403 /redirect Custom Footer Text v0.0.0-test"))
+		})
+
+		It("With a different code, uses the stock message for the correct code", func() {
+			recorder := httptest.NewRecorder()
+			errorPage.Render(recorder, 500, "/redirect", "Access Denied")
+
+			body, err := ioutil.ReadAll(recorder.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(body)).To(Equal("Internal Server Error Oops! Something went wrong. For more information contact your server administrator. /prefix/ 500 /redirect Custom Footer Text v0.0.0-test"))
+		})
+
+		It("With a message override, uses the message", func() {
+			recorder := httptest.NewRecorder()
+			errorPage.Render(recorder, 403, "/redirect", "Access Denied", "An extra message: %s", "with more context.")
+
+			body, err := ioutil.ReadAll(recorder.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(body)).To(Equal("Forbidden An extra message: with more context. /prefix/ 403 /redirect Custom Footer Text v0.0.0-test"))
 		})
 	})
 
@@ -44,7 +62,40 @@ var _ = Describe("Error Page", func() {
 
 			body, err := ioutil.ReadAll(recorder.Result().Body)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(string(body)).To(Equal("Bad Gateway Error proxying to upstream server /prefix/ 502  Custom Footer Text v0.0.0-test"))
+			Expect(string(body)).To(Equal("Bad Gateway There was a problem connecting to the upstream server. /prefix/ 502  Custom Footer Text v0.0.0-test"))
+		})
+	})
+
+	Context("With Debug enabled", func() {
+		BeforeEach(func() {
+			tmpl, err := template.New("").Parse("{{.Message}}")
+			Expect(err).ToNot(HaveOccurred())
+
+			errorPage.Template = tmpl
+			errorPage.Debug = true
+		})
+
+		Context("Render", func() {
+			It("Writes the detailed error in place of the message", func() {
+				recorder := httptest.NewRecorder()
+				errorPage.Render(recorder, 403, "/redirect", "Debug error")
+
+				body, err := ioutil.ReadAll(recorder.Result().Body)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(body)).To(Equal("Debug error"))
+			})
+		})
+
+		Context("ProxyErrorHandler", func() {
+			It("Writes a bad gateway error the response writer", func() {
+				req := httptest.NewRequest("", "/bad-gateway", nil)
+				recorder := httptest.NewRecorder()
+				errorPage.ProxyErrorHandler(recorder, req, errors.New("some upstream error"))
+
+				body, err := ioutil.ReadAll(recorder.Result().Body)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(string(body)).To(Equal("some upstream error"))
+			})
 		})
 	})
 })