Fix settings action always return error

ref DEV-2458

.vettedpositions

@@ -251,7 +251,7 @@
 /pkg/auth/webapp/context_holder_middleware.go:26:32: requestcontext
 /pkg/auth/webapp/intl_middleware.go:12:41: requestcontext
 /pkg/auth/webapp/require_authenticated_middleware.go:14:10: requestcontext
-/pkg/auth/webapp/session_middleware.go:57:10: requestcontext
+/pkg/auth/webapp/session_middleware.go:58:10: requestcontext
 /pkg/auth/webapp/settings_sub_routes_middleware.go:26:10: requestcontext
 /pkg/auth/webapp/success_page_middleware.go:41:10: requestcontext
 /pkg/auth/webapp/ui_param.go:55:28: requestcontext

pkg/auth/handler/webapp/alternatives.go

@@ -276,14 +276,14 @@ func handleAlternativeSteps(ctrl *Controller) {
 			}
 			result = &webapp.Result{
 				RedirectURI:      session.CurrentStep().URL().String(),
-				NavigationAction: "replace",
+				NavigationAction: webapp.NavigationActionReplace,
 			}
 		} else {
 			result, err = ctrl.InteractionPost(ctx, inputFn)
 			if err != nil {
 				return err
 			}
-			result.NavigationAction = "replace"
+			result.NavigationAction = webapp.NavigationActionReplace
 		}
 
 		result.WriteResponse(ctrl.response, ctrl.request)

pkg/auth/handler/webapp/authflow_controller.go

@@ -1116,7 +1116,7 @@ func (c *AuthflowController) finishSession(
 	result.RemoveQueries = setutil.Set[string]{
 		"x_step": struct{}{},
 	}
-	result.NavigationAction = "redirect"
+	result.NavigationAction = webapp.NavigationActionRedirect
 	result.RedirectURI = c.deriveFinishRedirectURI(r, s, finishedUIScreenData.FlowType, finishedUIScreenData.FinishRedirectURI)
 
 	switch finishedUIScreenData.FlowType {

pkg/auth/handler/webapp/authflow_wechat.go

@@ -159,7 +159,7 @@ func (h *AuthflowWechatHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
 			RawQuery: r.URL.Query().Encode(),
 		}
 		result := &webapp.Result{
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 			RedirectURI:      redirectURI.String(),
 		}
 		result.WriteResponse(w, r)

pkg/auth/handler/webapp/authflowv2/routes.go

@@ -175,7 +175,7 @@ func (n *AuthflowV2Navigator) NavigateChangePasswordSuccessPage(s *webapp.Authfl
 		}
 		u.RawQuery = q.Encode()
 		result = &webapp.Result{}
-		result.NavigationAction = "advance"
+		result.NavigationAction = webapp.NavigationActionAdvance
 		result.RedirectURI = u.String()
 		return result
 	}
@@ -340,7 +340,7 @@ func (n *AuthflowV2Navigator) navigateStepIdentify(ctx context.Context, s *webap
 		q.Set(webapp.AuthflowQueryKey, s.Screen.StateToken.XStep)
 		u.RawQuery = q.Encode()
 
-		result.NavigationAction = "replace"
+		result.NavigationAction = webapp.NavigationActionReplace
 		result.RedirectURI = u.String()
 	case config.AuthenticationFlowIdentificationOAuth:
 		data := s.StateTokenFlowResponse.Action.Data.(declarative.OAuthData)
@@ -368,7 +368,7 @@ func (n *AuthflowV2Navigator) navigateStepIdentify(ctx context.Context, s *webap
 			q.Set("state", stateToken)
 			authorizationURL.RawQuery = q.Encode()
 
-			result.NavigationAction = "redirect"
+			result.NavigationAction = webapp.NavigationActionRedirect
 			result.RedirectURI = authorizationURL.String()
 		}
 	case config.AuthenticationFlowIdentificationLDAP:
@@ -555,7 +555,7 @@ func (n *AuthflowV2Navigator) navigateAccountRecovery(s *webapp.AuthflowScreenWi
 			}
 		}
 		u.RawQuery = q.Encode()
-		result.NavigationAction = "replace"
+		result.NavigationAction = webapp.NavigationActionReplace
 		result.RedirectURI = u.String()
 	}
 	switch config.AuthenticationFlowStepType(s.StateTokenFlowResponse.Action.Type) {

pkg/auth/handler/webapp/authflowv2/settings_change_password.go

@@ -116,7 +116,10 @@ func (h *AuthflowV2SettingsChangePasswordHandler) ServeHTTP(w http.ResponseWrite
 			return err
 		}
 
-		result := webapp.Result{RedirectURI: changePasswordOutput.RedirectURI}
+		result := webapp.Result{
+			RedirectURI:      changePasswordOutput.RedirectURI,
+			NavigationAction: webapp.NavigationActionRedirect,
+		}
 		result.WriteResponse(w, r)
 		return nil
 	})

pkg/auth/handler/webapp/authflowv2/settings_delete_account_success.go

@@ -80,7 +80,8 @@ func (h *AuthflowV2SettingsDeleteAccountSuccessHandler) ServeHTTP(w http.Respons
 		}
 
 		result := webapp.Result{
-			RedirectURI: redirectURI,
+			RedirectURI:      redirectURI,
+			NavigationAction: webapp.NavigationActionRedirect,
 		}
 		result.WriteResponse(w, r)
 		return nil

pkg/auth/handler/webapp/authflowv2/verify_login_link.go

@@ -121,7 +121,7 @@ func (h *AuthflowV2VerifyLoginLinkOTPHandler) ServeHTTP(w http.ResponseWriter, r
 
 		result := webapp.Result{
 			RedirectURI:      url.String(),
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 		}
 		result.WriteResponse(w, r)
 	}

pkg/auth/handler/webapp/authflowv2/wechat.go

@@ -160,7 +160,7 @@ func (h *AuthflowV2WechatHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 			RawQuery: r.URL.Query().Encode(),
 		}
 		result := &webapp.Result{
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 			RedirectURI:      redirectURI.String(),
 		}
 		result.WriteResponse(w, r)

pkg/auth/handler/webapp/error_renderer.go

@@ -64,7 +64,7 @@ func (s *ErrorRenderer) renderInteractionError(ctx context.Context, w http.Respo
 	}
 	result := webapp.Result{
 		RedirectURI:      u.String(),
-		NavigationAction: "replace",
+		NavigationAction: webapp.NavigationActionReplace,
 		Cookies:          []*http.Cookie{cookie},
 	}
 	result.WriteResponse(w, r)
@@ -81,7 +81,7 @@ func (s *ErrorRenderer) MakeAuthflowErrorResult(ctx context.Context, w http.Resp
 
 		result := &webapp.Result{
 			RedirectURI:      u.String(),
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 			Cookies:          []*http.Cookie{cookie},
 		}
 
@@ -91,7 +91,7 @@ func (s *ErrorRenderer) MakeAuthflowErrorResult(ctx context.Context, w http.Resp
 	nonRecoverable := func() *webapp.Result {
 		result := &webapp.Result{
 			RedirectURI:      u.String(),
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 		}
 		err := s.ErrorService.SetNonRecoverableError(result, apierror)
 		if err != nil {

pkg/auth/handler/webapp/login_link_otp.go

@@ -199,7 +199,7 @@ func (h *LoginLinkOTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 
 		result := webapp.Result{
 			RedirectURI:      url.String(),
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 		}
 		result.WriteResponse(w, r)
 		return nil

pkg/auth/handler/webapp/panic_middleware.go

@@ -101,7 +101,7 @@ func (m *PanicMiddleware) Handle(next http.Handler) http.Handler {
 							// Show the error in the original page.
 							// The panic may come from an I/O error, which could recover by retrying.
 							RedirectURI:      r.URL.String(),
-							NavigationAction: "replace",
+							NavigationAction: webapp.NavigationActionReplace,
 						}
 						result.WriteResponse(w, r)
 					}

pkg/auth/handler/webapp/verify_login_link.go

@@ -100,7 +100,7 @@ func (h *VerifyLoginLinkOTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 
 		result := webapp.Result{
 			RedirectURI:      url.String(),
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 		}
 		result.WriteResponse(w, r)
 	}

pkg/auth/handler/webapp/wechat_auth.go

@@ -184,7 +184,7 @@ func (h *WechatAuthHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		// Otherwise redirect to the current page.
 		redirectURI := httputil.HostRelative(r.URL).String()
 		result := webapp.Result{
-			NavigationAction: "replace",
+			NavigationAction: webapp.NavigationActionReplace,
 			RedirectURI:      redirectURI,
 		}
 		result.WriteResponse(w, r)

pkg/auth/routes.go

@@ -258,7 +258,6 @@ func NewRouter(p *deps.RootProvider, configSource *configsource.ConfigSource) ht
 	)
 	webappSettingsSubRoutesChain := httproute.Chain(
 		webappAuthenticatedChain,
-		p.Middleware(newWebAppSessionMiddleware),
 		p.Middleware(newRequireSettingsEnabledMiddleware),
 		// SettingsSubRoutesMiddleware should be added to all the settings sub routes only
 		// but no /settings itself

pkg/auth/webapp/authflow_routes.go

@@ -262,7 +262,7 @@ func (n *AuthflowNavigator) navigateStepIdentify(ctx context.Context, s *Authflo
 		q.Set(AuthflowQueryKey, s.Screen.StateToken.XStep)
 		u.RawQuery = q.Encode()
 
-		result.NavigationAction = "replace"
+		result.NavigationAction = NavigationActionReplace
 		result.RedirectURI = u.String()
 	case config.AuthenticationFlowIdentificationOAuth:
 		data := s.StateTokenFlowResponse.Action.Data.(declarative.OAuthData)
@@ -289,7 +289,7 @@ func (n *AuthflowNavigator) navigateStepIdentify(ctx context.Context, s *Authflo
 			q.Set("state", stateToken)
 			authorizationURL.RawQuery = q.Encode()
 
-			result.NavigationAction = "redirect"
+			result.NavigationAction = NavigationActionRedirect
 			result.RedirectURI = authorizationURL.String()
 		}
 
@@ -435,7 +435,7 @@ func (n *AuthflowNavigator) navigateAccountRecovery(s *AuthflowScreenWithFlowRes
 			}
 		}
 		u.RawQuery = q.Encode()
-		result.NavigationAction = "replace"
+		result.NavigationAction = NavigationActionReplace
 		result.RedirectURI = u.String()
 	}
 	switch config.AuthenticationFlowStepType(s.StateTokenFlowResponse.Action.Type) {

pkg/auth/webapp/result.go

@@ -9,11 +9,19 @@ import (
 	"github.com/authgear/authgear-server/pkg/util/setutil"
 )
 
+type NavigationAction string
+
+const (
+	NavigationActionAdvance  NavigationAction = "advance"
+	NavigationActionReplace  NavigationAction = "replace"
+	NavigationActionRedirect NavigationAction = "redirect"
+)
+
 type Result struct {
 	UILocales        string              `json:"ui_locales,omitempty"`
 	ColorScheme      string              `json:"color_scheme,omitempty"`
 	RedirectURI      string              `json:"redirect_uri,omitempty"`
-	NavigationAction string              `json:"navigation_action,omitempty"`
+	NavigationAction NavigationAction    `json:"navigation_action,omitempty"`
 	Cookies          []*http.Cookie      `json:"cookies,omitempty"`
 	IsInteractionErr bool                `json:"is_interaction_err,omitempty"`
 	RemoveQueries    setutil.Set[string] `json:"remove_queries,omitempty"`
@@ -60,13 +68,13 @@ func (r *Result) WriteResponse(w http.ResponseWriter, req *http.Request) {
 
 	if req.Header.Get("X-Authgear-XHR") == "true" {
 		type xhrResponse struct {
-			RedirectURI string `json:"redirect_uri"`
-			Action      string `json:"action"`
+			RedirectURI string           `json:"redirect_uri"`
+			Action      NavigationAction `json:"action"`
 		}
 
 		action := r.NavigationAction
 		if action == "" {
-			action = "advance"
+			action = NavigationActionAdvance
 		}
 		data, err := json.Marshal(xhrResponse{
 			RedirectURI: redirectURI.String(),

pkg/auth/webapp/service2.go

@@ -480,7 +480,7 @@ func (s *Service2) afterPost(
 		result.RedirectURI = s.deriveFinishRedirectURI(session, graph)
 		switch graph.Intent.(type) {
 		case *intents.IntentAuthenticate:
-			result.NavigationAction = "redirect"
+			result.NavigationAction = NavigationActionRedirect
 			// Marked signed up in cookie after authorization.
 			// When user visit auth ui root "/", redirect user to "/login" if
 			// cookie exists
@@ -508,7 +508,7 @@ func (s *Service2) afterPost(
 				RawQuery: s.Request.URL.RawQuery,
 			}
 			result.RedirectURI = u.String()
-			result.NavigationAction = "replace"
+			result.NavigationAction = NavigationActionReplace
 		}
 	}
 	s.Logger.Debugf("interaction: redirect to %s", result.RedirectURI)

pkg/auth/webapp/session_authflow.go

@@ -732,15 +732,15 @@ func (s *AuthflowScreenWithFlowResponse) Navigate(ctx context.Context, navigator
 // Advance is for advancing to another page to drive the authflow.
 func (s *AuthflowScreenWithFlowResponse) Advance(route string, result *Result) {
 	s.advanceOrRedirect(route, result)
-	result.NavigationAction = "advance"
+	result.NavigationAction = NavigationActionAdvance
 }
 
 // RedirectToFinish is a fix for https://linear.app/authgear/issue/DEV-1793/investigate-sign-in-directly-with-httpsaccountsportalauthgearcom-crash
 // We need Turbo to visit /finish with a full browser redirect,
 // so CSP and connect-src will not kick in.
 func (s *AuthflowScreenWithFlowResponse) RedirectToFinish(route string, result *Result) {
 	s.advanceOrRedirect(route, result)
-	result.NavigationAction = "redirect"
+	result.NavigationAction = NavigationActionRedirect
 }
 
 // advanceOrRedirect is for internal use. You use Advance or RedirectToFinish instead.
@@ -763,7 +763,7 @@ func (s *AuthflowScreenWithFlowResponse) AdvanceWithQuery(route string, result *
 	u, _ := url.Parse(route)
 	u.RawQuery = q.Encode()
 
-	result.NavigationAction = "advance"
+	result.NavigationAction = NavigationActionAdvance
 	result.RedirectURI = u.String()
 }
 

pkg/auth/webapp/session_middleware.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"net/url"
 
 	"github.com/authgear/authgear-server/pkg/lib/oauth/oauthsession"
 	"github.com/authgear/authgear-server/pkg/lib/oauth/oidc"
@@ -58,27 +59,39 @@ func (m *SessionMiddleware) Handle(next http.Handler) http.Handler {
 
 		// Create the session now.
 		if oauthSessionID, ok := m.OAuthUIInfoResolver.GetOAuthSessionID(r, ""); ok {
-			ctx, result, session := m.createSessionFromOAuthSession(ctx, oauthSessionID)
-
-			for _, c := range result.Cookies {
-				httputil.UpdateCookie(w, c)
-			}
+			cookies := m.createSessionFromOAuthSession(ctx, oauthSessionID)
 
 			// Remove oauth session ID so that we do not create again.
 			m.OAuthUIInfoResolver.RemoveOAuthSessionID(w, r)
 
-			r = r.WithContext(WithSession(ctx, session))
-		} else if samlSessionID, ok := m.SAMLUIInfoResolver.GetSAMLSessionID(r, ""); ok {
-			ctx, result, session := m.createSessionFromSAMLSession(ctx, samlSessionID)
-
-			for _, c := range result.Cookies {
-				httputil.UpdateCookie(w, c)
+			// Redirect once to clear the oauth session id from query
+			u := url.URL{
+				Path:     r.URL.Path,
+				RawQuery: r.URL.Query().Encode(),
 			}
+			redirect := httputil.ResultRedirect{
+				URL:     u.String(),
+				Cookies: cookies,
+			}
+			redirect.WriteResponse(w, r)
+			return
+		} else if samlSessionID, ok := m.SAMLUIInfoResolver.GetSAMLSessionID(r, ""); ok {
+			cookies := m.createSessionFromSAMLSession(ctx, samlSessionID)
 
 			// Remove saml session ID so that we do not create again.
 			m.SAMLUIInfoResolver.RemoveSAMLSessionID(w, r)
 
-			r = r.WithContext(WithSession(ctx, session))
+			// Redirect once to clear the saml session id from query
+			u := url.URL{
+				Path:     r.URL.Path,
+				RawQuery: r.URL.Query().Encode(),
+			}
+			redirect := httputil.ResultRedirect{
+				URL:     u.String(),
+				Cookies: cookies,
+			}
+			redirect.WriteResponse(w, r)
+			return
 		} else {
 			// Or read from cookie
 			ctx, session, err := m.loadSession(ctx, r)
@@ -102,7 +115,7 @@ func (m *SessionMiddleware) Handle(next http.Handler) http.Handler {
 	})
 }
 
-func (m *SessionMiddleware) createSessionFromOAuthSession(ctx context.Context, oauthSessionID string) (context.Context, *Result, *Session) {
+func (m *SessionMiddleware) createSessionFromOAuthSession(ctx context.Context, oauthSessionID string) []*http.Cookie {
 	// When oauth session is not found, we fall back gracefully
 	// with a zero value of SessionOptions
 	sessionOptions := SessionOptions{}
@@ -146,10 +159,10 @@ func (m *SessionMiddleware) createSessionFromOAuthSession(ctx context.Context, o
 		panic(err)
 	}
 
-	return ctx, result, session
+	return result.Cookies
 }
 
-func (m *SessionMiddleware) createSessionFromSAMLSession(ctx context.Context, samlSessionID string) (context.Context, *Result, *Session) {
+func (m *SessionMiddleware) createSessionFromSAMLSession(ctx context.Context, samlSessionID string) []*http.Cookie {
 	// When saml session is not found, we fall back gracefully
 	// with a zero value of SessionOptions
 	sessionOptions := SessionOptions{}
@@ -184,7 +197,7 @@ func (m *SessionMiddleware) createSessionFromSAMLSession(ctx context.Context, sa
 		panic(err)
 	}
 
-	return ctx, result, session
+	return result.Cookies
 }
 
 func (m *SessionMiddleware) loadSession(ctx context.Context, r *http.Request) (context.Context, *Session, error) {