From 2092152b035a787bb7b001b60a8a8be6f30ff940 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 25 Jan 2021 13:47:44 +0000
Subject: [PATCH] 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>
---
 app/helpers/application_helper.rb            | 12 ++++++++++++
 app/views/layouts/application.html.erb       |  4 ++--
 spec/requests/application_controller_spec.rb | 16 +++++++++++++---
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 0b994496dc..0f94822298 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -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
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 165ece79e1..796a7be265 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -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>
 
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index 5e82551fc2..1ac69118a7 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@@ -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