From c48d895ea72451d40d5c1fdc91a1fd3bdb50335c Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 16 Feb 2020 12:56:53 +0100 Subject: Fix sign-ups without checked user agreement being accepted through the web form (#13088) * Fix user agreement not being verified * Fix tests * Fix up agreement field being dismissed --- app/controllers/auth/registrations_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 745b91d46..78feb1631 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -41,7 +41,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController resource.locale = I18n.locale resource.invite_code = params[:invite_code] if resource.invite_code.blank? - resource.agreement = true resource.current_sign_in_ip = request.remote_ip resource.build_account if resource.account.nil? @@ -49,7 +48,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController def configure_sign_up_params devise_parameter_sanitizer.permit(:sign_up) do |u| - u.permit({ account_attributes: [:username], invite_request_attributes: [:text] }, :email, :password, :password_confirmation, :invite_code) + u.permit({ account_attributes: [:username], invite_request_attributes: [:text] }, :email, :password, :password_confirmation, :invite_code, :agreement) end end -- cgit From d8e9bae482b2921c3a06e6d94c0b8a07cf0c41ff Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 19 Feb 2020 22:31:53 +0100 Subject: Fix account JSON/RSS not being cacheable due to wrong mime type comparison (#13116) `request.format` is not a symbol but a `Mime::Type`, so the condition actually never matched, and a session was created even for those requests, preventing caching. --- app/controllers/accounts_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 0a8015a56..124393d62 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -9,7 +9,7 @@ class AccountsController < ApplicationController before_action :set_cache_headers before_action :set_body_classes - skip_around_action :set_locale, if: -> { [:json, :rss].include?(request.format) } + skip_around_action :set_locale, if: -> { [:json, :rss].include?(request.format&.to_sym) } skip_before_action :require_functional! def show -- cgit From 7face973fa1c7d6c18b06d427ea0b7a741d11466 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 24 Feb 2020 22:21:40 +0100 Subject: Fix dismissing an announcement twice raising an obscure error (#13124) --- app/controllers/api/v1/announcements_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/announcements_controller.rb b/app/controllers/api/v1/announcements_controller.rb index 1e692ff75..ee79fc19f 100644 --- a/app/controllers/api/v1/announcements_controller.rb +++ b/app/controllers/api/v1/announcements_controller.rb @@ -11,7 +11,7 @@ class Api::V1::AnnouncementsController < Api::BaseController end def dismiss - AnnouncementMute.create!(account: current_account, announcement: @announcement) + AnnouncementMute.find_or_create_by!(account: current_account, announcement: @announcement) render_empty end -- cgit From 0c28a505dddd13e2773cd3d5e0beef76a21eb415 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 27 Feb 2020 12:32:54 +0100 Subject: Fix leak of arbitrary statuses through unfavourite action in REST API (#13161) --- .../api/v1/statuses/bookmarks_controller.rb | 27 +++---- .../statuses/favourited_by_accounts_controller.rb | 3 +- .../api/v1/statuses/favourites_controller.rb | 26 +++---- .../statuses/reblogged_by_accounts_controller.rb | 3 +- .../api/v1/statuses/reblogs_controller.rb | 27 +++---- .../api/v1/statuses/bookmarks_controller_spec.rb | 67 ++++++++++++----- .../api/v1/statuses/favourites_controller_spec.rb | 86 +++++++++++++++------- .../api/v1/statuses/reblogs_controller_spec.rb | 86 +++++++++++++++------- 8 files changed, 202 insertions(+), 123 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/statuses/bookmarks_controller.rb b/app/controllers/api/v1/statuses/bookmarks_controller.rb index bb9729cf5..a7f1eed00 100644 --- a/app/controllers/api/v1/statuses/bookmarks_controller.rb +++ b/app/controllers/api/v1/statuses/bookmarks_controller.rb @@ -5,35 +5,28 @@ class Api::V1::Statuses::BookmarksController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:bookmarks' } before_action :require_user! + before_action :set_status respond_to :json def create - @status = bookmarked_status + current_account.bookmarks.find_or_create_by!(account: current_account, status: @status) render json: @status, serializer: REST::StatusSerializer end def destroy - @status = requested_status - @bookmarks_map = { @status.id => false } + bookmark = current_account.bookmarks.find_by(status: @status) + bookmark&.destroy! - bookmark = Bookmark.find_by!(account: current_user.account, status: @status) - bookmark.destroy! - - render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, bookmarks_map: @bookmarks_map) + render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, bookmarks_map: { @status.id => false }) end private - def bookmarked_status - authorize_with current_user.account, requested_status, :show? - - bookmark = Bookmark.find_or_create_by!(account: current_user.account, status: requested_status) - - bookmark.status.reload - end - - def requested_status - Status.find(params[:status_id]) + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found end end diff --git a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb index 99eff360e..05f4acc33 100644 --- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -69,8 +69,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController @status = Status.find(params[:status_id]) authorize @status, :show? rescue Mastodon::NotPermittedError - # Reraise in order to get a 404 instead of a 403 error code - raise ActiveRecord::RecordNotFound + not_found end def pagination_params(core_params) diff --git a/app/controllers/api/v1/statuses/favourites_controller.rb b/app/controllers/api/v1/statuses/favourites_controller.rb index cceee9060..f18ace996 100644 --- a/app/controllers/api/v1/statuses/favourites_controller.rb +++ b/app/controllers/api/v1/statuses/favourites_controller.rb @@ -5,34 +5,26 @@ class Api::V1::Statuses::FavouritesController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:favourites' } before_action :require_user! + before_action :set_status respond_to :json def create - @status = favourited_status + FavouriteService.new.call(current_account, @status) render json: @status, serializer: REST::StatusSerializer end def destroy - @status = requested_status - @favourites_map = { @status.id => false } - - UnfavouriteWorker.perform_async(current_user.account_id, @status.id) - - render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, favourites_map: @favourites_map) + UnfavouriteWorker.perform_async(current_account.id, @status.id) + render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, favourites_map: { @status.id => false }) end private - def favourited_status - service_result.status.reload - end - - def service_result - FavouriteService.new.call(current_user.account, requested_status) - end - - def requested_status - Status.find(params[:status_id]) + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found end end diff --git a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb index cc285ad23..fa60e7d84 100644 --- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -66,8 +66,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController @status = Status.find(params[:status_id]) authorize @status, :show? rescue Mastodon::NotPermittedError - # Reraise in order to get a 404 instead of a 403 error code - raise ActiveRecord::RecordNotFound + not_found end def pagination_params(core_params) diff --git a/app/controllers/api/v1/statuses/reblogs_controller.rb b/app/controllers/api/v1/statuses/reblogs_controller.rb index 42381a37f..67106ccbe 100644 --- a/app/controllers/api/v1/statuses/reblogs_controller.rb +++ b/app/controllers/api/v1/statuses/reblogs_controller.rb @@ -5,33 +5,34 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:statuses' } before_action :require_user! + before_action :set_reblog respond_to :json def create - @status = ReblogService.new.call(current_user.account, status_for_reblog, reblog_params) + @status = ReblogService.new.call(current_account, @reblog, reblog_params) render json: @status, serializer: REST::StatusSerializer end def destroy - @status = status_for_destroy.reblog - @reblogs_map = { @status.id => false } + @status = current_account.statuses.find_by(reblog_of_id: @reblog.id) - authorize status_for_destroy, :unreblog? - status_for_destroy.discard - RemovalWorker.perform_async(status_for_destroy.id) + if @status + authorize @status, :unreblog? + @status.discard + RemovalWorker.perform_async(@status.id) + end - render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, reblogs_map: @reblogs_map) + render json: @reblog, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false }) end private - def status_for_reblog - Status.find params[:status_id] - end - - def status_for_destroy - @status_for_destroy ||= current_user.account.statuses.where(reblog_of_id: params[:status_id]).first! + def set_reblog + @reblog = Status.find(params[:status_id]) + authorize @reblog, :show? + rescue Mastodon::NotPermittedError + not_found end def reblog_params diff --git a/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb index b79853718..aa5ca433f 100644 --- a/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb @@ -21,36 +21,67 @@ describe Api::V1::Statuses::BookmarksController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(:success) - end + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json - it 'updates the bookmarked attribute' do - expect(user.account.bookmarked?(status)).to be true + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:bookmarked]).to be true + end end - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:id]).to eq status.id.to_s - expect(hash_body[:bookmarked]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - Bookmark.find_or_create_by!(account: user.account, status: status) - post :destroy, params: { status_id: status.id } - end + before do + Bookmark.find_or_create_by!(account: user.account, status: status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(:success) + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be false + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:bookmarked]).to be false + end end - it 'updates the bookmarked attribute' do - expect(user.account.bookmarked?(status)).to be false + context 'with private status that was not bookmarked' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end diff --git a/spec/controllers/api/v1/statuses/favourites_controller_spec.rb b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb index 24a760e20..6e947f5d2 100644 --- a/spec/controllers/api/v1/statuses/favourites_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb @@ -21,45 +21,77 @@ describe Api::V1::Statuses::FavouritesController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 1 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:favourites_count]).to eq 1 + expect(hash_body[:favourited]).to be true + end end - it 'updates the favourites count' do - expect(status.favourites.count).to eq 1 - end - - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:id]).to eq status.id.to_s - expect(hash_body[:favourites_count]).to eq 1 - expect(hash_body[:favourited]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - FavouriteService.new.call(user.account, status) - post :destroy, params: { status_id: status.id } - end + before do + FavouriteService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 0 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be false + end - it 'updates the favourites count' do - expect(status.favourites.count).to eq 0 + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:favourites_count]).to eq 0 + expect(hash_body[:favourited]).to be false + end end - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be false + context 'with private status that was not favourited' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end diff --git a/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb index d14ca3e8b..93b244cc3 100644 --- a/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb @@ -21,45 +21,77 @@ describe Api::V1::Statuses::ReblogsController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 1 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:reblog][:id]).to eq status.id.to_s + expect(hash_body[:reblog][:reblogs_count]).to eq 1 + expect(hash_body[:reblog][:reblogged]).to be true + end end - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 1 - end - - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:reblog][:id]).to eq status.id.to_s - expect(hash_body[:reblog][:reblogs_count]).to eq 1 - expect(hash_body[:reblog][:reblogged]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - ReblogService.new.call(user.account, status) - post :destroy, params: { status_id: status.id } - end + before do + ReblogService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 0 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be false + end - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 0 + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:reblogs_count]).to eq 0 + expect(hash_body[:reblogged]).to be false + end end - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be false + context 'with private status that was not reblogged' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end -- cgit