LG 10021 hide error after successful webauth authentication

[kevinsmaster5]

🎫 Ticket

LG-10021

🛠 Summary of changes

When a successful authentication is passed and the display of the progress and success containers are toggled, if an error alert container is present, it gets removed from the DOM.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

With an account that has Security Key or F/T Unlock as a MFA option and does not remember your browser...

• Login with username and password
• At Connect Security screen click "use security key" button
• Click cancel on the browser pop up
• You should see the error banner "Oops, something went wrong..."
• Click use security key again then follow through with the unlock procedure
• Security Key verified message should now show and the error banner should not appear.

[aduth]

One downside of how we stub out the WebAuthn behaviors in the specs is that none of this runs in JavaScript, which is where most users would encounter the errors. Curious how difficult it would be to have a JavaScript-enabled test.

[aduth]

I spent part of my afternoon yesterday exploring if we could have more realistic WebAuthn stubbing for JavaScript-enabled browsers in the spec. I think it's possible, but I didn't quite reach a working solution.

Two avenues I considered:

• Replacing the global navigator.credentials.get and navigator.credentials.create with a simulated result
• Using the Chrome DevTools Protocol to issue WebAuthn "bridge" commands (see BrowserEmulationHelper as an example of where we've used DevTools Protocol in the past)

I'd imagine the logic for these could live in the underlying helpers, e.g. mock_press_button_on_hardware_key_on_verification, using different code paths depending if javascript_enabled? is true.

I like the WebAuthn DevTools approach, but I couldn't quite figure out the right parameters to send, and which methods to call. The closest I got to a working solution was implementing the first approach. I got the tests passing locally, but strangely only when the browser was shown (i.e. using the SHOW_BROWSER=true environment variable). It's also kinda unwieldy because it inlines parts of base64ToArrayBuffer.

See code: https://github.com/18F/identity-idp/compare/aduth-try-webauthn-js-stub

Long story short: I worry this test isn't really giving us any regression assurances, since I suspect it passes on main as well. We could alternatively try a JavaScript spec which tests the JavaScript code. That would also be rather difficult with how packs are tightly integrated with page markup and very side-effecty. Maybe in the future we'd refactor the authentication button to a custom element like app/javascript/packages/webauthn/webauthn-authenticate-button-element.ts, which could be easier to test against.

Given all the challenges, I think it'd be okay to leave it like this for now, though maybe we should create a ticket to further explore one of the above alternatives?

[kevinsmaster5]

I will work on creating that ticket.

I can confirm that test does pass on main so your suspicion is correct.

[aduth]

I don't think it's in scope for this ticket, but I'm now curious if there's a similar issue when creating a WebAuthn MFA method for the first time. If so, we should create a ticket for that.

[aduth]

I left a couple long-winded remarks, but I think this is good to go 👍

If you agree on any of the "new ticket" suggestions I mentioned (for improved testing & applying this behavior for WebAuthn enrollment), would you be able to create those?

[aduth]

I spent part of my afternoon yesterday exploring if we could have more realistic WebAuthn stubbing for JavaScript-enabled browsers in the spec. I think it's possible, but I didn't quite reach a working solution.

Two avenues I considered:

• Replacing the global navigator.credentials.get and navigator.credentials.create with a simulated result
• Using the Chrome DevTools Protocol to issue WebAuthn "bridge" commands (see BrowserEmulationHelper as an example of where we've used DevTools Protocol in the past)

I'd imagine the logic for these could live in the underlying helpers, e.g. mock_press_button_on_hardware_key_on_verification, using different code paths depending if javascript_enabled? is true.

I like the WebAuthn DevTools approach, but I couldn't quite figure out the right parameters to send, and which methods to call. The closest I got to a working solution was implementing the first approach. I got the tests passing locally, but strangely only when the browser was shown (i.e. using the SHOW_BROWSER=true environment variable). It's also kinda unwieldy because it inlines parts of base64ToArrayBuffer.

See code: https://github.com/18F/identity-idp/compare/aduth-try-webauthn-js-stub

Long story short: I worry this test isn't really giving us any regression assurances, since I suspect it passes on main as well. We could alternatively try a JavaScript spec which tests the JavaScript code. That would also be rather difficult with how packs are tightly integrated with page markup and very side-effecty. Maybe in the future we'd refactor the authentication button to a custom element like app/javascript/packages/webauthn/webauthn-authenticate-button-element.ts, which could be easier to test against.

Given all the challenges, I think it'd be okay to leave it like this for now, though maybe we should create a ticket to further explore one of the above alternatives?

[aduth]

Technically we're not checking if it's shown, since the element will always exist in the markup, and we're removing it whether or not it's visible. To that point, we may be able to even go as far to assume the element exists and assign the variable above using the TypeScript non-null assertion operator, similar to what we're doing with webauthnInProgressContainer and webauthnSuccessContainer variables before it.

[kevinsmaster5]

I had encountered unwanted behavior where the user not clicking cancel initially got an error then had to re-attempt in order to authenticate. I could not discern why but adding the check with that if statement fixed it.
The element with the class .usa-alert--error gets inserted as that view loads and if it needs to display the error. If there's no error that element doesn't show up in the DOM.
Would it be more preferable to use a non-null operator?