LG-4305: Log new event in case of lockout from proofing (2)

[aduth]

Originally: #4803

Why: As a login.gov developer, i want to see an event in the event log that indicates that a user was locked out for 6 hours from proofing along with the relevant data points that directly resulted in that lockout, so that I can troubleshoot any issues reported by end users and pinpoint exactly what caused the lock out.

Builds on (merges to) #4824

Cherry-picks from #4803

[mitchellhenke]

I think we need to add this to the index call as well. This case would only be hit if the user submitted the form while they're already throttled, but we don't show the form if they are.

[aduth]

I think we need to add this to the index call as well. This case would only be hit if the user submitted the form while they're already throttled, but we don't show the form if they are.

Ah, gotcha, that makes sense. I'll update.

On a related topic, @amathews-fs and I had discussed a bit if we could log the event only at the point they're initially being redirected to the throttle, vs. each and every time they're presented with a throttle page. I thought that might be what was happening here, as far as being the throttled submission. It might just be simpler and most consistent to always log the throttle on every view, and group by user / session if we need to aggregate.

[mitchellhenke]

Unfortunately not sure it's possible. Current flow is for it to render index when it fails the result, and if that's the final attempt before throttling, index is the only place to reliably catch it.

And actually now that I look at it closer, it's weirdly doing a render of :index when the result is not successful, when it should probably be a redirect...

[aduth]

I think we need to add this to the index call as well. This case would only be hit if the user submitted the form while they're already throttled, but we don't show the form if they are.

Updated in 5dc89f7.

And actually now that I look at it closer, it's weirdly doing a render of :index when the result is not successful, when it should probably be a redirect...

Sounds like a correct assessment to me. Dunno if it makes sense here, but I included it in dd85512. Flash errors would be shown in the redirect, yeah? I don't feel the tests have great coverage for those, despite the test case claiming "to show errors". 🤷

[mitchellhenke]

it uses flash.now[:error] instead of flash[:error] to make the render show the flash message

[aduth]

Test coverage was better than I expected, since it appeared to catch that. Should be better now in d129979, though I don't feel great about the chain of changes required with persisting errors that were previously read directly by the view from the prior form submission. 😬

[mitchellhenke]

I think it should be ok 🙂

[aduth]

I tested it in the app. It seems to work fine, but only after I forcibly modify the code to trigger the validate_pending_profile validation, since from what I can tell, it's impossible for the user to actually trigger it, because its validation effectively occurs in a prior validation. We might want to clean this up (separately), though I guess it may be reasonable to have something explicitly checking the pending profile? 🤷

1. identity-idp/app/forms/verify_account_form.rb

   Line 5 in 376e0ac

   validate :validate_otp

2. identity-idp/app/forms/verify_account_form.rb

   Lines 49 to 50 in 376e0ac

   	def validate_otp
   	return if otp.blank? || valid_otp?

3. identity-idp/app/forms/verify_account_form.rb

   Lines 54 to 55 in 376e0ac

   	def valid_otp?
   	otp.present? && gpo_confirmation_code.present?

4. identity-idp/app/forms/verify_account_form.rb

   Lines 33 to 34 in 376e0ac

   	def gpo_confirmation_code
   	return if otp.blank? || pending_profile.blank?

[mitchellhenke]

ditto for above