From 2998432744fa958c567266c8899bc9204e3883e9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 18 Mar 2026 16:18:40 +1100 Subject: [PATCH 1/8] Remove use of devise token_authenticable Our production servers don't show any use of this feature. The associated column is nil for all users. The gem has not been updated in seven years and it's blocking an important upgrade of devise. --- app/models/spree/user.rb | 2 +- config/initializers/devise.rb | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index b31099752b..c04ad57268 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -8,7 +8,7 @@ module Spree searchable_attributes :email - devise :database_authenticatable, :token_authenticatable, :registerable, :recoverable, + devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :encryptable, :confirmable, encryptor: 'authlogic_sha512', reconfirmable: true, diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index d3348dc9a4..332ef1e6e3 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -138,11 +138,6 @@ Devise.setup do |config| config.case_insensitive_keys = [:email] end -Devise::TokenAuthenticatable.setup do |config| - # Defines name of the authentication token params key - config.token_authentication_key = :auth_token -end - if ENV["OPENID_APP_ID"].present? && ENV["OPENID_APP_SECRET"].present? Devise.setup do |config| site = if Rails.env.development? From ce90ec0f5b9b6ffacec05dd8b9627ade4e95a9fd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 18 Mar 2026 16:23:39 +1100 Subject: [PATCH 2/8] Ignore unused authentication_token column --- Gemfile | 1 - Gemfile.lock | 3 --- app/models/spree/user.rb | 1 + 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index fa407de82c..9fab20b216 100644 --- a/Gemfile +++ b/Gemfile @@ -63,7 +63,6 @@ gem "taler" gem 'devise' gem 'devise-encryptable' gem 'devise-i18n' -gem 'devise-token_authenticatable' gem 'jwt', '~> 2.3' gem 'oauth2', '~> 1.4.7' # Used for Stripe Connect diff --git a/Gemfile.lock b/Gemfile.lock index dc3878291e..7b91aa35d1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -293,8 +293,6 @@ GEM devise-i18n (1.15.0) devise (>= 4.9.0) rails-i18n - devise-token_authenticatable (1.1.0) - devise (>= 4.0.0, < 5.0.0) diff-lcs (1.6.2) digest (3.2.1) docile (1.4.1) @@ -1018,7 +1016,6 @@ DEPENDENCIES devise devise-encryptable devise-i18n - devise-token_authenticatable dfc_provider! digest dotenv diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index c04ad57268..cfebfd65ec 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -5,6 +5,7 @@ module Spree include SetUnusedAddressFields self.belongs_to_required_by_default = false + self.ignored_columns += [:authentication_token] searchable_attributes :email From b4b3e21cf6466501d473f08489a029e3ac0ccb8a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 18 Mar 2026 16:33:10 +1100 Subject: [PATCH 3/8] Replace deprecated option `bypass` - https://github.com/heartcombo/devise/pull/5803 --- app/controllers/spree/admin/users_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index 236bf66f96..fb9ff85d61 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -102,7 +102,7 @@ module Spree def sign_in_if_change_own_password return unless spree_current_user == @user && @user.password.present? - sign_in(@user, event: :authentication, bypass: true) + bypass_sign_in(@user) end def new_email_unconfirmed? From 75616e69e71ece1c07cd1c7e613444a45f39d47b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 18 Mar 2026 16:39:30 +1100 Subject: [PATCH 4/8] Bump devise from 4.9.4 to 5.0.3 --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7b91aa35d1..26c7677fe6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,10 +282,10 @@ GEM debug (1.11.1) irb (~> 1.10) reline (>= 0.3.8) - devise (4.9.4) + devise (5.0.3) bcrypt (~> 3.0) orm_adapter (~> 0.1) - railties (>= 4.1.0) + railties (>= 7.0) responders warden (~> 1.2.3) devise-encryptable (0.2.0) @@ -530,7 +530,7 @@ GEM net-protocol newrelic_rpm (9.24.0) nio4r (2.7.5) - nokogiri (1.19.1) + nokogiri (1.19.2) mini_portile2 (~> 2.8.2) racc (~> 1.4) nokogiri-html5-inference (0.3.0) From ee653bb825000d40c36cd3efa69f3eeca9f5d0c0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 19 Mar 2026 10:59:40 +1100 Subject: [PATCH 5/8] Remove redundant spec description Admin users are the only one who can manage users. --- spec/system/admin/users_spec.rb | 324 ++++++++++++++++---------------- 1 file changed, 161 insertions(+), 163 deletions(-) diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 4f228cb1d1..536eeaeb86 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -5,201 +5,199 @@ require "system_helper" RSpec.describe "Managing users" do include AuthenticationHelper - context "as super-admin" do + before do + login_as_admin + end + + context "from the index page" do before do - login_as_admin + create(:user, email: "a@example.com") + create(:user, email: "b@example.com") + + visit spree.admin_dashboard_path + click_link "Users" end - context "from the index page" do - before do - create(:user, email: "a@example.com") - create(:user, email: "b@example.com") - - visit spree.admin_dashboard_path - click_link "Users" + context "users index page with sorting" do + before(:each) do + click_link "users_email_title" end - context "users index page with sorting" do - before(:each) do - click_link "users_email_title" - end - - it "should list users with order email asc" do - expect(page).to have_css('table#listing_users') - within("table#listing_users") do - expect(page).to have_content("a@example.com") - expect(page).to have_content("b@example.com") - end - end - - it "should list users with order email desc" do - click_link "users_email_title" - within("table#listing_users") do - expect(page).to have_content("a@example.com") - expect(page).to have_content("b@example.com") - end + it "should list users with order email asc" do + expect(page).to have_css('table#listing_users') + within("table#listing_users") do + expect(page).to have_content("a@example.com") + expect(page).to have_content("b@example.com") end end - context "searching users" do - it "should display the correct results for a user search" do - fill_in "q_email_cont", with: "a@example" - click_button "Search" - within("table#listing_users") do - expect(page).to have_content("a@example") - expect(page).not_to have_content("b@example") - end + it "should list users with order email desc" do + click_link "users_email_title" + within("table#listing_users") do + expect(page).to have_content("a@example.com") + expect(page).to have_content("b@example.com") end end + end - context "editing users" do - before(:each) do - click_link("a@example.com") + context "searching users" do + it "should display the correct results for a user search" do + fill_in "q_email_cont", with: "a@example" + click_button "Search" + within("table#listing_users") do + expect(page).to have_content("a@example") + expect(page).not_to have_content("b@example") end + end + end - it "should allow editing the user password" do - fill_in "user_password", with: "welcome" - fill_in "user_password_confirmation", with: "welcome" - click_button "Update" + context "editing users" do + before(:each) do + click_link("a@example.com") + end - expect(page).to have_content("Account updated") - end + it "should allow editing the user password" do + fill_in "user_password", with: "welcome" + fill_in "user_password_confirmation", with: "welcome" + click_button "Update" - it "should let me edit the user email" do - fill_in "Email", with: "newemail@example.org" - click_button "Update" + expect(page).to have_content("Account updated") + end - expect(page).to have_content("The account will be updated once " \ - "the new email is confirmed.") - end + it "should let me edit the user email" do + fill_in "Email", with: "newemail@example.org" + click_button "Update" - it "should allow to generate, regenarate and clear the user api key" do - user = Spree::User.find_by(email: "a@example.com") - expect(page).to have_content "NO KEY" + expect(page).to have_content("The account will be updated once " \ + "the new email is confirmed.") + end - click_button "Generate API key" - first_user_api_key = user.reload.spree_api_key - expect(page).to have_content first_user_api_key + it "should allow to generate, regenarate and clear the user api key" do + user = Spree::User.find_by(email: "a@example.com") + expect(page).to have_content "NO KEY" - click_button "Regenerate Key" - second_user_api_key = user.reload.spree_api_key - expect(page).to have_content second_user_api_key - expect(second_user_api_key).not_to eq first_user_api_key + click_button "Generate API key" + first_user_api_key = user.reload.spree_api_key + expect(page).to have_content first_user_api_key - click_button "Clear key" - expect(page).to have_content "NO KEY" - end + click_button "Regenerate Key" + second_user_api_key = user.reload.spree_api_key + expect(page).to have_content second_user_api_key + expect(second_user_api_key).not_to eq first_user_api_key - it "should allow to disable the user and to enable it" do - expect(page).to have_unchecked_field "Disabled" - check "Disabled" - click_button "Update" + click_button "Clear key" + expect(page).to have_content "NO KEY" + end - expect(page).to have_content("Account updated") - expect(page).to have_checked_field "Disabled" - uncheck "Disabled" - click_button "Update" + it "should allow to disable the user and to enable it" do + expect(page).to have_unchecked_field "Disabled" + check "Disabled" + click_button "Update" - expect(page).to have_content("Account updated") - expect(page).to have_unchecked_field "Disabled" - end + expect(page).to have_content("Account updated") + expect(page).to have_checked_field "Disabled" + uncheck "Disabled" + click_button "Update" - it "should toggle the api key generation view" do - user = Spree::User.find_by(email: "a@example.com") + expect(page).to have_content("Account updated") + expect(page).to have_unchecked_field "Disabled" + end - expect(page).to have_content "NO KEY" - expect { - click_button("Generate API key") - expect(page).to have_content("Key generated") - }.to change { user.reload.spree_api_key }.from(nil) + it "should toggle the api key generation view" do + user = Spree::User.find_by(email: "a@example.com") + expect(page).to have_content "NO KEY" + expect { + click_button("Generate API key") + expect(page).to have_content("Key generated") + }.to change { user.reload.spree_api_key }.from(nil) + + expect(page).to have_unchecked_field "Show API key view for user" + + expect { + check "Show API key view for user" + expect(page).to have_content("Show API key view has been changed!") + expect(page).to have_checked_field "Show API key view for user" + }.to change { user.reload.show_api_key_view }.from(false).to(true) + + expect { + uncheck "Show API key view for user" + expect(page).to have_content("Show API key view has been changed!") expect(page).to have_unchecked_field "Show API key view for user" - - expect { - check "Show API key view for user" - expect(page).to have_content("Show API key view has been changed!") - expect(page).to have_checked_field "Show API key view for user" - }.to change { user.reload.show_api_key_view }.from(false).to(true) - - expect { - uncheck "Show API key view for user" - expect(page).to have_content("Show API key view has been changed!") - expect(page).to have_unchecked_field "Show API key view for user" - }.to change { user.reload.show_api_key_view }.to(false) - end - end - - context "pagination" do - before do - # creates 8 more users - 8.times { create(:user) } - expect(Spree::User.count).to eq 11 - visit spree.admin_users_path - end - it "displays pagination" do - # table displays 10 entries - within('tbody') do - expect(page).to have_css('tr', count: 10) - end - within ".pagination" do - expect(page).not_to have_content "Previous" - expect(page).to have_content "Next" - click_on "2" - end - # table displays 1 entry - within('tbody') do - expect(page).to have_css('tr', count: 1) - end - within ".pagination" do - expect(page).to have_content "Previous" - expect(page).not_to have_content "Next" - end - end + }.to change { user.reload.show_api_key_view }.to(false) end end - describe "creating a user" do - it "confirms successful creation" do - visit spree.new_admin_user_path - - # shows no confirmation message to start with - expect(page).not_to have_text "Email confirmation is pending" - - fill_in "Email", with: "user1@example.org" - fill_in "Password", with: "user1Secret" - fill_in "Confirm Password", with: "user1Secret" - - expect(page).to have_select "Language", selected: "English" - select "Español", from: "Language" - - perform_enqueued_jobs do - expect do - click_button "Create" - end.to change { Spree::User.count }.by 1 - expect(page).to have_text "Created Successfully" - expect(page).to have_text "Email confirmation is pending" - - expect(Spree::User.last.locale).to eq "es" - - expect(ActionMailer::Base.deliveries.first.subject).to match( - "Por favor, confirma tu cuenta de OFN" - ) - end + context "pagination" do + before do + # creates 8 more users + 8.times { create(:user) } + expect(Spree::User.count).to eq 11 + visit spree.admin_users_path end - end - - describe "resending confirmation email" do - let(:user) { create :user, confirmed_at: nil } - - it "displays success" do - visit spree.edit_admin_user_path user - - expect do - # The `a` element doesn't have an href, so we can't use click_link. - find("a", text: "Resend").click - expect(page).to have_text "Resend done" - end.to enqueue_job ActionMailer::MailDeliveryJob + it "displays pagination" do + # table displays 10 entries + within('tbody') do + expect(page).to have_css('tr', count: 10) + end + within ".pagination" do + expect(page).not_to have_content "Previous" + expect(page).to have_content "Next" + click_on "2" + end + # table displays 1 entry + within('tbody') do + expect(page).to have_css('tr', count: 1) + end + within ".pagination" do + expect(page).to have_content "Previous" + expect(page).not_to have_content "Next" + end end end end + + describe "creating a user" do + it "confirms successful creation" do + visit spree.new_admin_user_path + + # shows no confirmation message to start with + expect(page).not_to have_text "Email confirmation is pending" + + fill_in "Email", with: "user1@example.org" + fill_in "Password", with: "user1Secret" + fill_in "Confirm Password", with: "user1Secret" + + expect(page).to have_select "Language", selected: "English" + select "Español", from: "Language" + + perform_enqueued_jobs do + expect do + click_button "Create" + end.to change { Spree::User.count }.by 1 + expect(page).to have_text "Created Successfully" + expect(page).to have_text "Email confirmation is pending" + + expect(Spree::User.last.locale).to eq "es" + + expect(ActionMailer::Base.deliveries.first.subject).to match( + "Por favor, confirma tu cuenta de OFN" + ) + end + end + end + + describe "resending confirmation email" do + let(:user) { create :user, confirmed_at: nil } + + it "displays success" do + visit spree.edit_admin_user_path user + + expect do + # The `a` element doesn't have an href, so we can't use click_link. + find("a", text: "Resend").click + expect(page).to have_text "Resend done" + end.to enqueue_job ActionMailer::MailDeliveryJob + end + end end From c2907b839a60ff14bc51decd96330cdd5f1b60ba Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 19 Mar 2026 11:02:32 +1100 Subject: [PATCH 6/8] Remove ineffective sorting spec The spec was not really testing the order of users appearing on the page. It's also a UX detail only visible to super admins which is not important to test. So I'm not investing time to fix it. --- spec/system/admin/users_spec.rb | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 536eeaeb86..6a4129692d 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -18,28 +18,6 @@ RSpec.describe "Managing users" do click_link "Users" end - context "users index page with sorting" do - before(:each) do - click_link "users_email_title" - end - - it "should list users with order email asc" do - expect(page).to have_css('table#listing_users') - within("table#listing_users") do - expect(page).to have_content("a@example.com") - expect(page).to have_content("b@example.com") - end - end - - it "should list users with order email desc" do - click_link "users_email_title" - within("table#listing_users") do - expect(page).to have_content("a@example.com") - expect(page).to have_content("b@example.com") - end - end - end - context "searching users" do it "should display the correct results for a user search" do fill_in "q_email_cont", with: "a@example" From c5d38d684b08a8a12366c9fe8fd33efe4005a1d7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 19 Mar 2026 11:14:40 +1100 Subject: [PATCH 7/8] Remove repeated navigation to speed up spec --- spec/system/admin/users_spec.rb | 37 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 6a4129692d..4a53307e19 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -10,16 +10,13 @@ RSpec.describe "Managing users" do end context "from the index page" do - before do - create(:user, email: "a@example.com") - create(:user, email: "b@example.com") - - visit spree.admin_dashboard_path - click_link "Users" - end + let!(:user_a) { create(:user, email: "a@example.com") } + let!(:user_b) { create(:user, email: "b@example.com") } context "searching users" do it "should display the correct results for a user search" do + visit spree.admin_dashboard_path + click_link "Users" fill_in "q_email_cont", with: "a@example" click_button "Search" within("table#listing_users") do @@ -30,11 +27,9 @@ RSpec.describe "Managing users" do end context "editing users" do - before(:each) do - click_link("a@example.com") - end - it "should allow editing the user password" do + visit spree.admin_users_path + click_link("a@example.com") fill_in "user_password", with: "welcome" fill_in "user_password_confirmation", with: "welcome" click_button "Update" @@ -43,6 +38,8 @@ RSpec.describe "Managing users" do end it "should let me edit the user email" do + visit spree.edit_admin_user_path(user_a) + fill_in "Email", with: "newemail@example.org" click_button "Update" @@ -51,15 +48,16 @@ RSpec.describe "Managing users" do end it "should allow to generate, regenarate and clear the user api key" do - user = Spree::User.find_by(email: "a@example.com") + visit spree.edit_admin_user_path(user_a) + expect(page).to have_content "NO KEY" click_button "Generate API key" - first_user_api_key = user.reload.spree_api_key + first_user_api_key = user_a.reload.spree_api_key expect(page).to have_content first_user_api_key click_button "Regenerate Key" - second_user_api_key = user.reload.spree_api_key + second_user_api_key = user_a.reload.spree_api_key expect(page).to have_content second_user_api_key expect(second_user_api_key).not_to eq first_user_api_key @@ -68,6 +66,8 @@ RSpec.describe "Managing users" do end it "should allow to disable the user and to enable it" do + visit spree.edit_admin_user_path(user_a) + expect(page).to have_unchecked_field "Disabled" check "Disabled" click_button "Update" @@ -82,13 +82,13 @@ RSpec.describe "Managing users" do end it "should toggle the api key generation view" do - user = Spree::User.find_by(email: "a@example.com") + visit spree.edit_admin_user_path(user_a) expect(page).to have_content "NO KEY" expect { click_button("Generate API key") expect(page).to have_content("Key generated") - }.to change { user.reload.spree_api_key }.from(nil) + }.to change { user_a.reload.spree_api_key }.from(nil) expect(page).to have_unchecked_field "Show API key view for user" @@ -96,13 +96,13 @@ RSpec.describe "Managing users" do check "Show API key view for user" expect(page).to have_content("Show API key view has been changed!") expect(page).to have_checked_field "Show API key view for user" - }.to change { user.reload.show_api_key_view }.from(false).to(true) + }.to change { user_a.reload.show_api_key_view }.from(false).to(true) expect { uncheck "Show API key view for user" expect(page).to have_content("Show API key view has been changed!") expect(page).to have_unchecked_field "Show API key view for user" - }.to change { user.reload.show_api_key_view }.to(false) + }.to change { user_a.reload.show_api_key_view }.to(false) end end @@ -113,6 +113,7 @@ RSpec.describe "Managing users" do expect(Spree::User.count).to eq 11 visit spree.admin_users_path end + it "displays pagination" do # table displays 10 entries within('tbody') do From 66d6627c895facce8958f107c82d39b3bf320361 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 19 Mar 2026 11:22:41 +1100 Subject: [PATCH 8/8] Spec recently changed code path --- spec/system/admin/users_spec.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 4a53307e19..45f950f45c 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -5,8 +5,10 @@ require "system_helper" RSpec.describe "Managing users" do include AuthenticationHelper + let(:admin_user) { create(:admin_user) } + before do - login_as_admin + login_as admin_user end context "from the index page" do @@ -35,6 +37,18 @@ RSpec.describe "Managing users" do click_button "Update" expect(page).to have_content("Account updated") + expect(current_path).to eq spree.edit_admin_user_path(user_a) + end + + it "allows to change your own password without logging you out" do + visit spree.edit_admin_user_path(admin_user) + + fill_in "user_password", with: "welcome" + fill_in "user_password_confirmation", with: "welcome" + click_button "Update" + + expect(page).to have_content("Account updated") + expect(current_path).to eq spree.edit_admin_user_path(admin_user) end it "should let me edit the user email" do