Umbraco-CMS
Umbraco-CMS copied to clipboard
Autolinking - Make emailaddress fetching more robust
Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9
10.0.1
Bug summary
Umbraco has a autolinking abillity (https://our.umbraco.com/documentation/reference/security/auto-linking/). For autolinking to work, it needs a emailaddress of the external user. The emailaddress of the external user is fetched from a claim by using the claim-type: "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress".
In my opinion there are several issues:
- The documentation is not clear about which claim-type is being used to fetch the emailaddress.
- It's debatable if the "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" claim-type should be used.
- Also debatable: Umbraco has no abillity to override the claim-type that is being used to fetch the emailaddress (this is debatable because .netcore has a way to extend user claims, see the specifics).
More discussion can be found here: #12670
Specifics
The BackOfficeSignInManager class is responsible for the autolinking-feature. There is a AutoLinkAndSignInExternalAccount function which is being executed when the external-signin was succesfull. On row 191 you can see that the emailadress is being fetched from the claims:
Notice that the "ClaimTypes.Email" constant is being used, which refers to the value: "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress".
I did some google-ing this morning and i believe that the "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" claimtype is often used in the Microsoft AD realm (Azure B2C for instance). So, in my opinion, it is debatable if Umbraco should use that specific claimtype to retrieve the emailaddress. The umbraco-documentation should point this out, because without looking at the umbraco source code there is no way someone would know this! I spent a whole morning figuring out why the user did not get created, only when i cloned and debugged the umbracoCMS repo I figured out the issue.
So if your identity provider is not returning the emailaddress in the claim-type "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" you have a problem. Umbraco will just throw an "FailedNoEmail" error and thus the user will not be created in the umbraco backend!
Fortunatly I found a way to extend the authorization claims! There is a IClaimsTransformation you can implement, which let you add custom claims (original source: https://visualstudiomagazine.com/articles/2019/11/01/authorization-claims.aspx):
public class UserInfoClaims : IClaimsTransformation
{
public Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal principal)
{
var id = new ClaimsIdentity();
string email = principal.FindFirstValue("email");
if (!String.IsNullOrEmpty(email)){
id.AddClaim(new Claim(ClaimTypes.Email, email));
principal.AddIdentity(id);
}
return Task.FromResult(principal);
}
}
In this way you can add the "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" claimtype (which Umbraco relies on)!
Steps to reproduce
Reproducing is kind of tricky because you will need a Identity provider. I used ID4 as my identity provider: https://identityserver4.readthedocs.io/en/latest/
- Do a clean Umbraco 10.0.1 install
- Use openid-connect to hook up with a identity provider and make sure the identity provider is NOT returning the "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" claim. The following article is a good starting point: https://www.scottbrady91.com/umbraco/backoffice-sso-openid-connect
- After a successfull login, notice that the backoffice-user is not getting created and that the Umbraco-UI is blank (with console errors being thrown):
Expected result / actual result
- I would expect the documentation to be clear about what claim-type is being used to retrieve the emailaddress from the external login when using autolinking.
- I would expect the documentation to be clear about overruling the used emailaddress claim-type.
There are multiple options for fixing/improving this issue but they are all debatable i guess:
Improvement 1: Maybe the "AutoLinkAndSignInExternalAccount" function in the "BackOfficeSignInManager" should ALSO try to fetch the "email" claim type:
I think the "email" claim-type is more and more getting the default claim-type so it would be a improvement to also check for the claim-type.
Improvement 2: Maybe add a "EmailClaimType" to the "ExternalSignInAutoLinkOptions" so the developer specify the claim-type to be used for fetching the emailaddres:
But agiain, this can also be achieved by using the "IClaimsTransformation" but that would need more configuration.
Hi there @Domitnator!
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'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
- If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
- We'll replicate the issue to ensure that the problem is as described.
- We'll decide whether the behavior is an issue or if the behavior is intended.
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:
BTW: When we all agree on a solution i will be happy to create a pull request myself!
Thank you @Domitnator for the extensive research!! 👍
I'm going to have to send this to the team at HQ for discussion as I don't have any experience with this, so can't say anything conclusive about it. It might take a while to get back to you though, so I'll ask for your patience to start with. 😄 As such, as you suggest, I'd indeed advise not to start working on PRs yet until we have a more conclusive direction that we'd want to go in.
Hi @Domitnator
Thanks for the suggestions.. I fear it will be complicated not to use the ClaimTypes.Email
in a good way, and it would also have to be reflected for Members.
Did you try to map your email claim to the ClaimTypes.Email
?
The OpenIdConnectOptions
and most other (Like google) have an ClaimActions.MapJsonKey
where you can map it
options.ClaimActions.MapJsonKey(claimType: ClaimTypes.Email, jsonKey: "email");
That seems like a simpler solution to me, but let me know if I miss something.
@bergmania Thanks for you reply!
Mapping the claim types, the way you suggested, seems to work too!
options.ClaimActions.MapJsonKey(claimType: ClaimTypes.Email, jsonKey: "email");
I also tried mapping the claimtypes but I used a different way (unfortunately i don't recall which way i tried to map the claims exactly) but that did not seem to work. I agree that your solution is more simpler (and therefor preferable). Altough the IClaimsTransformation-option can also be handy for debugging purposes.
Maybe the umbraco documentation should point out the documentation-page about claim-mappings: https://docs.microsoft.com/en-us/aspnet/core/security/authentication/claims?view=aspnetcore-6.0 and offcourse state that by default the ClaimTypes.Email is used.
I'll be happy to update the documentation with this information and then we could close this issue!
@Domitnator, good to hear. It would be amazing if your will update the docs 💪
@bergmania Today a collegue of mine stept into the same issue, so i decided to create a new pull request. I've added just a simple fallback but it think this will be very convenient. In 99% of the cases it will be the ClaimTypes.Email claimtype or either the "email" claimtype. To keep it consistent i changed it for the backoffice users AND for the members!
I've got limited time right now but i will update the docs later this week!
Hi @Domitnator Did you have any solution to solve this issue? I'm having the same problem with auto-link with identityServer4 ( same with your Steps to reproduce ) . It seems like auto-linking didn't create user correctly(null) but let that user logged in :(
Hmm , the Auto-linking created User but immediately disable it.
After setting autoLinkUser.IsApproved = true
in OnAutoLinking
function and check (email and name) claim_type between Identity Provider and Umbraco correcly , it worked perfectly.