feat: mark recovery email address verified (#1665)

Closes #1662

docs/docs/self-service/flows/account-recovery.mdx

@@ -155,6 +155,17 @@ You should also configure how long a session is privileged. The user will only
 be able to update his/her password (or any other credential) for the specified
 amount of time after clicking on the recovery link.
 
+:::note
+
+If the email address used for recovery purposes is the same as the email used
+for verification purposes and the account has not yet been activated (see
+[Account Activation](../../verify-email-account-activation.mdx) - that is, if
+the email address is not yet verified, the verification status of the
+corresponding object in Kratos will be set to verified upon successful flow
+finalization.
+
+:::
+
 <Tabs
   defaultValue="oss"
   values={[

docs/docs/self-service/flows/verify-email-account-activation.mdx

@@ -88,14 +88,14 @@ selfservice:
     link:
       enabled: true
       config:
-        # Defines how long a recovery link is valid for (default 1h)
+        # Defines how long the verification or the recovery link is valid for (default 1h)
         lifespan: 15m
 
   flows:
     verification:
       enabled: true
 
-      # Defines how long a recovery flow (the UI interaction, not the link!)
+      # Defines how long the verification flow (the UI interaction, not the link!)
       # is valid for (default 1h)
       lifespan: 15m
 ```
@@ -112,13 +112,17 @@ Gateways, or make Ory Kratos prevent signing into accounts without verified
 addresses. Latter is possible by the usage of `require_verified_address` hook
 (See [Hooks](../hooks.mdx) for more details).
 
+:::note
+
 Please be aware, that since `require_verified_address` hook is enforcing a
 verified address before the user can login, a typo in an email address done by
-the user either during the registration or as part of a self service flow (email
+the user either during the registration or as part of a self-service flow (email
 change) will make the login for that user impossible. So you should think about
 measures to prevent such situations, like requiring two email addresses being
 configured by the user, thus having a backup if something goes wrong.
 
+:::
+
 ## Verification Methods
 
 Currently, Ory Kratos only supports one verification method:

selfservice/strategy/link/strategy_recovery.go

@@ -255,24 +255,19 @@ func (s *Strategy) Recover(w http.ResponseWriter, r *http.Request, f *recovery.F
 	}
 }
 
-func (s *Strategy) recoveryIssueSession(w http.ResponseWriter, r *http.Request, f *recovery.Flow, recoveredID uuid.UUID) error {
-	recovered, err := s.d.IdentityPool().GetIdentity(r.Context(), recoveredID)
-	if err != nil {
-		return s.HandleRecoveryError(w, r, f, nil, err)
-	}
-
+func (s *Strategy) recoveryIssueSession(w http.ResponseWriter, r *http.Request, f *recovery.Flow, id *identity.Identity) error {
 	f.UI.Messages.Clear()
 	f.State = recovery.StatePassedChallenge
 	f.SetCSRFToken(flow.GetCSRFToken(s.d, w, r, f.Type))
 	f.RecoveredIdentityID = uuid.NullUUID{
-		UUID:  recoveredID,
+		UUID:  id.ID,
 		Valid: true,
 	}
 	if err := s.d.RecoveryFlowPersister().UpdateRecoveryFlow(r.Context(), f); err != nil {
 		return s.HandleRecoveryError(w, r, f, nil, err)
 	}
 
-	sess, _ := session.NewActiveSession(recovered, s.d.Config(r.Context()), time.Now().UTC())
+	sess, _ := session.NewActiveSession(id, s.d.Config(r.Context()), time.Now().UTC())
 	if err := s.d.SessionManager().CreateAndIssueCookie(r.Context(), w, r, sess); err != nil {
 		return s.HandleRecoveryError(w, r, f, nil, err)
 	}
@@ -327,7 +322,19 @@ func (s *Strategy) recoveryUseToken(w http.ResponseWriter, r *http.Request, body
 		return s.HandleRecoveryError(w, r, f, body, err)
 	}
 
-	return s.recoveryIssueSession(w, r, f, token.RecoveryAddress.IdentityID)
+	recovered, err := s.d.IdentityPool().GetIdentity(r.Context(), token.RecoveryAddress.IdentityID)
+	if err != nil {
+		return s.HandleRecoveryError(w, r, f, nil, err)
+	}
+
+	// mark address as verified only for a self-service flow
+	if token.FlowID.Valid {
+		if err := s.markRecoveryAddressVerified(w, r, f, recovered, token.RecoveryAddress); err != nil {
+			return s.HandleRecoveryError(w, r, f, body, err)
+		}
+	}
+
+	return s.recoveryIssueSession(w, r, f, recovered)
 }
 
 func (s *Strategy) retryRecoveryFlowWithMessage(w http.ResponseWriter, r *http.Request, ft flow.Type, message *text.Message) error {
@@ -353,38 +360,61 @@ func (s *Strategy) retryRecoveryFlowWithMessage(w http.ResponseWriter, r *http.R
 	return errors.WithStack(flow.ErrCompletedByStrategy)
 }
 
-func (s *Strategy) recoveryHandleFormSubmission(w http.ResponseWriter, r *http.Request, req *recovery.Flow) error {
+func (s *Strategy) recoveryHandleFormSubmission(w http.ResponseWriter, r *http.Request, f *recovery.Flow) error {
 	body, err := s.decodeRecovery(r)
 	if err != nil {
-		return s.HandleRecoveryError(w, r, req, body, err)
+		return s.HandleRecoveryError(w, r, f, body, err)
 	}
 
 	if len(body.Email) == 0 {
-		return s.HandleRecoveryError(w, r, req, body, schema.NewRequiredError("#/email", "email"))
+		return s.HandleRecoveryError(w, r, f, body, schema.NewRequiredError("#/email", "email"))
 	}
 
-	if err := flow.EnsureCSRF(s.d, r, req.Type, s.d.Config(r.Context()).DisableAPIFlowEnforcement(), s.d.GenerateCSRFToken, body.CSRFToken); err != nil {
-		return s.HandleRecoveryError(w, r, req, body, err)
+	if err := flow.EnsureCSRF(s.d, r, f.Type, s.d.Config(r.Context()).DisableAPIFlowEnforcement(), s.d.GenerateCSRFToken, body.CSRFToken); err != nil {
+		return s.HandleRecoveryError(w, r, f, body, err)
 	}
 
-	if err := s.d.LinkSender().SendRecoveryLink(r.Context(), r, req, identity.VerifiableAddressTypeEmail, body.Email); err != nil {
+	if err := s.d.LinkSender().SendRecoveryLink(r.Context(), r, f, identity.VerifiableAddressTypeEmail, body.Email); err != nil {
 		if !errors.Is(err, ErrUnknownAddress) {
-			return s.HandleRecoveryError(w, r, req, body, err)
+			return s.HandleRecoveryError(w, r, f, body, err)
 		}
 		// Continue execution
 	}
 
-	req.UI.SetCSRF(s.d.GenerateCSRFToken(r))
-	req.UI.GetNodes().Upsert(
+	f.UI.SetCSRF(s.d.GenerateCSRFToken(r))
+	f.UI.GetNodes().Upsert(
 		// v0.5: form.Field{Name: "email", Type: "email", Required: true, Value: body.Body.Email}
 		node.NewInputField("email", body.Email, node.RecoveryLinkGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute),
 	)
 
-	req.Active = sqlxx.NullString(s.RecoveryNodeGroup())
-	req.State = recovery.StateEmailSent
-	req.UI.Messages.Set(text.NewRecoveryEmailSent())
-	if err := s.d.RecoveryFlowPersister().UpdateRecoveryFlow(r.Context(), req); err != nil {
-		return s.HandleRecoveryError(w, r, req, body, err)
+	f.Active = sqlxx.NullString(s.RecoveryNodeGroup())
+	f.State = recovery.StateEmailSent
+	f.UI.Messages.Set(text.NewRecoveryEmailSent())
+	if err := s.d.RecoveryFlowPersister().UpdateRecoveryFlow(r.Context(), f); err != nil {
+		return s.HandleRecoveryError(w, r, f, body, err)
+	}
+
+	return nil
+}
+
+func (s *Strategy) markRecoveryAddressVerified(w http.ResponseWriter, r *http.Request, f *recovery.Flow, id *identity.Identity, recoveryAddress *identity.RecoveryAddress) error {
+	var address *identity.VerifiableAddress
+	for idx := range id.VerifiableAddresses {
+		va := id.VerifiableAddresses[idx]
+		if va.Value == recoveryAddress.Value {
+			address = &va
+			break
+		}
+	}
+
+	if address != nil && !address.Verified { // can it be that the address is nil?
+		address.Verified = true
+		verifiedAt := sqlxx.NullTime(time.Now().UTC())
+		address.VerifiedAt = &verifiedAt
+		address.Status = identity.VerifiableAddressStatusCompleted
+		if err := s.d.PrivilegedIdentityPool().UpdateVerifiableAddress(r.Context(), address); err != nil {
+			return s.HandleRecoveryError(w, r, f, nil, err)
+		}
 	}
 
 	return nil

selfservice/strategy/link/strategy_recovery_test.go

@@ -113,7 +113,8 @@ func TestAdminStrategy(t *testing.T) {
 	})
 
 	t.Run("description=should create a valid recovery link and set the expiry time and not be able to recover the account", func(t *testing.T) {
-		id := identity.Identity{Traits: identity.Traits(`{"email":"recover.expired@ory.sh"}`)}
+		recoveryEmail := "recover.expired@ory.sh"
+		id := identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"email":"%s"}`, recoveryEmail))}
 
 		require.NoError(t, reg.IdentityManager().Create(context.Background(),
 			&id, identity.ManagerAllowWriteProtectedTraits))
@@ -133,10 +134,17 @@ func TestAdminStrategy(t *testing.T) {
 
 		// We end up here because the link is expired.
 		assert.Contains(t, res.Request.URL.Path, "/recover", rl.RecoveryLink)
+
+		addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+		assert.NoError(t, err)
+		assert.False(t, addr.Verified)
+		assert.Nil(t, addr.VerifiedAt)
+		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
 	})
 
 	t.Run("description=should create a valid recovery link and set the expiry time as well and recover the account", func(t *testing.T) {
-		id := identity.Identity{Traits: identity.Traits(`{"email":"recoverme@ory.sh"}`)}
+		recoveryEmail := "recoverme@ory.sh"
+		id := identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"email":"%s"}`, recoveryEmail))}
 
 		require.NoError(t, reg.IdentityManager().Create(context.Background(),
 			&id, identity.ManagerAllowWriteProtectedTraits))
@@ -159,18 +167,16 @@ func TestAdminStrategy(t *testing.T) {
 
 		require.Len(t, f.UI.Messages, 1)
 		assert.Equal(t, "You successfully recovered your account. Please change your password or set up an alternative login method (e.g. social sign in) within the next 60.00 minutes.", f.UI.Messages[0].Text)
+
+		addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+		assert.NoError(t, err)
+		assert.False(t, addr.Verified)
+		assert.Nil(t, addr.VerifiedAt)
+		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
 	})
 }
 
 func TestRecovery(t *testing.T) {
-	var identityToRecover = &identity.Identity{
-		Credentials: map[identity.CredentialsType]identity.Credentials{
-			"password": {Type: "password", Identifiers: []string{"recoverme@ory.sh"}, Config: sqlxx.JSONRawMessage(`{"hashed_password":"foo"}`)}},
-		Traits:   identity.Traits(`{"email":"recoverme@ory.sh"}`),
-		SchemaID: config.DefaultIdentityTraitsSchemaID,
-	}
-	var recoveryEmail = gjson.GetBytes(identityToRecover.Traits, "email").String()
-
 	conf, reg := internal.NewFastRegistryWithMocks(t)
 	initViper(t, conf)
 
@@ -181,8 +187,21 @@ func TestRecovery(t *testing.T) {
 
 	public, _ := testhelpers.NewKratosServerWithCSRF(t, reg)
 
-	require.NoError(t, reg.IdentityManager().Create(context.Background(), identityToRecover,
-		identity.ManagerAllowWriteProtectedTraits))
+	var createIdentityToRecover = func(email string) {
+		var id = &identity.Identity{
+			Credentials: map[identity.CredentialsType]identity.Credentials{
+				"password": {Type: "password", Identifiers: []string{email}, Config: sqlxx.JSONRawMessage(`{"hashed_password":"foo"}`)}},
+			Traits:   identity.Traits(fmt.Sprintf(`{"email":"%s"}`, email)),
+			SchemaID: config.DefaultIdentityTraitsSchemaID,
+		}
+		require.NoError(t, reg.IdentityManager().Create(context.Background(), id, identity.ManagerAllowWriteProtectedTraits))
+
+		addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, email)
+		assert.NoError(t, err)
+		assert.False(t, addr.Verified)
+		assert.Nil(t, addr.VerifiedAt)
+		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
+	}
 
 	var expect = func(t *testing.T, hc *http.Client, isAPI, isSPA bool, values func(url.Values), c int) string {
 		if hc == nil {
@@ -305,11 +324,17 @@ func TestRecovery(t *testing.T) {
 	})
 
 	t.Run("description=should recover an account", func(t *testing.T) {
-		var check = func(t *testing.T, actual string) {
-			assert.EqualValues(t, node.RecoveryLinkGroup, gjson.Get(actual, "active").String(), "%s", actual)
-			assert.EqualValues(t, recoveryEmail, gjson.Get(actual, "ui.nodes.#(attributes.name==email).attributes.value").String(), "%s", actual)
-			require.Len(t, gjson.Get(actual, "ui.messages").Array(), 1, "%s", actual)
-			assertx.EqualAsJSON(t, text.NewRecoveryEmailSent(), json.RawMessage(gjson.Get(actual, "ui.messages.0").Raw))
+		var check = func(t *testing.T, recoverySubmissionResponse, recoveryEmail string) {
+			addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+			assert.NoError(t, err)
+			assert.False(t, addr.Verified)
+			assert.Nil(t, addr.VerifiedAt)
+			assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
+
+			assert.EqualValues(t, node.RecoveryLinkGroup, gjson.Get(recoverySubmissionResponse, "active").String(), "%s", recoverySubmissionResponse)
+			assert.EqualValues(t, recoveryEmail, gjson.Get(recoverySubmissionResponse, "ui.nodes.#(attributes.name==email).attributes.value").String(), "%s", recoverySubmissionResponse)
+			require.Len(t, gjson.Get(recoverySubmissionResponse, "ui.messages").Array(), 1, "%s", recoverySubmissionResponse)
+			assertx.EqualAsJSON(t, text.NewRecoveryEmailSent(), json.RawMessage(gjson.Get(recoverySubmissionResponse, "ui.messages.0").Raw))
 
 			message := testhelpers.CourierExpectMessage(t, reg, recoveryEmail, "Recover access to your account")
 			assert.Contains(t, message.Body, "please recover access to your account by clicking the following link")
@@ -320,6 +345,7 @@ func TestRecovery(t *testing.T) {
 			assert.Contains(t, recoveryLink, "token=")
 
 			cl := testhelpers.NewClientWithCookies(t)
+
 			res, err := cl.Get(recoveryLink)
 			require.NoError(t, err)
 
@@ -329,26 +355,42 @@ func TestRecovery(t *testing.T) {
 			body := ioutilx.MustReadAll(res.Body)
 			assert.Equal(t, text.NewRecoverySuccessful(time.Now().Add(time.Hour)).Text,
 				gjson.GetBytes(body, "ui.messages.0.text").String())
-		}
 
-		var values = func(v url.Values) {
-			v.Set("email", recoveryEmail)
+			addr, err = reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+			assert.NoError(t, err)
+			assert.True(t, addr.Verified)
+			assert.NotEqual(t, sqlxx.NullTime{}, addr.VerifiedAt)
+			assert.Equal(t, identity.VerifiableAddressStatusCompleted, addr.Status)
 		}
 
 		t.Run("type=browser", func(t *testing.T) {
-			check(t, expectSuccess(t, nil, false, false, values))
+			email := "recoverme1@ory.sh"
+			createIdentityToRecover(email)
+			check(t, expectSuccess(t, nil, false, false, func(v url.Values) {
+				v.Set("email", email)
+			}), email)
 		})
 
 		t.Run("type=spa", func(t *testing.T) {
-			check(t, expectSuccess(t, nil, true, true, values))
+			email := "recoverme2@ory.sh"
+			createIdentityToRecover(email)
+			check(t, expectSuccess(t, nil, true, true, func(v url.Values) {
+				v.Set("email", email)
+			}), email)
 		})
 
 		t.Run("type=api", func(t *testing.T) {
-			check(t, expectSuccess(t, nil, true, false, values))
+			email := "recoverme3@ory.sh"
+			createIdentityToRecover(email)
+			check(t, expectSuccess(t, nil, true, false, func(v url.Values) {
+				v.Set("email", email)
+			}), email)
 		})
 	})
 
 	t.Run("description=should recover an account and set the csrf cookies", func(t *testing.T) {
+		recoveryEmail := "recoverme1@ory.sh"
+
 		var check = func(t *testing.T, actual string) {
 			message := testhelpers.CourierExpectMessage(t, reg, recoveryEmail, "Recover access to your account")
 			recoveryLink := testhelpers.CourierExpectLinkInMessage(t, message, 1)
@@ -397,6 +439,8 @@ func TestRecovery(t *testing.T) {
 	})
 
 	t.Run("description=should not be able to use an outdated link", func(t *testing.T) {
+		recoveryEmail := "recoverme4@ory.sh"
+		createIdentityToRecover(recoveryEmail)
 		conf.MustSet(config.ViperKeySelfServiceRecoveryRequestLifespan, time.Millisecond*200)
 		t.Cleanup(func() {
 			conf.MustSet(config.ViperKeySelfServiceRecoveryRequestLifespan, time.Minute)
@@ -412,9 +456,17 @@ func TestRecovery(t *testing.T) {
 		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().String())
+
+		addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+		assert.NoError(t, err)
+		assert.False(t, addr.Verified)
+		assert.Nil(t, addr.VerifiedAt)
+		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
 	})
 
 	t.Run("description=should not be able to use an outdated flow", func(t *testing.T) {
+		recoveryEmail := "recoverme5@ory.sh"
+		createIdentityToRecover(recoveryEmail)
 		conf.MustSet(config.ViperKeySelfServiceRecoveryRequestLifespan, time.Millisecond*200)
 		t.Cleanup(func() {
 			conf.MustSet(config.ViperKeySelfServiceRecoveryRequestLifespan, time.Minute)
@@ -444,6 +496,12 @@ func TestRecovery(t *testing.T) {
 
 		require.Len(t, rs.Ui.Messages, 1)
 		assert.Contains(t, rs.Ui.Messages[0].Text, "The recovery flow expired")
+
+		addr, err := reg.IdentityPool().FindVerifiableAddressByValue(context.Background(), identity.VerifiableAddressTypeEmail, recoveryEmail)
+		assert.NoError(t, err)
+		assert.False(t, addr.Verified)
+		assert.Nil(t, addr.VerifiedAt)
+		assert.Equal(t, identity.VerifiableAddressStatusPending, addr.Status)
 	})
 }