Skip to content

Commit

Permalink
feat(action): add permission on action routes (#27)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolasalexandre9 authored Mar 1, 2024
1 parent 79cdff5 commit c7361f7
Show file tree
Hide file tree
Showing 23 changed files with 318 additions and 190 deletions.
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ Metrics/ClassLength:
- 'packages/forest_admin_agent/lib/forest_admin_agent/utils/schema/generator_field.rb'
- 'packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb'
- 'packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb'
- 'packages/forest_admin_agent/lib/forest_admin_agent/routes/action/action.rb'
- 'packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/utils/collection.rb'
- 'packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/components/query/aggregation.rb'
- 'packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/components/query/filter_factory.rb'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ class ConflictError < HttpException
attr_reader :name

def initialize(message, name = 'ConflictError')
@name = name
super(429, 'Conflict', message)
super(429, message, name)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ class ForbiddenError < HttpException
attr_reader :name

def initialize(message, name = 'ForbiddenError')
@name = name
super(403, 'Forbidden', message)
super(403, message, name)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ module ForestAdminAgent
module Http
module Exceptions
class HttpException < StandardError
attr_reader :code, :status, :message, :name
attr_reader :status, :message, :name

def initialize(code, status, message, name = nil)
def initialize(status, message, name = nil)
super(message)
@code = code
@status = status
@message = message
@name = name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ class RequireApproval < HttpException
attr_reader :name, :data

def initialize(message, name = 'RequireApproval', data = [])
@name = name
super(403, message, name)
@data = data
super(403, 'Forbidden', message)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module ForestAdminAgent
module Http
module Exceptions
class UnprocessableError < HttpException
attr_reader :name

def initialize(message = '', name = 'UnprocessableError')
super(422, message, name)
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'jsonapi-serializers'
require 'active_support/inflector'
require 'jwt'

module ForestAdminAgent
module Routes
Expand Down Expand Up @@ -41,10 +42,11 @@ def setup_routes

def handle_request(args = {})
build(args)
args = middleware_custom_action_approval_request_data(args)
filter_for_caller = get_record_selection(args)
get_record_selection(args, include_user_scope: false)

# TODO: permission
@permissions.can_smart_action?(args, @collection, filter_for_caller)

raw_data = args.dig(:params, :data, :attributes, :values)

Expand Down Expand Up @@ -94,6 +96,25 @@ def handle_hook_request(args = {})

private

def middleware_custom_action_approval_request_data(args)
raise Http::Exceptions::UnprocessableError if args.dig(:params, :data, :attributes, :requester_id)

if (signed_request = args.dig(:params, :data, :attributes, :signed_approval_request))
args[:params][:data][:attributes][:signed_approval_request] = decode_signed_approval_request(signed_request)
end

args
end

def decode_signed_approval_request(signed_request)
ForestAdminDatasourceToolkit::Utils::HashHelper.convert_keys(JWT.decode(
signed_request,
Facades::Container.cache(:env_secret),
true,
{ algorithm: 'HS256' }
)[0])
end

def get_record_selection(args, include_user_scope: true)
attributes = args.dig(:params, :data, :attributes)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def can_smart_action?(request, collection, filter, allow_fetch: true)
smart_action_approval = SmartActionChecker.new(
request[:params],
collection,
collections_data[collection.name.to_sym][:actions][action[:name]],
collections_data[collection.name.to_sym][:actions][action['name'].to_sym],
caller,
user_data[:roleId],
filter
Expand Down Expand Up @@ -211,7 +211,7 @@ def find_action_from_endpoint(collection_name, endpoint, http_method)

return nil if actions.nil? || actions.empty?

action = actions.find { |a| a['endpoint'] == endpoint && a['http_method'].casecmp(http_method).zero? }
action = actions.find { |a| a['endpoint'] == endpoint && a['httpMethod'].casecmp(http_method).zero? }

raise ForestException, "The collection #{collection_name} does not have this smart action" if action.nil?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ def can_execute?
def can_approve?
if smart_action[:userApprovalEnabled].include?(role_id) &&
(smart_action[:userApprovalConditions].empty? || match_conditions(:userApprovalConditions)) &&
(attributes[:requester_id] != caller.id || smart_action[:selfApprovalEnabled].include?(role_id))
(attributes[:signed_approval_request][:data][:attributes][:requester_id] != caller.id ||
smart_action[:selfApprovalEnabled].include?(role_id))
return true
end

Expand Down Expand Up @@ -70,8 +71,7 @@ def match_conditions(condition_name)
else
Nodes::ConditionTreeLeaf.new(pk, 'IN', attributes[:ids])
end

condition = smart_action[condition_name][0]['filter']
condition = smart_action[condition_name][0][:filter]
conditional_filter = filter.override(
condition_tree: ConditionTreeFactory.intersect(
[
Expand All @@ -81,9 +81,9 @@ def match_conditions(condition_name)
]
)
)
rows = collection.aggregate(caller, conditional_filter, Aggregation.new(operation: 'Count'))

(rows[0]['value'] || 0) == attributes[:ids].count
rows = collection.aggregate(caller, conditional_filter, Aggregation.new(operation: 'Count'))
(rows.empty? ? 0 : rows[0]['value']) == attributes[:ids].count
rescue StandardError
raise ConflictError.new(
'The conditions to trigger this action cannot be verified. Please contact an administrator.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ class ConditionTreeParser

def self.from_plain_object(collection, filters)
if leaf?(filters)
operator = filters['operator'].titleize.tr(' ', '_')
operator = filters[:operator].titleize.tr(' ', '_')
value = parse_value(collection, filters)

return ConditionTreeLeaf.new(filters['field'], operator, value)
return ConditionTreeLeaf.new(filters[:field], operator, value)
end

if branch?(filters)
aggregator = filters['aggregator'].capitalize
aggregator = filters[:aggregator].capitalize
conditions = []
filters['conditions'].each do |sub_tree|
filters[:conditions].each do |sub_tree|
conditions << from_plain_object(collection, sub_tree)
end

Expand All @@ -31,11 +31,11 @@ def self.from_plain_object(collection, filters)
end

def self.parse_value(collection, leaf)
schema = Collection.get_field_schema(collection, leaf['field'])
operator = leaf['operator'].titleize.tr(' ', '_')
schema = Collection.get_field_schema(collection, leaf[:field])
operator = leaf[:operator].titleize.tr(' ', '_')

if operator == Operators::IN && leaf['field'].is_a?(String)
values = leaf['value'].split(',').map(&:strip)
if operator == Operators::IN && leaf[:field].is_a?(String)
values = leaf[:value].split(',').map(&:strip)

return values.map { |item| !%w[false 0 no].include?(item) } if schema.column_type == 'Boolean'

Expand All @@ -44,15 +44,15 @@ def self.parse_value(collection, leaf)
return values
end

leaf['value']
leaf[:value]
end

def self.leaf?(filters)
filters.key?('field') && filters.key?('operator')
filters.key?(:field) && filters.key?(:operator)
end

def self.branch?(filters)
filters.key?('aggregator') && filters.key?('conditions')
filters.key?(:aggregator) && filters.key?(:conditions)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ def self.parse_condition_tree(collection, args)

return if filters.nil?

filters = JSON.parse(filters) if filters.is_a? String
filters = JSON.parse(filters, symbolize_names: true) if filters.is_a? String
# TODO: add else for convert all keys to sym

ConditionTreeParser.from_plain_object(collection, filters)
# TODO: ConditionTreeValidator::validate($conditionTree, $collection);
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

RSpec.describe ForestAdminAgent do
it "has a version number" do
expect(ForestAdminAgent::VERSION).not_to eq 1
expect(described_class::VERSION).not_to eq 1
end
# pending "should something"
end
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ module Action

allow(ForestAdminAgent::Services::Permissions).to receive(:new).and_return(permissions)
allow(permissions).to receive_messages(
can_chart?: true,
can_smart_action?: true,
get_scope: nil,
get_user_data: {
id: 1,
Expand All @@ -88,6 +88,49 @@ module Action
)
end

describe 'checking user authorization' do
before do
@action_collection.add_action(
'foo',
BaseAction.new(
scope: Types::ActionScope::GLOBAL,
form: [
DynamicField.new(type: Types::FieldType::STRING, label: 'firstname')
]
) do |_context, result_builder|
result_builder.success
end
)
end

context 'when the request contains requester_id' do
it 'reject with UnprocessableError (prevent forged request)' do
args[:params][:data][:attributes][:requester_id] = 'requester_id'
allow(@action_collection).to receive(:execute)

expect { action.handle_request(args) }.to raise_error(Http::Exceptions::UnprocessableError)
end
end

context 'when the request is an approval' do
it 'get the signed parameters and change body' do
unsigned_request = { foo: 'value' }
signed_request = JWT.encode(
unsigned_request,
Facades::Container.cache(:env_secret),
'HS256'
)
args[:params][:data][:attributes][:signed_approval_request] = signed_request
allow(@action_collection).to receive(:execute)
action.handle_request(args)

expect(permissions).to have_received(:can_smart_action?) do |args, _collection, _filter_for_caller|
expect(args[:params][:data][:attributes][:signed_approval_request]).to eq(unsigned_request)
end
end
end
end

context 'with a single action used from list-view, detail-view & summary' do
before do
@action_collection.add_action(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,17 +473,22 @@ module Charts

describe 'inject_context_variables' do
it 'overrides the filter with the context variables' do
args[:params] = args[:params].merge({
type: 'Value',
sourceCollectionName: 'book',
aggregateFieldName: nil,
aggregator: 'Count',
filter: { 'aggregator' => 'and',
'conditions' => [{ 'operator' => 'equal',
'value' => '{{dropdown1.selectedValue}}', 'field' => 'title' }] },
contextVariables: { 'dropdown1.selectedValue' => 'FOO' },
timezone: 'Europe/Paris'
})
args[:params] = args[:params].merge(
{
type: 'Value',
sourceCollectionName: 'book',
aggregateFieldName: nil,
aggregator: 'Count',
filter: JSON.generate({
aggregator: 'and',
conditions: [
{ operator: 'equal', value: '{{dropdown1.selectedValue}}', field: 'title' }
]
}),
contextVariables: { 'dropdown1.selectedValue' => 'FOO' },
timezone: 'Europe/Paris'
}
)
allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ value: 10, group: [] }])
chart.handle_request(args)

Expand Down
Loading

0 comments on commit c7361f7

Please sign in to comment.