Implementing code-based email verification flow; issues _without_ verify_account_grace_period enabled

[moneill]

Hey there! I'm in the process of implementing code-based email verification following the discussion here: https://groups.google.com/g/rodauth/c/eBQem6q3Ne0/m/4jsJK7EZAwAJ. This is the rough account registration & verification flow I am trying to create:

1/ User creates a new account via /create-account and user is immediately redirected to /verify-account
2/ Verification email (containing 6 digit verification code) gets sent to user
3/ User enters the code from their email on /verify-account
4/ User is verified and redirected

With the verify_account_grace_period feature enabled, the above works as expected so long as the verification is completed within the grace period.

However, if the grace period has elapsed, or if verify_account_grace_period is disabled (my preference), the following exception occurs after a user enters their verification code:

undefined method `verification_key' for nil


Extracted source (around line #199):
              

    account_from_verify_account_key do |key|
      unless timing_safe_eql?(key, rails_account.verification_key.key)
        verify_account_key_error "Invalid verification key."
      end

Application Trace | Framework Trace | Full Trace
app/misc/rodauth_main.rb:199:in `block (2 levels) in <class:RodauthMain>'
app/misc/rodauth_app.rb:11:in `block in <class:RodauthApp>'

I have a barebones repo set up here to reproduce this: https://github.com/moneill/rodauth-debug (see rodauth_main.rb). My approach is a blend of https://groups.google.com/g/rodauth/c/eBQem6q3Ne0/m/4jsJK7EZAwAJ plus #249 (comment) for redirecting to the originally-requested page.

Any thoughts on what might I might be doing wrong? Appreciate any guidance folks might be able to offer!

[janko]

If you're not using verify_account_grace_period and have verify_account_autologin? false, then there will be no account logged in at the time of verification, so rails_account will return nil (that method returns the currently logged in account).

The following should probably work, though it's probably vulnerable to timing attacks:

account_from_verify_account_key do |code|
  verification_key = Account::VerificationKey.find_by(code: code)

  if verification_key.nil?
    verify_account_code_error "Invalid verification code."
  elsif verification_key.requested_at <= 24.hours.ago
    verify_account_code_error "This verification code has expired."
  end

  _account_from_id(verification_key.id)
end

[jeremyevans]

Certainly, a 6 digit code without token is a significant security vulnerability, unless you enforcing lockout after a small number of attempts. Otherwise, brute forcing the code is trivial.

[janko]

I was referring to the default Rodauth behavior with tokens (no 6-digit codes). When opening the verify account page from the email link, Rodauth will store the token in the session and redirect to the verify account page without the query param, to avoid leaking tokens via the Referer header if the user were to click on an external link.

However, that means if the user didn't finish verifying their account, they might not be aware that the verification token is remembered by the browser, an attacker could finish it for them on the same computer. Especially since the account verification token doesn't appear to have an expiration. Though the attacker could probably also look at the browser history as well.

[janko]

The 6-digit code in addition to the token sounds like it would patch this potential security vulnerability, because it's not enough for the attacker to retrieve the token, they would also have to know the contents of the email.

[jeremyevans]

I wouldn't consider the current behavior to be a vulnerability. Certainly using an additional 6-digit code would add an additional step, and therefore a small amount of security, but it also adds more work for the user, and for most applications I don't think the tradeoffs would be worth it.

Rodauth moves the token from the param to the session, which is the same place it stores the logged in user? So to say this is a vulnerability would be to say that how the logged in user is stored is also a vulnerability, correct? If an attacker gets access to a user's browser after they have logged in, you get the same issue, I think.

[janko]

For me it's not about the place where token is stored, but the expectations. When the user is logged in, they know they need to sign out if they're on a shared computer. But if they opened the verify account page and leave it, they might not expect that it was persisted somewhere. It's an implementation detail of Rodauth, e.g. Devise won't put anything in the session.

But a trivial browser history search would also uncover the unused verify account token, so I think I was just being paranoid 😄 Anyway, it's good to know that an additional code can easily be added to the account verification flow for people wanting extra security 👍🏻