diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index f31c2f6c992..ba098cef4ac 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -50,6 +50,11 @@ def validate_service_provider_and_authn_context return if result.success? capture_analytics + track_integration_errors( + event: :saml_auth_request, + errors: result.errors.values.flatten, + ) + render 'saml_idp/auth/error', status: :bad_request end @@ -248,4 +253,14 @@ def request_url url.query = Rack::Utils.build_query(query_params).presence url.to_s end + + def track_integration_errors(event:, errors: nil) + analytics.sp_integration_errors_present( + error_details: errors || saml_request.errors.uniq, + error_types: [:saml_request_errors], + event:, + integration_exists: saml_request_service_provider.present?, + request_issuer: saml_request&.issuer, + ) + end end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index f2d4f45a8b8..f4d17153b96 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -56,7 +56,7 @@ def pending_profile_policy end def check_sp_active - return if @authorize_form.service_provider&.active? + return if service_provider&.active? redirect_to sp_inactive_error_url end @@ -100,7 +100,7 @@ def email_address_id def ial_context IalContext.new( ial: resolved_authn_context_int_ial, - service_provider: @authorize_form.service_provider, + service_provider:, user: current_user, ) end @@ -121,7 +121,6 @@ def handle_successful_handoff redirect_user( @authorize_form.success_redirect_uri, - @authorize_form.service_provider.issuer, current_user.uuid, ) @@ -153,13 +152,13 @@ def build_authorize_form_from_params def secure_headers_override return if form_action_csp_disabled_and_not_server_side_redirect?( - issuer: @authorize_form.service_provider.issuer, + issuer: issuer, user_uuid: current_user&.uuid, ) csp_uris = SecureHeadersAllowList.csp_with_sp_redirect_uris( @authorize_form.redirect_uri, - @authorize_form.service_provider.redirect_uris, + service_provider.redirect_uris, ) override_form_action_csp(csp_uris) end @@ -172,7 +171,7 @@ def pre_validate_authorize_form result = @authorize_form.submit analytics.openid_connect_request_authorization( - **result.to_h.except(:redirect_uri, :code_digest).merge( + **result.to_h.except(:redirect_uri, :code_digest, :integration_errors).merge( user_fully_authenticated: user_fully_authenticated?, referer: request.referer, vtr_param: params[:vtr], @@ -180,19 +179,26 @@ def pre_validate_authorize_form ), ) return if result.success? + + if result.extra[:integration_errors].present? + analytics.sp_integration_errors_present( + **result.to_h[:integration_errors], + ) + end + redirect_uri = result.extra[:redirect_uri] if redirect_uri.nil? render :error else - redirect_user(redirect_uri, @authorize_form.service_provider.issuer, current_user&.uuid) + redirect_user(redirect_uri, current_user&.uuid) end end def sign_out_if_prompt_param_is_login_and_user_is_signed_in if @authorize_form.prompt != 'login' set_issuer_forced_reauthentication( - issuer: @authorize_form.service_provider.issuer, + issuer:, is_forced_reauthentication: false, ) end @@ -204,7 +210,7 @@ def sign_out_if_prompt_param_is_login_and_user_is_signed_in unless sp_session[:request_url] == request.original_url sign_out set_issuer_forced_reauthentication( - issuer: @authorize_form.service_provider.issuer, + issuer:, is_forced_reauthentication: true, ) end @@ -236,8 +242,8 @@ def track_events track_billing_events end - def redirect_user(redirect_uri, issuer, user_uuid) - case oidc_redirect_method(issuer: issuer, user_uuid: user_uuid) + def redirect_user(redirect_uri, user_uuid) + case oidc_redirect_method(issuer:, user_uuid: user_uuid) when 'client_side' @oidc_redirect_uri = redirect_uri render( @@ -258,6 +264,14 @@ def redirect_user(redirect_uri, issuer, user_uuid) end end + def service_provider + @authorize_form.service_provider + end + + def issuer + service_provider&.issuer + end + def sp_handoff_bouncer @sp_handoff_bouncer ||= SpHandoffBouncer.new(sp_session) end diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index ada39ed341e..c9ac544c72f 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -26,6 +26,8 @@ def show if result.success? && redirect_uri handle_successful_logout_request(result, redirect_uri) else + track_integration_errors(result:, event: :oidc_logout_requested) + render :error end end @@ -48,6 +50,8 @@ def delete if result.success? && redirect_uri handle_logout(result, redirect_uri) else + track_integration_errors(result:, event: :oidc_logout_submitted) + render :error end end @@ -141,11 +145,21 @@ def handle_logout(result, redirect_uri) # Convert FormResponse into loggable analytics event # @param [FormResponse] result def to_event(result) - result.to_h.except(:redirect_uri) + result.to_h.except(:redirect_uri, :integration_errors) end def logout_params params.permit(:client_id, :id_token_hint, :post_logout_redirect_uri, :state) end + + def track_integration_errors(result:, event:) + if result.extra[:integration_errors].present? + analytics.sp_integration_errors_present( + **result. + to_h[:integration_errors]. + merge(event:), + ) + end + end end end diff --git a/app/controllers/openid_connect/token_controller.rb b/app/controllers/openid_connect/token_controller.rb index de48ebb649d..8771aa8d3d7 100644 --- a/app/controllers/openid_connect/token_controller.rb +++ b/app/controllers/openid_connect/token_controller.rb @@ -15,7 +15,13 @@ def create analytics_attributes = result.to_h analytics_attributes[:expires_in] = response[:expires_in] - analytics.openid_connect_token(**analytics_attributes) + analytics.openid_connect_token(**analytics_attributes.except(:integration_errors)) + + if !result.success? && analytics_attributes[:integration_errors].present? + analytics.sp_integration_errors_present( + **analytics_attributes[:integration_errors], + ) + end render json: response, status: (result.success? ? :ok : :bad_request) diff --git a/app/controllers/openid_connect/user_info_controller.rb b/app/controllers/openid_connect/user_info_controller.rb index a1df4ba00f3..818e79d8511 100644 --- a/app/controllers/openid_connect/user_info_controller.rb +++ b/app/controllers/openid_connect/user_info_controller.rb @@ -18,11 +18,13 @@ def show def authenticate_identity_via_bearer_token verifier = AccessTokenVerifier.new(request.env['HTTP_AUTHORIZATION']) response, identity = verifier.submit - analytics.openid_connect_bearer_token(**response) + attributes = response.to_h + analytics.openid_connect_bearer_token(**attributes.except(:integration_errors)) if response.success? @current_identity = identity else + analytics.sp_integration_errors_present(**attributes[:integration_errors]) render json: { error: verifier.errors[:access_token].join(' ') }, status: :unauthorized end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 55083feb324..5c7ab57cbe5 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -61,7 +61,11 @@ def logout track_logout_event - return head(:bad_request) unless valid_saml_request? + unless valid_saml_request? + track_integration_errors(event: :saml_logout_request) + + return head(:bad_request) + end handle_valid_sp_logout_request end @@ -75,11 +79,21 @@ def remotelogout track_remote_logout_event(issuer) - return head(:bad_request) unless valid_saml_request? + unless valid_saml_request? + track_integration_errors(event: :saml_remote_logout_request) + + return head(:bad_request) + end user_id = find_user_from_session_index - return head(:bad_request) unless user_id.present? + unless user_id.present? + track_integration_errors( + event: :saml_remote_logout_request, + errors: [:no_user_found_from_session_index], + ) + return head(:bad_request) + end handle_valid_sp_remote_logout_request(user_id: user_id, issuer: issuer) end diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb index d272a10fa52..c3bbca2b17c 100644 --- a/app/forms/openid_connect_authorize_form.rb +++ b/app/forms/openid_connect_authorize_form.rb @@ -284,6 +284,7 @@ def extra_analytics_attributes code_digest: code ? Digest::SHA256.hexdigest(code) : nil, code_challenge_present: code_challenge.present?, service_provider_pkce: service_provider&.pkce, + integration_errors:, } end @@ -350,6 +351,18 @@ def ialmax_requested? Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_IAL[ial_values.sort.max] == 0 end + def integration_errors + return nil if @success || client_id.blank? + + { + error_details: errors.full_messages, + error_types: errors.attribute_names, + event: :oidc_request_authorization, + integration_exists: service_provider.present?, + request_issuer: client_id, + } + end + def facial_match_ial_requested? ial_values.any? { |ial| Saml::Idp::Constants::FACIAL_MATCH_IAL_CONTEXTS.include? ial } end diff --git a/app/forms/openid_connect_logout_form.rb b/app/forms/openid_connect_logout_form.rb index 582906342a2..731af2d981b 100644 --- a/app/forms/openid_connect_logout_form.rb +++ b/app/forms/openid_connect_logout_form.rb @@ -114,6 +114,16 @@ def id_token_hint_or_client_id_present ) end + def integration_errors + return nil if valid? + { + error_details: errors.full_messages, + error_types: errors.attribute_names, + integration_exists: service_provider.present?, + request_issuer: client_id || service_provider&.issuer, + } + end + def valid_client_id return unless client_id.present? && id_token_hint.blank? return if service_provider.present? @@ -144,6 +154,7 @@ def extra_analytics_attributes redirect_uri: redirect_uri, sp_initiated: true, oidc: true, + integration_errors:, } end diff --git a/app/forms/openid_connect_token_form.rb b/app/forms/openid_connect_token_form.rb index 3ae404bc13b..e5294c69ad0 100644 --- a/app/forms/openid_connect_token_form.rb +++ b/app/forms/openid_connect_token_form.rb @@ -204,6 +204,19 @@ def extra_analytics_attributes code_verifier_present: code_verifier.present?, service_provider_pkce: service_provider&.pkce, ial: identity&.ial, + integration_errors:, + } + end + + def integration_errors + return nil if valid? || client_id.blank? + + { + error_details: errors.full_messages, + error_types: errors.attribute_names, + event: :oidc_token_request, + integration_exists: service_provider.present?, + request_issuer: client_id, } end diff --git a/app/services/access_token_verifier.rb b/app/services/access_token_verifier.rb index 49372e308b2..bf122c1b316 100644 --- a/app/services/access_token_verifier.rb +++ b/app/services/access_token_verifier.rb @@ -21,6 +21,7 @@ def submit extra: { client_id: @identity&.service_provider, ial: @identity&.ial, + integration_errors:, }, ) @@ -69,4 +70,14 @@ def extract_access_token(header) access_token end + + def integration_errors + { + error_details: errors.full_messages, + error_types: errors.attribute_names, + event: :oidc_bearer_token_auth, + integration_exists: @identity&.service_provider.present?, + request_issuer: @identity&.service_provider, + } + end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 8e867239b69..ab4a9c3fec7 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -6915,6 +6915,32 @@ def sp_inactive_visit track_event('SP inactive visited') end + # @param [Array] error_details Full messages of the errors + # @param [Hash] error_types Types of errors that are surfaced + # @param [Symbol] event What part of the workflow the error occured in + # @param [Boolean] integration_exists Whether the requesting issuer maps to an SP + # @param [String] request_issuer The issuer in the request + # Monitoring service-provider specific integration errors + def sp_integration_errors_present( + error_details:, + error_types:, + event:, + integration_exists:, + request_issuer: nil, + **extra + ) + types = error_types.index_with { |_type| true } + track_event( + :sp_integration_errors_present, + error_details:, + error_types: types, + event:, + integration_exists:, + request_issuer:, + **extra, + ) + end + # Tracks when a user is redirected back to the service provider # @param [Integer] ial # @param [Integer] billed_ial diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 0590bca9983..f4ae03e8feb 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -153,6 +153,10 @@ sign_in_flow:, acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', ) + + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) end end @@ -2020,12 +2024,45 @@ errors: hash_including(:prompt), error_details: hash_including(:prompt), user_fully_authenticated: true, - acr_values: 'http://idmanagement.gov/ns/assurance/ial/1', + acr_values: acr_values, code_challenge_present: false, scope: 'openid', ) expect(@analytics).to_not have_logged_event('SP redirect initiated') + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Prompt Please fill in this field.', + ), + error_types: { prompt: true }, + event: :oidc_request_authorization, + integration_exists: true, + request_issuer: client_id, + ) + end + + context 'when there are multiple issues with the request' do + let(:acr_values) { nil } + + it 'notes all the integration errors' do + stub_analytics + + action + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Acr values Please fill in this field.', + 'Prompt Please fill in this field.', + ), + error_types: { acr_values: true, prompt: true }, + event: :oidc_request_authorization, + integration_exists: true, + request_issuer: client_id, + ) + end end end @@ -2054,6 +2091,17 @@ scope: 'openid profile', unknown_authn_contexts: unknown_value, ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Acr values Please fill in this field.', + ), + error_types: { acr_values: true }, + event: :oidc_request_authorization, + integration_exists: true, + request_issuer: client_id, + ) end context 'when there is also a valid acr_value' do diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index fc2499422b1..5eb418189fe 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -176,6 +176,10 @@ oidc: true, ), ) + + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) end end @@ -215,6 +219,17 @@ oidc: true, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Redirect uri redirect_uri does not match registered redirect_uri', + ), + error_types: { redirect_uri: true }, + event: :oidc_logout_requested, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end end @@ -238,6 +253,16 @@ oidc: true, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Id token hint id_token_hint was not recognized', + ), + error_types: { id_token_hint: true }, + event: :oidc_logout_requested, + integration_exists: false, + ) end end end @@ -317,6 +342,9 @@ oidc: true, ), ) + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) expect(response).to render_template(:confirm_logout) expect(response.body).to include service_provider.friendly_name end @@ -358,6 +386,17 @@ oidc: true, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Redirect uri redirect_uri does not match registered redirect_uri', + ), + error_types: { redirect_uri: true }, + event: :oidc_logout_requested, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end end end @@ -444,6 +483,10 @@ oidc: true, ), ) + + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) end end @@ -483,6 +526,18 @@ oidc: true, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Id token hint This application is misconfigured and should not be sending ' \ + 'id_token_hint. Please send client_id instead.', + ), + error_types: { id_token_hint: true }, + event: :oidc_logout_requested, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end end @@ -522,6 +577,17 @@ oidc: true, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Redirect uri redirect_uri does not match registered redirect_uri', + ), + error_types: { redirect_uri: true }, + event: :oidc_logout_requested, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end end end @@ -730,13 +796,15 @@ describe '#delete' do context 'when sending client_id' do + let(:params) do + { + client_id: service_provider.issuer, + post_logout_redirect_uri: post_logout_redirect_uri, + state: state, + } + end subject(:action) do - delete :delete, - params: { - client_id: service_provider.issuer, - post_logout_redirect_uri: post_logout_redirect_uri, - state: state, - } + delete :delete, params: end context 'user is signed in' do @@ -795,6 +863,30 @@ sp_initiated: true, oidc: true, ) + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) + end + + context 'when client_id is missing' do + before do + params.delete(:client_id) + end + it 'tracks an integration error event' do + stub_analytics + + action + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Client client_id is missing', + ), + error_types: { client_id: true }, + event: :oidc_logout_submitted, + integration_exists: false, + ) + end end end diff --git a/spec/controllers/openid_connect/token_controller_spec.rb b/spec/controllers/openid_connect/token_controller_spec.rb index d8648d7a274..037299f9590 100644 --- a/spec/controllers/openid_connect/token_controller_spec.rb +++ b/spec/controllers/openid_connect/token_controller_spec.rb @@ -67,6 +67,8 @@ ial: 1, } ) + + expect(@analytics).to_not have_logged_event(:sp_integration_errors_present) end end @@ -99,6 +101,17 @@ ial: 1, } ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Grant type is not included in the list', + ), + error_types: { grant_type: true }, + event: :oidc_token_request, + integration_exists: true, + request_issuer: client_id, + ) end end @@ -106,12 +119,16 @@ let(:code) { { nested: 'code' } } it 'is a 400 and has an error response and no id_token' do + stub_analytics + action expect(response).to be_bad_request json = JSON.parse(response.body).with_indifferent_access expect(json[:error]).to be_present expect(json).to_not have_key(:id_token) + + expect(@analytics).to_not have_logged_event(:sp_integration_errors_present) end end end diff --git a/spec/controllers/openid_connect/user_info_controller_spec.rb b/spec/controllers/openid_connect/user_info_controller_spec.rb index ca18ae0f26d..97c2eba1f9c 100644 --- a/spec/controllers/openid_connect/user_info_controller_spec.rb +++ b/spec/controllers/openid_connect/user_info_controller_spec.rb @@ -28,6 +28,16 @@ errors: hash_including(:access_token), error_details: hash_including(:access_token), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Access token No Authorization header provided', + ), + error_types: { access_token: true }, + event: :oidc_bearer_token_auth, + integration_exists: false, + ) end end @@ -52,6 +62,16 @@ errors: hash_including(:access_token), error_details: hash_including(:access_token), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Access token Malformed Authorization header', + ), + error_types: { access_token: true }, + event: :oidc_bearer_token_auth, + integration_exists: false, + ) end end @@ -75,6 +95,17 @@ errors: hash_including(:access_token), error_details: hash_including(:access_token), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: array_including( + 'Access token Could not find authorization for the contents of the provided ' \ + 'access_token or it may have expired', + ), + error_types: { access_token: true }, + event: :oidc_bearer_token_auth, + integration_exists: false, + ) end end @@ -135,6 +166,10 @@ ial: identity.ial, errors: {}, ) + + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) end it 'only changes the paths visited in session' do diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 8b587bf2753..a8b99889b54 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -46,6 +46,13 @@ 'Logout Initiated', hash_including(sp_initiated: true, oidc: false, saml_request_valid: false), ) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:issuer_missing_or_invald, :no_auth_or_logout_request, :invalid_signature], + error_types: { saml_request_errors: true }, + event: :saml_logout_request, + integration_exists: false, + ) end let(:service_provider) do @@ -112,12 +119,35 @@ expect(response).to be_ok end - it 'rejects requests from a wrong cert' do - delete :logout, params: UriService.params( - OneLogin::RubySaml::Logoutrequest.new.create(wrong_cert_settings), - ).merge(path_year: path_year) + context 'when the cert is not registered' do + it 'rejects requests from a wrong cert' do + delete :logout, params: UriService.params( + OneLogin::RubySaml::Logoutrequest.new.create(wrong_cert_settings), + ).merge(path_year: path_year) - expect(response).to be_bad_request + expect(response).to be_bad_request + end + + it 'tracks the request' do + stub_analytics + + delete :logout, params: UriService.params( + OneLogin::RubySaml::Logoutrequest.new.create(wrong_cert_settings), + ).merge(path_year: path_year) + + expect(@analytics).to have_logged_event( + 'Logout Initiated', + hash_including(sp_initiated: true, oidc: false, saml_request_valid: false), + ) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:invalid_signature], + error_types: { saml_request_errors: true }, + event: :saml_logout_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) + end end context 'cert element in SAML request is blank' do @@ -183,6 +213,13 @@ post :remotelogout, params: { SAMLRequest: 'foo', path_year: path_year } expect(@analytics).to have_logged_event('Remote Logout initiated', saml_request_valid: false) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:issuer_missing_or_invald, :no_auth_or_logout_request, :invalid_signature], + error_types: { saml_request_errors: true }, + event: :saml_remote_logout_request, + integration_exists: false, + ) end let(:agency) { create(:agency) } @@ -351,12 +388,21 @@ ), ).to eq(true) + stub_analytics post :remotelogout, params: payload.to_h.merge( Signature: Base64.encode64(signature), path_year: path_year, ) expect(response).to be_bad_request + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:no_user_found_from_session_index], + error_types: { saml_request_errors: true }, + event: :saml_remote_logout_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end it 'rejects requests from a correct cert but bad session index' do @@ -385,12 +431,21 @@ ), ).to eq(true) + stub_analytics post :remotelogout, params: payload.to_h.merge( Signature: Base64.encode64(signature), path_year: path_year, ) expect(response).to be_bad_request + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:no_user_found_from_session_index], + error_types: { saml_request_errors: true }, + event: :saml_remote_logout_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end it 'rejects requests from a correct cert but a non-associated user' do @@ -419,20 +474,38 @@ ), ).to eq(true) + stub_analytics post :remotelogout, params: payload.to_h.merge( Signature: Base64.encode64(signature), path_year: path_year, ) expect(response).to be_bad_request + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:no_user_found_from_session_index], + error_types: { saml_request_errors: true }, + event: :saml_remote_logout_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end it 'rejects requests from a wrong cert' do + stub_analytics post :remotelogout, params: UriService.params( OneLogin::RubySaml::Logoutrequest.new.create(wrong_cert_settings), ).merge(path_year: path_year) expect(response).to be_bad_request + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: [:invalid_signature], + error_types: { saml_request_errors: true }, + event: :saml_remote_logout_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end end @@ -1012,6 +1085,14 @@ def name_id_version(format_urn) unknown_authn_contexts: unknown_value, ), ) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: ['Unauthorized authentication context'], + error_types: { saml_request_errors: true }, + event: :saml_auth_request, + integration_exists: true, + request_issuer: saml_settings.issuer, + ) end context 'there is also a valid authn_context' do @@ -1030,6 +1111,9 @@ def name_id_version(format_urn) unknown_authn_contexts: unknown_value, ), ) + expect(@analytics).to_not have_logged_event( + :sp_integration_errors_present, + ) end context 'when it includes the ReqAttributes AuthnContext' do @@ -1276,6 +1360,14 @@ def name_id_version(format_urn) finish_profile: false, ), ) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: ['Unauthorized Service Provider'], + error_types: { saml_request_errors: true }, + event: :saml_auth_request, + integration_exists: false, + request_issuer: 'invalid_provider', + ) end end @@ -1321,6 +1413,14 @@ def name_id_version(format_urn) finish_profile: false, ), ) + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: ['Unauthorized Service Provider', 'Unauthorized authentication context'], + error_types: { saml_request_errors: true }, + event: :saml_auth_request, + integration_exists: false, + request_issuer: 'invalid_provider', + ) end end @@ -1364,6 +1464,15 @@ def name_id_version(format_urn) error_details: { service_provider: { no_cert_registered: true } }, ), ) + + expect(@analytics).to have_logged_event( + :sp_integration_errors_present, + error_details: ['Your service provider does not have a certificate registered.'], + error_types: { saml_request_errors: true }, + event: :saml_auth_request, + integration_exists: true, + request_issuer: service_provider.issuer, + ) end context 'when service provider has block_encryption set to none' do diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index fe1598234dc..dda53c85a94 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -54,6 +54,7 @@ code_digest: nil, code_challenge_present: false, service_provider_pkce: nil, + integration_errors: nil, ) end end @@ -79,6 +80,13 @@ code_digest: nil, code_challenge_present: false, service_provider_pkce: nil, + integration_errors: { + error_details: ['Response type is not included in the list'], + error_types: [:response_type], + event: :oidc_request_authorization, + integration_exists: true, + request_issuer: client_id, + }, ) end end diff --git a/spec/forms/openid_connect_token_form_spec.rb b/spec/forms/openid_connect_token_form_spec.rb index e9b8403418c..9f31828c95d 100644 --- a/spec/forms/openid_connect_token_form_spec.rb +++ b/spec/forms/openid_connect_token_form_spec.rb @@ -382,11 +382,12 @@ code_verifier_present: false, service_provider_pkce: nil, ial: 1, + integration_errors: nil, ) end end - context 'with invalid params' do + context 'with invalid code' do let(:code) { nil } it 'returns FormResponse with success: false' do @@ -402,6 +403,33 @@ ) end end + + context 'with the wrong grant_type ' do + let(:grant_type) { 'wrong' } + + it 'returns FormResponse with success: false and int error' do + submission = form.submit + + expect(submission.to_h).to include( + success: false, + errors: form.errors.messages, + error_details: hash_including(:grant_type), + client_id: client_id, + user_id: user.uuid, + code_digest: Digest::SHA256.hexdigest(code), + code_verifier_present: false, + service_provider_pkce: nil, + ial: 1, + integration_errors: { + error_details: ['Grant type is not included in the list'], + error_types: [:grant_type], + event: :oidc_token_request, + integration_exists: true, + request_issuer: client_id, + }, + ) + end + end end describe '#response' do