Fix not being able to block a subdomain of an already-blocked domain through the API (#30119)

app/controllers/api/v1/admin/domain_blocks_controller.rb

@@ -19,10 +19,11 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController
   def create
     authorize :domain_block, :create?
 
+    @domain_block = DomainBlock.new(resource_params)
     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?
+    return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if conflicts_with_existing_block?(@domain_block, existing_domain_block)
 
-    @domain_block = DomainBlock.create!(resource_params)
+    @domain_block.save!
     DomainBlockWorker.perform_async(@domain_block.id)
     log_action :create, @domain_block
     render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer
@@ -55,6 +56,10 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController
 
   private
 
+  def conflicts_with_existing_block?(domain_block, existing_domain_block)
+    existing_domain_block.present? && (existing_domain_block.domain == TagManager.instance.normalize_domain(domain_block.domain) || !domain_block.stricter_than?(existing_domain_block))
+  end
+
   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

spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb

@@ -16,6 +16,8 @@ RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do
     let(:scopes) { wrong_scope }
 
     it 'returns http forbidden' do
+      subject
+
       expect(response).to have_http_status(403)
     end
   end
@@ -24,6 +26,8 @@ RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do
     let(:role) { UserRole.find_by(name: wrong_role) }
 
     it 'returns http forbidden' do
+      subject
+
       expect(response).to have_http_status(403)
     end
   end
@@ -140,39 +144,70 @@ RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do
 
   describe 'POST #create' do
     let(:existing_block_domain) { 'example.com' }
+    let(:params) { { domain: 'foo.bar.com', severity: :silence } }
     let!(:block) { Fabricate(:domain_block, domain: existing_block_domain, severity: :suspend) }
 
-    before do
-      post :create, params: { domain: 'foo.bar.com', severity: :silence }
+    subject do
+      post :create, params: params
     end
 
     it_behaves_like 'forbidden for wrong scope', 'write:statuses'
     it_behaves_like 'forbidden for wrong role', ''
     it_behaves_like 'forbidden for wrong role', 'Moderator'
 
-    it 'returns http success' do
+    it 'creates a domain block and returns expected domain name', :aggregate_failures do
+      subject
+
       expect(response).to have_http_status(200)
+      expect(body_as_json[:domain]).to eq 'foo.bar.com'
+      expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil
     end
 
-    it 'returns expected domain name' do
-      json = body_as_json
-      expect(json[:domain]).to eq 'foo.bar.com'
-    end
+    context 'when a looser domain block already exists on a higher level domain' do
+      let(:params) { { domain: 'foo.bar.com', severity: :suspend } }
 
-    it 'creates a domain block' do
-      expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil
+      before do
+        Fabricate(:domain_block, domain: 'bar.com', severity: :silence)
+      end
+
+      it 'creates a domain block with the expected domain name and severity', :aggregate_failures do
+        subject
+
+        body = body_as_json
+
+        expect(response).to have_http_status(200)
+        expect(body).to match a_hash_including(
+          {
+            domain: 'foo.bar.com',
+            severity: 'suspend',
+          }
+        )
+
+        expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present
+      end
     end
 
-    context 'when a stricter domain block already exists' do
-      let(:existing_block_domain) { 'bar.com' }
+    context 'when a domain block already exists on the same domain' do
+      before do
+        Fabricate(:domain_block, domain: 'foo.bar.com', severity: :silence)
+      end
+
+      it 'returns existing domain block in error', :aggregate_failures do
+        subject
 
-      it 'returns http unprocessable entity' do
         expect(response).to have_http_status(422)
+        expect(body_as_json[:existing_domain_block][:domain]).to eq('foo.bar.com')
       end
+    end
+
+    context 'when a stricter domain block already exists on a higher level domain' do
+      let(:existing_block_domain) { 'bar.com' }
+
+      it 'returns http unprocessable entity with existing domain block in error', :aggregate_reblogs do
+        subject
 
-      it 'renders existing domain block in error' do
-        json = body_as_json
-        expect(json[:existing_domain_block][:domain]).to eq existing_block_domain
+        expect(response).to have_http_status(422)
+        expect(body_as_json[:existing_domain_block][:domain]).to eq existing_block_domain
       end
     end
   end