Allow users to update their email address #1745
18
app/controllers/email_confirmations_controller.rb
Normal file
@@ -0,0 +1,18 @@
|
||||
class EmailConfirmationsController < ApplicationController
|
||||
|
|
||||
skip_before_action :set_request_details, only: :new
|
||||
skip_authentication only: :new
|
||||
|
||||
def new
|
||||
# Returns nil if the token is invalid OR expired
|
||||
@user = User.find_by_token_for(:email_confirmation, params[:token])
|
||||
|
||||
if @user&.unconfirmed_email && @user&.update(
|
||||
email: @user.unconfirmed_email,
|
||||
unconfirmed_email: nil
|
||||
)
|
||||
redirect_to new_session_path, notice: t(".success_login")
|
||||
else
|
||||
redirect_to root_path, alert: t(".invalid_token")
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -22,6 +22,10 @@ class Settings::HostingsController < SettingsController
|
||||
Setting.require_invite_for_signup = hosting_params[:require_invite_for_signup]
|
||||
end
|
||||
|
||||
if hosting_params.key?(:require_email_confirmation)
|
||||
Setting.require_email_confirmation = hosting_params[:require_email_confirmation]
|
||||
end
|
||||
|
||||
if hosting_params.key?(:synth_api_key)
|
||||
Setting.synth_api_key = hosting_params[:synth_api_key]
|
||||
end
|
||||
@@ -34,7 +38,7 @@ class Settings::HostingsController < SettingsController
|
||||
|
||||
private
|
||||
def hosting_params
|
||||
params.require(:setting).permit(:render_deploy_hook, :upgrades_setting, :require_invite_for_signup, :synth_api_key)
|
||||
params.require(:setting).permit(:render_deploy_hook, :upgrades_setting, :require_invite_for_signup, :require_email_confirmation, :synth_api_key)
|
||||
end
|
||||
|
||||
def raise_if_not_self_hosted
|
||||
|
||||
@@ -4,10 +4,23 @@ class UsersController < ApplicationController
|
||||
def update
|
||||
|
There won't be any errors populated on the user model here for most cases (blank alert), so here and below we should probably just pass a string alert like "Something went wrong" There won't be any errors populated on the user model here for most cases (blank alert), so here and below we should probably just pass a string alert like "Something went wrong"
|
||||
@user = Current.user
|
||||
|
||||
@user.update!(user_params.except(:redirect_to, :delete_profile_image))
|
||||
@user.profile_image.purge if should_purge_profile_image?
|
||||
if email_changed?
|
||||
if @user.initiate_email_change(user_params[:email])
|
||||
if Rails.application.config.app_mode.self_hosted? && !Setting.require_email_confirmation
|
||||
handle_redirect(t(".success"))
|
||||
else
|
||||
redirect_to settings_profile_path, notice: t(".email_change_initiated")
|
||||
end
|
||||
else
|
||||
error_message = @user.errors.any? ? @user.errors.full_messages.to_sentence : t(".email_change_failed")
|
||||
redirect_to settings_profile_path, alert: error_message
|
||||
end
|
||||
else
|
||||
@user.update!(user_params.except(:redirect_to, :delete_profile_image))
|
||||
@user.profile_image.purge if should_purge_profile_image?
|
||||
|
||||
handle_redirect(t(".success"))
|
||||
handle_redirect(t(".success"))
|
||||
end
|
||||
end
|
||||
|
||||
def destroy
|
||||
@@ -38,9 +51,13 @@ class UsersController < ApplicationController
|
||||
user_params[:profile_image].blank?
|
||||
end
|
||||
|
||||
def email_changed?
|
||||
user_params[:email].present? && user_params[:email] != @user.email
|
||||
end
|
||||
|
||||
def user_params
|
||||
params.require(:user).permit(
|
||||
:first_name, :last_name, :profile_image, :redirect_to, :delete_profile_image, :onboarded_at,
|
||||
:first_name, :last_name, :email, :profile_image, :redirect_to, :delete_profile_image, :onboarded_at,
|
||||
family_attributes: [ :name, :currency, :country, :locale, :date_format, :timezone, :id, :data_enrichment_enabled ]
|
||||
)
|
||||
end
|
||||
|
||||
2
app/helpers/email_confirmations_helper.rb
Normal file
@@ -0,0 +1,2 @@
|
||||
module EmailConfirmationsHelper
|
||||
end
|
||||
15
app/mailers/email_confirmation_mailer.rb
Normal file
@@ -0,0 +1,15 @@
|
||||
class EmailConfirmationMailer < ApplicationMailer
|
||||
# Subject can be set in your I18n file at config/locales/en.yml
|
||||
# with the following lookup:
|
||||
#
|
||||
# en.email_confirmation_mailer.confirmation_email.subject
|
||||
#
|
||||
def confirmation_email
|
||||
@user = params[:user]
|
||||
@subject = t(".subject")
|
||||
@cta = t(".cta")
|
||||
@confirmation_url = new_email_confirmation_url(token: @user.generate_token_for(:email_confirmation))
|
||||
|
||||
mail to: @user.unconfirmed_email, subject: @subject
|
||||
end
|
||||
end
|
||||
@@ -20,4 +20,6 @@ class Setting < RailsSettings::Base
|
||||
field :synth_api_key, type: :string, default: ENV["SYNTH_API_KEY"]
|
||||
|
||||
field :require_invite_for_signup, type: :boolean, default: false
|
||||
|
||||
field :require_email_confirmation, type: :boolean, default: ENV.fetch("REQUIRE_EMAIL_CONFIRMATION", "true") == "true"
|
||||
end
|
||||
|
||||
@@ -7,9 +7,10 @@ class User < ApplicationRecord
|
||||
has_many :impersonated_support_sessions, class_name: "ImpersonationSession", foreign_key: :impersonated_id, dependent: :destroy
|
||||
accepts_nested_attributes_for :family, update_only: true
|
||||
|
||||
validates :email, presence: true, uniqueness: true
|
||||
validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
|
||||
validate :ensure_valid_profile_image
|
||||
normalizes :email, with: ->(email) { email.strip.downcase }
|
||||
normalizes :unconfirmed_email, with: ->(email) { email&.strip&.downcase }
|
||||
|
||||
normalizes :first_name, :last_name, with: ->(value) { value.strip.presence }
|
||||
|
||||
@@ -25,6 +26,30 @@ class User < ApplicationRecord
|
||||
password_salt&.last(10)
|
||||
|
I think we can remove this method and handle directly in the controller. Then the controller would just be: I think we can remove this method and handle directly in the controller. Then the controller would just be:
```rb
def confirm
# Returns nil if the token is invalid OR expired
@user = User.find_by_token_for(:email_confirmation, params[:token])
if @user&.unconfirmed_email && @user&.update(email: @user.unconfirmed_email, unconfirmed_email: nil)
redirect_to root_path, notice: t(".success_login")
else
redirect_to root_path, alert: t(".invalid_token")
end
end
```
|
||||
end
|
||||
|
||||
generates_token_for :email_confirmation, expires_in: 1.day do
|
||||
unconfirmed_email
|
||||
end
|
||||
|
||||
def pending_email_change?
|
||||
unconfirmed_email.present?
|
||||
end
|
||||
|
||||
def initiate_email_change(new_email)
|
||||
return false if new_email == email
|
||||
return false if new_email == unconfirmed_email
|
||||
|
||||
if Rails.application.config.app_mode.self_hosted? && !Setting.require_email_confirmation
|
||||
update(email: new_email)
|
||||
else
|
||||
if update(unconfirmed_email: new_email)
|
||||
EmailConfirmationMailer.with(user: self).confirmation_email.deliver_later
|
||||
true
|
||||
else
|
||||
false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def request_impersonation_for(user_id)
|
||||
impersonated = User.find(user_id)
|
||||
impersonator_support_sessions.create!(impersonated: impersonated)
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
<h1><%= t(".greeting") %></h1>
|
||||
|
||||
<p><%= t(".body") %></p>
|
||||
|
||||
<%= link_to @cta, @confirmation_url, class: "button" %>
|
||||
|
||||
<p class="footer"><%= t(".expiry_notice", hours: 24) %></p>
|
||||
@@ -0,0 +1,9 @@
|
||||
EmailConfirmation#confirmation_email
|
||||
|
||||
<%= t(".greeting") %>
|
||||
|
||||
<%= t(".body") %>
|
||||
|
||||
<%= t(".cta") %>: <%= @confirmation_url %>
|
||||
|
||||
<%= t(".expiry_notice", hours: 24) %>
|
||||
@@ -13,6 +13,20 @@
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<div class="flex items-center justify-between">
|
||||
<div class="space-y-1">
|
||||
<p class="text-sm"><%= t(".email_confirmation_title") %></p>
|
||||
<p class="text-gray-500 text-sm"><%= t(".email_confirmation_description") %></p>
|
||||
</div>
|
||||
|
||||
<%= styled_form_with model: Setting.new, url: settings_hosting_path, method: :patch, data: { controller: "auto-submit-form", "auto-submit-form-trigger-event-value" => "blur" } do |form| %>
|
||||
<div class="relative inline-block select-none">
|
||||
<%= form.check_box :require_email_confirmation, class: "sr-only peer", "data-auto-submit-form-target": "auto", "data-autosubmit-trigger-event": "input", disabled: !Current.user.admin? %>
|
||||
<%= form.label :require_email_confirmation, " ".html_safe, class: "maybe-switch" %>
|
||||
</div>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<% if Setting.require_invite_for_signup %>
|
||||
<div class="flex items-center justify-between mb-4">
|
||||
<div>
|
||||
|
||||
@@ -5,10 +5,16 @@
|
||||
<h1 class="text-gray-900 text-xl font-medium mb-4"><%= t(".page_title") %></h1>
|
||||
<div class="space-y-4">
|
||||
<%= settings_section title: t(".profile_title"), subtitle: t(".profile_subtitle") do %>
|
||||
<%= styled_form_with model: @user, class: "space-y-4" do |form| %>
|
||||
<%= styled_form_with model: @user, url: user_path(@user), class: "space-y-4" do |form| %>
|
||||
<%= render "settings/user_avatar_field", form: form, user: @user %>
|
||||
|
||||
<div>
|
||||
<%= form.email_field :email, placeholder: t(".email"), label: t(".email") %>
|
||||
<% if @user.unconfirmed_email.present? %>
|
||||
|
From a UX standpoint, I think we need some sort of message underneath this when From a UX standpoint, I think we need some sort of message underneath this when `unconfirmed_email` is present:
```erb
<p>You have requested to change your email to <%= @user.unconfirmed_email %>. Please go to your email and confirm for the change to take effect.</p>
```
|
||||
<p class="mt-2 text-sm text-gray-600">
|
||||
You have requested to change your email to <%= @user.unconfirmed_email %>. Please go to your email and confirm for the change to take effect.
|
||||
</p>
|
||||
<% end %>
|
||||
<div class="grid grid-cols-2 gap-4 mt-4">
|
||||
<%= form.text_field :first_name, placeholder: t(".first_name"), label: t(".first_name") %>
|
||||
<%= form.text_field :last_name, placeholder: t(".last_name"), label: t(".last_name") %>
|
||||
|
||||
@@ -18,10 +18,14 @@ Rails.application.configure do
|
||||
config.eager_load = ENV["CI"].present?
|
||||
|
||||
# Configure public file server for tests with Cache-Control for performance.
|
||||
config.public_file_server.enabled = true
|
||||
config.public_file_server.headers = {
|
||||
"Cache-Control" => "public, max-age=#{1.hour.to_i}"
|
||||
}
|
||||
|
||||
# Set default sender email for tests
|
||||
ENV["EMAIL_SENDER"] = "hello@maybefinance.com"
|
||||
|
||||
# Show full error reports and disable caching.
|
||||
config.consider_all_requests_local = true
|
||||
config.action_controller.perform_caching = false
|
||||
@@ -69,4 +73,6 @@ Rails.application.configure do
|
||||
config.active_record.encryption.encrypt_fixtures = true
|
||||
|
||||
config.autoload_paths += %w[test/support]
|
||||
|
||||
config.action_mailer.default_url_options = { host: "example.com" }
|
||||
end
|
||||
|
||||
@@ -73,3 +73,7 @@ en:
|
||||
or entering manually.
|
||||
update:
|
||||
success: "%{type} account updated"
|
||||
email_confirmations:
|
||||
new:
|
||||
success_login: "Your email has been confirmed. Please log in with your new email address."
|
||||
invalid_token: "Invalid or expired confirmation link."
|
||||
|
||||
9
config/locales/views/email_confirmation_mailer/en.yml
Normal file
@@ -0,0 +1,9 @@
|
||||
---
|
||||
en:
|
||||
email_confirmation_mailer:
|
||||
confirmation_email:
|
||||
subject: "Maybe: Confirm your email change"
|
||||
greeting: "Hello!"
|
||||
body: "You recently requested to change your email address. Click the button below to confirm this change."
|
||||
cta: "Confirm email change"
|
||||
expiry_notice: "This link will expire in %{hours} hours."
|
||||
@@ -79,6 +79,7 @@ en:
|
||||
invitation_link: Invitation link
|
||||
invite_member: Add member
|
||||
last_name: Last Name
|
||||
email: Email
|
||||
page_title: Account
|
||||
pending: Pending
|
||||
profile_subtitle: Customize how you appear on Maybe
|
||||
@@ -87,3 +88,6 @@ en:
|
||||
user_avatar_field:
|
||||
accepted_formats: JPG or PNG. 5MB max.
|
||||
choose: Choose
|
||||
users:
|
||||
update:
|
||||
success: Profile updated successfully
|
||||
|
||||
@@ -7,7 +7,9 @@ en:
|
||||
so via an invite code
|
||||
generate_tokens: Generate new code
|
||||
generated_tokens: Generated codes
|
||||
title: Require invite code for new sign ups
|
||||
title: Require invite code for signup
|
||||
email_confirmation_title: Require email confirmation
|
||||
email_confirmation_description: When enabled, users must confirm their email address when changing it.
|
||||
provider_settings:
|
||||
description: Configure settings for your hosting provider
|
||||
render_deploy_hook_label: Render Deploy Hook URL
|
||||
|
||||
@@ -4,4 +4,6 @@ en:
|
||||
destroy:
|
||||
success: Your account has been deleted.
|
||||
update:
|
||||
success: Your profile has been updated.
|
||||
success: "Your profile has been updated."
|
||||
email_change_initiated: "Please check your new email address for confirmation instructions."
|
||||
email_change_failed: "Failed to change email address."
|
||||
|
||||
@@ -9,6 +9,7 @@ Rails.application.routes.draw do
|
||||
resources :sessions, only: %i[new create destroy]
|
||||
|
Totally semantic and optional, but for consistency with the other routes I think this is a great use case for something like: Would still use Totally semantic and optional, but for consistency with the other routes I think this is a great use case for something like:
```rb
resource :email_confirmation, only: :new
```
Would still use `email_confirmations_controller` and the new route would be `new_email_confirmation_url`
|
||||
resource :password_reset, only: %i[new create edit update]
|
||||
resource :password, only: %i[edit update]
|
||||
resource :email_confirmation, only: :new
|
||||
|
||||
resources :users, only: %i[update destroy]
|
||||
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
class AddEmailConfirmationToUsers < ActiveRecord::Migration[7.2]
|
||||
def change
|
||||
add_column :users, :unconfirmed_email, :string
|
||||
add_column :users, :email_confirmation_token, :string
|
||||
|
I totally missed this yesterday, but what is I totally missed this yesterday, but what is `:email_confirmation_token` for here? It doesn't seem to be used anywhere unless I'm missing something
Ah, this is leftover from before switching to the rails Ah, this is leftover from before switching to the rails `generate_token_for` stuff. Will remove.
|
||||
add_column :users, :email_confirmation_sent_at, :datetime
|
||||
|
We should be able to delete We should be able to delete `:email_confirmation_sent_at` and just let the `generates_token_for` expiry handle the expiration validation (see my comment above about removing the `confirm_email_change` method)
|
||||
|
||||
add_index :users, :email_confirmation_token, unique: true
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
class RemoveEmailConfirmationSentAtFromUsers < ActiveRecord::Migration[7.2]
|
||||
def change
|
||||
remove_column :users, :email_confirmation_sent_at, :datetime
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,6 @@
|
||||
class RemoveEmailConfirmationTokenFromUsers < ActiveRecord::Migration[7.2]
|
||||
def change
|
||||
remove_index :users, :email_confirmation_token
|
||||
remove_column :users, :email_confirmation_token, :string
|
||||
end
|
||||
end
|
||||
3
db/schema.rb
generated
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[7.2].define(version: 2025_01_28_203303) do
|
||||
ActiveRecord::Schema[7.2].define(version: 2025_01_31_171943) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pgcrypto"
|
||||
enable_extension "plpgsql"
|
||||
@@ -661,6 +661,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_28_203303) do
|
||||
t.string "role", default: "member", null: false
|
||||
t.boolean "active", default: true, null: false
|
||||
t.datetime "onboarded_at"
|
||||
t.string "unconfirmed_email"
|
||||
t.index ["email"], name: "index_users_on_email", unique: true
|
||||
t.index ["family_id"], name: "index_users_on_family_id"
|
||||
end
|
||||
|
||||
12
test/controllers/email_confirmations_controller_test.rb
Normal file
@@ -0,0 +1,12 @@
|
||||
require "test_helper"
|
||||
|
||||
class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest
|
||||
test "should get confirm" do
|
||||
user = users(:new_email)
|
||||
user.update!(unconfirmed_email: "new@example.com")
|
||||
token = user.generate_token_for(:email_confirmation)
|
||||
|
||||
get new_email_confirmation_path(token: token)
|
||||
assert_redirected_to new_session_path
|
||||
end
|
||||
end
|
||||
@@ -10,7 +10,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
|
||||
|
||||
test "transaction count represents filtered total" do
|
||||
family = families(:empty)
|
||||
sign_in family.users.first
|
||||
sign_in users(:empty)
|
||||
account = family.accounts.create! name: "Test", balance: 0, currency: "USD", accountable: Depository.new
|
||||
|
||||
3.times do
|
||||
@@ -32,7 +32,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
|
||||
|
||||
test "can paginate" do
|
||||
family = families(:empty)
|
||||
sign_in family.users.first
|
||||
sign_in users(:empty)
|
||||
account = family.accounts.create! name: "Test", balance: 0, currency: "USD", accountable: Depository.new
|
||||
|
||||
11.times do
|
||||
|
||||
11
test/fixtures/users.yml
vendored
@@ -30,4 +30,13 @@ family_member:
|
||||
last_name: Dylan
|
||||
email: jakobdylan@yahoo.com
|
||||
password_digest: <%= BCrypt::Password.create('password') %>
|
||||
onboarded_at: <%= 3.days.ago %>
|
||||
onboarded_at: <%= 3.days.ago %>
|
||||
|
||||
new_email:
|
||||
family: empty
|
||||
first_name: Test
|
||||
last_name: User
|
||||
email: user@example.com
|
||||
unconfirmed_email: new@example.com
|
||||
password_digest: <%= BCrypt::Password.create('password123') %>
|
||||
onboarded_at: <%= Time.current %>
|
||||
14
test/mailers/email_confirmation_mailer_test.rb
Normal file
@@ -0,0 +1,14 @@
|
||||
require "test_helper"
|
||||
|
||||
class EmailConfirmationMailerTest < ActionMailer::TestCase
|
||||
test "confirmation_email" do
|
||||
user = users(:new_email)
|
||||
user.unconfirmed_email = "new@example.com"
|
||||
|
||||
mail = EmailConfirmationMailer.with(user: user).confirmation_email
|
||||
assert_equal I18n.t("email_confirmation_mailer.confirmation_email.subject"), mail.subject
|
||||
assert_equal [ user.unconfirmed_email ], mail.to
|
||||
assert_equal [ "hello@maybefinance.com" ], mail.from
|
||||
assert_match "confirm", mail.body.encoded
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,7 @@
|
||||
# Preview all emails at http://localhost:3000/rails/mailers/email_confirmation_mailer
|
||||
class EmailConfirmationMailerPreview < ActionMailer::Preview
|
||||
# Preview this email at http://localhost:3000/rails/mailers/email_confirmation_mailer/confirmation_email
|
||||
def confirmation_email
|
||||
EmailConfirmationMailer.confirmation_email
|
||||
end
|
||||
end
|
||||
@@ -38,8 +38,8 @@ class UserTest < ActiveSupport::TestCase
|
||||
end
|
||||
|
||||
test "email address is normalized" do
|
||||
@user.update!(email: " User@ExAMPle.CoM ")
|
||||
assert_equal "user@example.com", @user.reload.email
|
||||
@user.update!(email: " UNIQUE-User@ExAMPle.CoM ")
|
||||
assert_equal "unique-user@example.com", @user.reload.email
|
||||
end
|
||||
|
||||
test "display name" do
|
||||
|
||||
synxsystem96@gmail.com
Since we're skipping auth, this should never be true so we can remove it