fix: return 400 bad request for invalid login challenge (#3404)

ory · Jul 31, 2023 · ca34e9b · ca34e9b
1 parent 53080b0
commit ca34e9b
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)