feat(security): authorised only allowed stats queries using permissions (#434)

* feat(security): authorized only allowed live queries using permissions

* refactor: live query allowed

* test(permissions-checker): add is_authorize live query tests

* test(controller): add spec test to stats-controller

* feat(security): stats parameters permissions

* test: add test for stat with parameters permissions

* refactor(stat_controller): similar blocks for permission check

* fix(stat_controller): similar blocks for permission check

* fix(stat_controller): before_action and before_filter implementation

Author: Thenkei

app/controllers/forest_liana/stats_controller.rb

@@ -1,9 +1,21 @@
 module ForestLiana
   class StatsController < ForestLiana::ApplicationController
     if Rails::VERSION::MAJOR < 4
-      before_filter :find_resource, except: [:get_with_live_query]
+      before_filter only: [:get] do
+        find_resource()
+        check_permission('statWithParameters')
+      end
+      before_filter only: [:get_with_live_query] do
+        check_permission('liveQueries')
+      end
     else
-      before_action :find_resource, except: [:get_with_live_query]
+      before_action only: [:get] do
+        find_resource()
+        check_permission('statWithParameters')
+      end
+      before_action only: [:get_with_live_query] do
+        check_permission('liveQueries')
+      end
     end
 
     CHART_TYPE_VALUE = 'Value'
@@ -64,5 +76,38 @@ def find_resource
         render json: {status: 404}, status: :not_found, serializer: nil
       end
     end
+
+    def get_live_query_request_info
+      params['query']
+    end
+
+    def get_stat_parameter_request_info
+      parameters = Rails::VERSION::MAJOR < 5 ? params.dup : params.permit(params.keys).to_h;
+
+      # Notice: Removes useless properties
+      parameters.delete('timezone');
+      parameters.delete('controller');
+      parameters.delete('action');
+
+      return parameters;
+    end
+
+    def check_permission(permission_name)
+      begin
+        query_request = permission_name == 'liveQueries' ? get_live_query_request_info : get_stat_parameter_request_info;
+        checker = ForestLiana::PermissionsChecker.new(
+          nil,
+          permission_name,
+          @rendering_id,
+          user_id: forest_user['id'],
+          query_request_info: query_request
+        )
+
+        return head :forbidden unless checker.is_authorized?
+      rescue => error
+        FOREST_LOGGER.error "Stats execution error: #{error}"
+        render serializer: nil, json: { status: 400 }, status: :bad_request
+      end
+    end
   end
 end

app/services/forest_liana/permissions_checker.rb

@@ -6,13 +6,16 @@ class PermissionsChecker
     # TODO: handle cache scopes per rendering
     @@expiration_in_seconds = (ENV['FOREST_PERMISSIONS_EXPIRATION_IN_SECONDS'] || 3600).to_i
 
-    def initialize(resource, permission_name, rendering_id, user_id:, smart_action_request_info: nil, collection_list_parameters: nil)
-      @user_id = user_id
-      @collection_name = ForestLiana.name_for(resource)
+    def initialize(resource, permission_name, rendering_id, user_id: nil, smart_action_request_info: nil, collection_list_parameters: nil, query_request_info: nil)
+      
+      @collection_name = resource.present? ? ForestLiana.name_for(resource) : nil
       @permission_name = permission_name
       @rendering_id = rendering_id
+
+      @user_id = user_id
       @smart_action_request_info = smart_action_request_info
       @collection_list_parameters = collection_list_parameters
+      @query_request_info = query_request_info
     end
 
     def is_authorized?
@@ -48,6 +51,16 @@ def add_scopes_to_cache(permissions)
 
     def is_allowed
       permissions = get_permissions_content
+
+      # NOTICE: check liveQueries permissions
+      if @permission_name === 'liveQueries'
+        return live_query_allowed?
+      elsif @permission_name === 'statWithParameters'
+        return stat_with_parameters_allowed?
+      end
+
+      
+
       if permissions && permissions[@collection_name] &&
         permissions[@collection_name]['collection']
         if @permission_name === 'actions'
@@ -98,6 +111,16 @@ def get_permissions_content
       permissions && permissions['data'] && permissions['data']['collections']
     end
 
+    def get_live_query_permissions_content
+      permissions = get_permissions
+      permissions && permissions['stats'] && permissions['stats']['queries']
+    end
+    
+    def get_stat_with_parameters_content(statPermissionType)
+      permissions = get_permissions
+      permissions && permissions['stats'] && permissions['stats'][statPermissionType]
+    end
+
     def get_last_fetch
       permissions = get_permissions
       permissions && permissions['last_fetch']
@@ -138,6 +161,32 @@ def are_scopes_valid?(scope_permissions)
       ).is_scope_in_request?(@collection_list_parameters)
     end
 
+    def live_query_allowed?
+      live_queries_permissions = get_live_query_permissions_content
+
+      return false unless live_queries_permissions
+
+      # NOTICE: @query_request_info matching an existing live query 
+      return live_queries_permissions.include? @query_request_info
+    end
+
+    def stat_with_parameters_allowed?
+      permissionType = @query_request_info['type'].downcase + 's'
+      pool_permissions = get_stat_with_parameters_content(permissionType)
+
+      return false unless pool_permissions
+
+      # NOTICE: equivalent to Object.values in js
+      array_query_request_info = @query_request_info.values
+
+      # NOTICE: pool_permissions contains the @query_request_info
+      #   we use the intersection between statPermission and @query_request_info
+      return pool_permissions.any? {
+        |statPermission|
+          (array_query_request_info & statPermission.values) == array_query_request_info;
+      }
+    end
+
     def date_difference_in_seconds(date1, date2)
       (date1 - date2).to_i
     end

spec/requests/stats_spec.rb

@@ -0,0 +1,114 @@
+require 'rails_helper'
+require 'json'
+
+describe "Stats", type: :request do
+
+  token = JWT.encode({
+    id: 38,
+    email: '[email protected]',
+    first_name: 'Michael',
+    last_name: 'Kelso',
+    team: 'Operations',
+    rendering_id: 16,
+    exp: Time.now.to_i + 2.weeks.to_i
+  }, ForestLiana.auth_secret, 'HS256')
+
+  headers = {
+    'Accept' => 'application/json',
+    'Content-Type' => 'application/json',
+    'Authorization' => "Bearer #{token}"
+  }
+
+  let(:schema) {
+    [
+      ForestLiana::Model::Collection.new({
+        name: 'Products',
+        fields: [],
+        actions: []
+      })
+    ]
+  }
+
+  before do
+    allow(ForestLiana).to receive(:apimap).and_return(schema)
+  end
+
+  before(:each) do
+    allow(ForestLiana::IpWhitelist).to receive(:retrieve) { true }
+    allow(ForestLiana::IpWhitelist).to receive(:is_ip_whitelist_retrieved) { true }
+    allow(ForestLiana::IpWhitelist).to receive(:is_ip_valid) { true }
+    
+    allow_any_instance_of(ForestLiana::PermissionsChecker).to receive(:is_authorized?) { true }
+
+    allow_any_instance_of(ForestLiana::ValueStatGetter).to receive(:perform) { true }
+    allow_any_instance_of(ForestLiana::QueryStatGetter).to receive(:perform) { true }
+  end
+
+
+
+  describe 'POST /stats/:collection' do
+    params = { type: 'Value', collection: 'User', aggregate: 'Count' }
+
+    it 'should respond 200' do
+      data = ForestLiana::Model::Stat.new(value: { countCurrent: 0, countPrevious: 0 })
+      allow_any_instance_of(ForestLiana::ValueStatGetter).to receive(:record) { data }
+      # NOTICE: bypass : find_resource error
+      allow_any_instance_of(ForestLiana::StatsController).to receive(:find_resource) { true }
+      allow(ForestLiana::QueryHelper).to receive(:get_one_association_names_symbol) { true }
+
+      post '/forest/stats/Products', params: JSON.dump(params), headers: headers
+      expect(response.status).to eq(200)
+    end
+
+    it 'should respond 401 with no headers' do
+      post '/forest/stats/Products', params: JSON.dump(params)
+      expect(response.status).to eq(401)
+    end
+
+    it 'should respond 404 with non existing collection' do
+      allow_any_instance_of(ForestLiana::ValueStatGetter).to receive(:record) { nil }
+
+      post '/forest/stats/NoCollection', params: {}, headers: headers
+      expect(response.status).to eq(404)
+    end
+
+    # it 'should respond 403 Forbidden' do
+    #   allow_any_instance_of(ForestLiana::PermissionsChecker).to receive(:is_authorized?) { false }
+
+    #   post '/forest/stats/Products', params: JSON.dump(params), headers: headers
+    #   expect(response.status).to eq(403)
+    # end
+  end
+  
+  describe 'POST /stats' do
+    params = { query: 'SELECT COUNT(*) AS value FROM products;' }
+
+    it 'should respond 200' do
+      data = ForestLiana::Model::Stat.new(value: { value: 0, objective: 0 })
+      allow_any_instance_of(ForestLiana::QueryStatGetter).to receive(:record) { data }
+
+      post '/forest/stats', params: JSON.dump(params), headers: headers
+      expect(response.status).to eq(200)
+    end
+
+    it 'should respond 401 with no headers' do
+      post '/forest/stats', params: JSON.dump(params)
+      expect(response.status).to eq(401)
+    end
+
+    it 'should respond 403 Forbidden' do
+      allow_any_instance_of(ForestLiana::PermissionsChecker).to receive(:is_authorized?) { false }
+
+      post '/forest/stats', params: JSON.dump(params), headers: headers
+      expect(response.status).to eq(403)
+    end
+
+    it 'should respond 422 with unprocessable query' do
+      allow_any_instance_of(ForestLiana::QueryStatGetter).to receive(:perform) { raise ForestLiana::Errors::LiveQueryError.new }
+      
+      post '/forest/stats', params: JSON.dump(params), headers: headers
+      expect(response.status).to eq(422)
+    end
+  end
+
+end

spec/services/forest_liana/permissions_checker_acl_disabled_spec.rb

@@ -109,7 +109,7 @@ module ForestLiana
 
     describe 'handling cache' do
       let(:collection_name) { 'all_rights_collection' }
-      let(:fake_ressource) { nil }
+      let(:fake_ressource) { collection_name }
       let(:default_rendering_id) { 1 }
 
       context 'when calling twice the same permissions' do
@@ -191,7 +191,7 @@ module ForestLiana
 
 
     context 'scopes cache' do
-      let(:fake_ressource) { nil }
+      let(:fake_ressource) { collection_name }
       let(:rendering_id) { 1 }
       let(:collection_name) { 'custom' }
       let(:scope_permissions) { { rendering_id => { 'custom' => nil } } }
@@ -349,7 +349,7 @@ module ForestLiana
     describe '#is_authorized?' do
       # Resource is only used to retrieve the collection name as it's stubbed it does not
       # need to be defined
-      let(:fake_ressource) { nil }
+      let(:fake_ressource) { collection_name }
       let(:default_rendering_id) { nil }
       let(:api_permissions) { default_api_permissions }
       let(:collection_name) { 'all_rights_collection' }

spec/services/forest_liana/permissions_checker_acl_enabled_spec.rb

@@ -132,7 +132,7 @@ module ForestLiana
 
     describe 'handling cache' do
       let(:collection_name) { 'all_rights_collection_boolean' }
-      let(:fake_ressource) { nil }
+      let(:fake_ressource) { collection_name }
       let(:default_rendering_id) { 1 }
 
       context 'collections cache' do
@@ -396,7 +396,7 @@ module ForestLiana
     describe '#is_authorized?' do
       # Resource is only used to retrieve the collection name as it's stub it does not
       # need to be defined
-      let(:fake_ressource) { nil }
+      let(:fake_ressource) { collection_name }
       let(:default_rendering_id) { nil }
       let(:api_permissions) { default_api_permissions }