fix: consider OIDC registration flows errored with duplicate credential to be completed by strategy

[Saancreed]

Returning anything else here may cause Kratos to respond with two concatenated JSON objects: new login flow with actual error message as the first one and a very confusing '500, aborted registration hook execution' as the second one.

Related issue(s)

To reproduce this bug on current master:

1. Launch Kratos where default identity has email as identifier and allows registration via password and some OIDC provider (e.g. Google).
2. Register using password, providing the same email as the one from the Google account that will be used in the next step.
3. Try to register using Google account with matching email, using API flow and sending id_token in the request body to avoid webview/redirections.

This will trigger the code path designed to handle identity.ErrDuplicateCredentials and try to convert the registration to a login flow containing the expected An account with the same identifier (email, phone, username, ...) exists already. message, then write this new login flow into HTTP response body. However, because registration.ErrHookAbortFlow is returned just a bit further, the outer request handler will write another object to the response body, this one containing very confusing "500, aborted registration hook execution" error.

As far as I can tell, this is a malformed JSON.

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• 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 the 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 or changed the documentation.

Further Comments

I discovered this when rebasing #3416 because I ran into it while investigating why e2e tests related to settings flows are suddenly failing. Even though applying this fix this didn't fix those tests for me, I realized that this issue wasn't caused by my code and can be also be reproduced on current master when I prepare the request in a way that avoids the OIDC redirection by other means, e.g. passing the id_token from native Google SDK.

Here's a picture with request made using Postman, showing an example of such malformed response:

By the way, is it actually allowed for Kratos to reply with a totally different kind of flow (in this case, login one) when calling POST / Update on another flow directly in the response body rather than always issuing a redirect, either via HTTP 303 or 422 with {"error":{"id":"browser_location_change_required","code":422,"status":"Unprocessable Entity","message":"browser location change required"},"redirect_browser_to":"…/login?flow=…"}? Because the reference React / SPA client used in e2e tests doesn't seem to handle this well and never displays the An account with the same identifier (email, phone, username, ...) exists already. message to the user, instead simply returning to main/welcome page.

[codecov]

Codecov Report

Merging #3525 (9911986) into master (6a0a914) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 9911986 differs from pull request most recent head a6e768f. Consider uploading reports for the commit a6e768f to get more accurate results

@@            Coverage Diff             @@
##           master    #3525      +/-   ##
==========================================
+ Coverage   78.22%   78.24%   +0.01%     
==========================================
  Files         341      341              
  Lines       23141    23098      -43     
==========================================
- Hits        18103    18073      -30     
+ Misses       3681     3674       -7     
+ Partials     1357     1351       -6     

Files	Coverage Δ
selfservice/strategy/oidc/strategy.go	64.05% <100.00%> (ø)

... and 3 files with indirect coverage changes

[Benehiko]

Interesting :)

I think this LGTM

Seems this was my doing 4d2fda4 😆