From 6dd7180f056666d837bf71798f866db31f11f6d4 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 27 May 2022 16:21:59 +0200 Subject: Fix incorrect permission check for notifications destroy/dismiss endpoints (#1787) --- app/controllers/api/v1/notifications_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/controllers/api/v1') diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index c47d6ccfd..ac49167cb 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class Api::V1::NotificationsController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:notifications' }, except: [:clear, :dismiss] - before_action -> { doorkeeper_authorize! :write, :'write:notifications' }, only: [:clear, :dismiss] + before_action -> { doorkeeper_authorize! :read, :'read:notifications' }, except: [:clear, :dismiss, :destroy, :destroy_multiple] + before_action -> { doorkeeper_authorize! :write, :'write:notifications' }, only: [:clear, :dismiss, :destroy, :destroy_multiple] before_action :require_user! after_action :insert_pagination_headers, only: :index -- cgit From 28329ba62f90890b110517f6da69ca81d588f2f5 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 1 Jun 2022 17:31:36 +0200 Subject: Add /api/v1/admin/domain_blocks (#18247) * Add /api/v1/admin/domain_blocks Fixes #18140 - `GET /api/v1/admin/domain_blocks` lists domain blocks - `GET /api/v1/admin/domain_blocks/:id` shows one by ID - `DELETE /api/v1/admin/domain_blocks/:id` deletes a given domain block - `POST /api/v1/admin/domain_blocks` to create a new domain block: if it conflicts with an existing one, returns an error with an attribute `existing_domain_block` with the rendered domain block * Simplify conflict handling as suggested in review --- .../api/v1/admin/domain_blocks_controller.rb | 109 +++++++++++++++++ app/models/domain_block.rb | 1 + .../rest/admin/domain_block_serializer.rb | 11 ++ .../existing_domain_block_error_serializer.rb | 15 +++ config/locales/en.yml | 1 + config/routes.rb | 2 + .../api/v1/admin/domain_blocks_controller_spec.rb | 132 +++++++++++++++++++++ 7 files changed, 271 insertions(+) create mode 100644 app/controllers/api/v1/admin/domain_blocks_controller.rb create mode 100644 app/serializers/rest/admin/domain_block_serializer.rb create mode 100644 app/serializers/rest/admin/existing_domain_block_error_serializer.rb create mode 100644 spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb (limited to 'app/controllers/api/v1') diff --git a/app/controllers/api/v1/admin/domain_blocks_controller.rb b/app/controllers/api/v1/admin/domain_blocks_controller.rb new file mode 100644 index 000000000..229870eee --- /dev/null +++ b/app/controllers/api/v1/admin/domain_blocks_controller.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +class Api::V1::Admin::DomainBlocksController < Api::BaseController + include Authorization + include AccountableConcern + + LIMIT = 100 + + before_action -> { authorize_if_got_token! :'admin:read', :'admin:read:domain_blocks' }, only: [:index, :show] + before_action -> { authorize_if_got_token! :'admin:write', :'admin:write:domain_blocks' }, except: [:index, :show] + before_action :require_staff! + before_action :set_domain_blocks, only: :index + before_action :set_domain_block, only: [:show, :update, :destroy] + + after_action :insert_pagination_headers, only: :index + + PAGINATION_PARAMS = %i(limit).freeze + + def create + authorize :domain_block, :create? + + existing_domain_block = resource_params[:domain].present? ? DomainBlock.rule_for(resource_params[:domain]) : nil + return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if existing_domain_block.present? + + @domain_block = DomainBlock.create!(resource_params) + DomainBlockWorker.perform_async(@domain_block.id) + log_action :create, @domain_block + render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer + end + + def index + authorize :domain_block, :index? + render json: @domain_blocks, each_serializer: REST::Admin::DomainBlockSerializer + end + + def show + authorize @domain_block, :show? + render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer + end + + def update + authorize @domain_block, :update? + + @domain_block.update(domain_block_params) + severity_changed = @domain_block.severity_changed? + @domain_block.save! + DomainBlockWorker.perform_async(@domain_block.id, severity_changed) + log_action :update, @domain_block + render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer + end + + def destroy + authorize @domain_block, :destroy? + UnblockDomainService.new.call(@domain_block) + log_action :destroy, @domain_block + render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer + end + + private + + def set_domain_blocks + @domain_blocks = filtered_domain_blocks.order(id: :desc).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + end + + def set_domain_block + @domain_block = DomainBlock.find(params[:id]) + end + + def filtered_domain_blocks + # TODO: no filtering yet + DomainBlock.all + end + + def domain_block_params + params.permit(:severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + api_v1_admin_domain_blocks_url(pagination_params(max_id: pagination_max_id)) if records_continue? + end + + def prev_path + api_v1_admin_domain_blocks_url(pagination_params(min_id: pagination_since_id)) unless @domain_blocks.empty? + end + + def pagination_max_id + @domain_blocks.last.id + end + + def pagination_since_id + @domain_blocks.first.id + end + + def records_continue? + @domain_blocks.size == limit_param(LIMIT) + end + + def pagination_params(core_params) + params.slice(*PAGINATION_PARAMS).permit(*PAGINATION_PARAMS).merge(core_params) + end + + def resource_params + params.permit(:domain, :severity, :reject_media, :reject_reports, :private_comment, :public_comment, :obfuscate) + end +end diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 2b797ef91..a15206b5e 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -16,6 +16,7 @@ # class DomainBlock < ApplicationRecord + include Paginable include DomainNormalizable include DomainMaterializable diff --git a/app/serializers/rest/admin/domain_block_serializer.rb b/app/serializers/rest/admin/domain_block_serializer.rb new file mode 100644 index 000000000..b955d008a --- /dev/null +++ b/app/serializers/rest/admin/domain_block_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class REST::Admin::DomainBlockSerializer < ActiveModel::Serializer + attributes :id, :domain, :created_at, :severity, + :reject_media, :reject_reports, + :private_comment, :public_comment, :obfuscate + + def id + object.id.to_s + end +end diff --git a/app/serializers/rest/admin/existing_domain_block_error_serializer.rb b/app/serializers/rest/admin/existing_domain_block_error_serializer.rb new file mode 100644 index 000000000..629566dad --- /dev/null +++ b/app/serializers/rest/admin/existing_domain_block_error_serializer.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class REST::Admin::ExistingDomainBlockErrorSerializer < ActiveModel::Serializer + attributes :error + + has_one :existing_domain_block, serializer: REST::Admin::DomainBlockSerializer + + def error + I18n.t('admin.domain_blocks.existing_domain_block', name: existing_domain_block.domain) + end + + def existing_domain_block + object + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index fc0ec6e3e..6bb0cc7ab 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -428,6 +428,7 @@ en: destroyed_msg: Domain block has been undone domain: Domain edit: Edit domain block + existing_domain_block: You have already imposed stricter limits on %{name}. existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to unblock it first. new: create: Create block diff --git a/config/routes.rb b/config/routes.rb index b05d31e4e..dfce94929 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -559,6 +559,8 @@ Rails.application.routes.draw do end end + resources :domain_blocks, only: [:index, :show, :update, :create, :destroy] + namespace :trends do resources :tags, only: [:index] resources :links, only: [:index] diff --git a/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb new file mode 100644 index 000000000..196f6dc28 --- /dev/null +++ b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb @@ -0,0 +1,132 @@ +require 'rails_helper' + +RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do + render_views + + let(:role) { 'admin' } + let(:user) { Fabricate(:user, role: role) } + let(:scopes) { 'admin:read admin:write' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + shared_examples 'forbidden for wrong scope' do |wrong_scope| + let(:scopes) { wrong_scope } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + shared_examples 'forbidden for wrong role' do |wrong_role| + let(:role) { wrong_role } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + describe 'GET #index' do + let!(:block) { Fabricate(:domain_block) } + + before do + get :index + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', 'user' + it_behaves_like 'forbidden for wrong role', 'moderator' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns the expected domain blocks' do + json = body_as_json + expect(json.length).to eq 1 + expect(json[0][:id].to_i).to eq block.id + end + end + + describe 'GET #show' do + let!(:block) { Fabricate(:domain_block) } + + before do + get :show, params: { id: block.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', 'user' + it_behaves_like 'forbidden for wrong role', 'moderator' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns expected domain name' do + json = body_as_json + expect(json[:domain]).to eq block.domain + end + end + + describe 'DELETE #destroy' do + let!(:block) { Fabricate(:domain_block) } + + before do + delete :destroy, params: { id: block.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', 'user' + it_behaves_like 'forbidden for wrong role', 'moderator' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'deletes the block' do + expect(DomainBlock.find_by(id: block.id)).to be_nil + end + end + + describe 'POST #create' do + let(:existing_block_domain) { 'example.com' } + let!(:block) { Fabricate(:domain_block, domain: existing_block_domain, severity: :suspend) } + + before do + post :create, params: { domain: 'foo.bar.com', severity: :silence } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', 'user' + it_behaves_like 'forbidden for wrong role', 'moderator' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns expected domain name' do + json = body_as_json + expect(json[:domain]).to eq 'foo.bar.com' + end + + it 'creates a domain block' do + expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil + end + + context 'when a stricter domain block already exists' do + let(:existing_block_domain) { 'bar.com' } + + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end + + it 'renders existing domain block in error' do + json = body_as_json + expect(json[:existing_domain_block][:domain]).to eq existing_block_domain + end + end + end +end -- cgit