From 24e0790c55920d28ce09c9a3f189d13c945fb6d1 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 13:27:31 -0600 Subject: [PATCH 01/11] Change email address --- .../email_confirmations_controller.rb | 17 +++++++ .../settings/hostings_controller.rb | 6 ++- app/controllers/users_controller.rb | 24 +++++++-- app/helpers/email_confirmations_helper.rb | 2 + app/mailers/email_confirmation_mailer.rb | 15 ++++++ app/models/setting.rb | 2 + app/models/user.rb | 47 ++++++++++++++++- .../confirmation_email.html.erb | 7 +++ .../confirmation_email.text.erb | 9 ++++ .../email_confirmations/confirm.html.erb | 4 ++ .../hostings/_invite_code_settings.html.erb | 14 ++++++ app/views/settings/profiles/show.html.erb | 3 +- .../views/email_confirmation_mailer/en.yml | 9 ++++ .../locales/views/email_confirmations/en.yml | 7 +++ config/locales/views/settings/en.yml | 4 ++ config/locales/views/settings/hostings/en.yml | 4 +- config/locales/views/users/en.yml | 3 +- config/routes.rb | 3 ++ ...0191533_add_email_confirmation_to_users.rb | 9 ++++ db/schema.rb | 50 ++++++++++++++++++- .../email_confirmations_controller_test.rb | 8 +++ .../mailers/email_confirmation_mailer_test.rb | 11 ++++ .../email_confirmation_mailer_preview.rb | 7 +++ 23 files changed, 254 insertions(+), 11 deletions(-) create mode 100644 app/controllers/email_confirmations_controller.rb create mode 100644 app/helpers/email_confirmations_helper.rb create mode 100644 app/mailers/email_confirmation_mailer.rb create mode 100644 app/views/email_confirmation_mailer/confirmation_email.html.erb create mode 100644 app/views/email_confirmation_mailer/confirmation_email.text.erb create mode 100644 app/views/email_confirmations/confirm.html.erb create mode 100644 config/locales/views/email_confirmation_mailer/en.yml create mode 100644 config/locales/views/email_confirmations/en.yml create mode 100644 db/migrate/20250130191533_add_email_confirmation_to_users.rb create mode 100644 test/controllers/email_confirmations_controller_test.rb create mode 100644 test/mailers/email_confirmation_mailer_test.rb create mode 100644 test/mailers/previews/email_confirmation_mailer_preview.rb diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb new file mode 100644 index 00000000..c3e009df --- /dev/null +++ b/app/controllers/email_confirmations_controller.rb @@ -0,0 +1,17 @@ +class EmailConfirmationsController < ApplicationController + skip_authentication only: :confirm + + def confirm + @user = User.find_by_token_for(:email_confirmation, params[:token]) + + if @user&.confirm_email_change(params[:token]) + if Current.user == @user + redirect_to settings_profile_path, notice: t(".success") + else + redirect_to new_session_path, notice: t(".success_login") + end + else + redirect_to root_path, alert: t(".invalid_token") + end + end +end diff --git a/app/controllers/settings/hostings_controller.rb b/app/controllers/settings/hostings_controller.rb index 222ae018..aae6513c 100644 --- a/app/controllers/settings/hostings_controller.rb +++ b/app/controllers/settings/hostings_controller.rb @@ -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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 55b75581..c2dca295 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -4,10 +4,22 @@ 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 + redirect_to settings_profile_path, alert: @user.errors.full_messages.to_sentence + 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 +50,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 diff --git a/app/helpers/email_confirmations_helper.rb b/app/helpers/email_confirmations_helper.rb new file mode 100644 index 00000000..c5f58449 --- /dev/null +++ b/app/helpers/email_confirmations_helper.rb @@ -0,0 +1,2 @@ +module EmailConfirmationsHelper +end diff --git a/app/mailers/email_confirmation_mailer.rb b/app/mailers/email_confirmation_mailer.rb new file mode 100644 index 00000000..1cca9503 --- /dev/null +++ b/app/mailers/email_confirmation_mailer.rb @@ -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 = confirm_email_url(@user.email_confirmation_token) + + mail to: @user.unconfirmed_email, subject: @subject + end +end diff --git a/app/models/setting.rb b/app/models/setting.rb index d576fbea..41355fee 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 0d50ba87..afebc6d4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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,50 @@ 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, + email_confirmation_token: generate_token_for(:email_confirmation), + email_confirmation_sent_at: Time.current + ) + EmailConfirmationMailer.with(user: self).confirmation_email.deliver_later + true + else + false + end + end + end + + def confirm_email_change(token) + return false if token.blank? || token != email_confirmation_token + return false if email_confirmation_sent_at < 1.day.ago + + if update( + email: unconfirmed_email, + unconfirmed_email: nil, + email_confirmation_token: nil, + email_confirmation_sent_at: nil + ) + true + else + false + end + end + def request_impersonation_for(user_id) impersonated = User.find(user_id) impersonator_support_sessions.create!(impersonated: impersonated) diff --git a/app/views/email_confirmation_mailer/confirmation_email.html.erb b/app/views/email_confirmation_mailer/confirmation_email.html.erb new file mode 100644 index 00000000..be87ccd0 --- /dev/null +++ b/app/views/email_confirmation_mailer/confirmation_email.html.erb @@ -0,0 +1,7 @@ +

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

+ +

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

+ +<%= link_to @cta, @confirmation_url, class: "button" %> + +

<%= t(".expiry_notice", hours: 24) %>

\ No newline at end of file diff --git a/app/views/email_confirmation_mailer/confirmation_email.text.erb b/app/views/email_confirmation_mailer/confirmation_email.text.erb new file mode 100644 index 00000000..8f5fa14b --- /dev/null +++ b/app/views/email_confirmation_mailer/confirmation_email.text.erb @@ -0,0 +1,9 @@ +EmailConfirmation#confirmation_email + +<%= t(".greeting") %> + +<%= t(".body") %> + +<%= t(".cta") %>: <%= @confirmation_url %> + +<%= t(".expiry_notice", hours: 24) %> \ No newline at end of file diff --git a/app/views/email_confirmations/confirm.html.erb b/app/views/email_confirmations/confirm.html.erb new file mode 100644 index 00000000..fc5cd0f1 --- /dev/null +++ b/app/views/email_confirmations/confirm.html.erb @@ -0,0 +1,4 @@ +
+

EmailConfirmations#confirm

+

Find me in app/views/email_confirmations/confirm.html.erb

+
diff --git a/app/views/settings/hostings/_invite_code_settings.html.erb b/app/views/settings/hostings/_invite_code_settings.html.erb index e9889d75..de3b1e22 100644 --- a/app/views/settings/hostings/_invite_code_settings.html.erb +++ b/app/views/settings/hostings/_invite_code_settings.html.erb @@ -13,6 +13,20 @@ <% end %> +
+
+

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

+

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

+
+ + <%= 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| %> +
+ <%= 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" %> +
+ <% end %> +
+ <% if Setting.require_invite_for_signup %>
diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index c21775d6..a81be67b 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -5,10 +5,11 @@

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

<%= 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 %>
+ <%= form.email_field :email, placeholder: t(".email"), label: t(".email") %>
<%= 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") %> diff --git a/config/locales/views/email_confirmation_mailer/en.yml b/config/locales/views/email_confirmation_mailer/en.yml new file mode 100644 index 00000000..110746a8 --- /dev/null +++ b/config/locales/views/email_confirmation_mailer/en.yml @@ -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." \ No newline at end of file diff --git a/config/locales/views/email_confirmations/en.yml b/config/locales/views/email_confirmations/en.yml new file mode 100644 index 00000000..e9336a3b --- /dev/null +++ b/config/locales/views/email_confirmations/en.yml @@ -0,0 +1,7 @@ +--- +en: + email_confirmations: + confirm: + success: "Your email has been successfully updated." + success_login: "Your email has been confirmed. Please log in with your new email address." + invalid_token: "Invalid or expired confirmation link." \ No newline at end of file diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index f784db97..1c87a005 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -66,6 +66,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 @@ -74,3 +75,6 @@ en: user_avatar_field: accepted_formats: JPG or PNG. 5MB max. choose: Choose + users: + update: + success: Profile updated successfully diff --git a/config/locales/views/settings/hostings/en.yml b/config/locales/views/settings/hostings/en.yml index 90a89fd7..06334a31 100644 --- a/config/locales/views/settings/hostings/en.yml +++ b/config/locales/views/settings/hostings/en.yml @@ -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 diff --git a/config/locales/views/users/en.yml b/config/locales/views/users/en.yml index 6b488278..47e9b15e 100644 --- a/config/locales/views/users/en.yml +++ b/config/locales/views/users/en.yml @@ -4,4 +4,5 @@ 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." diff --git a/config/routes.rb b/config/routes.rb index d49ca187..e58669fc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,6 +9,9 @@ Rails.application.routes.draw do resources :sessions, only: %i[new create destroy] resource :password_reset, only: %i[new create edit update] resource :password, only: %i[edit update] + resources :email_confirmations, only: [] do + get :confirm, on: :collection + end resources :users, only: %i[update destroy] diff --git a/db/migrate/20250130191533_add_email_confirmation_to_users.rb b/db/migrate/20250130191533_add_email_confirmation_to_users.rb new file mode 100644 index 00000000..ac929b48 --- /dev/null +++ b/db/migrate/20250130191533_add_email_confirmation_to_users.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 5b0a0697..68bf7aa2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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_24_224316) do +ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -102,7 +102,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do t.decimal "balance", precision: 19, scale: 4 t.string "currency" t.boolean "is_active", default: true, null: false - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" t.boolean "scheduled_for_deletion", default: false @@ -197,6 +197,15 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do t.index ["family_id"], name: "index_categories_on_family_id" end + create_table "chats", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "user_id", null: false + t.string "title" + t.text "summary" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_chats_on_user_id" + end + create_table "credit_cards", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -479,6 +488,33 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do t.index ["family_id"], name: "index_merchants_on_family_id" end + create_table "messages", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "chat_id", null: false + t.uuid "user_id" + t.text "content" + t.text "log" + t.string "role" + t.string "status", default: "pending" + t.boolean "hidden", default: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["chat_id"], name: "index_messages_on_chat_id" + t.index ["user_id"], name: "index_messages_on_user_id" + end + + create_table "metrics", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "family_id", null: false + t.uuid "account_id" + t.string "kind", null: false + t.string "subkind" + t.date "date", null: false + t.decimal "value", precision: 10, scale: 2, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id"], name: "index_metrics_on_account_id" + t.index ["family_id"], name: "index_metrics_on_family_id" + end + create_table "other_assets", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -516,6 +552,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do t.string "available_products", default: [], array: true t.string "billed_products", default: [], array: true t.datetime "last_synced_at" + t.string "plaid_region", default: "us", null: false t.index ["family_id"], name: "index_plaid_items_on_family_id" end @@ -660,7 +697,11 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do t.string "role", default: "member", null: false t.boolean "active", default: true, null: false t.datetime "onboarded_at" + t.string "unconfirmed_email" + t.string "email_confirmation_token" + t.datetime "email_confirmation_sent_at" t.index ["email"], name: "index_users_on_email", unique: true + t.index ["email_confirmation_token"], name: "index_users_on_email_confirmation_token", unique: true t.index ["family_id"], name: "index_users_on_family_id" end @@ -691,6 +732,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do add_foreign_key "budget_categories", "categories" add_foreign_key "budgets", "families" add_foreign_key "categories", "families" + add_foreign_key "chats", "users" add_foreign_key "impersonation_session_logs", "impersonation_sessions" add_foreign_key "impersonation_sessions", "users", column: "impersonated_id" add_foreign_key "impersonation_sessions", "users", column: "impersonator_id" @@ -699,6 +741,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_24_224316) do add_foreign_key "invitations", "families" add_foreign_key "invitations", "users", column: "inviter_id" add_foreign_key "merchants", "families" + add_foreign_key "messages", "chats" + add_foreign_key "messages", "users" + add_foreign_key "metrics", "accounts" + add_foreign_key "metrics", "families" add_foreign_key "plaid_accounts", "plaid_items" add_foreign_key "plaid_items", "families" add_foreign_key "rejected_transfers", "account_transactions", column: "inflow_transaction_id" diff --git a/test/controllers/email_confirmations_controller_test.rb b/test/controllers/email_confirmations_controller_test.rb new file mode 100644 index 00000000..72ab66ad --- /dev/null +++ b/test/controllers/email_confirmations_controller_test.rb @@ -0,0 +1,8 @@ +require "test_helper" + +class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest + test "should get confirm" do + get email_confirmations_confirm_url + assert_response :success + end +end diff --git a/test/mailers/email_confirmation_mailer_test.rb b/test/mailers/email_confirmation_mailer_test.rb new file mode 100644 index 00000000..69a1b8e4 --- /dev/null +++ b/test/mailers/email_confirmation_mailer_test.rb @@ -0,0 +1,11 @@ +require "test_helper" + +class EmailConfirmationMailerTest < ActionMailer::TestCase + test "confirmation_email" do + mail = EmailConfirmationMailer.confirmation_email + assert_equal "Confirmation email", mail.subject + assert_equal [ "to@example.org" ], mail.to + assert_equal [ "from@example.com" ], mail.from + assert_match "Hi", mail.body.encoded + end +end diff --git a/test/mailers/previews/email_confirmation_mailer_preview.rb b/test/mailers/previews/email_confirmation_mailer_preview.rb new file mode 100644 index 00000000..36aa4648 --- /dev/null +++ b/test/mailers/previews/email_confirmation_mailer_preview.rb @@ -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 -- 2.53.0 From c3ea407613472278a6949d0296b6d319d8865590 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 13:51:05 -0600 Subject: [PATCH 02/11] Email confirmation --- app/controllers/email_confirmations_controller.rb | 3 ++- app/mailers/email_confirmation_mailer.rb | 2 +- app/models/user.rb | 3 --- config/routes.rb | 4 +++- db/schema.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb index c3e009df..8d0f848a 100644 --- a/app/controllers/email_confirmations_controller.rb +++ b/app/controllers/email_confirmations_controller.rb @@ -1,10 +1,11 @@ class EmailConfirmationsController < ApplicationController + skip_before_action :set_request_details, only: :confirm skip_authentication only: :confirm def confirm @user = User.find_by_token_for(:email_confirmation, params[:token]) - if @user&.confirm_email_change(params[:token]) + if @user&.confirm_email_change(@user.email_confirmation_token) if Current.user == @user redirect_to settings_profile_path, notice: t(".success") else diff --git a/app/mailers/email_confirmation_mailer.rb b/app/mailers/email_confirmation_mailer.rb index 1cca9503..e6e8cbc6 100644 --- a/app/mailers/email_confirmation_mailer.rb +++ b/app/mailers/email_confirmation_mailer.rb @@ -8,7 +8,7 @@ class EmailConfirmationMailer < ApplicationMailer @user = params[:user] @subject = t(".subject") @cta = t(".cta") - @confirmation_url = confirm_email_url(@user.email_confirmation_token) + @confirmation_url = confirm_email_email_confirmations_url(token: @user.generate_token_for(:email_confirmation)) mail to: @user.unconfirmed_email, subject: @subject end diff --git a/app/models/user.rb b/app/models/user.rb index afebc6d4..2d315ec4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -43,7 +43,6 @@ class User < ApplicationRecord else if update( unconfirmed_email: new_email, - email_confirmation_token: generate_token_for(:email_confirmation), email_confirmation_sent_at: Time.current ) EmailConfirmationMailer.with(user: self).confirmation_email.deliver_later @@ -55,13 +54,11 @@ class User < ApplicationRecord end def confirm_email_change(token) - return false if token.blank? || token != email_confirmation_token return false if email_confirmation_sent_at < 1.day.ago if update( email: unconfirmed_email, unconfirmed_email: nil, - email_confirmation_token: nil, email_confirmation_sent_at: nil ) true diff --git a/config/routes.rb b/config/routes.rb index 50f627d0..38229989 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -10,7 +10,9 @@ Rails.application.routes.draw do resource :password_reset, only: %i[new create edit update] resource :password, only: %i[edit update] resources :email_confirmations, only: [] do - get :confirm, on: :collection + collection do + get :confirm, as: :confirm_email + end end resources :users, only: %i[update destroy] diff --git a/db/schema.rb b/db/schema.rb index 0e3759d0..6d1171b2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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_30_191533) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" -- 2.53.0 From 6771c717609374b03098b0a05b322a69aeddce06 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 13:57:57 -0600 Subject: [PATCH 03/11] Email change test --- config/environments/test.rb | 2 ++ .../email_confirmations_controller_test.rb | 8 ++++++-- test/fixtures/users.yml | 11 ++++++++++- test/mailers/email_confirmation_mailer_test.rb | 13 ++++++++----- test/models/user_test.rb | 4 ++-- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index 37637b90..7b7c9f0c 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -69,4 +69,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 diff --git a/test/controllers/email_confirmations_controller_test.rb b/test/controllers/email_confirmations_controller_test.rb index 72ab66ad..d52c7aec 100644 --- a/test/controllers/email_confirmations_controller_test.rb +++ b/test/controllers/email_confirmations_controller_test.rb @@ -2,7 +2,11 @@ require "test_helper" class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest test "should get confirm" do - get email_confirmations_confirm_url - assert_response :success + user = users(:new_email) + user.update!(unconfirmed_email: "new@example.com", email_confirmation_sent_at: Time.current) + token = user.generate_token_for(:email_confirmation) + + get confirm_email_email_confirmations_path(token: token) + assert_redirected_to new_session_path end end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index d7e7444d..246fdf56 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -30,4 +30,13 @@ family_member: last_name: Dylan email: jakobdylan@yahoo.com password_digest: <%= BCrypt::Password.create('password') %> - onboarded_at: <%= 3.days.ago %> \ No newline at end of file + 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 %> \ No newline at end of file diff --git a/test/mailers/email_confirmation_mailer_test.rb b/test/mailers/email_confirmation_mailer_test.rb index 69a1b8e4..14832039 100644 --- a/test/mailers/email_confirmation_mailer_test.rb +++ b/test/mailers/email_confirmation_mailer_test.rb @@ -2,10 +2,13 @@ require "test_helper" class EmailConfirmationMailerTest < ActionMailer::TestCase test "confirmation_email" do - mail = EmailConfirmationMailer.confirmation_email - assert_equal "Confirmation email", mail.subject - assert_equal [ "to@example.org" ], mail.to - assert_equal [ "from@example.com" ], mail.from - assert_match "Hi", mail.body.encoded + 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 diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 582fc2e8..dde2c44e 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -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 -- 2.53.0 From ddd2b4d37e76366e4b9e23a6033f65f69c86d9ff Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 13:59:58 -0600 Subject: [PATCH 04/11] Lint --- app/models/user.rb | 2 +- test/controllers/email_confirmations_controller_test.rb | 2 +- test/mailers/email_confirmation_mailer_test.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 2d315ec4..92f76018 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -37,7 +37,7 @@ class User < ApplicationRecord 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 diff --git a/test/controllers/email_confirmations_controller_test.rb b/test/controllers/email_confirmations_controller_test.rb index d52c7aec..fef18fd7 100644 --- a/test/controllers/email_confirmations_controller_test.rb +++ b/test/controllers/email_confirmations_controller_test.rb @@ -5,7 +5,7 @@ class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest user = users(:new_email) user.update!(unconfirmed_email: "new@example.com", email_confirmation_sent_at: Time.current) token = user.generate_token_for(:email_confirmation) - + get confirm_email_email_confirmations_path(token: token) assert_redirected_to new_session_path end diff --git a/test/mailers/email_confirmation_mailer_test.rb b/test/mailers/email_confirmation_mailer_test.rb index 14832039..2727cfb9 100644 --- a/test/mailers/email_confirmation_mailer_test.rb +++ b/test/mailers/email_confirmation_mailer_test.rb @@ -4,11 +4,11 @@ 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_equal [ user.unconfirmed_email ], mail.to + assert_equal [ "hello@maybefinance.com" ], mail.from assert_match "confirm", mail.body.encoded end end -- 2.53.0 From e02da5881e7c9a0e55cfb06043948d974c798b4e Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 14:29:11 -0600 Subject: [PATCH 05/11] Schema reset --- db/schema.rb | 44 +------------------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 6d1171b2..efcbe9ec 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -102,7 +102,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do t.decimal "balance", precision: 19, scale: 4 t.string "currency" t.boolean "is_active", default: true, null: false - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" t.boolean "scheduled_for_deletion", default: false @@ -197,15 +197,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do t.index ["family_id"], name: "index_categories_on_family_id" end - create_table "chats", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "user_id", null: false - t.string "title" - t.text "summary" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["user_id"], name: "index_chats_on_user_id" - end - create_table "credit_cards", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -488,33 +479,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do t.index ["family_id"], name: "index_merchants_on_family_id" end - create_table "messages", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "chat_id", null: false - t.uuid "user_id" - t.text "content" - t.text "log" - t.string "role" - t.string "status", default: "pending" - t.boolean "hidden", default: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["chat_id"], name: "index_messages_on_chat_id" - t.index ["user_id"], name: "index_messages_on_user_id" - end - - create_table "metrics", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "family_id", null: false - t.uuid "account_id" - t.string "kind", null: false - t.string "subkind" - t.date "date", null: false - t.decimal "value", precision: 10, scale: 2, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["account_id"], name: "index_metrics_on_account_id" - t.index ["family_id"], name: "index_metrics_on_family_id" - end - create_table "other_assets", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -552,7 +516,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do t.string "available_products", default: [], array: true t.string "billed_products", default: [], array: true t.datetime "last_synced_at" - t.string "plaid_region", default: "us", null: false t.index ["family_id"], name: "index_plaid_items_on_family_id" end @@ -733,7 +696,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do add_foreign_key "budget_categories", "categories" add_foreign_key "budgets", "families" add_foreign_key "categories", "families" - add_foreign_key "chats", "users" add_foreign_key "impersonation_session_logs", "impersonation_sessions" add_foreign_key "impersonation_sessions", "users", column: "impersonated_id" add_foreign_key "impersonation_sessions", "users", column: "impersonator_id" @@ -742,10 +704,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do add_foreign_key "invitations", "families" add_foreign_key "invitations", "users", column: "inviter_id" add_foreign_key "merchants", "families" - add_foreign_key "messages", "chats" - add_foreign_key "messages", "users" - add_foreign_key "metrics", "accounts" - add_foreign_key "metrics", "families" add_foreign_key "plaid_accounts", "plaid_items" add_foreign_key "plaid_items", "families" add_foreign_key "rejected_transfers", "account_transactions", column: "inflow_transaction_id" -- 2.53.0 From 558ce6424fcd9f6ecb1e47f5e9c42c7331c4c81c Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 14:48:22 -0600 Subject: [PATCH 06/11] Set test email sender --- config/environments/test.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index 7b7c9f0c..23bd03f3 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -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 -- 2.53.0 From 1e621b4cd0724fd1b1810aa3fb4d6d40e14dd850 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Thu, 30 Jan 2025 15:00:16 -0600 Subject: [PATCH 07/11] Select specific user fixture --- test/controllers/transactions_controller_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index e4920819..73492e6e 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -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 -- 2.53.0 From 242bba2f22337d59ed6d48511cf0265c27e7e797 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 31 Jan 2025 10:22:19 -0600 Subject: [PATCH 08/11] Refactor/cleanup --- .../email_confirmations_controller.rb | 12 ++++++++---- app/controllers/users_controller.rb | 3 ++- app/mailers/email_confirmation_mailer.rb | 2 +- app/models/user.rb | 19 +------------------ .../email_confirmations/confirm.html.erb | 4 ---- app/views/settings/profiles/show.html.erb | 5 +++++ .../locales/views/email_confirmations/en.yml | 2 +- config/locales/views/users/en.yml | 1 + config/routes.rb | 6 +----- ...e_email_confirmation_sent_at_from_users.rb | 5 +++++ db/schema.rb | 3 +-- .../email_confirmations_controller_test.rb | 4 ++-- 12 files changed, 28 insertions(+), 38 deletions(-) delete mode 100644 app/views/email_confirmations/confirm.html.erb create mode 100644 db/migrate/20250130214500_remove_email_confirmation_sent_at_from_users.rb diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb index 8d0f848a..9c919134 100644 --- a/app/controllers/email_confirmations_controller.rb +++ b/app/controllers/email_confirmations_controller.rb @@ -1,11 +1,15 @@ class EmailConfirmationsController < ApplicationController - skip_before_action :set_request_details, only: :confirm - skip_authentication only: :confirm + skip_before_action :set_request_details, only: :new + skip_authentication only: :new - def confirm + def new + # Returns nil if the token is invalid OR expired @user = User.find_by_token_for(:email_confirmation, params[:token]) - if @user&.confirm_email_change(@user.email_confirmation_token) + if @user&.unconfirmed_email && @user&.update( + email: @user.unconfirmed_email, + unconfirmed_email: nil + ) if Current.user == @user redirect_to settings_profile_path, notice: t(".success") else diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index c2dca295..5bb4f45f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -12,7 +12,8 @@ class UsersController < ApplicationController redirect_to settings_profile_path, notice: t(".email_change_initiated") end else - redirect_to settings_profile_path, alert: @user.errors.full_messages.to_sentence + 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)) diff --git a/app/mailers/email_confirmation_mailer.rb b/app/mailers/email_confirmation_mailer.rb index e6e8cbc6..3ad99b96 100644 --- a/app/mailers/email_confirmation_mailer.rb +++ b/app/mailers/email_confirmation_mailer.rb @@ -8,7 +8,7 @@ class EmailConfirmationMailer < ApplicationMailer @user = params[:user] @subject = t(".subject") @cta = t(".cta") - @confirmation_url = confirm_email_email_confirmations_url(token: @user.generate_token_for(:email_confirmation)) + @confirmation_url = new_email_confirmation_url(token: @user.generate_token_for(:email_confirmation)) mail to: @user.unconfirmed_email, subject: @subject end diff --git a/app/models/user.rb b/app/models/user.rb index 92f76018..92171040 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -41,10 +41,7 @@ class User < ApplicationRecord if Rails.application.config.app_mode.self_hosted? && !Setting.require_email_confirmation update(email: new_email) else - if update( - unconfirmed_email: new_email, - email_confirmation_sent_at: Time.current - ) + if update(unconfirmed_email: new_email) EmailConfirmationMailer.with(user: self).confirmation_email.deliver_later true else @@ -53,20 +50,6 @@ class User < ApplicationRecord end end - def confirm_email_change(token) - return false if email_confirmation_sent_at < 1.day.ago - - if update( - email: unconfirmed_email, - unconfirmed_email: nil, - email_confirmation_sent_at: nil - ) - true - else - false - end - end - def request_impersonation_for(user_id) impersonated = User.find(user_id) impersonator_support_sessions.create!(impersonated: impersonated) diff --git a/app/views/email_confirmations/confirm.html.erb b/app/views/email_confirmations/confirm.html.erb deleted file mode 100644 index fc5cd0f1..00000000 --- a/app/views/email_confirmations/confirm.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -
-

EmailConfirmations#confirm

-

Find me in app/views/email_confirmations/confirm.html.erb

-
diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index a447f6f6..d0ffb6da 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -10,6 +10,11 @@
<%= form.email_field :email, placeholder: t(".email"), label: t(".email") %> + <% if @user.unconfirmed_email.present? %> +

+ You have requested to change your email to <%= @user.unconfirmed_email %>. Please go to your email and confirm for the change to take effect. +

+ <% end %>
<%= 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") %> diff --git a/config/locales/views/email_confirmations/en.yml b/config/locales/views/email_confirmations/en.yml index e9336a3b..c4963d5a 100644 --- a/config/locales/views/email_confirmations/en.yml +++ b/config/locales/views/email_confirmations/en.yml @@ -1,7 +1,7 @@ --- en: email_confirmations: - confirm: + new: success: "Your email has been successfully updated." success_login: "Your email has been confirmed. Please log in with your new email address." invalid_token: "Invalid or expired confirmation link." \ No newline at end of file diff --git a/config/locales/views/users/en.yml b/config/locales/views/users/en.yml index 47e9b15e..5eaf6f63 100644 --- a/config/locales/views/users/en.yml +++ b/config/locales/views/users/en.yml @@ -6,3 +6,4 @@ en: update: 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." diff --git a/config/routes.rb b/config/routes.rb index 38229989..33d534a2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,11 +9,7 @@ Rails.application.routes.draw do resources :sessions, only: %i[new create destroy] resource :password_reset, only: %i[new create edit update] resource :password, only: %i[edit update] - resources :email_confirmations, only: [] do - collection do - get :confirm, as: :confirm_email - end - end + resource :email_confirmation, only: :new resources :users, only: %i[update destroy] diff --git a/db/migrate/20250130214500_remove_email_confirmation_sent_at_from_users.rb b/db/migrate/20250130214500_remove_email_confirmation_sent_at_from_users.rb new file mode 100644 index 00000000..89533213 --- /dev/null +++ b/db/migrate/20250130214500_remove_email_confirmation_sent_at_from_users.rb @@ -0,0 +1,5 @@ +class RemoveEmailConfirmationSentAtFromUsers < ActiveRecord::Migration[7.2] + def change + remove_column :users, :email_confirmation_sent_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index efcbe9ec..25dd2beb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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_30_191533) do +ActiveRecord::Schema[7.2].define(version: 2025_01_30_214500) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -663,7 +663,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_191533) do t.datetime "onboarded_at" t.string "unconfirmed_email" t.string "email_confirmation_token" - t.datetime "email_confirmation_sent_at" t.index ["email"], name: "index_users_on_email", unique: true t.index ["email_confirmation_token"], name: "index_users_on_email_confirmation_token", unique: true t.index ["family_id"], name: "index_users_on_family_id" diff --git a/test/controllers/email_confirmations_controller_test.rb b/test/controllers/email_confirmations_controller_test.rb index fef18fd7..beee4f73 100644 --- a/test/controllers/email_confirmations_controller_test.rb +++ b/test/controllers/email_confirmations_controller_test.rb @@ -3,10 +3,10 @@ require "test_helper" class EmailConfirmationsControllerTest < ActionDispatch::IntegrationTest test "should get confirm" do user = users(:new_email) - user.update!(unconfirmed_email: "new@example.com", email_confirmation_sent_at: Time.current) + user.update!(unconfirmed_email: "new@example.com") token = user.generate_token_for(:email_confirmation) - get confirm_email_email_confirmations_path(token: token) + get new_email_confirmation_path(token: token) assert_redirected_to new_session_path end end -- 2.53.0 From 7352b658a37427d278ef45009d7b2c2298f03225 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 31 Jan 2025 11:22:00 -0600 Subject: [PATCH 09/11] Remove unused email_confirmation_token --- ...0131171943_remove_email_confirmation_token_from_users.rb | 6 ++++++ db/schema.rb | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20250131171943_remove_email_confirmation_token_from_users.rb diff --git a/db/migrate/20250131171943_remove_email_confirmation_token_from_users.rb b/db/migrate/20250131171943_remove_email_confirmation_token_from_users.rb new file mode 100644 index 00000000..db1db8b8 --- /dev/null +++ b/db/migrate/20250131171943_remove_email_confirmation_token_from_users.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 25dd2beb..8efce7d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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_30_214500) 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" @@ -662,9 +662,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_30_214500) do t.boolean "active", default: true, null: false t.datetime "onboarded_at" t.string "unconfirmed_email" - t.string "email_confirmation_token" t.index ["email"], name: "index_users_on_email", unique: true - t.index ["email_confirmation_token"], name: "index_users_on_email_confirmation_token", unique: true t.index ["family_id"], name: "index_users_on_family_id" end -- 2.53.0 From 45ed75030cb81b5a80c5a9e3490fceb67d72a43f Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 31 Jan 2025 11:22:51 -0600 Subject: [PATCH 10/11] Current user would never be true --- app/controllers/email_confirmations_controller.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb index 9c919134..eb5c3755 100644 --- a/app/controllers/email_confirmations_controller.rb +++ b/app/controllers/email_confirmations_controller.rb @@ -10,11 +10,7 @@ class EmailConfirmationsController < ApplicationController email: @user.unconfirmed_email, unconfirmed_email: nil ) - if Current.user == @user - redirect_to settings_profile_path, notice: t(".success") - else - redirect_to new_session_path, notice: t(".success_login") - end + redirect_to new_session_path, notice: t(".success_login") else redirect_to root_path, alert: t(".invalid_token") end -- 2.53.0 From 6b3341d81fb042349d811df23ed7620f8c345618 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Fri, 31 Jan 2025 11:26:19 -0600 Subject: [PATCH 11/11] Fix translation test failures --- config/locales/views/accounts/en.yml | 4 ++++ config/locales/views/email_confirmations/en.yml | 7 ------- 2 files changed, 4 insertions(+), 7 deletions(-) delete mode 100644 config/locales/views/email_confirmations/en.yml diff --git a/config/locales/views/accounts/en.yml b/config/locales/views/accounts/en.yml index 94aa32b2..9162acdd 100644 --- a/config/locales/views/accounts/en.yml +++ b/config/locales/views/accounts/en.yml @@ -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." diff --git a/config/locales/views/email_confirmations/en.yml b/config/locales/views/email_confirmations/en.yml deleted file mode 100644 index c4963d5a..00000000 --- a/config/locales/views/email_confirmations/en.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -en: - email_confirmations: - new: - success: "Your email has been successfully updated." - success_login: "Your email has been confirmed. Please log in with your new email address." - invalid_token: "Invalid or expired confirmation link." \ No newline at end of file -- 2.53.0