mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-10-29 17:04:04 +09:00
Remove push subscriptions on a password change, and post them again
A push subscription belongs to the user, not a session, so a device
signed out by a password change kept getting notifications. Changing
the password now removes every subscription along with the sessions,
in the same transaction.
A browser still signed in keeps its own subscription in its service
worker, so the notifications controller posts it once per session: the
bell names the session by a digest of its id, never its token, and the
browser remembers the last one it posted for. That post never
unsubscribes the browser when the server refuses it.
A request that authenticated just before the change could put a
subscription back after it. The create action now checks the session
still exists and saves in one immediate transaction, after validating
the endpoint outside it.
From #371.
(cherry picked from commit 6cab924097)
This commit is contained in:
1 parent
417f087964
commit
93718f7408
9 files changed
+201
-16
No files matched your search
@@ -136,6 +136,18 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_equal "Dave", users(:david).reload.name
|
||||
end
|
||||
|
||||
test "changing the password removes the other devices' push subscriptions, and this browser can register its own again" do
|
||||
stub_web_push_dns_resolution
|
||||
endpoint = push_subscriptions(:david_chrome).attributes.slice("endpoint", "p256dh_key", "auth_key")
|
||||
|
||||
put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
|
||||
assert_empty users(:david).push_subscriptions
|
||||
|
||||
post user_push_subscriptions_url, params: { push_subscription: endpoint }
|
||||
assert_response :ok
|
||||
assert_equal [ endpoint["endpoint"] ], users(:david).push_subscriptions.pluck(:endpoint)
|
||||
end
|
||||
|
||||
test "updates are limited to the current user" do
|
||||
put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
|
||||
|
||||
@@ -148,7 +160,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
|
||||
other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
|
||||
ActionCable.server.expects(:remote_connections).never
|
||||
|
||||
assert_no_difference -> { Session.count } do
|
||||
assert_no_changes -> { [ Session.count, Push::Subscription.count ] } do
|
||||
put user_profile_url, params: params
|
||||
end
|
||||
|
||||
|
||||
@@ -74,4 +74,52 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_redirected_to user_push_subscriptions_url
|
||||
end
|
||||
end
|
||||
|
||||
test "a signed-out session can't register a subscription" do
|
||||
Session.delete_all
|
||||
|
||||
assert_no_difference -> { Push::Subscription.count } do
|
||||
post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
|
||||
end
|
||||
|
||||
assert_redirected_to new_session_url
|
||||
end
|
||||
|
||||
test "a session signed out while its request is in flight doesn't register a subscription" do
|
||||
Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
|
||||
|
||||
assert_no_difference -> { Push::Subscription.count } do
|
||||
post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
|
||||
end
|
||||
|
||||
assert_response :unauthorized
|
||||
end
|
||||
|
||||
test "a session signed out while its request is in flight doesn't keep an existing subscription alive" do
|
||||
subscription = push_subscriptions(:david_chrome)
|
||||
Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
|
||||
|
||||
assert_no_changes -> { subscription.reload.updated_at } do
|
||||
post user_push_subscriptions_url, params: { push_subscription: subscription.attributes.slice("endpoint", "p256dh_key", "auth_key") }
|
||||
end
|
||||
|
||||
assert_response :unauthorized
|
||||
end
|
||||
|
||||
test "a room names its session for the browser to post its subscription again in a new one" do
|
||||
get room_url(rooms(:watercooler))
|
||||
first_marker = css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
|
||||
|
||||
assert_match(/\A\h{64}\z/, first_marker)
|
||||
assert_not_includes response.body, users(:david).sessions.order(:id).last.token
|
||||
|
||||
sign_in :david
|
||||
get room_url(rooms(:watercooler))
|
||||
assert_not_equal first_marker, css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
|
||||
end
|
||||
|
||||
private
|
||||
def new_subscription_params
|
||||
{ "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
|
||||
end
|
||||
end
|
||||
Reference in new issue
Block a user