feat: replace magic links with one time codes in recovery flow

[jonas-jonas]

This PR introduces a new code strategy to recover an account. Currently, if a user needs to initiate a recovery flow to recover a lost password/MFA/etc., they’ll receive an email containing a “magic link”. This link contains a flow_id and a recovery_token. This is problematic because some Anti Virus software opens links in emails to check for malicious content, etc.

Instead of the magic link, we send an 8-digit code that is clearly displayed in the email or SMS. A user can now copy/paste or type it manually into the text-field that is shown after the user clicks “submit” on the initiate flow page.

Requirements:

• [x] New Email Templates
• [x] New code strategy
• [x] Use code as recovery strategy
• [x] "Anti-Brute Force" - Invalidation of the flow after 5 tries
• [x] E2E Tests

The new flow would look like this:

The flow looks like this if an admin initiates the recovery:

Related issue(s)

Closes #1451

Checklist

• [x] I have read the contributing guidelines.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I am following the contributing code guidelines.
• [x] I have read the security policy.
• [x] 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 green light (please contact [email protected]) from the maintainers to push the changes.
• [x] I have added tests that prove my fix is effective or that my feature works.
• [ ] I have added or changed the documentation.

Further Comments

[codecov[bot]]

Codecov Report

Merging #2645 (a6c8240) into master (0cbfe41) will increase coverage by 0.15%. The diff coverage is 79.81%.

@@            Coverage Diff             @@
##           master    #2645      +/-   ##
==========================================
+ Coverage   75.62%   75.78%   +0.15%     
==========================================
  Files         294      302       +8     
  Lines       17164    17777     +613     
==========================================
+ Hits        12981    13472     +491     
- Misses       3190     3273      +83     
- Partials      993     1032      +39     

Impacted Files	Coverage Δ
driver/registry.go	61.11% <ø> (ø)
identity/credentials.go	75.00% <ø> (ø)
session/session.go	76.92% <0.00%> (-0.67%)	:arrow_down:
ui/node/node.go	92.30% <ø> (+1.09%)	:arrow_up:
courier/email_templates.go	65.95% <50.00%> (-1.49%)	:arrow_down:
selfservice/flow/type.go	50.00% <50.00%> (ø)
courier/template/email/recovery_code_invalid.go	60.00% <60.00%> (ø)
selfservice/strategy/code/strategy.go	66.66% <66.66%> (ø)
selfservice/strategy/code/strategy_recovery.go	69.59% <69.59%> (ø)
selfservice/flow/recovery/handler.go	61.66% <71.42%> (+2.34%)	:arrow_up:
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[aeneasr]

@jonas-jonas can you please rebase onto master?

[jonas-jonas]

@aeneasr rebase done - waiting for tests to confirm.

[aeneasr]

@jonas-jonas can you please work on documentation for this new feature?

We currently have this document ( https://www.ory.sh/docs/kratos/self-service/flows/account-recovery ) but I don't think it's that good (too technical IMO). Maybe we just create a new document under "identity management" called "Account recovery and password reset" which explains how the feature works (what it does), how to configure it, and how to use the SDK to implement it?

[jonas-jonas]

Please also add a "retry" button which, for example, creates a new flow where the user can input their email again. Maybe you can pre-fill the email with the email used in the current flow and add a short message.

I re-used the email parameter and handled it the same way that we handle a user submitting their email address the first time. This way, there was very little extra code needed. There is an E2E test here and an integration test here.

[jonas-jonas]

Instead of iterating over secrets you could consider using just one IN query. Depends though on the overhead of computing potentially unnecessary HMACs. @zepatrik

Not sure why I can't answer directly under this comment, but I thought I'd address this: I changed the implementation to fetch all possible codes from the DB as discussed and iterate through them to check if their codes match. If we find a matching row, we break the loop early. So in the best case this has O(1) and in the worst O(n^2) time complexity.

[aeneasr]

@jonas-jonas please resolve the merge conflicts :)

[aeneasr]

I'll check out the code locally and play around, but code-wise this looks stellar :) Once tests CI is green and I've finished my testing this can land on production next!