From 9a94ac5317f96ac406552300df1937608e98cdc9 Mon Sep 17 00:00:00 2001
From: Sebastiaan Janssen <sebastiaan@umbraco.com>
Date: Tue, 7 Nov 2017 16:51:40 +0100
Subject: [PATCH 1/2] Implement TwoFactorSignInAsync from Identity repo to make
 sure our custom GetVerifiedUserIdAsync fires

U4-10620 Umbraco 2FA implementation issues
---
 .../Security/BackOfficeSignInManager.cs       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
index 3c0057bcb5..2e20f37125 100644
--- a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
+++ b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
@@ -250,5 +250,42 @@ namespace Umbraco.Core.Security
             }
             return null;
         }
+
+        /// <inheritdoc />
+        /// <summary>
+        /// Two factor verification step
+        /// </summary>
+        /// <param name="provider"></param>
+        /// <param name="code"></param>
+        /// <param name="isPersistent"></param>
+        /// <param name="rememberBrowser"></param>
+        /// <returns></returns>
+        public override async Task<SignInStatus> TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberBrowser)
+        {
+            var userId = await GetVerifiedUserIdAsync();
+            if (userId == null)
+            {
+                return SignInStatus.Failure;
+            }
+            var user = await UserManager.FindByIdAsync(userId);
+            if (user == null)
+            {
+                return SignInStatus.Failure;
+            }
+            if (await UserManager.IsLockedOutAsync(user.Id))
+            {
+                return SignInStatus.LockedOut;
+            }
+            if (await UserManager.VerifyTwoFactorTokenAsync(user.Id, provider, code))
+            {
+                // When token is verified correctly, clear the access failed count used for lockout
+                await UserManager.ResetAccessFailedCountAsync(user.Id);
+                await SignInAsync(user, isPersistent, rememberBrowser);
+                return SignInStatus.Success;
+            }
+            // If the token is incorrect, record the failure which also may cause the user to be locked out
+            await UserManager.AccessFailedAsync(user.Id);
+            return SignInStatus.Failure;
+        }
     }
 }
\ No newline at end of file

From 95f632e1eac742a053c08d597c224d2d22d59ed3 Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Wed, 8 Nov 2017 17:16:11 +1100
Subject: [PATCH 2/2] Adds notes and overrides SendTwoFactorCodeAsync and
 checks for -1 instead of null

---
 .../Security/BackOfficeSignInManager.cs       | 30 +++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
index 2e20f37125..870ed9452c 100644
--- a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
+++ b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs
@@ -12,6 +12,7 @@ using Umbraco.Core.Models.Identity;
 
 namespace Umbraco.Core.Security
 {
+    //TODO: In v8 we need to change this to use an int? nullable TKey instead, see notes against overridden TwoFactorSignInAsync
     public class BackOfficeSignInManager : SignInManager<BackOfficeIdentityUser, int>
     {
         private readonly ILogger _logger;
@@ -251,7 +252,6 @@ namespace Umbraco.Core.Security
             return null;
         }
 
-        /// <inheritdoc />
         /// <summary>
         /// Two factor verification step
         /// </summary>
@@ -260,10 +260,16 @@ namespace Umbraco.Core.Security
         /// <param name="isPersistent"></param>
         /// <param name="rememberBrowser"></param>
         /// <returns></returns>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
         public override async Task<SignInStatus> TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberBrowser)
         {
             var userId = await GetVerifiedUserIdAsync();
-            if (userId == null)
+            if (userId == -1)
             {
                 return SignInStatus.Failure;
             }
@@ -287,5 +293,25 @@ namespace Umbraco.Core.Security
             await UserManager.AccessFailedAsync(user.Id);
             return SignInStatus.Failure;
         }
+
+        /// <summary>Send a two factor code to a user</summary>
+        /// <param name="provider"></param>
+        /// <returns></returns>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
+        public override async Task<bool> SendTwoFactorCodeAsync(string provider)
+        {
+            var userId = await GetVerifiedUserIdAsync();
+            if (userId == -1)
+                return false;
+
+            var token = await UserManager.GenerateTwoFactorTokenAsync(userId, provider);
+            var identityResult = await UserManager.NotifyTwoFactorTokenAsync(userId, provider, token);
+            return identityResult.Succeeded;
+        }
     }
 }
\ No newline at end of file