Skip to content

Commit e6611e9

Browse files
matthvmatthv
authored
fix(charts): user with permissions level that allows charts creation or edition should always be allow to perform charts requests released (#583)
1 parent a48ec7a commit e6611e9

12 files changed

+148
-97
lines changed

‎app/controllers/forest_liana/resources_controller.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ class ResourcesController < ForestLiana::ApplicationController
1616
def index
1717
begin
1818
if request.format == 'csv'
19-
checker = ForestLiana::PermissionsChecker.new(@resource, 'exportEnabled', @rendering_id, user_id: forest_user['id'])
19+
checker = ForestLiana::PermissionsChecker.new(@resource, 'exportEnabled', @rendering_id, user: forest_user)
2020
return head :forbidden unless checker.is_authorized?
2121
else
2222
checker = ForestLiana::PermissionsChecker.new(
2323
@resource,
2424
'browseEnabled',
2525
@rendering_id,
26-
user_id: forest_user['id'],
26+
user: forest_user,
2727
collection_list_parameters: get_collection_list_permission_info(forest_user, request)
2828
)
2929
return head :forbidden unless checker.is_authorized?
@@ -60,7 +60,7 @@ def count
6060
@resource,
6161
'browseEnabled',
6262
@rendering_id,
63-
user_id: forest_user['id'],
63+
user: forest_user,
6464
collection_list_parameters: get_collection_list_permission_info(forest_user, request)
6565
)
6666
return head :forbidden unless checker.is_authorized?
@@ -89,7 +89,7 @@ def count
8989

9090
def show
9191
begin
92-
checker = ForestLiana::PermissionsChecker.new(@resource, 'readEnabled', @rendering_id, user_id: forest_user['id'])
92+
checker = ForestLiana::PermissionsChecker.new(@resource, 'readEnabled', @rendering_id, user: forest_user)
9393
return head :forbidden unless checker.is_authorized?
9494

9595
getter = ForestLiana::ResourceGetter.new(@resource, params, forest_user)
@@ -107,7 +107,7 @@ def show
107107

108108
def create
109109
begin
110-
checker = ForestLiana::PermissionsChecker.new(@resource, 'addEnabled', @rendering_id, user_id: forest_user['id'])
110+
checker = ForestLiana::PermissionsChecker.new(@resource, 'addEnabled', @rendering_id, user: forest_user)
111111
return head :forbidden unless checker.is_authorized?
112112

113113
creator = ForestLiana::ResourceCreator.new(@resource, params)
@@ -131,7 +131,7 @@ def create
131131

132132
def update
133133
begin
134-
checker = ForestLiana::PermissionsChecker.new(@resource, 'editEnabled', @rendering_id, user_id: forest_user['id'])
134+
checker = ForestLiana::PermissionsChecker.new(@resource, 'editEnabled', @rendering_id, user: forest_user)
135135
return head :forbidden unless checker.is_authorized?
136136

137137
updater = ForestLiana::ResourceUpdater.new(@resource, params, forest_user)
@@ -154,7 +154,7 @@ def update
154154
end
155155

156156
def destroy
157-
checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
157+
checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user: forest_user)
158158
return head :forbidden unless checker.is_authorized?
159159

160160
collection_name = ForestLiana.name_for(@resource)
@@ -174,7 +174,7 @@ def destroy
174174
end
175175

176176
def destroy_bulk
177-
checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
177+
checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user: forest_user)
178178
return head :forbidden unless checker.is_authorized?
179179

180180
ids = ForestLiana::ResourcesGetter.get_ids_from_request(params, forest_user)

‎app/controllers/forest_liana/smart_actions_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def check_permission_for_smart_route
6363
find_resource(smart_action_request[:collection_name]),
6464
'actions',
6565
@rendering_id,
66-
user_id: forest_user['id'],
66+
user: forest_user,
6767
smart_action_request_info: get_smart_action_request_info
6868
)
6969
return head :forbidden unless checker.is_authorized?

‎app/controllers/forest_liana/stats_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def check_permission(permission_name)
105105
nil,
106106
permission_name,
107107
@rendering_id,
108-
user_id: forest_user['id'],
108+
user: forest_user,
109109
query_request_info: query_request
110110
)
111111

‎app/services/forest_liana/permissions_checker.rb

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ class PermissionsChecker
66

77
@@expiration_in_seconds = (ENV['FOREST_PERMISSIONS_EXPIRATION_IN_SECONDS'] || 3600).to_i
88

9-
def initialize(resource, permission_name, rendering_id, user_id: nil, smart_action_request_info: nil, collection_list_parameters: Hash.new, query_request_info: nil)
9+
ALLOWED_PERMISSION_LEVELS = %w[admin editor developer]
10+
11+
def initialize(resource, permission_name, rendering_id, user: nil, smart_action_request_info: nil, collection_list_parameters: Hash.new, query_request_info: nil)
1012
@collection_name = resource.present? ? ForestLiana.name_for(resource) : nil
1113
@permission_name = permission_name
1214
@rendering_id = rendering_id
1315

14-
@user_id = user_id
16+
@user = user
1517
@smart_action_request_info = smart_action_request_info
1618
@collection_list_parameters = collection_list_parameters
1719
@query_request_info = query_request_info
@@ -56,9 +58,9 @@ def is_allowed
5658

5759
# NOTICE: check liveQueries permissions
5860
if @permission_name === 'liveQueries'
59-
return live_query_allowed?
61+
return ALLOWED_PERMISSION_LEVELS.include?(@user['permission_level']) || live_query_allowed?
6062
elsif @permission_name === 'statWithParameters'
61-
return stat_with_parameters_allowed?
63+
return ALLOWED_PERMISSION_LEVELS.include?(@user['permission_level']) || stat_with_parameters_allowed?
6264
end
6365

6466
if permissions && permissions[@collection_name] &&
@@ -146,7 +148,7 @@ def get_smart_action_permissions(smart_actions_permissions)
146148
def is_user_allowed(permission_value)
147149
return false if permission_value.nil?
148150
return permission_value if permission_value.in? [true, false]
149-
permission_value.include?(@user_id.to_i)
151+
permission_value.include?(@user['id'].to_i)
150152
end
151153

152154
def smart_action_allowed?(smart_actions_permissions)

‎app/services/forest_liana/token.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ def self.create_token(user, rendering_id)
2222
role: user['role'],
2323
tags: user['tags'],
2424
rendering_id: rendering_id,
25-
exp: expiration_in_seconds()
25+
exp: expiration_in_seconds(),
26+
permission_level: user['permission_level'],
2627
}, ForestLiana.auth_secret, 'HS256')
2728
end
2829
end

‎spec/requests/actions_controller_spec.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
last_name: 'Kelso',
2626
team: 'Operations',
2727
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'
2930
}, ForestLiana.auth_secret, 'HS256')
3031
}
3132

‎spec/requests/count_spec.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
last_name: 'Kelso',
3030
team: 'Operations',
3131
rendering_id: 16,
32-
exp: Time.now.to_i + 2.weeks.to_i
32+
exp: Time.now.to_i + 2.weeks.to_i,
33+
permission_level: 'admin'
3334
}, ForestLiana.auth_secret, 'HS256')
3435

3536
headers = {

‎spec/requests/resources_spec.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
last_name: 'Kelso',
2929
team: 'Operations',
3030
rendering_id: 16,
31-
exp: Time.now.to_i + 2.weeks.to_i
31+
exp: Time.now.to_i + 2.weeks.to_i,
32+
permission_level: 'admin'
3233
}, ForestLiana.auth_secret, 'HS256')
3334

3435
headers = {

‎spec/requests/stats_spec.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
last_name: 'Kelso',
1111
team: 'Operations',
1212
rendering_id: 16,
13-
exp: Time.now.to_i + 2.weeks.to_i
13+
exp: Time.now.to_i + 2.weeks.to_i,
14+
permission_level: 'admin'
1415
}, ForestLiana.auth_secret, 'HS256')
1516

1617
headers = {
@@ -37,7 +38,7 @@
3738
allow(ForestLiana::IpWhitelist).to receive(:retrieve) { true }
3839
allow(ForestLiana::IpWhitelist).to receive(:is_ip_whitelist_retrieved) { true }
3940
allow(ForestLiana::IpWhitelist).to receive(:is_ip_valid) { true }
40-
41+
4142
allow_any_instance_of(ForestLiana::PermissionsChecker).to receive(:is_authorized?) { true }
4243

4344
allow_any_instance_of(ForestLiana::ValueStatGetter).to receive(:perform) { true }
@@ -81,7 +82,7 @@
8182
expect(response.status).to eq(403)
8283
end
8384
end
84-
85+
8586
describe 'POST /stats' do
8687
params = { query: 'SELECT COUNT(*) AS value FROM products;' }
8788

@@ -107,7 +108,7 @@
107108

108109
it 'should respond 422 with unprocessable query' do
109110
allow_any_instance_of(ForestLiana::QueryStatGetter).to receive(:perform) { raise ForestLiana::Errors::LiveQueryError.new }
110-
111+
111112
post '/forest/stats', params: JSON.dump(params), headers: headers
112113
expect(response.status).to eq(422)
113114
end

0 commit comments

Comments
 (0)