diff --git a/persistence/sql/persister_recovery_code.go b/persistence/sql/persister_recovery_code.go index d684a1cb4cb5..694e3f9e3b64 100644 --- a/persistence/sql/persister_recovery_code.go +++ b/persistence/sql/persister_recovery_code.go @@ -77,6 +77,15 @@ func (p *Persister) UseRecoveryCode(ctx context.Context, flowID uuid.UUID, userP return codeRow, nil } +func (p *Persister) CountRecoveryCodeSubmissions(ctx context.Context, flowID uuid.UUID) (count int, err error) { + ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.CountRecoveryCodeSubmissions") + defer otelx.End(span, &err) + + q := "SELECT submit_count FROM selfservice_recovery_flows WHERE id = ? AND nid = ?" + err = sqlcon.HandleError(p.GetConnection(ctx).RawQuery(q, flowID, p.NetworkID(ctx)).First(&count)) + return count, err +} + func (p *Persister) DeleteRecoveryCodesOfFlow(ctx context.Context, flowID uuid.UUID) (err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.DeleteRecoveryCodesOfFlow") defer otelx.End(span, &err) diff --git a/selfservice/strategy/code/persistence.go b/selfservice/strategy/code/persistence.go index ea5aaff682cc..a535b52803fe 100644 --- a/selfservice/strategy/code/persistence.go +++ b/selfservice/strategy/code/persistence.go @@ -13,6 +13,7 @@ type ( RecoveryCodePersister interface { CreateRecoveryCode(ctx context.Context, dto *CreateRecoveryCodeParams) (*RecoveryCode, error) UseRecoveryCode(ctx context.Context, fID uuid.UUID, code string) (*RecoveryCode, error) + CountRecoveryCodeSubmissions(ctx context.Context, fID uuid.UUID) (int, error) DeleteRecoveryCodesOfFlow(ctx context.Context, fID uuid.UUID) error } diff --git a/selfservice/strategy/code/strategy_recovery.go b/selfservice/strategy/code/strategy_recovery.go index bd132144b874..9f7878f34293 100644 --- a/selfservice/strategy/code/strategy_recovery.go +++ b/selfservice/strategy/code/strategy_recovery.go @@ -183,6 +183,15 @@ func (s *Strategy) Recover(w http.ResponseWriter, r *http.Request, f *recovery.F } else if err := flow.EnsureCSRF(s.deps, r, f.Type, s.deps.Config().DisableAPIFlowEnforcement(ctx), s.deps.GenerateCSRFToken, body.CSRFToken); err != nil { // If a CSRF violation occurs the flow is most likely FUBAR, as the user either lost the CSRF token, or an attack occured. // In this case, we just issue a new flow and "abandon" the old flow. + // + // When the code has already been submitted too often, prefer that error over CSRF. + // Otherwise the stale token from previous failed attempts hides the 410/too-often result. + if len(body.Code) > 0 { + if submitCount, countErr := s.deps.RecoveryCodePersister().CountRecoveryCodeSubmissions(ctx, f.ID); countErr == nil && + submitCount >= s.deps.Config().SelfServiceCodeMethodMaxSubmissions(ctx) { + return s.retryRecoveryFlow(w, r, f.Type, RetryWithError(ErrCodeSubmittedTooOften())) + } + } return s.retryRecoveryFlow(w, r, flow.TypeBrowser, RetryWithError(err)) } diff --git a/selfservice/strategy/code/strategy_recovery_test.go b/selfservice/strategy/code/strategy_recovery_test.go index 47fb9d1bd43f..bc56848bc73b 100644 --- a/selfservice/strategy/code/strategy_recovery_test.go +++ b/selfservice/strategy/code/strategy_recovery_test.go @@ -1552,6 +1552,70 @@ func TestRecovery_WithContinueWith(t *testing.T) { } }) + t.Run("description=should return submitted-too-often instead of CSRF when max submissions exceeded with a stale csrf token", func(t *testing.T) { + for _, testCase := range flowTypeCases { + if testCase.ClientType == RecoveryClientTypeAPI { + continue + } + t.Run("type="+testCase.ClientType.String(), func(t *testing.T) { + email := testhelpers.RandomEmail() + createIdentityToRecover(t, reg, email) + c := testCase.GetClient(t) + body := submitRecoveryForm(t, c, testCase.ClientType, func(v url.Values) { + v.Set("email", email) + }, http.StatusOK) + + initialFlowId := gjson.Get(body, "id") + + for submitTry := 0; submitTry < 5; submitTry++ { + inner := submitRecoveryCode(t, c, body, testCase.ClientType, "12312312", http.StatusOK) + testhelpers.AssertMessage(t, inner, "The recovery code is invalid or has already been used. Please try again.") + } + + action := gjson.Get(body, "ui.action").String() + require.NotEmpty(t, action) + + values := url.Values{ + "code": {"12312312"}, + "method": {"code"}, + "csrf_token": {"stale-csrf-token"}, + } + payload := values.Encode() + contentType := "application/x-www-form-urlencoded" + if testCase.ClientType != RecoveryClientTypeBrowser { + payload = testhelpers.EncodeFormAsJSON(t, true, values) + contentType = "application/json" + } + + res, err := c.Post(action, contentType, bytes.NewBufferString(payload)) + require.NoError(t, err) + got := string(ioutilx.MustReadAll(res.Body)) + require.NoError(t, res.Body.Close()) + + assert.NotEqual(t, http.StatusForbidden, res.StatusCode, "%s", got) + assert.NotEqual(t, text.ErrIDCSRF, gjson.Get(got, "error.id").String(), "%s", got) + + switch testCase.ClientType { + case RecoveryClientTypeBrowser: + assert.Equal(t, http.StatusOK, res.StatusCode, "%s", got) + require.Len(t, gjson.Get(got, "ui.messages").Array(), 1, "%s", got) + assert.Equal(t, "The request was submitted too often. Please request another code.", gjson.Get(got, "ui.messages.0.text").String()) + assert.NotEqual(t, gjson.Get(got, "id"), initialFlowId) + assert.True(t, gjson.Get(got, "ui.nodes.#(attributes.name==email)").Exists()) + case RecoveryClientTypeSPA: + assert.Equal(t, http.StatusBadRequest, res.StatusCode, "%s", got) + assert.Equal(t, "The request was submitted too often. Please request another code.", gjson.Get(got, "error.reason").String(), "%s", got) + continueWith := gjson.Get(got, "error.details.continue_with").Array() + assert.Len(t, continueWith, 1, "%s", got) + assert.Equal(t, "show_recovery_ui", continueWith[0].Get("action").String(), "%s", got) + flowId := continueWith[0].Get("flow.id").String() + assert.NotEmpty(t, flowId, "%s", got) + require.NotEqual(t, flowId, initialFlowId.String(), "%s", got) + } + }) + } + }) + t.Run("description=should be able to recover after using invalid code", func(t *testing.T) { for _, testCase := range flowTypeCases { t.Run("type="+testCase.ClientType.String(), func(t *testing.T) {