diff --git a/app/controllers/users/profiles_controller.rb b/app/controllers/users/profiles_controller.rb index 9f3726a7..96015d55 100644 --- a/app/controllers/users/profiles_controller.rb +++ b/app/controllers/users/profiles_controller.rb @@ -8,8 +8,6 @@ class Users::ProfilesController < ApplicationController def update @user.update user_params - disconnect_remote_connections if @user.saved_change_to_password_digest? - redirect_to user_profile_url, notice: update_notice end diff --git a/app/models/user.rb b/app/models/user.rb index 449ab02b..2df933ef 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,6 +21,7 @@ class User < ApplicationRecord after_create_commit :grant_membership_to_open_rooms after_update :sign_out_other_sessions, if: :saved_change_to_password_digest? + after_update_commit :disconnect_signed_out_sessions, if: :saved_change_to_password_digest? scope :ordered, -> { order("LOWER(name)") } scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") } @@ -57,6 +58,14 @@ class User < ApplicationRecord sessions.excluding(Current.session).delete_all end + # After commit, so a connection that drops can't come back on a session that's still there. The password is + # saved by then, so a realtime service that can't be reached mustn't fail the change. + def disconnect_signed_out_sessions + reset_remote_connections + rescue => error + Rails.logger.warn "Could not disconnect remote connections after a password change: #{error.class}" + end + # One statement, which SQLite runs under the write lock, so no room is closed between reading the open ones and # inserting. A room that Room#destroy_later closes is either closed before, and skipped, or after, and takes this # membership away with the others. A room that granted itself to every user when it was created is skipped too, diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 0926adaa..22b8109e 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -50,6 +50,14 @@ class UserTest < ActiveSupport::TestCase end end + test "changing the password outside a request closes the account's live connections" do + remote_connections = mock + remote_connections.expects(:disconnect).with(reconnect: true) + ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) }) + + users(:david).update!(password: "a new secret123456") + end + test "changing the password keeps the session it was changed from" do Current.session = users(:david).sessions.start!(user_agent: "This device", ip_address: "127.0.0.1") @@ -59,6 +67,8 @@ class UserTest < ActiveSupport::TestCase end test "changing other details keeps every session" do + ActionCable.server.expects(:remote_connections).never + assert_no_changes -> { users(:david).sessions.count } do users(:david).update!(name: "Dave", email_address: "dave@37signals.com") end