From 807c6a93fcad0b5056289773308295929b4aa0dd Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Mon, 17 Jan 2022 12:28:06 +0100 Subject: [PATCH] Feature/improve profile page (#828) * * Better design * Open profile in normal link. * Fix github email scope. * No Email merging. * Remove unused directly and fix a second link to the profile. --- backend/i18n/source/backend_en.json | 3 +- backend/src/Squidex.Shared/Texts.it.resx | 3 + backend/src/Squidex.Shared/Texts.nl.resx | 3 + backend/src/Squidex.Shared/Texts.resx | 5 +- backend/src/Squidex.Shared/Texts.zh.resx | 3 + .../Controllers/Account/AccountController.cs | 69 +++++-------------- .../IdentityServer/Controllers/Extensions.cs | 10 +-- .../Controllers/Profile/ProfileController.cs | 48 +++++++------ .../Views/Profile/Profile.cshtml | 18 +++-- .../IdentityServer/Views/Setup/Setup.cshtml | 2 +- .../GithubAuthenticationServices.cs | 1 + .../wwwroot/scripts/editor-references.html | 2 +- .../angular/external-link.directive.ts | 2 +- .../framework/angular/popup-link.directive.ts | 23 ------- frontend/src/app/framework/declarations.ts | 1 - frontend/src/app/framework/module.ts | 4 +- .../internal/profile-menu.component.html | 4 +- frontend/src/app/theme/_static.scss | 37 +++++++++- 18 files changed, 119 insertions(+), 119 deletions(-) delete mode 100644 frontend/src/app/framework/angular/popup-link.directive.ts diff --git a/backend/i18n/source/backend_en.json b/backend/i18n/source/backend_en.json index 21d2689df..732e56135 100644 --- a/backend/i18n/source/backend_en.json +++ b/backend/i18n/source/backend_en.json @@ -190,7 +190,7 @@ "contents.workflowErrorUpdate": "The workflow does not allow updates at status {status}", "dotnet_identity_DefaultEror": "An unknown failure has occurred.", "dotnet_identity_DuplicateEmail": "Email is already taken.", - "dotnet_identity_DuplicateUserName": "User name is already taken.", + "dotnet_identity_DuplicateUserName": "Email is already taken by another user. You can add this external login to your account if you go to the profile page.", "dotnet_identity_InvalidEmail": "Email is invalid.", "dotnet_identity_InvalidUserName": "User name '{0}' is invalid, can only contain letters or digits.", "dotnet_identity_LoginAlreadyAssociated": "A user with this login already exists.", @@ -250,6 +250,7 @@ "history.schemas.updated": "updated schema {[Name]}.", "history.statusChanged": "changed status of {[Schema]} content to {[Status]}.", "login.githubPrivateEmail": "Your email address is set to private in Github. Please set it to public to use Github login.", + "login.noEmailAddress": "We cannot get the email address from authentication provider.", "rules.ruleAlreadyRunning": "Another rule is already running.", "schemas.dateTimeCalculatedDefaultAndDefaultError": "Calculated default value and default value cannot be used together.", "schemas.duplicateFieldName": "Field '{field}' has been added twice.", diff --git a/backend/src/Squidex.Shared/Texts.it.resx b/backend/src/Squidex.Shared/Texts.it.resx index 82288bb53..62a716535 100644 --- a/backend/src/Squidex.Shared/Texts.it.resx +++ b/backend/src/Squidex.Shared/Texts.it.resx @@ -835,6 +835,9 @@ Il tuo indirizzo email è impostato su privato in Github. Impostalo come pubblico per poter utilizzare il login Github. + + We cannot get the email address from authentication provider. + È in esecuzione un'altra regola. diff --git a/backend/src/Squidex.Shared/Texts.nl.resx b/backend/src/Squidex.Shared/Texts.nl.resx index d2190ac4a..c43c14e66 100644 --- a/backend/src/Squidex.Shared/Texts.nl.resx +++ b/backend/src/Squidex.Shared/Texts.nl.resx @@ -835,6 +835,9 @@ Jouw e-mailadres is ingesteld op privé in Github. Stel het in op openbaar om Github-login te gebruiken. + + We cannot get the email address from authentication provider. + Er wordt al een andere regel uitgevoerd. diff --git a/backend/src/Squidex.Shared/Texts.resx b/backend/src/Squidex.Shared/Texts.resx index 7b77959b7..61f552480 100644 --- a/backend/src/Squidex.Shared/Texts.resx +++ b/backend/src/Squidex.Shared/Texts.resx @@ -656,7 +656,7 @@ Email is already taken. - User name is already taken. + Email is already taken by another user. You can add this external login to your account if you go to the profile page. Email is invalid. @@ -835,6 +835,9 @@ Your email address is set to private in Github. Please set it to public to use Github login. + + We cannot get the email address from authentication provider. + Another rule is already running. diff --git a/backend/src/Squidex.Shared/Texts.zh.resx b/backend/src/Squidex.Shared/Texts.zh.resx index 2e9fe5a0a..1869f726f 100644 --- a/backend/src/Squidex.Shared/Texts.zh.resx +++ b/backend/src/Squidex.Shared/Texts.zh.resx @@ -835,6 +835,9 @@ 您的邮箱在 Github 中设置为私有。请设置为公开以使用 Github 登录。 + + We cannot get the email address from authentication provider. + 另一个规则已经在运行。 diff --git a/backend/src/Squidex/Areas/IdentityServer/Controllers/Account/AccountController.cs b/backend/src/Squidex/Areas/IdentityServer/Controllers/Account/AccountController.cs index 3f4f73847..cdb1ff43c 100644 --- a/backend/src/Squidex/Areas/IdentityServer/Controllers/Account/AccountController.cs +++ b/backend/src/Squidex/Areas/IdentityServer/Controllers/Account/AccountController.cs @@ -181,11 +181,10 @@ namespace Squidex.Areas.IdentityServer.Controllers.Account { var provider = externalProviders[0].AuthenticationScheme; - var properties = - SignInManager.ConfigureExternalAuthenticationProperties(provider, - Url.Action(nameof(ExternalCallback), new { ReturnUrl = returnUrl })); + var challengeRedirectUrl = Url.Action(nameof(ExternalCallback)); + var challengeProperties = SignInManager.ConfigureExternalAuthenticationProperties(provider, challengeRedirectUrl); - return Challenge(properties, provider); + return Challenge(challengeProperties, provider); } var vm = new LoginVM @@ -204,25 +203,24 @@ namespace Squidex.Areas.IdentityServer.Controllers.Account [Route("account/external/")] public IActionResult External(string provider, string? returnUrl = null) { - var properties = - SignInManager.ConfigureExternalAuthenticationProperties(provider, - Url.Action(nameof(ExternalCallback), new { ReturnUrl = returnUrl })); + var challengeRedirectUrl = Url.Action(nameof(ExternalCallback), new { returnUrl }); + var challengeProperties = SignInManager.ConfigureExternalAuthenticationProperties(provider, challengeRedirectUrl); - return Challenge(properties, provider); + return Challenge(challengeProperties, provider); } [HttpGet] [Route("account/external-callback/")] public async Task ExternalCallback(string? returnUrl = null) { - var externalLogin = await SignInManager.GetExternalLoginInfoWithDisplayNameAsync(); + var login = await SignInManager.GetExternalLoginInfoWithDisplayNameAsync(); - if (externalLogin == null) + if (login == null) { return RedirectToAction(nameof(Login)); } - var result = await SignInManager.ExternalLoginSignInAsync(externalLogin.LoginProvider, externalLogin.ProviderKey, true); + var result = await SignInManager.ExternalLoginSignInAsync(login.LoginProvider, login.ProviderKey, true); if (!result.Succeeded && result.IsLockedOut) { @@ -235,42 +233,33 @@ namespace Squidex.Areas.IdentityServer.Controllers.Account if (isLoggedIn) { - user = await userService.FindByLoginAsync(externalLogin.LoginProvider, externalLogin.ProviderKey, HttpContext.RequestAborted); + user = await userService.FindByLoginAsync(login.LoginProvider, login.ProviderKey, HttpContext.RequestAborted); } else { - var email = externalLogin.Principal.GetEmail(); + var email = login.Principal.GetEmail(); if (string.IsNullOrWhiteSpace(email)) { - throw new DomainException("User has no exposed email address."); + throw new DomainException(T.Get("login.noEmailAddress")); } - user = await userService.FindByEmailAsync(email, HttpContext.RequestAborted); - - if (user != null) - { - var update = CreateUserValues(externalLogin, email, user); - - await userService.UpdateAsync(user.Id, update, ct: HttpContext.RequestAborted); - } - else + var values = new UserValues { - var update = CreateUserValues(externalLogin, email); + CustomClaims = login.Principal.Claims.GetSquidexClaims().ToList() + }; - user = await userService.CreateAsync(email, update, identityOptions.LockAutomatically, HttpContext.RequestAborted); - } + user = await userService.CreateAsync(email, values, identityOptions.LockAutomatically, HttpContext.RequestAborted); - await userService.AddLoginAsync(user.Id, externalLogin, HttpContext.RequestAborted); + await userService.AddLoginAsync(user.Id, login, + HttpContext.RequestAborted); - var (success, locked) = await LoginAsync(externalLogin); + (isLoggedIn, var locked) = await LoginAsync(login); if (locked) { return View(nameof(LockedOut)); } - - isLoggedIn = success; } if (!isLoggedIn) @@ -287,26 +276,6 @@ namespace Squidex.Areas.IdentityServer.Controllers.Account } } - private static UserValues CreateUserValues(ExternalLoginInfo externalLogin, string email, IUser? user = null) - { - var values = new UserValues - { - CustomClaims = externalLogin.Principal.Claims.GetSquidexClaims().ToList() - }; - - if (user != null && !user.Claims.HasPictureUrl()) - { - values.PictureUrl = GravatarHelper.CreatePictureUrl(email); - } - - if (user != null && !user.Claims.HasDisplayName()) - { - values.DisplayName = email; - } - - return values; - } - private async Task<(bool Success, bool Locked)> LoginAsync(UserLoginInfo externalLogin) { var result = await SignInManager.ExternalLoginSignInAsync(externalLogin.LoginProvider, externalLogin.ProviderKey, true); diff --git a/backend/src/Squidex/Areas/IdentityServer/Controllers/Extensions.cs b/backend/src/Squidex/Areas/IdentityServer/Controllers/Extensions.cs index fa883e815..c8ed1a3e0 100644 --- a/backend/src/Squidex/Areas/IdentityServer/Controllers/Extensions.cs +++ b/backend/src/Squidex/Areas/IdentityServer/Controllers/Extensions.cs @@ -16,23 +16,23 @@ namespace Squidex.Areas.IdentityServer.Controllers { public static async Task GetExternalLoginInfoWithDisplayNameAsync(this SignInManager signInManager, string? expectedXsrf = null) { - var externalLogin = await signInManager.GetExternalLoginInfoAsync(expectedXsrf); + var login = await signInManager.GetExternalLoginInfoAsync(expectedXsrf); - if (externalLogin == null) + if (login == null) { throw new InvalidOperationException("Request from external provider cannot be handled."); } - var email = externalLogin.Principal.GetEmail(); + var email = login.Principal.GetEmail(); if (string.IsNullOrWhiteSpace(email)) { throw new InvalidOperationException("External provider does not provide email claim."); } - externalLogin.ProviderDisplayName = email; + login.ProviderDisplayName = email; - return externalLogin; + return login; } public static async Task> GetExternalProvidersAsync(this SignInManager signInManager) diff --git a/backend/src/Squidex/Areas/IdentityServer/Controllers/Profile/ProfileController.cs b/backend/src/Squidex/Areas/IdentityServer/Controllers/Profile/ProfileController.cs index e627fbd11..8c459538f 100644 --- a/backend/src/Squidex/Areas/IdentityServer/Controllers/Profile/ProfileController.cs +++ b/backend/src/Squidex/Areas/IdentityServer/Controllers/Profile/ProfileController.cs @@ -59,18 +59,19 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile { await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme); - var properties = - SignInManager.ConfigureExternalAuthenticationProperties(provider, - Url.Action(nameof(AddLoginCallback)), userService.GetUserId(User, HttpContext.RequestAborted)); + var userId = userService.GetUserId(User, HttpContext.RequestAborted); - return Challenge(properties, provider); + var challengeRedirectUrl = Url.Action(nameof(AddLoginCallback)); + var challengeProperties = SignInManager.ConfigureExternalAuthenticationProperties(provider, userId); + + return Challenge(challengeProperties, provider); } [HttpGet] [Route("/account/profile/login-add-callback/")] public Task AddLoginCallback() { - return MakeChangeAsync(u => AddLoginAsync(u), + return MakeChangeAsync((id, ct) => AddLoginAsync(id, ct), T.Get("users.profile.addLoginDone"), None.Value); } @@ -78,7 +79,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/update/")] public Task UpdateProfile(ChangeProfileModel model) { - return MakeChangeAsync(id => userService.UpdateAsync(id, model.ToValues(), ct: HttpContext.RequestAborted), + return MakeChangeAsync((id, ct) => userService.UpdateAsync(id, model.ToValues(), ct: ct), T.Get("users.profile.updateProfileDone"), model); } @@ -86,7 +87,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/properties/")] public Task UpdateProperties(ChangePropertiesModel model) { - return MakeChangeAsync(id => userService.UpdateAsync(id, model.ToValues(), ct: HttpContext.RequestAborted), + return MakeChangeAsync((id, ct) => userService.UpdateAsync(id, model.ToValues(), ct: ct), T.Get("users.profile.updatePropertiesDone"), model); } @@ -94,7 +95,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/login-remove/")] public Task RemoveLogin(RemoveLoginModel model) { - return MakeChangeAsync(id => userService.RemoveLoginAsync(id, model.LoginProvider, model.ProviderKey, HttpContext.RequestAborted), + return MakeChangeAsync((id, ct) => userService.RemoveLoginAsync(id, model.LoginProvider, model.ProviderKey, ct), T.Get("users.profile.removeLoginDone"), model); } @@ -102,7 +103,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/password-set/")] public Task SetPassword(SetPasswordModel model) { - return MakeChangeAsync(id => userService.SetPasswordAsync(id, model.Password, ct: HttpContext.RequestAborted), + return MakeChangeAsync((id, ct) => userService.SetPasswordAsync(id, model.Password, ct: ct), T.Get("users.profile.setPasswordDone"), model); } @@ -110,7 +111,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/password-change/")] public Task ChangePassword(ChangePasswordModel model) { - return MakeChangeAsync(id => userService.SetPasswordAsync(id, model.Password, model.OldPassword, HttpContext.RequestAborted), + return MakeChangeAsync((id, ct) => userService.SetPasswordAsync(id, model.Password, model.OldPassword, ct), T.Get("users.profile.changePasswordDone"), model); } @@ -118,7 +119,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/generate-client-secret/")] public Task GenerateClientSecret() { - return MakeChangeAsync(id => GenerateClientSecretAsync(id), + return MakeChangeAsync((id, ct) => GenerateClientSecretAsync(id, ct), T.Get("users.profile.generateClientDone"), None.Value); } @@ -126,39 +127,42 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile [Route("/account/profile/upload-picture/")] public Task UploadPicture(List file) { - return MakeChangeAsync(user => UpdatePictureAsync(file, user), + return MakeChangeAsync((id, ct) => UpdatePictureAsync(file, id, ct), T.Get("users.profile.uploadPictureDone"), None.Value); } - private async Task GenerateClientSecretAsync(string id) + private async Task GenerateClientSecretAsync(string id, + CancellationToken ct) { var update = new UserValues { ClientSecret = RandomHash.New() }; - await userService.UpdateAsync(id, update, ct: HttpContext.RequestAborted); + await userService.UpdateAsync(id, update, ct: ct); } - private async Task AddLoginAsync(string id) + private async Task AddLoginAsync(string id, + CancellationToken ct) { - var externalLogin = await SignInManager.GetExternalLoginInfoWithDisplayNameAsync(id); + var login = await SignInManager.GetExternalLoginInfoWithDisplayNameAsync(id); - await userService.AddLoginAsync(id, externalLogin, HttpContext.RequestAborted); + await userService.AddLoginAsync(id, login, ct); } - private async Task UpdatePictureAsync(List files, string id) + private async Task UpdatePictureAsync(List files, string id, + CancellationToken ct) { if (files.Count != 1) { throw new ValidationException(T.Get("validation.onlyOneFile")); } - await UploadResizedAsync(files[0], id, HttpContext.RequestAborted); + await UploadResizedAsync(files[0], id, ct); var update = new UserValues { PictureUrl = SquidexClaimTypes.PictureUrlStore }; - await userService.UpdateAsync(id, update, ct: HttpContext.RequestAborted); + await userService.UpdateAsync(id, update, ct: ct); } private async Task UploadResizedAsync(IFormFile file, string id, @@ -193,7 +197,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile } } - private async Task MakeChangeAsync(Func action, string successMessage, TModel? model = null) where TModel : class + private async Task MakeChangeAsync(Func action, string successMessage, TModel? model = null) where TModel : class { var user = await userService.GetAsync(User, HttpContext.RequestAborted); @@ -210,7 +214,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Profile string errorMessage; try { - await action(user.Id); + await action(user.Id, HttpContext.RequestAborted); await SignInManager.SignInAsync((IdentityUser)user.Identity, true); diff --git a/backend/src/Squidex/Areas/IdentityServer/Views/Profile/Profile.cshtml b/backend/src/Squidex/Areas/IdentityServer/Views/Profile/Profile.cshtml index b427199a0..5a4c3a478 100644 --- a/backend/src/Squidex/Areas/IdentityServer/Views/Profile/Profile.cshtml +++ b/backend/src/Squidex/Areas/IdentityServer/Views/Profile/Profile.cshtml @@ -8,7 +8,7 @@ @if (ViewContext.ViewData.ModelState[field]?.ValidationState == Microsoft.AspNetCore.Mvc.ModelBinding.ModelValidationState.Invalid) {
- Html.ValidationMessage(field) + @Html.ValidationMessage(field)
} } @@ -77,6 +77,8 @@ @if (Model!.ExternalProviders.Any()) { +
+

@T.Get("users.profile.loginsTitle")

@@ -127,6 +129,8 @@ @if (Model!.HasPasswordAuth) { +
+

@T.Get("users.profile.passwordTitle")

@@ -189,19 +193,21 @@
} +
+

@T.Get("users.profile.clientTitle")

@T.Get("users.profile.clientHint") -
+
-
+
@@ -217,6 +223,8 @@
+
+

@T.Get("users.profile.propertiesTitle")

@@ -226,7 +234,7 @@
@for (var i = 0; i < Model!.Properties.Count; i++) { -
+
@{ RenderValidation($"Properties[{i}].Name"); } @@ -299,7 +307,7 @@ var template = document.createElement('template'); template.innerHTML = - `
+ `
diff --git a/backend/src/Squidex/Areas/IdentityServer/Views/Setup/Setup.cshtml b/backend/src/Squidex/Areas/IdentityServer/Views/Setup/Setup.cshtml index 5f8616c15..26d52c196 100644 --- a/backend/src/Squidex/Areas/IdentityServer/Views/Setup/Setup.cshtml +++ b/backend/src/Squidex/Areas/IdentityServer/Views/Setup/Setup.cshtml @@ -8,7 +8,7 @@ @if (ViewContext.ViewData.ModelState[field]?.ValidationState == Microsoft.AspNetCore.Mvc.ModelBinding.ModelValidationState.Invalid) {
- Html.ValidationMessage(field) + @Html.ValidationMessage(field)
} } diff --git a/backend/src/Squidex/Config/Authentication/GithubAuthenticationServices.cs b/backend/src/Squidex/Config/Authentication/GithubAuthenticationServices.cs index 8616c2698..d76f7150c 100644 --- a/backend/src/Squidex/Config/Authentication/GithubAuthenticationServices.cs +++ b/backend/src/Squidex/Config/Authentication/GithubAuthenticationServices.cs @@ -20,6 +20,7 @@ namespace Squidex.Config.Authentication options.ClientId = identityOptions.GithubClient; options.ClientSecret = identityOptions.GithubSecret; options.Events = new GithubHandler(); + options.Scope.Add("user:email"); }); } diff --git a/backend/src/Squidex/wwwroot/scripts/editor-references.html b/backend/src/Squidex/wwwroot/scripts/editor-references.html index d9ef2560c..4d4e17b33 100644 --- a/backend/src/Squidex/wwwroot/scripts/editor-references.html +++ b/backend/src/Squidex/wwwroot/scripts/editor-references.html @@ -19,7 +19,7 @@