fix: don't return nil if code is invalid (#3662)

* fix: don't return nil if code is invalid

* chore: add test

selfservice/strategy/code/strategy_recovery.go

@@ -254,8 +254,12 @@ func (s *Strategy) recoveryUseCode(w http.ResponseWriter, r *http.Request, body
 			return s.retryRecoveryFlow(w, r, f.Type, RetryWithError(err))
 		}
 
-		// No error
-		return nil
+		if f.Type == flow.TypeBrowser && !x.IsJSONRequest(r) {
+			http.Redirect(w, r, f.AppendTo(s.deps.Config().SelfServiceFlowRecoveryUI(r.Context())).String(), http.StatusSeeOther)
+		} else {
+			s.deps.Writer().Write(w, r, f)
+		}
+		return errors.WithStack(flow.ErrCompletedByStrategy)
 	} else if err != nil {
 		return s.retryRecoveryFlow(w, r, f.Type, RetryWithError(err))
 	}

selfservice/strategy/code/strategy_recovery_test.go

@@ -1597,7 +1597,7 @@ func TestRecovery_WithContinueWith(t *testing.T) {
 			t.Run("type="+testCase.ClientType.String(), func(t *testing.T) {
 				recoveryEmail := testhelpers.RandomEmail()
 				createIdentityToRecover(t, reg, recoveryEmail)
-				conf.MustSet(ctx, config.ViperKeySelfServiceRecoveryRequestLifespan, time.Millisecond*10)
+				conf.MustSet(ctx, config.ViperKeySelfServiceRecoveryRequestLifespan, time.Millisecond*100)
 				t.Cleanup(func() {
 					conf.MustSet(ctx, config.ViperKeySelfServiceRecoveryRequestLifespan, time.Minute)
 				})
@@ -1611,15 +1611,15 @@ func TestRecovery_WithContinueWith(t *testing.T) {
 					fallthrough
 				case RecoveryClientTypeSPA:
 					rs = testhelpers.GetRecoveryFlow(t, c, public)
-					time.Sleep(time.Millisecond * 11)
+					time.Sleep(time.Millisecond * 110)
 					res, err = c.PostForm(rs.Ui.Action, url.Values{"email": {recoveryEmail}, "method": {"code"}})
 					require.NoError(t, err)
 					assert.EqualValues(t, http.StatusOK, res.StatusCode)
 					assert.NotContains(t, res.Request.URL.String(), "flow="+rs.Id)
 					assert.Contains(t, res.Request.URL.String(), conf.SelfServiceFlowRecoveryUI(ctx).String())
 				case RecoveryClientTypeAPI:
 					rs = testhelpers.InitializeRecoveryFlowViaAPI(t, c, public)
-					time.Sleep(time.Millisecond * 11)
+					time.Sleep(time.Millisecond * 110)
 					form := testhelpers.EncodeFormAsJSON(t, true, url.Values{"email": {recoveryEmail}, "method": {"code"}})
 					res, err = c.Post(rs.Ui.Action, "application/json", bytes.NewBufferString(form))
 					require.NoError(t, err)

selfservice/strategy/code/strategy_verification.go

@@ -236,8 +236,12 @@ func (s *Strategy) verificationUseCode(w http.ResponseWriter, r *http.Request, c
 			return s.retryVerificationFlowWithError(w, r, f.Type, err)
 		}
 
-		// No error
-		return nil
+		if x.IsBrowserRequest(r) {
+			http.Redirect(w, r, f.AppendTo(s.deps.Config().SelfServiceFlowVerificationUI(r.Context())).String(), http.StatusSeeOther)
+		} else {
+			s.deps.Writer().Write(w, r, f)
+		}
+		return errors.WithStack(flow.ErrCompletedByStrategy)
 	} else if err != nil {
 		return s.retryVerificationFlowWithError(w, r, f.Type, err)
 	}

selfservice/strategy/code/strategy_verification_test.go

@@ -16,6 +16,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
+
 	"github.com/ory/x/urlx"
 
 	"github.com/ory/kratos/selfservice/strategy/code"
@@ -377,6 +379,11 @@ func TestVerification(t *testing.T) {
 		f, err := verification.NewFlow(conf, time.Hour, x.FakeCSRFToken, httptest.NewRequest("GET", requestURL, nil), code.NewStrategy(reg), fType)
 		require.NoError(t, err)
 		f.State = flow.StateEmailSent
+		u, err := url.Parse(f.RequestURL)
+		require.NoError(t, err)
+		f.OAuth2LoginChallenge = sqlxx.NullString(u.Query().Get("login_challenge"))
+		f.IdentityID = uuid.NullUUID{UUID: x.NewUUID(), Valid: true}
+		f.SessionID = uuid.NullUUID{UUID: x.NewUUID(), Valid: true}
 		require.NoError(t, reg.VerificationFlowPersister().CreateVerificationFlow(context.Background(), f))
 		email := identity.NewVerifiableEmailAddress(verificationEmail, identityToVerify.ID)
 		identityToVerify.VerifiableAddresses = append(identityToVerify.VerifiableAddresses, *email)
@@ -634,4 +641,25 @@ func TestVerification(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("case=doesn't continue with OAuth2 flow if code is invalid", func(t *testing.T) {
+		returnToURL := public.URL + "/after-verification"
+		conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{returnToURL})
+
+		client := testhelpers.NewClientWithCookies(t)
+		flow, _, _ := newValidFlow(t, flow.TypeBrowser, public.URL+verification.RouteInitBrowserFlow+"?"+url.Values{"return_to": {returnToURL}, "login_challenge": {"any_valid_challenge"}}.Encode())
+
+		body := fmt.Sprintf(
+			`{"csrf_token":"%s","code":"%s"}`, flow.CSRFToken, "2475",
+		)
+
+		res, err := client.Post(public.URL+verification.RouteSubmitFlow+"?"+url.Values{"flow": {flow.ID.String()}}.Encode(), "application/json", bytes.NewBuffer([]byte(body)))
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		responseBody := gjson.ParseBytes(ioutilx.MustReadAll(res.Body))
+
+		assert.Equal(t, responseBody.Get("state").String(), "sent_email", "%v", responseBody)
+		assert.Len(t, responseBody.Get("ui.messages").Array(), 1, "%v", responseBody)
+		assert.Equal(t, "The verification code is invalid or has already been used. Please try again.", responseBody.Get("ui.messages.0.text").String(), "%v", responseBody)
+	})
 }