diff --git a/app/controllers/key_pairs_controller.rb b/app/controllers/key_pairs_controller.rb index 23a57005bd..7bd4558ffe 100644 --- a/app/controllers/key_pairs_controller.rb +++ b/app/controllers/key_pairs_controller.rb @@ -1,7 +1,7 @@ class KeyPairsController < ApplicationController include Foreman::Controller::ComputeResourcesCommon - before_action :find_compute_resource, :except => [:show] + before_action :find_compute_resource before_action :find_resource, :only => [:show] def index @@ -44,7 +44,8 @@ def find_compute_resource end def find_resource - @key_pair = KeyPair.find(params[:id]) + @key_pair = @compute_resource.key_pair + return not_found unless @key_pair && @key_pair.id.to_s == params[:id].to_s end def action_permission diff --git a/test/controllers/key_pairs_controller_test.rb b/test/controllers/key_pairs_controller_test.rb index 2f7cad89c4..229d2590cc 100644 --- a/test/controllers/key_pairs_controller_test.rb +++ b/test/controllers/key_pairs_controller_test.rb @@ -17,13 +17,34 @@ class KeyPairsControllerTest < ActionController::TestCase end test "should download pem file" do - key = FactoryBot.create(:key_pair) + key = FactoryBot.create(:key_pair, :compute_resource => @compute_resource) get :show, params: { :compute_resource_id => @compute_resource.to_param, :id => key.id }, session: set_session_user assert_response :success assert_equal(key.secret, @response.body) refute @response.body.size.zero? end + test "should not download key pair when id belongs to a different compute resource" do + other_cr = FactoryBot.create(:ec2_cr) + other_key = FactoryBot.create(:key_pair, :compute_resource => other_cr) + get :show, params: { :compute_resource_id => @compute_resource.to_param, :id => other_key.id }, session: set_session_user + assert_response :not_found + end + + test "should not download another organization's key pair using own compute resource in url" do + org1 = taxonomies(:organization1) + org2 = taxonomies(:organization2) + loc = taxonomies(:location1) + cr_owned = FactoryBot.create(:ec2_cr, :organizations => [org1], :locations => [loc]) + cr_other = FactoryBot.create(:ec2_cr, :organizations => [org2], :locations => [loc]) + FactoryBot.create(:key_pair, :compute_resource => cr_owned) + other_key = FactoryBot.create(:key_pair, :compute_resource => cr_other) + + setup_user "view", "keypairs", nil, :one + get :show, params: { :compute_resource_id => cr_owned.to_param, :id => other_key.id }, session: set_session_user(:one) + assert_response :not_found + end + test "should recreate a key pair" do Foreman::Model::EC2.any_instance.stubs(:recreate).returns(KeyPair.create(:name => "foreman-#{Foreman.uuid}", :secret => "shhh",