From ef457b17653c3d5d390c7856f3953f3e6bd2b96c Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 6 Feb 2021 22:05:45 +0000 Subject: [PATCH] Move Error page rendering to app package --- oauthproxy.go | 81 +++++++++++++++----------------------- pkg/app/error_page.go | 56 ++++++++++++++++++++++++++ pkg/app/error_page_test.go | 35 ++++++++++++++++ 3 files changed, 122 insertions(+), 50 deletions(-) create mode 100644 pkg/app/error_page.go create mode 100644 pkg/app/error_page_test.go diff --git a/oauthproxy.go b/oauthproxy.go index 0ef5060c..e3fcf0f2 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -108,6 +108,7 @@ type OAuthProxy struct { sessionChain alice.Chain headersChain alice.Chain preAuthChain alice.Chain + errorPage *app.ErrorPage } // NewOAuthProxy creates a new instance of OAuthProxy from the options provided @@ -224,6 +225,12 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr sessionChain: sessionChain, headersChain: headersChain, preAuthChain: preAuthChain, + errorPage: &app.ErrorPage{ + Template: templates.Lookup("error.html"), + ProxyPrefix: opts.ProxyPrefix, + Footer: opts.Templates.Footer, + Version: VERSION, + }, }, nil } @@ -507,14 +514,14 @@ func (p *OAuthProxy) RobotsTxt(rw http.ResponseWriter, req *http.Request) { _, err := fmt.Fprintf(rw, "User-agent: *\nDisallow: /") if err != nil { logger.Printf("Error writing robots.txt: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } rw.WriteHeader(http.StatusOK) } // ErrorPage writes an error response -func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, title string, message string) { +func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code int, appError string) { redirectURL, err := p.getAppRedirect(req) if err != nil { logger.Errorf("Error obtaining redirect: %v", err) @@ -523,32 +530,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code i redirectURL = "/" } - rw.WriteHeader(code) - - // We allow unescaped template.HTML since it is user configured options - /* #nosec G203 */ - t := struct { - Title string - Message string - ProxyPrefix string - StatusCode int - Redirect string - Footer template.HTML - Version string - }{ - Title: title, - Message: message, - ProxyPrefix: p.ProxyPrefix, - StatusCode: code, - Redirect: redirectURL, - Footer: template.HTML(p.Footer), - Version: VERSION, - } - - if err := p.templates.ExecuteTemplate(rw, "error.html", t); err != nil { - logger.Printf("Error rendering error.html template: %v", err) - http.Error(rw, "Internal Server Error", http.StatusInternalServerError) - } + p.errorPage.Render(rw, code, redirectURL, appError) } // IsAllowedRequest is used to check if auth should be skipped for this request @@ -593,7 +575,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code err := p.ClearSessionCookie(rw, req) if err != nil { logger.Printf("Error clearing session cookie: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } rw.WriteHeader(code) @@ -601,7 +583,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code redirectURL, err := p.getAppRedirect(req) if err != nil { logger.Errorf("Error obtaining redirect: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } @@ -634,7 +616,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code err = p.templates.ExecuteTemplate(rw, "sign_in.html", t) if err != nil { logger.Printf("Error rendering sign_in.html template: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) } } @@ -662,7 +644,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { redirect, err := p.getAppRedirect(req) if err != nil { logger.Errorf("Error obtaining redirect: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } @@ -672,7 +654,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { err = p.SaveSession(rw, req, session) if err != nil { logger.Printf("Error saving session: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } http.Redirect(rw, req, redirect, http.StatusFound) @@ -711,7 +693,7 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) { err = json.NewEncoder(rw).Encode(userInfo) if err != nil { logger.Printf("Error encoding user info: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) } } @@ -720,13 +702,13 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) { redirect, err := p.getAppRedirect(req) if err != nil { logger.Errorf("Error obtaining redirect: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } err = p.ClearSessionCookie(rw, req) if err != nil { logger.Errorf("Error clearing session cookie: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } http.Redirect(rw, req, redirect, http.StatusFound) @@ -738,14 +720,14 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) { nonce, err := encryption.Nonce() if err != nil { logger.Errorf("Error obtaining nonce: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } p.SetCSRFCookie(rw, req, nonce) redirect, err := p.getAppRedirect(req) if err != nil { logger.Errorf("Error obtaining redirect: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } redirectURI := p.getOAuthRedirectURI(req) @@ -761,34 +743,34 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { err := req.ParseForm() if err != nil { logger.Errorf("Error while parsing OAuth2 callback: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } errorString := req.Form.Get("error") if errorString != "" { logger.Errorf("Error while parsing OAuth2 callback: %s", errorString) - p.ErrorPage(rw, req, http.StatusForbidden, "Permission Denied", errorString) + p.ErrorPage(rw, req, http.StatusForbidden, errorString) 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 Server Error", "Internal Error") + p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal 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 Server Error", "Internal Error") + p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal 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, "Internal Server Error", "Invalid State") + p.ErrorPage(rw, req, http.StatusInternalServerError, "Invalid State") return } nonce := state[0] @@ -796,13 +778,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, "Permission Denied", err.Error()) + p.ErrorPage(rw, req, http.StatusForbidden, err.Error()) 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, "Permission Denied", "CSRF Failed") + p.ErrorPage(rw, req, http.StatusForbidden, "CSRF Failed") return } @@ -820,13 +802,13 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { err := p.SaveSession(rw, req, session) if err != nil { logger.Errorf("Error saving session state for %s: %v", remoteAddr, err) - p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Server Error", err.Error()) + p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } 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, "Permission Denied", "Invalid Account") + p.ErrorPage(rw, req, http.StatusForbidden, "Invalid Account") } } @@ -908,13 +890,12 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) { } case ErrAccessDenied: - p.ErrorPage(rw, req, http.StatusUnauthorized, "Permission Denied", "Unauthorized") + p.ErrorPage(rw, req, http.StatusUnauthorized, "Unauthorized") default: // unknown error logger.Errorf("Unexpected internal error: %v", err) - p.ErrorPage(rw, req, http.StatusInternalServerError, - "Internal Error", "Internal Error") + p.ErrorPage(rw, req, http.StatusInternalServerError, "Internal Error") } } diff --git a/pkg/app/error_page.go b/pkg/app/error_page.go new file mode 100644 index 00000000..aa58f077 --- /dev/null +++ b/pkg/app/error_page.go @@ -0,0 +1,56 @@ +package app + +import ( + "html/template" + "net/http" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" +) + +// ErrorPage is used to render error pages. +type ErrorPage struct { + // Template is the error page HTML template. + Template *template.Template + + // ProxyPrefix is the prefix under which OAuth2 Proxy pages are served. + ProxyPrefix string + + // Footer is the footer to be displayed at the bottom of the page. + // If not set, a default footer will be used. + Footer string + + // Version is the OAuth2 Proxy version to be used in the default footer. + Version string +} + +// 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) { + rw.WriteHeader(status) + + // We allow unescaped template.HTML since it is user configured options + /* #nosec G203 */ + data := struct { + Title string + Message string + ProxyPrefix string + StatusCode int + Redirect string + Footer template.HTML + Version string + }{ + Title: http.StatusText(status), + Message: appError, + ProxyPrefix: e.ProxyPrefix, + StatusCode: status, + Redirect: redirectURL, + Footer: template.HTML(e.Footer), + Version: e.Version, + } + + if err := e.Template.Execute(rw, data); err != nil { + logger.Printf("Error rendering error template: %v", err) + http.Error(rw, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + } +} diff --git a/pkg/app/error_page_test.go b/pkg/app/error_page_test.go new file mode 100644 index 00000000..f80b0c78 --- /dev/null +++ b/pkg/app/error_page_test.go @@ -0,0 +1,35 @@ +package app + +import ( + "html/template" + "io/ioutil" + "net/http/httptest" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Error Page", func() { + + Context("Render", func() { + It("Writes the template to the response writer", func() { + tmpl, err := template.New("").Parse("{{.Title}} {{.Message}} {{.ProxyPrefix}} {{.StatusCode}} {{.Redirect}} {{.Footer}} {{.Version}}") + Expect(err).ToNot(HaveOccurred()) + + errorPage := &ErrorPage{ + Template: tmpl, + ProxyPrefix: "/prefix/", + Footer: "Custom Footer Text", + Version: "v0.0.0-test", + } + + recorder := httptest.NewRecorder() + errorPage.Render(recorder, 403, "/redirect", "Access Denied") + + 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")) + }) + }) + +})