Ability and specs

app/controllers/hub/clients_controller.rb

@@ -5,7 +5,7 @@ class ClientsController < Hub::BaseController
 
     before_action :load_vita_partners, only: [:new, :create, :index]
     before_action :load_users, only: [:index]
-    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect]
+    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect] # why except new and create
     before_action :setup_sortable_client, only: [:index]
     # need to use the presenter for :update bc it has ITIN applicant methods that are used in the form
     before_action :wrap_client_in_hub_presenter, only: [:show, :edit, :edit_take_action, :update, :update_take_action]
@@ -142,8 +142,8 @@ def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
     end
 
     def redirect_if_not_authorized
-      authorize! :hub_client_management, @client
-      # raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc? || cannot?(:hub_client_management, @client)
+      # todo: change name/back? redirects are happening based on ability in ability file
+      raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc?
     end
 
     def update_13614c_form_page1

app/lib/ability.rb

@@ -16,7 +16,7 @@ def initialize(user)
                  :edit_13614c_form_page3, :edit_13614c_form_page4, :edit_13614c_form_page5,
                  :update_13614c_form_page1, :update_13614c_form_page2,
                  :update_13614c_form_page3, :update_13614c_form_page4, :update_13614c_form_page5,
-                 :cancel_13614c, :resource_to_client_redirect,
+                 :cancel_13614c,
                  to: :hub_client_management
 
     accessible_groups = user.accessible_vita_partners
@@ -80,9 +80,9 @@ def initialize(user)
     can :read, Organization, id: accessible_groups.pluck(:id)
     can :read, Site, id: accessible_groups.pluck(:id)
 
-    # This was overly permissive. We should work out what the permissions should
-    # be for each role and reduce this check. As we need to modify this, please
-    # break out the role and specify permissions more granularly
+    # CLIENT CONTROLLER PERMISSIONS
+    # overly permissive, need to narrow permissions
+    # break out role and specify permissions when making modifications
     client_role_whitelist = [
       :client_success, :admin, :org_lead, :site_coordinator,
       :coalition_lead, :state_file_admin, :team_member
@@ -91,34 +91,25 @@ def initialize(user)
     if user.role?(client_role_whitelist)
       can :read, Client, vita_partner: accessible_groups
 
-      client_management_actions = [:create, :update, :edit, :hub_client_management]
-      can client_management_actions, Client, vita_partner: accessible_groups
-      # Can only take actions on non-archived intakes
-      cannot client_management_actions, Client, &:has_archived_intake?
+      can [:create, :update, :edit, :hub_client_management],
+          Client, vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
     if user.greeter?
       can [:update, :read, :hub_client_management],
         Client,
         tax_returns: {
-          current_state: [
-            'intake_ready',
-            'intake_greeter_info_requested',
-            'intake_needs_doc_help',
-          ],
+          current_state: %w[intake_ready intake_greeter_info_requested intake_needs_doc_help],
         },
-        vita_partner: accessible_groups
+        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
 
       can [:update, :read, :hub_client_management],
         Client,
         tax_returns: {
-          current_state: [
-            'file_not_filing',
-            'file_hold',
-          ],
+          current_state: %w[file_not_filing file_hold],
           assigned_user: user,
         },
-        vita_partner: accessible_groups
+        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
     # Only admins can destroy clients
@@ -128,7 +119,7 @@ def initialize(user)
       Note,
       Document,
       TaxReturn
-    ], client: { intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups }
+    ], client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } }
 
     can [:read], [
       Note,
@@ -144,8 +135,8 @@ def initialize(user)
       SystemNote,
     ], client: { vita_partner: accessible_groups }
 
-    can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups } }
-    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) }}
+    can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } } }
+    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) } }
 
     can :manage, EfileSubmission, tax_return: { client: { vita_partner: accessible_groups } }
 

spec/controllers/hub/clients_controller_spec.rb

@@ -938,10 +938,10 @@
           create(:archived_2021_gyr_intake, client: client)
         end
 
-        it "redirects to the /show page for the client" do
+        it "redirects to Access Denied page" do
           get :edit, params: params
 
-          expect(response).to redirect_to(hub_client_path(id: client.id))
+          expect(response).to be_forbidden
         end
       end
     end
@@ -1052,10 +1052,10 @@
           create(:archived_2021_gyr_intake, client: client)
         end
 
-        it "redirects to the /show page for the client" do
+        it "response is forbidden (403)" do
           post :update, params: { id: client.id }
 
-          expect(response).to redirect_to(hub_client_path(id: client.id))
+          expect(response).to be_forbidden
         end
       end
 
@@ -1299,13 +1299,11 @@
       end
 
       context "when the client is not hub updatable" do
-        before do
-          allow_any_instance_of(Hub::ClientsController::HubClientPresenter).to receive(:hub_status_updatable).and_return(false)
-        end
+        let(:intake) { build :ctc_intake, email_address: "[email protected]", sms_phone_number: "+14155551212" }
 
         it "raises bad request" do
           post :update_take_action, params: params
-          expect(response).to redirect_to hub_client_path(id: client.id)
+          expect(response).to be_forbidden
         end
       end