fix: accept OIDC login request in browser+JSON login flow

hydra/fake.go

@@ -14,33 +14,37 @@ import (
 )
 
 const (
-	FAKE_GET_LOGIN_REQUEST_RETURN_NIL_NIL = "b805f2d9-2f6d-4745-9d68-a17f48e25774"
-	FAKE_ACCEPT_REQUEST_FAIL              = "2e98454e-031b-4870-9ad6-8517df1ce604"
-	FAKE_SUCCESS                          = "5ff59a39-ecc5-467e-bb10-26644c0700ee"
+	FakeInvalidLoginChallenge = "2e98454e-031b-4870-9ad6-8517df1ce604"
+	FakeValidLoginChallenge   = "5ff59a39-ecc5-467e-bb10-26644c0700ee"
+	FakePostLoginURL          = "https://www.ory.sh/fake-post-login"
 )
 
+var ErrFakeAcceptLoginRequestFailed = errors.New("failed to accept login request")
+
 type FakeHydra struct{}
 
 var _ Hydra = &FakeHydra{}
 
-func NewFakeHydra() *FakeHydra {
+func NewFake() *FakeHydra {
 	return &FakeHydra{}
 }
 
-func (h *FakeHydra) AcceptLoginRequest(ctx context.Context, hlc uuid.UUID, sub string, amr session.AuthenticationMethods) (string, error) {
+func (h *FakeHydra) AcceptLoginRequest(_ context.Context, hlc uuid.UUID, _ string, _ session.AuthenticationMethods) (string, error) {
 	switch hlc.String() {
-	case FAKE_ACCEPT_REQUEST_FAIL:
-		return "", errors.New("failed to accept login request")
+	case FakeInvalidLoginChallenge:
+		return "", ErrFakeAcceptLoginRequestFailed
+	case FakeValidLoginChallenge:
+		return FakePostLoginURL, nil
 	default:
 		panic("unknown fake login_challenge " + hlc.String())
 	}
 }
 
-func (h *FakeHydra) GetLoginRequest(ctx context.Context, hlc uuid.NullUUID) (*hydraclientgo.OAuth2LoginRequest, error) {
+func (h *FakeHydra) GetLoginRequest(_ context.Context, hlc uuid.NullUUID) (*hydraclientgo.OAuth2LoginRequest, error) {
 	switch hlc.UUID.String() {
-	case FAKE_ACCEPT_REQUEST_FAIL:
+	case FakeInvalidLoginChallenge:
 		return &hydraclientgo.OAuth2LoginRequest{}, nil
-	case FAKE_SUCCESS:
+	case FakeValidLoginChallenge:
 		return &hydraclientgo.OAuth2LoginRequest{
 			RequestUrl: "https://www.ory.sh",
 		}, nil

internal/testhelpers/selfservice.go

@@ -161,7 +161,7 @@ func SelfServiceHookSettingsErrorHandler(t *testing.T, w http.ResponseWriter, r
 	return SelfServiceHookErrorHandler(t, w, r, settings.ErrHookAbortFlow, err)
 }
 
-func SelfServiceHookErrorHandler(t *testing.T, w http.ResponseWriter, r *http.Request, abortErr error, actualErr error) bool {
+func SelfServiceHookErrorHandler(t *testing.T, w http.ResponseWriter, _ *http.Request, abortErr error, actualErr error) bool {
 	if actualErr != nil {
 		t.Logf("received error: %+v", actualErr)
 		if errors.Is(actualErr, abortErr) {

selfservice/flow/login/handler_test.go

@@ -52,7 +52,7 @@ func init() {
 func TestFlowLifecycle(t *testing.T) {
 	ctx := context.Background()
 	conf, reg := internal.NewFastRegistryWithMocks(t)
-	reg.WithHydra(hydra.NewFakeHydra())
+	reg.WithHydra(hydra.NewFake())
 	router := x.NewRouterPublic()
 	ts, _ := testhelpers.NewKratosServerWithRouters(t, reg, router, x.NewRouterAdmin())
 	loginTS := testhelpers.NewLoginUIFlowEchoServer(t, reg)
@@ -592,7 +592,7 @@ func TestFlowLifecycle(t *testing.T) {
 			})
 
 			t.Run("case=refuses to parse oauth2 login challenge when Hydra is not configured", func(t *testing.T) {
-				res, body := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FAKE_GET_LOGIN_REQUEST_RETURN_NIL_NIL}}, false)
+				res, body := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FakeValidLoginChallenge}}, false)
 				require.Contains(t, res.Request.URL.String(), errorTS.URL)
 				require.Contains(t, string(body), "refusing to parse")
 			})
@@ -609,7 +609,7 @@ func TestFlowLifecycle(t *testing.T) {
 
 				res, _ := initUnauthenticatedFlow(t, url.Values{
 					"return_to":       {"https://example.com"},
-					"login_challenge": {hydra.FAKE_SUCCESS},
+					"login_challenge": {hydra.FakeValidLoginChallenge},
 				}, false)
 				require.Equal(t, http.StatusOK, res.StatusCode)
 				require.Contains(t, res.Request.URL.String(), loginTS.URL)
@@ -630,12 +630,12 @@ func TestFlowLifecycle(t *testing.T) {
 			})
 
 			t.Run("case=oauth2 flow init succeeds", func(t *testing.T) {
-				res, _ := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FAKE_SUCCESS}}, false)
+				res, _ := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FakeValidLoginChallenge}}, false)
 				require.Contains(t, res.Request.URL.String(), loginTS.URL)
 			})
 
 			t.Run("case=oauth2 flow init adds oauth2_login_request field", func(t *testing.T) {
-				res, body := initSPAFlow(t, url.Values{"login_challenge": {hydra.FAKE_SUCCESS}})
+				res, body := initSPAFlow(t, url.Values{"login_challenge": {hydra.FakeValidLoginChallenge}})
 				assert.NotContains(t, res.Request.URL.String(), loginTS.URL)
 
 				assert.NotEmpty(t, gjson.GetBytes(body, "oauth2_login_request").Value(), "%s", body)

selfservice/flow/login/hook.go

@@ -114,7 +114,15 @@ func (e *HookExecutor) handleLoginError(_ http.ResponseWriter, r *http.Request,
 	return flowError
 }
 
-func (e *HookExecutor) PostLoginHook(w http.ResponseWriter, r *http.Request, g node.UiNodeGroup, a *Flow, i *identity.Identity, s *session.Session, provider string) (err error) {
+func (e *HookExecutor) PostLoginHook(
+	w http.ResponseWriter,
+	r *http.Request,
+	g node.UiNodeGroup,
+	a *Flow,
+	i *identity.Identity,
+	s *session.Session,
+	provider string,
+) (err error) {
 	ctx := r.Context()
 	ctx, span := e.d.Tracer(ctx).Tracer().Start(ctx, "HookExecutor.PostLoginHook")
 	r = r.WithContext(ctx)
@@ -223,6 +231,17 @@ func (e *HookExecutor) PostLoginHook(w http.ResponseWriter, r *http.Request, g n
 		// Browser flows rely on cookies. Adding tokens in the mix will confuse consumers.
 		s.Token = ""
 
+		// If Kratos is used as a Hydra login provider, we need to redirect back to Hydra by returning a 422 status
+		// with the post login challenge URL as the body.
+		if a.OAuth2LoginChallenge.Valid {
+			postChallengeURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), a.OAuth2LoginChallenge.UUID, i.ID.String(), s.AMR)
+			if err != nil {
+				return err
+			}
+			e.d.Writer().WriteError(w, r, flow.NewBrowserLocationChangeRequiredError(postChallengeURL))
+			return nil
+		}
+
 		response := &APIFlowResponse{Session: s}
 		if required, _ := e.requiresAAL2(r, s, a); required {
 			// If AAL is not satisfied, we omit the identity to preserve the user's privacy in case of a phishing attack.

selfservice/flow/login/hook_test.go

@@ -10,8 +10,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
 	"github.com/stretchr/testify/require"
 
+	"github.com/ory/kratos/hydra"
 	"github.com/ory/kratos/session"
 
 	"github.com/gobuffalo/httptest"
@@ -29,43 +31,54 @@ import (
 )
 
 func TestLoginExecutor(t *testing.T) {
+	t.Parallel()
 	ctx := context.Background()
+
 	for _, strategy := range []identity.CredentialsType{
 		identity.CredentialsTypePassword,
 		identity.CredentialsTypeOIDC,
 		identity.CredentialsTypeTOTP,
 		identity.CredentialsTypeWebAuthn,
 		identity.CredentialsTypeLookup,
 	} {
+		strategy := strategy
+
 		t.Run("strategy="+strategy.String(), func(t *testing.T) {
+			t.Parallel()
+
 			conf, reg := internal.NewFastRegistryWithMocks(t)
+			reg.WithHydra(hydra.NewFake())
 			testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/login.schema.json")
 			conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh/")
 
-			newServer := func(t *testing.T, ft flow.Type, useIdentity *identity.Identity) *httptest.Server {
+			newServer := func(t *testing.T, ft flow.Type, useIdentity *identity.Identity, flowCallback ...func(*login.Flow)) *httptest.Server {
 				router := httprouter.New()
 
 				router.GET("/login/pre", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
-					f, err := login.NewFlow(conf, time.Minute, "", r, ft)
+					loginFlow, err := login.NewFlow(conf, time.Minute, "", r, ft)
 					require.NoError(t, err)
-					if testhelpers.SelfServiceHookLoginErrorHandler(t, w, r, reg.LoginHookExecutor().PreLoginHook(w, r, f)) {
+					if testhelpers.SelfServiceHookLoginErrorHandler(t, w, r, reg.LoginHookExecutor().PreLoginHook(w, r, loginFlow)) {
 						_, _ = w.Write([]byte("ok"))
 					}
 				})
 
 				router.GET("/login/post", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
-					a, err := login.NewFlow(conf, time.Minute, "", r, ft)
+					loginFlow, err := login.NewFlow(conf, time.Minute, "", r, ft)
 					require.NoError(t, err)
-					a.Active = strategy
-					a.RequestURL = x.RequestURL(r).String()
+					loginFlow.Active = strategy
+					loginFlow.RequestURL = x.RequestURL(r).String()
+					for _, cb := range flowCallback {
+						cb(loginFlow)
+					}
+
 					sess := session.NewInactiveSession()
 					sess.CompletedLoginFor(identity.CredentialsTypePassword, identity.AuthenticatorAssuranceLevel1)
 					if useIdentity == nil {
 						useIdentity = testhelpers.SelfServiceHookCreateFakeIdentity(t, reg)
 					}
 
 					testhelpers.SelfServiceHookLoginErrorHandler(t, w, r,
-						reg.LoginHookExecutor().PostLoginHook(w, r, strategy.ToUiNodeGroup(), a, useIdentity, sess, ""))
+						reg.LoginHookExecutor().PostLoginHook(w, r, strategy.ToUiNodeGroup(), loginFlow, useIdentity, sess, ""))
 				})
 
 				ts := httptest.NewServer(router)
@@ -149,6 +162,30 @@ func TestLoginExecutor(t *testing.T) {
 					assert.NotEmpty(t, gjson.Get(body, "session.identity.id").String())
 				})
 
+				t.Run("suite=handle login challenge with browser and application/json", func(t *testing.T) {
+					t.Run("case=includes the return_to address for a valid challenge", func(t *testing.T) {
+						t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
+
+						withOAuthChallenge := func(f *login.Flow) {
+							f.OAuth2LoginChallenge = uuid.NullUUID{Valid: true, UUID: uuid.Must(uuid.FromString(hydra.FakeValidLoginChallenge))}
+						}
+						res, body := makeRequestPost(t, newServer(t, flow.TypeBrowser, nil, withOAuthChallenge), true, url.Values{})
+						assert.EqualValues(t, http.StatusUnprocessableEntity, res.StatusCode)
+						assert.Equal(t, hydra.FakePostLoginURL, gjson.Get(body, "redirect_browser_to").String(), "%s", body)
+					})
+
+					t.Run("case=returns an error for an invalid challenge", func(t *testing.T) {
+						t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
+
+						withOAuthChallenge := func(f *login.Flow) {
+							f.OAuth2LoginChallenge = uuid.NullUUID{Valid: true, UUID: uuid.Must(uuid.FromString(hydra.FakeInvalidLoginChallenge))}
+						}
+						res, body := makeRequestPost(t, newServer(t, flow.TypeBrowser, nil, withOAuthChallenge), true, url.Values{})
+						assert.EqualValues(t, http.StatusInternalServerError, res.StatusCode)
+						assert.Equal(t, hydra.ErrFakeAcceptLoginRequestFailed.Error(), body, "%s", body)
+					})
+				})
+
 				t.Run("case=pass without hooks for browser flow with application/json", func(t *testing.T) {
 					t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
 

selfservice/strategy/oidc/strategy_login.go

@@ -149,7 +149,7 @@ func (s *Strategy) processLogin(w http.ResponseWriter, r *http.Request, a *login
 	return nil, s.handleError(w, r, a, provider.Config().ID, nil, errors.WithStack(herodot.ErrInternalServerError.WithReason("Unable to find matching OpenID Connect Credentials.").WithDebugf(`Unable to find credentials that match the given provider "%s" and subject "%s".`, provider.Config().ID, claims.Subject)))
 }
 
-func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, identityID uuid.UUID) (i *identity.Identity, err error) {
+func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ uuid.UUID) (i *identity.Identity, err error) {
 	if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel1); err != nil {
 		return nil, err
 	}

selfservice/strategy/oidc/strategy_test.go

@@ -155,6 +155,25 @@ func TestStrategy(t *testing.T) {
 		return makeRequestWithCookieJar(t, provider, action, fv, nil)
 	}
 
+	var makeJSONRequest = func(t *testing.T, provider string, action string, fv url.Values) (*http.Response, []byte) {
+		fv.Set("provider", provider)
+		client := testhelpers.NewClientWithCookieJar(t, nil, false)
+		req, err := http.NewRequest("POST", action, strings.NewReader(fv.Encode()))
+		require.NoError(t, err)
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		req.Header.Set("Accept", "application/json")
+		res, err := client.Do(req)
+		require.NoError(t, err, action)
+
+		body, err := io.ReadAll(res.Body)
+		require.NoError(t, res.Body.Close())
+		require.NoError(t, err)
+
+		require.Equal(t, 422, res.StatusCode, "%s: %s\n\t%s", action, res.Request.URL.String(), body)
+
+		return res, body
+	}
+
 	var makeAPICodeFlowRequest = func(t *testing.T, provider, action string) (returnToCode string) {
 		res, err := testhelpers.NewDebugClient(t).Post(action, "application/json", strings.NewReader(fmt.Sprintf(`{
 	"method": "oidc",
@@ -461,6 +480,25 @@ func TestStrategy(t *testing.T) {
 		})
 	})
 
+	t.Run("case=login with Browser+JSON", func(t *testing.T) {
+		subject = "login-with-browser-json@ory.sh"
+		scope = []string{"openid"}
+
+		t.Run("case=should pass login", func(t *testing.T) {
+			r := newBrowserLoginFlow(t, returnTS.URL, time.Minute)
+			action := assertFormValues(t, r.ID, "valid")
+			res, body := makeJSONRequest(t, "valid", action, url.Values{})
+
+			assert.Equal(t, "browser_location_change_required", gjson.GetBytes(body, "error.id").String(), "%s", body)
+
+			continuityCookie := res.Header.Get("Set-Cookie")
+			key, val, ok := strings.Cut(continuityCookie, "=")
+			require.True(t, ok)
+			assert.Equal(t, "ory_kratos_continuity", key)
+			assert.NotEmpty(t, val)
+		})
+	})
+
 	t.Run("suite=API with session token exchange code", func(t *testing.T) {
 		scope = []string{"openid"}
 

selfservice/strategy/password/op_login_test.go

@@ -91,7 +91,7 @@ func newHydra(t *testing.T, loginUI string, consentUI string) (hydraAdmin string
 	pool, err := dockertest.NewPool("")
 	require.NoError(t, err)
 
-	hydra, err := pool.RunWithOptions(&dockertest.RunOptions{
+	hydraResource, err := pool.RunWithOptions(&dockertest.RunOptions{
 		Repository: "oryd/hydra",
 		Tag:        "v2.0.0",
 		Env: []string{
@@ -110,18 +110,25 @@ func newHydra(t *testing.T, loginUI string, consentUI string) (hydraAdmin string
 	})
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		require.NoError(t, hydra.Close())
+		require.NoError(t, hydraResource.Close())
 	})
 
-	require.NoError(t, hydra.Expire(uint(60*5)))
+	require.NoError(t, hydraResource.Expire(uint(60*5)))
 
-	require.NotEmpty(t, hydra.GetPort("4444/tcp"), "%+v", hydra.Container.NetworkSettings.Ports)
-	require.NotEmpty(t, hydra.GetPort("4445/tcp"), "%+v", hydra.Container)
+	require.NotEmpty(t, hydraResource.GetPort("4444/tcp"), "%+v", hydraResource.Container.NetworkSettings.Ports)
+	require.NotEmpty(t, hydraResource.GetPort("4445/tcp"), "%+v", hydraResource.Container)
 
-	hydraPublic = "http://127.0.0.1:" + hydra.GetPort("4444/tcp")
-	hydraAdmin = "http://127.0.0.1:" + hydra.GetPort("4445/tcp")
+	hydraPublic = "http://127.0.0.1:" + hydraResource.GetPort("4444/tcp")
+	hydraAdmin = "http://127.0.0.1:" + hydraResource.GetPort("4445/tcp")
 
-	go pool.Client.Logs(docker.LogsOptions{ErrorStream: TestLogWriter{T: t, streamName: "hydra-stderr"}, OutputStream: TestLogWriter{T: t, streamName: "hydra-stdout"}, Stdout: true, Stderr: true, Follow: true, Container: hydra.Container.ID})
+	go pool.Client.Logs(docker.LogsOptions{
+		ErrorStream:  TestLogWriter{T: t, streamName: "hydra-stderr"},
+		OutputStream: TestLogWriter{T: t, streamName: "hydra-stdout"},
+		Stdout:       true,
+		Stderr:       true,
+		Follow:       true,
+		Container:    hydraResource.Container.ID,
+	})
 	hl := logrusx.New("hydra-ready-check", "hydra-ready-check")
 	err = resilience.Retry(hl, time.Second*1, time.Second*5, func() error {
 		pr := hydraPublic + "/health/ready"