diff --git a/app/controllers/direct_messages_controller.rb b/app/controllers/direct_messages_controller.rb index b105550ab..706d6fea2 100644 --- a/app/controllers/direct_messages_controller.rb +++ b/app/controllers/direct_messages_controller.rb @@ -1,31 +1,40 @@ class DirectMessagesController < ApplicationController - load_and_authorize_resource + before_action :authenticate_user! + load_and_authorize_resource :user, instance_name: :receiver + before_action :check_slug + load_resource through: :receiver, through_association: :direct_messages_received + authorize_resource except: :new def new - @receiver = User.find(params[:user_id]) - @direct_message = DirectMessage.new(receiver: @receiver) + authorize! :new, @direct_message, message: t("users.direct_messages.new.verified_only", + verify_account: helpers.link_to_verify_account) end def create - @sender = current_user - @receiver = User.find(params[:user_id]) + @direct_message.sender = current_user - @direct_message = DirectMessage.new(parsed_params) if @direct_message.save Mailer.direct_message_for_receiver(@direct_message).deliver_later Mailer.direct_message_for_sender(@direct_message).deliver_later - redirect_to [@receiver, @direct_message], notice: I18n.t("flash.actions.create.direct_message") + + redirect_to user_direct_message_path(@receiver, @direct_message), + notice: I18n.t("flash.actions.create.direct_message") else render :new end end def show - @direct_message = DirectMessage.find(params[:id]) end private + def check_slug + slug = params[:user_id].split("-", 2)[1] + + raise ActiveRecord::RecordNotFound unless @receiver.slug == slug.to_s + end + def direct_message_params params.require(:direct_message).permit(allowed_params) end @@ -33,8 +42,4 @@ class DirectMessagesController < ApplicationController def allowed_params [:title, :body] end - - def parsed_params - direct_message_params.merge(sender: @sender, receiver: @receiver) - end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8ccee9692..f38464d75 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,5 +1,6 @@ class UsersController < ApplicationController load_and_authorize_resource + before_action :check_slug helper_method :valid_interests_access? def show @@ -8,6 +9,12 @@ class UsersController < ApplicationController private + def check_slug + slug = params[:id].split("-", 2)[1] + + raise ActiveRecord::RecordNotFound unless @user.slug == slug.to_s + end + def valid_interests_access?(user) user.public_interests || user == current_user end 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/models/user.rb b/app/models/user.rb index b7aa66e3f..d9fc28938 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -433,6 +433,14 @@ class User < ApplicationRecord (Tenant.current_secrets.dig(:security, :lockable, :unlock_in) || 1).to_f.hours end + def to_param + "#{id}-#{slug}" + end + + def slug + username.to_s.parameterize + end + private def clean_document_number 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) %> - <% if not current_user %> -
-

- <%= sanitize(t("users.login_to_continue", - signin: link_to_signin, - signup: link_to_signup)) %> -

-
- <% elsif not @receiver.email_on_direct_message? %> -
-

- <%= t("users.direct_messages.new.direct_messages_bloqued") %> -

-
- <% 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 @@ <% end %> <% else %> -
+

- <%= sanitize(t("users.direct_messages.new.verified_only", - verify_account: link_to_verify_account)) %> + <%= t("users.direct_messages.new.direct_messages_bloqued") %>

<% end %> diff --git a/spec/controllers/direct_messages_controller_spec.rb b/spec/controllers/direct_messages_controller_spec.rb new file mode 100644 index 000000000..9cc8cc6d7 --- /dev/null +++ b/spec/controllers/direct_messages_controller_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +describe DirectMessagesController do + before { sign_in(create :user, :level_two) } + + describe "GET new" do + let!(:user) { create(:user, username: "James Jameson") } + + it "finds a user by ID and slug" do + get :new, params: { user_id: "#{user.id}-james-jameson" } + + expect(response).to be_successful + end + + it "does not find a user by just an ID" do + expect do + get :new, params: { user_id: user.id } + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user by just a slug" do + expect do + get :new, params: { user_id: "james-jameson" } + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user with the wrong slug" do + expect do + get :new, params: { user_id: "#{user.id}-James Jameson" } + end.to raise_error ActiveRecord::RecordNotFound + end + end + + describe "POST create" do + let!(:user) { create(:user, username: "James Jameson") } + let(:message_params) { { direct_message: { title: "Hello!", message: "How are you doing?" }} } + + it "finds a user by ID and slug" do + post :create, params: message_params.merge(user_id: "#{user.id}-james-jameson") + + expect(response).to be_successful + end + + it "does not find a user by just an ID" do + expect do + post :create, params: message_params.merge(user_id: user.id) + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user by just a slug" do + expect do + post :create, params: message_params.merge(user_id: "james-jameson") + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user with the wrong slug" do + expect do + post :create, params: message_params.merge(user_id: "#{user.id}-James Jameson") + end.to raise_error ActiveRecord::RecordNotFound + end + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb new file mode 100644 index 000000000..4f7e28740 --- /dev/null +++ b/spec/controllers/users_controller_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +describe UsersController do + describe "GET show" do + let!(:user) { create(:user, username: "James Jameson") } + + it "finds a user by ID and slug" do + get :show, params: { id: "#{user.id}-james-jameson" } + + expect(response).to be_successful + end + + it "does not find a user by just an ID" do + expect do + get :show, params: { id: user.id } + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user by just a slug" do + expect do + get :show, params: { id: "james-jameson" } + end.to raise_error ActiveRecord::RecordNotFound + end + + it "does not find a user with the wrong slug" do + expect do + get :show, params: { id: "#{user.id}-James Jameson" } + end.to raise_error ActiveRecord::RecordNotFound + end + + it "finds users without username by ID" do + user.erase + + get :show, params: { id: user.id } + + expect(response).to be_successful + end + end +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