Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion app/controllers/concerns/fully_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ def confirm_two_factor_authenticated(id = nil)

def delete_branded_experience
ServiceProviderRequest.from_uuid(sp_session[:request_id]).delete
session.delete(:sp)
end

def request_id
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def timeout

def check_user_needs_redirect
if user_fully_authenticated?
redirect_to after_sign_in_path_for(current_user)
redirect_to signed_in_path
elsif current_user
sign_out
end
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/saml_idp_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,8 @@
saml_get_auth(saml_settings)
end

it 'deletes SP metadata from session' do
expect(session.key?(:sp)).to eq(false)
it 'does not delete SP metadata from session' do
expect(session.key?(:sp)).to eq(true)
end
end
end
Expand Down
17 changes: 17 additions & 0 deletions spec/controllers/users/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -323,5 +323,22 @@
get :new
end
end

context 'with fully authenticated user who has a pending profile' do
it 'redirects to the verify profile page' do
profile = create(
:profile,
deactivation_reason: :verification_pending,
phone_confirmed: false,
pii: { otp: 'abc123', ssn: '6666', dob: '1920-01-01' }
)
user = profile.user

stub_sign_in(user)
get :new

expect(response).to redirect_to verify_account_path
end
end
end
end
10 changes: 5 additions & 5 deletions spec/features/openid_connect/openid_connect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
click_submit_default

expect(current_url).to start_with('http://localhost:7654/auth/result')
expect(page.get_rack_session.keys).to_not include('sp')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make these positive expectations?

expect(page.get_rack_session.keys).to include('sp')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Fixed. PTAL

expect(page.get_rack_session.keys).to include('sp')
end

it 'auto-allows and sets the correct CSP headers after an incorrect OTP' do
Expand Down Expand Up @@ -146,7 +146,7 @@
click_submit_default

expect(current_url).to start_with('http://localhost:7654/auth/result')
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end
end

Expand Down Expand Up @@ -238,7 +238,7 @@
click_button t('forms.buttons.continue')
redirect_uri = URI(current_url)
expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result')
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end
end
end
Expand Down Expand Up @@ -501,7 +501,7 @@
redirect_uri = URI(current_url)

expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result')
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end

perform_in_browser(:one) do
Expand All @@ -515,7 +515,7 @@
redirect_uri = URI(current_url)

expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result')
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end
end
end
Expand Down
13 changes: 8 additions & 5 deletions spec/features/saml/loa1_sso_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,22 @@
click_on t('forms.buttons.continue')

expect(current_url).to eq authn_request
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end
end

it 'takes user to the service provider, allows user to visit IDP' do
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)

user = create(:user, :signed_up)
saml_authn_request = auth_request.create(saml_settings)

visit saml_authn_request
sign_in_live_with_2fa(user)
click_link t('links.sign_in')
fill_in_credentials_and_submit(user.email, user.password)
click_submit_default

expect(current_url).to eq saml_authn_request
expect(page.get_rack_session.keys).to_not include('sp')

visit root_path
expect(current_path).to eq account_path
Expand Down Expand Up @@ -177,7 +180,7 @@
click_button t('forms.buttons.continue')

expect(current_url).to eq authn_request
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end

perform_in_browser(:one) do
Expand All @@ -190,7 +193,7 @@
click_button t('forms.buttons.continue')

expect(current_url).to eq authn_request
expect(page.get_rack_session.keys).to_not include('sp')
expect(page.get_rack_session.keys).to include('sp')
end
end
end
Expand Down