Merge pull request #5089 from coalest/4511-need-a-graceful-landing-if-someone-goes-to-a-screen-they-dont-have-access-to-while-logged-in

awwaiid · web-flow · commit e74739203b17 · 2025-03-14T18:28:48.000-04:00
Redirect to partners dashboard with flash message for org-related pages
diff --git a/app/controllers/account_requests_controller.rb b/app/controllers/account_requests_controller.rb
@@ -1,6 +1,7 @@
 class AccountRequestsController < ApplicationController
   skip_before_action :authorize_user
   skip_before_action :authenticate_user!
+  skip_before_action :require_organization
 
   before_action :set_account_request_from_token, only: [:received, :confirmation, :confirm]
 
diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb
@@ -1,6 +1,7 @@
 # [Super Admin] This is the parent controller for the Admin namespace, and also provides the Dashboard data for SuperAdmins.
 class AdminController < ApplicationController
   before_action :require_admin
+  skip_before_action :require_organization
 
   def require_admin
     verboten! unless current_user.has_cached_role?(Role::SUPER_ADMIN)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -6,6 +6,7 @@ class ApplicationController < ActionController::Base
   protect_from_forgery with: :exception
   before_action :authenticate_user!
   before_action :authorize_user
+  before_action :require_organization, unless: :devise_controller?
   before_action :log_active_user
   before_action :swaddled
   before_action :configure_permitted_parameters, if: :devise_controller?
@@ -82,6 +83,15 @@ def authorize_admin
       current_user.has_cached_role?(Role::ORG_ADMIN, current_organization)
   end
 
+  def require_organization
+    return if current_organization
+
+    respond_to do |format|
+      format.html { redirect_to dashboard_path_from_current_role, flash: {error: "That screen is not available. Please try again as a bank."} }
+      format.json { render body: nil, status: :forbidden }
+    end
+  end
+
   def log_active_user
     if current_user && should_update_last_request_at?
       # we don't want the user record to validate or run callbacks when we're tracking activity
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
@@ -10,6 +10,7 @@ class DistributionsController < ApplicationController
   before_action :enable_turbo!, only: %i[new show]
   skip_before_action :authenticate_user!, only: %i(calendar)
   skip_before_action :authorize_user, only: %i(calendar)
+  skip_before_action :require_organization, only: %i(calendar)
 
   def print
     @distribution = Distribution.find(params[:id])
diff --git a/app/controllers/partners/base_controller.rb b/app/controllers/partners/base_controller.rb
@@ -3,6 +3,7 @@ class BaseController < ApplicationController
     layout 'partners/application'
 
     before_action :require_partner
+    skip_before_action :require_organization
 
     private
 
@@ -11,11 +12,11 @@ def redirect_to_root
     end
 
     def require_partner
-      unless current_partner
-        respond_to do |format|
-          format.html { redirect_to dashboard_path, flash: {error: "Logged in user is not set up as a 'partner'."} }
-          format.json { render body: nil, status: :forbidden }
-        end
+      return if current_partner
+
+      respond_to do |format|
+        format.html { redirect_to dashboard_path, flash: {error: "That screen is not available. Please try again as a partner."} }
+        format.json { render body: nil, status: :forbidden }
       end
     end
 
diff --git a/app/controllers/partners/requests_controller.rb b/app/controllers/partners/requests_controller.rb
@@ -103,7 +103,7 @@ def require_partner_or_org_admin
 
     def redirect_invalid_user
       respond_to do |format|
-        format.html { redirect_to dashboard_path, flash: {error: "Logged in user is not set up as a 'partner'."} }
+        format.html { redirect_to dashboard_path, flash: {error: "That screen is not available. Please try again as a partner."} }
         format.json { render body: nil, status: :forbidden }
       end
     end
diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb
@@ -4,6 +4,7 @@
 class PartnersController < ApplicationController
   include Importable
   before_action :validate_user_role, only: :show
+  skip_before_action :require_organization, only: :show
 
   def index
     @partners = current_organization.partners.includes(:partner_group).alphabetized
diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb
@@ -2,6 +2,7 @@
 class StaticController < ApplicationController
   skip_before_action :authorize_user
   skip_before_action :authenticate_user!
+  skip_before_action :require_organization
   skip_before_action :log_active_user
 
   layout false
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
@@ -1,5 +1,7 @@
 # Provides scope-limited access to viewing the data of other users
 class UsersController < ApplicationController
+  skip_before_action :require_organization, only: [:switch_to_role]
+
   def index
     @users = current_organization.users
   end
diff --git a/spec/requests/adjustments_requests_spec.rb b/spec/requests/adjustments_requests_spec.rb
@@ -48,6 +48,8 @@
           expect(subject).to be_successful
         end
 
+        include_examples "restricts access to organization users/admins"
+
         context 'when filtering by date' do
           let!(:old_adjustment) { create(:adjustment, created_at: 7.days.ago) }
           let!(:new_adjustment) { create(:adjustment, created_at: 1.day.ago) }
diff --git a/spec/requests/dashboard_requests_spec.rb b/spec/requests/dashboard_requests_spec.rb
@@ -8,12 +8,16 @@
     end
 
     describe "GET #show" do
+      subject { get dashboard_path }
+
       it "returns http success" do
         get dashboard_path
         expect(response).to be_successful
         expect(response.body).not_to include('switch_to_partner_role')
       end
 
+      include_examples "restricts access to organization users/admins"
+
       context 'with both roles' do
         it 'should include the switch link' do
           partner = FactoryBot.create(:partner)
diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb
@@ -31,6 +31,8 @@
     end
 
     describe "GET #print" do
+      subject { get print_distribution_path(id: create(:distribution).id) }
+
       it "returns http success" do
         get print_distribution_path(id: create(:distribution).id)
         expect(response).to be_successful
@@ -52,6 +54,8 @@
           expect(response).to be_successful
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "GET #reclaim" do
@@ -62,6 +66,7 @@
     end
 
     describe "GET #index" do
+      subject { get distributions_path }
       let(:item) { create(:item, value_in_cents: 100, organization: organization) }
       let!(:distribution) { create(:distribution, :with_items, :past, item: item, item_quantity: 10, organization: organization) }
 
@@ -204,9 +209,13 @@
           expect(distribution_rows.count).to eq(1)
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "POST #create" do
+      subject { post distributions_path(distribution:) }
+
       let!(:storage_location) { create(:storage_location, organization: organization) }
       let!(:partner) { create(:partner, organization: organization) }
       let(:issued_at) { Time.current }
@@ -282,9 +291,12 @@
           expect(flash[:error]).to include("Distribution date and time can't be blank")
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "GET #new" do
+      subject { get new_distribution_path(default_params) }
       let!(:partner) { create(:partner, organization: organization) }
       let(:request) { create(:request, partner: partner, organization: organization, item_requests: item_requests) }
       let(:items) {
@@ -408,9 +420,13 @@
           end
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "GET #show" do
+      subject { get distribution_path(id: distribution.id) }
+
       let(:item) { create(:item, organization: organization) }
       let!(:distribution) { create(:distribution, :with_items, item: item, item_quantity: 1, organization: organization) }
 
@@ -456,9 +472,13 @@
           expect(response.body).to match(/please make the following items active: #{item.name}/)
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "GET #schedule" do
+      subject { get schedule_distributions_path }
+
       it "returns http success" do
         get schedule_distributions_path
         expect(response).to be_successful
@@ -467,6 +487,8 @@
         hash = url.match(/\?hash=(.*)/)[1]
         expect(crypt.decrypt_and_verify(CGI.unescape(hash))).to eq(organization.id)
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe 'PATCH #picked_up' do
@@ -483,10 +505,14 @@
         it 'redirects the user back to the distributions page' do
           expect(subject).to redirect_to distribution_path
         end
+
+        include_examples "restricts access to organization users/admins"
       end
     end
 
     describe "GET #pickup_day" do
+      subject { get pickup_day_distributions_path }
+
       it "returns http success" do
         get pickup_day_distributions_path
         expect(response).to be_successful
@@ -520,6 +546,8 @@
         expect(assigns(:daily_items).detect { |item| item[:name] == second_item.name }[:package_count]).to eq(2)
         expect(assigns(:daily_items).sum { |item| item[:package_count] }).to eq(7)
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     context "Looking at a different organization" do
@@ -528,6 +556,7 @@
     end
 
     describe "PATCH #update" do
+      subject { patch distribution_path(distribution_params) }
       let(:partner_name) { "Patrick" }
       let(:location) { create(:storage_location, organization: organization) }
       let(:partner) { create(:partner, name: partner_name, organization: organization) }
@@ -627,9 +656,13 @@
           end
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
 
     describe "GET #edit" do
+      subject { get edit_distribution_path(id: distribution.id) }
+
       let(:location) { create(:storage_location, organization: organization) }
       let(:partner) { create(:partner, organization: organization) }
 
@@ -780,6 +813,8 @@
           expect(response.body).to include("<option selected=\"selected\" value=\"#{storage_location.id}\">#{test_storage_name}</option>")
         end
       end
+
+      include_examples "restricts access to organization users/admins"
     end
   end
 
diff --git a/spec/requests/donations_requests_spec.rb b/spec/requests/donations_requests_spec.rb
@@ -23,6 +23,8 @@
 
         it { is_expected.to be_successful }
 
+        include_examples "restricts access to organization users/admins"
+
         it "should have the columns source and details" do
           expect(subject.body).to include("<th>Source</th>")
           expect(subject.body).to include("<th>Details</th>")
diff --git a/spec/requests/partners/requests_spec.rb b/spec/requests/partners/requests_spec.rb
@@ -81,7 +81,7 @@
 
           it "redirects to dashboard path and flashes an error" do
             subject
-            expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+            expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
             expect(response).to redirect_to(dashboard_path)
           end
         end
@@ -94,7 +94,7 @@
 
         it "redirects to dashboard path and flashes an error" do
           subject
-          expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+          expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
           expect(response).to redirect_to(dashboard_path)
         end
       end
@@ -106,7 +106,7 @@
 
       it "redirects to dashboard path and flashes an error" do
         subject
-        expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+        expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
         expect(response).to redirect_to(dashboard_path)
       end
     end
@@ -376,7 +376,7 @@
 
             it "redirects to dashboard path and flashes an error" do
               subject
-              expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+              expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
               expect(response).to redirect_to(dashboard_path)
             end
           end
@@ -389,7 +389,7 @@
 
           it "redirects to dashboard path and flashes an error" do
             subject
-            expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+            expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
             expect(response).to redirect_to(dashboard_path)
           end
         end
@@ -402,7 +402,7 @@
 
       it "redirects to dashboard path and flashes an error" do
         subject
-        expect(flash[:error]).to eq("Logged in user is not set up as a 'partner'.")
+        expect(flash[:error]).to eq("That screen is not available. Please try again as a partner.")
         expect(response).to redirect_to(dashboard_path)
       end
     end
diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb
@@ -18,6 +18,8 @@
       let!(:partner) { create(:partner, organization: organization) }
 
       it { is_expected.to be_successful }
+
+      include_examples "restricts access to organization users/admins"
     end
 
     context "csv" do
diff --git a/spec/requests/users_requests_spec.rb b/spec/requests/users_requests_spec.rb
@@ -10,10 +10,14 @@
   end
 
   describe "GET #index" do
+    subject { get users_path }
+
     it "returns http success" do
       get users_path
       expect(response).to be_successful
     end
+
+    include_examples "restricts access to organization users/admins"
   end
 
   describe "GET #new" do
diff --git a/spec/support/authorization_specs.rb b/spec/support/authorization_specs.rb
@@ -1,3 +1,32 @@
+RSpec.shared_examples "restricts access to organization users/admins" do
+  context "when signed in as partner" do
+    let(:partner) { create(:partner) }
+    let(:partner_user) { partner.primary_user }
+
+    before do
+      sign_in(partner_user)
+    end
+
+    it "redirects partners to their dashboard with a flash message" do
+      expect(subject).to redirect_to(partners_dashboard_path)
+      expect(flash[:error]).to eq("That screen is not available. Please try again as a bank.")
+    end
+  end
+
+  context "when signed in as super admin" do
+    let(:super_admin) { create(:super_admin) }
+
+    before do
+      sign_in(super_admin)
+    end
+
+    it "redirects admins to their dashboard with a flash message" do
+      expect(subject).to redirect_to(admin_dashboard_path)
+      expect(flash[:error]).to eq("That screen is not available. Please try again as a bank.")
+    end
+  end
+end
+
 RSpec.shared_examples "requiring authorization" do |constraints|
   it "redirects the user to the sign-in page for CRUD actions" do
     member_params = { organization_id: object.organization.to_param, id: object.id }
