Skip to content

Log the requested NameID format #10761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vrajmohan merged 1 commit into from
Jun 5, 2024
Merged

Log the requested NameID format #10761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/saml_idp_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def capture_analytics
requested_ial: requested_ial,
request_signed: saml_request.signed?,
matching_cert_serial: saml_request.service_provider.matching_cert&.serial&.to_s,
requested_nameid_format: saml_request.name_id_format,
)
analytics.saml_auth(**analytics_payload)
end
Expand Down
5 changes: 4 additions & 1 deletion app/services/analytics_events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4920,7 +4920,8 @@ def rules_of_use_visit
# @param [Boolean] success Whether form validation was successful
# @param [Hash] errors Errors resulting from form validation
# @param [Hash] error_details Details for errors that occurred in unsuccessful submission
# @param [String] nameid_format
# @param [String] nameid_format The NameID format sent in the response
# @param [String] requested_nameid_format The NameID format requested
# @param [Array] authn_context
# @param [String] authn_context_comparison
# @param [String] service_provider
Expand All @@ -4934,6 +4935,7 @@ def saml_auth(
success:,
errors:,
nameid_format:,
requested_nameid_format:,
authn_context:,
authn_context_comparison:,
service_provider:,
Expand All @@ -4952,6 +4954,7 @@ def saml_auth(
errors:,
error_details:,
nameid_format:,
requested_nameid_format:,
authn_context:,
authn_context_comparison:,
service_provider:,
Expand Down
122 changes: 70 additions & 52 deletions spec/controllers/saml_idp_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,21 +788,24 @@ def name_id_version(format_urn)
user_fully_authenticated: true,
})
expect(@analytics).to receive(:track_event).
with('SAML Auth', {
success: true,
errors: {},
error_details: nil,
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: [Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF],
authn_context_comparison: 'exact',
requested_ial: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
service_provider: sp1_issuer,
endpoint: "/api/saml/auth#{path_year}",
idv: false,
finish_profile: false,
request_signed: true,
matching_cert_serial: saml_test_sp_cert_serial,
})
with(
'SAML Auth',
hash_including(
success: true,
errors: {},
error_details: nil,
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: [Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF],
authn_context_comparison: 'exact',
requested_ial: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
service_provider: sp1_issuer,
endpoint: "/api/saml/auth#{path_year}",
idv: false,
finish_profile: false,
request_signed: true,
matching_cert_serial: saml_test_sp_cert_serial,
),
)
expect(@analytics).to receive(:track_event).with(
'SP redirect initiated',
ial: Idp::Constants::IAL2,
Expand Down Expand Up @@ -938,21 +941,24 @@ def name_id_version(format_urn)
user_fully_authenticated: true,
})
expect(@analytics).to receive(:track_event).
with('SAML Auth', {
success: true,
errors: {},
error_details: nil,
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: ['http://idmanagement.gov/ns/assurance/ial/1'],
authn_context_comparison: 'minimum',
requested_ial: 'ialmax',
service_provider: sp1_issuer,
endpoint: "/api/saml/auth#{path_year}",
idv: false,
finish_profile: false,
request_signed: true,
matching_cert_serial: saml_test_sp_cert_serial,
})
with(
'SAML Auth',
hash_including(
success: true,
errors: {},
error_details: nil,
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: ['http://idmanagement.gov/ns/assurance/ial/1'],
authn_context_comparison: 'minimum',
requested_ial: 'ialmax',
service_provider: sp1_issuer,
endpoint: "/api/saml/auth#{path_year}",
idv: false,
finish_profile: false,
request_signed: true,
matching_cert_serial: saml_test_sp_cert_serial,
),
)
expect(@analytics).to receive(:track_event).with(
'SP redirect initiated',
ial: 0,
Expand Down Expand Up @@ -1009,7 +1015,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end
end

Expand Down Expand Up @@ -1234,7 +1240,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end
end

Expand Down Expand Up @@ -1283,7 +1289,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end
end

Expand Down Expand Up @@ -1488,7 +1494,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end
end

Expand Down Expand Up @@ -1566,7 +1572,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end

it 'returns a 400' do
Expand Down Expand Up @@ -1615,7 +1621,7 @@ def name_id_version(format_urn)
}

expect(@analytics).to have_received(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))
end
end

Expand All @@ -1638,7 +1644,7 @@ def name_id_version(format_urn)
)
end

shared_examples_for 'sends the UUID' do
shared_examples_for 'sends the UUID' do |requested_nameid_format|
it 'sends the UUID' do
generate_saml_response(user, auth_settings)

Expand All @@ -1649,13 +1655,15 @@ def name_id_version(format_urn)
expect(@analytics).to have_logged_event(
'SAML Auth',
hash_including(
{ nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT, success: true },
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
requested_nameid_format: requested_nameid_format,
success: true,
),
)
end
end

shared_examples_for 'sends the email' do
shared_examples_for 'sends the email' do |requested_nameid_format|
it 'sends the email' do
generate_saml_response(user, auth_settings)

Expand All @@ -1666,13 +1674,15 @@ def name_id_version(format_urn)
expect(@analytics).to have_logged_event(
'SAML Auth',
hash_including(
{ nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL, success: true },
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL,
requested_nameid_format: requested_nameid_format,
success: true,
),
)
end
end

shared_examples_for 'returns an unauthorized nameid error' do |invalid_format|
shared_examples_for 'returns an unauthorized nameid error' do |requested_nameid_format|
it 'returns an error' do
generate_saml_response(user, auth_settings)

Expand All @@ -1682,7 +1692,9 @@ def name_id_version(format_urn)
expect(@analytics).to have_logged_event(
'SAML Auth',
hash_including(
{ nameid_format: invalid_format, success: false },
nameid_format: requested_nameid_format,
requested_nameid_format: requested_nameid_format,
success: false,
),
)
end
Expand All @@ -1694,7 +1706,7 @@ def name_id_version(format_urn)
context 'when the service provider is not configured with use_legacy_name_id_behavior' do
let(:use_legacy_name_id_behavior) { false }

it_behaves_like 'sends the UUID'
it_behaves_like 'sends the UUID', Saml::Idp::Constants::NAME_ID_FORMAT_UNSPECIFIED
end
context 'when the service provider is configured with use_legacy_name_id_behavior' do
let(:use_legacy_name_id_behavior) { true }
Expand All @@ -1717,18 +1729,18 @@ def name_id_version(format_urn)
context 'when the service provider is not configured with use_legacy_name_id_behavior' do
let(:use_legacy_name_id_behavior) { false }

it_behaves_like 'sends the UUID'
it_behaves_like 'sends the UUID', nil
end
context 'when the service provider is configured with use_legacy_name_id_behavior' do
let(:use_legacy_name_id_behavior) { true }
it_behaves_like 'sends the UUID'
it_behaves_like 'sends the UUID', nil
end
end

context 'when the NameID format is "persistent"' do
let(:name_identifier_format) { Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT }

it_behaves_like 'sends the UUID'
it_behaves_like 'sends the UUID', Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT
end

context 'when the NameID format is "email"' do
Expand All @@ -1743,7 +1755,7 @@ def name_id_version(format_urn)

context 'when the service provider is allowed to use email' do
let(:email_allowed) { true }
it_behaves_like 'sends the email'
it_behaves_like 'sends the email', Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL
end
end

Expand All @@ -1759,13 +1771,13 @@ def name_id_version(format_urn)
context 'when the service provider is not allowed to use email' do
let(:email_allowed) { false }

it_behaves_like 'sends the UUID'
it_behaves_like 'sends the UUID', 'urn:oasis:names:tc:SAML:1.1:nameid-format:transient'
end

context 'when the service provider is allowed to use email' do
let(:email_allowed) { true }

it_behaves_like 'sends the email'
it_behaves_like 'sends the email', 'urn:oasis:names:tc:SAML:1.1:nameid-format:transient'
end
end

Expand Down Expand Up @@ -2281,7 +2293,7 @@ def name_id_version(format_urn)
user_fully_authenticated: true,
})
expect(@analytics).to receive(:track_event).
with('SAML Auth', analytics_hash)
with('SAML Auth', hash_including(analytics_hash))

get :auth, params: { path_year: path_year }
end
Expand Down Expand Up @@ -2330,7 +2342,10 @@ def stub_requested_attributes
force_authn: false,
user_fully_authenticated: true,
})
expect(@analytics).to receive(:track_event).with('SAML Auth', analytics_hash)
expect(@analytics).to receive(:track_event).with(
'SAML Auth',
hash_including(analytics_hash),
)
expect(@analytics).to receive(:track_event).with(
'SP redirect initiated',
ial: 1,
Expand Down Expand Up @@ -2382,7 +2397,10 @@ def stub_requested_attributes
force_authn: false,
user_fully_authenticated: true,
})
expect(@analytics).to receive(:track_event).with('SAML Auth', analytics_hash)
expect(@analytics).to receive(:track_event).with(
'SAML Auth',
hash_including(analytics_hash),
)
expect(@analytics).to receive(:track_event).with(
'SP redirect initiated',
ial: 1,
Expand Down