Hide internal error for CCRA and EPS services when some settings is not set (#21562)

Author: carlosfelix2

modules/vaos/app/exceptions/vaos/exceptions/configuration_error.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'common/exceptions'
+
+module VAOS
+  module Exceptions
+    ##
+    # ConfigurationError represents a configuration issue in the VAOS service
+    # that prevents it from operating properly. This is used to provide a clean
+    # API response when there are internal configuration issues.
+    #
+    class ConfigurationError < Common::Exceptions::ServiceError
+      attr_reader :error
+
+      ##
+      # Initialize a new ConfigurationError
+      #
+      # @param error [StandardError] The original error that occurred
+      # @param service_name [String] The name of the service that encountered the error
+      #
+      def initialize(error, service_name = 'VAOS')
+        @error = error
+
+        super(
+          detail: "The #{service_name} service is unavailable due to a configuration issue",
+          source: service_name
+        )
+      end
+
+      ##
+      # Returns the HTTP status code for this error
+      #
+      # @return [Integer] The HTTP status code (503 Service Unavailable)
+      #
+      def status
+        503 # Service Unavailable
+      end
+
+      ##
+      # Returns the error code
+      #
+      # @return [String] The error code
+      #
+      def code
+        'VAOS_CONFIG_ERROR'
+      end
+
+      ##
+      # Override i18n_data to provide custom error information
+      #
+      # @return [Hash] The error data
+      #
+      def i18n_data
+        {
+          title: 'Service Configuration Error',
+          detail: @detail,
+          code:,
+          status: status.to_s
+        }
+      end
+    end
+  end
+end

modules/vaos/app/services/common/jwt_wrapper.rb

@@ -1,24 +1,40 @@
 # frozen_string_literal: true
 
+require_relative '../../exceptions/vaos/exceptions/configuration_error'
+
 module Common
   class JwtWrapper
     SIGNING_ALGORITHM = 'RS512'
 
+    class ConfigurationError < StandardError; end
+
     attr_reader :expiration, :settings
 
     delegate :key_path, :client_id, :kid, :audience_claim_url, to: :settings
 
-    def initialize(service_settings)
+    def initialize(service_settings, service_config)
       @settings = service_settings
+      @config = service_config
       @expiration = 5
     end
 
     def sign_assertion
       JWT.encode(claims, rsa_key, SIGNING_ALGORITHM, jwt_headers)
+    rescue ConfigurationError => e
+      Rails.logger.error("Service Configuration Error: #{e.message}")
+      raise VAOS::Exceptions::ConfigurationError.new(e, @config.service_name)
     end
 
     def rsa_key
-      @rsa_key ||= OpenSSL::PKey::RSA.new(File.read(key_path))
+      raise ConfigurationError, 'RSA key path is not configured' if key_path.blank?
+
+      raise ConfigurationError, "RSA key file not found at: #{key_path}" unless File.exist?(key_path)
+
+      @rsa_key ||= begin
+        OpenSSL::PKey::RSA.new(File.read(key_path))
+      rescue => e
+        raise ConfigurationError, "Failed to load RSA key: #{e.message}"
+      end
     end
 
     private

modules/vaos/app/services/concerns/token_authentication.rb

@@ -76,7 +76,7 @@ def token_headers
     #
     # @return [Common::JwtWrapper] the JWT wrapper instance.
     def jwt_wrapper
-      @jwt_wrapper ||= Common::JwtWrapper.new(settings)
+      @jwt_wrapper ||= Common::JwtWrapper.new(settings, config)
     end
   end
 

modules/vaos/spec/exceptions/vaos/exceptions/configuration_error_spec.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require_relative '../../../../app/exceptions/vaos/exceptions/configuration_error'
+
+describe VAOS::Exceptions::ConfigurationError do
+  let(:original_error) { StandardError.new('Test error message') }
+  let(:service_name) { 'TestService' }
+  let(:exception) { described_class.new(original_error, service_name) }
+
+  describe '#initialize' do
+    it 'stores the original error' do
+      expect(exception.error).to eq(original_error)
+    end
+  end
+
+  describe '#status' do
+    it 'returns 503 (Service Unavailable)' do
+      expect(exception.status).to eq(503)
+    end
+  end
+
+  describe '#code' do
+    it 'returns the error code' do
+      expect(exception.code).to eq('VAOS_CONFIG_ERROR')
+    end
+  end
+
+  describe '#i18n_data' do
+    it 'returns the custom error data' do
+      expect(exception.i18n_data).to include(
+        title: 'Service Configuration Error',
+        code: 'VAOS_CONFIG_ERROR',
+        status: '503'
+      )
+    end
+  end
+
+  describe '#errors' do
+    let(:error_object) { exception.errors.first }
+
+    it 'returns an array with a single error object' do
+      expect(exception.errors).to be_an(Array)
+      expect(exception.errors.size).to eq(1)
+    end
+
+    it 'formats the error with the correct title' do
+      expect(error_object.title).to eq('Service Configuration Error')
+    end
+
+    it 'includes the service name in the detail message' do
+      expect(error_object.detail).to eq("The #{service_name} service is unavailable due to a configuration issue")
+    end
+
+    it 'uses the standard error code' do
+      expect(error_object.code).to eq('VAOS_CONFIG_ERROR')
+    end
+
+    it 'includes the status code as a string' do
+      expect(error_object.status).to eq('503')
+    end
+
+    it 'includes the source' do
+      expect(error_object.source).to eq(service_name)
+    end
+  end
+end

modules/vaos/spec/requests/v2/referrals_spec.rb

@@ -48,6 +48,47 @@
         expect(first_referral['attributes']).to have_key('type_of_care')
       end
     end
+
+    context 'when a configuration error occurs' do
+      # Note we only test this once as the code is the same for both endpoints
+      let(:jwt_error) { Common::JwtWrapper::ConfigurationError.new('Configuration error occurred') }
+      let(:config_error) { VAOS::Exceptions::ConfigurationError.new(jwt_error, 'CCRA') }
+
+      before do
+        sign_in_as(user)
+        allow(service_double).to receive(:get_vaos_referral_list).and_raise(config_error)
+      end
+
+      it 'returns 503 Service Unavailable with properly formatted error response' do
+        get '/vaos/v2/referrals'
+
+        expect(response).to have_http_status(:service_unavailable)
+
+        response_data = JSON.parse(response.body)
+
+        expect(response_data).to have_key('errors')
+        expect(response_data['errors']).to be_an(Array)
+        expect(response_data['errors'].first).to include(
+          'title' => 'Service Configuration Error',
+          'detail' => 'The CCRA service is unavailable due to a configuration issue',
+          'code' => 'VAOS_CONFIG_ERROR',
+          'status' => '503'
+        )
+      end
+
+      it 'does not expose internal error details' do
+        get '/vaos/v2/referrals'
+
+        response_data = JSON.parse(response.body)
+
+        # Original error message is not leaked
+        expect(response_data['errors'].first['detail']).not_to include('Configuration error occurred')
+
+        # No stack trace is included
+        expect(response_data['errors'].first).not_to have_key('meta')
+        expect(response_data['errors'].first).not_to have_key('backtrace')
+      end
+    end
   end
 
   describe 'GET /vaos/v2/referrals/:id' do

modules/vaos/spec/services/common/jwt_wrapper_spec.rb

@@ -1,22 +1,28 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require_relative '../../../app/services/common/jwt_wrapper'
 
 describe Common::JwtWrapper do
-  subject { described_class.new(settings) }
+  subject { described_class.new(settings, service_config) }
 
-  let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) }
+  let(:service_name) { 'TestService' }
+  let(:service_config) { instance_double(VAOS::Configuration, service_name:) }
   let(:settings) do
-    OpenStruct.new({
-                     key_path: '/path/to/key.pem',
-                     client_id: 'test_client',
-                     kid: 'test_kid',
-                     audience_claim_url: 'https://test.example.com'
-                   })
+    OpenStruct.new(
+      key_path: '/path/to/key.pem',
+      client_id: 'test_client',
+      kid: 'test_kid',
+      audience_claim_url: 'http://test.example.com/token'
+    )
   end
 
+  let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) }
+
   before do
-    allow(File).to receive(:read).with('/path/to/key.pem').and_return(rsa_key)
+    allow(File).to receive(:exist?).and_call_original
+    allow(File).to receive(:exist?).with('/path/to/key.pem').and_return(true)
+    allow(File).to receive(:read).with('/path/to/key.pem').and_return(rsa_key.to_s)
   end
 
   describe 'constants' do
@@ -36,52 +42,100 @@
   end
 
   describe '#sign_assertion' do
-    let(:jwt_token) { subject.sign_assertion }
-    let(:decoded_token) do
-      JWT.decode(
-        jwt_token,
-        rsa_key.public_key,
-        true,
-        algorithm: described_class::SIGNING_ALGORITHM
-      )
-    end
+    let(:encoded_token) { 'encoded.jwt.token' }
+    let(:test_time) { Time.zone.at(1_234_567_890) }
 
-    it 'returns a valid JWT token' do
-      expect { decoded_token }.not_to raise_error
+    before do
+      # Fix the time for deterministic testing
+      allow(Time.zone).to receive(:now).and_return(test_time)
+      allow(JWT).to receive(:encode).and_return(encoded_token)
+      allow(Rails).to receive(:logger).and_return(double('Logger').as_null_object)
+      allow(OpenSSL::PKey::RSA).to receive(:new).and_return(rsa_key)
     end
 
-    it 'includes the correct headers' do
-      headers = decoded_token.last
-      expect(headers['kid']).to eq('test_kid')
-      expect(headers['typ']).to eq('JWT')
-      expect(headers['alg']).to eq('RS512')
+    context 'when JWT encoding is successful' do
+      it 'returns the encoded JWT token' do
+        expect(subject.sign_assertion).to eq(encoded_token)
+      end
+
+      it 'encodes with the correct parameters' do
+        # Just check that claims hash contains required keys
+        expect(JWT).to receive(:encode)
+          .with(
+            hash_including(:iss, :sub, :aud, :iat, :exp),
+            kind_of(OpenSSL::PKey::RSA),
+            'RS512',
+            hash_including(:kid, :typ, :alg)
+          )
+          .and_return(encoded_token)
+
+        subject.sign_assertion
+      end
     end
 
-    it 'includes the correct claims' do
-      claims = decoded_token.first
-      expect(claims['iss']).to eq('test_client')
-      expect(claims['sub']).to eq('test_client')
-      expect(claims['aud']).to eq('https://test.example.com')
-      expect(claims['iat']).to be_within(5).of(Time.zone.now.to_i)
-      expect(claims['exp']).to be_within(5).of(5.minutes.from_now.to_i)
+    context 'when RSA key loading fails' do
+      let(:wrapper_with_error) { described_class.new(settings, service_config) }
+
+      context 'when key file does not exist' do
+        before do
+          allow(File).to receive(:exist?).with('/path/to/key.pem').and_return(false)
+        end
+
+        it 'logs the error and raises a VAOS::Exceptions::ConfigurationError' do
+          expect(Rails.logger).to receive(:error).with(/Service Configuration Error: RSA key file not found/)
+          expect { wrapper_with_error.sign_assertion }.to raise_error(VAOS::Exceptions::ConfigurationError)
+        end
+      end
+
+      context 'when key path is nil' do
+        let(:settings_with_nil_path) do
+          OpenStruct.new(
+            key_path: nil,
+            client_id: 'test_client',
+            kid: 'test_kid',
+            audience_claim_url: 'http://test.example.com/token'
+          )
+        end
+
+        let(:wrapper_with_nil_path) { described_class.new(settings_with_nil_path, service_config) }
+
+        it 'logs the error and raises a VAOS::Exceptions::ConfigurationError' do
+          expect(Rails.logger).to receive(:error).with(/Service Configuration Error: RSA key path is not configured/)
+          expect { wrapper_with_nil_path.sign_assertion }.to raise_error(VAOS::Exceptions::ConfigurationError)
+        end
+      end
+
+      it 'includes the service name in the raised error' do
+        allow(File).to receive(:exist?).with('/path/to/key.pem').and_return(false)
+
+        exception = nil
+        begin
+          wrapper_with_error.sign_assertion
+        rescue VAOS::Exceptions::ConfigurationError => e
+          exception = e
+        end
+        expect(exception).not_to be_nil
+        expect(exception.errors.first.detail).to include(service_name)
+      end
     end
   end
 
   describe '#rsa_key' do
     it 'reads the key from the specified path' do
-      expect(File).to receive(:read).with('/path/to/key.pem').once.and_return(rsa_key)
-      2.times { subject.rsa_key } # Call twice to test memoization
+      expect(File).to receive(:read).with('/path/to/key.pem').once
+      subject.rsa_key
     end
 
     it 'returns an RSA key instance' do
+      allow(OpenSSL::PKey::RSA).to receive(:new).and_return(rsa_key)
       expect(subject.rsa_key).to be_a(OpenSSL::PKey::RSA)
     end
 
     it 'memoizes the RSA key' do
+      expect(File).to receive(:read).with('/path/to/key.pem').once
       first_call = subject.rsa_key
       second_call = subject.rsa_key
       expect(first_call).to eq(second_call)
-      expect(File).to have_received(:read).once
     end
   end
 end