Add settings to bypass 2fa for external logins (#11959)

bergmania · web-flow · commit 3d28552a77f1 · 2022-02-09T10:19:39.000+01:00
* Added settings for bypassing 2fa for external logins

* Fixed issue with saving roles using member ID before the member had an ID.

* Added missing extension method

* Removed test classes from git

* rollback csproj
diff --git a/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs b/src/Umbraco.Core/Configuration/Models/SecuritySettings.cs
@@ -11,6 +11,8 @@ namespace Umbraco.Cms.Core.Configuration.Models
     [UmbracoOptions(Constants.Configuration.ConfigSecurity)]
     public class SecuritySettings
     {
+        internal const bool StaticMemberBypassTwoFactorForExternalLogins = true;
+        internal const bool StaticUserBypassTwoFactorForExternalLogins = true;
         internal const bool StaticKeepUserLoggedIn = false;
         internal const bool StaticHideDisabledUsersInBackOffice = false;
         internal const bool StaticAllowPasswordReset = true;
@@ -66,5 +68,17 @@ public class SecuritySettings
         /// Gets or sets a value for the member password settings.
         /// </summary>
         public MemberPasswordConfigurationSettings MemberPassword { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether to bypass the two factor requirement in Umbraco when using external login for members. Thereby rely on the External login and potential 2FA at that provider.
+        /// </summary>
+        [DefaultValue(StaticMemberBypassTwoFactorForExternalLogins)]
+        public bool MemberBypassTwoFactorForExternalLogins { get; set; } = StaticMemberBypassTwoFactorForExternalLogins;
+
+        /// <summary>
+        /// Gets or sets a value indicating whether to bypass the two factor requirement in Umbraco when using external login for users. Thereby rely on the External login and potential 2FA at that provider.
+        /// </summary>
+        [DefaultValue(StaticUserBypassTwoFactorForExternalLogins)]
+        public bool UserBypassTwoFactorForExternalLogins { get; set; } = StaticUserBypassTwoFactorForExternalLogins;
     }
 }
diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs
@@ -112,6 +112,9 @@ public override Task<IdentityResult> CreateAsync(MemberIdentityUser user, Cancel
                 // create the member
                 _memberService.Save(memberEntity);
 
+                //We need to add roles now that the member has an Id. It do not work implicit in UpdateMemberProperties
+                _memberService.AssignRoles(new[] { memberEntity.Id },  user.Roles.Select(x => x.RoleId).ToArray());
+
                 if (!memberEntity.HasIdentity)
                 {
                     throw new DataException("Could not create the member, check logs for details");
diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs
@@ -10,6 +10,7 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core;
@@ -31,6 +32,7 @@
 using Umbraco.Cms.Web.Common.Attributes;
 using Umbraco.Cms.Web.Common.Authorization;
 using Umbraco.Cms.Web.Common.Controllers;
+using Umbraco.Cms.Web.Common.DependencyInjection;
 using Umbraco.Cms.Web.Common.Filters;
 using Umbraco.Extensions;
 using Constants = Umbraco.Cms.Core.Constants;
@@ -68,7 +70,10 @@ public class BackOfficeController : UmbracoController
         private readonly IBackOfficeTwoFactorOptions _backOfficeTwoFactorOptions;
         private readonly IManifestParser _manifestParser;
         private readonly ServerVariablesParser _serverVariables;
+        private readonly IOptions<SecuritySettings> _securitySettings;
 
+
+        [ActivatorUtilitiesConstructor]
         public BackOfficeController(
             IBackOfficeUserManager userManager,
             IRuntimeState runtimeState,
@@ -87,7 +92,8 @@ public BackOfficeController(
             IHttpContextAccessor httpContextAccessor,
             IBackOfficeTwoFactorOptions backOfficeTwoFactorOptions,
             IManifestParser manifestParser,
-            ServerVariablesParser serverVariables)
+            ServerVariablesParser serverVariables,
+            IOptions<SecuritySettings> securitySettings)
         {
             _userManager = userManager;
             _runtimeState = runtimeState;
@@ -107,6 +113,51 @@ public BackOfficeController(
             _backOfficeTwoFactorOptions = backOfficeTwoFactorOptions;
             _manifestParser = manifestParser;
             _serverVariables = serverVariables;
+            _securitySettings = securitySettings;
+        }
+
+        [Obsolete("Use ctor with all params. This overload will be removed in Umbraco 10.")]
+        public BackOfficeController(
+            IBackOfficeUserManager userManager,
+            IRuntimeState runtimeState,
+            IRuntimeMinifier runtimeMinifier,
+            IOptions<GlobalSettings> globalSettings,
+            IHostingEnvironment hostingEnvironment,
+            ILocalizedTextService textService,
+            IGridConfig gridConfig,
+            BackOfficeServerVariables backOfficeServerVariables,
+            AppCaches appCaches,
+            IBackOfficeSignInManager signInManager,
+            IBackOfficeSecurityAccessor backofficeSecurityAccessor,
+            ILogger<BackOfficeController> logger,
+            IJsonSerializer jsonSerializer,
+            IBackOfficeExternalLoginProviders externalLogins,
+            IHttpContextAccessor httpContextAccessor,
+            IBackOfficeTwoFactorOptions backOfficeTwoFactorOptions,
+            IManifestParser manifestParser,
+            ServerVariablesParser serverVariables)
+        : this(userManager,
+            runtimeState,
+            runtimeMinifier,
+            globalSettings,
+            hostingEnvironment,
+            textService,
+            gridConfig,
+            backOfficeServerVariables,
+            appCaches,
+            signInManager,
+            backofficeSecurityAccessor,
+            logger,
+            jsonSerializer,
+            externalLogins,
+            httpContextAccessor,
+            backOfficeTwoFactorOptions,
+            manifestParser,
+            serverVariables,
+            StaticServiceProvider.Instance.GetRequiredService<IOptions<SecuritySettings>>()
+            )
+        {
+
         }
 
         [HttpGet]
@@ -458,7 +509,7 @@ private async Task<IActionResult> ExternalSignInAsync(ExternalLoginInfo loginInf
             if (response == null) throw new ArgumentNullException(nameof(response));
 
             // Sign in the user with this external login provider (which auto links, etc...)
-            SignInResult result = await _signInManager.ExternalLoginSignInAsync(loginInfo, isPersistent: false);
+            SignInResult result = await _signInManager.ExternalLoginSignInAsync(loginInfo, isPersistent: false, bypassTwoFactor: _securitySettings.Value.UserBypassTwoFactorForExternalLogins);
 
             var errors = new List<string>();
 
diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilder.BackOfficeIdentity.cs
@@ -11,6 +11,7 @@
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Security;
 using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Infrastructure.Security;
 using Umbraco.Cms.Web.BackOffice.Security;
 using Umbraco.Cms.Web.Common.AspNetCore;
 using Umbraco.Cms.Web.Common.Security;
@@ -77,5 +78,14 @@ public static IUmbracoBuilder AddBackOfficeExternalLogins(this IUmbracoBuilder u
             return umbracoBuilder;
         }
 
+        public static BackOfficeIdentityBuilder AddTwoFactorProvider<T>(this BackOfficeIdentityBuilder identityBuilder, string providerName) where T : class, ITwoFactorProvider
+        {
+            identityBuilder.Services.AddSingleton<ITwoFactorProvider, T>();
+            identityBuilder.Services.AddSingleton<T>();
+            identityBuilder.AddTokenProvider<TwoFactorBackOfficeValidationProvider<T>>(providerName);
+
+            return identityBuilder;
+        }
+
     }
 }
diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj
@@ -17,6 +17,7 @@
     <ProjectReference Include="../Umbraco.Web.Website/Umbraco.Web.Website.csproj" />
     <ProjectReference Include="../Umbraco.Persistence.SqlCe/Umbraco.Persistence.SqlCe.csproj" Condition="'$(OS)' == 'Windows_NT'" />
   </ItemGroup>
+  
 
   <ItemGroup>
     <Folder Include="App_Plugins" />
diff --git a/src/Umbraco.Web.Website/Controllers/UmbExternalLoginController.cs b/src/Umbraco.Web.Website/Controllers/UmbExternalLoginController.cs
@@ -8,7 +8,9 @@
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.Logging;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Routing;
@@ -29,6 +31,7 @@ public class UmbExternalLoginController : SurfaceController
     {
         private readonly IMemberManager _memberManager;
         private readonly ITwoFactorLoginService _twoFactorLoginService;
+        private readonly IOptions<SecuritySettings> _securitySettings;
         private readonly ILogger<UmbExternalLoginController> _logger;
         private readonly IMemberSignInManagerExternalLogins _memberSignInManager;
 
@@ -42,7 +45,8 @@ public UmbExternalLoginController(
             IPublishedUrlProvider publishedUrlProvider,
             IMemberSignInManagerExternalLogins memberSignInManager,
             IMemberManager memberManager,
-            ITwoFactorLoginService twoFactorLoginService)
+            ITwoFactorLoginService twoFactorLoginService,
+            IOptions<SecuritySettings> securitySettings)
             : base(
                 umbracoContextAccessor,
                 databaseFactory,
@@ -55,6 +59,7 @@ public UmbExternalLoginController(
             _memberSignInManager = memberSignInManager;
             _memberManager = memberManager;
             _twoFactorLoginService = twoFactorLoginService;
+            _securitySettings = securitySettings;
         }
 
         /// <summary>
@@ -95,7 +100,7 @@ public async Task<IActionResult> ExternalLoginCallback(string returnUrl)
             }
             else
             {
-                SignInResult result = await _memberSignInManager.ExternalLoginSignInAsync(loginInfo, false);
+                SignInResult result = await _memberSignInManager.ExternalLoginSignInAsync(loginInfo, false, _securitySettings.Value.MemberBypassTwoFactorForExternalLogins);
 
                 if (result == SignInResult.Success)
                 {

Original file line number	Diff line number	Diff line change
`@@ -112,6 +112,9 @@ public override Task<IdentityResult> CreateAsync(MemberIdentityUser user, Cancel`
`112`	`112`	`// create the member`
`113`	`113`	`_memberService.Save(memberEntity);`
`114`	`114`
	`115`	`+ //We need to add roles now that the member has an Id. It do not work implicit in UpdateMemberProperties`
	`116`	`+ _memberService.AssignRoles(new[] { memberEntity.Id }, user.Roles.Select(x => x.RoleId).ToArray());`
	`117`	`+`
`115`	`118`	`if (!memberEntity.HasIdentity)`
`116`	`119`	`{`
`117`	`120`	`throw new DataException("Could not create the member, check logs for details");`