diff --git a/CHANGELOG.md b/CHANGELOG.md index f5d63cd6..c259b733 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - [#3374](https://github.com/oauth2-proxy/oauth2-proxy/pull/3374) fix: handle Unix socket RemoteAddr in IP resolution (@H1net) - [#3381](https://github.com/oauth2-proxy/oauth2-proxy/pull/3381) fix: do not log error for backend logout 204 (@artificiosus) - [#3327](https://github.com/oauth2-proxy/oauth2-proxy/pull/3327) fix: improve logging when session refresh token is missing (@yosri-brh) +- [#2767](https://github.com/oauth2-proxy/oauth2-proxy/pull/2767) fix: propagate errors during route building (@sybereal) ## Release Highlights diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index af4d2e84..857395ec 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -58,7 +58,9 @@ func NewProxy(upstreams options.UpstreamConfig, sigData *options.SignatureData, } } - registerTrailingSlashHandler(m.serveMux) + if err := registerTrailingSlashHandler(m.serveMux); err != nil { + return nil, fmt.Errorf("could not register trailing slash handler: %w", err) + } return m, nil } @@ -94,8 +96,7 @@ func (m *multiUpstreamProxy) registerHTTPUpstreamProxy(upstream options.Upstream // registerHandler ensures the given handler is regiestered with the serveMux. func (m *multiUpstreamProxy) registerHandler(upstream options.Upstream, handler http.Handler, writer pagewriter.Writer) error { if upstream.RewriteTarget == "" { - m.registerSimpleHandler(upstream.Path, handler) - return nil + return m.registerSimpleHandler(upstream.Path, handler) } return m.registerRewriteHandler(upstream, handler, writer) @@ -103,12 +104,12 @@ func (m *multiUpstreamProxy) registerHandler(upstream options.Upstream, handler // registerSimpleHandler maintains the behaviour of the go standard serveMux // by ensuring any path with a trailing `/` matches all paths under that prefix. -func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Handler) { +func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Handler) error { if strings.HasSuffix(path, "/") { - m.serveMux.PathPrefix(path).Handler(handler) - } else { - m.serveMux.Path(path).Handler(handler) + return m.serveMux.PathPrefix(path).Handler(handler).GetError() } + + return m.serveMux.Path(path).Handler(handler).GetError() } // registerRewriteHandler ensures the handler is registered for all paths @@ -123,19 +124,18 @@ func (m *multiUpstreamProxy) registerRewriteHandler(upstream options.Upstream, h rewrite := newRewritePath(rewriteRegExp, upstream.RewriteTarget, writer) h := alice.New(rewrite).Then(handler) - m.serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool { - return rewriteRegExp.MatchString(req.URL.Path) - }).Handler(h) - return nil + return m.serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool { + return rewriteRegExp.MatchString(req.URL.Path) + }).Handler(h).GetError() } // registerTrailingSlashHandler creates a new matcher that will check if the // requested path would match if it had a trailing slash appended. // If the path matches with a trailing slash, we send back a redirect. // This allows us to be consistent with the built in go servemux implementation. -func registerTrailingSlashHandler(serveMux *mux.Router) { - serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool { +func registerTrailingSlashHandler(serveMux *mux.Router) error { + return serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool { if strings.HasSuffix(req.URL.Path, "/") { return false } @@ -149,7 +149,7 @@ func registerTrailingSlashHandler(serveMux *mux.Router) { return serveMux.Match(slashReq, m) }).Handler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { http.Redirect(rw, req, req.URL.String()+"/", http.StatusMovedPermanently) - })) + })).GetError() } // sortByPathLongest ensures that the upstreams are sorted by longest path. diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index b9b8cf9c..aba4a730 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -383,6 +383,38 @@ var _ = Describe("Proxy Suite", func() { ) }) + Context("multiUpstreamProxy errors", func() { + type proxyErrorTableInput struct { + upstreams options.UpstreamConfig + expectedError string + } + + DescribeTable("NewProxy", func(in *proxyErrorTableInput) { + sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"} + + writer := &pagewriter.WriterFuncs{ + ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) { + rw.WriteHeader(502) + rw.Write([]byte("Proxy Error")) + }, + } + + _, err := NewProxy(in.upstreams, sigData, writer) + Expect(err).To(MatchError(in.expectedError)) + }, + Entry("regex matcher without rewrite target", &proxyErrorTableInput{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{{ + ID: "api", + Path: "^/api/$", + URI: "http://example.com", + }}, + }, + expectedError: `could not register http upstream "api": mux: path must start with a slash, got "^/api/$"`, + }), + ) + }) + Context("sortByPathLongest", func() { type sortByPathLongestTableInput struct { input []options.Upstream