Skip to content

Commit

Permalink
Allow users to update their email address (#1745)
Browse files Browse the repository at this point in the history
* Change email address

* Email confirmation

* Email change test

* Lint

* Schema reset

* Set test email sender

* Select specific user fixture

* Refactor/cleanup

* Remove unused email_confirmation_token

* Current user would never be true

* Fix translation test failures
  • Loading branch information
Shpigford authored Jan 31, 2025
1 parent 46e86a9 commit 41873de
Show file tree
Hide file tree
Showing 28 changed files with 225 additions and 15 deletions.
18 changes: 18 additions & 0 deletions app/controllers/email_confirmations_controller.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 5 additions & 1 deletion app/controllers/settings/hostings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def update
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
Expand All @@ -34,7 +38,7 @@ def update

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
Expand Down
25 changes: 21 additions & 4 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,23 @@ class UsersController < ApplicationController
def update
@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
Expand Down Expand Up @@ -38,9 +51,13 @@ def should_purge_profile_image?
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
Expand Down
2 changes: 2 additions & 0 deletions app/helpers/email_confirmations_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module EmailConfirmationsHelper
end
15 changes: 15 additions & 0 deletions app/mailers/email_confirmation_mailer.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions app/models/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 26 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -25,6 +26,30 @@ class User < ApplicationRecord
password_salt&.last(10)
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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
EmailConfirmation#confirmation_email

<%= t(".greeting") %>

<%= t(".body") %>

<%= t(".cta") %>: <%= @confirmation_url %>

<%= t(".expiry_notice", hours: 24) %>
14 changes: 14 additions & 0 deletions app/views/settings/hostings/_invite_code_settings.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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, "&nbsp;".html_safe, class: "maybe-switch" %>
</div>
<% end %>
</div>

<% if Setting.require_invite_for_signup %>
<div class="flex items-center justify-between mb-4">
<div>
Expand Down
8 changes: 7 additions & 1 deletion app/views/settings/profiles/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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? %>
<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") %>
Expand Down
6 changes: 6 additions & 0 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@
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"] = "[email protected]"

# Show full error reports and disable caching.
config.consider_all_requests_local = true
config.action_controller.perform_caching = false
Expand Down Expand Up @@ -69,4 +73,6 @@
config.active_record.encryption.encrypt_fixtures = true

config.autoload_paths += %w[test/support]

config.action_mailer.default_url_options = { host: "example.com" }
end
4 changes: 4 additions & 0 deletions config/locales/views/accounts/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 9 additions & 0 deletions config/locales/views/email_confirmation_mailer/en.yml
Original file line number Diff line number Diff line change
@@ -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."
4 changes: 4 additions & 0 deletions config/locales/views/settings/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -87,3 +88,6 @@ en:
user_avatar_field:
accepted_formats: JPG or PNG. 5MB max.
choose: Choose
users:
update:
success: Profile updated successfully
4 changes: 3 additions & 1 deletion config/locales/views/settings/hostings/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion config/locales/views/users/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
resources :sessions, only: %i[new create destroy]
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]

Expand Down
9 changes: 9 additions & 0 deletions db/migrate/20250130191533_add_email_confirmation_to_users.rb
Original file line number Diff line number Diff line change
@@ -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
add_column :users, :email_confirmation_sent_at, :datetime

add_index :users, :email_confirmation_token, unique: true
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RemoveEmailConfirmationSentAtFromUsers < ActiveRecord::Migration[7.2]
def change
remove_column :users, :email_confirmation_sent_at, :datetime
end
end
Original file line number Diff line number Diff line change
@@ -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 changes: 2 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions test/controllers/email_confirmations_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
require "test_helper"

class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest
test "should get confirm" do
user = users(:new_email)
user.update!(unconfirmed_email: "[email protected]")
token = user.generate_token_for(:email_confirmation)

get new_email_confirmation_path(token: token)
assert_redirected_to new_session_path
end
end
4 changes: 2 additions & 2 deletions test/controllers/transactions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
11 changes: 10 additions & 1 deletion test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,13 @@ family_member:
last_name: Dylan
email: [email protected]
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: [email protected]
unconfirmed_email: [email protected]
password_digest: <%= BCrypt::Password.create('password123') %>
onboarded_at: <%= Time.current %>
14 changes: 14 additions & 0 deletions test/mailers/email_confirmation_mailer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
require "test_helper"

class EmailConfirmationMailerTest < ActionMailer::TestCase
test "confirmation_email" do
user = users(:new_email)
user.unconfirmed_email = "[email protected]"

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 [ "[email protected]" ], mail.from
assert_match "confirm", mail.body.encoded
end
end
Loading

0 comments on commit 41873de

Please sign in to comment.