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

Implement a default service to link local user accounts with external ones during the registration process #16110

Merged
merged 43 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
aa734f1
feat(Users): added default services to retrieve user to link to login…
PiemP May 20, 2024
c387004
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP May 20, 2024
2ee51cd
fix: avoid null excpetion in AccountController
PiemP May 22, 2024
a12d557
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP May 22, 2024
0ce3a96
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP Jun 10, 2024
bd01473
style: name changes, docs, indent, new line
PiemP Jun 10, 2024
ed96f40
cleanup before comments
MikeAlhayek Jun 10, 2024
dfd2dd6
Merge branch 'main' into srvcs_acct_to_login_data
MikeAlhayek Jun 10, 2024
6d74eaf
style: from LinkParameterValue to ExternalUserIdentifier
PiemP Jun 10, 2024
30188bf
style: from LinkParameterValue to ExternalUserIdentifier
PiemP Jun 10, 2024
ed52a4c
style: from LinkParameterValue to ExternalUserIdentifier
PiemP Jun 10, 2024
4726652
style: rename method, comment interface
PiemP Jun 10, 2024
c8898a4
style: improved comments
PiemP Jun 10, 2024
e04b9aa
style: removed grammar errors in comments
PiemP Jun 10, 2024
1ce6665
Update src/OrchardCore.Modules/OrchardCore.Users/Views/Account/LinkEx…
MikeAlhayek Jun 10, 2024
5115dd3
Update src/OrchardCore/OrchardCore.Users.Abstractions/Services/IUserT…
MikeAlhayek Jun 10, 2024
604fd35
Update src/OrchardCore/OrchardCore.Users.Abstractions/Services/IUserT…
MikeAlhayek Jun 10, 2024
8179182
Update src/OrchardCore/OrchardCore.Users.Abstractions/Services/IUserT…
MikeAlhayek Jun 10, 2024
f210886
Update src/OrchardCore/OrchardCore.Users.Abstractions/Services/IUserT…
MikeAlhayek Jun 10, 2024
31967ed
cleanup
MikeAlhayek Jun 10, 2024
06e963f
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP Jun 11, 2024
010de02
fix: var name in account controller
PiemP Jun 11, 2024
c9914fe
fix: comments in IUserToExternaLoginProvider
PiemP Jun 11, 2024
12e0597
updated comment in IUserToExternalLoginProvider
PiemP Jun 12, 2024
3255f3d
simplified code in DefaultUserToExternalLoginProvider
PiemP Jun 12, 2024
a330c50
updated class comment IUserToExternalLoginProvider
PiemP Jun 12, 2024
7e50a95
style: code to new line in ExternalLoginInfoExtensions
PiemP Jun 12, 2024
139ca99
docs: updated comment in the AccountController about order
PiemP Jun 12, 2024
c28fbca
style: brackets on a new line in AccountController
PiemP Jun 12, 2024
c3f4bbe
style(Users.Abstractions): change name to interface
PiemP Jun 12, 2024
5fd181a
style: from GetUserAsync to FindByLoginAsync
PiemP Jun 12, 2024
ec6a034
Merge branch 'main' into srvcs_acct_to_login_data
PiemP Jun 13, 2024
63e26ca
Merge branch 'main' into srvcs_acct_to_login_data
PiemP Jun 13, 2024
5c378b8
fix: removed IExternalLoginMapper
PiemP Jun 15, 2024
f2faf17
fix: improve if declaration
PiemP Jun 15, 2024
a0ce348
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP Jun 15, 2024
807ca2d
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP Jun 17, 2024
da86ae6
Update AccountController.cs
MikeAlhayek Jun 17, 2024
6459e5d
Merge branch 'main' into srvcs_acct_to_login_data
PiemP Jun 18, 2024
d3e1010
Update src/OrchardCore.Modules/OrchardCore.Users/Controllers/AccountC…
PiemP Jun 18, 2024
3bfcd28
Merge remote-tracking branch 'upstream/main' into srvcs_acct_to_login…
PiemP Jun 18, 2024
a6ec2b9
Merge branch 'main' into srvcs_acct_to_login_data
hishamco Jun 19, 2024
7be7530
Merge branch 'main' into srvcs_acct_to_login_data
hishamco Jun 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class AccountController : AccountBaseController
private readonly IClock _clock;
private readonly IDistributedCache _distributedCache;
private readonly IEnumerable<IExternalLoginEventHandler> _externalLoginHandlers;
private readonly IEnumerable<IUserToExternalLoginProvider> _externalLoginUserLocator;
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved

private static readonly JsonMergeSettings _jsonMergeSettings = new()
{
Expand All @@ -78,7 +79,8 @@ public AccountController(
IShellFeaturesManager shellFeaturesManager,
IDisplayManager<LoginForm> loginFormDisplayManager,
IUpdateModelAccessor updateModelAccessor,
IEnumerable<IExternalLoginEventHandler> externalLoginHandlers)
IEnumerable<IExternalLoginEventHandler> externalLoginHandlers,
IEnumerable<IUserToExternalLoginProvider> externalLoginUserLocator)
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
{
_signInManager = signInManager;
_userManager = userManager;
Expand All @@ -94,6 +96,8 @@ public AccountController(
_loginFormDisplayManager = loginFormDisplayManager;
_updateModelAccessor = updateModelAccessor;
_externalLoginHandlers = externalLoginHandlers;
// Reverse the order of services to prioritize external services first, placing them before the default implementation.
PiemP marked this conversation as resolved.
Show resolved Hide resolved
_externalLoginUserLocator = externalLoginUserLocator.Reverse();

H = htmlLocalizer;
S = stringLocalizer;
Expand Down Expand Up @@ -296,7 +300,6 @@ public async Task<IActionResult> ChangePassword(ChangePasswordViewModel model, s
public IActionResult ChangePasswordConfirmation()
=> View();


[HttpPost]
[AllowAnonymous]
[ValidateAntiForgeryToken]
Expand All @@ -316,7 +319,7 @@ private async Task<SignInResult> ExternalLoginSignInAsync(IUser user, ExternalLo
var userInfo = user as User;

var context = new UpdateUserContext(user, info.LoginProvider, externalClaims, userInfo.Properties)
{
{
UserClaims = userInfo.UserClaims,
UserRoles = userRoles,
};
Expand Down Expand Up @@ -402,11 +405,9 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,
}
else
{
var email = info.Principal.FindFirstValue(ClaimTypes.Email) ?? info.Principal.FindFirstValue("email");

if (!string.IsNullOrWhiteSpace(email))
{
iUser = await _userManager.FindByEmailAsync(email);
var userLocator = _externalLoginUserLocator.Where(x => x.CanHandle(info)).FirstOrDefault();
if (userLocator != null) {
iUser = await userLocator.GetUserAsync(info);
PiemP marked this conversation as resolved.
Show resolved Hide resolved
}

ViewData["ReturnUrl"] = returnUrl;
Expand All @@ -427,7 +428,14 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,

// Link external login to an existing user
ViewData["UserName"] = iUser.UserName;
ViewData["Email"] = email;
if (userLocator != null)
{
ViewData["ExternalUserIdentifier"] = userLocator.GetIdentifierKey(info);
}
else
{
ViewData["ExternalUserIdentifier"] = info.GetEmail();
}

return View(nameof(LinkExternalLogin));
}
Expand All @@ -449,7 +457,7 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null,

// If registrationSettings.NoUsernameForExternalUsers is true, this username will not be used
UserName = await GenerateUsernameAsync(info),
Email = email
Email = info.GetEmail(),
};

// The user doesn't exist and no information required, we can create the account locally
Expand Down Expand Up @@ -648,9 +656,9 @@ public async Task<IActionResult> LinkExternalLogin(LinkExternalLoginViewModel mo

return NotFound();
}
var email = info.Principal.FindFirstValue(ClaimTypes.Email) ?? info.Principal.FindFirstValue("email");

var user = await _userManager.FindByEmailAsync(email);
var userLocator = _externalLoginUserLocator.Where(x => x.CanHandle(info)).FirstOrDefault();
var user = await userLocator?.GetUserAsync(info);

if (user == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<form asp-controller="Account" asp-action="LinkExternalLogin" asp-route-returnurl="@ViewData["ReturnUrl"]" method="post" class="form-horizontal no-multisubmit">
<h4>@T["Link your account."]</h4>
<p class="text-info">
@T["You've successfully authenticated with <strong>{0}</strong>. You already have an account with this email address. Enter your local account password and click the Register button to link the accounts and finish logging in.", ViewData["LoginProvider"]]
@T["You've successfully authenticated with <strong>{0}</strong>. You already have an account associated with <strong>{1}</strong>. Enter your local account password and click the Register button to link the accounts and finish logging in.", ViewData["LoginProvider"], ViewData["ExternalUserIdentifier"]]
PiemP marked this conversation as resolved.
Show resolved Hide resolved
</p>
<hr />
<div asp-validation-summary="ModelOnly"></div>
Expand All @@ -20,9 +20,9 @@
</div>

<div class="mb-3">
<label for="Email" class="col-md-3 form-label">@T["Email"]</label>
<label for="ExternalUserIdentifier" class="col-md-3 form-label">@T["The parameter that identify the existing account"]</label>
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
<div class="col-md-9">
<input id="Email" value="@ViewData["Email"]" class="form-control" disabled />
<input id="ExternalUserIdentifier" value="@ViewData["ExternalUserIdentifier"]" class="form-control" disabled />
</div>
</div>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Identity;

namespace OrchardCore.Users;

/// <summary>
/// Interface <c>IUserToExternalLoginProvider</c> allow to create services used to decide
/// when link an existing local user account to the login informations that comes from an external
/// login system like OpenId or GitHub.
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public interface IUserToExternalLoginProvider
PiemP marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Method <c>CanHandle</c> establish if the service that implement this interface can handle
/// this kind of external login.
/// </summary>
/// <param name="info">
/// external login information with type and other data to establish if
/// service can handle this kind of external login.
/// </param>
/// <returns>
/// True, if the service can handle the external login by the informations in the parameter. False, instead.
/// </returns>
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
bool CanHandle(ExternalLoginInfo info);

/// <summary>
/// Method <c>GetUserAsync</c> return a local user account that match the login data in the parameter.
/// </summary>
/// <param name="info">
/// external login information, used to establish if exists a local user account that match login data.
/// </param>
/// <returns>
/// an object that implement <c>IUser</c>, if the exist a local account that match the external login data. Null, instead.
/// </returns>
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
Task<IUser> GetUserAsync(ExternalLoginInfo info);

/// <summary>
/// Method <c>GetIdentifierKey</c> return the value of the login data used by the service to
/// match a local user account to the external login informations.
/// </summary>
/// <param name="info">
/// external login informations.
/// </param>
/// <returns>
/// a <c>string</c> that is used like an identifier to match an existing local user account.
/// Used in the related views to render the value that cause the page display.
/// </returns>
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
string GetIdentifierKey(ExternalLoginInfo info);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System.Security.Claims;

namespace Microsoft.AspNetCore.Identity;

public static class ExternalLoginInfoExtensions
{
public static string GetEmail(this ExternalLoginInfo info)
=> info.Principal.FindFirstValue(ClaimTypes.Email)
?? info.Principal.FindFirstValue("email");
PiemP marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ public static IServiceCollection AddUsers(this IServiceCollection services)
services.TryAddScoped<IUserAuthenticationTokenStore<IUser>>(sp => sp.GetRequiredService<UserStore>());
services.TryAddScoped<IUserPhoneNumberStore<IUser>>(sp => sp.GetRequiredService<UserStore>());

services.AddScoped<IUserToExternalLoginProvider, DefaultUserToExternalLoginProvider>();

services.AddScoped<NullRoleStore>();
services.TryAddScoped<IRoleClaimStore<IRole>>(sp => sp.GetRequiredService<NullRoleStore>());
services.TryAddScoped<IRoleStore<IRole>>(sp => sp.GetRequiredService<NullRoleStore>());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Identity;

namespace OrchardCore.Users.Services;

public class DefaultUserToExternalLoginProvider : IUserToExternalLoginProvider
PiemP marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly UserManager<IUser> _userManager;

public DefaultUserToExternalLoginProvider(UserManager<IUser> userManager)
{
_userManager = userManager;
}

public bool CanHandle(ExternalLoginInfo info)
=> true;

public async Task<IUser> GetUserAsync(ExternalLoginInfo info)
{
var email = info.GetEmail();

if (!string.IsNullOrWhiteSpace(email))
{
return await _userManager.FindByEmailAsync(email);
}

return null;
PiemP marked this conversation as resolved.
Show resolved Hide resolved
}

public string GetIdentifierKey(ExternalLoginInfo info)
=> info.GetEmail();
}