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 --- .../auth/registrations_controller_spec.rb | 28 ++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb index 3e11b34b5..c2e9f33a8 100644 --- a/spec/controllers/auth/registrations_controller_spec.rb +++ b/spec/controllers/auth/registrations_controller_spec.rb @@ -100,7 +100,7 @@ RSpec.describe Auth::RegistrationsController, type: :controller do subject do Setting.registrations_mode = 'open' request.headers["Accept-Language"] = accept_language - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } } + post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', agreement: 'true' } } end it 'redirects to setup' do @@ -116,6 +116,26 @@ RSpec.describe Auth::RegistrationsController, type: :controller do end end + context 'when user has not agreed to terms of service' do + around do |example| + registrations_mode = Setting.registrations_mode + example.run + Setting.registrations_mode = registrations_mode + end + + subject do + Setting.registrations_mode = 'open' + request.headers["Accept-Language"] = accept_language + post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', agreement: 'false' } } + end + + it 'does not create user' do + subject + user = User.find_by(email: 'test@example.com') + expect(user).to be_nil + end + end + context 'approval-based registrations without invite' do around do |example| registrations_mode = Setting.registrations_mode @@ -126,7 +146,7 @@ RSpec.describe Auth::RegistrationsController, type: :controller do subject do Setting.registrations_mode = 'approved' request.headers["Accept-Language"] = accept_language - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } } + post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', agreement: 'true' } } end it 'redirects to setup' do @@ -154,7 +174,7 @@ RSpec.describe Auth::RegistrationsController, type: :controller do Setting.registrations_mode = 'approved' request.headers["Accept-Language"] = accept_language invite = Fabricate(:invite, max_uses: nil, expires_at: 1.hour.ago) - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } } + post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code, agreement: 'true' } } end it 'redirects to setup' do @@ -182,7 +202,7 @@ RSpec.describe Auth::RegistrationsController, type: :controller do Setting.registrations_mode = 'approved' request.headers["Accept-Language"] = accept_language invite = Fabricate(:invite, max_uses: nil, expires_at: 1.hour.from_now) - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } } + post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code, agreement: 'true' } } end it 'redirects to setup' do -- 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 'spec/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