Added suggestions from code review.

Nuklon · Nov 15, 2021 · 3d43815 · 3d43815
1 parent 709c249
commit 3d43815
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
+        };
+    }
+}