umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.41k stars 2.66k forks source link

Autolinking does not create a user and results in a GetCurrentUser 401 #14775

Closed PimVendrig closed 11 months ago

PimVendrig commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

12.1.2

Bug summary

Following the instructions on https://docs.umbraco.com/umbraco-cms/v/10.latest-lts/reference/security/auto-linking#example-for-users I linked with the OpenIdConnect demo project on https://demo.duendesoftware.com to be able to autolink backoffice users.

When I log in, no user is created in Umbraco and I see a white screen in the backoffice with error GetCurrentUser 401 in the browser console.

Specifics

When I set allowManualLinking to true, I am able to link an external account to my local account. After logging out, I am able to log in with that external account. However it is not possible to log in with an unlinked external account.

The NameClaimType has been set to name, and email is present as well.

It seems related to #14741 and #14724, but I always return true at OnExternalLogin.

Steps to reproduce

  1. Clean umbraco install
  2. Add code from https://docs.umbraco.com/umbraco-cms/v/10.latest-lts/reference/security/auto-linking#example-for-users :
    • dotnet add package Microsoft.AspNetCore.Authentication.OpenIdConnect
    • Add class OpenIdConnectBackOfficeExternalLoginProviderOptions
    • Add class OpenIdConnectExtension
    • Include AddOpenIdConnectAuthentication in Startup
  3. Modify the options in OpenIdConnectExtension:
    options.Authority = "https://demo.duendesoftware.com";
    options.ClientId = "interactive.confidential";
    options.ClientSecret = "secret";
    ...
    //options.Scope.Add("roles");
  4. Add events for debugging in OpenIdConnectExtension:
    options.Events.OnTokenValidated = context => {
    return Task.CompletedTask;
    };
    options.Events.OnTicketReceived = context => {
    return Task.CompletedTask;
    };
  5. Modify the options in OpenIdConnectBackOfficeExternalLoginProviderOptions:
    allowManualLinking: true
  6. Run Umbraco, log in with your local admin account, click on your profile and click Link your OpenIdConnect account. Log in with alice/alice on demo.duendesoftware.com. The external account is linked succesfully.
  7. Log out at localhost and demo.duendesoftware.com
  8. Put breakpoints on OnAutoLinking, OnExternalLogin, OnTokenValidated, OnTicketReceived.
  9. Click Sign in with OpenIdConnect, log in with alice/alice on demo.duendesoftware.com. Notice the breakpoints at OnTokenValidated, OnTicketReceived and OnExternalLogin are hit, and the user is succesfully logged in. The details of the Claims in OnTicketReceived are as follows: image
  10. Log out at localhost and demo.duendesoftware.com
  11. Click Sign in with OpenIdConnect, log in with bob/bob on demo.duendesoftware.com. Notice only the breakpoints at OnTokenValidated and OnTicketReceived are hit. The user is shown the white screen and in the browser console the GetCurrentUser 401 error is shown.

Expected result / actual result

I would expect the breakpoint in OnAutoLinking to be hit, and a new user to be created when logging in as bob. Am I missing some configuration, or is there a bug here?


_This item has been added to our backlog AB#32692_

github-actions[bot] commented 1 year ago

Hi there @PimVendrig!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

kjac commented 1 year ago

Hi @PimVendrig 👋

Thank you for reaching out to us. And massive H5 for the excellent issue description and reproduction steps 💯

External auth can be tricky, and sometimes the devil is truly in the minuscule detail. Also sometimes there might be a bug involved 🙈

I'll dig into this one of the coming days and get back to you.

PimVendrig commented 1 year ago

Hi Kenn, Thank you for your response. I hope you can find out what is going wrong. Looking forward to see your results.

kjac commented 1 year ago

Hi @PimVendrig,

So... this is an interesting one 😄

It all boils down to your IdentityServer not supplying the correct claim for user emails. The email claim type should be http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress (corresponding to System.Security.Claims.ClaimTypes.Email) but is email:

image

For reference, here's a correct response from my test setup with GitHub auth for users:

image

As a result I am unable to perform log-in with both "alice" and "bob".

Here's where it gets interesting. We should see an error message on the log-in screen after the failed log-in attempt:

image

...but something in the code detects a successful login and tries to redirect the browser to the back-office. Which then fails miserably, causing the blank screen to appear.

Since you have managed to log-in the "alice" user, my guess is that something has changed in your IdentityServer setup over time, causing the email claim type to change in the response. If you can somehow re-construct the correct email claim type, things should work with your setup.

I'm going to take this issue back to the team. We definitively need to fix the log-in screen, so the error is shown correctly.

kjac commented 1 year ago

Quick follow-up. The next two weeks are completely blocked time-wise, but I'll be looking into this soon thereafter. Thank you so much for supplying a meaningful identity provider to test this on, @PimVendrig - even though I know you weren't really planning on doing that 😛

PimVendrig commented 1 year ago

Hi @kjac, Thank you for your investigation.

I have modified the code by adding the following line: options.ClaimActions.MapUniqueJsonKey(ClaimTypes.Email, "email");

Now the breakpoint in OnAutoLinking is hit when logging in as bob, and the user is created in Umbraco (now I can start writing the logic to implement the IsApproved etc).

The reason alice was linked succesfully I think, is because linking an external account to an existing account does not require an emailaddress since the user already has one. Configuring that the name-claim is different was possible in options.TokenValidationParameters.NameClaimType, however for Email it seems that it is not possible. Therefore used the approach to add the http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress-claim as well.

Thank you so much for supplying a meaningful identity provider to test this on, @PimVendrig

You're welcome! 😀

Zeegaan commented 11 months ago

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/14867