FIX: Cleanup authentication_data cookie after login (#11834)

This cookie is only used during login. Having it persist after that can
cause some unusual behavior, especially for sites with short session
lengths.

We were already deleting the cookie following a new signup, but not for
existing users.

This commit moves the cookie deletion logic out of the erb template, and
adds logic and tests to ensure it is always deleted consistently.

Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This commit is contained in:
David Taylor 2021-01-25 13:47:44 +00:00 committed by GitHub
parent e65c5b0aad
commit 2092152b03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 27 additions and 5 deletions

View File

@ -560,4 +560,16 @@ module ApplicationHelper
end
end
end
def authentication_data
return @authentication_data if defined?(@authentication_data)
@authentication_data = begin
value = cookies[:authentication_data]
if value
cookies.delete(:authentication_data, path: Discourse.base_path("/"))
end
current_user ? nil : value
end
end
end

View File

@ -60,8 +60,8 @@
<%= tag.meta id: 'data-discourse-setup', data: client_side_setup_data %>
<%- if !current_user && (data = cookies.delete(:authentication_data, path: Discourse.base_path("/"))) %>
<meta id="data-authentication" data-authentication-data="<%= data %>">
<%- if authentication_data %>
<meta id="data-authentication" data-authentication-data="<%= authentication_data %>">
<%- end %>
</head>

View File

@ -104,11 +104,21 @@ RSpec.describe ApplicationController do
end
it 'contains authentication data when cookies exist' do
COOKIE_DATA = "someauthenticationdata"
cookies['authentication_data'] = COOKIE_DATA
cookie_data = "someauthenticationdata"
cookies['authentication_data'] = cookie_data
get '/login'
expect(response.status).to eq(200)
expect(response.body).to include("data-authentication-data=\"#{COOKIE_DATA }\"")
expect(response.body).to include("data-authentication-data=\"#{cookie_data}\"")
expect(response.headers["Set-Cookie"]).to include("authentication_data=;") # Delete cookie
end
it 'deletes authentication data cookie even if already authenticated' do
sign_in(Fabricate(:user))
cookies['authentication_data'] = "someauthenticationdata"
get '/'
expect(response.status).to eq(200)
expect(response.body).not_to include("data-authentication-data=")
expect(response.headers["Set-Cookie"]).to include("authentication_data=;") # Delete cookie
end
end