From 3aabe424a80996bc4989620a6d74321eed5ff30c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 1 May 2019 13:44:18 +0200 Subject: [PATCH 1/7] example of 2fa login view --- .../components/application/umblogin.directive.js | 11 ++++++++--- .../src/views/components/application/umb-login.html | 8 ++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) 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 672a00e27c..da1d17f76a 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 @@ -56,6 +56,7 @@ vm.getStarted = getStarted; vm.inviteSavePassword = inviteSavePassword; vm.showLogin = showLogin; + vm.show2FALogin = show2FALogin; vm.showRequestPasswordReset = showRequestPasswordReset; vm.showSetPassword = showSetPassword; vm.loginSubmit = loginSubmit; @@ -219,7 +220,7 @@ //is Two Factor required? if (reason.status === 402) { vm.errorMsg = "Additional authentication required"; - show2FALoginDialog(reason.data.twoFactorView, submit); + show2FALogin(); } else { vm.loginStates.submitButton = "error"; @@ -403,8 +404,12 @@ }); } - function show2FALoginDialog(view, callback) { - // TODO: show 2FA window + function show2FALogin() { + + vm.errorMsg = ''; + resetInputValidation(); + vm.view = "2fa-login"; + } function resetInputValidation() { 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 d5dc203d67..fd9d4db6f7 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 @@ -258,6 +258,14 @@ Return to login form + +
+ +

2FA Login dialog

+

Here you go...

+ +
+ From b60fe9cc0ac3f787cea07d1265b18478aa834951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 1 May 2019 13:56:34 +0200 Subject: [PATCH 2/7] use external view --- .../components/application/umblogin.directive.js | 12 ++++++++---- .../src/views/components/application/umb-login.html | 5 +---- 2 files changed, 9 insertions(+), 8 deletions(-) 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 da1d17f76a..f1367efa7d 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 @@ -56,7 +56,6 @@ vm.getStarted = getStarted; vm.inviteSavePassword = inviteSavePassword; vm.showLogin = showLogin; - vm.show2FALogin = show2FALogin; vm.showRequestPasswordReset = showRequestPasswordReset; vm.showSetPassword = showSetPassword; vm.loginSubmit = loginSubmit; @@ -70,7 +69,9 @@ ).then(function (data) { vm.labels.usernameLabel = data[0]; vm.labels.usernamePlaceholder = data[1]; - }) + }) + + vm.twoFactor = {}; function onInit() { @@ -220,7 +221,8 @@ //is Two Factor required? if (reason.status === 402) { vm.errorMsg = "Additional authentication required"; - show2FALogin(); + show2FALogin(reason.data.twoFactorView, $scope.loginSubmit); + } else { vm.loginStates.submitButton = "error"; @@ -404,10 +406,12 @@ }); } - function show2FALogin() { + function show2FALogin(viewPath, submitCallback) { vm.errorMsg = ''; resetInputValidation(); + vm.twoFactor.submitCallback = submitCallback; + vm.twoFactor.view = viewPath; vm.view = "2fa-login"; } 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 fd9d4db6f7..d081804477 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 @@ -260,10 +260,7 @@
- -

2FA Login dialog

-

Here you go...

- +
From b4368666ca0a01b55f8764ba14bd2302fc82c0fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 1 May 2019 14:31:26 +0200 Subject: [PATCH 3/7] return to login-view on submit --- .../directives/components/application/umblogin.directive.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 f1367efa7d..6c8e6b82dd 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 @@ -190,7 +190,10 @@ } function loginSubmit(login, password) { - + + // make sure that we are returning to the login view. + vm.view = "login"; + // TODO: Do validation properly like in the invite password update //if the login and password are not empty we need to automatically From e2730f2b66bc54191c420673cfb85acc4a1387b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Wed, 1 May 2019 14:36:51 +0200 Subject: [PATCH 4/7] use vm.login and vm.password, not parsing it. --- .../components/application/umblogin.directive.js | 8 ++++---- .../src/views/components/application/umb-login.html | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) 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 6c8e6b82dd..6786ed10ae 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 @@ -189,7 +189,7 @@ vm.view = "set-password"; } - function loginSubmit(login, password) { + function loginSubmit() { // make sure that we are returning to the login view. vm.view = "login"; @@ -200,7 +200,7 @@ // 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 (login && password && login.length > 0 && 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); } @@ -211,7 +211,7 @@ vm.loginStates.submitButton = "busy"; - userService.authenticate(login, password) + userService.authenticate(vm.login, vm.password) .then(function (data) { vm.loginStates.submitButton = "success"; userService._retryRequestQueue(true); @@ -225,7 +225,7 @@ if (reason.status === 402) { vm.errorMsg = "Additional authentication required"; show2FALogin(reason.data.twoFactorView, $scope.loginSubmit); - + } else { vm.loginStates.submitButton = "error"; 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 d081804477..baf9af916c 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 @@ -146,7 +146,7 @@ -
+
{{vm.errorMsg}}
From 503d6d0265034538a6469fee06a847c813b8db5f Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Thu, 2 May 2019 14:44:33 +0200 Subject: [PATCH 5/7] Make the callback do something and cleanup code a little bit to limit it to only what we actually need --- .../components/application/umblogin.directive.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) 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 6786ed10ae..7fb75c45e6 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 @@ -16,7 +16,6 @@ function UmbLoginController($scope, $location, currentUserResource, formHelper, mediaHelper, umbRequestHelper, Upload, localizationService, userService, externalLoginInfo, resetPasswordCodeInfo, $timeout, authResource, $q) { const vm = this; - let twoFactorloginDialog = null; vm.invitedUser = null; @@ -69,7 +68,7 @@ ).then(function (data) { vm.labels.usernameLabel = data[0]; vm.labels.usernamePlaceholder = data[1]; - }) + }); vm.twoFactor = {}; @@ -224,8 +223,7 @@ //is Two Factor required? if (reason.status === 402) { vm.errorMsg = "Additional authentication required"; - show2FALogin(reason.data.twoFactorView, $scope.loginSubmit); - + show2FALoginDialog(reason.data.twoFactorView); } else { vm.loginStates.submitButton = "error"; @@ -409,14 +407,12 @@ }); } - function show2FALogin(viewPath, submitCallback) { - - vm.errorMsg = ''; - resetInputValidation(); - vm.twoFactor.submitCallback = submitCallback; + function show2FALoginDialog(viewPath) { + vm.twoFactor.submitCallback = function submitCallback() { + vm.onLogin(); + } vm.twoFactor.view = viewPath; vm.view = "2fa-login"; - } function resetInputValidation() { From e5839a89cd4b1853a4080004ac83cf3dac810920 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Thu, 2 May 2019 14:56:12 +0200 Subject: [PATCH 6/7] Fixes references to the "SuperUser" which used to have user Id 0, but now has userId -1 --- src/Umbraco.Web/Editors/AuthenticationController.cs | 7 +++---- src/Umbraco.Web/Security/BackOfficeSignInManager.cs | 12 ++++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index cafb85c3b4..2976c26d82 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -329,7 +329,7 @@ namespace Umbraco.Web.Editors public async Task> Get2FAProviders() { var userId = await SignInManager.GetVerifiedUserIdAsync(); - if (userId < 0) + if (userId < Core.Constants.Security.SuperUserId) { Logger.Warn("Get2FAProviders :: No verified user found, returning 404"); throw new HttpResponseException(HttpStatusCode.NotFound); @@ -345,7 +345,7 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); var userId = await SignInManager.GetVerifiedUserIdAsync(); - if (userId < 0) + if (userId < Core.Constants.Security.SuperUserId) { Logger.Warn("Get2FAProviders :: No verified user found, returning 404"); throw new HttpResponseException(HttpStatusCode.NotFound); @@ -475,8 +475,7 @@ namespace Umbraco.Web.Editors if (UserManager != null) { - var userId = -1; - int.TryParse(User.Identity.GetUserId(), out userId); + int.TryParse(User.Identity.GetUserId(), out var userId); UserManager.RaiseLogoutSuccessEvent(userId); } diff --git a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs index 3ce72852bf..6e32424201 100644 --- a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs @@ -227,7 +227,7 @@ namespace Umbraco.Web.Security } /// - /// Get the user id that has been verified already or -1. + /// Get the user id that has been verified already or the SuperUserId minus 1. /// /// /// @@ -240,7 +240,7 @@ namespace Umbraco.Web.Security { return ConvertIdFromString(result.Identity.GetUserId()); } - return -1; + return Constants.Security.SuperUserId - 1; } /// @@ -269,12 +269,12 @@ namespace Umbraco.Web.Security /// This is implemented because we cannot override GetVerifiedUserIdAsync and instead we have to shadow it /// so due to this and because we are using an INT as the TKey and not an object, it can never be null. Adding to that /// the default(int) value returned by the base class is always a valid user (i.e. the admin) so we just have to duplicate - /// all of this code to check for -1 instead. + /// all of this code to check for SuperUserId-1 instead. /// public override async Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberBrowser) { var userId = await GetVerifiedUserIdAsync(); - if (userId == -1) + if (userId == Constants.Security.SuperUserId - 1) { return SignInStatus.Failure; } @@ -306,12 +306,12 @@ namespace Umbraco.Web.Security /// This is implemented because we cannot override GetVerifiedUserIdAsync and instead we have to shadow it /// so due to this and because we are using an INT as the TKey and not an object, it can never be null. Adding to that /// the default(int) value returned by the base class is always a valid user (i.e. the admin) so we just have to duplicate - /// all of this code to check for -1 instead. + /// all of this code to check for SuperUserId-1 instead. /// public override async Task SendTwoFactorCodeAsync(string provider) { var userId = await GetVerifiedUserIdAsync(); - if (userId == -1) + if (userId == Constants.Security.SuperUserId - 1) return false; var token = await UserManager.GenerateTwoFactorTokenAsync(userId, provider); From c03a4d978b278a19fce79b8d0e08a0642187c7a2 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 6 May 2019 08:22:03 +0200 Subject: [PATCH 7/7] Use int.MinValue instead of calculating from SuperUserId --- src/Umbraco.Web/Editors/AuthenticationController.cs | 4 ++-- src/Umbraco.Web/Security/BackOfficeSignInManager.cs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 2976c26d82..c2c481e8e4 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -329,7 +329,7 @@ namespace Umbraco.Web.Editors public async Task> Get2FAProviders() { var userId = await SignInManager.GetVerifiedUserIdAsync(); - if (userId < Core.Constants.Security.SuperUserId) + if (userId == int.MinValue) { Logger.Warn("Get2FAProviders :: No verified user found, returning 404"); throw new HttpResponseException(HttpStatusCode.NotFound); @@ -345,7 +345,7 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); var userId = await SignInManager.GetVerifiedUserIdAsync(); - if (userId < Core.Constants.Security.SuperUserId) + if (userId == int.MinValue) { Logger.Warn("Get2FAProviders :: No verified user found, returning 404"); throw new HttpResponseException(HttpStatusCode.NotFound); diff --git a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs index 6e32424201..b33487bc8d 100644 --- a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs @@ -227,7 +227,7 @@ namespace Umbraco.Web.Security } /// - /// Get the user id that has been verified already or the SuperUserId minus 1. + /// Get the user id that has been verified already or int.MinValue if the user has not been verified yet /// /// /// @@ -240,7 +240,7 @@ namespace Umbraco.Web.Security { return ConvertIdFromString(result.Identity.GetUserId()); } - return Constants.Security.SuperUserId - 1; + return int.MinValue; } /// @@ -269,12 +269,12 @@ namespace Umbraco.Web.Security /// This is implemented because we cannot override GetVerifiedUserIdAsync and instead we have to shadow it /// so due to this and because we are using an INT as the TKey and not an object, it can never be null. Adding to that /// the default(int) value returned by the base class is always a valid user (i.e. the admin) so we just have to duplicate - /// all of this code to check for SuperUserId-1 instead. + /// all of this code to check for int.MinValue /// public override async Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberBrowser) { var userId = await GetVerifiedUserIdAsync(); - if (userId == Constants.Security.SuperUserId - 1) + if (userId == int.MinValue) { return SignInStatus.Failure; } @@ -306,12 +306,12 @@ namespace Umbraco.Web.Security /// This is implemented because we cannot override GetVerifiedUserIdAsync and instead we have to shadow it /// so due to this and because we are using an INT as the TKey and not an object, it can never be null. Adding to that /// the default(int) value returned by the base class is always a valid user (i.e. the admin) so we just have to duplicate - /// all of this code to check for SuperUserId-1 instead. + /// all of this code to check for int.MinVale instead. /// public override async Task SendTwoFactorCodeAsync(string provider) { var userId = await GetVerifiedUserIdAsync(); - if (userId == Constants.Security.SuperUserId - 1) + if (userId == int.MinValue) return false; var token = await UserManager.GenerateTwoFactorTokenAsync(userId, provider);