FIX: Ensure presence endpoints don't break the session (#17108)

Presence endpoints are often called asynchronously at the same time as other request, and never need to modify the session. Skipping ensures that an unneeded cookie rotation doesn't race against another request and cause issues.

This change brings presence in line with message-bus's behaviour.
This commit is contained in:
David Taylor 2022-06-16 14:38:43 +01:00 committed by GitHub
parent a4fc88ce68
commit c00205730e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 0 deletions

View File

@ -3,6 +3,7 @@
class PresenceController < ApplicationController class PresenceController < ApplicationController
skip_before_action :check_xhr skip_before_action :check_xhr
before_action :ensure_logged_in, only: [:update] before_action :ensure_logged_in, only: [:update]
before_action :skip_persist_session
MAX_CHANNELS_PER_REQUEST ||= 50 MAX_CHANNELS_PER_REQUEST ||= 50
@ -75,4 +76,12 @@ class PresenceController < ApplicationController
render json: response render json: response
end end
private
def skip_persist_session
# Presence endpoints are often called asynchronously at the same time as other request,
# and never need to modify the session. Skipping ensures that an unneeded cookie rotation
# doesn't race against another request and cause issues.
session.options[:skip] = true
end
end end

View File

@ -113,6 +113,25 @@ describe PresenceController do
expect(allowed_user_channel.user_ids).to eq([user.id]) expect(allowed_user_channel.user_ids).to eq([user.id])
expect(allowed_group_channel.user_ids).to eq([user.id]) expect(allowed_group_channel.user_ids).to eq([user.id])
end end
it "doesn't overwrite the session" do
sign_in(user)
session_cookie_name = "_forum_session"
get "/session/csrf.json"
expect(response.status).to eq(200)
expect(response.cookies.keys).to include(session_cookie_name)
client_id = SecureRandom.hex
post "/presence/update.json", params: {
client_id: client_id,
present_channels: [
ch1.name,
]
}
expect(response.status).to eq(200)
expect(response.cookies.keys).not_to include(session_cookie_name)
end
end end
describe "#get" do describe "#get" do