From fc79612f26f2def50a4db6546ca9059f39fde37c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 20 Oct 2022 17:56:46 +1100 Subject: [PATCH] Prevent users from changing API keys for others It was checking for the permission to create a user which everyone can do. Now it's checking for updating that particular user and doesn't allow generating new keys for other users any more. This would have been an inconvenience but not a big security issue because you can't view the key of another user. --- app/controllers/spree/api_keys_controller.rb | 2 +- spec/controllers/spree/api_keys_controller_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/api_keys_controller.rb b/app/controllers/spree/api_keys_controller.rb index e852afc23d..22be03d362 100644 --- a/app/controllers/spree/api_keys_controller.rb +++ b/app/controllers/spree/api_keys_controller.rb @@ -32,7 +32,7 @@ module Spree def load_object @user ||= find_user if @user - authorize! params[:action].to_sym, @user + authorize! :update, @user else redirect_to main_app.login_path end diff --git a/spec/controllers/spree/api_keys_controller_spec.rb b/spec/controllers/spree/api_keys_controller_spec.rb index 68c5f35b5f..c39b46e821 100644 --- a/spec/controllers/spree/api_keys_controller_spec.rb +++ b/spec/controllers/spree/api_keys_controller_spec.rb @@ -9,6 +9,7 @@ describe Spree::ApiKeysController, type: :controller, performance: true do include ControllerRequestsHelper let(:user) { create(:user) } + let(:other_user) { create(:user) } let(:redirect_path) { "#{spree.account_path}#/developer_settings" } before do @@ -21,6 +22,15 @@ describe Spree::ApiKeysController, type: :controller, performance: true do expect(user.spree_api_key).to be_present end + it "denies creating a new api key for other user" do + expect { + spree_post :create, id: other_user.id + other_user.reload + }.to_not change { + other_user.spree_api_key + } + end + it "redirects to the api keys tab on account page " do spree_post :create expect(response).to redirect_to redirect_path