mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-10-27 16:06:48 +09:00
Close live connections from the model on any password change
The sessions end wherever the password changes, but only the profile page closed the live connections, so a change from a console left a signed-out device receiving messages until it dropped. Close them after commit from User, logging rather than failing if the realtime service can't be reached, since the password is saved by then.
This commit is contained in:
1 parent
c78c3fd7ef
commit
c6e0105491
3 files changed
+19
-2
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user