LG-14261 Add attempt count to mfa setup auth events

[kevinsmaster5]

🎫 Ticket

Link to the relevant ticket: LG-14261

🛠 Summary of changes

Adds a session token to track number of attempts when setting up an mfa type. Total sum of attempts gets passed to analytics_events. Backup codes are excluded since during setup they're added instantly.

📜 Testing Plan

Checklist of steps to confirm the changes.

• [ ] In a separate console run make watch_events
• [ ] http://localhost:3000 with existing or new account add MFA.
• [ ] Observe in the watch_events feed 'Multi-Factor Authentication Setup' that a predictable value for properties > event_properties > mfa_attempts is registered

Attempt count will increment by failing and retrying to add mfa for example:

• Webauthn (Face|Touch or security key) clicking cancel at the browser dialog before completing setup
• PIV test screen select a failing condition
• SMS/Voice enter (change) supplied auth code
• Auth App enter wrong auth code

👀 Screenshots

[kevinsmaster5]

Taking a look at second_factor_attempts_count to see if that might take the place of creating a session token.

[kevinsmaster5]

Taking a look at second_factor_attempts_count to see if that might take the place of creating a session token.

seems to only work for OTP.

[kevinsmaster5]

Looks good and seems to work locally, but wondering if theres a way for us to push this out to a helper or concern for the verification controllers.

Do you mean something like abstracting this sort of thing into a concern?

session[:piv_cac_attempts] ||= 0 session[:piv_cac_attempts] += 1

It does look repetitive since it's sort of identical in 4 places.

[kevinsmaster5]

Added suggestion from https://github.com/18F/identity-idp/pull/11293#pullrequestreview-2341388595

[aduth]

If someone fails authenticating or setting up with one authentication method and then switches to another, what do we expect the mfa_attempts to be for that second MFA method? I think we wouldn't want to unfairly bias attempts for that second method based on failures with another method.

I think we should either only log the attempts for that individual method, or the log value should be a hash of attempts for each method. For both cases, I'd imagine the session value would be a hash of method-to-attempts.

[kevinsmaster5]

I think we should either only log the attempts for that individual method, or the log value should be a hash of attempts for each method. For both cases, I'd imagine the session value would be a hash of method-to-attempts.

So rather than "event_properties": { "success": true, "errors": {}, "multi_factor_auth_method": "piv_cac", "in_account_creation_flow": true, "enabled_mfa_methods_count": 1, "mfa_attempts": 1 },

Would we want to see something like "event_properties": { "success": true, "errors": {}, "multi_factor_auth_method": "piv_cac", "in_account_creation_flow": true, "enabled_mfa_methods_count": 1, "mfa_attempts": { "piv_cac": 1 } },

[aduth]

I think we could keep the logged value a single integer if we wanted, but in order to make sure that we're only logging attempts for that method, we'd need to track the attempts in the session as per-method (a hash).

So the session value would look something like the hash in your example, and getting the value from the session is something like user_session[:mfa_attempts][method].

[voidlily]

can you rebase this branch to pick up changes to the reviewapp deploy process?