Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only overwrite user.email after the new email has been verified #116

Open
mediafinger opened this issue Sep 4, 2024 · 1 comment
Open

Comments

@mediafinger
Copy link

Only overwrite existing email after the new email has been verified

The version of the authentication-zero gem we used in our project, would directly overwrite the user's email, potentially locking the user out of their account in case of a typo, or when they lack access to that email address.

Find below my refactoring that will wait for the verification (the user receiving the verification email to their new address and clicking on the link) before replacing user.email. Hopefully it can serve as inspiration :)

Note: the example code has a JSONB field user.settings where it stores the new_email before it is verified. The new_email is then deleted from the settings field after it has been verified. It should be easy to adapt this to your own use case, e.g. a separate field new_email in the users table.
The code is also using current_user frequently instead of @user as it is rendering views (mostly) with Phlex.

app/controllers/identity/emails_controller.rb

def update
  current_user.settings["new_email"] = user_params.delete(:email)

  if current_user.update(password_challenge: user_params[:password_challenge])
    send_email_and_redirect_to_root
  else
    notice = current_user.errors.full_messages

    render Identity::Emails::EditView.new(user: current_user, notice:), status: :unprocessable_entity
  end
end

private

def send_email_and_redirect_to_root
  if current_user.settings["new_email"].present?
    resend_email_verification(new_email: current_user.settings["new_email"])
    redirect_to root_path, notice: "Your email has been changed"
  else
    redirect_to root_path
  end
end

def resend_email_verification(new_email:)
  UserMailer.with(user: current_user, new_email:).email_verification.deliver_later
end

app/mailers/user_mailer.rb

def email_verification
  @user = params[:user]
  @new_email = params[:new_email]

  @signed_id = @user.generate_token_for(:email_verification)

  @email = @new_email || @user.email

  mail to: @email, subject: "Verify your email"
end

app/views/user_mailer/email_verification.html.erb

<p>This is to confirm that <%= @email %> is the email you want to use on your account. If you ever lose your password, that's where we'll email a reset link.</p>

app/controllers/identity/email_verifications_controller.rb

def show
  @user.email = @user.settings.delete("new_email") if @user.settings["new_email"].present?

  @user.update!(verified: true)

  redirect_to root_path, notice: "Thank you for verifying your email address"
end
@johannesschobel
Copy link

i like the idea described in this issue, however, i am not a fan of storing the new_email in the settings JSONB column for various reasons:

  • if the user object already has a settings field, this may cause issues
  • i think, the new_email has nothing to do with settings (in a semantic way). settings should cover, for example, theme information for the application, default language, and so on.

maybe you could

  • create a PR to address this issue
  • refactor the code to use a new_email field (char) directly and change the business logic accordingly
  • provide tests

what do you think? I think this would be a valuable contribution to this project.
All the best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants