From 2db807baa76575932d42764d5dd9263757edd09b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 24 Nov 2023 16:23:33 +0100
Subject: [PATCH] Restrict access to the "new" direct message action

This way only verified users will be able to access this page, which
shows the username of the receiver of the direct message. With this,
it's no longer possible for unverified users to browse direct message
URLs in order to collect usernames from every user.
---
 app/controllers/direct_messages_controller.rb |  6 +++++-
 app/models/abilities/everyone.rb              |  1 -
 app/views/direct_messages/new.html.erb        | 21 +++----------------
 spec/models/abilities/common_spec.rb          |  2 +-
 spec/system/direct_messages_spec.rb           |  6 +++---
 5 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/app/controllers/direct_messages_controller.rb b/app/controllers/direct_messages_controller.rb
index 33996565c..d680559c3 100644
--- a/app/controllers/direct_messages_controller.rb
+++ b/app/controllers/direct_messages_controller.rb
@@ -1,8 +1,12 @@
 class DirectMessagesController < ApplicationController
+  before_action :authenticate_user!
   load_and_authorize_resource :user, instance_name: :receiver
-  load_and_authorize_resource through: :receiver, through_association: :direct_messages_received
+  load_resource through: :receiver, through_association: :direct_messages_received
+  authorize_resource except: :new
 
   def new
+    authorize! :new, @direct_message, message: t("users.direct_messages.new.verified_only",
+                                                 verify_account: helpers.link_to_verify_account)
   end
 
   def create
diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb
index 252ce3896..fc77948bc 100644
--- a/app/models/abilities/everyone.rb
+++ b/app/models/abilities/everyone.rb
@@ -18,7 +18,6 @@ module Abilities
       can :read_results, Budget, id: Budget.finished.results_enabled.ids
       can :read_stats, Budget, id: Budget.valuating_or_later.stats_enabled.ids
       can :read_executions, Budget, phase: "finished"
-      can :new, DirectMessage
       can [:read, :debate, :draft_publication, :allegations, :result_publication,
            :proposals, :milestones], Legislation::Process, published: true
       can :summary, Legislation::Process,
diff --git a/app/views/direct_messages/new.html.erb b/app/views/direct_messages/new.html.erb
index 1343f6363..a945a03a5 100644
--- a/app/views/direct_messages/new.html.erb
+++ b/app/views/direct_messages/new.html.erb
@@ -6,21 +6,7 @@
       <%= t("users.direct_messages.new.title", receiver: @receiver.name) %>
     </h1>
 
-    <% if not current_user %>
-      <div class="callout primary">
-        <p>
-          <%= sanitize(t("users.login_to_continue",
-                         signin: link_to_signin,
-                         signup: link_to_signup)) %>
-        </p>
-      </div>
-    <% elsif not @receiver.email_on_direct_message? %>
-      <div class="callout primary">
-        <p>
-          <%= t("users.direct_messages.new.direct_messages_bloqued") %>
-        </p>
-      </div>
-    <% elsif can? :create, @direct_message %>
+    <% if @receiver.email_on_direct_message? %>
       <%= form_for [@receiver, @direct_message] do |f| %>
         <%= render "shared/errors", resource: @direct_message %>
 
@@ -32,10 +18,9 @@
         </div>
       <% end %>
     <% else %>
-      <div class="callout warning">
+      <div class="callout primary">
         <p>
-          <%= sanitize(t("users.direct_messages.new.verified_only",
-                         verify_account: link_to_verify_account)) %>
+          <%= t("users.direct_messages.new.direct_messages_bloqued") %>
         </p>
       </div>
     <% end %>
diff --git a/spec/models/abilities/common_spec.rb b/spec/models/abilities/common_spec.rb
index 392d4367b..9f80ee67c 100644
--- a/spec/models/abilities/common_spec.rb
+++ b/spec/models/abilities/common_spec.rb
@@ -92,7 +92,7 @@ describe Abilities::Common do
   it { should_not be_able_to(:comment_as_administrator, proposal) }
   it { should_not be_able_to(:comment_as_moderator, proposal)     }
 
-  it { should     be_able_to(:new,    DirectMessage) }
+  it { should_not be_able_to(:new,    DirectMessage) }
   it { should_not be_able_to(:create, DirectMessage) }
   it { should_not be_able_to(:show,   DirectMessage) }
 
diff --git a/spec/system/direct_messages_spec.rb b/spec/system/direct_messages_spec.rb
index dd008174e..74193ea6b 100644
--- a/spec/system/direct_messages_spec.rb
+++ b/spec/system/direct_messages_spec.rb
@@ -54,7 +54,7 @@ describe "Direct messages" do
       visit new_user_direct_message_path(receiver)
 
       expect(page).to have_content "To send a private message verify your account"
-      expect(page).not_to have_link "Send private message"
+      expect(page).to have_current_path root_path
     end
 
     scenario "User not logged in" do
@@ -62,8 +62,8 @@ describe "Direct messages" do
 
       visit new_user_direct_message_path(receiver)
 
-      expect(page).to have_content "You must sign in or sign up to continue."
-      expect(page).not_to have_link "Send private message"
+      expect(page).to have_content "You must sign in or register to continue."
+      expect(page).to have_current_path new_user_session_path
     end
 
     scenario "Accessing form directly" do