From 61c1af0124a086d034ad7d5f8d92557a445e107a Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nenciu.bianca@gmail.com>
Date: Mon, 9 Mar 2020 22:04:05 +0200
Subject: [PATCH] SECURITY: Ensure user can see group and group members

---
 app/controllers/directory_items_controller.rb    |  7 ++++++-
 spec/requests/directory_items_controller_spec.rb | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/controllers/directory_items_controller.rb b/app/controllers/directory_items_controller.rb
index e6d6ea4b3c9..5cf99b710cd 100644
--- a/app/controllers/directory_items_controller.rb
+++ b/app/controllers/directory_items_controller.rb
@@ -12,7 +12,12 @@ class DirectoryItemsController < ApplicationController
     result = DirectoryItem.where(period_type: period_type).includes(:user)
 
     if params[:group]
-      result = result.includes(user: :groups).where(users: { groups: { name: params[:group] } })
+      group = Group.find_by(name: params[:group])
+      raise Discourse::InvalidParameters.new(:group) if group.blank?
+      guardian.ensure_can_see!(group)
+      guardian.ensure_can_see_group_members!(group)
+
+      result = result.includes(user: :groups).where(users: { groups: { id: group.id } })
     else
       result = result.includes(user: :primary_group)
     end
diff --git a/spec/requests/directory_items_controller_spec.rb b/spec/requests/directory_items_controller_spec.rb
index 41f42b9c452..6d1e777e685 100644
--- a/spec/requests/directory_items_controller_spec.rb
+++ b/spec/requests/directory_items_controller_spec.rb
@@ -103,5 +103,20 @@ describe DirectoryItemsController do
       expect(json['directory_items'][0]['user']['username']).to eq(evil_trout.username) | eq(stage_user.username)
       expect(json['directory_items'][1]['user']['username']).to eq(evil_trout.username) | eq(stage_user.username)
     end
+
+    it "checks group permissions" do
+      group.update!(visibility_level: Group.visibility_levels[:members])
+
+      sign_in(evil_trout)
+      get '/directory_items.json', params: { period: 'all', group: group.name }
+      expect(response.status).to eq(200)
+
+      get '/directory_items.json', params: { period: 'all', group: 'not a group' }
+      expect(response.status).to eq(400)
+
+      sign_in(user)
+      get '/directory_items.json', params: { period: 'all', group: group.name }
+      expect(response.status).to eq(403)
+    end
   end
 end