public_git/oauth2-proxy
1
0
Fork 0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2026-10-19 07:45:00 +02:00
Code Issues Projects Releases Wiki Activity

Request ID Logging (#1087)

Browse Source 
* Add RequestID to the RequestScope

* Expose RequestID to auth & request loggers

* Use the RequestID in templated HTML pages

* Allow customizing the RequestID header

* Document new Request ID support

* Add more cases to scope/requestID tests

* Split Get vs Generate RequestID funtionality

* Add {{.RequestID}} to the request logger tests

* Move RequestID management to RequestScope

* Use HTML escape instead of sanitization for Request ID rendering
This commit is contained in:
Nick Meves
2021-03-21 18:20:57 +00:00
committed by GitHub
parent 4d9de06b1d
commit c1267bb92d
23 changed files with 274 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/app/pagewriter/error.html
+8 -1
View File
@@ -47,7 +47,7 @@
<h1 class="subtitle is-1">{{.Title}}</h1>
</div>
{{ if .Message }}
{{ if or .Message .RequestID }}
<div id="more-info" class="block card is-fullwidth is-shadowless">
<header class="card-header is-shadowless">
<p class="card-header-title">More Info</p>
@@ -56,9 +56,16 @@
</a>
</header>
<div class="card-content has-text-left is-hidden">
{{ if .Message }}
<div class="content">
{{.Message}}
</div>
{{ end }}
{{ if .RequestID }}
<div class="content">
Request ID: {{.RequestID}}
</div>
{{ end }}
</div>
</div>
{{ end }}
pkg/app/pagewriter/error_page.go
+31 -7
View File
@@ -5,6 +5,7 @@ import (
"html/template"
"net/http"
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
)
@@ -37,11 +38,25 @@ type errorPageWriter struct {
debug bool
}
// ErrorPageOpts bundles up all the content needed to write the Error Page
type ErrorPageOpts struct {
// HTTP status code
Status int
// Redirect URL for "Go back" and "Sign in" buttons
RedirectURL string
// The UUID of the request
RequestID string
// App Error shown in debug mode
AppError string
// Generic error messages shown in non-debug mode
Messages []interface{}
}
// WriteErrorPage 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 *errorPageWriter) WriteErrorPage(rw http.ResponseWriter, status int, redirectURL string, appError string, messages ...interface{}) {
rw.WriteHeader(status)
func (e *errorPageWriter) WriteErrorPage(rw http.ResponseWriter, opts ErrorPageOpts) {
rw.WriteHeader(opts.Status)
// We allow unescaped template.HTML since it is user configured options
/* #nosec G203 */
@@ -51,14 +66,16 @@ func (e *errorPageWriter) WriteErrorPage(rw http.ResponseWriter, status int, red
ProxyPrefix string
StatusCode int
Redirect string
RequestID string
Footer template.HTML
Version string
}{
Title: http.StatusText(status),
Message: e.getMessage(status, appError, messages...),
Title: http.StatusText(opts.Status),
Message: e.getMessage(opts.Status, opts.AppError, opts.Messages...),
ProxyPrefix: e.proxyPrefix,
StatusCode: status,
Redirect: redirectURL,
StatusCode: opts.Status,
Redirect: opts.RedirectURL,
RequestID: opts.RequestID,
Footer: template.HTML(e.footer),
Version: e.version,
}
@@ -74,7 +91,14 @@ func (e *errorPageWriter) WriteErrorPage(rw http.ResponseWriter, status int, red
// It is expected to always render a bad gateway error.
func (e *errorPageWriter) ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error) {
logger.Errorf("Error proxying to upstream server: %v", proxyErr)
e.WriteErrorPage(rw, http.StatusBadGateway, "", proxyErr.Error(), "There was a problem connecting to the upstream server.")
scope := middlewareapi.GetRequestScope(req)
e.WriteErrorPage(rw, ErrorPageOpts{
Status: http.StatusBadGateway,
RedirectURL: "", // The user is already logged in and has hit an upstream error. Makes no sense to redirect in this case.
RequestID: scope.RequestID,
AppError: proxyErr.Error(),
Messages: []interface{}{"There was a problem connecting to the upstream server."},
})
}
// getMessage creates the message for the template parameters.
pkg/app/pagewriter/error_page_test.go
+53 -9
View File
@@ -6,6 +6,7 @@ import (
"io/ioutil"
"net/http/httptest"
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)
@@ -14,7 +15,7 @@ var _ = Describe("Error Page Writer", func() {
var errorPage *errorPageWriter
BeforeEach(func() {
tmpl, err := template.New("").Parse("{{.Title}} {{.Message}} {{.ProxyPrefix}} {{.StatusCode}} {{.Redirect}} {{.Footer}} {{.Version}}")
tmpl, err := template.New("").Parse("{{.Title}} {{.Message}} {{.ProxyPrefix}} {{.StatusCode}} {{.Redirect}} {{.RequestID}} {{.Footer}} {{.Version}}")
Expect(err).ToNot(HaveOccurred())
errorPage = &errorPageWriter{
@@ -28,41 +29,77 @@ var _ = Describe("Error Page Writer", func() {
Context("WriteErrorPage", func() {
It("Writes the template to the response writer", func() {
recorder := httptest.NewRecorder()
errorPage.WriteErrorPage(recorder, 403, "/redirect", "Access Denied")
errorPage.WriteErrorPage(recorder, ErrorPageOpts{
Status: 403,
RedirectURL: "/redirect",
RequestID: testRequestID,
AppError: "Access Denied",
})
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
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"))
Expect(string(body)).To(Equal("Forbidden You do not have permission to access this resource. /prefix/ 403 /redirect 11111111-2222-4333-8444-555555555555 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.WriteErrorPage(recorder, 500, "/redirect", "Access Denied")
errorPage.WriteErrorPage(recorder, ErrorPageOpts{
Status: 500,
RedirectURL: "/redirect",
RequestID: testRequestID,
AppError: "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"))
Expect(string(body)).To(Equal("Internal Server Error Oops! Something went wrong. For more information contact your server administrator. /prefix/ 500 /redirect 11111111-2222-4333-8444-555555555555 Custom Footer Text v0.0.0-test"))
})
It("With a message override, uses the message", func() {
recorder := httptest.NewRecorder()
errorPage.WriteErrorPage(recorder, 403, "/redirect", "Access Denied", "An extra message: %s", "with more context.")
errorPage.WriteErrorPage(recorder, ErrorPageOpts{
Status: 403,
RedirectURL: "/redirect",
RequestID: testRequestID,
AppError: "Access Denied",
Messages: []interface{}{
"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"))
Expect(string(body)).To(Equal("Forbidden An extra message: with more context. /prefix/ 403 /redirect 11111111-2222-4333-8444-555555555555 Custom Footer Text v0.0.0-test"))
})
It("Sanitizes malicious user input", func() {
recorder := httptest.NewRecorder()
errorPage.WriteErrorPage(recorder, ErrorPageOpts{
Status: 403,
RedirectURL: "/redirect",
RequestID: "<script>alert(1)</script>",
AppError: "Access Denied",
})
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
Expect(string(body)).To(Equal("Forbidden You do not have permission to access this resource. /prefix/ 403 /redirect &lt;script&gt;alert(1)&lt;/script&gt; Custom Footer Text v0.0.0-test"))
})
})
Context("ProxyErrorHandler", func() {
It("Writes a bad gateway error the response writer", func() {
req := httptest.NewRequest("", "/bad-gateway", nil)
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{
RequestID: testRequestID,
})
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("Bad Gateway There was a problem connecting to the 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 11111111-2222-4333-8444-555555555555 Custom Footer Text v0.0.0-test"))
})
})
@@ -78,7 +115,11 @@ var _ = Describe("Error Page Writer", func() {
Context("WriteErrorPage", func() {
It("Writes the detailed error in place of the message", func() {
recorder := httptest.NewRecorder()
errorPage.WriteErrorPage(recorder, 403, "/redirect", "Debug error")
errorPage.WriteErrorPage(recorder, ErrorPageOpts{
Status: 403,
RedirectURL: "/redirect",
AppError: "Debug error",
})
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
@@ -89,6 +130,9 @@ var _ = Describe("Error Page Writer", func() {
Context("ProxyErrorHandler", func() {
It("Writes a bad gateway error the response writer", func() {
req := httptest.NewRequest("", "/bad-gateway", nil)
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{
RequestID: testRequestID,
})
recorder := httptest.NewRecorder()
errorPage.ProxyErrorHandler(recorder, req, errors.New("some upstream error"))
pkg/app/pagewriter/pagewriter.go
+2 -2
View File
@@ -10,8 +10,8 @@ import (
// It can also be used to write errors for the http.ReverseProxy used in the
// upstream package.
type Writer interface {
WriteSignInPage(rw http.ResponseWriter, redirectURL string)
WriteErrorPage(rw http.ResponseWriter, status int, redirectURL string, appError string, messages ...interface{})
WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string)
WriteErrorPage(rw http.ResponseWriter, opts ErrorPageOpts)
ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error)
}
pkg/app/pagewriter/pagewriter_suite_test.go
+2
View File
@@ -8,6 +8,8 @@ import (
. "github.com/onsi/gomega"
)
const testRequestID = "11111111-2222-4333-8444-555555555555"
func TestOptionsSuite(t *testing.T) {
logger.SetOutput(GinkgoWriter)
logger.SetErrOutput(GinkgoWriter)
pkg/app/pagewriter/pagewriter_test.go
+16 -4
View File
@@ -2,6 +2,7 @@ package pagewriter
import (
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
@@ -14,6 +15,7 @@ var _ = Describe("Writer", func() {
Context("NewWriter", func() {
var writer Writer
var opts Opts
var request *http.Request
BeforeEach(func() {
opts = Opts{
@@ -26,6 +28,8 @@ var _ = Describe("Writer", func() {
ProviderName: "<ProviderName>",
SignInMessage: "<SignInMessage>",
}
request = httptest.NewRequest("", "http://127.0.0.1/", nil)
})
Context("With no custom templates", func() {
@@ -37,7 +41,11 @@ var _ = Describe("Writer", func() {
It("Writes the default error template", func() {
recorder := httptest.NewRecorder()
writer.WriteErrorPage(recorder, 500, "/redirect", "Some debug error")
writer.WriteErrorPage(recorder, ErrorPageOpts{
Status: 500,
RedirectURL: "/redirect",
AppError: "Some debug error",
})
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
@@ -46,7 +54,7 @@ var _ = Describe("Writer", func() {
It("Writes the default sign in template", func() {
recorder := httptest.NewRecorder()
writer.WriteSignInPage(recorder, "/redirect")
writer.WriteSignInPage(recorder, request, "/redirect")
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
@@ -80,7 +88,11 @@ var _ = Describe("Writer", func() {
It("Writes the custom error template", func() {
recorder := httptest.NewRecorder()
writer.WriteErrorPage(recorder, 500, "/redirect", "Some debug error")
writer.WriteErrorPage(recorder, ErrorPageOpts{
Status: 500,
RedirectURL: "/redirect",
AppError: "Some debug error",
})
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
@@ -89,7 +101,7 @@ var _ = Describe("Writer", func() {
It("Writes the custom sign in template", func() {
recorder := httptest.NewRecorder()
writer.WriteSignInPage(recorder, "/redirect")
writer.WriteSignInPage(recorder, request, "/redirect")
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
pkg/app/pagewriter/sign_in_page.go
+9 -2
View File
@@ -13,6 +13,7 @@ import (
"html/template"
"net/http"
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
)
@@ -53,7 +54,7 @@ type signInPageWriter struct {
// WriteSignInPage writes the sign-in page to the given response writer.
// It uses the redirectURL to be able to set the final destination for the user post login.
func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, redirectURL string) {
func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string) {
// We allow unescaped template.HTML since it is user configured options
/* #nosec G203 */
t := struct {
@@ -79,7 +80,13 @@ func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, redirectURL s
err := s.template.Execute(rw, t)
if err != nil {
logger.Printf("Error rendering sign-in template: %v", err)
s.errorPageWriter.WriteErrorPage(rw, http.StatusInternalServerError, redirectURL, err.Error())
scope := middlewareapi.GetRequestScope(req)
s.errorPageWriter.WriteErrorPage(rw, ErrorPageOpts{
Status: http.StatusInternalServerError,
RedirectURL: redirectURL,
RequestID: scope.RequestID,
AppError: err.Error(),
})
}
}
pkg/app/pagewriter/sign_in_page_test.go
+12 -4
View File
@@ -5,11 +5,13 @@ import (
"fmt"
"html/template"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
@@ -18,10 +20,11 @@ import (
var _ = Describe("SignIn Page", func() {
Context("SignIn Page Writer", func() {
var request *http.Request
var signInPage *signInPageWriter
BeforeEach(func() {
errorTmpl, err := template.New("").Parse("{{.Title}}")
errorTmpl, err := template.New("").Parse("{{.Title}} | {{.RequestID}}")
Expect(err).ToNot(HaveOccurred())
errorPage := &errorPageWriter{
template: errorTmpl,
@@ -41,12 +44,17 @@ var _ = Describe("SignIn Page", func() {
displayLoginForm: true,
logoData: "Logo Data",
}
request = httptest.NewRequest("", "http://127.0.0.1/", nil)
request = middlewareapi.AddRequestScope(request, &middlewareapi.RequestScope{
RequestID: testRequestID,
})
})
Context("WriteSignInPage", func() {
It("Writes the template to the response writer", func() {
recorder := httptest.NewRecorder()
signInPage.WriteSignInPage(recorder, "/redirect")
signInPage.WriteSignInPage(recorder, request, "/redirect")
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
@@ -60,11 +68,11 @@ var _ = Describe("SignIn Page", func() {
signInPage.template = tmpl
recorder := httptest.NewRecorder()
signInPage.WriteSignInPage(recorder, "/redirect")
signInPage.WriteSignInPage(recorder, request, "/redirect")
body, err := ioutil.ReadAll(recorder.Result().Body)
Expect(err).ToNot(HaveOccurred())
Expect(string(body)).To(Equal("Internal Server Error"))
Expect(string(body)).To(Equal(fmt.Sprintf("Internal Server Error | %s", testRequestID)))
})
})
})
pkg/app/pagewriter/templates_test.go
+2
View File
@@ -51,6 +51,7 @@ var _ = Describe("Templates", func() {
StatusCode int
Title string
Message string
RequestID string
// For custom templates
TestString string
@@ -67,6 +68,7 @@ var _ = Describe("Templates", func() {
StatusCode: 404,
Title: "<title>",
Message: "<message>",
RequestID: "<request-id>",
TestString: "Testing",
}