From a128cf150689b627ad65ef6edb2c4065e7abf447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:39:35 +0100 Subject: [PATCH 01/11] [PM-12758] Add managed status to OrganizationUserDetailsResponseModel and OrganizationUserUserDetailsResponse (#4918) * Refactor OrganizationUsersController.Get to include organization management status of organization users in details endpoint * Refactor OrganizationUsersController.Get to include organization management status of an individual user in details endpoint * Remove redundant .ToDictionary() * Simpify the property xmldoc * Name tuple variables in OrganizationUsersController.Get * Name returned tuple objects in GetDetailsByIdWithCollectionsAsync method in OrganizationUserRepository * Refactor MembersController.Get to destructure tuple returned by GetDetailsByIdWithCollectionsAsync * Add test for OrganizationUsersController.Get to assert ManagedByOrganization is set accordingly --- .../OrganizationUsersController.cs | 37 ++++++++++++---- .../OrganizationUserResponseModel.cs | 17 +++++++- .../Public/Controllers/MembersController.cs | 5 +-- .../IOrganizationUserRepository.cs | 3 +- .../OrganizationUserRepository.cs | 7 ++-- .../OrganizationUserRepository.cs | 4 +- .../OrganizationUsersControllerTests.cs | 42 +++++++++++++++++-- 7 files changed, 91 insertions(+), 24 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index af0aede5a..b6d41ffec 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -53,6 +53,8 @@ public class OrganizationUsersController : Controller private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IDeleteManagedOrganizationUserAccountCommand _deleteManagedOrganizationUserAccountCommand; + private readonly IGetOrganizationUsersManagementStatusQuery _getOrganizationUsersManagementStatusQuery; + private readonly IFeatureService _featureService; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -73,7 +75,9 @@ public class OrganizationUsersController : Controller IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IDeleteManagedOrganizationUserAccountCommand deleteManagedOrganizationUserAccountCommand) + IDeleteManagedOrganizationUserAccountCommand deleteManagedOrganizationUserAccountCommand, + IGetOrganizationUsersManagementStatusQuery getOrganizationUsersManagementStatusQuery, + IFeatureService featureService) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -94,22 +98,28 @@ public class OrganizationUsersController : Controller _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _removeOrganizationUserCommand = removeOrganizationUserCommand; _deleteManagedOrganizationUserAccountCommand = deleteManagedOrganizationUserAccountCommand; + _getOrganizationUsersManagementStatusQuery = getOrganizationUsersManagementStatusQuery; + _featureService = featureService; } [HttpGet("{id}")] - public async Task Get(string id, bool includeGroups = false) + public async Task Get(Guid id, bool includeGroups = false) { - var organizationUser = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(new Guid(id)); - if (organizationUser == null || !await _currentContext.ManageUsers(organizationUser.Item1.OrganizationId)) + var (organizationUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); + if (organizationUser == null || !await _currentContext.ManageUsers(organizationUser.OrganizationId)) { throw new NotFoundException(); } - var response = new OrganizationUserDetailsResponseModel(organizationUser.Item1, organizationUser.Item2); + var managedByOrganization = await GetManagedByOrganizationStatusAsync( + organizationUser.OrganizationId, + [organizationUser.Id]); + + var response = new OrganizationUserDetailsResponseModel(organizationUser, managedByOrganization[organizationUser.Id], collections); if (includeGroups) { - response.Groups = await _groupRepository.GetManyIdsByUserIdAsync(organizationUser.Item1.Id); + response.Groups = await _groupRepository.GetManyIdsByUserIdAsync(organizationUser.Id); } return response; @@ -150,11 +160,13 @@ public class OrganizationUsersController : Controller } ); var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers); + var organizationUsersManagementStatus = await GetManagedByOrganizationStatusAsync(orgId, organizationUsers.Select(o => o.Id)); var responses = organizationUsers .Select(o => { var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == o.Id).twoFactorIsEnabled; - var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled); + var managedByOrganization = organizationUsersManagementStatus[o.Id]; + var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled, managedByOrganization); return orgUser; }); @@ -682,4 +694,15 @@ public class OrganizationUsersController : Controller return new ListResponseModel(result.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } + + private async Task> GetManagedByOrganizationStatusAsync(Guid orgId, IEnumerable userIds) + { + if (!_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + { + return userIds.ToDictionary(kvp => kvp, kvp => false); + } + + var usersOrganizationManagementStatus = await _getOrganizationUsersManagementStatusQuery.GetUsersOrganizationManagementStatusAsync(orgId, userIds); + return usersOrganizationManagementStatus; + } } diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs index 874169486..64dca73aa 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs @@ -64,20 +64,27 @@ public class OrganizationUserResponseModel : ResponseModel public class OrganizationUserDetailsResponseModel : OrganizationUserResponseModel { - public OrganizationUserDetailsResponseModel(OrganizationUser organizationUser, + public OrganizationUserDetailsResponseModel( + OrganizationUser organizationUser, + bool managedByOrganization, IEnumerable collections) : base(organizationUser, "organizationUserDetails") { + ManagedByOrganization = managedByOrganization; Collections = collections.Select(c => new SelectionReadOnlyResponseModel(c)); } public OrganizationUserDetailsResponseModel(OrganizationUserUserDetails organizationUser, + bool managedByOrganization, IEnumerable collections) : base(organizationUser, "organizationUserDetails") { + ManagedByOrganization = managedByOrganization; Collections = collections.Select(c => new SelectionReadOnlyResponseModel(c)); } + public bool ManagedByOrganization { get; set; } + public IEnumerable Collections { get; set; } [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] @@ -110,7 +117,7 @@ public class OrganizationUserUserMiniDetailsResponseModel : ResponseModel public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponseModel { public OrganizationUserUserDetailsResponseModel(OrganizationUserUserDetails organizationUser, - bool twoFactorEnabled, string obj = "organizationUserUserDetails") + bool twoFactorEnabled, bool managedByOrganization, string obj = "organizationUserUserDetails") : base(organizationUser, obj) { if (organizationUser == null) @@ -127,6 +134,7 @@ public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponse Groups = organizationUser.Groups; // Prevent reset password when using key connector. ResetPasswordEnrolled = ResetPasswordEnrolled && !organizationUser.UsesKeyConnector; + ManagedByOrganization = managedByOrganization; } public string Name { get; set; } @@ -134,6 +142,11 @@ public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponse public string AvatarColor { get; set; } public bool TwoFactorEnabled { get; set; } public bool SsoBound { get; set; } + /// + /// Indicates if the organization manages the user. If a user is "managed" by an organization, + /// the organization has greater control over their account, and some user actions are restricted. + /// + public bool ManagedByOrganization { get; set; } public IEnumerable Collections { get; set; } public IEnumerable Groups { get; set; } } diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs index 7515111d2..4e99353d4 100644 --- a/src/Api/AdminConsole/Public/Controllers/MembersController.cs +++ b/src/Api/AdminConsole/Public/Controllers/MembersController.cs @@ -71,14 +71,13 @@ public class MembersController : Controller [ProducesResponseType((int)HttpStatusCode.NotFound)] public async Task Get(Guid id) { - var userDetails = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); - var orgUser = userDetails?.Item1; + var (orgUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id); if (orgUser == null || orgUser.OrganizationId != _currentContext.OrganizationId) { return new NotFoundResult(); } var response = new MemberResponseModel(orgUser, await _userService.TwoFactorIsEnabledAsync(orgUser), - userDetails.Item2); + collections); return new JsonResult(response); } diff --git a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs index 54040e6dc..a3a68b5de 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs @@ -22,8 +22,7 @@ public interface IOrganizationUserRepository : IRepository GetByOrganizationAsync(Guid organizationId, Guid userId); Task>> GetByIdWithCollectionsAsync(Guid id); Task GetDetailsByIdAsync(Guid id); - Task>> - GetDetailsByIdWithCollectionsAsync(Guid id); + Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id); Task> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups = false, bool includeCollections = false); Task> GetManyDetailsByUserAsync(Guid userId, OrganizationUserStatusType? status = null); diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index 6da2f581f..361b1f058 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -196,8 +196,7 @@ public class OrganizationUserRepository : Repository, IO return results.SingleOrDefault(); } } - public async Task>> - GetDetailsByIdWithCollectionsAsync(Guid id) + public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id) { using (var connection = new SqlConnection(ConnectionString)) { @@ -206,9 +205,9 @@ public class OrganizationUserRepository : Repository, IO new { Id = id }, commandType: CommandType.StoredProcedure); - var user = (await results.ReadAsync()).SingleOrDefault(); + var organizationUserUserDetails = (await results.ReadAsync()).SingleOrDefault(); var collections = (await results.ReadAsync()).ToList(); - return new Tuple>(user, collections); + return (organizationUserUserDetails, collections); } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs index 089a0a5c5..0c9f1d0b9 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -248,7 +248,7 @@ public class OrganizationUserRepository : Repository>> GetDetailsByIdWithCollectionsAsync(Guid id) + public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection Collections)> GetDetailsByIdWithCollectionsAsync(Guid id) { var organizationUserUserDetails = await GetDetailsByIdAsync(id); using (var scope = ServiceScopeFactory.CreateScope()) @@ -265,7 +265,7 @@ public class OrganizationUserRepository : Repository>(organizationUserUserDetails, collections); + return (organizationUserUserDetails, collections); } } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index 492112e5a..2ff5c0cb4 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -3,6 +3,7 @@ using Bit.Api.AdminConsole.Controllers; using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Vault.AuthorizationHandlers.Collections; +using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; @@ -15,6 +16,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; +using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; @@ -185,14 +187,46 @@ public class OrganizationUsersControllerTests await Assert.ThrowsAsync(() => sutProvider.Sut.Invite(organizationAbility.Id, model)); } + [Theory] + [BitAutoData(true)] + [BitAutoData(false)] + public async Task Get_ReturnsUser( + bool accountDeprovisioningEnabled, + OrganizationUserUserDetails organizationUser, ICollection collections, + SutProvider sutProvider) + { + organizationUser.Permissions = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(accountDeprovisioningEnabled); + + sutProvider.GetDependency() + .ManageUsers(organizationUser.OrganizationId) + .Returns(true); + + sutProvider.GetDependency() + .GetDetailsByIdWithCollectionsAsync(organizationUser.Id) + .Returns((organizationUser, collections)); + + sutProvider.GetDependency() + .GetUsersOrganizationManagementStatusAsync(organizationUser.OrganizationId, Arg.Is>(ids => ids.Contains(organizationUser.Id))) + .Returns(new Dictionary { { organizationUser.Id, true } }); + + var response = await sutProvider.Sut.Get(organizationUser.Id, false); + + Assert.Equal(organizationUser.Id, response.Id); + Assert.Equal(accountDeprovisioningEnabled, response.ManagedByOrganization); + } + [Theory] [BitAutoData] - public async Task Get_ReturnsUsers( + public async Task GetMany_ReturnsUsers( ICollection organizationUsers, OrganizationAbility organizationAbility, SutProvider sutProvider) { - Get_Setup(organizationAbility, organizationUsers, sutProvider); - var response = await sutProvider.Sut.Get(organizationAbility.Id); + GetMany_Setup(organizationAbility, organizationUsers, sutProvider); + var response = await sutProvider.Sut.Get(organizationAbility.Id, false, false); Assert.True(response.Data.All(r => organizationUsers.Any(ou => ou.Id == r.Id))); } @@ -368,7 +402,7 @@ public class OrganizationUsersControllerTests await Assert.ThrowsAsync(() => sutProvider.Sut.BulkDeleteAccount(orgId, model)); } - private void Get_Setup(OrganizationAbility organizationAbility, + private void GetMany_Setup(OrganizationAbility organizationAbility, ICollection organizationUsers, SutProvider sutProvider) { From 0c346d6070d8aea5c987971478aaf6f19f42a347 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Thu, 24 Oct 2024 10:13:45 -0500 Subject: [PATCH 02/11] [PM-10314] Auto-enable Single Org when a Domain is Verified (#4897) Updated domain verification to auto-enable single org policy. --- .../IOrganizationHasVerifiedDomainsQuery.cs | 6 + .../OrganizationHasVerifiedDomainsQuery.cs | 10 ++ .../VerifyOrganizationDomainCommand.cs | 25 +++- .../Services/IOrganizationDomainService.cs | 4 - .../OrganizationDomainService.cs | 6 - .../Services/Implementations/PolicyService.cs | 16 ++- ...OrganizationServiceCollectionExtensions.cs | 1 + ...rganizationHasVerifiedDomainsQueryTests.cs | 57 +++++++++ .../VerifyOrganizationDomainCommandTests.cs | 108 +++++++++++++++++- .../OrganizationDomainServiceTests.cs | 44 ------- .../Services/PolicyServiceTests.cs | 29 +++++ 11 files changed, 244 insertions(+), 62 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IOrganizationHasVerifiedDomainsQuery.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQuery.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQueryTests.cs diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IOrganizationHasVerifiedDomainsQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IOrganizationHasVerifiedDomainsQuery.cs new file mode 100644 index 000000000..b7df7f83e --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IOrganizationHasVerifiedDomainsQuery.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; + +public interface IOrganizationHasVerifiedDomainsQuery +{ + Task HasVerifiedDomainsAsync(Guid orgId); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQuery.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQuery.cs new file mode 100644 index 000000000..15a36e4f0 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQuery.cs @@ -0,0 +1,10 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.Repositories; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; + +public class OrganizationHasVerifiedDomainsQuery(IOrganizationDomainRepository domainRepository) : IOrganizationHasVerifiedDomainsQuery +{ + public async Task HasVerifiedDomainsAsync(Guid orgId) => + (await domainRepository.GetDomainsByOrganizationIdAsync(orgId)).Any(od => od.VerifiedDate is not null); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs index 8e1a4d573..4a597a290 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs @@ -1,4 +1,7 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.AdminConsole.Services; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -15,6 +18,9 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand private readonly IDnsResolverService _dnsResolverService; private readonly IEventService _eventService; private readonly IGlobalSettings _globalSettings; + private readonly IPolicyService _policyService; + private readonly IFeatureService _featureService; + private readonly IOrganizationService _organizationService; private readonly ILogger _logger; public VerifyOrganizationDomainCommand( @@ -22,12 +28,18 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand IDnsResolverService dnsResolverService, IEventService eventService, IGlobalSettings globalSettings, + IPolicyService policyService, + IFeatureService featureService, + IOrganizationService organizationService, ILogger logger) { _organizationDomainRepository = organizationDomainRepository; _dnsResolverService = dnsResolverService; _eventService = eventService; _globalSettings = globalSettings; + _policyService = policyService; + _featureService = featureService; + _organizationService = organizationService; _logger = logger; } @@ -102,6 +114,8 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand if (await _dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt)) { domain.SetVerifiedDate(); + + await EnableSingleOrganizationPolicyAsync(domain.OrganizationId); } } catch (Exception e) @@ -112,4 +126,13 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand return domain; } + + private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId) + { + if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + { + await _policyService.SaveAsync( + new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true }, null); + } + } } diff --git a/src/Core/AdminConsole/Services/IOrganizationDomainService.cs b/src/Core/AdminConsole/Services/IOrganizationDomainService.cs index 8ed543f0e..463371c14 100644 --- a/src/Core/AdminConsole/Services/IOrganizationDomainService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationDomainService.cs @@ -4,8 +4,4 @@ public interface IOrganizationDomainService { Task ValidateOrganizationsDomainAsync(); Task OrganizationDomainMaintenanceAsync(); - /// - /// Indicates if the organization has any verified domains. - /// - Task HasVerifiedDomainsAsync(Guid orgId); } diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs index 890042b31..4ce33f3b5 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs @@ -106,12 +106,6 @@ public class OrganizationDomainService : IOrganizationDomainService } } - public async Task HasVerifiedDomainsAsync(Guid orgId) - { - var orgDomains = await _domainRepository.GetDomainsByOrganizationIdAsync(orgId); - return orgDomains.Any(od => od.VerifiedDate != null); - } - private async Task> GetAdminEmailsAsync(Guid organizationId) { var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId); diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 6ab90afe0..072aa8283 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; @@ -32,6 +33,7 @@ public class PolicyService : IPolicyService private readonly IFeatureService _featureService; private readonly ISavePolicyCommand _savePolicyCommand; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; + private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; public PolicyService( IApplicationCacheService applicationCacheService, @@ -45,7 +47,8 @@ public class PolicyService : IPolicyService ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, IFeatureService featureService, ISavePolicyCommand savePolicyCommand, - IRemoveOrganizationUserCommand removeOrganizationUserCommand) + IRemoveOrganizationUserCommand removeOrganizationUserCommand, + IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery) { _applicationCacheService = applicationCacheService; _eventService = eventService; @@ -59,6 +62,7 @@ public class PolicyService : IPolicyService _featureService = featureService; _savePolicyCommand = savePolicyCommand; _removeOrganizationUserCommand = removeOrganizationUserCommand; + _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; } public async Task SaveAsync(Policy policy, Guid? savingUserId) @@ -239,6 +243,7 @@ public class PolicyService : IPolicyService case PolicyType.SingleOrg: if (!policy.Enabled) { + await HasVerifiedDomainsAsync(org); await RequiredBySsoAsync(org); await RequiredByVaultTimeoutAsync(org); await RequiredByKeyConnectorAsync(org); @@ -279,6 +284,15 @@ public class PolicyService : IPolicyService } } + private async Task HasVerifiedDomainsAsync(Organization org) + { + if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + && await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(org.Id)) + { + throw new BadRequestException("Organization has verified domains."); + } + } + private async Task SetPolicyConfiguration(Policy policy) { await _policyRepository.UpsertAsync(policy); diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 58eb65faf..d11da2119 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -130,6 +130,7 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } private static void AddOrganizationAuthCommands(this IServiceCollection services) diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQueryTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQueryTests.cs new file mode 100644 index 000000000..f63f6e48b --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/OrganizationHasVerifiedDomainsQueryTests.cs @@ -0,0 +1,57 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationDomains; + +[SutProviderCustomize] +public class OrganizationHasVerifiedDomainsQueryTests +{ + [Theory, BitAutoData] + public async Task HasVerifiedDomainsAsync_WithVerifiedDomain_ReturnsTrue( + OrganizationDomain organizationDomain, + SutProvider sutProvider) + { + organizationDomain.SetVerifiedDate(); // Set the verified date to make it verified + + sutProvider.GetDependency() + .GetDomainsByOrganizationIdAsync(organizationDomain.OrganizationId) + .Returns(new List { organizationDomain }); + + var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationDomain.OrganizationId); + + Assert.True(result); + } + + [Theory, BitAutoData] + public async Task HasVerifiedDomainsAsync_WithoutVerifiedDomain_ReturnsFalse( + OrganizationDomain organizationDomain, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetDomainsByOrganizationIdAsync(organizationDomain.OrganizationId) + .Returns(new List { organizationDomain }); + + var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationDomain.OrganizationId); + + Assert.False(result); + } + + [Theory, BitAutoData] + public async Task HasVerifiedDomainsAsync_WithoutOrganizationDomains_ReturnsFalse( + Guid organizationId, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetDomainsByOrganizationIdAsync(organizationId) + .Returns(new List()); + + var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationId); + + Assert.False(result); + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs index d61ded28b..2fcaf8134 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs @@ -1,4 +1,7 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; +using Bit.Core.AdminConsole.Services; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -15,7 +18,7 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationDomains; public class VerifyOrganizationDomainCommandTests { [Theory, BitAutoData] - public async Task UserVerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimed(Guid id, + public async Task UserVerifyOrganizationDomainAsync_ShouldThrowConflict_WhenDomainHasBeenClaimed(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -37,7 +40,7 @@ public class VerifyOrganizationDomainCommandTests } [Theory, BitAutoData] - public async Task UserVerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimedByAnotherOrganization(Guid id, + public async Task UserVerifyOrganizationDomainAsync_ShouldThrowConflict_WhenDomainHasBeenClaimedByAnotherOrganization(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -61,7 +64,7 @@ public class VerifyOrganizationDomainCommandTests } [Theory, BitAutoData] - public async Task UserVerifyOrganizationDomain_ShouldVerifyDomainUpdateAndLogEvent_WhenTxtRecordExists(Guid id, + public async Task UserVerifyOrganizationDomainAsync_ShouldVerifyDomainUpdateAndLogEvent_WhenTxtRecordExists(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -91,7 +94,7 @@ public class VerifyOrganizationDomainCommandTests } [Theory, BitAutoData] - public async Task UserVerifyOrganizationDomain_ShouldNotSetVerifiedDate_WhenTxtRecordDoesNotExist(Guid id, + public async Task UserVerifyOrganizationDomainAsync_ShouldNotSetVerifiedDate_WhenTxtRecordDoesNotExist(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -120,7 +123,7 @@ public class VerifyOrganizationDomainCommandTests [Theory, BitAutoData] - public async Task SystemVerifyOrganizationDomain_CallsEventServiceWithUpdatedJobRunCount(SutProvider sutProvider) + public async Task SystemVerifyOrganizationDomainAsync_CallsEventServiceWithUpdatedJobRunCount(SutProvider sutProvider) { var domain = new OrganizationDomain() { @@ -137,4 +140,97 @@ public class VerifyOrganizationDomainCommandTests .LogOrganizationDomainEventAsync(default, EventType.OrganizationDomain_NotVerified, EventSystemUser.DomainVerification); } + + [Theory, BitAutoData] + public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningEnabled_WhenDomainIsVerified_ThenSingleOrgPolicyShouldBeEnabled( + OrganizationDomain domain, SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetClaimedDomainsByDomainNameAsync(domain.DomainName) + .Returns([]); + + sutProvider.GetDependency() + .ResolveAsync(domain.DomainName, domain.Txt) + .Returns(true); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); + + await sutProvider.GetDependency() + .Received(1) + .SaveAsync(Arg.Is(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), null); + } + + [Theory, BitAutoData] + public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningDisabled_WhenDomainIsVerified_ThenSingleOrgPolicyShouldBeNotBeEnabled( + OrganizationDomain domain, SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetClaimedDomainsByDomainNameAsync(domain.DomainName) + .Returns([]); + + sutProvider.GetDependency() + .ResolveAsync(domain.DomainName, domain.Txt) + .Returns(true); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(false); + + _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); + + await sutProvider.GetDependency() + .DidNotReceive() + .SaveAsync(Arg.Any(), null); + } + + [Theory, BitAutoData] + public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningEnabled_WhenDomainIsNotVerified_ThenSingleOrgPolicyShouldNotBeEnabled( + OrganizationDomain domain, SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetClaimedDomainsByDomainNameAsync(domain.DomainName) + .Returns([]); + + sutProvider.GetDependency() + .ResolveAsync(domain.DomainName, domain.Txt) + .Returns(false); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); + + await sutProvider.GetDependency() + .DidNotReceive() + .SaveAsync(Arg.Any(), null); + + } + + [Theory, BitAutoData] + public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningDisabled_WhenDomainIsNotVerified_ThenSingleOrgPolicyShouldBeNotBeEnabled( + OrganizationDomain domain, SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetClaimedDomainsByDomainNameAsync(domain.DomainName) + .Returns([]); + + sutProvider.GetDependency() + .ResolveAsync(domain.DomainName, domain.Txt) + .Returns(false); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); + + await sutProvider.GetDependency() + .DidNotReceive() + .SaveAsync(Arg.Any(), null); + } } diff --git a/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs index c779e3a1c..210726061 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs @@ -76,48 +76,4 @@ public class OrganizationDomainServiceTests await sutProvider.GetDependency().ReceivedWithAnyArgs(1) .DeleteExpiredAsync(7); } - - [Theory, BitAutoData] - public async Task HasVerifiedDomainsAsync_WithVerifiedDomain_ReturnsTrue( - OrganizationDomain organizationDomain, - SutProvider sutProvider) - { - organizationDomain.SetVerifiedDate(); // Set the verified date to make it verified - - sutProvider.GetDependency() - .GetDomainsByOrganizationIdAsync(organizationDomain.OrganizationId) - .Returns(new List { organizationDomain }); - - var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationDomain.OrganizationId); - - Assert.True(result); - } - - [Theory, BitAutoData] - public async Task HasVerifiedDomainsAsync_WithoutVerifiedDomain_ReturnsFalse( - OrganizationDomain organizationDomain, - SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetDomainsByOrganizationIdAsync(organizationDomain.OrganizationId) - .Returns(new List { organizationDomain }); - - var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationDomain.OrganizationId); - - Assert.False(result); - } - - [Theory, BitAutoData] - public async Task HasVerifiedDomainsAsync_WithoutOrganizationDomains_ReturnsFalse( - Guid organizationId, - SutProvider sutProvider) - { - sutProvider.GetDependency() - .GetDomainsByOrganizationIdAsync(organizationId) - .Returns(new List()); - - var result = await sutProvider.Sut.HasVerifiedDomainsAsync(organizationId); - - Assert.False(result); - } } diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index f9bc49bbe..da3f2b267 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services.Implementations; @@ -815,4 +816,32 @@ public class PolicyServiceTests new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true } }); } + + + [Theory, BitAutoData] + public async Task SaveAsync_GivenOrganizationUsingPoliciesAndHasVerifiedDomains_WhenSingleOrgPolicyIsDisabled_ThenAnErrorShouldBeThrownOrganizationHasVerifiedDomains( + [AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy, Organization org, SutProvider sutProvider) + { + org.Id = policy.OrganizationId; + org.UsePolicies = true; + + policy.Enabled = false; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + sutProvider.GetDependency() + .GetByIdAsync(policy.OrganizationId) + .Returns(org); + + sutProvider.GetDependency() + .HasVerifiedDomainsAsync(org.Id) + .Returns(true); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, null)); + + Assert.Equal("Organization has verified domains.", badRequestException.Message); + } } From c028c68d9ca89d522c8c9fab006b885235311eb1 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:41:25 -0700 Subject: [PATCH 03/11] [PM-6666] Two factor Validator refactor (#4894) * initial device removal * Unit Testing * Finalized tests * initial commit refactoring two factor * initial tests * Unit Tests * initial device removal * Unit Testing * Finalized tests * initial commit refactoring two factor * initial tests * Unit Tests * Fixing some tests * renaming and reorganizing * refactored two factor flows * fixed a possible issue with object mapping. * Update TwoFactorAuthenticationValidator.cs removed unused code --- src/Identity/IdentityServer/ApiClient.cs | 1 + .../BaseRequestValidator.cs | 305 ++-------- .../CustomTokenRequestValidator.cs | 39 +- .../DeviceValidator.cs | 8 +- .../ResourceOwnerPasswordValidator.cs | 31 +- .../TwoFactorAuthenticationValidator.cs | 297 +++++++++ .../WebAuthnGrantValidator.cs | 36 +- .../Utilities/ServiceCollectionExtensions.cs | 2 + .../ResourceOwnerPasswordValidatorTests.cs | 7 +- .../BaseRequestValidatorTests.cs | 74 +-- .../IdentityServer/DeviceValidatorTests.cs | 2 +- .../TwoFactorAuthenticationValidatorTests.cs | 575 ++++++++++++++++++ .../BaseRequestValidatorTestWrapper.cs | 26 +- .../Wrappers/UserManagerTestWrapper.cs | 96 +++ 14 files changed, 1119 insertions(+), 380 deletions(-) rename src/Identity/IdentityServer/{ => RequestValidators}/BaseRequestValidator.cs (53%) rename src/Identity/IdentityServer/{ => RequestValidators}/CustomTokenRequestValidator.cs (88%) rename src/Identity/IdentityServer/{ => RequestValidators}/DeviceValidator.cs (89%) rename src/Identity/IdentityServer/{ => RequestValidators}/ResourceOwnerPasswordValidator.cs (89%) create mode 100644 src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs rename src/Identity/IdentityServer/{ => RequestValidators}/WebAuthnGrantValidator.cs (82%) create mode 100644 test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs create mode 100644 test/Identity.Test/Wrappers/UserManagerTestWrapper.cs diff --git a/src/Identity/IdentityServer/ApiClient.cs b/src/Identity/IdentityServer/ApiClient.cs index 02fd3dd40..5d768ae80 100644 --- a/src/Identity/IdentityServer/ApiClient.cs +++ b/src/Identity/IdentityServer/ApiClient.cs @@ -1,4 +1,5 @@ using Bit.Core.Settings; +using Bit.Identity.IdentityServer.RequestValidators; using Duende.IdentityServer.Models; namespace Bit.Identity.IdentityServer; diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs similarity index 53% rename from src/Identity/IdentityServer/BaseRequestValidator.cs rename to src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs index 8129a1a10..185d32a7f 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs @@ -1,15 +1,10 @@ using System.Security.Claims; -using System.Text.Json; using Bit.Core; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; -using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Api.Response; -using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; @@ -17,32 +12,26 @@ using Bit.Core.Enums; using Bit.Core.Identity; using Bit.Core.Models.Api; using Bit.Core.Models.Api.Response; -using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Bit.Core.Utilities; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Identity; -namespace Bit.Identity.IdentityServer; +namespace Bit.Identity.IdentityServer.RequestValidators; public abstract class BaseRequestValidator where T : class { private UserManager _userManager; private readonly IEventService _eventService; private readonly IDeviceValidator _deviceValidator; - private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider; - private readonly ITemporaryDuoWebV4SDKService _duoWebV4SDKService; - private readonly IOrganizationRepository _organizationRepository; + private readonly ITwoFactorAuthenticationValidator _twoFactorAuthenticationValidator; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IApplicationCacheService _applicationCacheService; private readonly IMailService _mailService; private readonly ILogger _logger; private readonly GlobalSettings _globalSettings; private readonly IUserRepository _userRepository; - private readonly IDataProtectorTokenFactory _tokenDataFactory; protected ICurrentContext CurrentContext { get; } protected IPolicyService PolicyService { get; } @@ -56,18 +45,14 @@ public abstract class BaseRequestValidator where T : class IUserService userService, IEventService eventService, IDeviceValidator deviceValidator, - IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, - ITemporaryDuoWebV4SDKService duoWebV4SDKService, - IOrganizationRepository organizationRepository, + ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator, IOrganizationUserRepository organizationUserRepository, - IApplicationCacheService applicationCacheService, IMailService mailService, ILogger logger, ICurrentContext currentContext, GlobalSettings globalSettings, IUserRepository userRepository, IPolicyService policyService, - IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) @@ -76,18 +61,14 @@ public abstract class BaseRequestValidator where T : class _userService = userService; _eventService = eventService; _deviceValidator = deviceValidator; - _organizationDuoWebTokenProvider = organizationDuoWebTokenProvider; - _duoWebV4SDKService = duoWebV4SDKService; - _organizationRepository = organizationRepository; + _twoFactorAuthenticationValidator = twoFactorAuthenticationValidator; _organizationUserRepository = organizationUserRepository; - _applicationCacheService = applicationCacheService; _mailService = mailService; _logger = logger; CurrentContext = currentContext; _globalSettings = globalSettings; PolicyService = policyService; _userRepository = userRepository; - _tokenDataFactory = tokenDataFactory; FeatureService = featureService; SsoConfigRepository = ssoConfigRepository; UserDecryptionOptionsBuilder = userDecryptionOptionsBuilder; @@ -104,12 +85,6 @@ public abstract class BaseRequestValidator where T : class request.UserName, validatorContext.CaptchaResponse.Score); } - var twoFactorToken = request.Raw["TwoFactorToken"]?.ToString(); - var twoFactorProvider = request.Raw["TwoFactorProvider"]?.ToString(); - var twoFactorRemember = request.Raw["TwoFactorRemember"]?.ToString() == "1"; - var twoFactorRequest = !string.IsNullOrWhiteSpace(twoFactorToken) && - !string.IsNullOrWhiteSpace(twoFactorProvider); - var valid = await ValidateContextAsync(context, validatorContext); var user = validatorContext.User; if (!valid) @@ -123,17 +98,37 @@ public abstract class BaseRequestValidator where T : class return; } - var (isTwoFactorRequired, twoFactorOrganization) = await RequiresTwoFactorAsync(user, request); + var (isTwoFactorRequired, twoFactorOrganization) = await _twoFactorAuthenticationValidator.RequiresTwoFactorAsync(user, request); + var twoFactorToken = request.Raw["TwoFactorToken"]?.ToString(); + var twoFactorProvider = request.Raw["TwoFactorProvider"]?.ToString(); + var twoFactorRemember = request.Raw["TwoFactorRemember"]?.ToString() == "1"; + var validTwoFactorRequest = !string.IsNullOrWhiteSpace(twoFactorToken) && + !string.IsNullOrWhiteSpace(twoFactorProvider); + if (isTwoFactorRequired) { - if (!twoFactorRequest || !Enum.TryParse(twoFactorProvider, out TwoFactorProviderType twoFactorProviderType)) + // 2FA required and not provided response + if (!validTwoFactorRequest || + !Enum.TryParse(twoFactorProvider, out TwoFactorProviderType twoFactorProviderType)) { - await BuildTwoFactorResultAsync(user, twoFactorOrganization, context); + var resultDict = await _twoFactorAuthenticationValidator + .BuildTwoFactorResultAsync(user, twoFactorOrganization); + if (resultDict == null) + { + await BuildErrorResultAsync("No two-step providers enabled.", false, context, user); + return; + } + + // Include Master Password Policy in 2FA response + resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user)); + SetTwoFactorResult(context, resultDict); return; } - var verified = await VerifyTwoFactor(user, twoFactorOrganization, - twoFactorProviderType, twoFactorToken); + var verified = await _twoFactorAuthenticationValidator + .VerifyTwoFactor(user, twoFactorOrganization, twoFactorProviderType, twoFactorToken); + + // 2FA required but request not valid or remember token expired response if (!verified || isBot) { if (twoFactorProviderType != TwoFactorProviderType.Remember) @@ -143,16 +138,20 @@ public abstract class BaseRequestValidator where T : class } else if (twoFactorProviderType == TwoFactorProviderType.Remember) { - await BuildTwoFactorResultAsync(user, twoFactorOrganization, context); + var resultDict = await _twoFactorAuthenticationValidator + .BuildTwoFactorResultAsync(user, twoFactorOrganization); + + // Include Master Password Policy in 2FA response + resultDict.Add("MasterPasswordPolicy", await GetMasterPasswordPolicy(user)); + SetTwoFactorResult(context, resultDict); } return; } } else { - twoFactorRequest = false; + validTwoFactorRequest = false; twoFactorRemember = false; - twoFactorToken = null; } // Force legacy users to the web for migration @@ -165,7 +164,6 @@ public abstract class BaseRequestValidator where T : class } } - // Returns true if can finish validation process if (await IsValidAuthTypeAsync(user, request.GrantType)) { var device = await _deviceValidator.SaveDeviceAsync(user, request); @@ -174,8 +172,7 @@ public abstract class BaseRequestValidator where T : class await BuildErrorResultAsync("No device information provided.", false, context, user); return; } - - await BuildSuccessResultAsync(user, context, device, twoFactorRequest && twoFactorRemember); + await BuildSuccessResultAsync(user, context, device, validTwoFactorRequest && twoFactorRemember); } else { @@ -238,67 +235,6 @@ public abstract class BaseRequestValidator where T : class await SetSuccessResult(context, user, claims, customResponse); } - protected async Task BuildTwoFactorResultAsync(User user, Organization organization, T context) - { - var providerKeys = new List(); - var providers = new Dictionary>(); - - var enabledProviders = new List>(); - if (organization?.GetTwoFactorProviders() != null) - { - enabledProviders.AddRange(organization.GetTwoFactorProviders().Where( - p => organization.TwoFactorProviderIsEnabled(p.Key))); - } - - if (user.GetTwoFactorProviders() != null) - { - foreach (var p in user.GetTwoFactorProviders()) - { - if (await _userService.TwoFactorProviderIsEnabledAsync(p.Key, user)) - { - enabledProviders.Add(p); - } - } - } - - if (!enabledProviders.Any()) - { - await BuildErrorResultAsync("No two-step providers enabled.", false, context, user); - return; - } - - foreach (var provider in enabledProviders) - { - providerKeys.Add((byte)provider.Key); - var infoDict = await BuildTwoFactorParams(organization, user, provider.Key, provider.Value); - providers.Add(((byte)provider.Key).ToString(), infoDict); - } - - var twoFactorResultDict = new Dictionary - { - { "TwoFactorProviders", providers.Keys }, - { "TwoFactorProviders2", providers }, - { "MasterPasswordPolicy", await GetMasterPasswordPolicy(user) }, - }; - - // If we have email as a 2FA provider, we might need an SsoEmail2fa Session Token - if (enabledProviders.Any(p => p.Key == TwoFactorProviderType.Email)) - { - twoFactorResultDict.Add("SsoEmail2faSessionToken", - _tokenDataFactory.Protect(new SsoEmail2faSessionTokenable(user))); - - twoFactorResultDict.Add("Email", user.Email); - } - - SetTwoFactorResult(context, twoFactorResultDict); - - if (enabledProviders.Count() == 1 && enabledProviders.First().Key == TwoFactorProviderType.Email) - { - // Send email now if this is their only 2FA method - await _userService.SendTwoFactorEmailAsync(user); - } - } - protected async Task BuildErrorResultAsync(string message, bool twoFactorRequest, T context, User user) { if (user != null) @@ -329,35 +265,13 @@ public abstract class BaseRequestValidator where T : class protected abstract void SetErrorResult(T context, Dictionary customResponse); protected abstract ClaimsPrincipal GetSubject(T context); - protected virtual async Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request) - { - if (request.GrantType == "client_credentials") - { - // Do not require MFA for api key logins - return new Tuple(false, null); - } - - var individualRequired = _userManager.SupportsUserTwoFactor && - await _userManager.GetTwoFactorEnabledAsync(user) && - (await _userManager.GetValidTwoFactorProvidersAsync(user)).Count > 0; - - Organization firstEnabledOrg = null; - var orgs = (await CurrentContext.OrganizationMembershipAsync(_organizationUserRepository, user.Id)).ToList(); - if (orgs.Count > 0) - { - var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); - var twoFactorOrgs = orgs.Where(o => OrgUsing2fa(orgAbilities, o.Id)); - if (twoFactorOrgs.Any()) - { - var userOrgs = await _organizationRepository.GetManyByUserIdAsync(user.Id); - firstEnabledOrg = userOrgs.FirstOrDefault( - o => orgs.Any(om => om.Id == o.Id) && o.TwoFactorIsEnabled()); - } - } - - return new Tuple(individualRequired || firstEnabledOrg != null, firstEnabledOrg); - } - + /// + /// Check if the user is required to authenticate via SSO. If the user requires SSO, but they are + /// logging in using an API Key (client_credentials) then they are allowed to bypass the SSO requirement. + /// + /// user trying to login + /// magic string identifying the grant type requested + /// private async Task IsValidAuthTypeAsync(User user, string grantType) { if (grantType == "authorization_code" || grantType == "client_credentials") @@ -367,7 +281,6 @@ public abstract class BaseRequestValidator where T : class return true; } - // Check if user belongs to any organization with an active SSO policy var anySsoPoliciesApplicableToUser = await PolicyService.AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); if (anySsoPoliciesApplicableToUser) @@ -379,134 +292,6 @@ public abstract class BaseRequestValidator where T : class return true; } - private bool OrgUsing2fa(IDictionary orgAbilities, Guid orgId) - { - return orgAbilities != null && orgAbilities.ContainsKey(orgId) && - orgAbilities[orgId].Enabled && orgAbilities[orgId].Using2fa; - } - - private async Task VerifyTwoFactor(User user, Organization organization, TwoFactorProviderType type, - string token) - { - switch (type) - { - case TwoFactorProviderType.Authenticator: - case TwoFactorProviderType.Email: - case TwoFactorProviderType.Duo: - case TwoFactorProviderType.YubiKey: - case TwoFactorProviderType.WebAuthn: - case TwoFactorProviderType.Remember: - if (type != TwoFactorProviderType.Remember && - !await _userService.TwoFactorProviderIsEnabledAsync(type, user)) - { - return false; - } - // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt - if (FeatureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) - { - if (type == TwoFactorProviderType.Duo) - { - if (!token.Contains(':')) - { - // We have to send the provider to the DuoWebV4SDKService to create the DuoClient - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); - return await _duoWebV4SDKService.ValidateAsync(token, provider, user); - } - } - } - - return await _userManager.VerifyTwoFactorTokenAsync(user, - CoreHelpers.CustomProviderName(type), token); - case TwoFactorProviderType.OrganizationDuo: - if (!organization?.TwoFactorProviderIsEnabled(type) ?? true) - { - return false; - } - - // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt - if (FeatureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) - { - if (type == TwoFactorProviderType.OrganizationDuo) - { - if (!token.Contains(':')) - { - // We have to send the provider to the DuoWebV4SDKService to create the DuoClient - var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); - return await _duoWebV4SDKService.ValidateAsync(token, provider, user); - } - } - } - - return await _organizationDuoWebTokenProvider.ValidateAsync(token, organization, user); - default: - return false; - } - } - - private async Task> BuildTwoFactorParams(Organization organization, User user, - TwoFactorProviderType type, TwoFactorProvider provider) - { - switch (type) - { - case TwoFactorProviderType.Duo: - case TwoFactorProviderType.WebAuthn: - case TwoFactorProviderType.Email: - case TwoFactorProviderType.YubiKey: - if (!await _userService.TwoFactorProviderIsEnabledAsync(type, user)) - { - return null; - } - - var token = await _userManager.GenerateTwoFactorTokenAsync(user, - CoreHelpers.CustomProviderName(type)); - if (type == TwoFactorProviderType.Duo) - { - var duoResponse = new Dictionary - { - ["Host"] = provider.MetaData["Host"], - ["AuthUrl"] = await _duoWebV4SDKService.GenerateAsync(provider, user), - }; - - return duoResponse; - } - else if (type == TwoFactorProviderType.WebAuthn) - { - if (token == null) - { - return null; - } - - return JsonSerializer.Deserialize>(token); - } - else if (type == TwoFactorProviderType.Email) - { - var twoFactorEmail = (string)provider.MetaData["Email"]; - var redactedEmail = CoreHelpers.RedactEmailAddress(twoFactorEmail); - return new Dictionary { ["Email"] = redactedEmail }; - } - else if (type == TwoFactorProviderType.YubiKey) - { - return new Dictionary { ["Nfc"] = (bool)provider.MetaData["Nfc"] }; - } - - return null; - case TwoFactorProviderType.OrganizationDuo: - if (await _organizationDuoWebTokenProvider.CanGenerateTwoFactorTokenAsync(organization)) - { - var duoResponse = new Dictionary - { - ["Host"] = provider.MetaData["Host"], - ["AuthUrl"] = await _duoWebV4SDKService.GenerateAsync(provider, user), - }; - - return duoResponse; - } - return null; - default: - return null; - } - } - private async Task ResetFailedAuthDetailsAsync(User user) { // Early escape if db hit not necessary @@ -546,7 +331,7 @@ public abstract class BaseRequestValidator where T : class } /// - /// checks to see if a user is trying to log into a new device + /// checks to see if a user is trying to log into a new device /// and has reached the maximum number of failed login attempts. /// /// boolean diff --git a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs similarity index 88% rename from src/Identity/IdentityServer/CustomTokenRequestValidator.cs rename to src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs index 0d7a92c8a..c826243f8 100644 --- a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/CustomTokenRequestValidator.cs @@ -1,9 +1,7 @@ using System.Diagnostics; using System.Security.Claims; using Bit.Core.AdminConsole.Services; -using Bit.Core.Auth.Identity; using Bit.Core.Auth.Models.Api.Response; -using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; @@ -11,7 +9,6 @@ using Bit.Core.IdentityServer; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Duende.IdentityServer.Extensions; using Duende.IdentityServer.Validation; using HandlebarsDotNet; @@ -20,7 +17,7 @@ using Microsoft.AspNetCore.Identity; #nullable enable -namespace Bit.Identity.IdentityServer; +namespace Bit.Identity.IdentityServer.RequestValidators; public class CustomTokenRequestValidator : BaseRequestValidator, ICustomTokenRequestValidator @@ -29,28 +26,36 @@ public class CustomTokenRequestValidator : BaseRequestValidator userManager, - IDeviceValidator deviceValidator, IUserService userService, IEventService eventService, - IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, - ITemporaryDuoWebV4SDKService duoWebV4SDKService, - IOrganizationRepository organizationRepository, + IDeviceValidator deviceValidator, + ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator, IOrganizationUserRepository organizationUserRepository, - IApplicationCacheService applicationCacheService, IMailService mailService, ILogger logger, ICurrentContext currentContext, GlobalSettings globalSettings, - ISsoConfigRepository ssoConfigRepository, IUserRepository userRepository, IPolicyService policyService, - IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, - IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) - : base(userManager, userService, eventService, deviceValidator, - organizationDuoWebTokenProvider, duoWebV4SDKService, organizationRepository, organizationUserRepository, - applicationCacheService, mailService, logger, currentContext, globalSettings, - userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, + ISsoConfigRepository ssoConfigRepository, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder + ) + : base( + userManager, + userService, + eventService, + deviceValidator, + twoFactorAuthenticationValidator, + organizationUserRepository, + mailService, + logger, + currentContext, + globalSettings, + userRepository, + policyService, + featureService, + ssoConfigRepository, userDecryptionOptionsBuilder) { _userManager = userManager; @@ -70,7 +75,7 @@ public class CustomTokenRequestValidator : BaseRequestValidator + /// Save a device to the database. If the device is already known, it will be returned. + /// + /// The user is assumed NOT null, still going to check though + /// Duende Validated Request that contains the data to create the device object + /// Returns null if user or device is malformed; The existing device if already in DB; a new device login public async Task SaveDeviceAsync(User user, ValidatedTokenRequest request) { var device = GetDeviceFromRequest(request); diff --git a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs similarity index 89% rename from src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs rename to src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs index 08560e240..f072a6417 100644 --- a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/ResourceOwnerPasswordValidator.cs @@ -1,8 +1,6 @@ using System.Security.Claims; using Bit.Core; using Bit.Core.AdminConsole.Services; -using Bit.Core.Auth.Identity; -using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; using Bit.Core.Context; @@ -10,13 +8,12 @@ using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Bit.Core.Utilities; using Duende.IdentityServer.Models; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Identity; -namespace Bit.Identity.IdentityServer; +namespace Bit.Identity.IdentityServer.RequestValidators; public class ResourceOwnerPasswordValidator : BaseRequestValidator, IResourceOwnerPasswordValidator @@ -31,11 +28,8 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator logger, ICurrentContext currentContext, @@ -44,14 +38,25 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator tokenDataFactory, IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) - : base(userManager, userService, eventService, deviceValidator, - organizationDuoWebTokenProvider, duoWebV4SDKService, organizationRepository, organizationUserRepository, - applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, - tokenDataFactory, featureService, ssoConfigRepository, userDecryptionOptionsBuilder) + : base( + userManager, + userService, + eventService, + deviceValidator, + twoFactorAuthenticationValidator, + organizationUserRepository, + mailService, + logger, + currentContext, + globalSettings, + userRepository, + policyService, + featureService, + ssoConfigRepository, + userDecryptionOptionsBuilder) { _userManager = userManager; _currentContext = currentContext; diff --git a/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs new file mode 100644 index 000000000..323d09c0e --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/TwoFactorAuthenticationValidator.cs @@ -0,0 +1,297 @@ + +using System.Text.Json; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Tokens; +using Bit.Core.Utilities; +using Duende.IdentityServer.Validation; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Identity.IdentityServer.RequestValidators; + +public interface ITwoFactorAuthenticationValidator +{ + /// + /// Check if the user is required to use two-factor authentication to login. This is based on the user's + /// enabled two-factor providers, the user's organizations enabled two-factor providers, and the grant type. + /// Client credentials and webauthn grant types do not require two-factor authentication. + /// + /// the active user for the request + /// the request that contains the grant types + /// boolean + Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request); + /// + /// Builds the two-factor authentication result for the user based on the available two-factor providers + /// from either their user account or Organization. + /// + /// user trying to login + /// organization associated with the user; Can be null + /// Dictionary with the TwoFactorProviderType as the Key and the Provider Metadata as the Value + Task> BuildTwoFactorResultAsync(User user, Organization organization); + /// + /// Uses the built in userManager methods to verify the two-factor token for the user. If the organization uses + /// organization duo, it will use the organization duo token provider to verify the token. + /// + /// the active User + /// organization of user; can be null + /// Two Factor Provider to use to verify the token + /// secret passed from the user and consumed by the two-factor provider's verify method + /// boolean + Task VerifyTwoFactor(User user, Organization organization, TwoFactorProviderType twoFactorProviderType, string token); +} + +public class TwoFactorAuthenticationValidator( + IUserService userService, + UserManager userManager, + IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, + ITemporaryDuoWebV4SDKService duoWebV4SDKService, + IFeatureService featureService, + IApplicationCacheService applicationCacheService, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository, + IDataProtectorTokenFactory ssoEmail2faSessionTokeFactory, + ICurrentContext currentContext) : ITwoFactorAuthenticationValidator +{ + private readonly IUserService _userService = userService; + private readonly UserManager _userManager = userManager; + private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider = organizationDuoWebTokenProvider; + private readonly ITemporaryDuoWebV4SDKService _duoWebV4SDKService = duoWebV4SDKService; + private readonly IFeatureService _featureService = featureService; + private readonly IApplicationCacheService _applicationCacheService = applicationCacheService; + private readonly IOrganizationUserRepository _organizationUserRepository = organizationUserRepository; + private readonly IOrganizationRepository _organizationRepository = organizationRepository; + private readonly IDataProtectorTokenFactory _ssoEmail2faSessionTokeFactory = ssoEmail2faSessionTokeFactory; + private readonly ICurrentContext _currentContext = currentContext; + + public async Task> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request) + { + if (request.GrantType == "client_credentials" || request.GrantType == "webauthn") + { + /* + Do not require MFA for api key logins. + We consider Fido2 userVerification a second factor, so we don't require a second factor here. + */ + return new Tuple(false, null); + } + + var individualRequired = _userManager.SupportsUserTwoFactor && + await _userManager.GetTwoFactorEnabledAsync(user) && + (await _userManager.GetValidTwoFactorProvidersAsync(user)).Count > 0; + + Organization firstEnabledOrg = null; + var orgs = (await _currentContext.OrganizationMembershipAsync(_organizationUserRepository, user.Id)).ToList(); + if (orgs.Count > 0) + { + var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); + var twoFactorOrgs = orgs.Where(o => OrgUsing2fa(orgAbilities, o.Id)); + if (twoFactorOrgs.Any()) + { + var userOrgs = await _organizationRepository.GetManyByUserIdAsync(user.Id); + firstEnabledOrg = userOrgs.FirstOrDefault( + o => orgs.Any(om => om.Id == o.Id) && o.TwoFactorIsEnabled()); + } + } + + return new Tuple(individualRequired || firstEnabledOrg != null, firstEnabledOrg); + } + + public async Task> BuildTwoFactorResultAsync(User user, Organization organization) + { + var enabledProviders = await GetEnabledTwoFactorProvidersAsync(user, organization); + if (enabledProviders.Count == 0) + { + return null; + } + + var providers = new Dictionary>(); + foreach (var provider in enabledProviders) + { + var twoFactorParams = await BuildTwoFactorParams(organization, user, provider.Key, provider.Value); + providers.Add(((byte)provider.Key).ToString(), twoFactorParams); + } + + var twoFactorResultDict = new Dictionary + { + { "TwoFactorProviders", null }, + { "TwoFactorProviders2", providers }, // backwards compatibility + }; + + // If we have email as a 2FA provider, we might need an SsoEmail2fa Session Token + if (enabledProviders.Any(p => p.Key == TwoFactorProviderType.Email)) + { + twoFactorResultDict.Add("SsoEmail2faSessionToken", + _ssoEmail2faSessionTokeFactory.Protect(new SsoEmail2faSessionTokenable(user))); + + twoFactorResultDict.Add("Email", user.Email); + } + + if (enabledProviders.Count == 1 && enabledProviders.First().Key == TwoFactorProviderType.Email) + { + // Send email now if this is their only 2FA method + await _userService.SendTwoFactorEmailAsync(user); + } + + return twoFactorResultDict; + } + + public async Task VerifyTwoFactor( + User user, + Organization organization, + TwoFactorProviderType type, + string token) + { + if (organization != null && type == TwoFactorProviderType.OrganizationDuo) + { + if (organization.TwoFactorProviderIsEnabled(type)) + { + // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt + if (_featureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) + { + if (!token.Contains(':')) + { + // We have to send the provider to the DuoWebV4SDKService to create the DuoClient + var provider = organization.GetTwoFactorProvider(TwoFactorProviderType.OrganizationDuo); + return await _duoWebV4SDKService.ValidateAsync(token, provider, user); + } + } + return await _organizationDuoWebTokenProvider.ValidateAsync(token, organization, user); + } + return false; + } + + switch (type) + { + case TwoFactorProviderType.Authenticator: + case TwoFactorProviderType.Email: + case TwoFactorProviderType.Duo: + case TwoFactorProviderType.YubiKey: + case TwoFactorProviderType.WebAuthn: + case TwoFactorProviderType.Remember: + if (type != TwoFactorProviderType.Remember && + !await _userService.TwoFactorProviderIsEnabledAsync(type, user)) + { + return false; + } + // DUO SDK v4 Update: try to validate the token - PM-5156 addresses tech debt + if (_featureService.IsEnabled(FeatureFlagKeys.DuoRedirect)) + { + if (type == TwoFactorProviderType.Duo) + { + if (!token.Contains(':')) + { + // We have to send the provider to the DuoWebV4SDKService to create the DuoClient + var provider = user.GetTwoFactorProvider(TwoFactorProviderType.Duo); + return await _duoWebV4SDKService.ValidateAsync(token, provider, user); + } + } + } + return await _userManager.VerifyTwoFactorTokenAsync(user, + CoreHelpers.CustomProviderName(type), token); + default: + return false; + } + } + + private async Task>> GetEnabledTwoFactorProvidersAsync( + User user, Organization organization) + { + var enabledProviders = new List>(); + var organizationTwoFactorProviders = organization?.GetTwoFactorProviders(); + if (organizationTwoFactorProviders != null) + { + enabledProviders.AddRange( + organizationTwoFactorProviders.Where( + p => (p.Value?.Enabled ?? false) && organization.Use2fa)); + } + + var userTwoFactorProviders = user.GetTwoFactorProviders(); + var userCanAccessPremium = await _userService.CanAccessPremium(user); + if (userTwoFactorProviders != null) + { + enabledProviders.AddRange( + userTwoFactorProviders.Where(p => + // Providers that do not require premium + (p.Value.Enabled && !TwoFactorProvider.RequiresPremium(p.Key)) || + // Providers that require premium and the User has Premium + (p.Value.Enabled && TwoFactorProvider.RequiresPremium(p.Key) && userCanAccessPremium))); + } + + return enabledProviders; + } + + /// + /// Builds the parameters for the two-factor authentication + /// + /// We need the organization for Organization Duo Provider type + /// The user for which the token is being generated + /// Provider Type + /// Raw data that is used to create the response + /// a dictionary with the correct provider configuration or null if the provider is not configured properly + private async Task> BuildTwoFactorParams(Organization organization, User user, + TwoFactorProviderType type, TwoFactorProvider provider) + { + // We will always return this dictionary. If none of the criteria is met then it will return null. + var twoFactorParams = new Dictionary(); + + // OrganizationDuo is odd since it doesn't use the UserManager built-in TwoFactor flows + /* + Note: Duo is in the midst of being updated to use the UserManager built-in TwoFactor class + in the future the `AuthUrl` will be the generated "token" - PM-8107 + */ + if (type == TwoFactorProviderType.OrganizationDuo && + await _organizationDuoWebTokenProvider.CanGenerateTwoFactorTokenAsync(organization)) + { + twoFactorParams.Add("Host", provider.MetaData["Host"]); + twoFactorParams.Add("AuthUrl", await _duoWebV4SDKService.GenerateAsync(provider, user)); + + return twoFactorParams; + } + + // Individual 2FA providers use the UserManager built-in TwoFactor flow so we can generate the token before building the params + var token = await _userManager.GenerateTwoFactorTokenAsync(user, + CoreHelpers.CustomProviderName(type)); + switch (type) + { + /* + Note: Duo is in the midst of being updated to use the UserManager built-in TwoFactor class + in the future the `AuthUrl` will be the generated "token" - PM-8107 + */ + case TwoFactorProviderType.Duo: + twoFactorParams.Add("Host", provider.MetaData["Host"]); + twoFactorParams.Add("AuthUrl", await _duoWebV4SDKService.GenerateAsync(provider, user)); + break; + case TwoFactorProviderType.WebAuthn: + if (token != null) + { + twoFactorParams = JsonSerializer.Deserialize>(token); + } + break; + case TwoFactorProviderType.Email: + var twoFactorEmail = (string)provider.MetaData["Email"]; + var redactedEmail = CoreHelpers.RedactEmailAddress(twoFactorEmail); + twoFactorParams.Add("Email", redactedEmail); + break; + case TwoFactorProviderType.YubiKey: + twoFactorParams.Add("Nfc", (bool)provider.MetaData["Nfc"]); + break; + } + + // return null if the dictionary is empty + return twoFactorParams.Count > 0 ? twoFactorParams : null; + } + + private bool OrgUsing2fa(IDictionary orgAbilities, Guid orgId) + { + return orgAbilities != null && orgAbilities.ContainsKey(orgId) && + orgAbilities[orgId].Enabled && orgAbilities[orgId].Using2fa; + } +} diff --git a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs similarity index 82% rename from src/Identity/IdentityServer/WebAuthnGrantValidator.cs rename to src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs index 7bf90c756..515dca782 100644 --- a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/WebAuthnGrantValidator.cs @@ -1,10 +1,8 @@ using System.Security.Claims; using System.Text.Json; using Bit.Core; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Identity; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.WebAuthnLogin; @@ -19,7 +17,7 @@ using Duende.IdentityServer.Validation; using Fido2NetLib; using Microsoft.AspNetCore.Identity; -namespace Bit.Identity.IdentityServer; +namespace Bit.Identity.IdentityServer.RequestValidators; public class WebAuthnGrantValidator : BaseRequestValidator, IExtensionGrantValidator { @@ -34,11 +32,8 @@ public class WebAuthnGrantValidator : BaseRequestValidator logger, ICurrentContext currentContext, @@ -46,16 +41,27 @@ public class WebAuthnGrantValidator : BaseRequestValidator tokenDataFactory, IDataProtectorTokenFactory assertionOptionsDataProtector, IFeatureService featureService, IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder, IAssertWebAuthnLoginCredentialCommand assertWebAuthnLoginCredentialCommand ) - : base(userManager, userService, eventService, deviceValidator, - organizationDuoWebTokenProvider, duoWebV4SDKService, organizationRepository, organizationUserRepository, - applicationCacheService, mailService, logger, currentContext, globalSettings, - userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, userDecryptionOptionsBuilder) + : base( + userManager, + userService, + eventService, + deviceValidator, + twoFactorAuthenticationValidator, + organizationUserRepository, + mailService, + logger, + currentContext, + globalSettings, + userRepository, + policyService, + featureService, + ssoConfigRepository, + userDecryptionOptionsBuilder) { _assertionOptionsDataProtector = assertionOptionsDataProtector; _assertWebAuthnLoginCredentialCommand = assertWebAuthnLoginCredentialCommand; @@ -122,12 +128,6 @@ public class WebAuthnGrantValidator : BaseRequestValidator> RequiresTwoFactorAsync(User user, ValidatedTokenRequest request) - { - // We consider Fido2 userVerification a second factor, so we don't require a second factor here. - return Task.FromResult(new Tuple(false, null)); - } - protected override void SetTwoFactorResult(ExtensionGrantValidationContext context, Dictionary customResponse) { diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index 43532cb3f..36c38615a 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -3,6 +3,7 @@ using Bit.Core.IdentityServer; using Bit.Core.Settings; using Bit.Core.Utilities; using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.SharedWeb.Utilities; using Duende.IdentityServer.ResponseHandling; using Duende.IdentityServer.Services; @@ -21,6 +22,7 @@ public static class ServiceCollectionExtensions services.AddTransient(); services.AddTransient(); services.AddTransient(); + services.AddTransient(); var issuerUri = new Uri(globalSettings.BaseServiceUri.InternalIdentity); var identityServerBuilder = services diff --git a/test/Identity.IntegrationTest/RequestValidation/ResourceOwnerPasswordValidatorTests.cs b/test/Identity.IntegrationTest/RequestValidation/ResourceOwnerPasswordValidatorTests.cs index 91d0ee01f..703faed48 100644 --- a/test/Identity.IntegrationTest/RequestValidation/ResourceOwnerPasswordValidatorTests.cs +++ b/test/Identity.IntegrationTest/RequestValidation/ResourceOwnerPasswordValidatorTests.cs @@ -5,7 +5,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.Identity.Models.Request.Accounts; using Bit.IntegrationTestCommon.Factories; using Bit.Test.Common.AutoFixture.Attributes; @@ -237,6 +237,11 @@ public class ResourceOwnerPasswordValidatorTests : IClassFixture>(); + await factory.RegisterAsync(new RegisterRequestModel + { + Email = DefaultUsername, + MasterPasswordHash = DefaultPassword + }); var user = await userManager.FindByEmailAsync(DefaultUsername); Assert.NotNull(user); diff --git a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs index 39b7edf8d..d0372202a 100644 --- a/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/BaseRequestValidatorTests.cs @@ -1,8 +1,7 @@ using Bit.Core; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Services; -using Bit.Core.Auth.Identity; -using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; @@ -11,8 +10,8 @@ using Bit.Core.Models.Api; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.Identity.Test.Wrappers; using Bit.Test.Common.AutoFixture.Attributes; using Duende.IdentityServer.Validation; @@ -32,18 +31,14 @@ public class BaseRequestValidatorTests private readonly IUserService _userService; private readonly IEventService _eventService; private readonly IDeviceValidator _deviceValidator; - private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider; - private readonly ITemporaryDuoWebV4SDKService _duoWebV4SDKService; - private readonly IOrganizationRepository _organizationRepository; + private readonly ITwoFactorAuthenticationValidator _twoFactorAuthenticationValidator; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IApplicationCacheService _applicationCacheService; private readonly IMailService _mailService; private readonly ILogger _logger; private readonly ICurrentContext _currentContext; private readonly GlobalSettings _globalSettings; private readonly IUserRepository _userRepository; private readonly IPolicyService _policyService; - private readonly IDataProtectorTokenFactory _tokenDataFactory; private readonly IFeatureService _featureService; private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IUserDecryptionOptionsBuilder _userDecryptionOptionsBuilder; @@ -52,43 +47,35 @@ public class BaseRequestValidatorTests public BaseRequestValidatorTests() { + _userManager = SubstituteUserManager(); _userService = Substitute.For(); _eventService = Substitute.For(); _deviceValidator = Substitute.For(); - _organizationDuoWebTokenProvider = Substitute.For(); - _duoWebV4SDKService = Substitute.For(); - _organizationRepository = Substitute.For(); + _twoFactorAuthenticationValidator = Substitute.For(); _organizationUserRepository = Substitute.For(); - _applicationCacheService = Substitute.For(); _mailService = Substitute.For(); _logger = Substitute.For>(); _currentContext = Substitute.For(); _globalSettings = Substitute.For(); _userRepository = Substitute.For(); _policyService = Substitute.For(); - _tokenDataFactory = Substitute.For>(); _featureService = Substitute.For(); _ssoConfigRepository = Substitute.For(); _userDecryptionOptionsBuilder = Substitute.For(); - _userManager = SubstituteUserManager(); _sut = new BaseRequestValidatorTestWrapper( _userManager, _userService, _eventService, _deviceValidator, - _organizationDuoWebTokenProvider, - _duoWebV4SDKService, - _organizationRepository, + _twoFactorAuthenticationValidator, _organizationUserRepository, - _applicationCacheService, _mailService, _logger, _currentContext, _globalSettings, _userRepository, _policyService, - _tokenDataFactory, _featureService, _ssoConfigRepository, _userDecryptionOptionsBuilder); @@ -116,7 +103,7 @@ public class BaseRequestValidatorTests var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; - // Assert + // Assert await _eventService.Received(1) .LogUserEventAsync(context.CustomValidatorRequestContext.User.Id, Core.Enums.EventType.User_FailedLogIn); @@ -127,7 +114,7 @@ public class BaseRequestValidatorTests /* Logic path ValidateAsync -> UpdateFailedAuthDetailsAsync -> _mailService.SendFailedLoginAttemptsEmailAsync |-> BuildErrorResultAsync -> _eventService.LogUserEventAsync - (self hosted) |-> _logger.LogWarning() + (self hosted) |-> _logger.LogWarning() |-> SetErrorResult */ [Theory, BitAutoData] @@ -154,7 +141,7 @@ public class BaseRequestValidatorTests /* Logic path ValidateAsync -> UpdateFailedAuthDetailsAsync -> _mailService.SendFailedLoginAttemptsEmailAsync - |-> BuildErrorResultAsync -> _eventService.LogUserEventAsync + |-> BuildErrorResultAsync -> _eventService.LogUserEventAsync |-> SetErrorResult */ [Theory, BitAutoData] @@ -202,6 +189,9 @@ public class BaseRequestValidatorTests { // Arrange var context = CreateContext(tokenRequest, requestContext, grantResult); + _twoFactorAuthenticationValidator + .RequiresTwoFactorAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new Tuple(false, default))); context.CustomValidatorRequestContext.CaptchaResponse.IsBot = false; _sut.isValid = true; @@ -230,6 +220,9 @@ public class BaseRequestValidatorTests { // Arrange var context = CreateContext(tokenRequest, requestContext, grantResult); + _twoFactorAuthenticationValidator + .RequiresTwoFactorAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new Tuple(false, null))); context.CustomValidatorRequestContext.CaptchaResponse.IsBot = false; _sut.isValid = true; @@ -237,7 +230,7 @@ public class BaseRequestValidatorTests context.CustomValidatorRequestContext.User.CreationDate = DateTime.UtcNow - TimeSpan.FromDays(1); _globalSettings.DisableEmailNewDevice = false; - context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device + context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device _deviceValidator.SaveDeviceAsync(Arg.Any(), Arg.Any()) .Returns(device); @@ -267,10 +260,13 @@ public class BaseRequestValidatorTests context.CustomValidatorRequestContext.User.CreationDate = DateTime.UtcNow - TimeSpan.FromDays(1); _globalSettings.DisableEmailNewDevice = false; - context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device + context.ValidatedTokenRequest.GrantType = "client_credentials"; // This || AuthCode will allow process to continue to get device _deviceValidator.SaveDeviceAsync(Arg.Any(), Arg.Any()) .Returns(device); + _twoFactorAuthenticationValidator + .RequiresTwoFactorAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new Tuple(false, null))); // Act await _sut.ValidateAsync(context); @@ -306,10 +302,13 @@ public class BaseRequestValidatorTests _policyService.AnyPoliciesApplicableToUserAsync( Arg.Any(), PolicyType.RequireSso, OrganizationUserStatusType.Confirmed) .Returns(Task.FromResult(true)); + _twoFactorAuthenticationValidator + .RequiresTwoFactorAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new Tuple(false, null))); // Act await _sut.ValidateAsync(context); - // Assert + // Assert Assert.True(context.GrantResult.IsError); var errorResponse = (ErrorResponseModel)context.GrantResult.CustomResponse["ErrorModel"]; Assert.Equal("SSO authentication is required.", errorResponse.Message); @@ -330,6 +329,9 @@ public class BaseRequestValidatorTests context.ValidatedTokenRequest.ClientId = "Not Web"; _sut.isValid = true; _featureService.IsEnabled(FeatureFlagKeys.BlockLegacyUsers).Returns(true); + _twoFactorAuthenticationValidator + .RequiresTwoFactorAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new Tuple(false, null))); // Act await _sut.ValidateAsync(context); @@ -341,28 +343,6 @@ public class BaseRequestValidatorTests , errorResponse.Message); } - [Theory, BitAutoData] - public async Task RequiresTwoFactorAsync_ClientCredentialsGrantType_ShouldReturnFalse( - [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, - CustomValidatorRequestContext requestContext, - GrantValidationResult grantResult) - { - // Arrange - var context = CreateContext(tokenRequest, requestContext, grantResult); - - context.CustomValidatorRequestContext.CaptchaResponse.IsBot = false; - context.ValidatedTokenRequest.GrantType = "client_credentials"; - - // Act - var result = await _sut.TestRequiresTwoFactorAsync( - context.CustomValidatorRequestContext.User, - context.ValidatedTokenRequest); - - // Assert - Assert.False(result.Item1); - Assert.Null(result.Item2); - } - private BaseRequestValidationContextFake CreateContext( ValidatedTokenRequest tokenRequest, CustomValidatorRequestContext requestContext, diff --git a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs index 1f4d5a807..2db792c93 100644 --- a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs @@ -4,7 +4,7 @@ using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Bit.Test.Common.AutoFixture.Attributes; using Duende.IdentityServer.Validation; using NSubstitute; diff --git a/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs new file mode 100644 index 000000000..5783375ff --- /dev/null +++ b/test/Identity.Test/IdentityServer/TwoFactorAuthenticationValidatorTests.cs @@ -0,0 +1,575 @@ +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Tokens; +using Bit.Identity.IdentityServer.RequestValidators; +using Bit.Identity.Test.Wrappers; +using Bit.Test.Common.AutoFixture.Attributes; +using Duende.IdentityServer.Validation; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using NSubstitute; +using Xunit; +using AuthFixtures = Bit.Identity.Test.AutoFixture; + +namespace Bit.Identity.Test.IdentityServer; + +public class TwoFactorAuthenticationValidatorTests +{ + private readonly IUserService _userService; + private readonly UserManagerTestWrapper _userManager; + private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider; + private readonly ITemporaryDuoWebV4SDKService _temporaryDuoWebV4SDKService; + private readonly IFeatureService _featureService; + private readonly IApplicationCacheService _applicationCacheService; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IOrganizationRepository _organizationRepository; + private readonly IDataProtectorTokenFactory _ssoEmail2faSessionTokenable; + private readonly ICurrentContext _currentContext; + private readonly TwoFactorAuthenticationValidator _sut; + + public TwoFactorAuthenticationValidatorTests() + { + _userService = Substitute.For(); + _userManager = SubstituteUserManager(); + _organizationDuoWebTokenProvider = Substitute.For(); + _temporaryDuoWebV4SDKService = Substitute.For(); + _featureService = Substitute.For(); + _applicationCacheService = Substitute.For(); + _organizationUserRepository = Substitute.For(); + _organizationRepository = Substitute.For(); + _ssoEmail2faSessionTokenable = Substitute.For>(); + _currentContext = Substitute.For(); + + _sut = new TwoFactorAuthenticationValidator( + _userService, + _userManager, + _organizationDuoWebTokenProvider, + _temporaryDuoWebV4SDKService, + _featureService, + _applicationCacheService, + _organizationUserRepository, + _organizationRepository, + _ssoEmail2faSessionTokenable, + _currentContext); + } + + [Theory] + [BitAutoData("password")] + [BitAutoData("authorization_code")] + public async void RequiresTwoFactorAsync_IndividualOnly_Required_ReturnTrue( + string grantType, + [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request, + User user) + { + // Arrange + request.GrantType = grantType; + // All three of these must be true for the two factor authentication to be required + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.SUPPORTS_TWO_FACTOR = true; + // In order for the two factor authentication to be required, the user must have at least one two factor provider + _userManager.TWO_FACTOR_PROVIDERS = ["email"]; + + // Act + var result = await _sut.RequiresTwoFactorAsync(user, request); + + // Assert + Assert.True(result.Item1); + Assert.Null(result.Item2); + } + + [Theory] + [BitAutoData("client_credentials")] + [BitAutoData("webauthn")] + public async void RequiresTwoFactorAsync_NotRequired_ReturnFalse( + string grantType, + [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request, + User user) + { + // Arrange + request.GrantType = grantType; + + // Act + var result = await _sut.RequiresTwoFactorAsync(user, request); + + // Assert + Assert.False(result.Item1); + Assert.Null(result.Item2); + } + + [Theory] + [BitAutoData("password")] + [BitAutoData("authorization_code")] + public async void RequiresTwoFactorAsync_IndividualFalse_OrganizationRequired_ReturnTrue( + string grantType, + [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request, + User user, + OrganizationUserOrganizationDetails orgUser, + Organization organization, + ICollection organizationCollection) + { + // Arrange + request.GrantType = grantType; + // Link the orgUser to the User making the request + orgUser.UserId = user.Id; + // Link organization to the organization user + organization.Id = orgUser.OrganizationId; + + // Set Organization 2FA to required + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + + // Make sure organization list is not empty + organizationCollection.Clear(); + // Fix OrganizationUser Permissions field + orgUser.Permissions = "{}"; + organizationCollection.Add(new CurrentContextOrganization(orgUser)); + + _currentContext.OrganizationMembershipAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(organizationCollection)); + + _applicationCacheService.GetOrganizationAbilitiesAsync() + .Returns(new Dictionary() + { + { orgUser.OrganizationId, new OrganizationAbility(organization)} + }); + + _organizationRepository.GetManyByUserIdAsync(Arg.Any()).Returns([organization]); + + // Act + var result = await _sut.RequiresTwoFactorAsync(user, request); + + // Assert + Assert.True(result.Item1); + Assert.NotNull(result.Item2); + Assert.IsType(result.Item2); + } + + [Theory] + [BitAutoData] + public async void BuildTwoFactorResultAsync_NoProviders_ReturnsNull( + User user, + Organization organization) + { + // Arrange + organization.Use2fa = true; + organization.TwoFactorProviders = "{}"; + organization.Enabled = true; + + user.TwoFactorProviders = ""; + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, organization); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async void BuildTwoFactorResultAsync_OrganizationProviders_NotEnabled_ReturnsNull( + User user, + Organization organization) + { + // Arrange + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationNotEnabledDuoProviderJson(); + organization.Enabled = true; + + user.TwoFactorProviders = null; + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, organization); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async void BuildTwoFactorResultAsync_OrganizationProviders_ReturnsNotNull( + User user, + Organization organization) + { + // Arrange + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + + user.TwoFactorProviders = null; + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, organization); + + // Assert + Assert.NotNull(result); + Assert.IsType>(result); + Assert.NotEmpty(result); + Assert.True(result.ContainsKey("TwoFactorProviders2")); + } + + [Theory] + [BitAutoData] + public async void BuildTwoFactorResultAsync_IndividualProviders_NotEnabled_ReturnsNull( + User user) + { + // Arrange + user.TwoFactorProviders = GetTwoFactorIndividualNotEnabledProviderJson(TwoFactorProviderType.Email); + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, null); + + // Assert + Assert.Null(result); + } + + [Theory] + [BitAutoData] + public async void BuildTwoFactorResultAsync_IndividualProviders_ReturnsNotNull( + User user) + { + // Arrange + _userService.CanAccessPremium(user).Returns(true); + + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(TwoFactorProviderType.Duo); + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, null); + + // Assert + Assert.NotNull(result); + Assert.IsType>(result); + Assert.NotEmpty(result); + Assert.True(result.ContainsKey("TwoFactorProviders2")); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Email)] + public async void BuildTwoFactorResultAsync_IndividualEmailProvider_SendsEmail_SetsSsoToken_ReturnsNotNull( + TwoFactorProviderType providerType, + User user) + { + // Arrange + var providerTypeInt = (int)providerType; + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.SUPPORTS_TWO_FACTOR = true; + _userManager.TWO_FACTOR_PROVIDERS = [providerType.ToString()]; + + _userService.TwoFactorProviderIsEnabledAsync(Arg.Any(), user) + .Returns(true); + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, null); + + // Assert + Assert.NotNull(result); + Assert.IsType>(result); + Assert.NotEmpty(result); + Assert.True(result.ContainsKey("TwoFactorProviders2")); + var providers = (Dictionary>)result["TwoFactorProviders2"]; + Assert.True(providers.ContainsKey(providerTypeInt.ToString())); + Assert.True(result.ContainsKey("SsoEmail2faSessionToken")); + Assert.True(result.ContainsKey("Email")); + + await _userService.Received(1).SendTwoFactorEmailAsync(Arg.Any()); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Duo)] + [BitAutoData(TwoFactorProviderType.WebAuthn)] + [BitAutoData(TwoFactorProviderType.Email)] + [BitAutoData(TwoFactorProviderType.YubiKey)] + [BitAutoData(TwoFactorProviderType.OrganizationDuo)] + public async void BuildTwoFactorResultAsync_IndividualProvider_ReturnMatchesType( + TwoFactorProviderType providerType, + User user) + { + // Arrange + var providerTypeInt = (int)providerType; + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.SUPPORTS_TWO_FACTOR = true; + _userManager.TWO_FACTOR_PROVIDERS = [providerType.ToString()]; + _userManager.TWO_FACTOR_TOKEN = "{\"Key1\":\"WebauthnToken\"}"; + + _userService.CanAccessPremium(user).Returns(true); + + // Act + var result = await _sut.BuildTwoFactorResultAsync(user, null); + + // Assert + Assert.NotNull(result); + Assert.IsType>(result); + Assert.NotEmpty(result); + Assert.True(result.ContainsKey("TwoFactorProviders2")); + var providers = (Dictionary>)result["TwoFactorProviders2"]; + Assert.True(providers.ContainsKey(providerTypeInt.ToString())); + } + + [Theory] + [BitAutoData] + public async void VerifyTwoFactorAsync_Individual_TypeNull_ReturnsFalse( + User user, + string token) + { + // Arrange + _userService.TwoFactorProviderIsEnabledAsync( + TwoFactorProviderType.Email, user).Returns(true); + + _userManager.TWO_FACTOR_PROVIDERS = ["email"]; + + // Act + var result = await _sut.VerifyTwoFactor( + user, null, TwoFactorProviderType.U2f, token); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async void VerifyTwoFactorAsync_Individual_NotEnabled_ReturnsFalse( + User user, + string token) + { + // Arrange + _userService.TwoFactorProviderIsEnabledAsync( + TwoFactorProviderType.Email, user).Returns(false); + + _userManager.TWO_FACTOR_PROVIDERS = ["email"]; + + // Act + var result = await _sut.VerifyTwoFactor( + user, null, TwoFactorProviderType.Email, token); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData] + public async void VerifyTwoFactorAsync_Organization_NotEnabled_ReturnsFalse( + User user, + string token) + { + // Arrange + _userService.TwoFactorProviderIsEnabledAsync( + TwoFactorProviderType.OrganizationDuo, user).Returns(false); + + _userManager.TWO_FACTOR_PROVIDERS = ["OrganizationDuo"]; + + // Act + var result = await _sut.VerifyTwoFactor( + user, null, TwoFactorProviderType.OrganizationDuo, token); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Duo)] + [BitAutoData(TwoFactorProviderType.WebAuthn)] + [BitAutoData(TwoFactorProviderType.Email)] + [BitAutoData(TwoFactorProviderType.YubiKey)] + [BitAutoData(TwoFactorProviderType.Remember)] + public async void VerifyTwoFactorAsync_Individual_ValidToken_ReturnsTrue( + TwoFactorProviderType providerType, + User user, + string token) + { + // Arrange + _userService.TwoFactorProviderIsEnabledAsync( + providerType, user).Returns(true); + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactor(user, null, providerType, token); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Duo)] + [BitAutoData(TwoFactorProviderType.WebAuthn)] + [BitAutoData(TwoFactorProviderType.Email)] + [BitAutoData(TwoFactorProviderType.YubiKey)] + [BitAutoData(TwoFactorProviderType.Remember)] + public async void VerifyTwoFactorAsync_Individual_InvalidToken_ReturnsFalse( + TwoFactorProviderType providerType, + User user, + string token) + { + // Arrange + _userService.TwoFactorProviderIsEnabledAsync( + providerType, user).Returns(true); + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; + + // Act + var result = await _sut.VerifyTwoFactor(user, null, providerType, token); + + // Assert + Assert.False(result); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.OrganizationDuo)] + public async void VerifyTwoFactorAsync_Organization_ValidToken_ReturnsTrue( + TwoFactorProviderType providerType, + User user, + Organization organization, + string token) + { + // Arrange + _organizationDuoWebTokenProvider.ValidateAsync( + token, organization, user).Returns(true); + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + + // Act + var result = await _sut.VerifyTwoFactor( + user, organization, providerType, token); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Duo)] + [BitAutoData(TwoFactorProviderType.OrganizationDuo)] + public async void VerifyTwoFactorAsync_TemporaryDuoService_ValidToken_ReturnsTrue( + TwoFactorProviderType providerType, + User user, + Organization organization, + string token) + { + // Arrange + _featureService.IsEnabled(FeatureFlagKeys.DuoRedirect).Returns(true); + _userService.TwoFactorProviderIsEnabledAsync(providerType, user).Returns(true); + _temporaryDuoWebV4SDKService.ValidateAsync( + token, Arg.Any(), user).Returns(true); + + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.TWO_FACTOR_TOKEN = token; + _userManager.TWO_FACTOR_TOKEN_VERIFIED = true; + + // Act + var result = await _sut.VerifyTwoFactor( + user, organization, providerType, token); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData(TwoFactorProviderType.Duo)] + [BitAutoData(TwoFactorProviderType.OrganizationDuo)] + public async void VerifyTwoFactorAsync_TemporaryDuoService_InvalidToken_ReturnsFalse( + TwoFactorProviderType providerType, + User user, + Organization organization, + string token) + { + // Arrange + _featureService.IsEnabled(FeatureFlagKeys.DuoRedirect).Returns(true); + _userService.TwoFactorProviderIsEnabledAsync(providerType, user).Returns(true); + _temporaryDuoWebV4SDKService.ValidateAsync( + token, Arg.Any(), user).Returns(true); + + user.TwoFactorProviders = GetTwoFactorIndividualProviderJson(providerType); + organization.Use2fa = true; + organization.TwoFactorProviders = GetTwoFactorOrganizationDuoProviderJson(); + organization.Enabled = true; + + _userManager.TWO_FACTOR_ENABLED = true; + _userManager.TWO_FACTOR_TOKEN = token; + _userManager.TWO_FACTOR_TOKEN_VERIFIED = false; + + // Act + var result = await _sut.VerifyTwoFactor( + user, organization, providerType, token); + + // Assert + Assert.True(result); + } + + private static UserManagerTestWrapper SubstituteUserManager() + { + return new UserManagerTestWrapper( + Substitute.For>(), + Substitute.For>(), + Substitute.For>(), + Enumerable.Empty>(), + Enumerable.Empty>(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For>>()); + } + + private static string GetTwoFactorOrganizationDuoProviderJson(bool enabled = true) + { + return + "{\"6\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } + + private static string GetTwoFactorOrganizationNotEnabledDuoProviderJson(bool enabled = true) + { + return + "{\"6\":{\"Enabled\":false,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}"; + } + + private static string GetTwoFactorIndividualProviderJson(TwoFactorProviderType providerType) + { + return providerType switch + { + TwoFactorProviderType.Duo => "{\"2\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}", + TwoFactorProviderType.Email => "{\"1\":{\"Enabled\":true,\"MetaData\":{\"Email\":\"user@test.dev\"}}}", + TwoFactorProviderType.WebAuthn => "{\"7\":{\"Enabled\":true,\"MetaData\":{\"Key1\":{\"Name\":\"key1\",\"Descriptor\":{\"Type\":0,\"Id\":\"keyId\",\"Transports\":null},\"PublicKey\":\"key\",\"UserHandle\":\"handle\",\"SignatureCounter\":0,\"CredType\":\"none\",\"RegDate\":\"2022-01-01T00:00:00Z\",\"AaGuid\":\"00000000-0000-0000-0000-000000000000\",\"Migrated\":false}}}}", + TwoFactorProviderType.YubiKey => "{\"3\":{\"Enabled\":true,\"MetaData\":{\"Id\":\"yubikeyId\",\"Nfc\":true}}}", + TwoFactorProviderType.OrganizationDuo => "{\"6\":{\"Enabled\":true,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}", + _ => "{}", + }; + } + + private static string GetTwoFactorIndividualNotEnabledProviderJson(TwoFactorProviderType providerType) + { + return providerType switch + { + TwoFactorProviderType.Duo => "{\"2\":{\"Enabled\":false,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}", + TwoFactorProviderType.Email => "{\"1\":{\"Enabled\":false,\"MetaData\":{\"Email\":\"user@test.dev\"}}}", + TwoFactorProviderType.WebAuthn => "{\"7\":{\"Enabled\":false,\"MetaData\":{\"Key1\":{\"Name\":\"key1\",\"Descriptor\":{\"Type\":0,\"Id\":\"keyId\",\"Transports\":null},\"PublicKey\":\"key\",\"UserHandle\":\"handle\",\"SignatureCounter\":0,\"CredType\":\"none\",\"RegDate\":\"2022-01-01T00:00:00Z\",\"AaGuid\":\"00000000-0000-0000-0000-000000000000\",\"Migrated\":false}}}}", + TwoFactorProviderType.YubiKey => "{\"3\":{\"Enabled\":false,\"MetaData\":{\"Id\":\"yubikeyId\",\"Nfc\":true}}}", + TwoFactorProviderType.OrganizationDuo => "{\"6\":{\"Enabled\":false,\"MetaData\":{\"ClientSecret\":\"secretClientSecret\",\"ClientId\":\"clientId\",\"Host\":\"example.com\"}}}", + _ => "{}", + }; + } +} diff --git a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs index 26043fd59..f7cfd1d39 100644 --- a/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs +++ b/test/Identity.Test/Wrappers/BaseRequestValidatorTestWrapper.cs @@ -1,16 +1,13 @@ using System.Security.Claims; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Services; -using Bit.Core.Auth.Identity; -using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Bit.Identity.IdentityServer; +using Bit.Identity.IdentityServer.RequestValidators; using Duende.IdentityServer.Models; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Identity; @@ -54,38 +51,30 @@ IBaseRequestValidatorTestWrapper IUserService userService, IEventService eventService, IDeviceValidator deviceValidator, - IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, - ITemporaryDuoWebV4SDKService duoWebV4SDKService, - IOrganizationRepository organizationRepository, + ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator, IOrganizationUserRepository organizationUserRepository, - IApplicationCacheService applicationCacheService, IMailService mailService, ILogger logger, ICurrentContext currentContext, GlobalSettings globalSettings, IUserRepository userRepository, IPolicyService policyService, - IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) : - base( + base( userManager, userService, eventService, deviceValidator, - organizationDuoWebTokenProvider, - duoWebV4SDKService, - organizationRepository, + twoFactorAuthenticationValidator, organizationUserRepository, - applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, - tokenDataFactory, featureService, ssoConfigRepository, userDecryptionOptionsBuilder) @@ -98,13 +87,6 @@ IBaseRequestValidatorTestWrapper await ValidateAsync(context, context.ValidatedTokenRequest, context.CustomValidatorRequestContext); } - public async Task> TestRequiresTwoFactorAsync( - User user, - ValidatedTokenRequest context) - { - return await RequiresTwoFactorAsync(user, context); - } - protected override ClaimsPrincipal GetSubject( BaseRequestValidationContextFake context) { diff --git a/test/Identity.Test/Wrappers/UserManagerTestWrapper.cs b/test/Identity.Test/Wrappers/UserManagerTestWrapper.cs new file mode 100644 index 000000000..f1207a4b9 --- /dev/null +++ b/test/Identity.Test/Wrappers/UserManagerTestWrapper.cs @@ -0,0 +1,96 @@ + +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Bit.Identity.Test.Wrappers; + +public class UserManagerTestWrapper : UserManager where TUser : class +{ + /// + /// Modify this value to mock the responses from UserManager.GetTwoFactorEnabledAsync() + /// + public bool TWO_FACTOR_ENABLED { get; set; } = false; + /// + /// Modify this value to mock the responses from UserManager.GetValidTwoFactorProvidersAsync() + /// + public IList TWO_FACTOR_PROVIDERS { get; set; } = []; + /// + /// Modify this value to mock the responses from UserManager.GenerateTwoFactorTokenAsync() + /// + public string TWO_FACTOR_TOKEN { get; set; } = string.Empty; + /// + /// Modify this value to mock the responses from UserManager.VerifyTwoFactorTokenAsync() + /// + public bool TWO_FACTOR_TOKEN_VERIFIED { get; set; } = false; + + /// + /// Modify this value to mock the responses from UserManager.SupportsUserTwoFactor + /// + public bool SUPPORTS_TWO_FACTOR { get; set; } = false; + + public override bool SupportsUserTwoFactor + { + get + { + return SUPPORTS_TWO_FACTOR; + } + } + + public UserManagerTestWrapper( + IUserStore store, + IOptions optionsAccessor, + IPasswordHasher passwordHasher, + IEnumerable> userValidators, + IEnumerable> passwordValidators, + ILookupNormalizer keyNormalizer, + IdentityErrorDescriber errors, + IServiceProvider services, + ILogger> logger) + : base(store, optionsAccessor, passwordHasher, userValidators, passwordValidators, + keyNormalizer, errors, services, logger) + { } + + /// + /// return class variable TWO_FACTOR_ENABLED + /// + /// + /// + public override async Task GetTwoFactorEnabledAsync(TUser user) + { + return TWO_FACTOR_ENABLED; + } + + /// + /// return class variable TWO_FACTOR_PROVIDERS + /// + /// + /// + public override async Task> GetValidTwoFactorProvidersAsync(TUser user) + { + return TWO_FACTOR_PROVIDERS; + } + + /// + /// return class variable TWO_FACTOR_TOKEN + /// + /// + /// + /// + public override async Task GenerateTwoFactorTokenAsync(TUser user, string tokenProvider) + { + return TWO_FACTOR_TOKEN; + } + + /// + /// return class variable TWO_FACTOR_TOKEN_VERIFIED + /// + /// + /// + /// + /// + public override async Task VerifyTwoFactorTokenAsync(TUser user, string tokenProvider, string token) + { + return TWO_FACTOR_TOKEN_VERIFIED; + } +} From 9a499df0e72af6b06d0909552a1dea32753039c8 Mon Sep 17 00:00:00 2001 From: Vince Grassia <593223+vgrassia@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:15:16 -0400 Subject: [PATCH 04/11] BRE-344 - Add PR logic to Repository Management workflow (#4938) --- .github/workflows/repository-management.yml | 56 +++++++++++++++++++-- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/.github/workflows/repository-management.yml b/.github/workflows/repository-management.yml index eb4187c59..7207d2253 100644 --- a/.github/workflows/repository-management.yml +++ b/.github/workflows/repository-management.yml @@ -135,13 +135,61 @@ jobs: git config --local user.email "actions@github.com" git config --local user.name "Github Actions" + - name: Create version branch + id: create-branch + run: | + NAME=version_bump_${{ github.ref_name }}_$(date +"%Y-%m-%d") + git switch -c $NAME + echo "name=$NAME" >> $GITHUB_OUTPUT + - name: Commit files run: git commit -m "Bumped version to ${{ steps.set-final-version-output.outputs.version }}" -a - name: Push changes + run: git push + + - name: Generate GH App token + uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0 + id: app-token + with: + app-id: ${{ secrets.BW_GHAPP_ID }} + private-key: ${{ secrets.BW_GHAPP_KEY }} + owner: ${{ github.repository_owner }} + + - name: Create version PR + id: create-pr + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_BRANCH: ${{ steps.create-branch.outputs.name }} + TITLE: "Bump version to ${{ steps.set-final-version-output.outputs.version }}" run: | - git pull -pt - git push + PR_URL=$(gh pr create --title "$TITLE" \ + --base "main" \ + --head "$PR_BRANCH" \ + --label "version update" \ + --label "automated pr" \ + --body " + ## Type of change + - [ ] Bug fix + - [ ] New feature development + - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) + - [ ] Build/deploy pipeline (DevOps) + - [X] Other + ## Objective + Automated version bump to ${{ steps.set-final-version-output.outputs.version }}") + echo "pr_number=${PR_URL##*/}" >> $GITHUB_OUTPUT + + - name: Approve PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.create-pr.outputs.pr_number }} + run: gh pr review $PR_NUMBER --approve + + - name: Merge PR + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ steps.create-pr.outputs.pr_number }} + run: gh pr merge $PR_NUMBER --squash --auto --delete-branch cherry_pick: @@ -153,7 +201,7 @@ jobs: uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1 with: ref: main - + - name: Install xmllint run: | sudo apt-get update @@ -189,7 +237,7 @@ jobs: RC_VERSION=$(xmllint -xpath "/Project/PropertyGroup/Version/text()" Directory.Build.props) echo "rc_version=$RC_VERSION" >> $GITHUB_OUTPUT fi - + - name: Configure Git run: | git config --local user.email "actions@github.com" From f43f59e4b4af5e61f5b0c298715aeb3ee991ac5a Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 24 Oct 2024 12:45:13 -0700 Subject: [PATCH 05/11] Register noop notification registration service for self host lacking necessary data (#4939) --- src/SharedWeb/Utilities/ServiceCollectionExtensions.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index b0a2c42ea..5a5585952 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -274,6 +274,11 @@ public static class ServiceCollectionExtensions services.AddKeyedSingleton("implementation"); services.AddSingleton(); } + else + { + services.AddSingleton(); + } + if (CoreHelpers.SettingHasValue(globalSettings.InternalIdentityKey) && CoreHelpers.SettingHasValue(globalSettings.BaseServiceUri.InternalNotifications)) { @@ -290,10 +295,6 @@ public static class ServiceCollectionExtensions services.AddKeyedSingleton("implementation"); } } - else - { - services.AddSingleton(); - } if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Mail.ConnectionString)) { From 6272e84c9265800b5158c5e461e2608b8d0809c7 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:17:28 +1000 Subject: [PATCH 06/11] Remove feature flag (#4931) Co-authored-by: MtnBurrit0 <77340197+mimartin12@users.noreply.github.com> --- .../OrganizationUsersController.cs | 1 - ...tionUserUserDetailsAuthorizationHandler.cs | 28 +------ src/Core/Constants.cs | 1 - ...serUserDetailsAuthorizationHandlerTests.cs | 76 ------------------- 4 files changed, 1 insertion(+), 105 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index b6d41ffec..89a8627e9 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -126,7 +126,6 @@ public class OrganizationUsersController : Controller } [HttpGet("mini-details")] - [RequireFeature(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi)] public async Task> GetMiniDetails(Guid orgId) { var authorizationResult = await _authorizationService.AuthorizeAsync(User, new OrganizationScope(orgId), diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs index dcfe630e3..e890e4d9f 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs @@ -1,7 +1,6 @@ #nullable enable using Bit.Core.Context; using Bit.Core.Enums; -using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; @@ -10,12 +9,10 @@ public class OrganizationUserUserDetailsAuthorizationHandler : AuthorizationHandler { private readonly ICurrentContext _currentContext; - private readonly IFeatureService _featureService; - public OrganizationUserUserDetailsAuthorizationHandler(ICurrentContext currentContext, IFeatureService featureService) + public OrganizationUserUserDetailsAuthorizationHandler(ICurrentContext currentContext) { _currentContext = currentContext; - _featureService = featureService; } protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, @@ -37,29 +34,6 @@ public class OrganizationUserUserDetailsAuthorizationHandler } private async Task CanReadAllAsync(Guid organizationId) - { - if (_featureService.IsEnabled(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi)) - { - return await CanReadAllAsync_vNext(organizationId); - } - - return await CanReadAllAsync_vCurrent(organizationId); - } - - private async Task CanReadAllAsync_vCurrent(Guid organizationId) - { - // All users of an organization can read all other users of that organization for collection access management - var org = _currentContext.GetOrganization(organizationId); - if (org is not null) - { - return true; - } - - // Allow provider users to read all organization users if they are a provider for the target organization - return await _currentContext.ProviderUserForOrgAsync(organizationId); - } - - private async Task CanReadAllAsync_vNext(Guid organizationId) { // Admins can access this for general user management var organization = _currentContext.GetOrganization(organizationId); diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index d4408e7a3..a1cb3e2c6 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -141,7 +141,6 @@ public static class FeatureFlagKeys public const string EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill"; public const string StorageReseedRefactor = "storage-reseed-refactor"; public const string TrialPayment = "PM-8163-trial-payment"; - public const string Pm3478RefactorOrganizationUserApi = "pm-3478-refactor-organizationuser-api"; public const string RemoveServerVersionHeader = "remove-server-version-header"; public const string AccessIntelligence = "pm-13227-access-intelligence"; public const string VerifiedSsoDomainEndpoint = "pm-12337-refactor-sso-details-endpoint"; diff --git a/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs index 4d9208a2b..4a3a7f647 100644 --- a/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs @@ -2,7 +2,6 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; using Bit.Core.Context; using Bit.Core.Enums; -using Bit.Core.Services; using Bit.Core.Test.AdminConsole.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -24,7 +23,6 @@ public class OrganizationUserUserDetailsAuthorizationHandlerTests CurrentContextOrganization organization, SutProvider sutProvider) { - EnableFeatureFlag(sutProvider); organization.Type = userType; sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); @@ -48,7 +46,6 @@ public class OrganizationUserUserDetailsAuthorizationHandlerTests CurrentContextOrganization organization, SutProvider sutProvider) { - EnableFeatureFlag(sutProvider); organization.Type = OrganizationUserType.User; sutProvider.GetDependency() .ProviderUserForOrgAsync(organization.Id) @@ -69,7 +66,6 @@ public class OrganizationUserUserDetailsAuthorizationHandlerTests CurrentContextOrganization organization, SutProvider sutProvider) { - EnableFeatureFlag(sutProvider); organization.Type = OrganizationUserType.User; sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns(organization); sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); @@ -88,78 +84,6 @@ public class OrganizationUserUserDetailsAuthorizationHandlerTests public async Task ReadAll_NotMember_NoSuccess( CurrentContextOrganization organization, SutProvider sutProvider) - { - EnableFeatureFlag(sutProvider); - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserUserDetailsOperations.ReadAll }, - new ClaimsPrincipal(), - new OrganizationScope(organization.Id) - ); - - sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); - sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); - - await sutProvider.Sut.HandleAsync(context); - Assert.False(context.HasSucceeded); - } - - private void EnableFeatureFlag(SutProvider sutProvider) - { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi) - .Returns(true); - } - - // TESTS WITH FLAG DISABLED - TO BE DELETED IN FLAG CLEANUP - - [Theory, CurrentContextOrganizationCustomize] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Owner)] - [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Custom)] - public async Task FlagDisabled_ReadAll_AnyMemberOfOrg_Success( - OrganizationUserType userType, - Guid userId, SutProvider sutProvider, - CurrentContextOrganization organization) - { - organization.Type = userType; - - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserUserDetailsOperations.ReadAll }, - new ClaimsPrincipal(), - new OrganizationScope(organization.Id)); - - sutProvider.GetDependency().UserId.Returns(userId); - sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData, CurrentContextOrganizationCustomize] - public async Task FlagDisabled_ReadAll_ProviderUser_Success( - CurrentContextOrganization organization, - SutProvider sutProvider) - { - organization.Type = OrganizationUserType.User; - sutProvider.GetDependency() - .ProviderUserForOrgAsync(organization.Id) - .Returns(true); - - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserUserDetailsOperations.ReadAll }, - new ClaimsPrincipal(), - new OrganizationScope(organization.Id)); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task FlagDisabled_ReadAll_NotMember_NoSuccess( - CurrentContextOrganization organization, - SutProvider sutProvider) { var context = new AuthorizationHandlerContext( new[] { OrganizationUserUserDetailsOperations.ReadAll }, From 53ad9df003fc855bbd95b6be93247d902816dca1 Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Fri, 25 Oct 2024 03:56:03 +0200 Subject: [PATCH 07/11] [PM-13451] Update subscription setup process to support MOE providers (#4937) --- .../Billing/ProviderBillingService.cs | 45 ++++++------------- .../StaticStore/Plans/EnterprisePlan.cs | 2 + 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs b/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs index 787a11d1e..19991dab2 100644 --- a/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs +++ b/bitwarden_license/src/Commercial.Core/Billing/ProviderBillingService.cs @@ -379,42 +379,23 @@ public class ProviderBillingService( var subscriptionItemOptionsList = new List(); - var teamsProviderPlan = - providerPlans.SingleOrDefault(providerPlan => providerPlan.PlanType == PlanType.TeamsMonthly); - - if (teamsProviderPlan == null || !teamsProviderPlan.IsConfigured()) + foreach (var providerPlan in providerPlans) { - logger.LogError("Cannot start subscription for provider ({ProviderID}) that has no configured Teams plan", provider.Id); + var plan = StaticStore.GetPlan(providerPlan.PlanType); - throw new BillingException(); + if (!providerPlan.IsConfigured()) + { + logger.LogError("Cannot start subscription for provider ({ProviderID}) that has no configured {ProviderName} plan", provider.Id, plan.Name); + throw new BillingException(); + } + + subscriptionItemOptionsList.Add(new SubscriptionItemOptions + { + Price = plan.PasswordManager.StripeProviderPortalSeatPlanId, + Quantity = providerPlan.SeatMinimum + }); } - var teamsPlan = StaticStore.GetPlan(PlanType.TeamsMonthly); - - subscriptionItemOptionsList.Add(new SubscriptionItemOptions - { - Price = teamsPlan.PasswordManager.StripeProviderPortalSeatPlanId, - Quantity = teamsProviderPlan.SeatMinimum - }); - - var enterpriseProviderPlan = - providerPlans.SingleOrDefault(providerPlan => providerPlan.PlanType == PlanType.EnterpriseMonthly); - - if (enterpriseProviderPlan == null || !enterpriseProviderPlan.IsConfigured()) - { - logger.LogError("Cannot start subscription for provider ({ProviderID}) that has no configured Enterprise plan", provider.Id); - - throw new BillingException(); - } - - var enterprisePlan = StaticStore.GetPlan(PlanType.EnterpriseMonthly); - - subscriptionItemOptionsList.Add(new SubscriptionItemOptions - { - Price = enterprisePlan.PasswordManager.StripeProviderPortalSeatPlanId, - Quantity = enterpriseProviderPlan.SeatMinimum - }); - var subscriptionCreateOptions = new SubscriptionCreateOptions { AutomaticTax = new SubscriptionAutomaticTaxOptions diff --git a/src/Core/Billing/Models/StaticStore/Plans/EnterprisePlan.cs b/src/Core/Billing/Models/StaticStore/Plans/EnterprisePlan.cs index f1fff0762..2d498a765 100644 --- a/src/Core/Billing/Models/StaticStore/Plans/EnterprisePlan.cs +++ b/src/Core/Billing/Models/StaticStore/Plans/EnterprisePlan.cs @@ -87,7 +87,9 @@ public record EnterprisePlan : Plan AdditionalStoragePricePerGb = 4; StripeStoragePlanId = "storage-gb-annually"; StripeSeatPlanId = "2023-enterprise-org-seat-annually"; + StripeProviderPortalSeatPlanId = "password-manager-provider-portal-enterprise-annually-2024"; SeatPrice = 72; + ProviderPortalSeatPrice = 72; } else { From d0c9953444c00c357440b647631650f120906aac Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Sat, 26 Oct 2024 09:43:27 -0700 Subject: [PATCH 08/11] [PM-8213] Feature flag for `new-device-verification` (#4944) --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index a1cb3e2c6..b64d46b5b 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -148,6 +148,7 @@ public static class FeatureFlagKeys public const string Pm13322AddPolicyDefinitions = "pm-13322-add-policy-definitions"; public const string LimitCollectionCreationDeletionSplit = "pm-10863-limit-collection-creation-deletion-split"; public const string GeneratorToolsModernization = "generator-tools-modernization"; + public const string NewDeviceVerification = "new-device-verification"; public static List GetAllKeys() { From e2a69c432c72613052400a7d66b89268546146db Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 10:00:33 +0100 Subject: [PATCH 09/11] [deps] Tools: Update LaunchDarkly.ServerSdk to 8.6.0 (#4950) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 5174543c6..4c8c88ebd 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -59,7 +59,7 @@ - + From 109ba14cf40025f36ef11c46c2a3fdde0d823538 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 10:02:40 +0100 Subject: [PATCH 10/11] [deps] Tools: Update aws-sdk-net monorepo (#4946) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 4c8c88ebd..fd463b075 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -21,8 +21,8 @@ - - + + From cc6e41b42aa8eb6c043a60d2e0b5f6e62b18f894 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 10:29:34 -0400 Subject: [PATCH 11/11] [deps] DbOps: Update Microsoft.Azure.Cosmos to 3.45.0 (#4949) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index fd463b075..35cdbaf90 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -36,7 +36,7 @@ - +