revert: "feat(handler/error): remove automatic retry"

This reverts commit 083cb54df7.
This commit is contained in:
Trong Huu Nguyen
2023-12-20 11:05:28 +01:00
parent e71e4a2fda
commit 41f4354ce4
5 changed files with 86 additions and 1 deletions

View File

@@ -18,6 +18,7 @@ const (
var (
Login = login(DefaultPrefix)
Logout = logout(DefaultPrefix)
Retry = retry(DefaultPrefix)
Session = session(DefaultPrefix)
ErrInvalidValue = errors.New("invalid value")
ErrDecrypt = errors.New("unable to decrypt, key or scheme mismatch")
@@ -148,6 +149,7 @@ func ClearLegacyCookies(w http.ResponseWriter, opts Options) {
func ConfigureCookieNamesWithPrefix(prefix string) {
Login = login(prefix)
Logout = logout(prefix)
Retry = retry(prefix)
Session = session(prefix)
}
@@ -163,6 +165,10 @@ func logout(prefix string) string {
return withPrefix(prefix, "logout")
}
func retry(prefix string) string {
return withPrefix(prefix, "retry")
}
func session(prefix string) string {
return withPrefix(prefix, "session")
}

View File

@@ -194,10 +194,12 @@ func TestCookie_Decrypt(t *testing.T) {
func TestCookieNames(t *testing.T) {
assert.Equal(t, "io.nais.wonderwall.callback", cookie.Login)
assert.Equal(t, "io.nais.wonderwall.logout", cookie.Logout)
assert.Equal(t, "io.nais.wonderwall.retry", cookie.Retry)
assert.Equal(t, "io.nais.wonderwall.session", cookie.Session)
cookie.ConfigureCookieNamesWithPrefix("some-prefix")
assert.Equal(t, "some-prefix.callback", cookie.Login)
assert.Equal(t, "some-prefix.logout", cookie.Logout)
assert.Equal(t, "some-prefix.retry", cookie.Retry)
assert.Equal(t, "some-prefix.session", cookie.Session)
}

View File

@@ -4,11 +4,13 @@ import (
"context"
"errors"
"net/http"
"strconv"
"strings"
"github.com/go-chi/chi/v5/middleware"
log "github.com/sirupsen/logrus"
"github.com/nais/wonderwall/pkg/cookie"
"github.com/nais/wonderwall/pkg/handler/templates"
mw "github.com/nais/wonderwall/pkg/middleware"
"github.com/nais/wonderwall/pkg/openid"
@@ -16,6 +18,11 @@ import (
urlpkg "github.com/nais/wonderwall/pkg/url"
)
const (
// MaxAutoRetryAttempts is the maximum number of times to automatically redirect the user to retry their original request.
MaxAutoRetryAttempts = 3
)
type Page struct {
CorrelationID string
RetryURI string
@@ -66,12 +73,34 @@ func (s *Standalone) respondError(w http.ResponseWriter, r *http.Request, status
logger := mw.LogEntryFrom(r)
msg := "error in route: %+v"
incrementRetryAttempt(w, r, s.GetCookieOptions(r))
attempts, ok := getRetryAttempts(r)
if !ok || attempts < MaxAutoRetryAttempts {
loginCookie, err := openid.GetLoginCookie(r, s.Crypter)
if err != nil {
loginCookie = nil
}
retryUri := s.Retry(r, loginCookie)
logger.Infof(msg, cause)
logger.Infof("errorhandler: auto-retry (attempt %d/%d) redirecting to %q...", attempts+1, MaxAutoRetryAttempts, retryUri)
http.Redirect(w, r, retryUri, http.StatusTemporaryRedirect)
return
}
if level == log.WarnLevel || errors.Is(cause, context.Canceled) {
logger.Warnf(msg, cause)
} else {
logger.Errorf(msg, cause)
}
logger.Infof("errorhandler: maximum retry attempts exceeded; executing error template...")
s.defaultErrorResponse(w, r, statusCode)
}
func (s *Standalone) defaultErrorResponse(w http.ResponseWriter, r *http.Request, statusCode int) {
w.WriteHeader(statusCode)
loginCookie, err := openid.GetLoginCookie(r, s.Crypter)
@@ -85,6 +114,32 @@ func (s *Standalone) respondError(w http.ResponseWriter, r *http.Request, status
}
err = templates.ErrorTemplate.Execute(w, errorPage)
if err != nil {
logger.Errorf("errorhandler: executing error template: %+v", err)
mw.LogEntryFrom(r).Errorf("errorhandler: executing error template: %+v", err)
}
}
func getRetryAttempts(r *http.Request) (int, bool) {
c, err := cookie.Get(r, cookie.Retry)
if err != nil {
return 0, false
}
val, err := strconv.Atoi(c.Value)
if err != nil {
return 0, false
}
return val, true
}
func incrementRetryAttempt(w http.ResponseWriter, r *http.Request, opts cookie.Options) {
val := 1
prev, ok := getRetryAttempts(r)
if ok {
val = prev + 1
}
c := cookie.Make(cookie.Retry, strconv.Itoa(val), opts)
cookie.Set(w, c)
}

View File

@@ -8,6 +8,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/nais/wonderwall/pkg/cookie"
errorhandler "github.com/nais/wonderwall/pkg/handler"
"github.com/nais/wonderwall/pkg/ingress"
mw "github.com/nais/wonderwall/pkg/middleware"
"github.com/nais/wonderwall/pkg/mock"
@@ -44,6 +46,23 @@ func TestHandler_Error(t *testing.T) {
} {
t.Run(test.name, func(t *testing.T) {
r := idp.GetRequest(idp.RelyingPartyServer.URL)
// should be automatically redirected to the retry URI until maximum attempts are exhausted
for i := 0; i < errorhandler.MaxAutoRetryAttempts; i++ {
w := httptest.NewRecorder()
test.fn(w, r, fmt.Errorf("some error"))
assert.Equal(t, http.StatusTemporaryRedirect, w.Result().StatusCode)
// ensure cookie in request is set/updated
for _, c := range w.Result().Cookies() {
if c.Name == cookie.Retry {
r.Header.Set("Cookie", fmt.Sprintf("%s=%s", c.Name, c.Value))
}
}
}
// should return an error response when maximum attempts are exhausted
w := httptest.NewRecorder()
test.fn(w, r, fmt.Errorf("some error"))
assert.Equal(t, test.expectedStatusCode, w.Result().StatusCode)

View File

@@ -263,6 +263,7 @@ func (s *Standalone) LoginCallback(w http.ResponseWriter, r *http.Request) {
logger.WithFields(fields).Info("callback: successful login")
metrics.ObserveLogin(amr, redirect)
cookie.Clear(w, cookie.Retry, s.GetCookieOptions(r))
http.Redirect(w, r, redirect, http.StatusFound)
}
@@ -344,6 +345,7 @@ func (s *Standalone) LogoutCallback(w http.ResponseWriter, r *http.Request) {
logoutCallback := s.Client.LogoutCallback(r, logoutCookie, s.Redirect)
redirect := logoutCallback.PostLogoutRedirectURI()
cookie.Clear(w, cookie.Retry, s.GetCookieOptions(r))
logger.Infof("logout/callback: redirecting to %q", redirect)
http.Redirect(w, r, redirect, http.StatusFound)
}
@@ -370,6 +372,7 @@ func (s *Standalone) LogoutFrontChannel(w http.ResponseWriter, r *http.Request)
}
logger.WithField("sid", id).Info("front-channel logout: session deleted")
cookie.Clear(w, cookie.Retry, s.GetCookieOptions(r))
metrics.ObserveLogout(metrics.LogoutOperationFrontChannel)
w.WriteHeader(http.StatusOK)
}