Fix anonymous visitors getting a session cookie on first visit (#24584)
This commit is contained in:
		
					parent
					
						
							
								6084461cd0
							
						
					
				
			
			
				commit
				
					
						276c39361b
					
				
			
		
					 8 changed files with 64 additions and 26 deletions
				
			
		|  | @ -8,7 +8,6 @@ class Api::BaseController < ApplicationController | |||
|   include AccessTokenTrackingConcern | ||||
|   include ApiCachingConcern | ||||
| 
 | ||||
|   skip_before_action :store_current_location | ||||
|   skip_before_action :require_functional!, unless: :whitelist_mode? | ||||
| 
 | ||||
|   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 :whitelist_mode? | ||||
|   helper_method :body_class_string | ||||
|   helper_method :skip_csrf_meta_tags? | ||||
| 
 | ||||
|   rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request | ||||
|   rescue_from Mastodon::NotPermittedError, with: :forbidden | ||||
|  | @ -36,7 +37,7 @@ class ApplicationController < ActionController::Base | |||
|     service_unavailable | ||||
|   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 :set_cache_control_defaults | ||||
|  | @ -57,14 +58,25 @@ class ApplicationController < ActionController::Base | |||
|     !authorized_fetch_mode? | ||||
|   end | ||||
| 
 | ||||
|   def store_current_location | ||||
|     store_location_for(:user, request.url) unless [:json, :rss].include?(request.format&.to_sym) | ||||
|   def store_referrer | ||||
|     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 | ||||
| 
 | ||||
|   def require_functional! | ||||
|     redirect_to edit_user_registration_path unless current_user.functional? | ||||
|   end | ||||
| 
 | ||||
|   def skip_csrf_meta_tags? | ||||
|     false | ||||
|   end | ||||
| 
 | ||||
|   def after_sign_out_path_for(_resource_or_scope) | ||||
|     if ENV['OMNIAUTH_ONLY'] == 'true' && ENV['OIDC_ENABLED'] == 'true' | ||||
|       '/auth/auth/openid_connect/logout' | ||||
|  |  | |||
|  | @ -10,6 +10,10 @@ module WebAppControllerConcern | |||
|     vary_by 'Accept, Accept-Language, Cookie' | ||||
|   end | ||||
| 
 | ||||
|   def skip_csrf_meta_tags? | ||||
|     current_user.nil? | ||||
|   end | ||||
| 
 | ||||
|   def set_app_body_class | ||||
|     @body_classes = 'app-body' | ||||
|   end | ||||
|  |  | |||
|  | @ -3,7 +3,6 @@ | |||
| class MediaController < ApplicationController | ||||
|   include Authorization | ||||
| 
 | ||||
|   skip_before_action :store_current_location | ||||
|   skip_before_action :require_functional!, unless: :whitelist_mode? | ||||
| 
 | ||||
|   before_action :authenticate_user!, if: :whitelist_mode? | ||||
|  |  | |||
|  | @ -6,7 +6,6 @@ class MediaProxyController < ApplicationController | |||
|   include Redisable | ||||
|   include Lockable | ||||
| 
 | ||||
|   skip_before_action :store_current_location | ||||
|   skip_before_action :require_functional! | ||||
| 
 | ||||
|   before_action :authenticate_user!, if: :whitelist_mode? | ||||
|  |  | |||
|  | @ -30,7 +30,7 @@ | |||
|     = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous' | ||||
|     = javascript_pack_tag 'common', 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 } | ||||
| 
 | ||||
|     = 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 | ||||
|   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 | ||||
|     before do | ||||
|       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…
	
	Add table
		Add a link
		
	
		Reference in a new issue