From 844870273ffb1a663c768494c265caef6768ff83 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Tue, 7 Jul 2020 15:26:31 +0200
Subject: Fix other sessions not being logged out on password change (#14252)
While OAuth tokens were immediately revoked, accessing the home
controller immediately generated new OAuth tokens and "revived"
the session due to a combination of using remember_me tokens and
overwriting the `authenticate_user!` method
---
app/controllers/auth/passwords_controller.rb | 5 ++++-
app/controllers/auth/registrations_controller.rb | 8 +++++++-
app/controllers/home_controller.rb | 4 +++-
3 files changed, 14 insertions(+), 3 deletions(-)
(limited to 'app/controllers')
diff --git a/app/controllers/auth/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb
index b98bcecd0..5db2668f7 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 78feb1631..d31966248 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 7c8a18d17..702889cd0 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/(?<tag>.+)\z})
+
redirect_to(matches ? tag_path(CGI.unescape(matches[:tag])) : default_redirect_path)
end
--
cgit
From 6e25574ce599cbc37b7215ded03c7d07208af6bb Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Tue, 7 Jul 2020 15:26:51 +0200
Subject: Fix media attachments enumeration (#14254)
* Fix media attachment enumeration
* Switch media_attachments id to snowflake ids
Co-authored-by: Thibaut Girka <thib@sitedethib.com>
---
app/controllers/media_proxy_controller.rb | 5 ++-
...213645_media_attachment_ids_to_timestamp_ids.rb | 17 +++++++++
db/schema.rb | 26 +++++++-------
spec/controllers/media_controller_spec.rb | 3 +-
spec/controllers/media_proxy_controller_spec.rb | 42 ++++++++++++++++++++++
5 files changed, 77 insertions(+), 16 deletions(-)
create mode 100644 db/migrate/20200622213645_media_attachment_ids_to_timestamp_ids.rb
create mode 100644 spec/controllers/media_proxy_controller_spec.rb
(limited to 'app/controllers')
diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb
index a8261ec2b..0b1d09de9 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/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 000000000..5c6865b92
--- /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 712ba1d2d..cf31b6d23 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -77,6 +77,16 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
t.index ["target_account_id"], name: "index_account_moderation_notes_on_target_account_id"
end
+ create_table "account_notes", force: :cascade do |t|
+ t.bigint "account_id"
+ t.bigint "target_account_id"
+ t.text "comment", null: false
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true
+ t.index ["target_account_id"], name: "index_account_notes_on_target_account_id"
+ end
+
create_table "account_pins", force: :cascade do |t|
t.bigint "account_id"
t.bigint "target_account_id"
@@ -471,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"
@@ -833,16 +843,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
t.index ["user_id"], name: "index_user_invite_requests_on_user_id"
end
- create_table "account_notes", force: :cascade do |t|
- t.bigint "account_id"
- t.bigint "target_account_id"
- t.text "comment", null: false
- t.datetime "created_at", null: false
- t.datetime "updated_at", null: false
- t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true
- t.index ["target_account_id"], name: "index_account_notes_on_target_account_id"
- end
-
create_table "users", force: :cascade do |t|
t.string "email", default: "", null: false
t.datetime "created_at", null: false
@@ -918,6 +918,8 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
add_foreign_key "account_migrations", "accounts", on_delete: :cascade
add_foreign_key "account_moderation_notes", "accounts"
add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id"
+ add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade
+ add_foreign_key "account_notes", "accounts", on_delete: :cascade
add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_pins", "accounts", on_delete: :cascade
add_foreign_key "account_stats", "accounts", on_delete: :cascade
@@ -999,8 +1001,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
add_foreign_key "statuses_tags", "tags", name: "fk_3081861e21", on_delete: :cascade
add_foreign_key "tombstones", "accounts", on_delete: :cascade
add_foreign_key "user_invite_requests", "users", on_delete: :cascade
- add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade
- add_foreign_key "account_notes", "accounts", on_delete: :cascade
add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade
add_foreign_key "users", "invites", on_delete: :nullify
add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify
diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb
index ac44a76f2..2925aed59 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 000000000..32510cf43
--- /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
--
cgit