Merge pull request #4699 from h-kataria/report_editor_rbac_check_fixed

Fixed code to check for correct RBAC feature when adding/editing report

(cherry picked from commit e1897a0)

https://bugzilla.redhat.com/show_bug.cgi?id=1631892

app/controllers/report_controller/reports/editor.rb

@@ -46,7 +46,7 @@ def default_chargeback_allocated_method
   def miq_report_new
     assert_privileges("miq_report_new")
     @_params.delete :id # incase add button was pressed from report show screen.
-    miq_report_edit
+    miq_report_add_edit
   end
 
   def miq_report_copy
@@ -65,7 +65,7 @@ def miq_report_copy
   end
 
   def miq_report_edit
-    assert_privileges("miq_report_edit")
+    assert_privileges(params[:id] || (@edit && @edit[:rpt_id]) ? "miq_report_edit" : "miq_report_new")
     case params[:button]
     when "cancel"
       @edit[:rpt_id] ?
@@ -121,29 +121,30 @@ def miq_report_edit
         replace_right_cell
       end
     else
-      add_flash(_("All changes have been reset"), :warning) if params[:button] == "reset"
-      @in_a_form = true
-      @report = nil     # Clear any saved report object
-      if params[:tab] # Came in to change the tab
-        @rpt = @edit[:rpt_id] ? MiqReport.for_user(current_user).find(@edit[:rpt_id]) :
-            MiqReport.new
-        check_tabs
-      else
-        @sb[:miq_tab] = "edit_1"
-        @rpt = params[:id] && params[:id] != "new" ? MiqReport.for_user(current_user).find(params[:id]) :
-                MiqReport.new
-        if @rpt.rpt_type == "Default"
-          flash_to_session(_('Default reports can not be edited'), :error)
-          redirect_to(:action => "show", :id => @rpt.id)
-          return
-        end
-        set_form_vars
+      miq_report_add_edit
+    end
+  end
+
+  def miq_report_add_edit
+    add_flash(_("All changes have been reset"), :warning) if params[:button] == "reset"
+    @in_a_form = true
+    @report = nil # Clear any saved report object
+    if params[:tab] # Came in to change the tab
+      @rpt = @edit[:rpt_id] ? MiqReport.for_user(current_user).find(@edit[:rpt_id]) : MiqReport.new
+      check_tabs
+    else
+      @sb[:miq_tab] = "edit_1"
+      @rpt = params[:id] && params[:id] != "new" ? MiqReport.for_user(current_user).find(params[:id]) : MiqReport.new
+      if @rpt.rpt_type == "Default"
+        flash_to_session(_('Default reports can not be edited'), :error)
+        redirect_to(:action => "show", :id => @rpt.id)
+        return
       end
-      build_edit_screen
-      @changed          = (@edit[:new] != @edit[:current])
-      session[:changed] = @changed
-      replace_right_cell
+      set_form_vars
     end
+    build_edit_screen
+    session[:changed] = @changed = (@edit[:new] != @edit[:current])
+    replace_right_cell
   end
 
   # AJAX driven routine to check for changes in ANY field on the form

spec/controllers/miq_report_controller/reports/editor_spec.rb

@@ -117,10 +117,9 @@
     end
 
     context "#miq_report_edit" do
-      let(:user) { stub_user(:features => :all) }
-      before { user.save! }
-
       it "should build tabs with correct tab id after reset button is pressed to prevent error when changing tabs" do
+        user = stub_user(:features => :all)
+        user.save!
         ApplicationController.handle_exceptions = true
 
         rep = FactoryGirl.create(
@@ -160,6 +159,45 @@
         expect(assigns(:tabs)).to include(["edit_1", "Columns"])
         expect(assigns(:active_tab)).to eq("edit_1")
       end
+
+      it "should allow user with miq_report_edit access to edit a report" do
+        user = FactoryGirl.create(:user, :features => %w(miq_report_edit))
+        login_as user
+        EvmSpecHelper.seed_specific_product_features(%w(miq_report_edit))
+        ApplicationController.handle_exceptions = true
+
+        rep = FactoryGirl.create(
+          :miq_report,
+          :rpt_type   => "Custom",
+          :miq_group  => user.current_group,
+          :db         => "Host",
+          :name       => 'name',
+          :title      => 'title',
+          :db_options => {},
+          :col_order  => ["name"],
+          :headers    => ["Name"],
+          :tz         => nil
+        )
+        allow(controller).to receive(:load_edit).and_return(true)
+        allow(controller).to receive(:replace_right_cell)
+        controller.instance_variable_set(:@sb, {})
+        controller.instance_variable_set(:@_params, :id => rep.id)
+        controller.send(:miq_report_edit)
+        expect(response.status).to eq(200)
+      end
+
+      it "should allow user with miq_report_new access to add a new report" do
+        login_as FactoryGirl.create(:user, :features => %w(miq_report_new))
+        EvmSpecHelper.seed_specific_product_features(%w(miq_report_new))
+        ApplicationController.handle_exceptions = true
+
+        allow(controller).to receive(:load_edit).and_return(true)
+        allow(controller).to receive(:replace_right_cell)
+        controller.instance_variable_set(:@sb, {})
+        controller.instance_variable_set(:@_params, :pressed => 'miq_report_new')
+        controller.send(:miq_report_edit)
+        expect(response.status).to eq(200)
+      end
     end
 
     describe "set_form_vars" do