From 8c7f1f01ffc1ef31dfe1ad6fbf09d34c1c6dd91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 14 Jul 2021 20:45:38 +0000 Subject: [PATCH] do not try to add the selected policy to the context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .bingo/goverage.mod | 5 ++++- changelog/unreleased/proxy-access-log.md | 5 ++++- proxy/pkg/middleware/accesslog.go | 2 -- proxy/pkg/middleware/oidc_auth.go | 1 + proxy/pkg/proxy/policy/selector.go | 14 -------------- proxy/pkg/proxy/proxy.go | 1 - 6 files changed, 9 insertions(+), 19 deletions(-) diff --git a/.bingo/goverage.mod b/.bingo/goverage.mod index d3f4a0e39..ab529e4ed 100644 --- a/.bingo/goverage.mod +++ b/.bingo/goverage.mod @@ -2,4 +2,7 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT go 1.16 -require github.com/haya14busa/goverage v0.0.0-20180129164344-eec3514a20b5 +require ( + github.com/haya14busa/goverage v0.0.0-20180129164344-eec3514a20b5 + golang.org/x/tools v0.1.5 // indirect +) diff --git a/changelog/unreleased/proxy-access-log.md b/changelog/unreleased/proxy-access-log.md index 967082c82..1f5a1d7da 100644 --- a/changelog/unreleased/proxy-access-log.md +++ b/changelog/unreleased/proxy-access-log.md @@ -1,5 +1,8 @@ Bugfix: log all requests in the proxy access log -We now use a dedicated middleware to log all requests, regardless of routing selector outcome. The log now includes the remote address and the selected routing policy. +We now use a dedicated middleware to log all requests, regardless of routing selector outcome. +While the log now includes the remote address, the selected routing policy is only logged when log level +is set to debug because the request context cannot be changed in the `directorSelectionDirector`, as per +the `ReverseProxy.Director` documentation. https://github.com/owncloud/ocis/pull/2301 diff --git a/proxy/pkg/middleware/accesslog.go b/proxy/pkg/middleware/accesslog.go index 63e27e0bc..21c88ee1d 100644 --- a/proxy/pkg/middleware/accesslog.go +++ b/proxy/pkg/middleware/accesslog.go @@ -6,7 +6,6 @@ import ( "github.com/go-chi/chi/middleware" "github.com/owncloud/ocis/ocis-pkg/log" - "github.com/owncloud/ocis/proxy/pkg/proxy/policy" ) // AccessLog is a middleware to log http requests at info level logging. @@ -22,7 +21,6 @@ func AccessLog(logger log.Logger) func(http.Handler) http.Handler { Str("request", r.Header.Get("X-Request-ID")). Str("remote-addr", r.RemoteAddr). Str("method", r.Method). - Str("routing-policy", policy.ContextGetPolicy(r.Context())). Int("status", wrap.Status()). Str("path", r.URL.Path). Dur("duration", time.Since(start)). diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go index 1b868223e..c21bd6a43 100644 --- a/proxy/pkg/middleware/oidc_auth.go +++ b/proxy/pkg/middleware/oidc_auth.go @@ -96,6 +96,7 @@ func (m oidcAuth) getClaims(token string, req *http.Request) (claims oidc.Standa return } + // TODO allow extracting arbitrary claims ... or require idp to send a specific claim if err := userInfo.Claims(&claims); err != nil { m.logger.Error().Err(err).Interface("userinfo", userInfo).Msg("failed to unmarshal userinfo claims") status = http.StatusInternalServerError diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 083637963..aa33b6410 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -20,20 +20,6 @@ var ( ErrUnexpectedConfigError = fmt.Errorf("could not initialize policy-selector for given config") ) -// policyKey serves as key for the routing policy in the context -var policyKey struct{} - -// ContextGetPolicy returns the policy if set in the given context. -func ContextGetPolicy(ctx context.Context) string { - u, _ := ctx.Value(policyKey).(string) - return u -} - -// ContextSetPolicy stores the selected policy in the context. -func ContextSetPolicy(ctx context.Context, p string) context.Context { - return context.WithValue(ctx, policyKey, p) -} - // Selector is a function which selects a proxy-policy based on the request. // // A policy is a random name which identifies a set of proxy-routes: diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index d2eb4cd8a..d722439e0 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -146,7 +146,6 @@ func (p *MultiHostReverseProxy) directorSelectionDirector(r *http.Request) { Str("routeType", string(rt)). Msg("director found") - r = r.WithContext(policy.ContextSetPolicy(r.Context(), pol)) p.Directors[pol][rt][endpoint](r) return }