fix: resolve password continuity issues

go.mod

@@ -28,7 +28,6 @@ require (
 	github.com/google/go-github/v27 v27.0.1
 	github.com/google/uuid v1.1.1
 	github.com/gorilla/context v1.1.1
-	github.com/gorilla/securecookie v1.1.1
 	github.com/gorilla/sessions v1.1.3
 	github.com/hashicorp/golang-lru v0.5.1
 	github.com/imdario/mergo v0.3.7
@@ -45,7 +44,7 @@ require (
 	github.com/ory/go-acc v0.1.0
 	github.com/ory/go-convenience v0.1.0
 	github.com/ory/graceful v0.1.1
-	github.com/ory/herodot v0.8.1
+	github.com/ory/herodot v0.8.2
 	github.com/ory/jsonschema/v3 v3.0.1
 	github.com/ory/sdk/swagutil v0.0.0-20200407150153-53df6d772608
 	github.com/ory/viper v1.7.4

go.sum

@@ -886,8 +886,8 @@ github.com/ory/graceful v0.1.1/go.mod h1:zqu70l95WrKHF4AZ6tXHvAqAvpY6M7g6ttaAVcM
 github.com/ory/herodot v0.6.2/go.mod h1:3BOneqcyBsVybCPAJoi92KN2BpJHcmDqAMcAAaJiJow=
 github.com/ory/herodot v0.7.0 h1:DGPUyPDBZwQSaQzci4UW/edjG6OWixZTwXyfjBgEVgs=
 github.com/ory/herodot v0.7.0/go.mod h1:YXKOfAXYdQojDP5sD8m0ajowq3+QXNdtxA+QiUXBwn0=
-github.com/ory/herodot v0.8.1 h1:eG4zsUfNVwkIpUQf6+YT141DoSZuuMrU2A1jvmsOC1c=
-github.com/ory/herodot v0.8.1/go.mod h1:kFWnruHnnokHH4e7tbkGyHOjHGj70sJTrdiz01Xcq4Y=
+github.com/ory/herodot v0.8.2 h1:Lq5DpT81tkcegzp1QFqVFlcDWNCcq9xIq5FQ191rI0E=
+github.com/ory/herodot v0.8.2/go.mod h1:kFWnruHnnokHH4e7tbkGyHOjHGj70sJTrdiz01Xcq4Y=
 github.com/ory/jsonschema/v3 v3.0.1 h1:xzV7w2rt/Qn+jvh71joIXNKKOCqqNyTlaIxdxU0IQJc=
 github.com/ory/jsonschema/v3 v3.0.1/go.mod h1:jgLHekkFk0uiGdEWGleC+tOm6JSSP8cbf17PnBuGXlw=
 github.com/ory/sdk/swagutil v0.0.0-20200202121523-307941feee4b h1:xn3WcZ8Oy285KYiCnoscQxkyRfJZT+KhIbU9LEhPLyw=

internal/testhelpers/selfservice_settings.go

@@ -16,12 +16,15 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/negroni"
 
+	"github.com/ory/x/urlx"
+
 	"github.com/ory/viper"
 	"github.com/ory/x/pointerx"
 
 	"github.com/ory/kratos/driver"
 	"github.com/ory/kratos/driver/configuration"
 	"github.com/ory/kratos/identity"
+	"github.com/ory/kratos/internal/httpclient/client"
 	"github.com/ory/kratos/internal/httpclient/client/common"
 	"github.com/ory/kratos/internal/httpclient/models"
 	"github.com/ory/kratos/selfservice/flow/settings"
@@ -95,6 +98,29 @@ func NewSettingsUITestServer(t *testing.T) *httptest.Server {
 	return ts
 }
 
+func NewSettingsLoginAcceptAPIServer(t *testing.T, adminClient *client.OryKratos) *httptest.Server {
+	var called int
+	loginTS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, 0, called)
+		called++
+
+		viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "5m")
+
+		res, err := adminClient.Common.GetSelfServiceBrowserLoginRequest(common.NewGetSelfServiceBrowserLoginRequestParams().WithRequest(r.URL.Query().Get("request")))
+		require.NoError(t, err)
+		require.NotEmpty(t, res.Payload.RequestURL)
+
+		redir := urlx.ParseOrPanic(*res.Payload.RequestURL).Query().Get("return_to")
+		t.Logf("Redirecting to: %s", redir)
+		http.Redirect(w, r, redir, http.StatusFound)
+	}))
+	t.Cleanup(func() {
+		loginTS.Close()
+	})
+	viper.Set(configuration.ViperKeyURLsLogin, loginTS.URL+"/login")
+	return loginTS
+}
+
 func NewSettingsAPIServer(t *testing.T, reg *driver.RegistryDefault, ids []identity.Identity) (*httptest.Server, *httptest.Server) {
 	public, admin := x.NewRouterPublic(), x.NewRouterAdmin()
 	reg.SettingsHandler().RegisterAdminRoutes(admin)

selfservice/flow/login/handler.go

@@ -109,7 +109,7 @@ func (h *Handler) NewLoginRequest(w http.ResponseWriter, r *http.Request, redir
 func (h *Handler) initLoginRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	if err := h.NewLoginRequest(w, r, func(a *Request) (string, error) {
 		// we assume an error means the user has no session
-		if _, err := h.d.SessionManager().FetchFromRequest(r.Context(), w, r); err != nil {
+		if _, err := h.d.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
 			return urlx.CopyWithQuery(h.c.LoginURL(), url.Values{"request": {a.ID.String()}}).String(), nil
 		}
 

selfservice/flow/registration/handler.go

@@ -109,7 +109,7 @@ func (h *Handler) NewRegistrationRequest(w http.ResponseWriter, r *http.Request,
 func (h *Handler) initRegistrationRequest(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	if err := h.NewRegistrationRequest(w, r, func(a *Request) (string, error) {
 		// we assume an error means the user has no session
-		if _, err := h.d.SessionManager().FetchFromRequest(r.Context(), w, r); err != nil {
+		if _, err := h.d.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
 			return urlx.CopyWithQuery(h.c.RegisterURL(), url.Values{"request": {a.ID.String()}}).String(), nil
 		}
 

selfservice/flow/settings/error.go

@@ -59,7 +59,7 @@ func (s *ErrorHandler) reauthenticate(
 		return
 	}
 
-	returnTo := urlx.CopyWithQuery(urlx.AppendPaths(s.c.SelfPublicURL(),r.URL.Path),r.URL.Query())
+	returnTo := urlx.CopyWithQuery(urlx.AppendPaths(s.c.SelfPublicURL(), r.URL.Path), r.URL.Query())
 	s.c.SelfPublicURL()
 	u := urlx.AppendPaths(
 		urlx.CopyWithQuery(s.c.SelfPublicURL(), url.Values{
@@ -90,18 +90,18 @@ func (s *ErrorHandler) HandleSettingsError(
 		return
 	}
 
+	if errors.Is(err, ErrRequestNeedsReAuthentication) {
+		s.reauthenticate(w, r, rr)
+		return
+	}
+
 	if _, ok := rr.Methods[method]; !ok {
 		s.d.SelfServiceErrorManager().Forward(r.Context(), w, r, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected settings method %s to exist.", method)))
 		return
 	}
 
 	rr.Active = sqlxx.NullString(method)
 
-	if errors.Is(err, ErrRequestNeedsReAuthentication) {
-		s.reauthenticate(w, r, rr)
-		return
-	}
-
 	if err := rr.Methods[method].Config.ParseError(err); err != nil {
 		s.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
 		return

selfservice/flow/settings/handler.go

@@ -93,7 +93,7 @@ func (h *Handler) RegisterAdminRoutes(admin *x.RouterAdmin) {
 //       302: emptyResponse
 //       500: genericError
 func (h *Handler) initUpdateSettings(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	s, err := h.d.SessionManager().FetchFromRequest(r.Context(), w, r)
+	s, err := h.d.SessionManager().FetchFromRequest(r.Context(), r)
 	if err != nil {
 		h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
 		return
@@ -171,7 +171,6 @@ func (h *Handler) adminFetchUpdateSettingsRequest(w http.ResponseWriter, r *http
 	}
 }
 
-
 func (h *Handler) wrapErrorForbidden(err error, shouldWrap bool) error {
 	if shouldWrap {
 		return herodot.ErrForbidden.WithReasonf("Access privileges are missing, invalid, or not sufficient to access this endpoint.").WithTrace(err).WithDebugf("%s", err)
@@ -188,7 +187,7 @@ func (h *Handler) fetchUpdateSettingsRequest(w http.ResponseWriter, r *http.Requ
 	}
 
 	if checkSession {
-		sess, err := h.d.SessionManager().FetchFromRequest(r.Context(), w, r)
+		sess, err := h.d.SessionManager().FetchFromRequest(r.Context(), r)
 		if err != nil {
 			return h.wrapErrorForbidden(err, checkSession)
 		}

selfservice/flow/settings/strategy_profile.go

@@ -141,7 +141,7 @@ func (s *StrategyTraits) PopulateSettingsMethod(r *http.Request, ss *session.Ses
 //       302: emptyResponse
 //       500: genericError
 func (s *StrategyTraits) handleSubmit(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	ss, err := s.d.SessionManager().FetchFromRequest(r.Context(), w, r)
+	ss, err := s.d.SessionManager().FetchFromRequest(r.Context(), r)
 	if err != nil {
 		s.handleSettingsError(w, r, nil, nil, nil, nil, err)
 		return
@@ -311,11 +311,6 @@ func (s *StrategyTraits) handleSettingsError(w http.ResponseWriter, r *http.Requ
 			s.d.SettingsRequestErrorHandler().HandleSettingsError(w, r, rr, err, s.SettingsStrategyID())
 			return
 		}
-
-		if err := s.d.SettingsRequestPersister().UpdateSettingsRequest(r.Context(), rr); err != nil {
-			s.d.SettingsRequestErrorHandler().HandleSettingsError(w, r, rr, err, s.SettingsStrategyID())
-			return
-		}
 	}
 
 	s.d.SettingsRequestErrorHandler().HandleSettingsError(w, r, rr, err, s.SettingsStrategyID())

selfservice/flow/settings/strategy_profile_test.go

@@ -15,8 +15,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/gjson"
 
-	"github.com/ory/x/urlx"
-
 	"github.com/ory/x/pointerx"
 
 	"github.com/ory/x/httpx"
@@ -156,26 +154,10 @@ func TestStrategyTraits(t *testing.T) {
 		})
 
 		t.Run("description=should update protected field with sudo mode", func(t *testing.T) {
-			var called int
-			loginTS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, 0, called)
-				called++
-
-				viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "5m")
-				t.Cleanup(func() {
-					viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "1ns")
-				})
-
-				res, err := adminClient.Common.GetSelfServiceBrowserLoginRequest(common.NewGetSelfServiceBrowserLoginRequestParams().WithRequest(r.URL.Query().Get("request")))
-				require.NoError(t, err)
-				require.NotEmpty(t, res.Payload.RequestURL)
-
-				redir := urlx.ParseOrPanic(*res.Payload.RequestURL).Query().Get("return_to")
-				t.Logf("Redirecting to: %s", redir)
-				http.Redirect(w, r, redir, http.StatusFound)
-			}))
-			defer loginTS.Close()
-			viper.Set(configuration.ViperKeyURLsLogin, loginTS.URL+"/login")
+			_ = testhelpers.NewSettingsLoginAcceptAPIServer(t, adminClient)
+			t.Cleanup(func() {
+				viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "1ns")
+			})
 
 			config := testhelpers.GetSettingsMethodConfig(t, primaryUser, publicTS, settings.StrategyTraitsID)
 			newEmail := "not-john-doe@mail.com"

selfservice/strategy/oidc/strategy.go

@@ -161,7 +161,7 @@ func (s *Strategy) handleAuth(w http.ResponseWriter, r *http.Request, ps httprou
 	}
 
 	// we assume an error means the user has no session
-	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), w, r); err == nil {
+	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err == nil {
 		if !ar.IsForced() {
 			http.Redirect(w, r, s.c.DefaultReturnToURL().String(), http.StatusFound)
 			return
@@ -249,7 +249,7 @@ func (s *Strategy) handleCallback(w http.ResponseWriter, r *http.Request, ps htt
 	}
 
 	// we assume an error means the user has no session
-	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), w, r); err == nil {
+	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err == nil {
 		if !ar.IsForced() {
 			http.Redirect(w, r, s.c.DefaultReturnToURL().String(), http.StatusFound)
 			return

selfservice/strategy/oidc/strategy_helper_test.go

@@ -140,7 +140,7 @@ func newHydraIntegration(t *testing.T, remote *string, subject *string, scope *[
 
 func newReturnTs(t *testing.T, reg driver.Registry) *httptest.Server {
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		sess, err := reg.SessionManager().FetchFromRequest(r.Context(), w, r)
+		sess, err := reg.SessionManager().FetchFromRequest(r.Context(), r)
 		require.NoError(t, err)
 		require.Empty(t, sess.Identity.Credentials)
 		reg.Writer().Write(w, r, sess)

selfservice/strategy/password/login.go

@@ -53,7 +53,7 @@ func (s *Strategy) handleLogin(w http.ResponseWriter, r *http.Request, _ httprou
 		return
 	}
 
-	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), w, r); err == nil {
+	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err == nil {
 		if !ar.Forced {
 			http.Redirect(w, r, s.c.DefaultReturnToURL().String(), http.StatusFound)
 			return
@@ -114,18 +114,33 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, sr *login.Request) error
 		return errors.WithStack(herodot.ErrBadRequest.WithReasonf("Unable to decode POST body: %s", err))
 	}
 
+	var identifier string
+	if !sr.IsForced() {
+		// do nothing
+	} else if sess, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
+		// do nothing
+	} else if id, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), sess.IdentityID); err != nil {
+		// do nothing
+	} else if creds, ok := id.GetCredentials(s.ID()); !ok {
+		// do nothing
+	} else if len(creds.Identifiers) == 0 {
+		// do nothing
+	} else {
+		identifier = creds.Identifiers[0]
+	}
+
 	action := urlx.CopyWithQuery(
 		urlx.AppendPaths(s.c.SelfPublicURL(), LoginPath),
 		url.Values{"request": {sr.ID.String()}},
 	)
-
 	f := &form.HTMLForm{
 		Action: action.String(),
 		Method: "POST",
 		Fields: form.Fields{
 			{
 				Name:     "identifier",
 				Type:     "text",
+				Value:    identifier,
 				Required: true,
 			},
 			{

selfservice/strategy/password/settings.go

@@ -74,7 +74,7 @@ type completeSelfServiceBrowserSettingsPasswordFlowPayload struct {
 //       302: emptyResponse
 //       500: genericError
 func (s *Strategy) submitSettingsFlow(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
-	ss, err := s.d.SessionManager().FetchFromRequest(r.Context(), w, r)
+	ss, err := s.d.SessionManager().FetchFromRequest(r.Context(), r)
 	if err != nil {
 		s.handleSettingsError(w, r, nil, ss, nil, err)
 		return
@@ -122,7 +122,6 @@ func (s *Strategy) completeSettingsFlow(
 	w http.ResponseWriter, r *http.Request,
 	ss *session.Session, p *completeSelfServiceBrowserSettingsPasswordFlowPayload,
 ) {
-
 	ar, err := s.d.SettingsRequestPersister().GetSettingsRequest(r.Context(), x.ParseUUID(p.RequestID))
 	if err != nil {
 		s.handleSettingsError(w, r, nil, ss, p, err)
@@ -135,7 +134,7 @@ func (s *Strategy) completeSettingsFlow(
 	}
 
 	if ss.AuthenticatedAt.Add(s.c.SelfServicePrivilegedSessionMaxAge()).Before(time.Now()) {
-		s.handleSettingsError(w, r, nil, ss, p, errors.WithStack(settings.ErrRequestNeedsReAuthentication))
+		s.handleSettingsError(w, r, ar, ss, p, errors.WithStack(settings.ErrRequestNeedsReAuthentication))
 		return
 	}
 
@@ -165,7 +164,7 @@ func (s *Strategy) completeSettingsFlow(
 	c, ok := i.GetCredentials(s.ID())
 	if !ok {
 		c = &identity.Credentials{Type: s.ID(),
-			// Prevent duplicates
+			// We need to insert a random identifier now...
 			Identifiers: []string{x.NewUUID().String()}}
 	}
 
@@ -213,10 +212,12 @@ func (s *Strategy) PopulateSettingsMethod(r *http.Request, ss *session.Session,
 
 func (s *Strategy) handleSettingsError(w http.ResponseWriter, r *http.Request, rr *settings.Request, ss *session.Session, p *completeSelfServiceBrowserSettingsPasswordFlowPayload, err error) {
 	if errors.Is(err, settings.ErrRequestNeedsReAuthentication) {
-		if _, err := s.d.ContinuityManager().Continue(r.Context(), r,
+		if err := s.d.ContinuityManager().Pause(
+			r.Context(), w, r,
 			continuityKeySettings,
+			continuity.WithPayload(p),
 			continuity.WithIdentity(ss.Identity),
-			continuity.WithPayload(&p),
+			continuity.WithLifespan(time.Minute*15),
 		); err != nil {
 			s.d.SettingsRequestErrorHandler().HandleSettingsError(w, r, rr, err, s.SettingsStrategyID())
 			return