From 290d78cea4850982a2843dc1a2954f0d66fe58d8 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sun, 13 Nov 2022 05:57:10 +0000 Subject: Allow unsetting x-amz-acl S3 Permission headers (#20510) Some "S3 Compatible" storage providers (Cloudflare R2 is one such example) don't support setting ACLs on individual uploads with the `x-amz-acl` header, and instead just have a visibility for the whole bucket. To support uploads to such providers without getting unsupported errors back, lets use a black `S3_PERMISSION` env var to indicate that these headers shouldn't be sent. This is tested as working with Cloudflare R2. --- config/initializers/paperclip.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'config/initializers') diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 26b0a2f7c..5c182ade4 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -67,6 +67,12 @@ if ENV['S3_ENABLED'] == 'true' retry_limit: 0, } ) + + if ENV['S3_PERMISSION'] == '' + Paperclip::Attachment.default_options.merge!( + s3_permissions: ->(*) { nil } + ) + end if ENV.has_key?('S3_ENDPOINT') Paperclip::Attachment.default_options[:s3_options].merge!( -- cgit From 9d039209cc0791ec09ebdbb93da4d63f815e1b07 Mon Sep 17 00:00:00 2001 From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon, 14 Nov 2022 04:26:49 +0000 Subject: Add `Cache-Control` header to openstack-stored files (#20610) When storing files in S3, paperclip is configured with a Cache-Control header indicating the file is immutable, however no such header was added when using OpenStack storage. Luckily Paperclip's fog integration makes this trivial, with a simple `fog_file` `Cache-Control` default doing the trick. --- config/initializers/paperclip.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'config/initializers') diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 5c182ade4..a2285427c 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -125,6 +125,8 @@ elsif ENV['SWIFT_ENABLED'] == 'true' openstack_region: ENV['SWIFT_REGION'], openstack_cache_ttl: ENV.fetch('SWIFT_CACHE_TTL') { 60 }, }, + + fog_file: { 'Cache-Control' => 'public, max-age=315576000, immutable' }, fog_directory: ENV['SWIFT_CONTAINER'], fog_host: ENV['SWIFT_OBJECT_URL'], -- cgit From 21fd25a269cca742af431f0d13299e139f267346 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 14 Nov 2022 20:26:31 +0100 Subject: Fix rate limiting for paths with formats (#20675) --- Gemfile | 3 +- Gemfile.lock | 1 + config/initializers/rack_attack.rb | 49 ++++++++++------- config/routes.rb | 6 +- spec/config/initializers/rack_attack_spec.rb | 82 ++++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 spec/config/initializers/rack_attack_spec.rb (limited to 'config/initializers') diff --git a/Gemfile b/Gemfile index 07c300510..355b7e43f 100644 --- a/Gemfile +++ b/Gemfile @@ -122,6 +122,7 @@ group :test do gem 'simplecov', '~> 0.21', require: false gem 'webmock', '~> 3.18' gem 'rspec_junit_formatter', '~> 0.6' + gem 'rack-test', '~> 2.0' end group :development do @@ -152,7 +153,5 @@ end gem 'concurrent-ruby', require: false gem 'connection_pool', require: false - gem 'xorcist', '~> 1.1' - gem 'cocoon', '~> 1.2' diff --git a/Gemfile.lock b/Gemfile.lock index 28c6e7348..87d07b631 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -815,6 +815,7 @@ DEPENDENCIES rack (~> 2.2.4) rack-attack (~> 6.6) rack-cors (~> 1.1) + rack-test (~> 2.0) rails (~> 6.1.7) rails-controller-testing (~> 1.0) rails-i18n (~> 6.0) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 745eb5d3b..72ef7ba80 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -17,6 +17,18 @@ class Rack::Attack @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end + def throttleable_remote_ip + @throttleable_remote_ip ||= begin + ip = IPAddr.new(remote_ip) + + if ip.ipv6? + ip.mask(64) + else + ip + end + end.to_s + end + def authenticated_user_id authenticated_token&.resource_owner_id end @@ -29,6 +41,10 @@ class Rack::Attack path.start_with?('/api') end + def path_matches?(other_path) + /\A#{Regexp.escape(other_path)}(\..*)?\z/ =~ path + end + def web_request? !api_request? end @@ -51,19 +67,19 @@ class Rack::Attack end throttle('throttle_unauthenticated_api', limit: 300, period: 5.minutes) do |req| - req.remote_ip if req.api_request? && req.unauthenticated? + req.throttleable_remote_ip if req.api_request? && req.unauthenticated? end throttle('throttle_api_media', limit: 30, period: 30.minutes) do |req| - req.authenticated_user_id if req.post? && req.path.match?('^/api/v\d+/media') + req.authenticated_user_id if req.post? && req.path.match?(/\A\/api\/v\d+\/media\z/i) end throttle('throttle_media_proxy', limit: 30, period: 10.minutes) do |req| - req.remote_ip if req.path.start_with?('/media_proxy') + req.throttleable_remote_ip if req.path.start_with?('/media_proxy') end throttle('throttle_api_sign_up', limit: 5, period: 30.minutes) do |req| - req.remote_ip if req.post? && req.path == '/api/v1/accounts' + req.throttleable_remote_ip if req.post? && req.path == '/api/v1/accounts' end throttle('throttle_authenticated_paging', limit: 300, period: 15.minutes) do |req| @@ -71,39 +87,34 @@ class Rack::Attack end throttle('throttle_unauthenticated_paging', limit: 300, period: 15.minutes) do |req| - req.remote_ip if req.paging_request? && req.unauthenticated? + req.throttleable_remote_ip if req.paging_request? && req.unauthenticated? end - API_DELETE_REBLOG_REGEX = /\A\/api\/v1\/statuses\/[\d]+\/unreblog/.freeze - API_DELETE_STATUS_REGEX = /\A\/api\/v1\/statuses\/[\d]+/.freeze + API_DELETE_REBLOG_REGEX = /\A\/api\/v1\/statuses\/[\d]+\/unreblog\z/.freeze + API_DELETE_STATUS_REGEX = /\A\/api\/v1\/statuses\/[\d]+\z/.freeze throttle('throttle_api_delete', limit: 30, period: 30.minutes) do |req| req.authenticated_user_id if (req.post? && req.path.match?(API_DELETE_REBLOG_REGEX)) || (req.delete? && req.path.match?(API_DELETE_STATUS_REGEX)) end throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req| - if req.post? && req.path == '/auth' - addr = req.remote_ip - addr = IPAddr.new(addr) if addr.is_a?(String) - addr = addr.mask(64) if addr.ipv6? - addr.to_s - end + req.throttleable_remote_ip if req.post? && req.path_matches?('/auth') end throttle('throttle_password_resets/ip', limit: 25, period: 5.minutes) do |req| - req.remote_ip if req.post? && req.path == '/auth/password' + req.throttleable_remote_ip if req.post? && req.path_matches?('/auth/password') end throttle('throttle_password_resets/email', limit: 5, period: 30.minutes) do |req| - req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/password' + req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/password') end throttle('throttle_email_confirmations/ip', limit: 25, period: 5.minutes) do |req| - req.remote_ip if req.post? && %w(/auth/confirmation /api/v1/emails/confirmations).include?(req.path) + req.throttleable_remote_ip if req.post? && (req.path_matches?('/auth/confirmation') || req.path == '/api/v1/emails/confirmations') end throttle('throttle_email_confirmations/email', limit: 5, period: 30.minutes) do |req| - if req.post? && req.path == '/auth/password' + if req.post? && req.path_matches?('/auth/password') req.params.dig('user', 'email').presence elsif req.post? && req.path == '/api/v1/emails/confirmations' req.authenticated_user_id @@ -111,11 +122,11 @@ class Rack::Attack end throttle('throttle_login_attempts/ip', limit: 25, period: 5.minutes) do |req| - req.remote_ip if req.post? && req.path == '/auth/sign_in' + req.throttleable_remote_ip if req.post? && req.path_matches?('/auth/sign_in') end throttle('throttle_login_attempts/email', limit: 25, period: 1.hour) do |req| - req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/sign_in' + req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in') end self.throttled_responder = lambda do |request| diff --git a/config/routes.rb b/config/routes.rb index f095ff655..98aa5a033 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -73,7 +73,7 @@ Rails.application.routes.draw do end end - devise_for :users, path: 'auth', controllers: { + devise_for :users, path: 'auth', format: false, controllers: { omniauth_callbacks: 'auth/omniauth_callbacks', sessions: 'auth/sessions', registrations: 'auth/registrations', @@ -216,7 +216,7 @@ Rails.application.routes.draw do resource :relationships, only: [:show, :update] resource :statuses_cleanup, controller: :statuses_cleanup, only: [:show, :update] - get '/media_proxy/:id/(*any)', to: 'media_proxy#show', as: :media_proxy + get '/media_proxy/:id/(*any)', to: 'media_proxy#show', as: :media_proxy, format: false resource :authorize_interaction, only: [:show, :create] resource :share, only: [:show, :create] @@ -404,7 +404,7 @@ Rails.application.routes.draw do get '/admin', to: redirect('/admin/dashboard', status: 302) - namespace :api do + namespace :api, format: false do # OEmbed get '/oembed', to: 'oembed#show', as: :oembed diff --git a/spec/config/initializers/rack_attack_spec.rb b/spec/config/initializers/rack_attack_spec.rb new file mode 100644 index 000000000..581021cb9 --- /dev/null +++ b/spec/config/initializers/rack_attack_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' + +describe Rack::Attack do + include Rack::Test::Methods + + def app + Rails.application + end + + shared_examples 'throttled endpoint' do + context 'when the number of requests is lower than the limit' do + it 'does not change the request status' do + limit.times do + request.call + expect(last_response.status).to_not eq(429) + end + end + end + + context 'when the number of requests is higher than the limit' do + it 'returns http too many requests' do + (limit * 2).times do |i| + request.call + expect(last_response.status).to eq(429) if i > limit + end + end + end + end + + let(:remote_ip) { '1.2.3.5' } + + describe 'throttle excessive sign-up requests by IP address' do + context 'through the website' do + let(:limit) { 25 } + let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + + context 'for exact path' do + let(:path) { '/auth' } + it_behaves_like 'throttled endpoint' + end + + context 'for path with format' do + let(:path) { '/auth.html' } + it_behaves_like 'throttled endpoint' + end + end + + context 'through the API' do + let(:limit) { 5 } + let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + + context 'for exact path' do + let(:path) { '/api/v1/accounts' } + it_behaves_like 'throttled endpoint' + end + + context 'for path with format' do + let(:path) { '/api/v1/accounts.json' } + + it 'returns http not found' do + request.call + expect(last_response.status).to eq(404) + end + end + end + end + + describe 'throttle excessive sign-in requests by IP address' do + let(:limit) { 25 } + let(:request) { ->() { post path, {}, 'REMOTE_ADDR' => remote_ip } } + + context 'for exact path' do + let(:path) { '/auth/sign_in' } + it_behaves_like 'throttled endpoint' + end + + context 'for path with format' do + let(:path) { '/auth/sign_in.html' } + it_behaves_like 'throttled endpoint' + end + end +end -- cgit