12 files changed, 62 insertions(+), 45 deletions(-)

M app/controllers/accounts_controller.rb
M app/controllers/api/base_controller.rb
M app/controllers/application_controller.rb
M app/controllers/concerns/cache_concern.rb
M app/controllers/concerns/web_app_controller_concern.rb
M app/controllers/follower_accounts_controller.rb
M app/controllers/following_accounts_controller.rb
M app/controllers/statuses_controller.rb
M app/controllers/tags_controller.rb
M spec/controllers/accounts_controller_spec.rb
M spec/controllers/statuses_controller_spec.rb
M spec/controllers/tags_controller_spec.rb

@@ 7,7 7,7 @@ class AccountsController < ApplicationController
   include AccountControllerConcern
   include SignatureAuthentication
 
-  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+  vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' }
 
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
 

@@ 12,7 12,6 @@ class Api::BaseController < ApplicationController
 
   before_action :require_authenticated_user!, if: :disallow_unauthenticated_api_access?
   before_action :require_not_suspended!
-  before_action :set_cache_control_defaults
 
   protect_from_forgery with: :null_session
 


@@ 148,10 147,6 @@ class Api::BaseController < ApplicationController
     doorkeeper_authorize!(*scopes) if doorkeeper_token
   end
 
-  def set_cache_control_defaults
-    response.cache_control.replace(private: true, no_store: true)
-  end
-
   def disallow_unauthenticated_api_access?
     ENV['DISALLOW_UNAUTHENTICATED_API_ACCESS'] == 'true' || Rails.configuration.x.whitelist_mode
   end

@@ 38,6 38,8 @@ class ApplicationController < ActionController::Base
   before_action :store_current_location, except: :raise_not_found, unless: :devise_controller?
   before_action :require_functional!, if: :user_signed_in?
 
+  before_action :set_cache_control_defaults
+
   skip_before_action :verify_authenticity_token, only: :raise_not_found
 
   def raise_not_found


@@ 152,4 154,8 @@ class ApplicationController < ActionController::Base
       format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code }
     end
   end
+
+  def set_cache_control_defaults
+    response.cache_control.replace(private: true, no_store: true)
+  end
 end

@@ 163,6 163,20 @@ module CacheConcern
     end
   end
 
+  included do
+    after_action :enforce_cache_control!
+  end
+
+  # Prevents high-entropy headers such as `Cookie`, `Signature` or `Authorization`
+  # from being used as cache keys, while allowing to `Vary` on them (to not serve
+  # anonymous cached data to authenticated requests when authentication matters)
+  def enforce_cache_control!
+    vary = response.headers['Vary']&.split&.map { |x| x.strip.downcase }
+    return unless vary.present? && %w(cookie authorization signature).any? { |header| vary.include?(header) && request.headers[header].present? }
+
+    response.cache_control.replace(private: true, no_store: true)
+  end
+
   def render_with_cache(**options)
     raise ArgumentError, 'Only JSON render calls are supported' unless options.key?(:json) || block_given?
 

@@ 6,6 6,8 @@ module WebAppControllerConcern
   included do
     prepend_before_action :redirect_unauthenticated_to_permalinks!
     before_action :set_app_body_class
+
+    vary_by 'Accept, Accept-Language, Cookie'
   end
 
   def set_app_body_class

@@ 5,7 5,7 @@ class FollowerAccountsController < ApplicationController
   include SignatureVerification
   include WebAppControllerConcern
 
-  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+  vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' }
 
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
 

@@ 5,7 5,7 @@ class FollowingAccountsController < ApplicationController
   include SignatureVerification
   include WebAppControllerConcern
 
-  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+  vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' }
 
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
 

@@ 6,7 6,7 @@ class StatusesController < ApplicationController
   include Authorization
   include AccountOwnedConcern
 
-  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+  vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' }
 
   before_action :require_account_signature!, only: [:show, :activity], if: -> { request.format == :json && authorized_fetch_mode? }
   before_action :set_status


@@ 30,7 30,7 @@ class StatusesController < ApplicationController
       end
 
       format.json do
-        expires_in 3.minutes, public: @status.distributable? && public_fetch_mode?
+        expires_in 3.minutes, public: true if @status.distributable? && public_fetch_mode?
         render_with_cache json: @status, content_type: 'application/activity+json', serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter
       end
     end

@@ 7,7 7,7 @@ class TagsController < ApplicationController
   PAGE_SIZE     = 20
   PAGE_SIZE_MAX = 200
 
-  vary_by -> { public_fetch_mode? ? 'Accept' : 'Accept, Signature' }
+  vary_by -> { public_fetch_mode? ? 'Accept, Accept-Language, Cookie' : 'Accept, Accept-Language, Cookie, Signature' }
 
   before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? }
   before_action :authenticate_user!, if: :whitelist_mode?

@@ 218,8 218,8 @@ RSpec.describe AccountsController, type: :controller do
           expect(response.media_type).to eq 'application/activity+json'
         end
 
-        it 'returns public Cache-Control header' do
-          expect(response.headers['Cache-Control']).to include 'public'
+        it 'returns private Cache-Control header' do
+          expect(response.headers['Cache-Control']).to include 'private'
         end
 
         it 'renders account' do

@@ 16,7 16,7 @@ describe StatusesController do
     end
 
     it 'returns Vary header' do
-      expect(response.headers['Vary']).to include 'Accept'
+      expect(response.headers['Vary']).to include 'Accept, Accept-Language, Cookie'
     end
 
     it 'returns public Cache-Control header' do


@@ 84,7 84,7 @@ describe StatusesController do
         end
 
         it 'returns Vary header' do
-          expect(response.headers['Vary']).to eq 'Accept'
+          expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
         end
 
         it 'returns public Cache-Control header' do


@@ 109,7 109,7 @@ describe StatusesController do
         end
 
         it 'returns Vary header' do
-          expect(response.headers['Vary']).to eq 'Accept'
+          expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
         end
 
         it_behaves_like 'cacheable response'


@@ 208,11 208,11 @@ describe StatusesController do
           end
 
           it 'returns Vary header' do
-            expect(response.headers['Vary']).to eq 'Accept'
+            expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
           end
 
-          it 'returns no Cache-Control header' do
-            expect(response.headers).to_not include 'Cache-Control'
+          it 'returns private Cache-Control header' do
+            expect(response.headers['Cache-Control']).to include 'private'
           end
 
           it 'renders status' do


@@ 233,11 233,11 @@ describe StatusesController do
           end
 
           it 'returns Vary header' do
-            expect(response.headers['Vary']).to eq 'Accept'
+            expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
           end
 
-          it 'returns public Cache-Control header' do
-            expect(response.headers['Cache-Control']).to include 'public'
+          it 'returns private Cache-Control header' do
+            expect(response.headers['Cache-Control']).to include 'private'
           end
 
           it 'returns Content-Type header' do


@@ 272,11 272,11 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
-            it 'returns no Cache-Control header' do
-              expect(response.headers).to_not include 'Cache-Control'
+            it 'returns private Cache-Control header' do
+              expect(response.headers['Cache-Control']).to include 'private'
             end
 
             it 'renders status' do


@@ 297,7 297,7 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
             it 'returns private Cache-Control header' do


@@ 359,11 359,11 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
-            it 'returns no Cache-Control header' do
-              expect(response.headers).to_not include 'Cache-Control'
+            it 'returns private Cache-Control header' do
+              expect(response.headers['Cache-Control']).to include 'private'
             end
 
             it 'renders status' do


@@ 384,7 384,7 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
             it 'returns private Cache-Control header' do


@@ 472,11 472,11 @@ describe StatusesController do
           end
 
           it 'returns Vary header' do
-            expect(response.headers['Vary']).to eq 'Accept'
+            expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
           end
 
-          it 'returns no Cache-Control header' do
-            expect(response.headers).to_not include 'Cache-Control'
+          it 'returns private Cache-Control header' do
+            expect(response.headers['Cache-Control']).to include 'private'
           end
 
           it 'renders status' do


@@ 497,7 497,7 @@ describe StatusesController do
           end
 
           it 'returns Vary header' do
-            expect(response.headers['Vary']).to eq 'Accept'
+            expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
           end
 
           it_behaves_like 'cacheable response'


@@ 534,11 534,11 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
-            it 'returns no Cache-Control header' do
-              expect(response.headers).to_not include 'Cache-Control'
+            it 'returns private Cache-Control header' do
+              expect(response.headers['Cache-Control']).to include 'private'
             end
 
             it 'renders status' do


@@ 559,7 559,7 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
             it 'returns private Cache-Control header' do


@@ 621,11 621,11 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
-            it 'returns no Cache-Control header' do
-              expect(response.headers).to_not include 'Cache-Control'
+            it 'returns private Cache-Control header' do
+              expect(response.headers['Cache-Control']).to include 'private'
             end
 
             it 'renders status' do


@@ 646,7 646,7 @@ describe StatusesController do
             end
 
             it 'returns Vary header' do
-              expect(response.headers['Vary']).to eq 'Accept'
+              expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
             end
 
             it 'returns private Cache-Control header' do


@@ 827,7 827,7 @@ describe StatusesController do
       end
 
       it 'returns Vary header' do
-        expect(response.headers['Vary']).to eq 'Accept'
+        expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
       end
 
       it 'returns public Cache-Control header' do

@@ 21,7 21,7 @@ RSpec.describe TagsController, type: :controller do
         end
 
         it 'returns Vary header' do
-          expect(response.headers['Vary']).to eq 'Accept'
+          expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
         end
 
         it 'returns public Cache-Control header' do


@@ 37,7 37,7 @@ RSpec.describe TagsController, type: :controller do
         end
 
         it 'returns Vary header' do
-          expect(response.headers['Vary']).to eq 'Accept'
+          expect(response.headers['Vary']).to eq 'Accept, Accept-Language, Cookie'
         end
 
         it 'returns public Cache-Control header' do