From ac9dedf32775ca8058b02b4afa2e5bcb8876efae Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 22 Sep 2020 11:05:37 +1000 Subject: [PATCH] Ensure we don't swallow exceptions in angular when logging in, fixes a null ref in angular preventing login in authorize screen, fixes login screen to use an explicit ng-form instance. --- .../Security/MembershipProviderBase.cs | 4 +- .../application/umblogin.directive.js | 44 +++++++++---------- .../src/common/services/focuslock.service.js | 8 +++- .../src/common/services/user.service.js | 16 ++++--- .../components/application/umb-login.html | 2 +- 5 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/Umbraco.Core/Security/MembershipProviderBase.cs b/src/Umbraco.Core/Security/MembershipProviderBase.cs index 633e12bcc1..0bc8de492a 100644 --- a/src/Umbraco.Core/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/MembershipProviderBase.cs @@ -31,7 +31,9 @@ namespace Umbraco.Core.Security public bool VerifyPassword(string password, string hashedPassword) { - if (string.IsNullOrWhiteSpace(hashedPassword)) throw new ArgumentException("Value cannot be null or whitespace.", "hashedPassword"); + if (string.IsNullOrWhiteSpace(hashedPassword)) + return false; + return CheckPassword(password, hashedPassword); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js index f3fa74f6a1..ed20203d46 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js @@ -40,7 +40,7 @@ uploadProgress: 0, maxFileSize: Umbraco.Sys.ServerVariables.umbracoSettings.maxFileSize + "KB", acceptedFileTypes: mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.imageFileTypes), - uploaded: false + uploaded: false }; vm.allowPasswordReset = Umbraco.Sys.ServerVariables.umbracoSettings.canSendRequiredEmail && Umbraco.Sys.ServerVariables.umbracoSettings.allowPasswordReset; @@ -73,7 +73,7 @@ vm.setPasswordSubmit = setPasswordSubmit; vm.labels = {}; localizationService.localizeMany([ - vm.usernameIsEmail ? "general_email" : "general_username", + vm.usernameIsEmail ? "general_email" : "general_username", vm.usernameIsEmail ? "placeholders_email" : "placeholders_usernameHint", vm.usernameIsEmail ? "placeholders_emptyEmail" : "placeholders_emptyUsername", "placeholders_emptyPassword"] @@ -83,7 +83,7 @@ vm.labels.usernameError = data[2]; vm.labels.passwordError = data[3]; }); - + vm.twoFactor = {}; vm.loginSuccess = loginSuccess; @@ -111,11 +111,11 @@ //localize the text localizationService.localize("errorHandling_errorInPasswordFormat", [ - vm.invitedUserPasswordModel.passwordPolicies.minPasswordLength, - vm.invitedUserPasswordModel.passwordPolicies.minNonAlphaNumericChars - ]).then(function (data) { - vm.invitedUserPasswordModel.passwordPolicyText = data; - }); + vm.invitedUserPasswordModel.passwordPolicies.minPasswordLength, + vm.invitedUserPasswordModel.passwordPolicies.minNonAlphaNumericChars + ]).then(function (data) { + vm.invitedUserPasswordModel.passwordPolicyText = data; + }); }) ]).then(function () { vm.inviteStep = Number(inviteVal); @@ -157,12 +157,12 @@ function getStarted() { $location.search('invite', null); - if(vm.onLogin) { + if (vm.onLogin) { vm.onLogin(); } } - function inviteSavePassword () { + function inviteSavePassword() { if (formHelper.submitForm({ scope: $scope })) { @@ -219,32 +219,32 @@ } function loginSubmit() { - - if (formHelper.submitForm({ scope: $scope })) { + + if (formHelper.submitForm({ scope: $scope, formCtrl: vm.loginForm })) { //if the login and password are not empty we need to automatically // validate them - this is because if there are validation errors on the server // then the user has to change both username & password to resubmit which isn't ideal, // so if they're not empty, we'll just make sure to set them to valid. - if (vm.login && vm.password && vm.login.length > 0 && vm.password.length > 0) { + if (vm.login && vm.password && vm.login.length > 0 && vm.password.length > 0) { vm.loginForm.username.$setValidity('auth', true); vm.loginForm.password.$setValidity('auth', true); } - + if (vm.loginForm.$invalid) { SetTitle(); return; } - + // make sure that we are returning to the login view. vm.view = "login"; vm.loginStates.submitButton = "busy"; userService.authenticate(vm.login, vm.password) - .then(function(data) { - loginSuccess(); - }, - function(reason) { + .then(function (data) { + loginSuccess(); + }, + function (reason) { //is Two Factor required? if (reason.status === 402) { @@ -266,13 +266,13 @@ //setup a watch for both of the model values changing, if they change // while the form is invalid, then revalidate them so that the form can // be submitted again. - vm.loginForm.username.$viewChangeListeners.push(function() { + vm.loginForm.username.$viewChangeListeners.push(function () { if (vm.loginForm.$invalid) { vm.loginForm.username.$setValidity('auth', true); vm.loginForm.password.$setValidity('auth', true); } }); - vm.loginForm.password.$viewChangeListeners.push(function() { + vm.loginForm.password.$viewChangeListeners.push(function () { if (vm.loginForm.$invalid) { vm.loginForm.username.$setValidity('auth', true); vm.loginForm.password.$setValidity('auth', true); @@ -477,7 +477,7 @@ case "2fa-login": title = "Two Factor Authentication"; break; - } + } $scope.$emit("$changeTitle", title); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/focuslock.service.js b/src/Umbraco.Web.UI.Client/src/common/services/focuslock.service.js index a3dd91194e..9ce2f41691 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/focuslock.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/focuslock.service.js @@ -5,11 +5,15 @@ var elementToInert = document.querySelector('#mainwrapper'); function addInertAttribute() { - elementToInert.setAttribute('inert', true); + if (elementToInert) { + elementToInert.setAttribute('inert', true); + } } function removeInertAttribute() { - elementToInert.removeAttribute('inert'); + if (elementToInert) { + elementToInert.removeAttribute('inert'); + } } var service = { diff --git a/src/Umbraco.Web.UI.Client/src/common/services/user.service.js b/src/Umbraco.Web.UI.Client/src/common/services/user.service.js index 3e8975eb0c..00871caab1 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/user.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/user.service.js @@ -166,7 +166,7 @@ angular.module('umbraco.services') }, /** Internal method to retry all request after sucessfull login */ - _retryRequestQueue: function(success) { + _retryRequestQueue: function (success) { retryRequestQueue(success) }, @@ -185,18 +185,22 @@ angular.module('umbraco.services') authenticate: function (login, password) { return authResource.performLogin(login, password) - .then(function(data) { + .then(function (data) { // Check if user has a start node set. - if(data.startContentIds.length === 0 && data.startMediaIds.length === 0){ + if (data.startContentIds.length === 0 && data.startMediaIds.length === 0) { var errorMsg = "User has no start-nodes"; var result = { errorMsg: errorMsg, user: data, authenticated: false, lastUserId: lastUserId, loginType: "credentials" }; eventsService.emit("app.notAuthenticated", result); + // TODO: How does this make sense? How can you throw from a promise? Does this get caught by the rejection? + // If so then return $q.reject should be used. throw result; } - + return data; - + + }, function (err) { + return $q.reject(err); }).then(this.setAuthenticationSuccessful); }, setAuthenticationSuccessful: function (data) { @@ -251,7 +255,7 @@ angular.module('umbraco.services') /** Returns the current user object in a promise */ getCurrentUser: function (args) { - + if (!currentUser) { return authResource.getCurrentUser() .then(function (data) { diff --git a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html index 82328fbf7d..e13e8df1d2 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html @@ -100,7 +100,7 @@ -
+