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
25 changes: 23 additions & 2 deletions app/javascript/packs/personal-key-page-controller.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import base32Crockford from 'base32-crockford-browser';

const modalSelector = '#personal-key-confirm';
const modal = new window.LoginGov.Modal({ el: modalSelector });

Expand Down Expand Up @@ -43,12 +45,31 @@ function resetForm() {
unsetInvalidHTML();
}

function formatInput(value) {
// Coerce mistaken user input from 'problem' letters:
// https://en.wikipedia.org/wiki/Base32#Crockford.27s_Base32
value = base32Crockford.decode(value);
value = base32Crockford.encode(value);

// Add back the dashes
value = value.toString().match(/.{4}/g).join('-');

// And uppercase
return value.toUpperCase();
}

function handleSubmit(event) {
event.preventDefault();

const value = input.value;
// As above, in case browser lacks HTML5 validation (e.g., IE < 11)
if (input.value.length < 19) {
setInvalidHTML();
return;
}

const value = formatInput(input.value);

if (value.toUpperCase() === personalKey) {
if (value === personalKey) {
unsetInvalidHTML();
// Recovery code page, without js enabled, has a form submission that posts
// to the server with no body.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"dependencies": {
"@rails/webpacker": "^3.5.5",
"base32-crockford-browser": "^1.0.0",
"basscss-sass": "^3.0.0",
"classlist.js": "^1.1.20150312",
"clipboard": "^1.6.1",
Expand Down
21 changes: 20 additions & 1 deletion spec/features/saml/loa1_sso_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
it 'user can view and confirm personal key during sign up', :js do
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
user = create(:user, :with_phone)
code = 'ABC1-DEF2-GHI3-JKL4'
code = 'ABC1-DEF2-GH13-JK14'
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.

What was the reason for changing this?

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.

This example key was incorrect based on our code, and wasn't exposed earlier for lack of test coverage. See:

https://github.com/18F/identity-idp/blob/master/app/services/personal_key_generator.rb#L35

stub_personal_key(user: user, code: code)

loa1_sp_session
Expand All @@ -95,6 +95,25 @@
expect(current_path).to eq sign_up_completed_path
end

it 'coerces invalid characters into their Crockford Base32 equivalents', :js do
displayed_personal_key = '0000-1111-1111-1234'
misread_personal_key = 'ooOO-iiII-llLL-1234'

allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
user = create(:user, :with_phone)
stub_personal_key(user: user, code: displayed_personal_key)

loa1_sp_session
sign_in_and_require_viewing_personal_key(user)
expect(current_path).to eq sign_up_personal_key_path

click_on(t('forms.buttons.continue'))
enter_personal_key_words_on_modal(misread_personal_key)
click_on t('forms.buttons.continue'), class: 'personal-key-confirm'

expect(current_path).to eq sign_up_completed_path
end

it 'redirects user to SP after asking for new personal key during sign up' do
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)

Expand Down
21 changes: 21 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,12 @@ balanced-match@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767"

base32-crockford-browser@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/base32-crockford-browser/-/base32-crockford-browser-1.0.0.tgz#3684970a5826ba1430f01e719cf923b00d773dd8"
dependencies:
optimist ">=0.1.0"

base64-js@^1.0.2:
version "1.2.1"
resolved "https://registry.yarnpkg.com/base64-js/-/base64-js-1.2.1.tgz#a91947da1f4a516ea38e5b4ec0ec3773675e0886"
Expand Down Expand Up @@ -4099,6 +4105,10 @@ minimist@^1.1.3, minimist@^1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.0.tgz#a35008b20f41383eec1fb914f4cd5df79a264284"

minimist@~0.0.1:
version "0.0.10"
resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf"

mississippi@^1.3.0:
version "1.3.1"
resolved "https://registry.yarnpkg.com/mississippi/-/mississippi-1.3.1.tgz#2a8bb465e86550ac8b36a7b6f45599171d78671e"
Expand Down Expand Up @@ -4465,6 +4475,13 @@ opn@^5.1.0:
dependencies:
is-wsl "^1.1.0"

optimist@>=0.1.0:
version "0.6.1"
resolved "https://registry.yarnpkg.com/optimist/-/optimist-0.6.1.tgz#da3ea74686fa21a19a111c326e90eb15a0196686"
dependencies:
minimist "~0.0.1"
wordwrap "~0.0.2"

optimize-css-assets-webpack-plugin@^3.2.0:
version "3.2.0"
resolved "https://registry.yarnpkg.com/optimize-css-assets-webpack-plugin/-/optimize-css-assets-webpack-plugin-3.2.0.tgz#09a40c4cefde1dd0142444a873c56aa29eb18e6f"
Expand Down Expand Up @@ -6851,6 +6868,10 @@ wordwrap@0.0.2:
version "0.0.2"
resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.2.tgz#b79669bb42ecb409f83d583cad52ca17eaa1643f"

wordwrap@~0.0.2:
version "0.0.3"
resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.3.tgz#a3d5da6cd5c0bc0008d37234bbaf1bed63059107"

wordwrap@~1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-1.0.0.tgz#27584810891456a4171c8d0226441ade90cbcaeb"
Expand Down