diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb index b98bcecd0d..5db2668f72 100644 --- a/app/controllers/auth/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -8,7 +8,10 @@ class Auth::PasswordsController < Devise::PasswordsController def update super do |resource| - resource.session_activations.destroy_all if resource.errors.empty? + if resource.errors.empty? + resource.session_activations.destroy_all + resource.forget_me! + end end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 78feb1631e..d319662486 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Auth::RegistrationsController < Devise::RegistrationsController + include Devise::Controllers::Rememberable + layout :determine_layout before_action :set_invite, only: [:new, :create] @@ -24,7 +26,11 @@ class Auth::RegistrationsController < Devise::RegistrationsController def update super do |resource| - resource.clear_other_sessions(current_session.session_id) if resource.saved_change_to_encrypted_password? + if resource.saved_change_to_encrypted_password? + resource.clear_other_sessions(current_session.session_id) + resource.forget_me! + remember_me(resource) + end end end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index e56b0d58dd..568025bb61 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class HomeController < ApplicationController + before_action :redirect_unauthenticated_to_permalinks! before_action :authenticate_user! before_action :set_referrer_policy_header @@ -10,7 +11,7 @@ class HomeController < ApplicationController private - def authenticate_user! + def redirect_unauthenticated_to_permalinks! return if user_signed_in? matches = request.path.match(/\A\/web\/(statuses|accounts)\/([\d]+)\z/) @@ -35,6 +36,7 @@ class HomeController < ApplicationController end matches = request.path.match(%r{\A/web/timelines/tag/(?.+)\z}) + redirect_to(matches ? tag_path(CGI.unescape(matches[:tag])) : default_redirect_path) end diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb index a8261ec2ba..0b1d09de93 100644 --- a/app/controllers/media_proxy_controller.rb +++ b/app/controllers/media_proxy_controller.rb @@ -2,6 +2,7 @@ class MediaProxyController < ApplicationController include RoutingHelper + include Authorization skip_before_action :store_current_location skip_before_action :require_functional! @@ -10,12 +11,14 @@ class MediaProxyController < ApplicationController rescue_from ActiveRecord::RecordInvalid, with: :not_found rescue_from Mastodon::UnexpectedResponseError, with: :not_found + rescue_from Mastodon::NotPermittedError, with: :not_found rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error def show RedisLock.acquire(lock_options) do |lock| if lock.acquired? - @media_attachment = MediaAttachment.remote.find(params[:id]) + @media_attachment = MediaAttachment.remote.attached.find(params[:id]) + authorize @media_attachment.status, :show? redownload! if @media_attachment.needs_redownload? && !reject_media? else raise Mastodon::RaceConditionError diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index deaca7f3bd..1720697702 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -6627,7 +6627,6 @@ noscript { padding: 15px; padding-bottom: 10px; color: $primary-text-color; - border-radius: 4px; font-size: 14px; font-weight: 400; border-bottom: 1px solid lighten($ui-base-color, 12%); diff --git a/chart/Chart.yaml b/chart/Chart.yaml index f028227e35..783569451d 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -20,7 +20,7 @@ version: 0.1.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. -appVersion: 3.1.4 +appVersion: 3.1.5 dependencies: - name: elasticsearch diff --git a/chart/values.yaml.template b/chart/values.yaml.template index 0710722a67..4a8286d001 100644 --- a/chart/values.yaml.template +++ b/chart/values.yaml.template @@ -4,7 +4,7 @@ image: repository: tootsuite/mastodon pullPolicy: Always # https://hub.docker.com/r/tootsuite/mastodon/tags - tag: v3.1.4 + tag: v3.1.5 # alternatively, use `latest` for the latest release or `edge` for the image # built from the most recent commit # diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 9c18816dd8..512db73278 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -38,15 +38,6 @@ class Rack::Attack end end - PROTECTED_PATHS = %w( - /auth/sign_in - /auth - /auth/password - /auth/confirmation - ).freeze - - PROTECTED_PATHS_REGEX = Regexp.union(PROTECTED_PATHS.map { |path| /\A#{Regexp.escape(path)}/ }) - Rack::Attack.safelist('allow from localhost') do |req| req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' end @@ -86,8 +77,32 @@ class Rack::Attack req.authenticated_user_id if (req.post? && req.path =~ API_DELETE_REBLOG_REGEX) || (req.delete? && req.path =~ API_DELETE_STATUS_REGEX) end - throttle('protected_paths', limit: 25, period: 5.minutes) do |req| - req.remote_ip if req.post? && req.path =~ PROTECTED_PATHS_REGEX + throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req| + req.remote_ip if req.post? && req.path == '/auth' + end + + throttle('throttle_password_resets/ip', limit: 25, period: 5.minutes) do |req| + req.remote_ip if req.post? && req.path == '/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' + end + + throttle('throttle_email_confirmations/ip', limit: 25, period: 5.minutes) do |req| + req.remote_ip if req.post? && req.path == '/auth/confirmation' + end + + throttle('throttle_email_confirmations/email', limit: 5, period: 30.minutes) do |req| + req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/password' + end + + throttle('throttle_login_attempts/ip', limit: 25, period: 5.minutes) do |req| + req.remote_ip if req.post? && req.path == '/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' end self.throttled_response = lambda do |env| diff --git a/config/initializers/rack_attack_logging.rb b/config/initializers/rack_attack_logging.rb index c30bd8a643..ab4822e96b 100644 --- a/config/initializers/rack_attack_logging.rb +++ b/config/initializers/rack_attack_logging.rb @@ -2,5 +2,6 @@ ActiveSupport::Notifications.subscribe(/rack_attack/) do |_name, _start, _finish req = payload[:request] next unless [:throttle, :blacklist].include? req.env['rack.attack.match_type'] + Rails.logger.info("Rate limit hit (#{req.env['rack.attack.match_type']}): #{req.ip} #{req.request_method} #{req.fullpath}") end diff --git a/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb b/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb new file mode 100644 index 0000000000..5c6865b927 --- /dev/null +++ b/db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb @@ -0,0 +1,17 @@ +class MediaAttachmentIdsToTimestampIds < ActiveRecord::Migration[5.1] + def up + # Set up the media_attachments.id column to use our timestamp-based IDs. + safety_assured do + execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT timestamp_id('media_attachments')") + end + + # Make sure we have a sequence to use. + Mastodon::Snowflake.ensure_id_sequences_exist + end + + def down + execute("LOCK media_attachments") + execute("SELECT setval('media_attachments_id_seq', (SELECT MAX(id) FROM media_attachments))") + execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT nextval('media_attachments_id_seq')") + end +end diff --git a/db/schema.rb b/db/schema.rb index 2a6e322350..93e9a7cfca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -481,7 +481,7 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do t.index ["user_id", "timeline"], name: "index_markers_on_user_id_and_timeline", unique: true end - create_table "media_attachments", force: :cascade do |t| + create_table "media_attachments", id: :bigint, default: -> { "timestamp_id('media_attachments'::text)" }, force: :cascade do |t| t.bigint "status_id" t.string "file_file_name" t.string "file_content_type" diff --git a/lib/paperclip/media_type_spoof_detector_extensions.rb b/lib/paperclip/media_type_spoof_detector_extensions.rb index 9c05573564..363934d8d1 100644 --- a/lib/paperclip/media_type_spoof_detector_extensions.rb +++ b/lib/paperclip/media_type_spoof_detector_extensions.rb @@ -2,8 +2,16 @@ module Paperclip module MediaTypeSpoofDetectorExtensions - def calculated_content_type - @calculated_content_type ||= type_from_mime_magic || type_from_file_command + def mapping_override_mismatch? + !Array(mapped_content_type).include?(calculated_content_type) && !Array(mapped_content_type).include?(type_from_mime_magic) + end + + def calculated_media_type_from_mime_magic + @calculated_media_type_from_mime_magic ||= type_from_mime_magic.split('/').first + end + + def calculated_type_mismatch? + !media_types_from_name.include?(calculated_media_type) && !media_types_from_name.include?(calculated_media_type_from_mime_magic) end def type_from_mime_magic diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb index ac44a76f20..2925aed599 100644 --- a/spec/controllers/media_controller_spec.rb +++ b/spec/controllers/media_controller_spec.rb @@ -28,9 +28,8 @@ describe MediaController do end it 'raises when not permitted to view' do - status = Fabricate(:status) + status = Fabricate(:status, visibility: :direct) media_attachment = Fabricate(:media_attachment, status: status) - allow_any_instance_of(MediaController).to receive(:authorize).and_raise(ActiveRecord::RecordNotFound) get :show, params: { id: media_attachment.to_param } expect(response).to have_http_status(404) diff --git a/spec/controllers/media_proxy_controller_spec.rb b/spec/controllers/media_proxy_controller_spec.rb new file mode 100644 index 0000000000..32510cf43d --- /dev/null +++ b/spec/controllers/media_proxy_controller_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe MediaProxyController do + render_views + + before do + stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) + end + + describe '#show' do + it 'redirects when attached to a status' do + status = Fabricate(:status) + media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(302) + end + + it 'responds with missing when there is not an attached status' do + media_attachment = Fabricate(:media_attachment, status: nil, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(404) + end + + it 'raises when id cant be found' do + get :show, params: { id: 'missing' } + + expect(response).to have_http_status(404) + end + + it 'raises when not permitted to view' do + status = Fabricate(:status, visibility: :direct) + media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png') + get :show, params: { id: media_attachment.id } + + expect(response).to have_http_status(404) + end + end +end