Successfull execution of a recovery flow does not update the Verified state of the used email address

[dadrus]

Describe the bug

If kratos is configured to allow logins with verified addresses only (via the require_verified_address hook), a "login" is still possible via the recovery flow. The recovery flow does however not update the Verified state of the used email address to true, thus a regular login is still not possible.

Reproducing the bug

• Configure kratos self service flows to look like follows:

    login:
      ui_url: <url for login>
      after:
        password:
          hooks:
            - hook: require_verified_address

    registration:
      ui_url: <url for registration>
      after:
        default_browser_return_url: <url to inform the user, that email verification is pending>

    verification:
      enabled: true
      ui_url: <url for verification>
      after:
        default_browser_return_url: <url to inform the user, that email verification is done>

    recovery:
      enabled: true
      ui_url: <url for recovery>

When the user registers and does not click the link in the received email to verify the email address and tries to login, the login is rejected with an error message saying, the account is not yet active (which is expected). When the user now goes through the password recovery flow and clicks the recovery link in the received (password recovery) email, the user is able to change the password (as expected) and kratos issues a valid session, so the user is logged in (this is expected as well). If the user now logs out and tries to login regularly, he/she will again run into an error saying the account is not active (not expected), even the previously executed password recovery flow effectively did an email verification.

Expected behavior

The email used for recovery flow is marked as verified, so the user can login.

Additional notes

@aeneasr: actually this is my fault, as I had not this part of kratos in my mind while working on the PR for #1328. If

kratos/selfservice/strategy/link/strategy_recovery.go

Line 258 in 1ba6c4a

func (s *Strategy) recoveryIssueSession(w http.ResponseWriter, r *http.Request, f *recovery.Flow, recoveredID uuid.UUID) error {

is the proper function to fix it, a PR will follow soon. ;) Btw. where do I get the email used for recovery from?

[dadrus]

My naive bug fix would look like this:

func (s *Strategy) recoveryIssueSession(w http.ResponseWriter, r *http.Request, f *recovery.Flow, recoveryAddress *identity.RecoveryAddress) error {
	recovered, err := s.d.IdentityPool().GetIdentity(r.Context(), recoveryAddress.IdentityID)
	if err != nil {
		return s.HandleRecoveryError(w, r, f, nil, err)
	}

	var address *identity.VerifiableAddress
	for _, va := range recovered.VerifiableAddresses {
		if va.Value == recoveryAddress.Value {
			address = &va
			break
		}
	}

	if address != nil && !address.Verified { // can it be that the address is ni
		address.Verified = true
		address.VerifiedAt = sqlxx.NullTime(time.Now().UTC())
		address.Status = identity.VerifiableAddressStatusCompleted
		if err := s.d.PrivilegedIdentityPool().UpdateVerifiableAddress(r.Context(), address); err != nil {
			return s.HandleRecoveryError(w, r, f, nil, err)
		}
	}

       // the remaining part of the method as it is today

The changes would be

• method signature. Instead of passing the Id of the identity, the recovery address object is passed (as it is available to the method calling this one).
• looking for the verifiable address matching the recovery address
• if it is not yet verified, set it to verified.

[aeneasr]

It's definitely hacky I would say but the problem really isn't your hook, it's how recovery works at the moment (issuing a session). I think this workaround (please only execute it if recovery is actually enabled) is ok until we have #1451

[dadrus]

Ok. Thanks. However, if the recovery process will be changed to an OTP based one, the OTP will anyway be delivered through a specific channel (email, telephone) to the user. So if the user was able to receive and successfully use that value to finalize the recovery process, kratos should update the corresponding address to being verified as well. With other words, we have to take this issue into account regardless of how the recovery process is implemented. As long as it is using a separate channel for delivering tokens (everything else would be a security issue), that delivery channel should be marked as verified upon successful recovery finalization. So actually is doesn't matter, whether a session is issued or not. It is about whether the given channel was verified or not. And both, the regular email verification flow, as well as the recovery flow (whatever shape it is going to take in the future) do at the end a channel verification.

So to fix the behavior, currently implemented for recovery, do you see a better solution approach compared to the one sketched in my last comment?

[aeneasr]

Makes sense!