Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrchardCore.Users - a service to decide which kind of parameter use to link external login to existing account in the second check #16026

Closed
PiemP opened this issue May 10, 2024 Discussed in #16023 · 4 comments · Fixed by #16110

Comments

@PiemP
Copy link
Contributor

PiemP commented May 10, 2024

Discussed in #16023

Each module that add an extenal login provider (like GitHub or OpenID) can specify the value of ClaimTypes.NameIdentifier to link the external login information to an existing OrchardCore account.

But in this line of code, OrchardCore force another check over external login data that could redirect a user to a view that ask about link external login data to an existing account by checking the email value returned by the login provider.

var email = info.Principal.FindFirstValue(ClaimTypes.Email) ?? info.Principal.FindFirstValue("email");
if (!string.IsNullOrWhiteSpace(email))
{
iUser = await _userManager.FindByEmailAsync(email);
}
ViewData["ReturnUrl"] = returnUrl;
ViewData["LoginProvider"] = info.LoginProvider;
if (iUser != null)
{
if (iUser is User userToLink && registrationSettings.UsersMustValidateEmail && !userToLink.EmailConfirmed)
{
return RedirectToAction(nameof(RegistrationController.ConfirmEmailSent),
new
{
Area = "OrchardCore.Users",
Controller = typeof(RegistrationController).ControllerName(),
ReturnUrl = returnUrl,
});
}
// Link external login to an existing user
ViewData["UserName"] = iUser.UserName;
ViewData["Email"] = email;
return View(nameof(LinkExternalLogin));
}

I would like each module that add an external login can manage this behavior by a dedicated service. If a module doesn't implement this service: the actual behavior is applied.

Probably could be useful to have a settings to disable this code.

I'm not sure if this "second check" over external login data is correct or it's nature: seems a second chance to connect external login data to user account.

@sebastienros sebastienros added this to the 2.x milestone May 16, 2024
@Piedone
Copy link
Member

Piedone commented May 16, 2024

Would you like to venture into contributing this?

@PiemP
Copy link
Contributor Author

PiemP commented May 17, 2024

Would you like to venture into contributing this?

Yes... this week I have some issue at home but next week I try to build something that, I hope, are acceptable.

@Piedone
Copy link
Member

Piedone commented May 17, 2024

Thank you!

PiemP added a commit to PiemP/OrchardCore that referenced this issue May 20, 2024
… data

feat(Users): changed account controller code to use new services
feat(Users): load all services in reversed enum to ret last registered services
feat(Users): updated view code

resolve OrchardCMS#16026
@Piedone
Copy link
Member

Piedone commented Jun 14, 2024

See #16110 (comment) for a description of the use case of this issue in detail.

@MikeAlhayek MikeAlhayek modified the milestones: 2.x, 2.0 Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants