LG-14552: Ensure users can't reach the IPP verify info controller without a threatmetrix session id#11858
Conversation
…reatmetrix session id
…ify info controller without a threatmetrix session id
| include IdvStepConcern | ||
|
|
||
| before_action :confirm_step_allowed | ||
| before_action :confirm_in_person_address_step_needed, only: :show |
There was a problem hiding this comment.
I removed confirm_in_person_address_step_needed in order to conform with the flow policy documentation.
| user_session.fetch('idv/in_person', {}) | ||
| end | ||
|
|
||
| def confirm_repeat_ssn |
There was a problem hiding this comment.
If anyone could offer more insight or historical context about this method's purpose, I'd appreciate it.
There was a problem hiding this comment.
I think this was in place to support going back to the SSN entry screen from the verify info step back when the back button was not allowed. FlowPolicy obviates the need for (IMO) janky stuff like checking the request.referer.
| expect(response).to render_template :show | ||
| end | ||
|
|
||
| context 'when address1 present' do |
There was a problem hiding this comment.
Since the flow policy stipulates that we should rely on the before_action confirm_step_allowed and not use other before_actions, I removed the before_action confirm_in_person_address_step_needed . As a result, this test is no longer relevant.
@jennyverdeyen I'd be curious to hear any thoughts you have on this!
There was a problem hiding this comment.
This makes sense! Since we know this was a duplicate check for the redirect behavior that was already being handled in the state id controller, I went to see if there were tests for this in the state id controller specs. I think there maybe aren't... Is this something we should add there?
There was a problem hiding this comment.
@jennyverdeyen Good point! I added a new test to the address_controller_spec in 2b4aa47. Let me know if that provides enough coverage or if you want me to add another test.
eileen-nava
changed the title
jennyverdeyen
left a comment
jennyverdeyen
left a comment
There was a problem hiding this comment.
I responded to a comment you left and made a suggestion but I'll leave it up to you if it needs to be addressed in this PR. Otherwise I approve, looks great!
| expect(response).to render_template :show | ||
| end | ||
|
|
||
| context 'when address1 present' do |
There was a problem hiding this comment.
This makes sense! Since we know this was a duplicate check for the redirect behavior that was already being handled in the state id controller, I went to see if there were tests for this in the state id controller specs. I think there maybe aren't... Is this something we should add there?
3 participants
🎫 Ticket
LG-14552: Prevent navigating to IPP VerifyInfoController without a threatmetrix_session_id
🛠 Summary of changes
For historical context, here was a related Ada ticket, https://cm-jira.usa.gov/browse/LG-14393, and PR #11254.
📜 Testing Plan
Run automated tests.