Refactor Cache-Control and Vary definitions (#24347)

spec/controllers/accounts_controller_spec.rb

@@ -17,6 +17,10 @@ RSpec.describe AccountsController, type: :controller do
       expect(session).to be_empty
     end
 
+    it 'returns Vary header' do
+      expect(response.headers['Vary']).to include 'Accept'
+    end
+
     it 'returns public Cache-Control header' do
       expect(response.headers['Cache-Control']).to include 'public'
     end

spec/controllers/admin/base_controller_spec.rb

@@ -18,6 +18,14 @@ describe Admin::BaseController, type: :controller do
     expect(response).to have_http_status(403)
   end
 
+  it 'returns private cache control headers' do
+    routes.draw { get 'success' => 'admin/base#success' }
+    sign_in(Fabricate(:user, role: UserRole.find_by(name: 'Moderator')))
+    get :success
+
+    expect(response.headers['Cache-Control']).to include('private, no-store')
+  end
+
   it 'renders admin layout as a moderator' do
     routes.draw { get 'success' => 'admin/base#success' }
     sign_in(Fabricate(:user, role: UserRole.find_by(name: 'Moderator')))

spec/controllers/api/base_controller_spec.rb

@@ -15,6 +15,12 @@ describe Api::BaseController do
     end
   end
 
+  it 'returns private cache control headers by default' do
+    routes.draw { get 'success' => 'api/base#success' }
+    get :success
+    expect(response.headers['Cache-Control']).to include('private, no-store')
+  end
+
   describe 'forgery protection' do
     before do
       routes.draw { post 'success' => 'api/base#success' }

spec/controllers/api/oembed_controller_spec.rb

@@ -17,5 +17,9 @@ RSpec.describe Api::OEmbedController, type: :controller do
     it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 end

spec/controllers/auth/registrations_controller_spec.rb

@@ -33,27 +33,42 @@ RSpec.describe Auth::RegistrationsController, type: :controller do
   end
 
   describe 'GET #edit' do
-    it 'returns http success' do
+    before do
       request.env['devise.mapping'] = Devise.mappings[:user]
       sign_in(Fabricate(:user))
       get :edit
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control header' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'GET #update' do
-    it 'returns http success' do
+    let(:user) { Fabricate(:user) }
+
+    before do
       request.env['devise.mapping'] = Devise.mappings[:user]
-      sign_in(Fabricate(:user), scope: :user)
+      sign_in(user, scope: :user)
       post :update
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
 
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
+
     context 'when suspended' do
+      let(:user) { Fabricate(:user, account_attributes: { username: 'test', suspended_at: Time.now.utc }) }
+
       it 'returns http forbidden' do
-        request.env['devise.mapping'] = Devise.mappings[:user]
-        sign_in(Fabricate(:user, account_attributes: { username: 'test', suspended_at: Time.now.utc }), scope: :user)
-        post :update
         expect(response).to have_http_status(403)
       end
     end

spec/controllers/custom_css_controller_spec.rb

@@ -6,9 +6,25 @@ describe CustomCssController do
   render_views
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns public cache control header' do
+      expect(response.headers['Cache-Control']).to include('public')
+    end
+
+    it 'does not set cookies' do
+      expect(response.cookies).to be_empty
+      expect(response.headers['Set-Cookies']).to be_nil
+    end
+
+    it 'does not set sessions' do
+      expect(session).to be_empty
+    end
   end
 end

spec/controllers/filters/statuses_controller_spec.rb

@@ -18,21 +18,27 @@ describe Filters::StatusesController do
 
     context 'with a signed in user' do
       context 'with the filter user signed in' do
-        before { sign_in(filter.account.user) }
+        before do
+          sign_in(filter.account.user)
+          get :index, params: { filter_id: filter }
+        end
 
         it 'returns http success' do
-          get :index, params: { filter_id: filter }
-
           expect(response).to have_http_status(200)
         end
+
+        it 'returns private cache control headers' do
+          expect(response.headers['Cache-Control']).to include('private, no-store')
+        end
       end
 
       context 'with another user signed in' do
-        before { sign_in(Fabricate(:user)) }
+        before do
+          sign_in(Fabricate(:user))
+          get :index, params: { filter_id: filter }
+        end
 
         it 'returns http not found' do
-          get :index, params: { filter_id: filter }
-
           expect(response).to have_http_status(404)
         end
       end

spec/controllers/filters_controller_spec.rb

@@ -7,21 +7,28 @@ describe FiltersController do
 
   describe 'GET #index' do
     context 'with signed out user' do
-      it 'redirects' do
+      before do
         get :index
+      end
 
+      it 'redirects' do
         expect(response).to be_redirect
       end
     end
 
     context 'with a signed in user' do
-      before { sign_in(Fabricate(:user)) }
+      before do
+        sign_in(Fabricate(:user))
+        get :index
+      end
 
       it 'returns http success' do
-        get :index
-
         expect(response).to have_http_status(200)
       end
+
+      it 'returns private cache control headers' do
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
     end
   end
 end

spec/controllers/invites_controller_spec.rb

@@ -5,35 +5,40 @@ require 'rails_helper'
 describe InvitesController do
   render_views
 
+  let(:user) { Fabricate(:user) }
+
   before do
     sign_in user
   end
 
   describe 'GET #index' do
-    subject { get :index }
-
-    let(:user) { Fabricate(:user) }
-    let!(:invite) { Fabricate(:invite, user: user) }
+    before do
+      Fabricate(:invite, user: user)
+    end
 
     context 'when everyone can invite' do
       before do
         UserRole.everyone.update(permissions: UserRole.everyone.permissions | UserRole::FLAGS[:invite_users])
+        get :index
       end
 
-      it 'renders index page' do
-        expect(subject).to render_template :index
-        expect(assigns(:invites)).to include invite
-        expect(assigns(:invites).count).to eq 1
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
+
+      it 'returns private cache control headers' do
+        expect(response.headers['Cache-Control']).to include('private, no-store')
       end
     end
 
     context 'when not everyone can invite' do
       before do
         UserRole.everyone.update(permissions: UserRole.everyone.permissions & ~UserRole::FLAGS[:invite_users])
+        get :index
       end
 
-      it 'returns 403' do
-        expect(subject).to have_http_status 403
+      it 'returns http forbidden' do
+        expect(response).to have_http_status(403)
       end
     end
   end
@@ -42,8 +47,6 @@ describe InvitesController do
     subject { post :create, params: { invite: { max_uses: '10', expires_in: 1800 } } }
 
     context 'when everyone can invite' do
-      let(:user) { Fabricate(:user) }
-
       before do
         UserRole.everyone.update(permissions: UserRole.everyone.permissions | UserRole::FLAGS[:invite_users])
       end
@@ -56,26 +59,28 @@ describe InvitesController do
     end
 
     context 'when not everyone can invite' do
-      let(:user) { Fabricate(:user) }
-
       before do
         UserRole.everyone.update(permissions: UserRole.everyone.permissions & ~UserRole::FLAGS[:invite_users])
       end
 
-      it 'returns 403' do
-        expect(subject).to have_http_status 403
+      it 'returns http forbidden' do
+        expect(subject).to have_http_status(403)
       end
     end
   end
 
   describe 'DELETE #create' do
-    subject { delete :destroy, params: { id: invite.id } }
+    let(:invite) { Fabricate(:invite, user: user, expires_at: nil) }
 
-    let(:user) { Fabricate(:user) }
-    let!(:invite) { Fabricate(:invite, user: user, expires_at: nil) }
+    before do
+      delete :destroy, params: { id: invite.id }
+    end
+
+    it 'redirects' do
+      expect(response).to redirect_to invites_path
+    end
 
     it 'expires invite' do
-      expect(subject).to redirect_to invites_path
       expect(invite.reload).to be_expired
     end
   end

spec/controllers/manifests_controller_spec.rb

@@ -7,11 +7,24 @@ describe ManifestsController do
 
   describe 'GET #show' do
     before do
-      get :show, format: :json
+      get :show
     end
 
     it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns public cache control header' do
+      expect(response.headers['Cache-Control']).to include('public')
+    end
+
+    it 'does not set cookies' do
+      expect(response.cookies).to be_empty
+      expect(response.headers['Set-Cookies']).to be_nil
+    end
+
+    it 'does not set sessions' do
+      expect(session).to be_empty
+    end
   end
 end

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -31,6 +31,11 @@ RSpec.describe Oauth::AuthorizationsController, type: :controller do
         expect(response).to have_http_status(200)
       end
 
+      it 'returns private cache control headers' do
+        subject
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
+
       it 'gives options to authorize and deny' do
         subject
         expect(response.body).to match(/Authorize/)

spec/controllers/oauth/authorized_applications_controller_spec.rb

@@ -27,6 +27,11 @@ describe Oauth::AuthorizedApplicationsController do
         expect(response).to have_http_status(200)
       end
 
+      it 'returns private cache control headers' do
+        subject
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
+
       include_examples 'stores location for user'
     end
 

spec/controllers/relationships_controller_spec.rb

@@ -7,42 +7,39 @@ describe RelationshipsController do
 
   let(:user) { Fabricate(:user) }
 
-  shared_examples 'authenticate user' do
-    it 'redirects when not signed in' do
-      expect(subject).to redirect_to '/auth/sign_in'
-    end
-  end
-
   describe 'GET #show' do
-    subject { get :show, params: { page: 2, relationship: 'followed_by' } }
+    context 'when signed in' do
+      before do
+        sign_in user, scope: :user
+        get :show, params: { page: 2, relationship: 'followed_by' }
+      end
 
-    it 'assigns @accounts' do
-      Fabricate(:account, domain: 'old').follow!(user.account)
-      Fabricate(:account, domain: 'recent').follow!(user.account)
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
 
-      sign_in user, scope: :user
-      subject
-
-      assigned = assigns(:accounts).per(1).to_a
-      expect(assigned.size).to eq 1
-      expect(assigned[0].domain).to eq 'old'
+      it 'returns private cache control headers' do
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
     end
 
-    it 'returns http success' do
-      sign_in user, scope: :user
-      subject
-      expect(response).to have_http_status(200)
-    end
+    context 'when not signed in' do
+      before do
+        get :show, params: { page: 2, relationship: 'followed_by' }
+      end
 
-    include_examples 'authenticate user'
+      it 'redirects when not signed in' do
+        expect(response).to redirect_to '/auth/sign_in'
+      end
+    end
   end
 
   describe 'PATCH #update' do
-    let(:poopfeast) { Fabricate(:account, username: 'poopfeast', domain: 'example.com') }
+    let(:alice) { Fabricate(:account, username: 'alice', domain: 'example.com') }
 
     shared_examples 'redirects back to followers page' do
       it 'redirects back to followers page' do
-        poopfeast.follow!(user.account)
+        alice.follow!(user.account)
 
         sign_in user, scope: :user
         subject
@@ -58,27 +55,36 @@ describe RelationshipsController do
     end
 
     context 'when select parameter is provided' do
-      subject { patch :update, params: { form_account_batch: { account_ids: [poopfeast.id] }, remove_domains_from_followers: '' } }
+      subject { patch :update, params: { form_account_batch: { account_ids: [alice.id] }, remove_domains_from_followers: '' } }
 
       it 'soft-blocks followers from selected domains' do
-        poopfeast.follow!(user.account)
+        alice.follow!(user.account)
 
         sign_in user, scope: :user
         subject
 
-        expect(poopfeast.following?(user.account)).to be false
+        expect(alice.following?(user.account)).to be false
       end
 
       it 'does not unfollow users from selected domains' do
-        user.account.follow!(poopfeast)
+        user.account.follow!(alice)
 
         sign_in user, scope: :user
         subject
 
-        expect(user.account.following?(poopfeast)).to be true
+        expect(user.account.following?(alice)).to be true
+      end
+
+      context 'when not signed in' do
+        before do
+          subject
+        end
+
+        it 'redirects when not signed in' do
+          expect(response).to redirect_to '/auth/sign_in'
+        end
       end
 
-      include_examples 'authenticate user'
       include_examples 'redirects back to followers page'
     end
   end

spec/controllers/settings/aliases_controller_spec.rb

@@ -13,10 +13,17 @@ describe Settings::AliasesController do
   end
 
   describe 'GET #index' do
-    it 'returns http success' do
+    before do
       get :index
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'POST #create' do

spec/controllers/settings/applications_controller_spec.rb

@@ -13,13 +13,17 @@ describe Settings::ApplicationsController do
   end
 
   describe 'GET #index' do
-    let!(:other_app) { Fabricate(:application) }
-
-    it 'shows apps' do
+    before do
+      Fabricate(:application)
       get :index
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
-      expect(assigns(:applications)).to include(app)
-      expect(assigns(:applications)).to_not include(other_app)
+    end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end
 

spec/controllers/settings/deletes_controller_spec.rb

@@ -11,20 +11,27 @@ describe Settings::DeletesController do
 
       before do
         sign_in user, scope: :user
+        get :show
       end
 
       it 'renders confirmation page' do
-        get :show
         expect(response).to have_http_status(200)
       end
 
+      it 'returns private cache control headers' do
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
+
       context 'when suspended' do
         let(:user) { Fabricate(:user, account_attributes: { suspended_at: Time.now.utc }) }
 
         it 'returns http forbidden' do
-          get :show
           expect(response).to have_http_status(403)
         end
+
+        it 'returns private cache control headers' do
+          expect(response.headers['Cache-Control']).to include('private, no-store')
+        end
       end
     end
 

spec/controllers/settings/exports_controller_spec.rb

@@ -11,16 +11,16 @@ describe Settings::ExportsController do
 
       before do
         sign_in user, scope: :user
+        get :show
       end
 
-      it 'renders export' do
-        get :show
-
-        export = assigns(:export)
-        expect(export).to be_instance_of Export
-        expect(export.account).to eq user.account
+      it 'returns http success' do
         expect(response).to have_http_status(200)
       end
+
+      it 'returns private cache control headers' do
+        expect(response.headers['Cache-Control']).to include('private, no-store')
+      end
     end
 
     context 'when not signed in' do

spec/controllers/settings/imports_controller_spec.rb

@@ -10,10 +10,17 @@ RSpec.describe Settings::ImportsController, type: :controller do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'POST #create' do

spec/controllers/settings/login_activities_controller_spec.rb

@@ -12,9 +12,16 @@ describe Settings::LoginActivitiesController do
   end
 
   describe 'GET #index' do
-    it 'returns http success' do
+    before do
       get :index
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 end

spec/controllers/settings/migration/redirects_controller_spec.rb

@@ -12,10 +12,17 @@ describe Settings::Migration::RedirectsController do
   end
 
   describe 'GET #new' do
-    it 'returns http success' do
+    before do
       get :new
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'POST #create' do

spec/controllers/settings/preferences/appearance_controller_spec.rb

@@ -12,11 +12,17 @@ describe Settings::Preferences::AppearanceController do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
 
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'PUT #update' do

spec/controllers/settings/preferences/notifications_controller_spec.rb

@@ -12,10 +12,17 @@ describe Settings::Preferences::NotificationsController do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'PUT #update' do

spec/controllers/settings/preferences/other_controller_spec.rb

@@ -12,10 +12,17 @@ describe Settings::Preferences::OtherController do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'PUT #update' do

spec/controllers/settings/profiles_controller_spec.rb

@@ -13,10 +13,17 @@ RSpec.describe Settings::ProfilesController, type: :controller do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'PUT #update' do

spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb

@@ -26,23 +26,25 @@ describe Settings::TwoFactorAuthenticationMethodsController do
       describe 'when user has enabled otp' do
         before do
           user.update(otp_required_for_login: true)
+          get :index
         end
 
         it 'returns http success' do
-          get :index
-
           expect(response).to have_http_status(200)
         end
+
+        it 'returns private cache control headers' do
+          expect(response.headers['Cache-Control']).to include('private, no-store')
+        end
       end
 
       describe 'when user has not enabled otp' do
         before do
           user.update(otp_required_for_login: false)
+          get :index
         end
 
         it 'redirects to enable otp' do
-          get :index
-
           expect(response).to redirect_to(settings_otp_authentication_path)
         end
       end

spec/controllers/statuses_cleanup_controller_spec.rb

@@ -11,19 +11,32 @@ RSpec.describe StatusesCleanupController, type: :controller do
   end
 
   describe 'GET #show' do
-    it 'returns http success' do
+    before do
       get :show
+    end
+
+    it 'returns http success' do
       expect(response).to have_http_status(200)
     end
+
+    it 'returns private cache control headers' do
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
   end
 
   describe 'PUT #update' do
-    it 'updates the account status cleanup policy' do
+    before do
       put :update, params: { account_statuses_cleanup_policy: { enabled: true, min_status_age: 2.weeks.seconds, keep_direct: false, keep_polls: true } }
-      expect(response).to redirect_to(statuses_cleanup_path)
+    end
+
+    it 'updates the account status cleanup policy' do
       expect(@user.account.statuses_cleanup_policy.enabled).to be true
       expect(@user.account.statuses_cleanup_policy.keep_direct).to be false
       expect(@user.account.statuses_cleanup_policy.keep_polls).to be true
     end
+
+    it 'redirects' do
+      expect(response).to redirect_to(statuses_cleanup_path)
+    end
   end
 end

spec/controllers/statuses_controller_spec.rb

@@ -15,6 +15,10 @@ describe StatusesController do
       expect(session).to be_empty
     end
 
+    it 'returns Vary header' do
+      expect(response.headers['Vary']).to include 'Accept'
+    end
+
     it 'returns public Cache-Control header' do
       expect(response.headers['Cache-Control']).to include 'public'
     end

spec/controllers/tags_controller_spec.rb

@@ -6,21 +6,50 @@ RSpec.describe TagsController, type: :controller do
   render_views
 
   describe 'GET #show' do
-    let!(:tag)     { Fabricate(:tag, name: 'test') }
-    let!(:local)   { Fabricate(:status, tags: [tag], text: 'local #test') }
-    let!(:remote)  { Fabricate(:status, tags: [tag], text: 'remote #test', account: Fabricate(:account, domain: 'remote')) }
-    let!(:late)    { Fabricate(:status, tags: [tag], text: 'late #test') }
+    let(:format) { 'html' }
+    let(:tag) { Fabricate(:tag, name: 'test') }
+    let(:tag_name) { tag&.name }
+
+    before do
+      get :show, params: { id: tag_name, format: format }
+    end
 
     context 'when tag exists' do
-      it 'returns http success' do
-        get :show, params: { id: 'test', max_id: late.id }
-        expect(response).to have_http_status(200)
+      context 'when requested as HTML' do
+        it 'returns http success' do
+          expect(response).to have_http_status(200)
+        end
+
+        it 'returns Vary header' do
+          expect(response.headers['Vary']).to eq 'Accept'
+        end
+
+        it 'returns public Cache-Control header' do
+          expect(response.headers['Cache-Control']).to include 'public'
+        end
+      end
+
+      context 'when requested as JSON' do
+        let(:format) { 'json' }
+
+        it 'returns http success' do
+          expect(response).to have_http_status(200)
+        end
+
+        it 'returns Vary header' do
+          expect(response.headers['Vary']).to eq 'Accept'
+        end
+
+        it 'returns public Cache-Control header' do
+          expect(response.headers['Cache-Control']).to include 'public'
+        end
       end
     end
 
     context 'when tag does not exist' do
+      let(:tag_name) { 'hoge' }
+
       it 'returns http not found' do
-        get :show, params: { id: 'none' }
         expect(response).to have_http_status(404)
       end
     end