DEV: Extend specs coverage for non-admin access to admin endpoints (#18833)

Replace base controller class inheritance specs with explicit specs
for non-staff and moderator access to admin resources

spec/requests/admin/web_hooks_controller_spec.rb

@@ -1,20 +1,15 @@
 # frozen_string_literal: true
 
 RSpec.describe Admin::WebHooksController do
+  fab!(:web_hook) { Fabricate(:web_hook) }
+  fab!(:admin) { Fabricate(:admin) }
+  fab!(:moderator) { Fabricate(:moderator) }
+  fab!(:user) { Fabricate(:user) }
 
-  it 'is a subclass of AdminController' do
-    expect(Admin::WebHooksController < Admin::AdminController).to eq(true)
-  end
+  describe '#create' do
+    context "when logged in as admin" do
+      before { sign_in(admin) }
 
-  context 'while logged in as an admin' do
-    fab!(:web_hook) { Fabricate(:web_hook) }
-    fab!(:admin) { Fabricate(:admin) }
-
-    before do
-      sign_in(admin)
-    end
-
-    describe '#create' do
       it 'creates a webhook' do
         post "/admin/api/web_hooks.json", params: {
           web_hook: {
@@ -58,7 +53,45 @@ RSpec.describe Admin::WebHooksController do
       end
     end
 
-    describe '#update' do
+    shared_examples "webhook creation not allowed" do
+      it "prevents creation with a 404 response" do
+        post "/admin/api/web_hooks.json", params: {
+          web_hook: {
+            payload_url: 'https://meta.discourse.org/',
+            content_type: 1,
+            secret: "a_secret_for_webhooks",
+            wildcard_web_hook: false,
+            active: true,
+            verify_certificate: true,
+            web_hook_event_type_ids: [1],
+            group_ids: [],
+            category_ids: []
+          }
+        }
+
+        expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"]).to include(I18n.t("not_found"))
+        expect(response.parsed_body["web_hook"]).to be_nil
+      end
+    end
+
+    context "when logged in as a moderator" do
+      before { sign_in(moderator) }
+
+      include_examples "webhook creation not allowed"
+    end
+
+    context "when logged in as a non-staff user" do
+      before  { sign_in(user) }
+
+      include_examples "webhook creation not allowed"
+    end
+  end
+
+  describe '#update' do
+    context "when logged in as admin" do
+      before { sign_in(admin) }
+
       it "logs webhook update" do
         put "/admin/api/web_hooks/#{web_hook.id}.json", params: {
           web_hook: { active: false, payload_url: "https://test.com" }
@@ -71,7 +104,37 @@ RSpec.describe Admin::WebHooksController do
       end
     end
 
-    describe '#destroy' do
+    shared_examples "webhook update not allowed" do
+      it "prevents updates with a 404 response" do
+        current_payload_url = web_hook.payload_url
+        put "/admin/api/web_hooks/#{web_hook.id}.json", params: {
+          web_hook: { active: false, payload_url: "https://test.com" }
+        }
+
+        web_hook.reload
+        expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"]).to include(I18n.t("not_found"))
+        expect(web_hook.payload_url).to eq(current_payload_url)
+      end
+    end
+
+    context "when logged in as a moderator" do
+      before { sign_in(moderator) }
+
+      include_examples "webhook update not allowed"
+    end
+
+    context "when logged in as a non-staff user" do
+      before  { sign_in(user) }
+
+      include_examples "webhook update not allowed"
+    end
+  end
+
+  describe '#destroy' do
+    context "when logged in as admin" do
+      before { sign_in(admin) }
+
       it "logs webhook destroy" do
         delete "/admin/api/web_hooks/#{web_hook.id}.json", params: {
           web_hook: { active: false, payload_url: "https://test.com" }
@@ -82,7 +145,33 @@ RSpec.describe Admin::WebHooksController do
       end
     end
 
-    describe '#ping' do
+    shared_examples "webhook deletion not allowed" do
+      it "prevents deletion with a 404 response" do
+        delete "/admin/api/web_hooks/#{web_hook.id}.json"
+
+        expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"]).to include(I18n.t("not_found"))
+        expect(web_hook.reload).to be_present
+      end
+    end
+
+    context "when logged in as a moderator" do
+      before { sign_in(moderator) }
+
+      include_examples "webhook deletion not allowed"
+    end
+
+    context "when logged in as a non-staff user" do
+      before  { sign_in(user) }
+
+      include_examples "webhook deletion not allowed"
+    end
+  end
+
+  describe '#ping' do
+    context "when logged in as admin" do
+      before { sign_in(admin) }
+
       it 'enqueues the ping event' do
         expect do
           post "/admin/api/web_hooks/#{web_hook.id}/ping.json"
@@ -95,62 +184,87 @@ RSpec.describe Admin::WebHooksController do
       end
     end
 
-    describe '#redeliver_event' do
-      let!(:web_hook_event) do
-        WebHookEvent.create!(
-          web_hook: web_hook,
-          payload: "abc",
-          headers: JSON.dump(aa: "1", bb: "2"),
+    shared_examples "webhook ping not allowed" do
+      it "fails to enqueue a ping with 404 response" do
+        expect do
+          post "/admin/api/web_hooks/#{web_hook.id}/ping.json"
+        end.not_to change { Jobs::EmitWebHookEvent.jobs.size }
+
+        expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"]).to include(I18n.t("not_found"))
+      end
+    end
+
+    context "when logged in as a moderator" do
+      before { sign_in(moderator) }
+
+      include_examples "webhook ping not allowed"
+    end
+
+    context "when logged in as a non-staff user" do
+      before  { sign_in(user) }
+
+      include_examples "webhook ping not allowed"
+    end
+  end
+
+  describe '#redeliver_event' do
+    let!(:web_hook_event) do
+      WebHookEvent.create!(
+        web_hook: web_hook,
+        payload: "abc",
+        headers: JSON.dump(aa: "1", bb: "2"),
+      )
+    end
+
+    before { sign_in(admin) }
+
+    it 'emits the web hook and updates the response headers and body' do
+      stub_request(:post, web_hook.payload_url)
+        .with(body: "abc", headers: { "aa" => 1, "bb" => 2 })
+        .to_return(
+          status: 402,
+          body: "efg",
+          headers: { "Content-Type" => "application/json", "yoo" => "man" }
         )
-      end
+      post "/admin/api/web_hooks/#{web_hook.id}/events/#{web_hook_event.id}/redeliver.json"
+      expect(response.status).to eq(200)
 
-      it 'emits the web hook and updates the response headers and body' do
-        stub_request(:post, web_hook.payload_url)
-          .with(body: "abc", headers: { "aa" => 1, "bb" => 2 })
-          .to_return(
-            status: 402,
-            body: "efg",
-            headers: { "Content-Type" => "application/json", "yoo" => "man" }
-          )
+      parsed_event = response.parsed_body["web_hook_event"]
+      expect(parsed_event["id"]).to eq(web_hook_event.id)
+      expect(parsed_event["status"]).to eq(402)
+
+      expect(JSON.parse(parsed_event["headers"])).to eq({ "aa" => "1", "bb" => "2" })
+      expect(parsed_event["payload"]).to eq("abc")
+
+      expect(JSON.parse(parsed_event["response_headers"])).to eq({ "content-type" => "application/json", "yoo" => "man" })
+      expect(parsed_event["response_body"]).to eq("efg")
+    end
+
+    it "doesn't emit the web hook if the payload URL resolves to an internal IP" do
+      FinalDestination::TestHelper.stub_to_fail do
         post "/admin/api/web_hooks/#{web_hook.id}/events/#{web_hook_event.id}/redeliver.json"
-        expect(response.status).to eq(200)
-
-        parsed_event = response.parsed_body["web_hook_event"]
-        expect(parsed_event["id"]).to eq(web_hook_event.id)
-        expect(parsed_event["status"]).to eq(402)
-
-        expect(JSON.parse(parsed_event["headers"])).to eq({ "aa" => "1", "bb" => "2" })
-        expect(parsed_event["payload"]).to eq("abc")
-
-        expect(JSON.parse(parsed_event["response_headers"])).to eq({ "content-type" => "application/json", "yoo" => "man" })
-        expect(parsed_event["response_body"]).to eq("efg")
       end
+      expect(response.status).to eq(200)
 
-      it "doesn't emit the web hook if the payload URL resolves to an internal IP" do
-        FinalDestination::TestHelper.stub_to_fail do
-          post "/admin/api/web_hooks/#{web_hook.id}/events/#{web_hook_event.id}/redeliver.json"
-        end
-        expect(response.status).to eq(200)
+      parsed_event = response.parsed_body["web_hook_event"]
+      expect(parsed_event["id"]).to eq(web_hook_event.id)
+      expect(parsed_event["response_headers"]).to eq({ error: I18n.t("webhooks.payload_url.blocked_or_internal") }.to_json)
+      expect(parsed_event["status"]).to eq(-1)
+      expect(parsed_event["response_body"]).to eq(nil)
+    end
 
-        parsed_event = response.parsed_body["web_hook_event"]
-        expect(parsed_event["id"]).to eq(web_hook_event.id)
-        expect(parsed_event["response_headers"]).to eq({ error: I18n.t("webhooks.payload_url.blocked_or_internal") }.to_json)
-        expect(parsed_event["status"]).to eq(-1)
-        expect(parsed_event["response_body"]).to eq(nil)
+    it "doesn't emit the web hook if the payload URL resolves to a blocked IP" do
+      FinalDestination::TestHelper.stub_to_fail do
+        post "/admin/api/web_hooks/#{web_hook.id}/events/#{web_hook_event.id}/redeliver.json"
       end
+      expect(response.status).to eq(200)
 
-      it "doesn't emit the web hook if the payload URL resolves to a blocked IP" do
-        FinalDestination::TestHelper.stub_to_fail do
-          post "/admin/api/web_hooks/#{web_hook.id}/events/#{web_hook_event.id}/redeliver.json"
-        end
-        expect(response.status).to eq(200)
-
-        parsed_event = response.parsed_body["web_hook_event"]
-        expect(parsed_event["id"]).to eq(web_hook_event.id)
-        expect(parsed_event["response_headers"]).to eq({ error: I18n.t("webhooks.payload_url.blocked_or_internal") }.to_json)
-        expect(parsed_event["status"]).to eq(-1)
-        expect(parsed_event["response_body"]).to eq(nil)
-      end
+      parsed_event = response.parsed_body["web_hook_event"]
+      expect(parsed_event["id"]).to eq(web_hook_event.id)
+      expect(parsed_event["response_headers"]).to eq({ error: I18n.t("webhooks.payload_url.blocked_or_internal") }.to_json)
+      expect(parsed_event["status"]).to eq(-1)
+      expect(parsed_event["response_body"]).to eq(nil)
     end
   end
 end