From ba409f6ad5434078336608c549817523c678cd1c Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Wed, 28 Oct 2026 11:52:17 +0100 Subject: [PATCH] fix(agent): align action approval requests with the signed payload [PRD-1764] --- .../routes/action/actions.rb | 29 ++- .../routes/action/actions_spec.rb | 5 +- .../routes/resources/action/actions_spec.rb | 177 +++++++++++++++++- .../services/smart_action_checker_spec.rb | 31 +++ 4 files changed, 232 insertions(+), 10 deletions(-) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb index 6b9689fa7..b8775081a 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb @@ -1,5 +1,7 @@ require 'jsonapi-serializers' require 'active_support/inflector' +require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/object/deep_dup' require 'jwt' module ForestAdminAgent @@ -50,7 +52,7 @@ def setup_routes def handle_request(args = {}) context = build(args) ensure_served_action(context) - args = middleware_custom_action_approval_request_data(args) + args = middleware_custom_action_approval_request_data(args, context) filter_for_caller = get_record_selection(args, context) context.permissions.can_smart_action?(args, context.collection, filter_for_caller, action_name: @action_name) @@ -210,23 +212,38 @@ def audited_record_ids(context, filter) [] end - def middleware_custom_action_approval_request_data(args) + def middleware_custom_action_approval_request_data(args, context) 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) + signed_request = args.dig(:params, :data, :attributes, :signed_approval_request) + return args if signed_request.nil? + + payload = decode_signed_approval_request(signed_request) + data = payload[:data] + attributes = data[:attributes] if data.is_a?(ActiveSupport::HashWithIndifferentAccess) + + unless attributes.is_a?(ActiveSupport::HashWithIndifferentAccess) && + attributes[:collection_name] == context.collection.name + raise Http::Exceptions::ForbiddenError, 'This approval request was not signed for this collection.' end + raise Http::Exceptions::ForbiddenError, 'Approval must name its records.' if attributes[:all_records] + attributes[:signed_approval_request] = payload.deep_dup + args[:params][:data] = data args end def decode_signed_approval_request(signed_request) - ForestAdminDatasourceToolkit::Utils::HashHelper.convert_keys(JWT.decode( + raise Http::Exceptions::ForbiddenError, 'Invalid approval request.' unless signed_request.is_a?(String) + + JWT.decode( signed_request, Facades::Container.cache(:env_secret), true, { algorithm: 'HS256' } - )[0]) + )[0].with_indifferent_access + rescue JWT::DecodeError + raise Http::Exceptions::ForbiddenError, 'Invalid approval request.' end def get_record_selection(args, context) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb index 7a0be720a..47e6e31c7 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb @@ -193,7 +193,10 @@ def request(attributes) end it 'refuses the approval of a request naming two ids and runs nothing' do - signed = JWT.encode({ data: { attributes: { ids: %w[4 7] } } }, Facades::Container.cache(:env_secret), 'HS256') + signed = JWT.encode( + { data: { attributes: { ids: %w[4 7], collection_name: 'orders' } } }, + Facades::Container.cache(:env_secret), 'HS256' + ) expect { route.handle_request(request(ids: %w[4 7], signed_approval_request: signed)) } .to raise_error(Http::Exceptions::UnprocessableError, message) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb index e651fdff7..1eb6c9649 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb @@ -155,20 +155,191 @@ def declare_actions(actions) end context 'when the request is an approval' do - it 'get the signed parameters and change body' do - unsigned_request = { foo: 'value' } + it 'hands the checker the decoded request and uses the signed payload' do + unsigned_request = { + data: { + attributes: { + requester_id: 20, + ids: ['123e4567-e89b-12d3-a456-426614174999'], + collection_name: 'book', + values: {} + } + } + } signed_request = JWT.encode( unsigned_request, Facades::Container.cache(:env_secret), 'HS256' ) args[:params][:data][:attributes][:signed_approval_request] = signed_request + args[:params][:data][:attributes][:values] = { 'firstname' => 'John' } allow(@action_collection).to receive(:execute).and_return({}) 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) + expect(args[:params][:data][:attributes][:signed_approval_request][:data][:attributes][:requester_id]).to eq(20) + expect(args[:params][:data][:attributes][:ids]).to eq(['123e4567-e89b-12d3-a456-426614174999']) end + expect(@action_collection).to have_received(:execute) do |_caller, _action, data, _filter| + expect(data).to eq({}) + end + end + end + end + + context 'when the request carries a signed approval request' do + let(:book_id) { '123e4567-e89b-12d3-a456-426614174000' } + let(:signed_id) { '123e4567-e89b-12d3-a456-426614174999' } + let(:other_id) { '123e4567-e89b-12d3-a456-426614174888' } + + def approval_token(attributes = {}, requester_id: 20, exp: Time.now.to_i + 600) + JWT.encode( + { + data: { + type: 'custom-action-requests', + id: 42, + attributes: { + requester_id: requester_id, + ids: [signed_id], + collection_name: 'book', + smart_action_id: 'smartActionId', + values: {}, + parent_collection_name: nil, + parent_collection_id: nil, + parent_association_name: nil, + all_records: false, + all_records_subset_query: nil + }.merge(attributes) + }, + approver: 1, + iat: Time.now.to_i, + exp: exp + }, + Facades::Container.cache(:env_secret), + 'HS256' + ) + end + + def approval_request(token, body_attributes = {}) + args[:params][:data][:attributes][:signed_approval_request] = token + args[:params][:data][:attributes].merge!(body_attributes) + args + end + + before do + @action_collection.add_action( + 'foo', + ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction.new( + scope: ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::BULK, + form: [ + { type: ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType::NUMBER, + label: 'amount' }, + { type: ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType::COLLECTION, + label: 'customer', collection_name: 'book' } + ] + ) { |_context, result_builder| result_builder.success } + ) + allow(@action_collection).to receive(:execute).and_return({}) + end + + it 'runs the action on the signed ids and hands the checker the signed requester' do + action.handle_request(approval_request(approval_token, ids: [other_id])) + + expect(@action_collection).to have_received(:execute) do |_caller, _name, _data, filter| + expect(filter.condition_tree.to_h).to eq({ field: 'id', operator: Operators::EQUAL, value: signed_id }) + end + expect(permissions).to have_received(:can_smart_action?) do |a, _collection, _filter| + expect(a.dig(:params, :data, :attributes, :signed_approval_request, :data, :attributes, :requester_id)).to eq(20) + end + end + + it 'runs the action with the signed values' do + action.handle_request(approval_request(approval_token({ 'values' => { 'amount' => 10 } }), values: { 'amount' => 999_999 })) + + expect(@action_collection).to have_received(:execute) do |_caller, _name, data, _filter| + expect(data).to eq('amount' => 10) + end + end + + it 'parses the signed values like the frontend ones: a collection field is unpacked, not passed through' do + action.handle_request(approval_request(approval_token({ 'values' => { 'customer' => book_id } }))) + + expect(@action_collection).to have_received(:execute) do |_caller, _name, data, _filter| + expect(data).to eq('customer' => [book_id]) + end + end + + it 'refuses a token signed for another collection before any permission check' do + expect do + action.handle_request(approval_request(approval_token({ 'collection_name' => 'other_collection' }))) + end.to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + + expect(permissions).not_to have_received(:can_smart_action?) + expect(@action_collection).not_to have_received(:execute) + end + + it 'refuses a token that does not say which collection it was signed for' do + expect do + action.handle_request(approval_request(approval_token({ 'collection_name' => nil }))) + end.to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + + expect(@action_collection).not_to have_received(:execute) + end + + it 'refuses a token whose data does not carry server-shaped attributes' do + [JWT.encode({ data: { type: 'custom-action-requests' } }, Facades::Container.cache(:env_secret), 'HS256'), + JWT.encode({ data: [] }, Facades::Container.cache(:env_secret), 'HS256')].each do |token| + expect { action.handle_request(approval_request(token)) } + .to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + end + + expect(@action_collection).not_to have_received(:execute) + end + + it 'refuses a token that selects all records instead of naming them' do + token = approval_token({ 'all_records' => true, 'all_records_subset_query' => { 'search' => 'needle' } }) + + expect { action.handle_request(approval_request(token)) } + .to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + + expect(permissions).not_to have_received(:can_smart_action?) + expect(@action_collection).not_to have_received(:execute) + end + + it 'refuses an expired token with a 403-class error, not a 500' do + expect do + action.handle_request(approval_request(approval_token(exp: Time.now.to_i - 10))) + end.to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + + expect(@action_collection).not_to have_received(:execute) + end + + it 'refuses a token signed with another secret, or not a token at all' do + other_secret_token = JWT.encode({ data: { attributes: { ids: [signed_id], collection_name: 'book' } } }, 'not-the-env-secret', 'HS256') + + [other_secret_token, 'abc', true, 123, [], {}, false].each do |token| + expect { action.handle_request(approval_request(token)) } + .to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError) + end + + expect(@action_collection).not_to have_received(:execute) + end + + it 'applies the outer query filters within the signed selection' do + args[:params][:filters] = { field: 'id', operator: Operators::EQUAL, value: book_id } + + action.handle_request(approval_request(approval_token({ 'ids' => [book_id, signed_id] }), ids: [book_id])) + + expect(@action_collection).to have_received(:execute) do |_caller, _name, _data, filter| + expect(filter.condition_tree.to_h).to eq( + { + aggregator: 'And', + conditions: [ + { field: 'id', operator: Operators::EQUAL, value: book_id }, + { field: 'id', operator: Operators::IN, value: [book_id, signed_id] } + ] + } + ) end end end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/services/smart_action_checker_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/services/smart_action_checker_spec.rb index 350e73a16..a5fad2558 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/services/smart_action_checker_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/services/smart_action_checker_spec.rb @@ -468,6 +468,37 @@ module Services expect(smart_action_checker).to be_can_execute end + it 'refuses to approve the caller own request on the signed payload shape' do + parameters[:data][:attributes][:signed_approval_request] = { + data: { + attributes: { + requester_id: 1 + } + } + }.with_indifferent_access + smart_action[:userApprovalEnabled] = [1] + + smart_action_checker = described_class.new(parameters, @datasource.get_collection('Book'), smart_action, + build_caller, 1, Filter.new) + expect { smart_action_checker.can_execute? } + .to raise_error(ForbiddenError, 'You don\'t have the permission to trigger this action.') + end + + it 'approves another requester request on the same shape' do + parameters[:data][:attributes][:signed_approval_request] = { + data: { + attributes: { + requester_id: 20 + } + } + }.with_indifferent_access + smart_action[:userApprovalEnabled] = [1] + + smart_action_checker = described_class.new(parameters, @datasource.get_collection('Book'), smart_action, + build_caller, 1, Filter.new) + expect(smart_action_checker).to be_can_execute + end + it 'returns true when the user can approve and there is no userApprovalConditions and user roleId is present into selfApprovalEnabled' do parameters[:data][:attributes][:signed_approval_request] = { data: {