From 4bbd710f3ec8008bd1c52fda6a3a2440f3b35cd1 Mon Sep 17 00:00:00 2001 From: Lex Alexander Date: Tue, 17 Dec 2024 20:07:37 -0800 Subject: [PATCH 1/4] Prevent Opening JSON Key Path unless It's an IO Object --- lib/fcm.rb | 22 ++++++++++++++++++---- spec/fcm_spec.rb | 43 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 3622c77..1bd781f 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -4,6 +4,8 @@ require "googleauth" class FCM + class InvalidCredentialError < StandardError; end + BASE_URI = "https://fcm.googleapis.com" BASE_URI_V1 = "https://fcm.googleapis.com/v1/projects/" DEFAULT_TIMEOUT = 30 @@ -291,11 +293,23 @@ def jwt_token token["access_token"] end + def credentials_error_msg(json_key_path) + param_klass = if @json_key_path.nil? + 'nil' + else + "a #{@json_key_path.class.name}" + end + + "credentials must be an IO-like object or a path. You passed #{param_klass}" + end + def json_key @json_key ||= if @json_key_path.respond_to?(:read) - @json_key_path - else - File.open(@json_key_path) - end + @json_key_path + elsif (@json_key_path.is_a?(String) || @json_key_path.respond_to?(:open)) && File.file?(@json_key_path) + File.open(@json_key_path) + else + raise InvalidCredentialError, credentials_error_msg(@json_key_path) + end end end diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index a3f24e1..e87bda3 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -13,15 +13,31 @@ } end + let(:json_credentials) do + { + "type": 'service_account', + "project_id": 'example', + "private_key_id": 'c09c4593eee53707ca9f4208fbd6fe72b29fc7ab', + "private_key": OpenSSL::PKey::RSA.new(2048), + "client_email": '83315528762cf7e0-7bbcc3aad87e0083391bc7f234d487c8@developer.gserviceaccount.com', + "client_id": 'acedc3c0a63b3562376386f0-f3b94aafbecd0e7d60563bf7bb8bb47f.apps.googleusercontent.com', + "auth_uri": 'https://accounts.google.com/o/oauth2/auth', + "token_uri": 'https://oauth2.googleapis.com/token', + "auth_provider_x509_cert_url": 'https://www.googleapis.com/oauth2/v1/certs', + "client_x509_cert_url": 'https://www.googleapis.com/robot/v1/metadata/x509/fd6b61037dd2bb8585527679-7bbcc3aad87e0083391bc7f234d487c8%40developer.gserviceaccount.com', + "universe_domain": 'googleapis.com' + }.to_json + end + before do allow(client).to receive(:json_key) # Mock the Google::Auth::ServiceAccountCredentials - allow(Google::Auth::ServiceAccountCredentials).to receive(:make_creds). - and_return(double(fetch_access_token!: { 'access_token' => mock_token })) + allow(Google::Auth::ServiceAccountCredentials).to receive(:make_creds) + .and_return(double(fetch_access_token!: { 'access_token' => mock_token })) end - it "should initialize" do + it 'should initialize' do expect { client }.not_to raise_error end @@ -35,6 +51,27 @@ fcm = FCM.new(StringIO.new("hey")) expect(fcm.__send__(:json_key).class).to eq(StringIO) end + + it "raises an error when passed a non-existent credentials file path" do + fcm = FCM.new('spec/fake_credentials.json', '', {}) + expect { fcm.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) + end + + it "raises an error when passed a string of a file that does not exist" do + fcm = FCM.new("fake_credentials.json", '', {}) + expect { fcm.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) + end + + it 'raises an error when passed a non IO-like object' do + fcm_with_non_io_objects = [ + fcm_with_nil_creds = FCM.new(nil, '', {}), + fcm_with_hash_creds = FCM.new({}, '', {}), + fcm_with_json = FCM.new(json_credentials, '', {}) + ] + fcm_with_non_io_objects.each do |fcm_with_non_io_object| + expect { fcm_with_non_io_object.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) + end + end end describe "#send_v1 or #send_notification_v1" do From bb135568689331b615f2050d696fcbd599fa55c1 Mon Sep 17 00:00:00 2001 From: Lex Alexander Date: Tue, 17 Dec 2024 21:18:37 -0800 Subject: [PATCH 2/4] Fix linting issues --- lib/fcm.rb | 28 +++++++++++++----------- spec/fcm_spec.rb | 56 ++++++++++++++++++++++++++++++------------------ 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 1bd781f..b2f6edf 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -293,23 +293,25 @@ def jwt_token token["access_token"] end - def credentials_error_msg(json_key_path) - param_klass = if @json_key_path.nil? - 'nil' - else - "a #{@json_key_path.class.name}" - end + def credentials_error_msg(param) + error_msg = 'credentials must be an IO-like ' \ + 'object or path You passed ' + + error_msg += param.class.name.to_s + raise InvalidCredentialError, error_msg + end - "credentials must be an IO-like object or a path. You passed #{param_klass}" + def filename_or_io_like?(path) + (path.is_a?(String) || path.respond_to?(:open)) && File.file?(path) end def json_key @json_key ||= if @json_key_path.respond_to?(:read) - @json_key_path - elsif (@json_key_path.is_a?(String) || @json_key_path.respond_to?(:open)) && File.file?(@json_key_path) - File.open(@json_key_path) - else - raise InvalidCredentialError, credentials_error_msg(@json_key_path) - end + @json_key_path + elsif filename_or_io_like?(@json_key_path) + File.open(@json_key_path) + else + credentials_error_msg(@json_key_path) + end end end diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index e87bda3..b5fbe86 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -13,18 +13,33 @@ } end + let(:client_email) do + '83315528762cf7e0-7bbcc3aad87e0083391bc7f234d487' \ + 'c8@developer.gserviceaccount.com' + end + + let(:client_x509_cert_url) do + 'https://www.googleapis.com/robot/v1/metadata/x509/' \ + 'fd6b61037dd2bb8585527679" + "-7bbcc3aad87e0083391b' \ + 'c7f234d487c8%40developer.gserviceaccount.com' + end + + let(:creds_error) do + FCM::InvalidCredentialError + end + let(:json_credentials) do { "type": 'service_account', "project_id": 'example', "private_key_id": 'c09c4593eee53707ca9f4208fbd6fe72b29fc7ab', "private_key": OpenSSL::PKey::RSA.new(2048), - "client_email": '83315528762cf7e0-7bbcc3aad87e0083391bc7f234d487c8@developer.gserviceaccount.com', - "client_id": 'acedc3c0a63b3562376386f0-f3b94aafbecd0e7d60563bf7bb8bb47f.apps.googleusercontent.com', + "client_email": client_email, + "client_id": 'acedc3c0a63b3562376386f0.apps.googleusercontent.com', "auth_uri": 'https://accounts.google.com/o/oauth2/auth', "token_uri": 'https://oauth2.googleapis.com/token', "auth_provider_x509_cert_url": 'https://www.googleapis.com/oauth2/v1/certs', - "client_x509_cert_url": 'https://www.googleapis.com/robot/v1/metadata/x509/fd6b61037dd2bb8585527679-7bbcc3aad87e0083391bc7f234d487c8%40developer.gserviceaccount.com', + "client_x509_cert_url": client_x509_cert_url, "universe_domain": 'googleapis.com' }.to_json end @@ -42,35 +57,34 @@ end describe "credentials path" do - it "can be a path to a file" do + it 'can be a path to a file' do fcm = FCM.new("README.md") expect(fcm.__send__(:json_key).class).to eq(File) end - it "can be an IO object" do - fcm = FCM.new(StringIO.new("hey")) + it 'can be an IO object' do + fcm = FCM.new(StringIO.new('hey')) expect(fcm.__send__(:json_key).class).to eq(StringIO) end - it "raises an error when passed a non-existent credentials file path" do - fcm = FCM.new('spec/fake_credentials.json', '', {}) - expect { fcm.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) + it 'raises an error when passed a non IO-like object' do + [ + FCM.new(nil, '', {}), + FCM.new({}, '', {}), + FCM.new(json_credentials, '', {}) + ].each do |fcm| + expect { fcm.__send__(:json_key) }.to raise_error(creds_error) + end end - it "raises an error when passed a string of a file that does not exist" do - fcm = FCM.new("fake_credentials.json", '', {}) - expect { fcm.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) + it 'raises an error when passed a non-existent credentials file path' do + fcm = FCM.new('spec/fake_credentials.json', '', {}) + expect { fcm.__send__(:json_key) }.to raise_error(creds_error) end - it 'raises an error when passed a non IO-like object' do - fcm_with_non_io_objects = [ - fcm_with_nil_creds = FCM.new(nil, '', {}), - fcm_with_hash_creds = FCM.new({}, '', {}), - fcm_with_json = FCM.new(json_credentials, '', {}) - ] - fcm_with_non_io_objects.each do |fcm_with_non_io_object| - expect { fcm_with_non_io_object.__send__(:json_key).class }.to raise_error(FCM::InvalidCredentialError) - end + it 'raises an error when passed a string of a file that does not exist' do + fcm = FCM.new('fake_credentials.json', '', {}) + expect { fcm.__send__(:json_key) }.to raise_error(creds_error) end end From fc8c60a8f2e68834740ed30c75a61e3e94c30eef Mon Sep 17 00:00:00 2001 From: Lex Alexander Date: Tue, 17 Dec 2024 23:02:58 -0800 Subject: [PATCH 3/4] Update checking for a valid credentials paths and filename --- lib/fcm.rb | 16 +++++++++++----- spec/fcm_spec.rb | 46 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index b2f6edf..192ffe5 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -295,20 +295,26 @@ def jwt_token def credentials_error_msg(param) error_msg = 'credentials must be an IO-like ' \ - 'object or path You passed ' + 'object or path. You passed' - error_msg += param.class.name.to_s + param_klass = param.nil? ? 'nil' : "a #{param.class.name}" + error_msg += " #{param_klass}." raise InvalidCredentialError, error_msg end - def filename_or_io_like?(path) - (path.is_a?(String) || path.respond_to?(:open)) && File.file?(path) + def open_json_key_path?(path) + valid_io_object = path.respond_to?(:open) + return true if valid_io_object && File.file?(path) + + max_path_len = 1024 + valid_path = path.is_a?(String) && path.length <= max_path_len + valid_path && File.file?(path) end def json_key @json_key ||= if @json_key_path.respond_to?(:read) @json_key_path - elsif filename_or_io_like?(@json_key_path) + elsif open_json_key_path?(@json_key_path) File.open(@json_key_path) else credentials_error_msg(@json_key_path) diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index b5fbe86..4332ba1 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -1,4 +1,5 @@ require "spec_helper" +require 'tempfile' describe FCM do let(:project_name) { 'test-project' } @@ -24,6 +25,10 @@ 'c7f234d487c8%40developer.gserviceaccount.com' end + let(:large_file_name) do + Array.new(1021) { 'a' }.join('') + '.txt' + end + let(:creds_error) do FCM::InvalidCredentialError end @@ -33,7 +38,7 @@ "type": 'service_account', "project_id": 'example', "private_key_id": 'c09c4593eee53707ca9f4208fbd6fe72b29fc7ab', - "private_key": OpenSSL::PKey::RSA.new(2048), + "private_key": OpenSSL::PKey::RSA.new(2048).to_s, "client_email": client_email, "client_id": 'acedc3c0a63b3562376386f0.apps.googleusercontent.com', "auth_uri": 'https://accounts.google.com/o/oauth2/auth', @@ -62,19 +67,42 @@ expect(fcm.__send__(:json_key).class).to eq(File) end + it 'raises an error when passed a large path' do + expect do + FCM.new(large_file_name).__send__(:json_key) + end.to raise_error(creds_error) + end + it 'can be an IO object' do fcm = FCM.new(StringIO.new('hey')) expect(fcm.__send__(:json_key).class).to eq(StringIO) + + temp_file = Tempfile.new('hello_world.json') + temp_file.write(json_credentials) + fcm_with_temp_file = FCM.new(temp_file) + + expect do + fcm_with_temp_file + end.not_to raise_error + temp_file.close + temp_file.unlink end it 'raises an error when passed a non IO-like object' do - [ - FCM.new(nil, '', {}), - FCM.new({}, '', {}), - FCM.new(json_credentials, '', {}) - ].each do |fcm| - expect { fcm.__send__(:json_key) }.to raise_error(creds_error) - end + expect do + FCM.new(nil, '', {}).__send__(:json_key) + end.to raise_error(creds_error, 'credentials must be' \ + ' an IO-like object or path. You passed nil.') + + expect do + FCM.new(json_credentials, '', {}).__send__(:json_key) + end.to raise_error(creds_error, 'credentials must be' \ + ' an IO-like object or path. You passed a String.') + + expect do + FCM.new({}, '', {}).__send__(:json_key) + end.to raise_error(creds_error, 'credentials must be' \ + ' an IO-like object or path. You passed a Hash.') end it 'raises an error when passed a non-existent credentials file path' do @@ -83,7 +111,7 @@ end it 'raises an error when passed a string of a file that does not exist' do - fcm = FCM.new('fake_credentials.json', '', {}) + fcm = FCM.new('example.txt', '', {}) expect { fcm.__send__(:json_key) }.to raise_error(creds_error) end end From 7a7b0c49fa8572d676a7c4e56388759c932f45f6 Mon Sep 17 00:00:00 2001 From: Lex Alexander Date: Wed, 18 Dec 2024 00:44:26 -0800 Subject: [PATCH 4/4] Rename open_json_key_path to valid_json_key_path? --- lib/fcm.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 192ffe5..3a65b7a 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -302,7 +302,7 @@ def credentials_error_msg(param) raise InvalidCredentialError, error_msg end - def open_json_key_path?(path) + def valid_json_key_path?(path) valid_io_object = path.respond_to?(:open) return true if valid_io_object && File.file?(path) @@ -314,7 +314,7 @@ def open_json_key_path?(path) def json_key @json_key ||= if @json_key_path.respond_to?(:read) @json_key_path - elsif open_json_key_path?(@json_key_path) + elsif valid_json_key_path?(@json_key_path) File.open(@json_key_path) else credentials_error_msg(@json_key_path)