From e610a5ef54c2a67146c1b6b58281af164da57d9e Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 13 Apr 2016 13:51:12 +0200 Subject: [PATCH] Changes the password reset link to be a real link (not an angular deep link), this means there is less logging of the reset code in a query string and less visibility of it, this also means that the validation of the code happens instantly. The premise for this is the same as how we deal with external authentication requests and uses ViewData/TempData with redirects. Fixes the models to have the correct attributes to be able to directly json serialize them. --- .../src/common/resources/auth.resource.js | 2 +- .../views/common/dialogs/login.controller.js | 33 +++++---- .../src/views/common/dialogs/login.html | 5 +- .../umbraco/Views/Default.cshtml | 4 +- .../Editors/AuthenticationController.cs | 51 ++++++-------- .../Editors/BackOfficeController.cs | 67 ++++++++++++++----- .../HtmlHelperBackOfficeExtensions.cs | 38 ++++++++++- .../Models/RequestPasswordResetModel.cs | 2 + src/Umbraco.Web/Models/SetPasswordModel.cs | 1 + .../Models/ValidatePasswordResetCodeModel.cs | 1 + src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 11 files changed, 132 insertions(+), 74 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js index 30585d326d..20ebaa10c0 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js @@ -170,7 +170,7 @@ function authResource($q, $http, umbRequestHelper, angularHelper) { */ performSetPassword: function (userId, password, confirmPassword, resetCode) { - if (!userId) { + if (userId === undefined || userId === null) { return angularHelper.rejectedPromise({ errorMsg: 'User Id cannot be empty' }); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.controller.js index ff9981eba1..1b1b3bb402 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.controller.js @@ -1,5 +1,5 @@ angular.module("umbraco").controller("Umbraco.Dialogs.LoginController", - function ($scope, $cookies, localizationService, userService, externalLoginInfo, $timeout, $location, authResource) { + function ($scope, $cookies, localizationService, userService, externalLoginInfo, resetPasswordCodeInfo, $timeout, authResource) { var setFieldFocus = function(form, field) { $timeout(function() { @@ -63,6 +63,7 @@ $scope.externalLoginFormAction = Umbraco.Sys.ServerVariables.umbracoUrls.externalLoginsUrl; $scope.externalLoginProviders = externalLoginInfo.providers; $scope.externalLoginInfo = externalLoginInfo; + $scope.resetPasswordCodeInfo = resetPasswordCodeInfo; $scope.activateKonamiMode = function () { if ($cookies.konamiLogin == "1") { @@ -77,21 +78,6 @@ } } - // Set initial view - either set password if reset code provided in querystring - // otherwise login form - var userId = $location.search().userId; - var resetCode = $location.search().resetCode; - if (userId && resetCode) { - authResource.performValidatePasswordResetCode(userId, resetCode) - .then(function () { - $scope.showSetPassword(); - }, function () { - $scope.view = "password-reset-code-expired"; - }); - } else { - $scope.showLogin(); - } - $scope.loginSubmit = function (login, password) { //if the login and password are not empty we need to automatically @@ -172,7 +158,7 @@ return; } - authResource.performSetPassword(userId, password, confirmPassword, resetCode) + authResource.performSetPassword($scope.resetPasswordCodeInfo.resetCodeModel.userId, password, confirmPassword, $scope.resetPasswordCodeInfo.resetCodeModel.resetCode) .then(function () { $scope.showSetPasswordConfirmation = true; $scope.resetComplete = true; @@ -199,4 +185,17 @@ }); } + + //Now, show the correct panel: + + if ($scope.resetPasswordCodeInfo.resetCodeModel) { + $scope.showSetPassword(); + } + else if ($scope.resetPasswordCodeInfo.errors.length > 0) { + $scope.view = "password-reset-code-expired"; + } + else { + $scope.showLogin(); + } + }); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.html b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.html index f97588cee0..12f25f42a5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/dialogs/login.html @@ -122,9 +122,10 @@
-
- The link you have clicked on is invalid or has expired. +
+ {{error}}
+
Return to login form
diff --git a/src/Umbraco.Web.UI/umbraco/Views/Default.cshtml b/src/Umbraco.Web.UI/umbraco/Views/Default.cshtml index a63fd3c670..07063934b3 100644 --- a/src/Umbraco.Web.UI/umbraco/Views/Default.cshtml +++ b/src/Umbraco.Web.UI/umbraco/Views/Default.cshtml @@ -72,8 +72,8 @@ diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 50deec6360..c03ff624a4 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -6,6 +6,7 @@ using System.Net.Http; using System.Threading.Tasks; using System.Web; using System.Web.Http; +using System.Web.Mvc; using AutoMapper; using Microsoft.AspNet.Identity; using Microsoft.AspNet.Identity.Owin; @@ -99,7 +100,7 @@ namespace Umbraco.Web.Editors /// Checks if the current user's cookie is valid and if so returns OK or a 400 (BadRequest) /// /// - [HttpGet] + [System.Web.Http.HttpGet] public bool IsAuthenticated() { var attempt = UmbracoContext.Security.AuthorizeRequest(); @@ -231,9 +232,6 @@ namespace Umbraco.Web.Editors { throw new HttpResponseException(HttpStatusCode.BadRequest); } - - var http = EnsureHttpContext(); - var identityUser = await SignInManager.UserManager.FindByEmailAsync(model.Email); if (identityUser != null) { @@ -241,7 +239,7 @@ namespace Umbraco.Web.Editors if (user != null && user.IsLockedOut == false) { var code = await UserManager.GeneratePasswordResetTokenAsync(identityUser.Id); - var callbackUrl = ConstuctCallbackUrl(http.Request.Url, identityUser.Id, code); + var callbackUrl = ConstuctCallbackUrl(identityUser.Id, code); var message = Services.TextService.Localize("resetPasswordEmailCopyFormat", new[] {identityUser.UserName, callbackUrl}); await UserManager.SendEmailAsync(identityUser.Id, Services.TextService.Localize("login/resetPasswordEmailCopySubject"), @@ -252,37 +250,28 @@ namespace Umbraco.Web.Editors return Request.CreateResponse(HttpStatusCode.OK); } - private static string ConstuctCallbackUrl(Uri url, int userId, string code) + private string ConstuctCallbackUrl(int userId, string code) { - return string.Format("{0}://{1}/umbraco/#/login?userId={2}&resetCode={3}", - url.Scheme, - url.Host + (url.Port == 80 ? string.Empty : ":" + url.Port), - userId, - HttpUtility.UrlEncode(code)); - } + //get an mvc helper to get the url + var http = EnsureHttpContext(); + var urlHelper = new UrlHelper(http.Request.RequestContext); - /// - /// Processes a password reset request. Looks for a match on the provided email address - /// and if found sends an email with a link to reset it - /// - /// - [SetAngularAntiForgeryTokens] - public async Task PostValidatePasswordResetCode(ValidatePasswordResetCodeModel model) - { - var user = UserManager.FindById(model.UserId); - if (user != null) - { - var result = await UserManager.UserTokenProvider.ValidateAsync("ResetPassword", - model.ResetCode, UserManager, user); - if (result) + var action = urlHelper.Action("ValidatePasswordResetCode", "BackOffice", + new { - return Request.CreateResponse(HttpStatusCode.OK); - } - } + area = GlobalSettings.UmbracoMvcArea, + u = userId, + r = code + }); - return Request.CreateValidationErrorResponse("Password reset code not valid"); - } + //TODO: Virtual path? + return string.Format("{0}://{1}{2}", + http.Request.Url.Scheme, + http.Request.Url.Host + (http.Request.Url.Port == 80 ? string.Empty : ":" + http.Request.Url.Port), + action); + } + /// /// Processes a set password request. Validates the request and sets a new password. /// diff --git a/src/Umbraco.Web/Editors/BackOfficeController.cs b/src/Umbraco.Web/Editors/BackOfficeController.cs index 297221dacc..b5cea7a945 100644 --- a/src/Umbraco.Web/Editors/BackOfficeController.cs +++ b/src/Umbraco.Web/Editors/BackOfficeController.cs @@ -4,6 +4,8 @@ using System.Configuration; using System.Globalization; using System.IO; using System.Linq; +using System.Net; +using System.Net.Http; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -26,6 +28,7 @@ using Umbraco.Core.Manifest; using Umbraco.Core.Models; using Umbraco.Core.Models.Identity; using Umbraco.Core.Security; +using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Mvc; using Umbraco.Web.PropertyEditors; @@ -34,6 +37,7 @@ using Umbraco.Web.Trees; using Umbraco.Web.UI.JavaScript; using Umbraco.Web.WebApi.Filters; using Umbraco.Web.WebServices; +using Umbraco.Core.Services; using Action = umbraco.BusinessLogic.Actions.Action; using Constants = Umbraco.Core.Constants; @@ -49,6 +53,10 @@ namespace Umbraco.Web.Editors private BackOfficeUserManager _userManager; private BackOfficeSignInManager _signInManager; + private const string TokenExternalSignInError = "ExternalSignInError"; + private const string TokenPasswordResetCode = "PasswordResetCode"; + private static readonly string[] TempDataTokenNames = { TokenExternalSignInError, TokenPasswordResetCode }; + protected BackOfficeSignInManager SignInManager { get { return _signInManager ?? (_signInManager = OwinContext.Get()); } @@ -431,7 +439,25 @@ namespace Umbraco.Web.Editors User.Identity.GetUserId()); } + [HttpGet] + public async Task ValidatePasswordResetCode([Bind(Prefix = "u")]int userId, [Bind(Prefix = "r")]string resetCode) + { + var user = UserManager.FindById(userId); + if (user != null) + { + var result = await UserManager.UserTokenProvider.ValidateAsync("ResetPassword", resetCode, UserManager, user); + if (result) + { + //Add a flag and redirect for it to be displayed + TempData[TokenPasswordResetCode] = new ValidatePasswordResetCodeModel {UserId = userId, ResetCode = resetCode}; + return RedirectToLocal(Url.Action("Default", "BackOffice")); + } + } + //Add error and redirect for it to be displayed + TempData[TokenPasswordResetCode] = new[] { Services.TextService.Localize("login/resetCodeExpired") }; + return RedirectToLocal(Url.Action("Default", "BackOffice")); + } [HttpGet] public async Task ExternalLinkLoginCallback() @@ -443,7 +469,7 @@ namespace Umbraco.Web.Editors if (loginInfo == null) { //Add error and redirect for it to be displayed - TempData["ExternalSignInError"] = new[] { "An error occurred, could not get external login info" }; + TempData[TokenExternalSignInError] = new[] { "An error occurred, could not get external login info" }; return RedirectToLocal(Url.Action("Default", "BackOffice")); } @@ -454,27 +480,32 @@ namespace Umbraco.Web.Editors } //Add errors and redirect for it to be displayed - TempData["ExternalSignInError"] = result.Errors; + TempData[TokenExternalSignInError] = result.Errors; return RedirectToLocal(Url.Action("Default", "BackOffice")); } /// - /// Used by Default and AuthorizeUpgrade to render as per normal if there's no external login info, otherwise - /// process the external login info. + /// Used by Default and AuthorizeUpgrade to render as per normal if there's no external login info, + /// otherwise process the external login info. /// - /// - private async Task RenderDefaultOrProcessExternalLoginAsync(Func defaultResponse, Func externalSignInResponse) + /// + private async Task RenderDefaultOrProcessExternalLoginAsync( + Func defaultResponse, + Func externalSignInResponse) { if (defaultResponse == null) throw new ArgumentNullException("defaultResponse"); if (externalSignInResponse == null) throw new ArgumentNullException("externalSignInResponse"); ViewBag.UmbracoPath = GlobalSettings.UmbracoMvcArea; - //check if there's errors in the TempData, assign to view bag and render the view - if (TempData["ExternalSignInError"] != null) - { - ViewBag.ExternalSignInError = TempData["ExternalSignInError"]; - return defaultResponse(); + //check if there is the TempData with the any token name specified, if so, assign to view bag and render the view + foreach (var tempDataTokenName in TempDataTokenNames) + { + if (TempData[tempDataTokenName] != null) + { + ViewData[tempDataTokenName] = TempData[tempDataTokenName]; + return defaultResponse(); + } } //First check if there's external login info, if there's not proceed as normal @@ -512,7 +543,7 @@ namespace Umbraco.Web.Editors { if (await AutoLinkAndSignInExternalAccount(loginInfo) == false) { - ViewBag.ExternalSignInError = new[] { "The requested provider (" + loginInfo.Login.LoginProvider + ") has not been linked to to an account" }; + ViewData[TokenExternalSignInError] = new[] { "The requested provider (" + loginInfo.Login.LoginProvider + ") has not been linked to to an account" }; } //Remove the cookie otherwise this message will keep appearing @@ -547,7 +578,7 @@ namespace Umbraco.Web.Editors //we are allowing auto-linking/creating of local accounts if (loginInfo.Email.IsNullOrWhiteSpace()) { - ViewBag.ExternalSignInError = new[] { "The requested provider (" + loginInfo.Login.LoginProvider + ") has not provided an email address, the account cannot be linked." }; + ViewData[TokenExternalSignInError] = new[] { "The requested provider (" + loginInfo.Login.LoginProvider + ") has not provided an email address, the account cannot be linked." }; } else { @@ -556,7 +587,7 @@ namespace Umbraco.Web.Editors var foundByEmail = Services.UserService.GetByEmail(loginInfo.Email); if (foundByEmail != null) { - ViewBag.ExternalSignInError = new[] { "A user with this email address already exists locally. You will need to login locally to Umbraco and link this external provider: " + loginInfo.Login.LoginProvider }; + ViewData[TokenExternalSignInError] = new[] { "A user with this email address already exists locally. You will need to login locally to Umbraco and link this external provider: " + loginInfo.Login.LoginProvider }; } else { @@ -564,7 +595,7 @@ namespace Umbraco.Web.Editors var userType = Services.UserService.GetUserTypeByAlias(defaultUserType); if (userType == null) { - ViewBag.ExternalSignInError = new[] { "Could not auto-link this account, the specified User Type does not exist: " + defaultUserType }; + ViewData[TokenExternalSignInError] = new[] { "Could not auto-link this account, the specified User Type does not exist: " + defaultUserType }; } else { @@ -592,21 +623,21 @@ namespace Umbraco.Web.Editors if (userCreationResult.Succeeded == false) { - ViewBag.ExternalSignInError = userCreationResult.Errors; + ViewData[TokenExternalSignInError] = userCreationResult.Errors; } else { var linkResult = await UserManager.AddLoginAsync(autoLinkUser.Id, loginInfo.Login); if (linkResult.Succeeded == false) { - ViewBag.ExternalSignInError = linkResult.Errors; + ViewData[TokenExternalSignInError] = linkResult.Errors; //If this fails, we should really delete the user since it will be in an inconsistent state! var deleteResult = await UserManager.DeleteAsync(autoLinkUser); if (deleteResult.Succeeded == false) { //DOH! ... this isn't good, combine all errors to be shown - ViewBag.ExternalSignInError = linkResult.Errors.Concat(deleteResult.Errors); + ViewData[TokenExternalSignInError] = linkResult.Errors.Concat(deleteResult.Errors); } } else diff --git a/src/Umbraco.Web/HtmlHelperBackOfficeExtensions.cs b/src/Umbraco.Web/HtmlHelperBackOfficeExtensions.cs index 2dc9841293..71fc3be8f1 100644 --- a/src/Umbraco.Web/HtmlHelperBackOfficeExtensions.cs +++ b/src/Umbraco.Web/HtmlHelperBackOfficeExtensions.cs @@ -9,6 +9,7 @@ using Newtonsoft.Json; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Web.Editors; +using Umbraco.Web.Models; namespace Umbraco.Web { @@ -61,12 +62,12 @@ namespace Umbraco.Web } /// - /// Used to render the script that will pass in the angular externalLoginInfo service on page load + /// Used to render the script that will pass in the angular "externalLoginInfo" service/value on page load /// /// /// /// - public static IHtmlString AngularExternalLoginInfoValuesScript(this HtmlHelper html, IEnumerable externalLoginErrors) + public static IHtmlString AngularValueExternalLoginInfoScript(this HtmlHelper html, IEnumerable externalLoginErrors) { var loginProviders = html.ViewContext.HttpContext.GetOwinContext().Authentication.GetExternalAuthenticationTypes() .Where(p => p.Properties.ContainsKey("UmbracoBackOffice")) @@ -98,5 +99,38 @@ namespace Umbraco.Web return html.Raw(sb.ToString()); } + + /// + /// Used to render the script that will pass in the angular "resetPasswordCodeInfo" service/value on page load + /// + /// + /// + /// + public static IHtmlString AngularValueResetPasswordCodeInfoScript(this HtmlHelper html, object val) + { + var sb = new StringBuilder(); + sb.AppendLine(); + sb.AppendLine(@"var errors = [];"); + + var errors = val as IEnumerable; + if (errors != null) + { + foreach (var error in errors) + { + sb.AppendFormat(@"errors.push(""{0}"");", error).AppendLine(); + } + } + + var resetCodeModel = val as ValidatePasswordResetCodeModel; + + + sb.AppendLine(@"app.value(""resetPasswordCodeInfo"", {"); + sb.AppendLine(@"errors: errors,"); + sb.Append(@"resetCodeModel: "); + sb.AppendLine(JsonConvert.SerializeObject(resetCodeModel)); + sb.AppendLine(@"});"); + + return html.Raw(sb.ToString()); + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/RequestPasswordResetModel.cs b/src/Umbraco.Web/Models/RequestPasswordResetModel.cs index 650dd8ed11..0ea173bfd6 100644 --- a/src/Umbraco.Web/Models/RequestPasswordResetModel.cs +++ b/src/Umbraco.Web/Models/RequestPasswordResetModel.cs @@ -3,6 +3,8 @@ using System.Runtime.Serialization; namespace Umbraco.Web.Models { + + [DataContract(Name = "requestPasswordReset", Namespace = "")] public class RequestPasswordResetModel { [Required] diff --git a/src/Umbraco.Web/Models/SetPasswordModel.cs b/src/Umbraco.Web/Models/SetPasswordModel.cs index cc70989d66..02d0e4f901 100644 --- a/src/Umbraco.Web/Models/SetPasswordModel.cs +++ b/src/Umbraco.Web/Models/SetPasswordModel.cs @@ -3,6 +3,7 @@ using System.Runtime.Serialization; namespace Umbraco.Web.Models { + [DataContract(Name = "setPassword", Namespace = "")] public class SetPasswordModel { [Required] diff --git a/src/Umbraco.Web/Models/ValidatePasswordResetCodeModel.cs b/src/Umbraco.Web/Models/ValidatePasswordResetCodeModel.cs index 61db6907ef..cba92eeff7 100644 --- a/src/Umbraco.Web/Models/ValidatePasswordResetCodeModel.cs +++ b/src/Umbraco.Web/Models/ValidatePasswordResetCodeModel.cs @@ -3,6 +3,7 @@ using System.Runtime.Serialization; namespace Umbraco.Web.Models { + [DataContract(Name = "validatePasswordReset", Namespace = "")] public class ValidatePasswordResetCodeModel { [Required] diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 16b220b232..48005b1209 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -342,9 +342,9 @@ - +