From 3d438159816ad157083f640030689809e54fcfb9 Mon Sep 17 00:00:00 2001
From: Andy Butland <abutland73@gmail.com>
Date: Mon, 15 Nov 2021 11:17:47 +0100
Subject: [PATCH] Added suggestions from code review.

---
 .../Controllers/BackOfficeController.cs           |  5 +++++
 .../Security/BackOfficeSignInManager.cs           | 15 +++++++++------
 .../Security/ExternalLoginSignInResult.cs         | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 6 deletions(-)
 create mode 100644 src/Umbraco.Web.BackOffice/Security/ExternalLoginSignInResult.cs

diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs
index b6df2ebf4aa7..aa5fc83e1e6a 100644
--- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs
+++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs
@@ -517,6 +517,11 @@ private async Task<IActionResult> ExternalSignInAsync(ExternalLoginInfo loginInf
                 // Failed only occurs when the user does not exist
                 errors.Add("The requested provider (" + loginInfo.LoginProvider + ") has not been linked to an account, the provider must be linked from the back office.");
             }
+            else if (result == ExternalLoginSignInResult.NotAllowed)
+            {
+                // This occurs when the external provider has approved the login but custom logic in OnExternalLogin has denined it.
+                errors.Add($"The user {loginInfo.Principal.Identity.Name} for the external provider {loginInfo.ProviderDisplayName} has not been accepted and cannot sign in.");
+            }
             else if (result == AutoLinkSignInResult.FailedNotLinked)
             {
                 errors.Add("The requested provider (" + loginInfo.LoginProvider + ") has not been linked to an account, the provider must be linked from the back office.");
diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs
index 11f77a34e988..4426e07e22e1 100644
--- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs
+++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs
@@ -77,8 +77,8 @@ public async Task<SignInResult> ExternalLoginSignInAsync(ExternalLoginInfo login
                 var shouldSignIn = autoLinkOptions.OnExternalLogin(user, loginInfo);
                 if (shouldSignIn == false)
                 {
-                    Logger.LogWarning("The AutoLinkOptions of the external authentication provider '{LoginProvider}' have refused the login based on the OnExternalLogin method. Affected user id: '{UserId}'", loginInfo.LoginProvider, user.Id);
-                    return SignInResult.NotAllowed;
+                    LogFailedExternalLogin(loginInfo, user);
+                    return ExternalLoginSignInResult.NotAllowed;
                 }
             }
 
@@ -196,8 +196,8 @@ private async Task<SignInResult> AutoLinkAndSignInExternalAccount(ExternalLoginI
                     var shouldSignIn = autoLinkOptions.OnExternalLogin(autoLinkUser, loginInfo);
                     if (shouldSignIn == false)
                     {
-                        Logger.LogWarning("The AutoLinkOptions of the external authentication provider '{LoginProvider}' have refused the login based on the OnExternalLogin method. Affected user id: '{UserId}'", loginInfo.LoginProvider, autoLinkUser.Id);
-                        return SignInResult.NotAllowed;
+                        LogFailedExternalLogin(loginInfo, autoLinkUser);
+                        return ExternalLoginSignInResult.NotAllowed;
                     }
                     else
                     {
@@ -238,8 +238,8 @@ private async Task<SignInResult> AutoLinkAndSignInExternalAccount(ExternalLoginI
                         var shouldSignIn = autoLinkOptions.OnExternalLogin(autoLinkUser, loginInfo);
                         if (shouldSignIn == false)
                         {
-                            Logger.LogWarning("The AutoLinkOptions of the external authentication provider '{LoginProvider}' have refused the login based on the OnExternalLogin method. Affected user id: '{UserId}'", loginInfo.LoginProvider, autoLinkUser.Id);
-                            return SignInResult.NotAllowed;
+                            LogFailedExternalLogin(loginInfo, autoLinkUser);
+                            return ExternalLoginSignInResult.NotAllowed;
                         }
                         else
                         {
@@ -283,5 +283,8 @@ private async Task<SignInResult> LinkUser(BackOfficeIdentityUser autoLinkUser, E
                 return AutoLinkSignInResult.FailedLinkingUser(errors);
             }
         }
+
+        private void LogFailedExternalLogin(ExternalLoginInfo loginInfo, BackOfficeIdentityUser user) =>
+            Logger.LogWarning("The AutoLinkOptions of the external authentication provider '{LoginProvider}' have refused the login based on the OnExternalLogin method. Affected user id: '{UserId}'", loginInfo.LoginProvider, user.Id);
     }
 }
diff --git a/src/Umbraco.Web.BackOffice/Security/ExternalLoginSignInResult.cs b/src/Umbraco.Web.BackOffice/Security/ExternalLoginSignInResult.cs
new file mode 100644
index 000000000000..188961b2acd2
--- /dev/null
+++ b/src/Umbraco.Web.BackOffice/Security/ExternalLoginSignInResult.cs
@@ -0,0 +1,15 @@
+using Microsoft.AspNetCore.Identity;
+
+namespace Umbraco.Cms.Web.BackOffice.Security
+{
+    /// <summary>
+    /// Result returned from signing in when external logins are used.
+    /// </summary>
+    public class ExternalLoginSignInResult : SignInResult
+    {
+        public static ExternalLoginSignInResult NotAllowed { get; } = new ExternalLoginSignInResult()
+        {
+            Succeeded = false
+        };
+    }
+}