From 04fab6104a711548a4683a7d3d16d90593bc082c Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Fri, 6 May 2022 12:28:58 +0200 Subject: [PATCH] feat(handler/callback): add retries for requests to external services --- go.mod | 1 + go.sum | 2 + pkg/router/handler_callback.go | 69 +++++++++++++++++++++++++++------- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 7348d7f..66c3d0a 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/nais/liberator v0.0.0-20220505083635-84398d40ee40 github.com/prometheus/client_golang v1.12.1 github.com/rs/zerolog v1.26.1 + github.com/sethvargo/go-retry v0.2.3 github.com/sirupsen/logrus v1.8.1 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.11.0 diff --git a/go.sum b/go.sum index 8b07c60..bf50b7e 100644 --- a/go.sum +++ b/go.sum @@ -261,6 +261,8 @@ github.com/rs/xid v1.3.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.18.1-0.20200514152719-663cbb4c8469/go.mod h1:IzD0RJ65iWH0w97OQQebJEvTZYvsCUm9WVLWBQrJRjo= github.com/rs/zerolog v1.26.1 h1:/ihwxqH+4z8UxyI70wM1z9yCvkWcfz/a3mj48k/Zngc= github.com/rs/zerolog v1.26.1/go.mod h1:/wSSJWX7lVrsOwlbyTRSOJvqRlc+WjWlfes+CiJ+tmc= +github.com/sethvargo/go-retry v0.2.3 h1:oYlgvIvsju3jNbottWABtbnoLC+GDtLdBHxKWxQm/iU= +github.com/sethvargo/go-retry v0.2.3/go.mod h1:1afjQuvh7s4gflMObvjLPaWgluLLyhA1wmVZ6KLpICw= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= diff --git a/pkg/router/handler_callback.go b/pkg/router/handler_callback.go index a82f49a..2245707 100644 --- a/pkg/router/handler_callback.go +++ b/pkg/router/handler_callback.go @@ -7,14 +7,21 @@ import ( "net/http" "time" + "github.com/sethvargo/go-retry" log "github.com/sirupsen/logrus" "golang.org/x/oauth2" "github.com/nais/wonderwall/pkg/jwt" + "github.com/nais/wonderwall/pkg/loginstatus" "github.com/nais/wonderwall/pkg/openid" logentry "github.com/nais/wonderwall/pkg/router/middleware" ) +const ( + retryBaseDuration = 50 * time.Millisecond + retryMaxDuration = 1 * time.Second +) + func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) { // unconditionally clear login cookie h.clearLoginCookies(w) @@ -77,13 +84,13 @@ func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) { } if h.Config.Loginstatus.Enabled { - loginstatusToken, err := h.Loginstatus.ExchangeToken(r.Context(), tokens.AccessToken) + tokenResponse, err := h.getLoginstatusToken(r.Context(), tokens) if err != nil { h.InternalError(w, r, fmt.Errorf("callback: exchanging loginstatus token: %w", err)) return } - h.Loginstatus.SetCookie(w, loginstatusToken, h.CookieOptions) + h.Loginstatus.SetCookie(w, tokenResponse, h.CookieOptions) log.Info("callback: successfully fetched loginstatus token") } @@ -92,25 +99,55 @@ func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) { } func (h *Handler) codeExchangeForToken(ctx context.Context, loginCookie *openid.LoginCookie, code string) (*oauth2.Token, error) { - clientAssertion, err := openid.ClientAssertion(h.Provider, time.Second*30) - if err != nil { - return nil, fmt.Errorf("creating client assertion: %w", err) - } + var tokens *oauth2.Token + err := retry.Do(ctx, backoff(), func(ctx context.Context) error { + clientAssertion, err := openid.ClientAssertion(h.Provider, time.Second*30) + if err != nil { + return fmt.Errorf("creating client assertion: %w", err) + } - opts := []oauth2.AuthCodeOption{ - oauth2.SetAuthURLParam("code_verifier", loginCookie.CodeVerifier), - oauth2.SetAuthURLParam("client_assertion", clientAssertion), - oauth2.SetAuthURLParam("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"), - } + opts := []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", loginCookie.CodeVerifier), + oauth2.SetAuthURLParam("client_assertion", clientAssertion), + oauth2.SetAuthURLParam("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"), + } - tokens, err := h.OauthConfig.Exchange(ctx, code, opts...) + tokens, err = h.OauthConfig.Exchange(ctx, code, opts...) + if err != nil { + log.Warnf("callback: exchanging authorization code for token; retrying: %+v", err) + return retry.RetryableError(err) + } + + return nil + }) if err != nil { - return nil, fmt.Errorf("exchanging code for token: %w", err) + return nil, err } return tokens, nil } +func (h *Handler) getLoginstatusToken(ctx context.Context, tokens *jwt.Tokens) (*loginstatus.TokenResponse, error) { + var tokenResponse *loginstatus.TokenResponse + + err := retry.Do(ctx, backoff(), func(ctx context.Context) error { + var err error + + tokenResponse, err = h.Loginstatus.ExchangeToken(ctx, tokens.AccessToken) + if err != nil { + log.Warnf("callback: exchanging loginstatus token; retrying: %+v", err) + return retry.RetryableError(err) + } + + return nil + }) + if err != nil { + return nil, err + } + + return tokenResponse, nil +} + func logSuccessfulLogin(r *http.Request, tokens *jwt.Tokens, referer string) { fields := map[string]interface{}{ "redirect_to": referer, @@ -120,3 +157,9 @@ func logSuccessfulLogin(r *http.Request, tokens *jwt.Tokens, referer string) { logger := logentry.LogEntry(r.Context()).With().Fields(fields).Logger() logger.Info().Msg("callback: successful login") } + +func backoff() retry.Backoff { + b := retry.NewFibonacci(retryBaseDuration) + b = retry.WithMaxDuration(retryMaxDuration, b) + return b +}