Skip to content

Commit e9932a6

Browse files
committed
enhance error handling
1 parent d227ca5 commit e9932a6

File tree

4 files changed

+20
-17
lines changed

4 files changed

+20
-17
lines changed

app/controllers/v3/application_controller.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class ApplicationController < ActionController::Base
9393
rescue_from CloudController::Errors::CompoundError, with: :handle_compound_error
9494
rescue_from ActionDispatch::Http::Parameters::ParseError, with: :handle_invalid_request_body
9595
rescue_from Sequel::DatabaseConnectionError, Sequel::DatabaseDisconnectError, with: :handle_db_connection_error
96+
rescue_from OpenSSL::Cipher::CipherError, with: :handle_decryption_error
9697

9798
def configuration
9899
Config.config
@@ -219,6 +220,11 @@ def handle_db_connection_error(_)
219220
handle_api_error(error)
220221
end
221222

223+
def handle_decryption_error(_)
224+
error = CloudController::Errors::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials')
225+
handle_api_error(error)
226+
end
227+
222228
def handle_exception(error)
223229
presenter = ErrorPresenter.new(error, Rails.env.test?, V3ErrorHasher.new(error))
224230
logger.info(presenter.log_message)

app/controllers/v3/service_credential_bindings_controller.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,15 @@ def details
129129
ensure_service_credential_binding_is_accessible!
130130
not_found! unless can_read_secrets_in_the_binding_space?
131131
not_found_with_message!(service_credential_binding) unless service_credential_binding.create_succeeded?
132-
132+
# begin
133133
credentials = if service_credential_binding.is_a?(ServiceKey) && service_credential_binding.credhub_reference?
134134
fetch_credentials_value(service_credential_binding.credhub_reference)
135135
else
136136
begin
137137
service_credential_binding.credentials
138-
rescue StandardError => e
138+
rescue VCAP::CloudController::Encryptor::EncryptorError => e
139139
logger.error("Failed to decrypt credentials: #{e.message}")
140-
raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity',
141-
"Cannot read credentials for \
142-
service_credential_binding \
143-
with guid: #{service_credential_binding.guid}")
140+
raise CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials')
144141
end
145142
end
146143

lib/cloud_controller/encryptor.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
module VCAP::CloudController
99
module Encryptor
1010
ENCRYPTION_ITERATIONS = 2048
11-
11+
class EncryptorError < StandardError; end
1212
class << self
1313
ALGORITHM = 'AES-128-CBC'.freeze
1414

@@ -47,11 +47,11 @@ def decrypt(encrypted_input, salt, iterations:, label: nil)
4747
return unless encrypted_input
4848

4949
key = key_to_use(label)
50-
begin
50+
# begin
5151
decrypt_raw(encrypted_input, key, salt, iterations:)
52-
rescue OpenSSL::Cipher::CipherError, StandardError => e
53-
raise StandardError.new("Decryption failed: #{e.message}")
54-
end
52+
# rescue OpenSSL::Cipher::CipherError, StandardError => e
53+
# raise StandardError.new("Decryption failed: #{e.message}")
54+
# end
5555
end
5656

5757
def decrypt_raw(encrypted_input, key, salt, iterations:)
@@ -88,9 +88,9 @@ def run_cipher(cipher, input, salt, key, iterations:)
8888
else
8989
begin
9090
cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256'))
91-
rescue StandardError => e
91+
rescue OpenSSL::Cipher::CipherError => e
9292
logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") if key.nil?
93-
raise
93+
raise EncryptorError
9494
end
9595
cipher.iv = salt
9696
end

spec/request/service_credential_bindings_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -631,17 +631,17 @@ def check_filtered_bindings(*bindings)
631631
foo: 'fooencryptionkey',
632632
death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey'
633633
}
634+
allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false)
634635
end
635636

636637
it 'fails to decrypt the credentials and returns a 500 error' do
637638
app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label')
638639

639-
allow(VCAP::CloudController::Encryptor).to receive(:decrypt_raw).and_raise(StandardError.new('Decryption failed'))
640-
640+
allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(VCAP::CloudController::Encryptor::EncryptorError)
641641
api_call.call(admin_headers)
642642

643-
expect(last_response).to have_status_code(422)
644-
expect(parsed_response['errors'].first['detail']).to match(/Cannot read credentials/i)
643+
expect(last_response).to have_status_code(500)
644+
expect(parsed_response['errors'].first['detail']).to match(/Failed/i)
645645
end
646646
end
647647

0 commit comments

Comments
 (0)