-
Notifications
You must be signed in to change notification settings - Fork 166
Don't allow any /verify/* path in prod without SP context (LG-3942) #4543
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,17 @@ | ||
| module IdvSession | ||
| extend ActiveSupport::Concern | ||
|
|
||
| included do | ||
| before_action :sp_context_needed? | ||
| end | ||
|
|
||
| def sp_context_needed? | ||
| return if sp_from_sp_session.present? | ||
| return if LoginGov::Hostdata.env != 'prod' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this use the config from #4545 ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this is out of date and superceded by #4545 |
||
|
|
||
| redirect_to account_url | ||
| end | ||
|
|
||
| def confirm_idv_session_started | ||
| redirect_to idv_doc_auth_url if idv_session.applicant.blank? | ||
| end | ||
|
|
@@ -40,11 +51,4 @@ def idv_session | |
| def idv_attempter_throttled? | ||
| Throttler::IsThrottled.call(current_user.id, :idv_resolution) | ||
| end | ||
|
|
||
| def sp_context_needed? | ||
| return if sp_from_sp_session.present? | ||
| return if LoginGov::Hostdata.env != 'prod' | ||
|
|
||
| redirect_to account_url | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,12 @@ | |||||
| stub_sign_in(user) | ||||||
| end | ||||||
|
|
||||||
| describe 'before_actions' do | ||||||
| it 'includes before_actions from IdvSession' do | ||||||
| expect(subject).to have_actions(:sp_context_needed?) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT of using
Suggested change
|
||||||
| end | ||||||
| end | ||||||
|
|
||||||
| describe '#update' do | ||||||
| it 'sets the uuid in session for the enter info step' do | ||||||
| controller.user_session['idv/cac'] = flow_session | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is pretty broad, I just want to double-check we want to verify in all cases.
One example: When we send user an email including a link to reverify, will there be an SP associated?
identity-idp/app/views/user_mailer/confirm_email_and_reverify.html.erb
Lines 31 to 34 in b7298bc