From 9ca314c8e919ad54b3be68d851457383283f185d Mon Sep 17 00:00:00 2001 From: Eric Schultz Date: Thu, 30 Aug 2018 15:04:11 -0500 Subject: [PATCH] Fix bug where p2p campaign editors couldn't add offsite donations --- .../nonprofits/donations_controller.rb | 22 ++- .../support/api_shared_user_verification.rb | 179 ++++++++++++++++++ spec/controllers/nonprofits/donations_spec.rb | 26 ++- .../support/new_controller_user_context.rb | 53 ++++++ .../contexts/general_shared_user_context.rb | 134 +++++++++++++ 5 files changed, 409 insertions(+), 5 deletions(-) create mode 100644 spec/api/support/api_shared_user_verification.rb create mode 100644 spec/controllers/support/new_controller_user_context.rb create mode 100644 spec/support/contexts/general_shared_user_context.rb diff --git a/app/controllers/nonprofits/donations_controller.rb b/app/controllers/nonprofits/donations_controller.rb index eb33d2f1..53d37be8 100644 --- a/app/controllers/nonprofits/donations_controller.rb +++ b/app/controllers/nonprofits/donations_controller.rb @@ -3,7 +3,8 @@ module Nonprofits class DonationsController < ApplicationController include NonprofitHelper - before_filter :authenticate_nonprofit_user!, only: [:index, :update, :create_offsite] + before_filter :authenticate_nonprofit_user!, only: [:index, :update] + before_filter :authenticate_campaign_editor!, only: [:create_offsite] # get /nonprofit/:nonprofit_id/donations def index @@ -60,5 +61,24 @@ module Nonprofits json_saved UpdateDonation.from_followup(donation, params[:donation]) end + # this is a special, weird case + private + + def current_campaign + if !@campaign && params[:donation] && params[:donation][:campaign_id] + @campaign = Campaign.where('id = ? ', params[:donation][:campaign_id]).first + end + return @campaign + end + + def current_campaign_editor? + !params[:preview] && (current_nonprofit_user? || (current_campaign && current_role?(:campaign_editor, current_campaign.id)) || current_role?(:super_admin)) + end + + def authenticate_campaign_editor! + unless current_campaign_editor? + block_with_sign_in 'You need to be a campaign editor to do that.' + end + end end end diff --git a/spec/api/support/api_shared_user_verification.rb b/spec/api/support/api_shared_user_verification.rb new file mode 100644 index 00000000..ade50170 --- /dev/null +++ b/spec/api/support/api_shared_user_verification.rb @@ -0,0 +1,179 @@ +# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later +require 'controllers/support/general_shared_user_context' +RSpec.shared_context :api_shared_user_verification do + include_context :general_shared_user_context + let(:user_as_np_admin) { + __create_admin(nonprofit) + } + + let(:user_as_other_np_admin) { + __create_admin(other_nonprofit) + } + + let(:user_as_np_associate){ + __create_associate(nonprofit) + } + + let(:user_as_other_np_associate){ + __create_associate(other_nonprofit) + } + + let(:unauth_user) { + force_create(:user) + } + + + let(:campaign_editor) { + __create(:campaign_editor, campaign) + } + + let(:confirmed_user){ + force_create(:user, confirmed_at: Time.current) + } + + let(:event_editor) { + __create(:event_editor,event) + } + + let(:super_admin) { + __create(:super_admin, other_nonprofit) + } + + let(:user_with_profile) { + u = force_create(:user) + force_create(:profile, user: u) + u + } + + let(:all_users) do + {:user_as_np_admin => user_as_np_admin, + :user_as_other_np_admin => user_as_other_np_admin, + :user_as_np_associate => user_as_np_associate, + :user_as_other_np_associate => user_as_other_np_associate, + :unauth_user => unauth_user, + :campaign_editor => campaign_editor, + :event_editor => event_editor, + :super_admin => super_admin, + :user_with_profile => user_with_profile + } + end + + let(:roles__open_to_all) do + [nil, :user_as_np_admin, + :user_as_other_np_admin, + :user_as_np_associate, + :user_as_other_np_associate, + :unauth_user, + :campaign_editor, + :event_editor, + :super_admin, + :user_with_profile + ] + end + + let(:roles__open_to_np_associate) do + [:user_as_np_admin, + + :user_as_np_associate, + + :super_admin + + ] + end + + def __create(name, host) + u = force_create(:user) + force_create(:role, user: u, name: name, host:host) + u + end + + def __create_admin(host) + u = force_create(:user) + force_create(:role, user: u, name: :nonprofit_admin, host:host) + u + end + + def __create_associate(host) + u = force_create(:user) + force_create(:role, user: u, name: :nonprofit_associate, host:host) + u + end + + def sign_in(user_to_signin) + post_via_redirect 'users/sign_in', 'user[email]' => user_to_signin.email, 'user[password]' => user_to_signin.password, format: "json" + end + + def sign_out + send(:get, 'users/sign_out') + end + + def send(method, *args) + case method + when :get + return xhr(:get, *args) + when :post + return xhr(:post, *args) + when :delete + return xhr(:delete, *args) + when :put + return xhr(:put, *args) + end + end + + def accept(user_to_signin:, method:, action:, args:) + new_user = user_to_signin + if (user_to_signin != nil && user_to_signin.is_a?(OpenStruct)) + new_user = user_to_signin.value + end + sign_in new_user if new_user + # allows us to run the helpers but ignore what the controller action does + # + send(method, action, args) + expect(response.status).to eq(200), "expcted success for user: #{(user_to_signin.is_a?(OpenStruct) ? user_to_signin.key.to_s + ":" : "")} #{new_user&.attributes}" + sign_out + end + + def reject(user_to_signin:, method:, action:, args:) + + new_user = user_to_signin + if (user_to_signin != nil && user_to_signin.is_a?(OpenStruct)) + new_user = user_to_signin.value + end + sign_in new_user if new_user + send(method, action, args) + expect(response.status).to eq(401), "expected failure for user: #{(user_to_signin.is_a?(OpenStruct) ? user_to_signin.key.to_s + ":" : "")} #{new_user&.attributes}" + sign_out + end + + alias_method :redirects_to, :reject + + def run_authorization_tests(details, &block) + @method = details[:method] + @successful_users = details[:successful_users] + @action = details[:action] + @block_to_get_arguments_to_run = block || ->(_) {} #no-op + accept_test_for_nil = false + all_users.each do |k,v| + os = OpenStruct.new + os.key = k + os.value = v + + if k.nil? + accept(user_to_signin: nil, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + accept_test_for_nil = true + end + if @successful_users.include? k + accept(user_to_signin: os, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + else + reject(user_to_signin: os, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + end + end + + unless accept_test_for_nil + reject(user_to_signin: nil, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(nil)) + end + end + +end + + diff --git a/spec/controllers/nonprofits/donations_spec.rb b/spec/controllers/nonprofits/donations_spec.rb index 8dee197e..24e45032 100644 --- a/spec/controllers/nonprofits/donations_spec.rb +++ b/spec/controllers/nonprofits/donations_spec.rb @@ -1,19 +1,21 @@ # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later require 'rails_helper' require 'controllers/support/shared_user_context' +require 'controllers/support/new_controller_user_context' +require 'support/contexts/shared_donation_charge_context' describe Nonprofits::DonationsController, :type => :controller do - include_context :shared_user_context + describe 'rejects unauthenticated users' do describe 'index' do + include_context :shared_user_context include_context :open_to_np_associate, :get, :index, nonprofit_id: :__our_np end - describe 'create_offsite' do - include_context :open_to_np_associate, :post, :create_offsite, nonprofit_id: :__our_np - end + describe 'update' do + include_context :shared_user_context include_context :open_to_np_associate, :put, :update, nonprofit_id: :__our_np end @@ -28,4 +30,20 @@ describe Nonprofits::DonationsController, :type => :controller do include_context :open_to_all, :put, :followup, nonprofit_id: :__our_np end end +end + +describe 'Nonprofits::DonationsController::create_offsite', :type => :request do + describe 'create_offsite' do + include_context :shared_donation_charge_context + include_context :new_controller_user_context + + it 'reject non-campaign editors (and np authorized folks)' do + run_authorization_tests({method: :post, action: "/nonprofits/#{nonprofit.id}/donations/create_offsite", + successful_users: roles__open_to_campaign_editor}) do |_| + {nonprofit_id: nonprofit.id, + donation: {campaign_id: campaign.id}} + end + end + #include_context :open_to_np_associate, :post, :create_offsite, nonprofit_id: :__our_np + end end \ No newline at end of file diff --git a/spec/controllers/support/new_controller_user_context.rb b/spec/controllers/support/new_controller_user_context.rb new file mode 100644 index 00000000..7c08bf81 --- /dev/null +++ b/spec/controllers/support/new_controller_user_context.rb @@ -0,0 +1,53 @@ +# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later +require 'support/contexts/general_shared_user_context' + +RSpec.shared_context :new_controller_user_context do + include_context :general_shared_user_context + + def sign_in(user_to_signin) + post_via_redirect 'users/sign_in', 'user[email]' => user_to_signin.email, 'user[password]' => user_to_signin.password, format: "json" + end + + def sign_out + send(:get, 'users/sign_out') + end + + def send(method, *args) + case method + when :get + return xhr(:get, *args) + when :post + return xhr(:post, *args) + when :delete + return xhr(:delete, *args) + when :put + return xhr(:put, *args) + end + end + + def accept(user_to_signin:, method:, action:, args:) + new_user = user_to_signin + if (user_to_signin != nil && user_to_signin.is_a?(OpenStruct)) + new_user = user_to_signin.value + end + sign_in new_user if new_user + # allows us to run the helpers but ignore what the controller action does + # expect_any_instance_of(described_class).to receive(action).and_return(ActionController::TestResponse.new(200)) + # expect_any_instance_of(described_class).to receive(:render).and_return(nil) + send(method, action, args) + expect(response.status).to_not eq(302), "expected success for user: #{(user_to_signin.is_a?(OpenStruct) ? user_to_signin.key.to_s + ":" : "")} #{new_user&.attributes}" + sign_out + end + + def reject(user_to_signin:, method:, action:, args:) + + new_user = user_to_signin + if (user_to_signin != nil && user_to_signin.is_a?(OpenStruct)) + new_user = user_to_signin.value + end + sign_in new_user if new_user + send(method, action, args) + expect(response.status).to eq(302), "expected failure for user: #{(user_to_signin.is_a?(OpenStruct) ? user_to_signin.key.to_s + ":" : "")} #{new_user&.attributes}" + sign_out + end +end \ No newline at end of file diff --git a/spec/support/contexts/general_shared_user_context.rb b/spec/support/contexts/general_shared_user_context.rb new file mode 100644 index 00000000..fce26940 --- /dev/null +++ b/spec/support/contexts/general_shared_user_context.rb @@ -0,0 +1,134 @@ +# License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later +RSpec.shared_context :general_shared_user_context do + let(:user_as_np_admin) { + __create_admin(nonprofit) + } + + let(:user_as_other_np_admin) { + __create_admin(other_nonprofit) + } + + let(:user_as_np_associate){ + __create_associate(nonprofit) + } + + let(:user_as_other_np_associate){ + __create_associate(other_nonprofit) + } + + let(:unauth_user) { + force_create(:user) + } + + + let(:campaign_editor) { + __create(:campaign_editor, campaign) + } + + let(:confirmed_user){ + force_create(:user, confirmed_at: Time.current) + } + + let(:event_editor) { + __create(:event_editor,event) + } + + let(:super_admin) { + __create(:super_admin, other_nonprofit) + } + + let(:user_with_profile) { + u = force_create(:user) + force_create(:profile, user: u) + u + } + + let(:all_users) do + {:user_as_np_admin => user_as_np_admin, + :user_as_other_np_admin => user_as_other_np_admin, + :user_as_np_associate => user_as_np_associate, + :user_as_other_np_associate => user_as_other_np_associate, + :unauth_user => unauth_user, + :campaign_editor => campaign_editor, + :event_editor => event_editor, + :super_admin => super_admin, + :user_with_profile => user_with_profile + } + end + + let(:roles__open_to_all) do + [nil, :user_as_np_admin, + :user_as_other_np_admin, + :user_as_np_associate, + :user_as_other_np_associate, + :unauth_user, + :campaign_editor, + :event_editor, + :super_admin, + :user_with_profile + ] + end + + let(:roles__open_to_np_associate) do + [:user_as_np_admin, + + :user_as_np_associate, + + :super_admin + + ] + end + + let(:roles__open_to_campaign_editor) do + [:user_as_np_admin, + :user_as_np_associate, + :campaign_editor, + :super_admin + ] + end + + def __create(name, host) + u = force_create(:user) + force_create(:role, user: u, name: name, host:host) + u + end + + def __create_admin(host) + u = force_create(:user) + force_create(:role, user: u, name: :nonprofit_admin, host:host) + u + end + + def __create_associate(host) + u = force_create(:user) + force_create(:role, user: u, name: :nonprofit_associate, host:host) + u + end + + def run_authorization_tests(details, &block) + @method = details[:method] + @successful_users = details[:successful_users] + @action = details[:action] + @block_to_get_arguments_to_run = block || ->(_) {} #no-op + accept_test_for_nil = false + all_users.each do |k,v| + os = OpenStruct.new + os.key = k + os.value = v + + if k.nil? + accept(user_to_signin: nil, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + accept_test_for_nil = true + end + if @successful_users.include? k + accept(user_to_signin: os, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + else + reject(user_to_signin: os, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(v)) + end + end + + unless accept_test_for_nil + reject(user_to_signin: nil, method:@method, action: @action, args: @block_to_get_arguments_to_run.call(nil)) + end + end +end \ No newline at end of file