From 2f4005cc0982678eaac9bd1fcf56bf9e0dd56d7f Mon Sep 17 00:00:00 2001 From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com> Date: Wed, 28 Jun 2023 11:02:06 -0700 Subject: [PATCH] authenticate: remove extraneous error log (#4319) Currently the Authenticate.storeIdentityProfile() method always emits an Error log. If there is no error from cookieChunker.SetCookie(), this results in an empty log entry: {"level":"error","time":"2023-06-27T23:56:38Z"} Refactor this method to instead return the error from SetCookie(), and update the calling code so that it logs a message only when this error is non-nil. (Moving the log call to the calling method gives access to the request context, so the log entry will include the request ID and other related info.) --- authenticate/handlers.go | 4 +++- authenticate/identity_profile.go | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/authenticate/handlers.go b/authenticate/handlers.go index 9c4edb98f..a679cc98d 100644 --- a/authenticate/handlers.go +++ b/authenticate/handlers.go @@ -443,7 +443,9 @@ Or contact your administrator. if err != nil { return nil, httputil.NewError(http.StatusInternalServerError, err) } - a.storeIdentityProfile(w, state.cookieCipher, profile) + if err := a.storeIdentityProfile(w, state.cookieCipher, profile); err != nil { + log.Error(r.Context()).Err(err).Msg("failed to store identity profile") + } // ... and the user state to local storage. if err := state.sessionStore.SaveSession(w, r, &newState); err != nil { diff --git a/authenticate/identity_profile.go b/authenticate/identity_profile.go index 78c270880..7dba691b8 100644 --- a/authenticate/identity_profile.go +++ b/authenticate/identity_profile.go @@ -14,7 +14,6 @@ import ( "github.com/pomerium/pomerium/internal/httputil" "github.com/pomerium/pomerium/internal/identity" - "github.com/pomerium/pomerium/internal/log" "github.com/pomerium/pomerium/internal/sessions" "github.com/pomerium/pomerium/internal/urlutil" "github.com/pomerium/pomerium/pkg/cryptutil" @@ -85,7 +84,7 @@ func (a *Authenticate) loadIdentityProfile(r *http.Request, aead cipher.AEAD) (* return &profile, nil } -func (a *Authenticate) storeIdentityProfile(w http.ResponseWriter, aead cipher.AEAD, profile *identitypb.Profile) { +func (a *Authenticate) storeIdentityProfile(w http.ResponseWriter, aead cipher.AEAD, profile *identitypb.Profile) error { options := a.options.Load() decrypted, err := protojson.Marshal(profile) @@ -98,6 +97,5 @@ func (a *Authenticate) storeIdentityProfile(w http.ResponseWriter, aead cipher.A cookie.Name = urlutil.QueryIdentityProfile cookie.Value = base64.RawURLEncoding.EncodeToString(encrypted) cookie.Path = "/" - err = cookieChunker.SetCookie(w, cookie) - log.Error(context.Background()).Err(err).Send() + return cookieChunker.SetCookie(w, cookie) }