Fix anonymous visitors getting a session cookie on first visit (#24584)
This commit is contained in:
		
							parent
							
								
									b61ff36351
								
							
						
					
					
						commit
						31bd0da41f
					
				
					 8 changed files with 64 additions and 26 deletions
				
			
		| 
						 | 
					@ -8,7 +8,6 @@ class Api::BaseController < ApplicationController
 | 
				
			||||||
  include AccessTokenTrackingConcern
 | 
					  include AccessTokenTrackingConcern
 | 
				
			||||||
  include ApiCachingConcern
 | 
					  include ApiCachingConcern
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  skip_before_action :store_current_location
 | 
					 | 
				
			||||||
  skip_before_action :require_functional!, unless: :whitelist_mode?
 | 
					  skip_before_action :require_functional!, unless: :whitelist_mode?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_action :require_authenticated_user!, if: :disallow_unauthenticated_api_access?
 | 
					  before_action :require_authenticated_user!, if: :disallow_unauthenticated_api_access?
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -20,6 +20,7 @@ class ApplicationController < ActionController::Base
 | 
				
			||||||
  helper_method :sso_account_settings
 | 
					  helper_method :sso_account_settings
 | 
				
			||||||
  helper_method :whitelist_mode?
 | 
					  helper_method :whitelist_mode?
 | 
				
			||||||
  helper_method :body_class_string
 | 
					  helper_method :body_class_string
 | 
				
			||||||
 | 
					  helper_method :skip_csrf_meta_tags?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
 | 
					  rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
 | 
				
			||||||
  rescue_from Mastodon::NotPermittedError, with: :forbidden
 | 
					  rescue_from Mastodon::NotPermittedError, with: :forbidden
 | 
				
			||||||
| 
						 | 
					@ -36,7 +37,7 @@ class ApplicationController < ActionController::Base
 | 
				
			||||||
    service_unavailable
 | 
					    service_unavailable
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_action :store_current_location, except: :raise_not_found, unless: :devise_controller?
 | 
					  before_action :store_referrer, except: :raise_not_found, if: :devise_controller?
 | 
				
			||||||
  before_action :require_functional!, if: :user_signed_in?
 | 
					  before_action :require_functional!, if: :user_signed_in?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_action :set_cache_control_defaults
 | 
					  before_action :set_cache_control_defaults
 | 
				
			||||||
| 
						 | 
					@ -57,14 +58,25 @@ class ApplicationController < ActionController::Base
 | 
				
			||||||
    !authorized_fetch_mode?
 | 
					    !authorized_fetch_mode?
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def store_current_location
 | 
					  def store_referrer
 | 
				
			||||||
    store_location_for(:user, request.url) unless [:json, :rss].include?(request.format&.to_sym)
 | 
					    return if request.referer.blank?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    redirect_uri = URI(request.referer)
 | 
				
			||||||
 | 
					    return if redirect_uri.path.start_with?('/auth')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    stored_url = redirect_uri.to_s if redirect_uri.host == request.host && redirect_uri.port == request.port
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    store_location_for(:user, stored_url)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def require_functional!
 | 
					  def require_functional!
 | 
				
			||||||
    redirect_to edit_user_registration_path unless current_user.functional?
 | 
					    redirect_to edit_user_registration_path unless current_user.functional?
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def skip_csrf_meta_tags?
 | 
				
			||||||
 | 
					    false
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def after_sign_out_path_for(_resource_or_scope)
 | 
					  def after_sign_out_path_for(_resource_or_scope)
 | 
				
			||||||
    if ENV['OMNIAUTH_ONLY'] == 'true' && ENV['OIDC_ENABLED'] == 'true'
 | 
					    if ENV['OMNIAUTH_ONLY'] == 'true' && ENV['OIDC_ENABLED'] == 'true'
 | 
				
			||||||
      '/auth/auth/openid_connect/logout'
 | 
					      '/auth/auth/openid_connect/logout'
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -10,6 +10,10 @@ module WebAppControllerConcern
 | 
				
			||||||
    vary_by 'Accept, Accept-Language, Cookie'
 | 
					    vary_by 'Accept, Accept-Language, Cookie'
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def skip_csrf_meta_tags?
 | 
				
			||||||
 | 
					    current_user.nil?
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def set_app_body_class
 | 
					  def set_app_body_class
 | 
				
			||||||
    @body_classes = 'app-body'
 | 
					    @body_classes = 'app-body'
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -3,7 +3,6 @@
 | 
				
			||||||
class MediaController < ApplicationController
 | 
					class MediaController < ApplicationController
 | 
				
			||||||
  include Authorization
 | 
					  include Authorization
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  skip_before_action :store_current_location
 | 
					 | 
				
			||||||
  skip_before_action :require_functional!, unless: :whitelist_mode?
 | 
					  skip_before_action :require_functional!, unless: :whitelist_mode?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_action :authenticate_user!, if: :whitelist_mode?
 | 
					  before_action :authenticate_user!, if: :whitelist_mode?
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -6,7 +6,6 @@ class MediaProxyController < ApplicationController
 | 
				
			||||||
  include Redisable
 | 
					  include Redisable
 | 
				
			||||||
  include Lockable
 | 
					  include Lockable
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  skip_before_action :store_current_location
 | 
					 | 
				
			||||||
  skip_before_action :require_functional!
 | 
					  skip_before_action :require_functional!
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_action :authenticate_user!, if: :whitelist_mode?
 | 
					  before_action :authenticate_user!, if: :whitelist_mode?
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -30,7 +30,7 @@
 | 
				
			||||||
    = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'
 | 
					    = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'
 | 
				
			||||||
    = javascript_pack_tag 'common', crossorigin: 'anonymous'
 | 
					    = javascript_pack_tag 'common', crossorigin: 'anonymous'
 | 
				
			||||||
    = javascript_pack_tag "locale_#{I18n.locale}", crossorigin: 'anonymous'
 | 
					    = javascript_pack_tag "locale_#{I18n.locale}", crossorigin: 'anonymous'
 | 
				
			||||||
    = csrf_meta_tags
 | 
					    = csrf_meta_tags unless skip_csrf_meta_tags?
 | 
				
			||||||
    %meta{ name: 'style-nonce', content: request.content_security_policy_nonce }
 | 
					    %meta{ name: 'style-nonce', content: request.content_security_policy_nonce }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    = stylesheet_link_tag '/inert.css', skip_pipeline: true, media: 'all', id: 'inert-style'
 | 
					    = stylesheet_link_tag '/inert.css', skip_pipeline: true, media: 'all', id: 'inert-style'
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -132,25 +132,6 @@ describe ApplicationController, type: :controller do
 | 
				
			||||||
    include_examples 'respond_with_error', 422
 | 
					    include_examples 'respond_with_error', 422
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  describe 'before_action :store_current_location' do
 | 
					 | 
				
			||||||
    it 'stores location for user if it is not devise controller' do
 | 
					 | 
				
			||||||
      routes.draw { get 'success' => 'anonymous#success' }
 | 
					 | 
				
			||||||
      get 'success'
 | 
					 | 
				
			||||||
      expect(controller.stored_location_for(:user)).to eq '/success'
 | 
					 | 
				
			||||||
    end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    context do
 | 
					 | 
				
			||||||
      controller Devise::SessionsController do
 | 
					 | 
				
			||||||
      end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
      it 'does not store location for user if it is devise controller' do
 | 
					 | 
				
			||||||
        @request.env['devise.mapping'] = Devise.mappings[:user]
 | 
					 | 
				
			||||||
        get 'create'
 | 
					 | 
				
			||||||
        expect(controller.stored_location_for(:user)).to be_nil
 | 
					 | 
				
			||||||
      end
 | 
					 | 
				
			||||||
    end
 | 
					 | 
				
			||||||
  end
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  describe 'before_action :check_suspension' do
 | 
					  describe 'before_action :check_suspension' do
 | 
				
			||||||
    before do
 | 
					    before do
 | 
				
			||||||
      routes.draw { get 'success' => 'anonymous#success' }
 | 
					      routes.draw { get 'success' => 'anonymous#success' }
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
							
								
								
									
										44
									
								
								spec/requests/anonymous_cookies_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										44
									
								
								spec/requests/anonymous_cookies_spec.rb
									
									
									
									
									
										Normal file
									
								
							| 
						 | 
					@ -0,0 +1,44 @@
 | 
				
			||||||
 | 
					# frozen_string_literal: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					require 'rails_helper'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					context 'when visited anonymously' do
 | 
				
			||||||
 | 
					  around do |example|
 | 
				
			||||||
 | 
					    old = ActionController::Base.allow_forgery_protection
 | 
				
			||||||
 | 
					    ActionController::Base.allow_forgery_protection = true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    example.run
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    ActionController::Base.allow_forgery_protection = old
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  describe 'account pages' do
 | 
				
			||||||
 | 
					    it 'do not set cookies' do
 | 
				
			||||||
 | 
					      alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
 | 
				
			||||||
 | 
					      _status = Fabricate(:status, account: alice, text: 'Hello World')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      get '/@alice'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      expect(response.cookies).to be_empty
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  describe 'status pages' do
 | 
				
			||||||
 | 
					    it 'do not set cookies' do
 | 
				
			||||||
 | 
					      alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
 | 
				
			||||||
 | 
					      status = Fabricate(:status, account: alice, text: 'Hello World')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      get short_account_status_url(alice, status)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      expect(response.cookies).to be_empty
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  describe 'the /about page' do
 | 
				
			||||||
 | 
					    it 'does not set cookies' do
 | 
				
			||||||
 | 
					      get '/about'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      expect(response.cookies).to be_empty
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
		Loading…
	
		Reference in a new issue