feat: use method selector in 2fa login

[pcaillaudm]

Related Issue or Design Document

• resolves 2FA login should use method selector #354

Checklist

• I have read the contributing guidelines and signed the CLA.
• I have referenced an issue containing the design document if my change introduces a new feature.
• I have read the security policy.
• I confirm that this pull request does not address a security vulnerability.
  If this pull request addresses a security vulnerability,
  I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
• I have added tests that prove my fix is effective or that my feature works.
• I have added the necessary documentation within the code base (if appropriate).

Further comments

Multiple 2fa methods

Single 2fa method

[codecov]

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 50.61%. Comparing base (f3fad4d) to head (f8dc5f9).
Report is 34 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
+ Coverage   42.43%   50.61%   +8.18%     
==========================================
  Files         136      138       +2     
  Lines        2008     2114     +106     
  Branches      288      316      +28     
==========================================
+ Hits          852     1070     +218     
+ Misses       1149     1032     -117     
- Partials        7       12       +5     

Components	Coverage Δ
@ory/elements-react	47.40% <41.79%> (+10.61%)	⬆️
@ory/nextjs	64.33% <74.24%> (-1.65%)	⬇️

[jonas-jonas]

When code is configured for MFA, the method selector seems to be missing the identifier payload param, leading to this behavior:

Screen.Recording.2025-02-19.at.10.17.54.AM.mov

[jonas-jonas]

I think this is a bit too technical. The previous version might be more accurate. This came from figma?

[pcaillaudm]

Yes I took it from figma. But I do agree, I think the previous version is better in terms of clarity. I can change it back.

[jonas-jonas]

• I think we also need to handle this case:

1. Project has MFA enabled and TOTP enabled
2. user registers, sets up TOTP
3. Project owner disables TOTP, but still requires MFA for all users
4. User sees this empty MFA screen:

In this case, I think we need to show an error. You might be able to adjust this: https://github.com/ory/elements/blob/master/packages/elements-react/src/components/form/form.tsx#L238-L245

[pcaillaudm]

When code is configured for MFA, the method selector seems to be missing the identifier payload param, leading to this behavior:

Screen.Recording.2025-02-19.at.10.17.54.AM.mov

I think this happens because the UpdateLoginFlow endpoint requires the method attribute to be defined. The problem is that the CreateBrowserLoginFlow endpoint does not return a hidden code method node in the case of aal2.

This is working in the current AX because UpdateLoginFlow defaults the method attribute to code if it is not present in the request, but it is supposed to be required.

Your point about the missing identifier node is also valid. Even if it is not strictly required to proceed with the flow, it is necessary to display the "Adjust button" on the 2fa form.

The issue is that not all 2fa methods include the identifier node when initiating an aal2 flow with CreateBrowserLoginFlow. It is not included with totp, lookup_secret, or code methods for instance.

Should I create pull requests in Kratos to address these issues?