fix: add error.id to invalid cookie/token settings flow (#1919)

Closes #1888

Co-authored-by: aeneasr <[email protected]>

selfservice/flow/login/handler.go

@@ -108,7 +108,7 @@ func (h *Handler) NewLoginFlow(w http.ResponseWriter, r *http.Request, ft flow.T
 
 	// We assume an error means the user has no session
 	sess, err := h.d.SessionManager().FetchFromRequest(r.Context(), r)
-	if errors.Is(err, session.ErrNoActiveSessionFound) {
+	if e := new(session.ErrNoActiveSessionFound); errors.As(err, &e) {
 		// No session exists yet
 
 		// We can not request an AAL > 1 because we must first verify the first factor.
@@ -550,7 +550,7 @@ func (h *Handler) submitFlow(w http.ResponseWriter, r *http.Request, _ httproute
 
 		http.Redirect(w, r, h.d.Config(r.Context()).SelfServiceBrowserDefaultReturnTo().String(), http.StatusSeeOther)
 		return
-	} else if errors.Is(err, session.ErrNoActiveSessionFound) {
+	} else if e := new(session.ErrNoActiveSessionFound); errors.As(err, &e) {
 		// Only failure scenario here is if we try to upgrade the session to a higher AAL without actually
 		// having a session.
 		if f.RequestedAAL > identity.AuthenticatorAssuranceLevel1 {

selfservice/flow/settings/error.go

@@ -131,7 +131,7 @@ func (s *ErrorHandler) WriteFlowError(
 		shouldRespondWithJSON = true
 	}
 
-	if errors.Is(err, session.ErrNoActiveSessionFound) {
+	if e := new(session.ErrNoActiveSessionFound); errors.As(err, &e) {
 		if shouldRespondWithJSON {
 			s.d.Writer().WriteError(w, r, err)
 		} else {

selfservice/flow/settings/error_test.go

@@ -203,7 +203,7 @@ func TestHandleError(t *testing.T) {
 
 				settingsFlow = newFlow(t, time.Minute, flow.TypeBrowser)
 				settingsFlow.IdentityID = id.ID
-				flowError = errors.WithStack(session.ErrNoActiveSessionFound)
+				flowError = errors.WithStack(session.NewErrNoActiveSessionFound())
 				flowMethod = settings.StrategyProfile
 
 				res, err := ts.Client().Do(testhelpers.NewHTTPGetJSONRequest(t, ts.URL+"/error"))
@@ -213,7 +213,7 @@ func TestHandleError(t *testing.T) {
 
 				body, err := ioutil.ReadAll(res.Body)
 				require.NoError(t, err)
-				assert.Equal(t, session.ErrNoActiveSessionFound.Reason(), gjson.GetBytes(body, "error.reason").String(), "%s", body)
+				assert.Equal(t, session.NewErrNoActiveSessionFound().Reason(), gjson.GetBytes(body, "error.reason").String(), "%s", body)
 			})
 
 			t.Run("case=aal too low", func(t *testing.T) {
@@ -294,7 +294,7 @@ func TestHandleError(t *testing.T) {
 
 			settingsFlow = newFlow(t, time.Minute, flow.TypeBrowser)
 			settingsFlow.IdentityID = id.ID
-			flowError = errors.WithStack(session.ErrNoActiveSessionFound)
+			flowError = errors.WithStack(session.NewErrNoActiveSessionFound())
 			flowMethod = settings.StrategyProfile
 
 			res, err := ts.Client().Get(ts.URL + "/error")

selfservice/flow/settings/handler.go

@@ -84,7 +84,7 @@ func (h *Handler) RegisterPublicRoutes(public *x.RouterPublic) {
 
 	public.GET(RouteInitBrowserFlow, h.d.SessionHandler().IsAuthenticated(h.initBrowserFlow, func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 		if x.IsJSONRequest(r) {
-			h.d.Writer().WriteError(w, r, errors.WithStack(herodot.ErrForbidden.WithReason("Please include a valid session cookie or session token when calling this endpoint.")))
+			h.d.Writer().WriteError(w, r, session.NewErrNoActiveSessionFound())
 		} else {
 			http.Redirect(w, r, h.d.Config(r.Context()).SelfServiceFlowLoginUI().String(), http.StatusFound)
 		}

selfservice/flow/settings/handler_test.go

@@ -3,6 +3,7 @@ package settings_test
 import (
 	"context"
 	"encoding/json"
+	"github.com/ory/kratos/text"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -116,6 +117,12 @@ func TestHandler(t *testing.T) {
 
 	t.Run("endpoint=init", func(t *testing.T) {
 		t.Run("description=init a flow as API", func(t *testing.T) {
+			t.Run("description=without privileges", func(t *testing.T) {
+				res, body := initFlow(t, new(http.Client), true)
+				assert.Equal(t, http.StatusUnauthorized, res.StatusCode, "%s", body)
+				assert.Equal(t, text.ErrNoActiveSession, gjson.GetBytes(body, "error.id").String(),"%s", body)
+			})
+
 			t.Run("description=success", func(t *testing.T) {
 				user1 := testhelpers.NewHTTPClientWithArbitrarySessionToken(t, reg)
 				res, body := initFlow(t, user1, true)
@@ -132,6 +139,12 @@ func TestHandler(t *testing.T) {
 		})
 
 		t.Run("description=init a flow as browser", func(t *testing.T) {
+			t.Run("description=without privileges", func(t *testing.T) {
+				res, body := initSPAFlow(t, new(http.Client))
+				assert.Equal(t, http.StatusUnauthorized, res.StatusCode, "%s", body)
+				assert.Equal(t, text.ErrNoActiveSession, gjson.GetBytes(body, "error.id").String(),"%s", body)
+			})
+
 			t.Run("description=success", func(t *testing.T) {
 				user1 := testhelpers.NewHTTPClientWithArbitrarySessionCookie(t, reg)
 				res, body := initFlow(t, user1, false)
@@ -363,6 +376,7 @@ func TestHandler(t *testing.T) {
 			})
 		})
 	})
+
 	t.Run("case=relative redirect when self-service settings ui is a relative url", func(t *testing.T) {
 		reg.Config(context.Background()).MustSet(config.ViperKeySelfServiceSettingsURL, "/settings-ts")
 		user1 := testhelpers.NewNoRedirectHTTPClientWithArbitrarySessionCookie(t, reg)

session/handler.go

@@ -9,8 +9,6 @@ import (
 
 	"github.com/ory/x/decoderx"
 
-	"github.com/ory/x/errorsx"
-
 	"github.com/ory/herodot"
 
 	"github.com/ory/kratos/driver/config"
@@ -234,7 +232,7 @@ func (h *Handler) IsAuthenticated(wrap httprouter.Handle, onUnauthenticated http
 				return
 			}
 
-			h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrForbidden.WithReason("This endpoint can only be accessed with a valid session. Please log in and try again.")))
+			h.r.Writer().WriteError(w, r, errors.WithStack(NewErrNoActiveSessionFound().WithReason("This endpoint can only be accessed with a valid session. Please log in and try again.")))
 			return
 		}
 
@@ -245,7 +243,7 @@ func (h *Handler) IsAuthenticated(wrap httprouter.Handle, onUnauthenticated http
 func (h *Handler) IsNotAuthenticated(wrap httprouter.Handle, onAuthenticated httprouter.Handle) httprouter.Handle {
 	return func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 		if _, err := h.r.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
-			if errorsx.Cause(err).Error() == ErrNoActiveSessionFound.Error() {
+			if e := new(ErrNoActiveSessionFound); errors.As(err, &e) {
 				wrap(w, r, ps)
 				return
 			}

session/handler_test.go

@@ -345,7 +345,7 @@ func TestIsAuthenticated(t *testing.T) {
 		{
 			c:    http.DefaultClient,
 			call: "/privileged/without-callback",
-			code: http.StatusForbidden,
+			code: http.StatusUnauthorized,
 		},
 	} {
 		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {

session/manager.go

@@ -13,10 +13,21 @@ import (
 	"github.com/ory/herodot"
 )
 
-var (
-	// ErrNoActiveSessionFound is returned when no active cookie session could be found in the request.
-	ErrNoActiveSessionFound = herodot.ErrUnauthorized.WithID(text.ErrNoActiveSession).WithError("request does not have a valid authentication session").WithReason("No active session was found in this request.")
-)
+// ErrNoActiveSessionFound is returned when no active cookie session could be found in the request.
+type ErrNoActiveSessionFound struct {
+	*herodot.DefaultError `json:"error"`
+}
+
+// NewErrNoActiveSessionFound creates a new ErrNoActiveSessionFound
+func NewErrNoActiveSessionFound() *ErrNoActiveSessionFound {
+	return &ErrNoActiveSessionFound{
+		DefaultError: herodot.ErrUnauthorized.WithID(text.ErrNoActiveSession).WithError("request does not have a valid authentication session").WithReason("No active session was found in this request."),
+	}
+}
+
+func (e *ErrNoActiveSessionFound) EnhanceJSONError() interface{} {
+	return e
+}
 
 // ErrAALNotSatisfied is returned when an active session was found but the requested AAL is not satisfied.
 //

session/manager_http.go

@@ -132,19 +132,19 @@ func (s *ManagerHTTP) extractToken(r *http.Request) string {
 func (s *ManagerHTTP) FetchFromRequest(ctx context.Context, r *http.Request) (*Session, error) {
 	token := s.extractToken(r)
 	if token == "" {
-		return nil, errors.WithStack(ErrNoActiveSessionFound)
+		return nil, errors.WithStack(NewErrNoActiveSessionFound())
 	}
 
 	se, err := s.r.SessionPersister().GetSessionByToken(ctx, token)
 	if err != nil {
 		if errors.Is(err, herodot.ErrNotFound) || errors.Is(err, sqlcon.ErrNoRows) {
-			return nil, errors.WithStack(ErrNoActiveSessionFound)
+			return nil, errors.WithStack(NewErrNoActiveSessionFound())
 		}
 		return nil, err
 	}
 
 	if !se.IsActive() {
-		return nil, errors.WithStack(ErrNoActiveSessionFound)
+		return nil, errors.WithStack(NewErrNoActiveSessionFound())
 	}
 
 	se.Identity = se.Identity.CopyWithoutCredentials()