From d3e9a028f5003d806f9403f08775b0d7235a915d Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Wed, 2 Jun 2021 13:18:45 -0400
Subject: [PATCH] SECURITY: Do not allow unauthorized access to category edit
 UI (#13252)

---
 .../discourse/app/routes/edit-category.js     |  7 ++++++
 .../tests/acceptance/category-edit-test.js    | 23 +++++++++++++++++++
 .../tests/fixtures/category-fixtures.js       |  3 ++-
 app/serializers/category_serializer.rb        |  4 ++++
 spec/serializers/category_serializer_spec.rb  | 15 ++++++++++++
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/discourse/app/routes/edit-category.js b/app/assets/javascripts/discourse/app/routes/edit-category.js
index f9ebcf1255c..a3c43dbff59 100644
--- a/app/assets/javascripts/discourse/app/routes/edit-category.js
+++ b/app/assets/javascripts/discourse/app/routes/edit-category.js
@@ -11,6 +11,13 @@ export default DiscourseRoute.extend({
     );
   },
 
+  afterModel(model) {
+    if (!model.can_edit) {
+      this.replaceWith("/404");
+      return;
+    }
+  },
+
   titleToken() {
     return I18n.t("category.edit_dialog_title", {
       categoryName: this.currentModel.name,
diff --git a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js
index e9f21bf7680..8ed7bcfa584 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/category-edit-test.js
@@ -136,3 +136,26 @@ acceptance("Category Edit", function (needs) {
     );
   });
 });
+
+acceptance("Category Edit - no permission to edit", function (needs) {
+  needs.user();
+  needs.pretender((server, helper) => {
+    server.get("/c/bug/find_by_slug.json", () => {
+      return helper.response(200, {
+        category: {
+          id: 1,
+          name: "bug",
+          color: "e9dd00",
+          text_color: "000000",
+          slug: "bug",
+          can_edit: false,
+        },
+      });
+    });
+  });
+
+  test("returns 404", async function (assert) {
+    await visit("/c/bug/edit");
+    assert.equal(currentURL(), "/404");
+  });
+});
diff --git a/app/assets/javascripts/discourse/tests/fixtures/category-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/category-fixtures.js
index 1ee9cf607eb..6edf9ea190b 100644
--- a/app/assets/javascripts/discourse/tests/fixtures/category-fixtures.js
+++ b/app/assets/javascripts/discourse/tests/fixtures/category-fixtures.js
@@ -45,7 +45,8 @@ export default {
       name: "testing",
       color: "0088CC",
       text_color: "FFFFFF",
-      slug: "testing"
+      slug: "testing",
+      can_edit: true
     }
   }
 };
diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb
index 4789df1cba9..fe42e1172a7 100644
--- a/app/serializers/category_serializer.rb
+++ b/app/serializers/category_serializer.rb
@@ -45,6 +45,10 @@ class CategorySerializer < SiteCategorySerializer
     end
   end
 
+  def include_available_groups?
+    scope && scope.can_edit?(object)
+  end
+
   def available_groups
     Group.order(:name).pluck(:name) - group_permissions.map { |g| g[:group_name] }
   end
diff --git a/spec/serializers/category_serializer_spec.rb b/spec/serializers/category_serializer_spec.rb
index 754d673aa92..16f72f147fc 100644
--- a/spec/serializers/category_serializer_spec.rb
+++ b/spec/serializers/category_serializer_spec.rb
@@ -43,4 +43,19 @@ describe CategorySerializer do
       expect(json[:notification_level]).to eq(NotificationLevels.all[:watching])
     end
   end
+
+  describe "available groups" do
+    fab!(:user) { Fabricate(:user) }
+    fab!(:admin) { Fabricate(:admin) }
+
+    it "not included for a regular user" do
+      json = described_class.new(category, scope: Guardian.new(user), root: false).as_json
+      expect(json[:available_groups]).to eq(nil)
+    end
+
+    it "included for an admin" do
+      json = described_class.new(category, scope: Guardian.new(admin), root: false).as_json
+      expect(json[:available_groups]).to eq(Group.order(:name).pluck(:name) - ['everyone'])
+    end
+  end
 end