Annotate sign-in reCAPTCHA from 2-factor attempts

[vrajmohan]

changelog: Upcoming Features, Recaptcha, Annotate sign-in reCAPTCHA from 2-factor attempts

🎫 Ticket

Link to the relevant ticket:
Annotate sign-in reCAPTCHA after fully authenticated

🛠 Summary of changes

1. Store the recaptcha_assessment_id in session.
2. Annotate with INITIATED_TWO_FACTOR when attempting any MFA method if there is a recaptcha_assessment_id in session.
3. Annotate with PASSED_TWO_FACTOR or FAILED_TWO_FACTOR depending on success when completing any MFA method if there is a recaptcha_assessment_id in session.
4. Recording the recaptcha_annotation along with the pre-existing event whenever an annotation is made.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Enable reCAPTCHA in local environment using credentials from https://docs.google.com/document/d/1mXa1KtVdXvJ1QPF8T0vvhWxsabr-OzuPqcgNre_3wqU/edit?pli=1&tab=t.0
• Bump sign_in_recaptcha_percent_tested to 100
• Authenticate to your local environment using an incognito window and go through various MFA options. Fail intentionally so that you also test the FAILED_TWO_FACTOR annotations.
• Observe the events with recaptcha_annotation in log/events.log

[vrajmohan]

Some open questions:

1. Annotate on all MFA results - annotate on failures as well as it is an easy lift
2. What is the value of an INITITATED_TWO_FACTOR annotation?
3. Use plain :recaptcha_assessment_id in session as there's no value in a more specific namespace
4. Should the session value be deleted upon use?
5. Value in annotation argument to RecaptchaAnnotator

[aduth]

What is the value of an INITITATED_TWO_FACTOR annotation?

From what I recall, it's used by Google to interpret as a failed or abandoned attempt if the user never subsequently succeeds . The documentation says "old assessment with ENUM_VALUES.INITIATED_TWO_FACTOR reason that has not been overwritten with PASSED_TWO_FACTOR is treated as an abandoned 2FA flow".

Should the session value be deleted upon use?

I think it's best to clean up if we don't intend to use it again (including browser back button, etc.), particularly if we're going to store it in session and not in user_session (the latter having a more predictable clean-up mechanism).

[vrajmohan]

Deleting the annotation id after use would prevent us from annotating a subsequent PASSED_TWO_FACTOR after a FAILED_TWO_FACTOR annotation. (I acknowledge that FAILED_TWO_FACTOR is scope creep for this story.) We could definitely delete after a PASSED_TWO_FACTOR, as I see less benefit in accounting for the other way (ie PASSED_TWO_FACTOR followed by FAILED_TWO_FACTOR).

[aduth]

Is this to avoid errors on missing documentation for the analytics method call? I'd expect we'd update the analytics method signatures to include this property if it's something we expect will be logged.

[vrajmohan]

IMO, the recaptcha_annotation is of lesser interest in the events being logged and I was leveraging the **extra argument in the signatures of all the affected events.

If we think it is important and want to add the documentation, I can do so.

[aduth]

I think it's relevant to be documented.

[aduth]

Is this missing INITIATED_TWO_FACTOR for phone MFAs?

[aduth]

To leave open the option of using reCAPTCHA for other, non-sign-in features, I think it could be wise to add a qualifier here, similar to what we do with user_session[:phone_recaptcha_assessment_id] (e.g. session[:sign_in_recaptcha_assessment_id]).

[vrajmohan]

This was my open question #3:

Use plain :recaptcha_assessment_id in session as there's no value in a more specific namespace

Will Google assign different assessment ids for the same device session?

My thought was to create a ticket to move user_session[:phone_recaptcha_assessment_id] also to use session[:recaptcha_assessment_id].

[aduth]

They're separate assessments, and we'd want to annotate them separately. I don't think we want to consolidate the values.

[vrajmohan]

Is this missing INITIATED_TWO_FACTOR for phone MFAs?

I was confused between otp_verification and totp_verification. Looking deeper...

[aduth]

LGTM