Skip to content

Remove temporary 50/50 state fix from the SAML controller #10144

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
jmhooper merged 2 commits into from
Feb 23, 2024
Merged

Remove temporary 50/50 state fix from the SAML controller #10144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee01be5
Remove temporary 50/50 state fix from the SAML controller
jmhooper Feb 22, 2024
dd410ca
delint
jmhooper Feb 23, 2024
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
9 changes: 1 addition & 8 deletions app/controllers/saml_idp_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,8 @@ def saml_metadata
SamlEndpoint.new(params[:path_year]).saml_metadata
end

def ialmax_request_with_ial1_acr_and_pii_requested_and_locked?
requested_ial == 'ialmax' &&
current_user.identity_verified? &&
!Pii::Cacher.new(current_user, user_session).exists_in_session?
end

def prompt_for_password_if_ial2_request_and_pii_locked
return unless pii_requested_but_locked? ||
ialmax_request_with_ial1_acr_and_pii_requested_and_locked?
return unless pii_requested_but_locked?
redirect_to capture_password_url
end

Expand Down
38 changes: 1 addition & 37 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 @@ -731,10 +731,9 @@ def name_id_version(format_urn)
)
end
let(:sign_in_flow) { :sign_in }
let(:skip_sign_in) { false }

before do
stub_sign_in(user) unless skip_sign_in
stub_sign_in(user)
session[:sign_in_flow] = sign_in_flow
IdentityLinker.new(user, ServiceProvider.find_by(issuer: sp1_issuer)).link_identity(ial: 2)
user.identities.last.update!(
Expand Down Expand Up @@ -825,41 +824,6 @@ def name_id_version(format_urn)
expect(response).to redirect_to capture_password_url
end
end

context 'profile is not in the session and an incorrect ACR value was stored' do
let(:pii) { nil }
let(:skip_sign_in) { true }

before do
IdentityLinker.new(
user,
ServiceProvider.find_by(issuer: sp1_issuer),
).link_identity(ial: 2)
user.identities.last.update!(
verified_attributes: %w[email given_name family_name social_security_number address],
)
allow(subject).to receive(:attribute_asserter) { asserter }
end

it 'redirects the user to capture password' do
params = {
SAMLRequest: CGI.unescape(saml_request(ialmax_settings)),
path_year: SamlAuthHelper::PATH_YEAR,
}

# Initial request to store the SP request
get :auth, params: params

# The old code would store the IAL1 authn context instead of the IALMAX context
# This commit duplicates that behavior by overriding the value in the session here
controller.session[:sp][:acr_values] = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
stub_sign_in(user)

get :auth, params: params

expect(response).to redirect_to capture_password_url
end
end
end

context 'authn_context is invalid' do
Expand Down