From 68e9b7e1403103517516cbaeabbc0cfe16298abf Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 30 Sep 2020 12:03:18 +0200 Subject: [PATCH] proxy: only use logrus --- proxy/pkg/proxy/oauthproxy.go | 61 +++++++++++++++---------------- proxy/pkg/proxy/templates.go | 24 ++---------- proxy/pkg/proxy/templates_test.go | 32 +--------------- proxy/pkg/server/middleware.go | 3 +- 4 files changed, 35 insertions(+), 85 deletions(-) diff --git a/proxy/pkg/proxy/oauthproxy.go b/proxy/pkg/proxy/oauthproxy.go index 1590bfb5a..5cc10032a 100644 --- a/proxy/pkg/proxy/oauthproxy.go +++ b/proxy/pkg/proxy/oauthproxy.go @@ -24,7 +24,6 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/pkg/cookies" "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/pkg/ip" - "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/pkg/middleware" "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions" "github.com/oauth2-proxy/oauth2-proxy/pkg/upstream" @@ -117,7 +116,7 @@ func NewOAuthProxy(opts *options.Options) (*OAuthProxy, error) { return nil, fmt.Errorf("error initialising session store: %v", err) } - templates := loadTemplates(opts.CustomTemplatesDir) + templates := getTemplates() proxyErrorHandler := upstream.NewProxyErrorHandler(templates.Lookup("error.html"), opts.ProxyPrefix) upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), proxyErrorHandler) if err != nil { @@ -336,7 +335,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex domain = h } if !strings.HasSuffix(domain, cookieDomain) { - logger.Errorf("Warning: request host is %q but using configured cookie domain of %q", domain, cookieDomain) + p.logger.Errorf("Warning: request host is %q but using configured cookie domain of %q", domain, cookieDomain) } } @@ -383,7 +382,7 @@ func (p *OAuthProxy) SaveSession(rw http.ResponseWriter, req *http.Request, s *s func (p *OAuthProxy) RobotsTxt(rw http.ResponseWriter) { _, err := fmt.Fprintf(rw, "User-agent: *\nDisallow: /") if err != nil { - logger.Printf("Error writing robots.txt: %v", err) + p.logger.Printf("Error writing robots.txt: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -404,7 +403,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, code int, title string, m } err := p.templates.ExecuteTemplate(rw, "error.html", t) if err != nil { - logger.Printf("Error rendering error.html template: %v", err) + p.logger.Printf("Error rendering error.html template: %v", err) http.Error(rw, "Internal Server Error", http.StatusInternalServerError) } } @@ -414,7 +413,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code prepareNoCache(rw) err := p.ClearSessionCookie(rw, req) if err != nil { - logger.Printf("Error clearing session cookie: %v", err) + p.logger.Printf("Error clearing session cookie: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -422,7 +421,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code redirectURL, err := p.GetRedirect(req) if err != nil { - logger.Errorf("Error obtaining redirect: %v", err) + p.logger.Errorf("Error obtaining redirect: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -455,7 +454,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.logger.Printf("Error rendering sign_in.html template: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) } } @@ -472,10 +471,10 @@ func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool) { } // check auth if p.basicAuthValidator.Validate(user, passwd) { - logger.PrintAuthf(user, req, logger.AuthSuccess, "Authenticated via HtpasswdFile") + p.logger.WithField("user", user).WithField("status", "AuthSuccess").Info("Authenticated via HtpasswdFile") return user, true } - logger.PrintAuthf(user, req, logger.AuthFailure, "Invalid authentication via HtpasswdFile") + p.logger.WithField("user", user).WithField("status", "AuthFailure").Info("Invalid authentication via HtpasswdFile") return "", false } @@ -550,7 +549,7 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool { case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"): redirectURL, err := url.Parse(redirect) if err != nil { - logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect) + p.logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect) return false } redirectHostname := redirectURL.Hostname() @@ -575,10 +574,10 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool { } } - logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect) + p.logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect) return false default: - logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect) + p.logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect) return false } } @@ -622,7 +621,7 @@ func (p *OAuthProxy) IsTrustedIP(req *http.Request) bool { remoteAddr, err := ip.GetClientIP(p.realClientIPParser, req) if err != nil { - logger.Errorf("Error obtaining real IP for trusted IP list: %v", err) + p.logger.Errorf("Error obtaining real IP for trusted IP list: %v", err) // Possibly spoofed X-Real-IP header return false } @@ -665,7 +664,7 @@ func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { redirect, err := p.GetRedirect(req) if err != nil { - logger.Errorf("Error obtaining redirect: %v", err) + p.logger.Errorf("Error obtaining redirect: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -675,7 +674,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { session := &sessionsapi.SessionState{User: user} err = p.SaveSession(rw, req, session) if err != nil { - logger.Printf("Error saving session: %v", err) + p.logger.Printf("Error saving session: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -708,7 +707,7 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) { rw.WriteHeader(http.StatusOK) err = json.NewEncoder(rw).Encode(userInfo) if err != nil { - logger.Printf("Error encoding user info: %v", err) + p.logger.Printf("Error encoding user info: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) } } @@ -717,13 +716,13 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) { func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) { redirect, err := p.GetRedirect(req) if err != nil { - logger.Errorf("Error obtaining redirect: %v", err) + p.logger.Errorf("Error obtaining redirect: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } err = p.ClearSessionCookie(rw, req) if err != nil { - logger.Errorf("Error clearing session cookie: %v", err) + p.logger.Errorf("Error clearing session cookie: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -735,14 +734,14 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) { prepareNoCache(rw) nonce, err := encryption.Nonce() if err != nil { - logger.Errorf("Error obtaining nonce: %v", err) + p.logger.Errorf("Error obtaining nonce: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } p.SetCSRFCookie(rw, req, nonce) redirect, err := p.GetRedirect(req) if err != nil { - logger.Errorf("Error obtaining redirect: %v", err) + p.logger.Errorf("Error obtaining redirect: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } @@ -758,27 +757,27 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { // finish the oauth cycle err := req.ParseForm() if err != nil { - logger.Errorf("Error while parsing OAuth2 callback: %v", err) + p.logger.Errorf("Error while parsing OAuth2 callback: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } errorString := req.Form.Get("error") if errorString != "" { - logger.Errorf("Error while parsing OAuth2 callback: %s", errorString) + p.logger.Errorf("Error while parsing OAuth2 callback: %s", errorString) p.ErrorPage(rw, http.StatusForbidden, "Permission Denied", errorString) return } session, err := p.redeemCode(req.Context(), req.Host, req.Form.Get("code")) if err != nil { - logger.Errorf("Error redeeming code during OAuth2 callback: %v", err) + p.logger.Errorf("Error redeeming code during OAuth2 callback: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error") return } s := strings.SplitN(req.Form.Get("state"), ":", 2) if len(s) != 2 { - logger.Error("Error while parsing OAuth2 state: invalid length") + p.logger.Error("Error while parsing OAuth2 state: invalid length") p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Invalid State") return } @@ -786,13 +785,13 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { redirect := s[1] 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.logger.WithField("user", session.Email).WithField("status", "AuthFailure").Info("Invalid authentication via OAuth2: unable to obtain CSRF cookie") p.ErrorPage(rw, http.StatusForbidden, "Permission Denied", 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.logger.WithField("user", session.Email).WithField("status", "AuthFailure").Info("Invalid authentication via OAuth2: CSRF token mismatch, potential attack") p.ErrorPage(rw, http.StatusForbidden, "Permission Denied", "CSRF Failed") return } @@ -803,16 +802,16 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { // set cookie, or deny if p.provider.ValidateGroup(session.Email) { - logger.PrintAuthf(session.Email, req, logger.AuthSuccess, "Authenticated via OAuth2: %s", session) + p.logger.WithField("user", session.Email).WithField("status", "AuthFailure").Infof("Authenticated via OAuth2: %s", session) err := p.SaveSession(rw, req, session) if err != nil { - logger.Printf("Error saving session state for %s: %v", remoteAddr, err) + p.logger.Printf("Error saving session state for %s: %v", remoteAddr, err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error()) return } http.Redirect(rw, req, redirect, http.StatusFound) } else { - logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unauthorized") + p.logger.WithField("user", session.Email).WithField("status", "AuthFailure").Info("Invalid authentication via OAuth2: unauthorized") p.ErrorPage(rw, http.StatusForbidden, "Permission Denied", "Invalid Account") } } @@ -864,7 +863,7 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) { default: // unknown error - logger.Errorf("Unexpected internal error: %v", err) + p.logger.Errorf("Unexpected internal error: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Error", "Internal Error") } diff --git a/proxy/pkg/proxy/templates.go b/proxy/pkg/proxy/templates.go index 329910703..9a770fa0c 100644 --- a/proxy/pkg/proxy/templates.go +++ b/proxy/pkg/proxy/templates.go @@ -2,28 +2,10 @@ package proxy import ( "html/template" - "path" - "strings" - "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + log "github.com/sirupsen/logrus" ) -func loadTemplates(dir string) *template.Template { - if dir == "" { - return getTemplates() - } - logger.Printf("using custom template directory %q", dir) - funcMap := template.FuncMap{ - "ToUpper": strings.ToUpper, - "ToLower": strings.ToLower, - } - t, err := template.New("").Funcs(funcMap).ParseFiles(path.Join(dir, "sign_in.html"), path.Join(dir, "error.html")) - if err != nil { - logger.Fatalf("failed parsing template %s", err) - } - return t -} - func getTemplates() *template.Template { t, err := template.New("foo").Parse(`{{define "sign_in.html"}} @@ -163,7 +145,7 @@ func getTemplates() *template.Template { {{end}}`) if err != nil { - logger.Fatalf("failed parsing template %s", err) + log.Fatalf("failed parsing template %s", err) } t, err = t.Parse(`{{define "error.html"}} @@ -181,7 +163,7 @@ func getTemplates() *template.Template { {{end}}`) if err != nil { - logger.Fatalf("failed parsing template %s", err) + log.Fatalf("failed parsing template %s", err) } return t } diff --git a/proxy/pkg/proxy/templates_test.go b/proxy/pkg/proxy/templates_test.go index 5d5008379..36ef7482f 100644 --- a/proxy/pkg/proxy/templates_test.go +++ b/proxy/pkg/proxy/templates_test.go @@ -2,10 +2,6 @@ package proxy import ( "bytes" - "io/ioutil" - "log" - "os" - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -18,7 +14,7 @@ func TestLoadTemplates(t *testing.T) { TestString: "Testing", } - templates := loadTemplates("") + templates := getTemplates() assert.NotEqual(t, templates, nil) var defaultSignin bytes.Buffer @@ -28,32 +24,6 @@ func TestLoadTemplates(t *testing.T) { var defaultError bytes.Buffer templates.ExecuteTemplate(&defaultError, "error.html", data) assert.Equal(t, "\n", defaultError.String()[0:16]) - - dir, err := ioutil.TempDir("", "templatetest") - if err != nil { - log.Fatal(err) - } - defer os.RemoveAll(dir) - - templateHTML := `{{.TestString}} {{.TestString | ToLower}} {{.TestString | ToUpper}}` - signInFile := filepath.Join(dir, "sign_in.html") - if err := ioutil.WriteFile(signInFile, []byte(templateHTML), 0666); err != nil { - log.Fatal(err) - } - errorFile := filepath.Join(dir, "error.html") - if err := ioutil.WriteFile(errorFile, []byte(templateHTML), 0666); err != nil { - log.Fatal(err) - } - templates = loadTemplates(dir) - assert.NotEqual(t, templates, nil) - - var sitpl bytes.Buffer - templates.ExecuteTemplate(&sitpl, "sign_in.html", data) - assert.Equal(t, "Testing testing TESTING", sitpl.String()) - - var errtpl bytes.Buffer - templates.ExecuteTemplate(&errtpl, "error.html", data) - assert.Equal(t, "Testing testing TESTING", errtpl.String()) } func TestTemplatesCompile(t *testing.T) { diff --git a/proxy/pkg/server/middleware.go b/proxy/pkg/server/middleware.go index 2a4024f7d..2076bb3a3 100644 --- a/proxy/pkg/server/middleware.go +++ b/proxy/pkg/server/middleware.go @@ -8,7 +8,6 @@ import ( "net/http" "time" - "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" log "github.com/sirupsen/logrus" ) @@ -114,7 +113,7 @@ func (h loggingHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { "RequestMethod": req.Method, "ResponseSize": responseLogger.Size(), "StatusCode": responseLogger.Status(), - "Timestamp": logger.FormatTimestamp(t), + "Timestamp": t, "Upstream": responseLogger.upstream, "UserAgent": req.UserAgent(), "Username": responseLogger.authInfo,