diff --git a/hydra/fake.go b/hydra/fake.go index b476418d8c4..02c908d0b0c 100644 --- a/hydra/fake.go +++ b/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 diff --git a/internal/testhelpers/selfservice.go b/internal/testhelpers/selfservice.go index fc2c1156aa3..a827aad0a6d 100644 --- a/internal/testhelpers/selfservice.go +++ b/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) { diff --git a/selfservice/flow/login/handler_test.go b/selfservice/flow/login/handler_test.go index 9665c77a848..d8887784ded 100644 --- a/selfservice/flow/login/handler_test.go +++ b/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) diff --git a/selfservice/flow/login/hook.go b/selfservice/flow/login/hook.go index 3703ce9629c..8405d6b7f04 100644 --- a/selfservice/flow/login/hook.go +++ b/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. diff --git a/selfservice/flow/login/hook_test.go b/selfservice/flow/login/hook_test.go index f103453b024..a47c594fc18 100644 --- a/selfservice/flow/login/hook_test.go +++ b/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,7 +31,9 @@ import ( ) func TestLoginExecutor(t *testing.T) { + t.Parallel() ctx := context.Background() + for _, strategy := range []identity.CredentialsType{ identity.CredentialsTypePassword, identity.CredentialsTypeOIDC, @@ -37,27 +41,36 @@ func TestLoginExecutor(t *testing.T) { 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 { @@ -65,7 +78,7 @@ func TestLoginExecutor(t *testing.T) { } 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)) diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index 40999b3ff0f..edf3faeb80a 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/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 } diff --git a/selfservice/strategy/oidc/strategy_test.go b/selfservice/strategy/oidc/strategy_test.go index 7205ee20d77..18167803345 100644 --- a/selfservice/strategy/oidc/strategy_test.go +++ b/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"} diff --git a/selfservice/strategy/password/op_login_test.go b/selfservice/strategy/password/op_login_test.go index 35a2ea57e32..35136a399b4 100644 --- a/selfservice/strategy/password/op_login_test.go +++ b/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"