fix(permission): forward action approval exception for return expected response (#685)

Author: nicolasalexandre9

app/controllers/forest_liana/application_controller.rb

@@ -6,7 +6,8 @@ class ApplicationController < ForestLiana::BaseController
     rescue_from ForestLiana::Ability::Exceptions::AccessDenied, with: :render_error
     rescue_from ForestLiana::Errors::HTTP403Error, with: :render_error
     rescue_from ForestLiana::Errors::HTTP422Error, with: :render_error
-    rescue_from ForestLiana::Errors::ExpectedError, with: :render_error
+    rescue_from ForestLiana::Ability::Exceptions::ActionConditionError, with: :render_error
+    rescue_from ForestLiana::Ability::Exceptions::UnknownCollection, with: :render_error
 
     def self.papertrail?
       Object.const_get('PaperTrail::Version').is_a?(Class) rescue false

app/services/forest_liana/ability/exceptions/unknown_collection.rb

@@ -0,0 +1,16 @@
+module ForestLiana
+  module Ability
+    module Exceptions
+      class UnknownCollection < ForestLiana::Errors::ExpectedError
+        def initialize(collection_name)
+          super(
+            409,
+            :conflict,
+            "The collection #{collection_name} doesn't exist",
+            'collection not found'
+          )
+        end
+      end
+    end
+  end
+end

app/services/forest_liana/ability/permission.rb

@@ -25,8 +25,10 @@ def is_crud_authorized?(action, user, collection)
           end
 
           is_allowed
+        rescue ForestLiana::Errors::ExpectedError => exception
+          raise exception
         rescue
-          raise ForestLiana::Errors::ExpectedError.new(409, :conflict, "The collection #{collection_name} doesn't exist", 'collection not found')
+          raise ForestLiana::Ability::Exceptions::UnknownCollection.new(collection_name)
         end
       end
 
@@ -35,13 +37,16 @@ def is_smart_action_authorized?(user, collection, parameters, endpoint, http_met
 
         user_data = get_user_data(user['id'])
         collections_data = get_collections_permissions_data
+        collection_name = ForestLiana.name_for(collection)
         begin
-          action = find_action_from_endpoint(ForestLiana.name_for(collection), endpoint, http_method).name
+          action = find_action_from_endpoint(collection_name, endpoint, http_method).name
 
-          smart_action_approval = SmartActionChecker.new(parameters, collection, collections_data[ForestLiana.name_for(collection)][:actions][action], user_data)
+          smart_action_approval = SmartActionChecker.new(parameters, collection, collections_data[collection_name][:actions][action], user_data)
           smart_action_approval.can_execute?
+        rescue ForestLiana::Errors::ExpectedError => exception
+          raise exception
         rescue
-          raise ForestLiana::Errors::ExpectedError.new(409, :conflict, "The collection #{collection} doesn't exist", 'collection not found')
+          raise ForestLiana::Ability::Exceptions::UnknownCollection.new(collection_name)
         end
       end
 
@@ -66,7 +71,7 @@ def is_chart_authorized?(user, parameters)
 
       private
 
-      def get_user_data(user_id)
+      def get_user_data(user_id, force_fetch = true)
         cache = Rails.cache.fetch('forest.users', expires_in: TTL) do
           users = {}
           get_permissions('/liana/v4/permissions/users').each do |user|
@@ -76,7 +81,12 @@ def get_user_data(user_id)
           users
         end
 
-        cache[user_id.to_s]
+        if !cache.key?(user_id.to_s) && force_fetch
+          Rails.cache.delete('forest.users')
+          get_user_data(user_id, false)
+        else
+          cache[user_id.to_s]
+        end
       end
 
       def get_collections_permissions_data(force_fetch = false)

spec/dummy/app/controllers/forest/islands_controller.rb

@@ -2,4 +2,8 @@ class Forest::IslandsController < ForestLiana::SmartActionsController
   def test
     render json: { success: 'You are OK.' }
   end
+
+  def unknown_action
+    render json: { success: 'unknown action' }
+  end
 end

spec/dummy/config/routes.rb

@@ -1,6 +1,7 @@
 Rails.application.routes.draw do
   namespace :forest do
     post '/actions/test' => 'islands#test'
+    post '/actions/unknown_action' => 'islands#unknown_action'
     get '/Owner/count' , to: 'owners#count'
     get '/Owner/:id/relationships/trees/count' , to: 'owner_trees#count'
   end

spec/dummy/lib/forest_liana/collections/island.rb

@@ -26,6 +26,8 @@ class Forest::Island
     enums: %w[a b c],
   }
 
+  action 'test'
+
   action 'my_action',
     fields: [foo],
     hooks: {

spec/requests/actions_controller_spec.rb

@@ -261,10 +261,6 @@
   end
 
   describe 'calling the action' do
-    before(:each) do
-      allow_any_instance_of(ForestLiana::Ability).to receive(:forest_authorize!) { true }
-    end
-
     let(:all_records) { false }
     let(:params) {
       {
@@ -283,13 +279,119 @@
 
     describe 'without scopes' do
       it 'should respond 200 and perform the action' do
+        allow_any_instance_of(ForestLiana::Ability).to receive(:forest_authorize!) { true }
         post '/forest/actions/test', params: JSON.dump(params), headers: headers
         expect(response.status).to eq(200)
         expect(JSON.parse(response.body)).to eq({'success' => 'You are OK.'})
       end
+
+      let(:params) {
+        {
+          data: {
+            attributes: {
+              collection_name: 'Island',
+              ids: ['1'],
+              all_records: all_records,
+              smart_action_id: 'Island-Test'
+            },
+            type: 'custom-action-requests'
+          },
+          timezone: 'Europe/Paris'
+        }
+      }
+
+      describe 'with invalid conditions' do
+        it 'should respond a 409' do
+          Rails.cache.write('forest.has_permission', true)
+          Rails.cache.write('forest.users', {'38' => { 'id' => 38, 'roleId' => 1, 'rendering_id' => '13' }})
+          Rails.cache.write(
+            'forest.collections',
+            {
+              'Island' => {
+                :actions =>
+                  {
+                    'test' => { 'triggerEnabled' => [1],
+                      'triggerConditions' => [],
+                      'approvalRequired' => [1],
+                      'approvalRequiredConditions' =>
+                        [
+                          { 'filter' =>
+                            { 'field' => 'id',
+                              'value' => 2,
+                              'source' => 'data',
+                              'operator' => 'foo-greater-than'
+                            },
+                            'roleId' => 1
+                          }
+                        ],
+                    }
+                  }
+              }
+            }
+          )
+
+          post '/forest/actions/test', params: JSON.dump(params), headers: headers
+
+          expect(response.status).to eq(409)
+          expect(JSON.parse(response.body)).to eq(
+            {
+              "errors" => [
+                {
+                  "status" => 409,
+                  "detail" => "The conditions to trigger this action cannot be verified. Please contact an administrator.",
+                  "name" => "InvalidActionConditionError"
+                }
+              ]
+            }
+          )
+        end
+      end
+
+      describe 'with unknown action' do
+        it 'should respond a 409' do
+          Rails.cache.write('forest.has_permission', true)
+          Rails.cache.write('forest.users', {'38' => { 'id' => 38, 'roleId' => 1, 'rendering_id' => '13' }})
+          Rails.cache.write(
+            'forest.collections',
+            {
+              'Island' => {
+                :actions =>
+                  {
+                    'test' => { 'triggerEnabled' => [1],
+                      'triggerConditions' => [],
+                      'approvalRequired' => [1],
+                      'approvalRequiredConditions' =>
+                        [
+                          { 'filter' =>
+                            { 'field' => 'id',
+                              'value' => 2,
+                              'source' => 'data',
+                              'operator' => 'foo-greater-than'
+                            },
+                            'roleId' => 1
+                          }
+                        ],
+                    }
+                  }
+              }
+            }
+          )
+
+          post '/forest/actions/unknown_action', params: JSON.dump(params), headers: headers
+
+          expect(response.status).to eq(409)
+          expect(JSON.parse(response.body)).to eq(
+            {"errors"=> [{"detail" => "The collection Island doesn't exist", "name" => "collection not found", "status" => 409}]}
+          )
+        end
+      end
     end
 
     describe 'with scopes' do
+      before(:each) do
+        allow_any_instance_of(ForestLiana::Ability).to receive(:forest_authorize!) { true }
+      end
+
       describe 'when record is in scope' do
         let(:scope_filters) {
           {

spec/services/forest_liana/ability/permission_spec.rb

@@ -81,7 +81,11 @@ module Ability
 
 
         it 'should throw an exception when the collection doesn\'t exist' do
-            expect {dummy_class.is_crud_authorized?('browse', user, String)}.to raise_error(ForestLiana::Errors::ExpectedError, 'The collection String doesn\'t exist')
+          allow_any_instance_of(ForestLiana::Ability::Fetch)
+            .to receive(:get_permissions)
+              .and_return({ "collections" => {} })
+            expect {dummy_class.is_crud_authorized?('browse', user, String)}
+              .to raise_error(ForestLiana::Ability::Exceptions::UnknownCollection, 'The collection String doesn\'t exist')
         end
 
         it 'should re-fetch the permission once when user permission is not allowed' do
@@ -211,6 +215,30 @@ module Ability
           expect(dummy_class.is_crud_authorized?('browse', user, Island)).to equal true
         end
 
+        it 'should re-fetch the users list once when user doesn\'t exist' do
+          Rails.cache.write('forest.users', {'2' => { 'id' => 2, 'roleId' => 1, 'rendering_id' => '1' }})
+          # Rails.cache.write('forest.users', {'1' => user})
+          Rails.cache.write(
+            'forest.collections',
+            'Island' => {
+              'browse' => [1],
+              'read' => [1],
+              'edit' => [1],
+              'add' => [1],
+              'delete' => [1],
+              'export' => [1],
+            }
+          )
+
+          allow_any_instance_of(ForestLiana::Ability::Fetch)
+            .to receive(:get_permissions)
+              .and_return(
+                [user]
+              )
+
+          expect(dummy_class.is_crud_authorized?('browse', user, Island)).to equal true
+        end
+
         it 'should return false when user permission is not allowed' do
           Rails.cache.delete('forest.users')
 
@@ -372,7 +400,8 @@ module Ability
         end
 
         it 'should throw an exception when the collection doesn\'t exist' do
-          expect {dummy_class.is_smart_action_authorized?(user, String, parameters, '/forest/actions/my_action', 'POST')}.to raise_error(ForestLiana::Errors::ExpectedError, 'The collection String doesn\'t exist')
+          expect {dummy_class.is_smart_action_authorized?(user, String, parameters, '/forest/actions/my_action', 'POST')}
+            .to raise_error(ForestLiana::Ability::Exceptions::UnknownCollection, 'The collection String doesn\'t exist')
         end
       end