From b609f6c11c4f5bff678836bc240e58d6dd122feb Mon Sep 17 00:00:00 2001
From: Andrei Prigorshnev <a.prigorshnev@gmail.com>
Date: Wed, 29 Sep 2021 16:24:28 +0400
Subject: [PATCH] FIX: restrict other user's notification routes (#14442)

It was possible to see notifications of other users using routes:
- notifications/responses
- notifications/likes-received
- notifications/mentions
- notifications/edits

We weren't showing anything private (like notifications about private messages), only things that're publicly available in other places. But anyway, it feels strange that it's possible to look at notifications of someone else. Additionally, there is a risk that we can unintentionally leak something on these pages in the future.

This commit restricts these routes.
---
 app/controllers/user_actions_controller.rb    |  6 +--
 app/models/user_action.rb                     | 10 ++++
 lib/guardian/user_guardian.rb                 |  5 ++
 spec/requests/user_actions_controller_spec.rb | 49 +++++++++++++++++++
 4 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/app/controllers/user_actions_controller.rb b/app/controllers/user_actions_controller.rb
index 2c133273cf3..bee259e9518 100644
--- a/app/controllers/user_actions_controller.rb
+++ b/app/controllers/user_actions_controller.rb
@@ -1,18 +1,18 @@
 # frozen_string_literal: true
 
 class UserActionsController < ApplicationController
-
   def index
     params.require(:username)
     params.permit(:filter, :offset, :acting_username, :limit)
 
     user = fetch_user_from_params(include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts))
-    raise Discourse::NotFound unless guardian.can_see_profile?(user)
-
     offset = [0, params[:offset].to_i].max
     action_types = (params[:filter] || "").split(",").map(&:to_i)
     limit = params.fetch(:limit, 30).to_i
 
+    raise Discourse::NotFound unless guardian.can_see_profile?(user)
+    raise Discourse::NotFound unless guardian.can_see_user_actions?(user, action_types)
+
     opts = {
       user_id: user.id,
       user: user,
diff --git a/app/models/user_action.rb b/app/models/user_action.rb
index 732ec9387ae..0332c9a7141 100644
--- a/app/models/user_action.rb
+++ b/app/models/user_action.rb
@@ -55,6 +55,16 @@ class UserAction < ActiveRecord::Base
       assigned: 16)
   end
 
+  def self.private_types
+    @private_types ||= [
+      WAS_LIKED,
+      RESPONSE,
+      MENTION,
+      QUOTE,
+      EDIT
+    ]
+  end
+
   def self.last_action_in_topic(user_id, topic_id)
     UserAction.where(user_id: user_id,
                      target_topic_id: topic_id,
diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb
index ef13588cef2..564bdcfa352 100644
--- a/lib/guardian/user_guardian.rb
+++ b/lib/guardian/user_guardian.rb
@@ -129,6 +129,11 @@ module UserGuardian
     true
   end
 
+  def can_see_user_actions?(user, action_types)
+    return true if !@user.anonymous? && (@user.id == user.id || is_admin?)
+    (action_types & UserAction.private_types).empty?
+  end
+
   def allowed_user_field_ids(user)
     @allowed_user_field_ids ||= {}
 
diff --git a/spec/requests/user_actions_controller_spec.rb b/spec/requests/user_actions_controller_spec.rb
index a12af23d6a8..c70ab0ca7a0 100644
--- a/spec/requests/user_actions_controller_spec.rb
+++ b/spec/requests/user_actions_controller_spec.rb
@@ -99,5 +99,54 @@ describe UserActionsController do
       end
     end
 
+    context "other users' activity" do
+      fab!(:another_user) { Fabricate(:user) }
+
+      UserAction.private_types.each do |action_type|
+        action_name = UserAction.types.key(action_type)
+        it "anonymous users cannot list other users' actions of type: #{action_name}" do
+          list_and_check(action_type, 404)
+        end
+      end
+
+      UserAction.private_types.each do |action_type|
+        fab!(:user) { Fabricate(:user) }
+        action_name = UserAction.types.key(action_type)
+
+        it "logged in users cannot list other users' actions of type: #{action_name}" do
+          sign_in(user)
+          list_and_check(action_type, 404)
+        end
+      end
+
+      UserAction.private_types.each do |action_type|
+        fab!(:moderator) { Fabricate(:moderator) }
+        action_name = UserAction.types.key(action_type)
+
+        it "moderators cannot list other users' actions of type: #{action_name}" do
+          sign_in(moderator)
+          list_and_check(action_type, 404)
+        end
+      end
+
+      UserAction.private_types.each do |action_type|
+        fab!(:admin) { Fabricate(:admin) }
+        action_name = UserAction.types.key(action_type)
+
+        it "admins can list other users' actions of type: #{action_name}" do
+          sign_in(admin)
+          list_and_check(action_type, 200)
+        end
+      end
+
+      def list_and_check(action_type, expected_response)
+        get "/user_actions.json", params: {
+          filter: action_type,
+          username: another_user.username
+        }
+
+        expect(response.status).to eq(expected_response)
+      end
+    end
   end
 end