-
Notifications
You must be signed in to change notification settings - Fork 167
LG-7014 Device Profiling on the SSN page #6694
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 10 commits
719b9a1
84ddc25
01ebbda
2908bc7
ddd0ef0
227a018
e20904d
1b6047b
32a885b
10ecc4e
82ee43c
aec8ab8
a14ee08
7060439
75516f4
b6f5357
0272f7c
b2bf717
c940d22
0f6ebc5
d09f775
13b089e
5ced292
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 |
|---|---|---|
|
|
@@ -7,6 +7,10 @@ def call | |
| return invalid_state_response if invalid_state? | ||
|
|
||
| flow_session[:pii_from_doc][:ssn] = flow_params[:ssn] | ||
|
|
||
| if IdentityConfig.store.proofing_device_profiling_collecting_enabled | ||
| flow_session[:threatmetrix_session_id] = threatmetrix_session_id | ||
|
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. Will we want this to be present in the in-person proofing flow version of the SSN step as well?
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. That is a very good question, I am not 100% certain, we need to confirm
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. update: Let's include this on all instances of the SSN page and we can add more specific feature flags if needed |
||
| end | ||
| end | ||
|
|
||
| def extra_view_variables | ||
|
|
@@ -29,6 +33,11 @@ def invalid_state_response | |
| mark_step_incomplete(:document_capture) | ||
| FormResponse.new(success: false) | ||
| end | ||
|
|
||
| def threatmetrix_session_id | ||
| return nil if !IdentityConfig.store.proofing_device_profiling_collecting_enabled | ||
|
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. Same as above, do we need to check |
||
| SecureRandom.uuid | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| <%= render 'idv/shared/ssn', flow_session: flow_session, success_alert_enabled: !updating_ssn, updating_ssn: updating_ssn %> | ||
| <%= render 'idv/shared/ssn', flow_session: flow_session, success_alert_enabled: !updating_ssn, updating_ssn: updating_ssn, threatmetrix_session_id: threatmetrix_session_id %> |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,10 @@ | |||||||
| IdentityConfig.store.asset_host.presence, | ||||||||
| ].compact | ||||||||
|
|
||||||||
| if IdentityConfig.store.proofing_device_profiling_collecting_enabled | ||||||||
| script_src << 'h.online-metrix.net' | ||||||||
| end | ||||||||
|
|
||||||||
|
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. After talking with @zachmargolis, I realized that I gave some bad guidance here. Let's not modify CSP in all environments: We want to ensure that we are not making calls to 3rd parties outside of production / staging. Apologies!
Suggested change
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. We'd still need some CSP revisions for this to work at all, wouldn't we? I would expect some
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. That's my understanding as well
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. I think we'll need the following:
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. (I'll leave a followup ticket for CSP stuff)
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. Out of curiosity, why would
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. ThreatMetrix uses |
||||||||
| script_src = [:self, :unsafe_eval] if !Rails.env.production? | ||||||||
|
|
||||||||
| style_src = [:self, IdentityConfig.store.asset_host.presence].compact | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| require 'rails_helper' | ||
|
|
||
| describe 'idv/shared/_ssn.html.erb' do | ||
| include Devise::Test::ControllerHelpers | ||
|
|
||
| let(:proofing_device_profiling_collecting_enabled) { nil } | ||
| let(:lexisnexis_threatmetrix_account_id) { 'test_org_id' } | ||
| let(:session_id) { 'ABCD-1234' } | ||
| let(:updating_ssn) { false } | ||
| let(:js_domain) { 'h.online-metrix.net' } | ||
|
|
||
| let(:tags_js_url) { | ||
| "https://#{js_domain}/fp/tags.js?org_id=#{lexisnexis_threatmetrix_account_id}&session_id=#{session_id}" | ||
| } | ||
|
|
||
| let(:tags_iframe_url) { | ||
| "https://#{js_domain}/fp/tags?org_id=#{lexisnexis_threatmetrix_account_id}&session_id=#{session_id}" | ||
| } | ||
|
|
||
| let(:script_tag_rendered) { | ||
| expect(rendered).to have_css("script[src='#{tags_js_url}']", visible: false) | ||
| } | ||
|
|
||
| let(:noscript_tag_rendered) { | ||
| expect(rendered).to have_css("noscript iframe[src='#{tags_iframe_url}']", visible: false) | ||
| } | ||
|
|
||
| let(:session_id_input_rendered) { | ||
| expect(rendered). | ||
| to have_css( | ||
| "input[type=hidden][name='doc_auth[threatmetrix_session_id]'][value='#{session_id}']", | ||
| visible: false, | ||
| ) | ||
| } | ||
|
|
||
| let(:script_tag_not_rendered) { | ||
| expect(rendered).not_to have_css("script[src*='#{js_domain}']", visible: false) | ||
| } | ||
|
|
||
| let(:noscript_tag_not_rendered) { | ||
| expect(rendered).not_to have_css("noscript iframe[src*='#{js_domain}']", visible: false) | ||
| } | ||
|
zachmargolis marked this conversation as resolved.
Outdated
|
||
|
|
||
| let(:session_id_input_not_rendered) { | ||
| expect(rendered). | ||
| not_to have_css('input[name="doc_auth[threatmetrix_session_id]"]', visible: false) | ||
| } | ||
|
zachmargolis marked this conversation as resolved.
Outdated
|
||
|
|
||
| before :each do | ||
| allow(view).to receive(:url_for).and_return('https://example.com/') | ||
|
|
||
| allow(IdentityConfig.store). | ||
| to receive(:proofing_device_profiling_collecting_enabled). | ||
| and_return(proofing_device_profiling_collecting_enabled) | ||
| allow(IdentityConfig.store). | ||
| to receive(:lexisnexis_threatmetrix_account_id).and_return(lexisnexis_threatmetrix_account_id) | ||
|
|
||
| render partial: 'idv/shared/ssn', locals: { | ||
| flow_session: { | ||
| threatmetrix_session_id: session_id, | ||
| }, | ||
| success_alert_enabled: false, | ||
| updating_ssn: updating_ssn, | ||
| } | ||
| end | ||
|
|
||
| context 'when threatmetrix collection enabled' do | ||
| let(:proofing_device_profiling_collecting_enabled) { true } | ||
|
|
||
| context 'and org id specified' do | ||
| context 'and entering ssn for the first time' do | ||
| describe '<script> tag' do | ||
| it 'is rendered' do | ||
| script_tag_rendered | ||
| end | ||
| end | ||
|
|
||
| describe '<noscript> tag' do | ||
| it 'is rendered' do | ||
| noscript_tag_rendered | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'updating ssn already entered' do | ||
| let(:updating_ssn) { true } | ||
|
|
||
| it 'does not render <script> tag' do | ||
| script_tag_not_rendered | ||
| end | ||
| it 'does not render <noscript> tag' do | ||
| noscript_tag_not_rendered | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'org id not specified' do | ||
| let(:lexisnexis_threatmetrix_account_id) { '' } | ||
|
|
||
| it 'does not render <script> tag' do | ||
| script_tag_not_rendered | ||
| end | ||
| it 'does not render <noscript> tag' do | ||
| noscript_tag_not_rendered | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'threatmetrix collection disabled' do | ||
| let(:proofing_device_profiling_collecting_enabled) { false } | ||
|
|
||
| it 'does not render <script> tag' do | ||
| script_tag_not_rendered | ||
| end | ||
| it 'does not render <noscript> tag' do | ||
| noscript_tag_not_rendered | ||
| end | ||
| end | ||
| end | ||
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.
Will this overwrite the existing
:threatmetrix_session_id(if the user is coming back to edit their SSN)? I think we want to hang on to the original session ID in that case.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.
It seems like it would, yeah, which we probably want to avoid?