fix(charts): user with permissions level that allows charts creation or edition should always be allow to perform charts requests released (#583)

matthv · web-flow · commit e6611e9c1adc · 2022-09-08T10:04:52.000+02:00
diff --git a/app/controllers/forest_liana/resources_controller.rb b/app/controllers/forest_liana/resources_controller.rb
@@ -16,14 +16,14 @@ class ResourcesController < ForestLiana::ApplicationController
     def index
       begin
         if request.format == 'csv'
-          checker = ForestLiana::PermissionsChecker.new(@resource, 'exportEnabled', @rendering_id, user_id: forest_user['id'])
+          checker = ForestLiana::PermissionsChecker.new(@resource, 'exportEnabled', @rendering_id, user: forest_user)
           return head :forbidden unless checker.is_authorized?
         else
           checker = ForestLiana::PermissionsChecker.new(
             @resource,
             'browseEnabled',
             @rendering_id,
-            user_id: forest_user['id'],
+            user: forest_user,
             collection_list_parameters: get_collection_list_permission_info(forest_user, request)
           )
           return head :forbidden unless checker.is_authorized?
@@ -60,7 +60,7 @@ def count
           @resource,
           'browseEnabled',
           @rendering_id,
-          user_id: forest_user['id'],
+          user: forest_user,
           collection_list_parameters: get_collection_list_permission_info(forest_user, request)
         )
         return head :forbidden unless checker.is_authorized?
@@ -89,7 +89,7 @@ def count
 
     def show
       begin
-        checker = ForestLiana::PermissionsChecker.new(@resource, 'readEnabled', @rendering_id, user_id: forest_user['id'])
+        checker = ForestLiana::PermissionsChecker.new(@resource, 'readEnabled', @rendering_id, user: forest_user)
         return head :forbidden unless checker.is_authorized?
 
         getter = ForestLiana::ResourceGetter.new(@resource, params, forest_user)
@@ -107,7 +107,7 @@ def show
 
     def create
       begin
-        checker = ForestLiana::PermissionsChecker.new(@resource, 'addEnabled', @rendering_id, user_id: forest_user['id'])
+        checker = ForestLiana::PermissionsChecker.new(@resource, 'addEnabled', @rendering_id, user: forest_user)
         return head :forbidden unless checker.is_authorized?
 
         creator = ForestLiana::ResourceCreator.new(@resource, params)
@@ -131,7 +131,7 @@ def create
 
     def update
       begin
-        checker = ForestLiana::PermissionsChecker.new(@resource, 'editEnabled', @rendering_id, user_id: forest_user['id'])
+        checker = ForestLiana::PermissionsChecker.new(@resource, 'editEnabled', @rendering_id, user: forest_user)
         return head :forbidden unless checker.is_authorized?
 
         updater = ForestLiana::ResourceUpdater.new(@resource, params, forest_user)
@@ -154,7 +154,7 @@ def update
     end
 
     def destroy
-      checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
+      checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user: forest_user)
       return head :forbidden unless checker.is_authorized?
 
       collection_name = ForestLiana.name_for(@resource)
@@ -174,7 +174,7 @@ def destroy
     end
 
     def destroy_bulk
-      checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
+      checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user: forest_user)
       return head :forbidden unless checker.is_authorized?
 
       ids = ForestLiana::ResourcesGetter.get_ids_from_request(params, forest_user)
diff --git a/app/controllers/forest_liana/smart_actions_controller.rb b/app/controllers/forest_liana/smart_actions_controller.rb
@@ -63,7 +63,7 @@ def check_permission_for_smart_route
             find_resource(smart_action_request[:collection_name]),
             'actions',
             @rendering_id,
-            user_id: forest_user['id'],
+            user: forest_user,
             smart_action_request_info: get_smart_action_request_info
           )
           return head :forbidden unless checker.is_authorized?
diff --git a/app/controllers/forest_liana/stats_controller.rb b/app/controllers/forest_liana/stats_controller.rb
@@ -105,7 +105,7 @@ def check_permission(permission_name)
           nil,
           permission_name,
           @rendering_id,
-          user_id: forest_user['id'],
+          user: forest_user,
           query_request_info: query_request
         )
 
diff --git a/app/services/forest_liana/permissions_checker.rb b/app/services/forest_liana/permissions_checker.rb
@@ -6,12 +6,14 @@ class PermissionsChecker
 
     @@expiration_in_seconds = (ENV['FOREST_PERMISSIONS_EXPIRATION_IN_SECONDS'] || 3600).to_i
 
-    def initialize(resource, permission_name, rendering_id, user_id: nil, smart_action_request_info: nil, collection_list_parameters: Hash.new, query_request_info: nil)
+    ALLOWED_PERMISSION_LEVELS = %w[admin editor developer]
+
+    def initialize(resource, permission_name, rendering_id, user: nil, smart_action_request_info: nil, collection_list_parameters: Hash.new, 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
+      @user = user
       @smart_action_request_info = smart_action_request_info
       @collection_list_parameters = collection_list_parameters
       @query_request_info = query_request_info
@@ -56,9 +58,9 @@ def is_allowed
 
       # NOTICE: check liveQueries permissions
       if @permission_name === 'liveQueries'
-        return live_query_allowed?
+        return ALLOWED_PERMISSION_LEVELS.include?(@user['permission_level']) || live_query_allowed?
       elsif @permission_name === 'statWithParameters'
-        return stat_with_parameters_allowed?
+        return ALLOWED_PERMISSION_LEVELS.include?(@user['permission_level']) || stat_with_parameters_allowed?
       end
 
       if permissions && permissions[@collection_name] &&
@@ -146,7 +148,7 @@ def get_smart_action_permissions(smart_actions_permissions)
     def is_user_allowed(permission_value)
       return false if permission_value.nil?
       return permission_value if permission_value.in? [true, false]
-      permission_value.include?(@user_id.to_i)
+      permission_value.include?(@user['id'].to_i)
     end
 
     def smart_action_allowed?(smart_actions_permissions)
diff --git a/app/services/forest_liana/token.rb b/app/services/forest_liana/token.rb
@@ -22,7 +22,8 @@ def self.create_token(user, rendering_id)
         role: user['role'],
         tags: user['tags'],
         rendering_id: rendering_id,
-        exp: expiration_in_seconds()
+        exp: expiration_in_seconds(),
+        permission_level: user['permission_level'],
       }, ForestLiana.auth_secret, 'HS256')
     end
   end
diff --git a/spec/requests/actions_controller_spec.rb b/spec/requests/actions_controller_spec.rb
@@ -25,7 +25,8 @@
       last_name: 'Kelso',
       team: 'Operations',
       rendering_id: rendering_id,
-      exp: Time.now.to_i + 2.weeks.to_i
+      exp: Time.now.to_i + 2.weeks.to_i,
+      permission_level: 'admin'
     }, ForestLiana.auth_secret, 'HS256')
   }
 
diff --git a/spec/requests/count_spec.rb b/spec/requests/count_spec.rb
@@ -29,7 +29,8 @@
                        last_name: 'Kelso',
                        team: 'Operations',
                        rendering_id: 16,
-                       exp: Time.now.to_i + 2.weeks.to_i
+                       exp: Time.now.to_i + 2.weeks.to_i,
+                       permission_level: 'admin'
                      }, ForestLiana.auth_secret, 'HS256')
 
   headers = {
diff --git a/spec/requests/resources_spec.rb b/spec/requests/resources_spec.rb
@@ -28,7 +28,8 @@
     last_name: 'Kelso',
     team: 'Operations',
     rendering_id: 16,
-    exp: Time.now.to_i + 2.weeks.to_i
+    exp: Time.now.to_i + 2.weeks.to_i,
+    permission_level: 'admin'
   }, ForestLiana.auth_secret, 'HS256')
 
   headers = {
diff --git a/spec/requests/stats_spec.rb b/spec/requests/stats_spec.rb
@@ -10,7 +10,8 @@
     last_name: 'Kelso',
     team: 'Operations',
     rendering_id: 16,
-    exp: Time.now.to_i + 2.weeks.to_i
+    exp: Time.now.to_i + 2.weeks.to_i,
+    permission_level: 'admin'
   }, ForestLiana.auth_secret, 'HS256')
 
   headers = {
@@ -37,7 +38,7 @@
     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 }
@@ -81,7 +82,7 @@
       expect(response.status).to eq(403)
     end
   end
-  
+
   describe 'POST /stats' do
     params = { query: 'SELECT COUNT(*) AS value FROM products;' }
 
@@ -107,7 +108,7 @@
 
     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
diff --git a/spec/services/forest_liana/permissions_checker_acl_disabled_spec.rb b/spec/services/forest_liana/permissions_checker_acl_disabled_spec.rb
diff --git a/spec/services/forest_liana/permissions_checker_acl_enabled_spec.rb b/spec/services/forest_liana/permissions_checker_acl_enabled_spec.rb
diff --git a/spec/services/forest_liana/permissions_checker_live_queries_spec.rb b/spec/services/forest_liana/permissions_checker_live_queries_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ def check_permission_for_smart_route`
`63`	`63`	`find_resource(smart_action_request[:collection_name]),`
`64`	`64`	`'actions',`
`65`	`65`	`@rendering_id,`
`66`		`- user_id: forest_user['id'],`
	`66`	`+ user: forest_user,`
`67`	`67`	`smart_action_request_info: get_smart_action_request_info`
`68`	`68`	`)`
`69`	`69`	`return head :forbidden unless checker.is_authorized?`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ def check_permission(permission_name)`
`105`	`105`	`nil,`
`106`	`106`	`permission_name,`
`107`	`107`	`@rendering_id,`
`108`		`- user_id: forest_user['id'],`
	`108`	`+ user: forest_user,`
`109`	`109`	`query_request_info: query_request`
`110`	`110`	`)`
`111`	`111`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,8 @@`
`25`	`25`	`last_name: 'Kelso',`
`26`	`26`	`team: 'Operations',`
`27`	`27`	`rendering_id: rendering_id,`
`28`		`- exp: Time.now.to_i + 2.weeks.to_i`
	`28`	`+ exp: Time.now.to_i + 2.weeks.to_i,`
	`29`	`+ permission_level: 'admin'`
`29`	`30`	`}, ForestLiana.auth_secret, 'HS256')`
`30`	`31`	`}`
`31`	`32`