diff --git a/app/controllers/v0/sign_in_controller.rb b/app/controllers/v0/sign_in_controller.rb index bcc43b4e6e5..ebfb30e273a 100644 --- a/app/controllers/v0/sign_in_controller.rb +++ b/app/controllers/v0/sign_in_controller.rb @@ -229,13 +229,13 @@ def validate_authorize_params(type, client_id, acr, operation) if client_config(client_id).blank? raise SignIn::Errors::MalformedParamsError.new message: 'Client id is not valid' end - unless SignIn::Constants::Auth::CSP_TYPES.include?(type) + unless client_config(client_id).valid_credential_service_provider?(type) raise SignIn::Errors::MalformedParamsError.new message: 'Type is not valid' end unless SignIn::Constants::Auth::OPERATION_TYPES.include?(operation) raise SignIn::Errors::MalformedParamsError.new message: 'Operation is not valid' end - unless SignIn::Constants::Auth::ACR_VALUES.include?(acr) + unless client_config(client_id).valid_service_level?(acr) raise SignIn::Errors::MalformedParamsError.new message: 'ACR is not valid' end end diff --git a/app/models/sign_in/client_config.rb b/app/models/sign_in/client_config.rb index 25e50316246..a32155790b2 100644 --- a/app/models/sign_in/client_config.rb +++ b/app/models/sign_in/client_config.rb @@ -22,6 +22,9 @@ class ClientConfig < ApplicationRecord validates :client_id, presence: true, uniqueness: true validates :logout_redirect_uri, presence: true, if: :cookie_auth? validates :access_token_attributes, inclusion: { in: Constants::AccessToken::USER_ATTRIBUTES } + validates :service_levels, presence: true, inclusion: { in: Constants::Auth::ACR_VALUES, allow_nil: false } + validates :credential_service_providers, presence: true, + inclusion: { in: Constants::Auth::CSP_TYPES, allow_nil: false } def self.valid_client_id?(client_id:) find_by(client_id:).present? @@ -49,6 +52,14 @@ def va_terms_enforced? enforced_terms == Constants::Auth::VA_TERMS end + def valid_credential_service_provider?(type) + credential_service_providers.include?(type) + end + + def valid_service_level?(acr) + service_levels.include?(acr) + end + private def appropriate_mock_environment? diff --git a/spec/controllers/v0/sign_in_controller_spec.rb b/spec/controllers/v0/sign_in_controller_spec.rb index 5a147efd7ce..358189fdedc 100644 --- a/spec/controllers/v0/sign_in_controller_spec.rb +++ b/spec/controllers/v0/sign_in_controller_spec.rb @@ -10,7 +10,9 @@ get(:authorize, params: authorize_params) end - let!(:client_config) { create(:client_config, authentication:, pkce:) } + let!(:client_config) do + create(:client_config, authentication:, pkce:, credential_service_providers:, service_levels:) + end let(:authorize_params) do {}.merge(type) .merge(code_challenge) @@ -26,6 +28,8 @@ let(:code_challenge_method) { { code_challenge_method: 'some-code-challenge-method' } } let(:client_id) { { client_id: client_id_value } } let(:pkce) { true } + let(:credential_service_providers) { %w[idme logingov dslogon mhv] } + let(:service_levels) { %w[loa1 loa3 ial1 ial2 min] } let(:client_id_value) { client_config.client_id } let(:authentication) { SignIn::Constants::Auth::COOKIE } let(:client_state) { {} } @@ -228,9 +232,10 @@ it_behaves_like 'error response' end - context 'when type param is given but not in CSP_TYPES' do - let(:type_value) { 'some-undefined-type' } + context 'when type param is given but not in client credential_service_providers' do + let(:type_value) { 'idme' } let(:type) { { type: type_value } } + let(:credential_service_providers) { ['logingov'] } let(:expected_error) { 'Type is not valid' } it_behaves_like 'error response' @@ -245,21 +250,22 @@ it_behaves_like 'error response' end - context 'and acr param is given but not in ACR_VALUES' do - let(:acr_value) { 'some-undefiend-acr' } + context 'and acr param is given but not in client service_levels' do + let(:acr_value) { 'ial1' } + let(:service_levels) { ['ial2'] } let(:expected_error) { 'ACR is not valid' } it_behaves_like 'error response' end - context 'and acr param is given and in ACR_VALUES but not valid for logingov' do + context 'and acr param is given and in client service_levels but not valid for logingov' do let(:acr_value) { 'loa1' } let(:expected_error) { 'Invalid ACR for logingov' } it_behaves_like 'error response' end - context 'and acr param is given and in ACR_VALUES and valid for logingov' do + context 'and acr param is given and in client service_levels and valid for logingov' do let(:acr_value) { 'ial1' } context 'and code_challenge_method is not given' do @@ -408,21 +414,22 @@ it_behaves_like 'error response' end - context 'and acr param is given but not in ACR_VALUES' do - let(:acr_value) { 'some-undefiend-acr' } + context 'and acr param is given but not in client service_levels' do + let(:acr_value) { 'loa1' } + let(:service_levels) { ['loa3'] } let(:expected_error) { 'ACR is not valid' } it_behaves_like 'error response' end - context 'and acr param is given and in ACR_VALUES but not valid for type' do + context 'and acr param is given and in client service_levels but not valid for type' do let(:acr_value) { 'ial1' } let(:expected_error) { "Invalid ACR for #{type_value}" } it_behaves_like 'error response' end - context 'and acr param is given and in ACR_VALUES and valid for type' do + context 'and acr param is given and in client service_levels and valid for type' do let(:acr_value) { 'loa1' } context 'and code_challenge_method is not given' do diff --git a/spec/models/sign_in/client_config_spec.rb b/spec/models/sign_in/client_config_spec.rb index bc5173cef16..4838ad3e74e 100644 --- a/spec/models/sign_in/client_config_spec.rb +++ b/spec/models/sign_in/client_config_spec.rb @@ -17,7 +17,9 @@ certificates:, access_token_attributes:, enforced_terms:, - terms_of_use_url:) + terms_of_use_url:, + service_levels:, + credential_service_providers:) end let(:client_id) { 'some-client-id' } let(:authentication) { SignIn::Constants::Auth::API } @@ -32,6 +34,8 @@ let(:access_token_attributes) { [] } let(:enforced_terms) { SignIn::Constants::Auth::VA_TERMS } let(:terms_of_use_url) { 'some-terms-of-use-url' } + let(:service_levels) { %w[loa1 loa3 ial1 ial2 min] } + let(:credential_service_providers) { %w[idme logingov dslogon mhv] } describe 'validations' do subject { client_config } @@ -217,6 +221,66 @@ end end + describe '#credential_service_providers' do + context 'when credential_service_providers is empty' do + let(:credential_service_providers) { [] } + let(:expected_error_message) { "Validation failed: Credential service providers can't be blank" } + let(:expected_error) { ActiveRecord::RecordInvalid } + + it 'raises validation error' do + expect { subject }.to raise_error(expected_error, expected_error_message) + end + end + + context 'when credential_service_providers contain values not included in CSP_TYPES constant' do + let(:credential_service_providers) { %w[idme logingov dslogon mhv bad_csp] } + let(:expected_error_message) { 'Validation failed: Credential service providers is not included in the list' } + let(:expected_error) { ActiveRecord::RecordInvalid } + + it 'raises validation error' do + expect { subject }.to raise_error(expected_error, expected_error_message) + end + end + + context 'when all credential_service_providers values are included in CSP_TYPES constant' do + let(:credential_service_providers) { SignIn::Constants::Auth::CSP_TYPES } + + it 'does not raise validation error' do + expect { subject }.not_to raise_error + end + end + end + + describe '#service_levels' do + context 'when service_levels is empty' do + let(:service_levels) { [] } + let(:expected_error_message) { "Validation failed: Service levels can't be blank" } + let(:expected_error) { ActiveRecord::RecordInvalid } + + it 'raises validation error' do + expect { subject }.to raise_error(expected_error, expected_error_message) + end + end + + context 'when service_levels contain values not included in ACR_VALUES constant' do + let(:service_levels) { %w[loa1 loa3 ial1 ial2 min bad_acr] } + let(:expected_error_message) { 'Validation failed: Service levels is not included in the list' } + let(:expected_error) { ActiveRecord::RecordInvalid } + + it 'raises validation error' do + expect { subject }.to raise_error(expected_error, expected_error_message) + end + end + + context 'when all service_levels values are included in ACR_VALUES constant' do + let(:service_levels) { SignIn::Constants::Auth::ACR_VALUES } + + it 'does not raise validation error' do + expect { subject }.not_to raise_error + end + end + end + describe '#enforced_terms' do context 'when enforced_terms is arbitrary' do let(:enforced_terms) { 'some-enforced-terms' } @@ -378,6 +442,46 @@ end end + describe '#valid_credential_service_provider?' do + subject { client_config.valid_credential_service_provider?(type) } + + context 'when type is included in csps' do + let(:type) { 'idme' } + + it 'returns true' do + expect(subject).to be(true) + end + end + + context 'when type is not included in csps' do + let(:type) { 'bad_csp' } + + it 'returns false' do + expect(subject).to be(false) + end + end + end + + describe '#valid_service_level?' do + subject { client_config.valid_service_level?(acr) } + + context 'when acr is included in acrs' do + let(:acr) { 'loa1' } + + it 'returns true' do + expect(subject).to be(true) + end + end + + context 'when acr is not included in acrs' do + let(:acr) { 'bad_acr' } + + it 'returns false' do + expect(subject).to be(false) + end + end + end + describe '#mock_auth?' do subject { client_config.mock_auth? }