From d546badb87c6d58540436cb9b333d74b48893fa5 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Mon, 31 Jul 2023 10:57:25 +0200 Subject: [PATCH] fix: return 400 bad request for invalid login challenge --- hydra/fake.go | 3 ++- hydra/hydra.go | 8 +++++++- selfservice/flow/login/handler.go | 2 +- selfservice/flow/login/handler_test.go | 7 +++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/hydra/fake.go b/hydra/fake.go index 3d1bf852c97d..90215b8b83ef 100644 --- a/hydra/fake.go +++ b/hydra/fake.go @@ -7,6 +7,7 @@ import ( "context" "errors" + "github.com/ory/herodot" hydraclientgo "github.com/ory/hydra-client-go/v2" "github.com/ory/kratos/session" ) @@ -41,7 +42,7 @@ func (h *FakeHydra) AcceptLoginRequest(_ context.Context, loginChallenge string, func (h *FakeHydra) GetLoginRequest(_ context.Context, loginChallenge string) (*hydraclientgo.OAuth2LoginRequest, error) { switch loginChallenge { case FakeInvalidLoginChallenge: - return &hydraclientgo.OAuth2LoginRequest{}, nil + return nil, herodot.ErrBadRequest.WithReasonf("Unable to get OAuth 2.0 Login Challenge.") case FakeValidLoginChallenge: return &hydraclientgo.OAuth2LoginRequest{ RequestUrl: "https://www.ory.sh", diff --git a/hydra/hydra.go b/hydra/hydra.go index 695eeed55e91..c0e94f63d5d3 100644 --- a/hydra/hydra.go +++ b/hydra/hydra.go @@ -137,7 +137,13 @@ func (h *DefaultHydra) GetLoginRequest(ctx context.Context, loginChallenge strin hlr, r, err := aa.GetOAuth2LoginRequest(ctx).LoginChallenge(loginChallenge).Execute() if err != nil { - innerErr := herodot.ErrInternalServerError.WithWrap(err).WithReasonf("Unable to get OAuth 2.0 Login Challenge.") + var innerErr *herodot.DefaultError + if r == nil || r.StatusCode >= 500 { + innerErr = &herodot.ErrInternalServerError + } else { + innerErr = &herodot.ErrBadRequest + } + innerErr = innerErr.WithReasonf("Unable to get OAuth 2.0 Login Challenge.") if r != nil { innerErr = innerErr. WithDetail("status_code", r.StatusCode). diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go index 4493703fcbab..de545cedcd69 100644 --- a/selfservice/flow/login/handler.go +++ b/selfservice/flow/login/handler.go @@ -419,7 +419,7 @@ func (h *Handler) createBrowserLoginFlow(w http.ResponseWriter, r *http.Request, hydraLoginRequest, err = h.d.Hydra().GetLoginRequest(r.Context(), string(hydraLoginChallenge)) if err != nil { - h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, errors.WithStack(herodot.ErrInternalServerError.WithReason("Failed to retrieve OAuth 2.0 login request."))) + h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err) return } diff --git a/selfservice/flow/login/handler_test.go b/selfservice/flow/login/handler_test.go index eb1b7e3d6a4a..ada0814aaf1d 100644 --- a/selfservice/flow/login/handler_test.go +++ b/selfservice/flow/login/handler_test.go @@ -751,6 +751,13 @@ func TestFlowLifecycle(t *testing.T) { assert.Equal(t, "https://www.ory.sh", gjson.GetBytes(body, "return_to").Value()) }) + t.Run("case=invalid oauth2 login challenge returns 400 Bad Request", func(t *testing.T) { + res, body := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FakeInvalidLoginChallenge}}, false) + assert.Contains(t, res.Request.URL.String(), errorTS.URL) + assert.Equal(t, int64(http.StatusBadRequest), gjson.GetBytes(body, "code").Int()) + assert.Contains(t, gjson.GetBytes(body, "reason").String(), "Unable to get OAuth 2.0 Login Challenge") + }) + t.Run("case=oauth2 flow init succeeds", func(t *testing.T) { res, _ := initAuthenticatedFlow(t, url.Values{"login_challenge": {hydra.FakeValidLoginChallenge}}, false) require.Contains(t, res.Request.URL.String(), loginTS.URL)