fix: handle subject mismatch gracefully (#3619)

We now redirect to the original request URL if the subjects between
the remembered Hydra session and what was confirmed by the login
screen does not match.

consent/handler.go

@@ -423,46 +423,57 @@ func (h *Handler) acceptOAuth2LoginRequest(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	var p flow.HandledLoginRequest
+	var handledLoginRequest flow.HandledLoginRequest
 	d := json.NewDecoder(r.Body)
 	d.DisallowUnknownFields()
-	if err := d.Decode(&p); err != nil {
+	if err := d.Decode(&handledLoginRequest); err != nil {
 		h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithWrap(err).WithHintf("Unable to decode body because: %s", err)))
 		return
 	}
 
-	if p.Subject == "" {
+	if handledLoginRequest.Subject == "" {
 		h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Field 'subject' must not be empty.")))
 		return
 	}
 
-	p.ID = challenge
-	ar, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
+	handledLoginRequest.ID = challenge
+	loginRequest, err := h.r.ConsentManager().GetLoginRequest(ctx, challenge)
 	if err != nil {
 		h.r.Writer().WriteError(w, r, err)
 		return
-	} else if ar.Subject != "" && p.Subject != ar.Subject {
-		h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Field 'subject' does not match subject from previous authentication.")))
+	} else if loginRequest.Subject != "" && handledLoginRequest.Subject != loginRequest.Subject {
+		// The subject that was confirmed by the login screen does not match what we
+		// remembered in the session cookie. We handle this gracefully by redirecting the
+		// original authorization request URL, but attaching "prompt=login" to the query.
+		// This forces the user to log in again.
+		requestURL, err := url.Parse(loginRequest.RequestURL)
+		if err != nil {
+			h.r.Writer().WriteError(w, r, err)
+			return
+		}
+		h.r.Writer().Write(w, r, &flow.OAuth2RedirectTo{
+			RedirectTo: urlx.SetQuery(requestURL, url.Values{"prompt": {"login"}}).String(),
+		})
 		return
 	}
 
-	if ar.Skip {
-		p.Remember = true // If skip is true remember is also true to allow consecutive calls as the same user!
-		p.AuthenticatedAt = ar.AuthenticatedAt
+	if loginRequest.Skip {
+		handledLoginRequest.Remember = true // If skip is true remember is also true to allow consecutive calls as the same user!
+		handledLoginRequest.AuthenticatedAt = loginRequest.AuthenticatedAt
 	} else {
-		p.AuthenticatedAt = sqlxx.NullTime(time.Now().UTC().
+		handledLoginRequest.AuthenticatedAt = sqlxx.NullTime(time.Now().UTC().
 			// Rounding is important to avoid SQL time synchronization issues in e.g. MySQL!
 			Truncate(time.Second))
-		ar.AuthenticatedAt = p.AuthenticatedAt
+		loginRequest.AuthenticatedAt = handledLoginRequest.AuthenticatedAt
 	}
-	p.RequestedAt = ar.RequestedAt
+	handledLoginRequest.RequestedAt = loginRequest.RequestedAt
 
 	f, err := flowctx.Decode[flow.Flow](ctx, h.r.FlowCipher(), challenge, flowctx.AsLoginChallenge)
 	if err != nil {
 		h.r.Writer().WriteError(w, r, err)
 		return
 	}
-	request, err := h.r.ConsentManager().HandleLoginRequest(ctx, f, challenge, &p)
+	request, err := h.r.ConsentManager().HandleLoginRequest(ctx, f, challenge, &handledLoginRequest)
 	if err != nil {
 		h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
 		return

consent/strategy_oauth_test.go

@@ -16,15 +16,14 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/exp/slices"
+	"golang.org/x/oauth2"
+
 	"github.com/ory/hydra/v2/aead"
 	"github.com/ory/hydra/v2/consent"
 	"github.com/ory/hydra/v2/flow"
 	"github.com/ory/hydra/v2/oauth2/flowctx"
 	"github.com/ory/hydra/v2/x"
-	"github.com/ory/x/ioutilx"
-
-	"golang.org/x/exp/slices"
-	"golang.org/x/oauth2"
 
 	"github.com/ory/x/pointerx"
 
@@ -636,7 +635,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 		})
 	})
 
-	t.Run("case=should fail at login screen because subject in login challenge does not match subject from previous session", func(t *testing.T) {
+	t.Run("case=should retry the authorization with prompt=login if subject in login challenge does not match subject from previous session", func(t *testing.T) {
 		// Previously: This should fail at login screen because subject from accept does not match subject from session
 		c := createDefaultClient(t)
 		testhelpers.NewLoginConsentUI(t, reg.Config(),
@@ -649,13 +648,15 @@ func TestStrategyLoginConsentNext(t *testing.T) {
 
 		testhelpers.NewLoginConsentUI(t, reg.Config(),
 			func(w http.ResponseWriter, r *http.Request) {
-				_, res, err := adminClient.OAuth2Api.AcceptOAuth2LoginRequest(context.Background()).
+				res, _, err := adminClient.OAuth2Api.AcceptOAuth2LoginRequest(context.Background()).
 					LoginChallenge(r.URL.Query().Get("login_challenge")).
 					AcceptOAuth2LoginRequest(hydra.AcceptOAuth2LoginRequest{
 						Subject: "not-aeneas-rekkas",
 					}).Execute()
-				require.Error(t, err)
-				assert.Contains(t, string(ioutilx.MustReadAll(res.Body)), "Field 'subject' does not match subject from previous authentication")
+				require.NoError(t, err)
+				redirectURL, err := url.Parse(res.RedirectTo)
+				require.NoError(t, err)
+				assert.Equal(t, "login", redirectURL.Query().Get("prompt"))
 				w.WriteHeader(http.StatusBadRequest)
 			},
 			testhelpers.HTTPServerNoExpectedCallHandler(t))