From 03c0102276ef7fef600d702f82de29c7ee9203bf Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 21 Apr 2022 15:41:03 -0400 Subject: [PATCH 1/6] LG-6160: Validate personal key value as case-insensitive changelog: Upcoming Features, Identity Verification, Add personal key step screen --- .../steps/personal-key-confirm/personal-key-input.spec.tsx | 6 +++--- .../steps/personal-key-confirm/personal-key-input.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx index 70d63c24fb6..b3e090cc46a 100644 --- a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx +++ b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx @@ -49,12 +49,12 @@ describe('PersonalKeyInput', () => { expect(input.value).to.equal('1234-1234-1234-1234'); }); - it('validates the input value against the expected value', async () => { - const { getByRole } = render(); + it('validates the input value against the expected value, case-insensitive', async () => { + const { getByRole } = render(); const input = getByRole('textbox') as HTMLInputElement; - await userEvent.type(input, '0000-0000-0000-000'); + await userEvent.type(input, 'ABCD-0000-defg-000'); input.checkValidity(); expect(input.validationMessage).to.equal('users.personal_key.confirmation_error'); diff --git a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx index 478244e64bb..2ed6661ff8c 100644 --- a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx +++ b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx @@ -23,7 +23,7 @@ function PersonalKeyInput( ) { const validate = useCallback( (value) => { - if (expectedValue && value !== expectedValue) { + if (expectedValue && value.toLowerCase() !== expectedValue.toLowerCase()) { throw new Error(t('users.personal_key.confirmation_error')); } }, From d4599a58a2dba596c39a6b4a42d25626f8c81014 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Apr 2022 13:35:23 -0400 Subject: [PATCH 2/6] Enhance feature spec for supportable, jumbled entry --- spec/support/features/personal_key_helper.rb | 24 +++++++++++++++----- spec/support/features/session_helper.rb | 11 +++------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/spec/support/features/personal_key_helper.rb b/spec/support/features/personal_key_helper.rb index 7b95f69a9a6..31cf2bb3847 100644 --- a/spec/support/features/personal_key_helper.rb +++ b/spec/support/features/personal_key_helper.rb @@ -27,11 +27,23 @@ def trigger_reset_password_and_click_email_link(email) click_email_link_matching(/reset_password_token/) end - def scrape_personal_key - new_personal_key_words = [] - page.all(:css, '[data-personal-key]').each do |node| - new_personal_key_words << node.text - end - new_personal_key_words.join('-') + def scrape_personal_key(jumbled_for_entry: false) + code_segments = page.all(:css, '.separator-text__code').map(&:text) + + return code_segments.join('-') if !jumbled_for_entry + + # Include dash between some segments and not others + code = code_segments[0..1].join('-') + code_segments[2..3].join + + # Randomize case + code = code.chars.map { |c| (rand 2) == 0 ? c.downcase : c.upcase }.join + + # De-normalize Crockford encoding + code = code.sub('1', 'l').sub('0', 'O') + + # Add extra characters + code += 'abc123qwerty' + + code end end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 931eabe6d07..16c3150d4a8 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -2,6 +2,8 @@ module Features module SessionHelper + include PersonalKeyHelper + VALID_PASSWORD = 'Val!d Pass w0rd'.freeze def sign_up_with(email) @@ -307,18 +309,11 @@ def sign_in_with_totp_enabled_user end def acknowledge_and_confirm_personal_key(js: true) - extra_characters_get_ignored = 'abc123qwerty' - code_words = [] - - page.all(:css, '[data-personal-key]').map do |node| - code_words << node.text - end - button_text = t('forms.buttons.continue') click_on button_text, class: 'personal-key-continue' if js - fill_in 'personal_key', with: code_words.join.downcase + extra_characters_get_ignored + fill_in 'personal_key', with: scrape_personal_key(jumbled_for_entry: true) find_all('.personal-key-confirm', text: button_text).first.click end From 87b7c5d83507b789ddcd3f25383bdcf93bb5c2a0 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Apr 2022 13:53:57 -0400 Subject: [PATCH 3/6] Create standalone test case for personal key feature spec --- spec/support/features/personal_key_helper.rb | 20 ++------------- .../shared_examples_for_personal_keys.rb | 25 ++++++++++++++++++- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/spec/support/features/personal_key_helper.rb b/spec/support/features/personal_key_helper.rb index 31cf2bb3847..3b2a651a25c 100644 --- a/spec/support/features/personal_key_helper.rb +++ b/spec/support/features/personal_key_helper.rb @@ -27,23 +27,7 @@ def trigger_reset_password_and_click_email_link(email) click_email_link_matching(/reset_password_token/) end - def scrape_personal_key(jumbled_for_entry: false) - code_segments = page.all(:css, '.separator-text__code').map(&:text) - - return code_segments.join('-') if !jumbled_for_entry - - # Include dash between some segments and not others - code = code_segments[0..1].join('-') + code_segments[2..3].join - - # Randomize case - code = code.chars.map { |c| (rand 2) == 0 ? c.downcase : c.upcase }.join - - # De-normalize Crockford encoding - code = code.sub('1', 'l').sub('0', 'O') - - # Add extra characters - code += 'abc123qwerty' - - code + def scrape_personal_key + page.all(:css, '.separator-text__code').map(&:text).join('-') end end diff --git a/spec/support/shared_examples_for_personal_keys.rb b/spec/support/shared_examples_for_personal_keys.rb index a6d7b950c77..ca655eaedd2 100644 --- a/spec/support/shared_examples_for_personal_keys.rb +++ b/spec/support/shared_examples_for_personal_keys.rb @@ -1,5 +1,5 @@ shared_examples_for 'personal key page' do - include XPathHelper + include PersonalKeyHelper context 'informational text' do context 'modal content' do @@ -33,5 +33,28 @@ code = page.all('[data-personal-key]').map(&:text).join('-') expect(copied_text).to eq(code) end + + it 'validates as case-insensitive, crockford-normalized, length-limited, dash-flexible' do + code_segments = scrape_personal_key.split('-') + + # Include dash between some segments and not others + code = code_segments[0..1].join('-') + code_segments[2..3].join + + # Randomize case + code = code.chars.map { |c| (rand 2) == 0 ? c.downcase : c.upcase }.join + + # De-normalize Crockford encoding + code = code.sub('1', 'l').sub('0', 'O') + + # Add extra characters + code += 'abc123qwerty' + + click_acknowledge_personal_key + page.find(':focus').fill_in with: code + + path_before_submit = current_path + within('[role=dialog]') { click_on t('forms.buttons.continue') } + expect(current_path).not_to eq path_before_submit + end end end From 64a2269fc97186bf04f82035789986604f552d0a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Apr 2022 13:55:13 -0400 Subject: [PATCH 4/6] Drop removed kwarg --- spec/support/features/session_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 16c3150d4a8..a12a124dbff 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -313,7 +313,7 @@ def acknowledge_and_confirm_personal_key(js: true) click_on button_text, class: 'personal-key-continue' if js - fill_in 'personal_key', with: scrape_personal_key(jumbled_for_entry: true) + fill_in 'personal_key', with: scrape_personal_key find_all('.personal-key-confirm', text: button_text).first.click end From a6404d6d1ddd06bed7a80cb7c6b6721728be148c Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Apr 2022 14:20:34 -0400 Subject: [PATCH 5/6] Add Crockford Base32 normalization for PersonalKeyInput validation --- .../personal-key-confirm/personal-key-input.spec.tsx | 10 +++++----- .../steps/personal-key-confirm/personal-key-input.tsx | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx index b3e090cc46a..eccb759d083 100644 --- a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx +++ b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx @@ -49,17 +49,17 @@ describe('PersonalKeyInput', () => { expect(input.value).to.equal('1234-1234-1234-1234'); }); - it('validates the input value against the expected value, case-insensitive', async () => { - const { getByRole } = render(); + it('validates the input value against the expected value (case-insensitive, crockford)', async () => { + const { getByRole } = render(); const input = getByRole('textbox') as HTMLInputElement; - await userEvent.type(input, 'ABCD-0000-defg-000'); + await userEvent.type(input, 'ABCDoOlL-defg-001'); input.checkValidity(); expect(input.validationMessage).to.equal('users.personal_key.confirmation_error'); - await userEvent.type(input, '0'); + await userEvent.type(input, '1'); input.checkValidity(); - expect(input.validationMessage).to.be.empty(); + expect(input.validity.valid).to.be.true(); }); }); diff --git a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx index 2ed6661ff8c..03d2a835c3f 100644 --- a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx +++ b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.tsx @@ -17,13 +17,22 @@ interface PersonalKeyInputProps { onChange?: (nextValue: string) => void; } +/** + * Normalize an input value for validation comparison. + * + * @param string Denormalized value. + * + * @return Normalized value. + */ +const normalize = (string: string) => string.toLowerCase().replace(/o/g, '0').replace(/[il]/g, '1'); + function PersonalKeyInput( { expectedValue, onChange = () => {} }: PersonalKeyInputProps, ref: ForwardedRef, ) { const validate = useCallback( (value) => { - if (expectedValue && value.toLowerCase() !== expectedValue.toLowerCase()) { + if (expectedValue && normalize(value) !== normalize(expectedValue)) { throw new Error(t('users.personal_key.confirmation_error')); } }, From fbfb9afd15e79a2d7e43cef4db20b553a73aeb01 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 22 Apr 2022 14:22:48 -0400 Subject: [PATCH 6/6] get some i-coverage in there too --- .../steps/personal-key-confirm/personal-key-input.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx index eccb759d083..828de0d5291 100644 --- a/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx +++ b/app/javascript/packages/verify-flow/steps/personal-key-confirm/personal-key-input.spec.tsx @@ -50,11 +50,11 @@ describe('PersonalKeyInput', () => { }); it('validates the input value against the expected value (case-insensitive, crockford)', async () => { - const { getByRole } = render(); + const { getByRole } = render(); const input = getByRole('textbox') as HTMLInputElement; - await userEvent.type(input, 'ABCDoOlL-defg-001'); + await userEvent.type(input, 'ABCDoOlL-defg-iI1'); input.checkValidity(); expect(input.validationMessage).to.equal('users.personal_key.confirmation_error');