Use the new plog pkg in auth_handler.go

- Add a new helper method to plog to make a consistent way to log
  expected errors at the info level (as opposed to unexpected
  system errors that would be logged using plog.Error)

Signed-off-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Andrew Keesler 2020-11-10 10:33:52 -08:00 committed by Ryan Richard
parent b9726615dd
commit 005225d5f9
2 changed files with 28 additions and 10 deletions

View File

@ -9,17 +9,17 @@ import (
"net/http" "net/http"
"time" "time"
"go.pinniped.dev/internal/plog"
"github.com/ory/fosite" "github.com/ory/fosite"
"github.com/ory/fosite/handler/openid" "github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt" "github.com/ory/fosite/token/jwt"
"golang.org/x/oauth2"
"k8s.io/klog/v2"
"go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/httputil/httperr"
"go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/internal/oidcclient/nonce" "go.pinniped.dev/internal/oidcclient/nonce"
"go.pinniped.dev/internal/oidcclient/pkce" "go.pinniped.dev/internal/oidcclient/pkce"
"go.pinniped.dev/internal/oidcclient/state" "go.pinniped.dev/internal/oidcclient/state"
"golang.org/x/oauth2"
) )
type IDPListGetter interface { type IDPListGetter interface {
@ -44,14 +44,14 @@ func NewHandler(
authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r) authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r)
if err != nil { if err != nil {
klog.InfoS("authorize request error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues()) plog.Info("authorize request error", fositeErrorForLog(err)...)
oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
return nil return nil
} }
upstreamIDP, err := chooseUpstreamIDP(idpListGetter) upstreamIDP, err := chooseUpstreamIDP(idpListGetter)
if err != nil { if err != nil {
klog.InfoS("authorize request upstream selection error", "err", err) plog.InfoErr("authorize request error", err)
return err return err
} }
@ -72,14 +72,14 @@ func NewHandler(
}, },
}) })
if err != nil { if err != nil {
klog.InfoS("authorize response error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues()) plog.Info("authorize response error", fositeErrorForLog(err)...)
oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
return nil return nil
} }
stateValue, nonceValue, pkceValue, err := generateParams(generateState, generateNonce, generatePKCE) stateValue, nonceValue, pkceValue, err := generateParams(generateState, generateNonce, generatePKCE)
if err != nil { if err != nil {
klog.InfoS("authorize generate error", "err", err) plog.InfoErr("authorize generate error", err)
return err return err
} }
@ -130,18 +130,30 @@ func generateParams(
) (state.State, nonce.Nonce, pkce.Code, error) { ) (state.State, nonce.Nonce, pkce.Code, error) {
stateValue, err := generateState() stateValue, err := generateState()
if err != nil { if err != nil {
klog.InfoS("error generating state param", "err", err) plog.InfoErr("error generating state param", err)
return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating state param", err) return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating state param", err)
} }
nonceValue, err := generateNonce() nonceValue, err := generateNonce()
if err != nil { if err != nil {
klog.InfoS("error generating nonce param", "err", err) plog.InfoErr("error generating nonce param", err)
return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating nonce param", err) return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating nonce param", err)
} }
pkceValue, err := generatePKCE() pkceValue, err := generatePKCE()
if err != nil { if err != nil {
klog.InfoS("error generating PKCE param", "err", err) plog.InfoErr("error generating PKCE param", err)
return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating PKCE param", err) return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating PKCE param", err)
} }
return stateValue, nonceValue, pkceValue, nil return stateValue, nonceValue, pkceValue, nil
} }
func fositeErrorForLog(err error) []interface{} {
rfc6749Error := fosite.ErrorToRFC6749Error(err)
keysAndValues := make([]interface{}, 0)
keysAndValues = append(keysAndValues, "name")
keysAndValues = append(keysAndValues, rfc6749Error.Name)
keysAndValues = append(keysAndValues, "status")
keysAndValues = append(keysAndValues, rfc6749Error.Status())
keysAndValues = append(keysAndValues, "description")
keysAndValues = append(keysAndValues, rfc6749Error.Description)
return keysAndValues
}

View File

@ -5,6 +5,7 @@ package plog
import "k8s.io/klog/v2" import "k8s.io/klog/v2"
// Use Error to log an unexpected system error.
func Error(err error, msg string, keysAndValues ...interface{}) { func Error(err error, msg string, keysAndValues ...interface{}) {
klog.ErrorS(err, msg, keysAndValues...) klog.ErrorS(err, msg, keysAndValues...)
} }
@ -22,6 +23,11 @@ func Info(msg string, keysAndValues ...interface{}) {
klog.V(klogLevelInfo).InfoS(msg, keysAndValues...) klog.V(klogLevelInfo).InfoS(msg, keysAndValues...)
} }
// Use InfoErr to log an expected error, e.g. validation failure of an http parameter.
func InfoErr(msg string, err error, keysAndValues ...interface{}) {
klog.V(klogLevelInfo).InfoS(msg, append([]interface{}{"error", err}, keysAndValues)...)
}
func Debug(msg string, keysAndValues ...interface{}) { func Debug(msg string, keysAndValues ...interface{}) {
klog.V(klogLevelDebug).InfoS(msg, keysAndValues...) klog.V(klogLevelDebug).InfoS(msg, keysAndValues...)
} }