From 4fec7cadb70af181aa01c476712b3c3db107f24a Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Fri, 18 Oct 2024 07:45:34 -0500 Subject: [PATCH] [PM-13722] Refactor `ValidateOrganizationsDomainAsync` (#4905) Refactored ValidateOrganizationsDomainAsync to use VerifyOrganizationDomainAsync --- .../OrganizationDomainController.cs | 2 +- .../CreateOrganizationDomainCommand.cs | 7 -- .../IVerifyOrganizationDomainCommand.cs | 3 +- .../VerifyOrganizationDomainCommand.cs | 65 ++++++++++++++++--- .../OrganizationDomainService.cs | 52 +++++---------- .../OrganizationDomainControllerTests.cs | 4 +- .../VerifyOrganizationDomainCommandTests.cs | 36 +++++++--- .../OrganizationDomainServiceTests.cs | 15 ++--- 8 files changed, 109 insertions(+), 75 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationDomainController.cs b/src/Api/AdminConsole/Controllers/OrganizationDomainController.cs index af7a162d80..b9afde2724 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationDomainController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationDomainController.cs @@ -101,7 +101,7 @@ public class OrganizationDomainController : Controller throw new NotFoundException(); } - organizationDomain = await _verifyOrganizationDomainCommand.VerifyOrganizationDomainAsync(organizationDomain); + organizationDomain = await _verifyOrganizationDomainCommand.UserVerifyOrganizationDomainAsync(organizationDomain); return new OrganizationDomainResponseModel(organizationDomain); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/CreateOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/CreateOrganizationDomainCommand.cs index be8ed0e640..192ab3b79d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/CreateOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/CreateOrganizationDomainCommand.cs @@ -6,7 +6,6 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Utilities; -using Microsoft.Extensions.Logging; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; @@ -14,21 +13,15 @@ public class CreateOrganizationDomainCommand : ICreateOrganizationDomainCommand { private readonly IOrganizationDomainRepository _organizationDomainRepository; private readonly IEventService _eventService; - private readonly IDnsResolverService _dnsResolverService; - private readonly ILogger _logger; private readonly IGlobalSettings _globalSettings; public CreateOrganizationDomainCommand( IOrganizationDomainRepository organizationDomainRepository, IEventService eventService, - IDnsResolverService dnsResolverService, - ILogger logger, IGlobalSettings globalSettings) { _organizationDomainRepository = organizationDomainRepository; _eventService = eventService; - _dnsResolverService = dnsResolverService; - _logger = logger; _globalSettings = globalSettings; } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IVerifyOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IVerifyOrganizationDomainCommand.cs index b62a9f9327..6b5beb11ff 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IVerifyOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/Interfaces/IVerifyOrganizationDomainCommand.cs @@ -4,5 +4,6 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfa public interface IVerifyOrganizationDomainCommand { - Task VerifyOrganizationDomainAsync(OrganizationDomain organizationDomain); + Task UserVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain); + Task SystemVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs index 5f9476db8f..8e1a4d5735 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs @@ -4,6 +4,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Settings; using Microsoft.Extensions.Logging; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; @@ -13,34 +14,85 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand private readonly IOrganizationDomainRepository _organizationDomainRepository; private readonly IDnsResolverService _dnsResolverService; private readonly IEventService _eventService; + private readonly IGlobalSettings _globalSettings; private readonly ILogger _logger; public VerifyOrganizationDomainCommand( IOrganizationDomainRepository organizationDomainRepository, IDnsResolverService dnsResolverService, IEventService eventService, + IGlobalSettings globalSettings, ILogger logger) { _organizationDomainRepository = organizationDomainRepository; _dnsResolverService = dnsResolverService; _eventService = eventService; + _globalSettings = globalSettings; _logger = logger; } - public async Task VerifyOrganizationDomainAsync(OrganizationDomain domain) + + public async Task UserVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain) { + var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain); + + await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + domainVerificationResult.VerifiedDate != null + ? EventType.OrganizationDomain_Verified + : EventType.OrganizationDomain_NotVerified); + + await _organizationDomainRepository.ReplaceAsync(domainVerificationResult); + + return domainVerificationResult; + } + + public async Task SystemVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain) + { + organizationDomain.SetJobRunCount(); + + var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain); + + if (domainVerificationResult.VerifiedDate is not null) + { + _logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain"); + + await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + EventType.OrganizationDomain_Verified, + EventSystemUser.DomainVerification); + } + else + { + domainVerificationResult.SetNextRunDate(_globalSettings.DomainVerification.VerificationInterval); + + await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + EventType.OrganizationDomain_NotVerified, + EventSystemUser.DomainVerification); + + _logger.LogInformation(Constants.BypassFiltersEventId, + "Verification for organization {OrgId} with domain {Domain} failed", + domainVerificationResult.OrganizationId, domainVerificationResult.DomainName); + } + + await _organizationDomainRepository.ReplaceAsync(domainVerificationResult); + + return domainVerificationResult; + } + + private async Task VerifyOrganizationDomainAsync(OrganizationDomain domain) + { + domain.SetLastCheckedDate(); + if (domain.VerifiedDate is not null) { - domain.SetLastCheckedDate(); await _organizationDomainRepository.ReplaceAsync(domain); throw new ConflictException("Domain has already been verified."); } var claimedDomain = await _organizationDomainRepository.GetClaimedDomainsByDomainNameAsync(domain.DomainName); - if (claimedDomain.Any()) + + if (claimedDomain.Count > 0) { - domain.SetLastCheckedDate(); await _organizationDomainRepository.ReplaceAsync(domain); throw new ConflictException("The domain is not available to be claimed."); } @@ -58,11 +110,6 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand domain.DomainName, e.Message); } - domain.SetLastCheckedDate(); - await _organizationDomainRepository.ReplaceAsync(domain); - - await _eventService.LogOrganizationDomainEventAsync(domain, - domain.VerifiedDate != null ? EventType.OrganizationDomain_Verified : EventType.OrganizationDomain_NotVerified); return domain; } } diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs index b526f27d9d..890042b314 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationDomainService.cs @@ -1,4 +1,5 @@ -using Bit.Core.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; @@ -10,26 +11,29 @@ public class OrganizationDomainService : IOrganizationDomainService { private readonly IOrganizationDomainRepository _domainRepository; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IDnsResolverService _dnsResolverService; private readonly IEventService _eventService; private readonly IMailService _mailService; + private readonly IVerifyOrganizationDomainCommand _verifyOrganizationDomainCommand; + private readonly TimeProvider _timeProvider; private readonly ILogger _logger; private readonly IGlobalSettings _globalSettings; public OrganizationDomainService( IOrganizationDomainRepository domainRepository, IOrganizationUserRepository organizationUserRepository, - IDnsResolverService dnsResolverService, IEventService eventService, IMailService mailService, + IVerifyOrganizationDomainCommand verifyOrganizationDomainCommand, + TimeProvider timeProvider, ILogger logger, IGlobalSettings globalSettings) { _domainRepository = domainRepository; _organizationUserRepository = organizationUserRepository; - _dnsResolverService = dnsResolverService; _eventService = eventService; _mailService = mailService; + _verifyOrganizationDomainCommand = verifyOrganizationDomainCommand; + _timeProvider = timeProvider; _logger = logger; _globalSettings = globalSettings; } @@ -37,7 +41,7 @@ public class OrganizationDomainService : IOrganizationDomainService public async Task ValidateOrganizationsDomainAsync() { //Date should be set 1 hour behind to ensure it selects all domains that should be verified - var runDate = DateTime.UtcNow.AddHours(-1); + var runDate = _timeProvider.GetUtcNow().UtcDateTime.AddHours(-1); var verifiableDomains = await _domainRepository.GetManyByNextRunDateAsync(runDate); @@ -45,43 +49,17 @@ public class OrganizationDomainService : IOrganizationDomainService foreach (var domain in verifiableDomains) { + _logger.LogInformation(Constants.BypassFiltersEventId, + "Attempting verification for organization {OrgId} with domain {Domain}", + domain.OrganizationId, + domain.DomainName); + try { - _logger.LogInformation(Constants.BypassFiltersEventId, "Attempting verification for organization {OrgId} with domain {Domain}", domain.OrganizationId, domain.DomainName); - - var status = await _dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt); - if (status) - { - _logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain"); - - // Update entry on OrganizationDomain table - domain.SetLastCheckedDate(); - domain.SetVerifiedDate(); - domain.SetJobRunCount(); - await _domainRepository.ReplaceAsync(domain); - - await _eventService.LogOrganizationDomainEventAsync(domain, EventType.OrganizationDomain_Verified, - EventSystemUser.DomainVerification); - } - else - { - // Update entry on OrganizationDomain table - domain.SetLastCheckedDate(); - domain.SetJobRunCount(); - domain.SetNextRunDate(_globalSettings.DomainVerification.VerificationInterval); - await _domainRepository.ReplaceAsync(domain); - - await _eventService.LogOrganizationDomainEventAsync(domain, EventType.OrganizationDomain_NotVerified, - EventSystemUser.DomainVerification); - _logger.LogInformation(Constants.BypassFiltersEventId, "Verification for organization {OrgId} with domain {Domain} failed", - domain.OrganizationId, domain.DomainName); - } + _ = await _verifyOrganizationDomainCommand.SystemVerifyOrganizationDomainAsync(domain); } catch (Exception ex) { - // Update entry on OrganizationDomain table - domain.SetLastCheckedDate(); - domain.SetJobRunCount(); domain.SetNextRunDate(_globalSettings.DomainVerification.VerificationInterval); await _domainRepository.ReplaceAsync(domain); diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationDomainControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationDomainControllerTests.cs index 1ff4b519c0..352f089db7 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationDomainControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationDomainControllerTests.cs @@ -229,13 +229,13 @@ public class OrganizationDomainControllerTests sutProvider.GetDependency() .GetDomainByIdOrganizationIdAsync(organizationDomain.Id, organizationDomain.OrganizationId) .Returns(organizationDomain); - sutProvider.GetDependency().VerifyOrganizationDomainAsync(organizationDomain) + sutProvider.GetDependency().UserVerifyOrganizationDomainAsync(organizationDomain) .Returns(new OrganizationDomain()); var result = await sutProvider.Sut.Verify(organizationDomain.OrganizationId, organizationDomain.Id); await sutProvider.GetDependency().Received(1) - .VerifyOrganizationDomainAsync(organizationDomain); + .UserVerifyOrganizationDomainAsync(organizationDomain); Assert.IsType(result); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs index 416d86c5d2..d61ded28ba 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs @@ -15,7 +15,7 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationDomains; public class VerifyOrganizationDomainCommandTests { [Theory, BitAutoData] - public async Task VerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimed(Guid id, + public async Task UserVerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimed(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -30,14 +30,14 @@ public class VerifyOrganizationDomainCommandTests .GetByIdAsync(id) .Returns(expected); - var requestAction = async () => await sutProvider.Sut.VerifyOrganizationDomainAsync(expected); + var requestAction = async () => await sutProvider.Sut.UserVerifyOrganizationDomainAsync(expected); var exception = await Assert.ThrowsAsync(requestAction); Assert.Contains("Domain has already been verified.", exception.Message); } [Theory, BitAutoData] - public async Task VerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimedByAnotherOrganization(Guid id, + public async Task UserVerifyOrganizationDomain_ShouldThrowConflict_WhenDomainHasBeenClaimedByAnotherOrganization(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -54,14 +54,14 @@ public class VerifyOrganizationDomainCommandTests .GetClaimedDomainsByDomainNameAsync(expected.DomainName) .Returns(new List { expected }); - var requestAction = async () => await sutProvider.Sut.VerifyOrganizationDomainAsync(expected); + var requestAction = async () => await sutProvider.Sut.UserVerifyOrganizationDomainAsync(expected); var exception = await Assert.ThrowsAsync(requestAction); Assert.Contains("The domain is not available to be claimed.", exception.Message); } [Theory, BitAutoData] - public async Task VerifyOrganizationDomain_ShouldVerifyDomainUpdateAndLogEvent_WhenTxtRecordExists(Guid id, + public async Task UserVerifyOrganizationDomain_ShouldVerifyDomainUpdateAndLogEvent_WhenTxtRecordExists(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -81,7 +81,7 @@ public class VerifyOrganizationDomainCommandTests .ResolveAsync(expected.DomainName, Arg.Any()) .Returns(true); - var result = await sutProvider.Sut.VerifyOrganizationDomainAsync(expected); + var result = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(expected); Assert.NotNull(result.VerifiedDate); await sutProvider.GetDependency().Received(1) @@ -91,7 +91,7 @@ public class VerifyOrganizationDomainCommandTests } [Theory, BitAutoData] - public async Task VerifyOrganizationDomain_ShouldNotSetVerifiedDate_WhenTxtRecordDoesNotExist(Guid id, + public async Task UserVerifyOrganizationDomain_ShouldNotSetVerifiedDate_WhenTxtRecordDoesNotExist(Guid id, SutProvider sutProvider) { var expected = new OrganizationDomain @@ -111,10 +111,30 @@ public class VerifyOrganizationDomainCommandTests .ResolveAsync(expected.DomainName, Arg.Any()) .Returns(false); - var result = await sutProvider.Sut.VerifyOrganizationDomainAsync(expected); + var result = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(expected); Assert.Null(result.VerifiedDate); await sutProvider.GetDependency().Received(1) .LogOrganizationDomainEventAsync(Arg.Any(), EventType.OrganizationDomain_NotVerified); } + + + [Theory, BitAutoData] + public async Task SystemVerifyOrganizationDomain_CallsEventServiceWithUpdatedJobRunCount(SutProvider sutProvider) + { + var domain = new OrganizationDomain() + { + Id = Guid.NewGuid(), + OrganizationId = Guid.NewGuid(), + CreationDate = DateTime.UtcNow, + DomainName = "test.com", + Txt = "btw+12345", + }; + + _ = await sutProvider.Sut.SystemVerifyOrganizationDomainAsync(domain); + + await sutProvider.GetDependency().ReceivedWithAnyArgs(1) + .LogOrganizationDomainEventAsync(default, EventType.OrganizationDomain_NotVerified, + EventSystemUser.DomainVerification); + } } diff --git a/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs index ddd9accd0d..c779e3a1cc 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationDomainServiceTests.cs @@ -1,8 +1,7 @@ -using Bit.Core.AdminConsole.Services.Implementations; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.AdminConsole.Services.Implementations; using Bit.Core.Entities; -using Bit.Core.Enums; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -36,18 +35,14 @@ public class OrganizationDomainServiceTests Txt = "btw+6789" } }; + sutProvider.GetDependency().GetManyByNextRunDateAsync(default) .ReturnsForAnyArgs(domains); await sutProvider.Sut.ValidateOrganizationsDomainAsync(); - await sutProvider.GetDependency().ReceivedWithAnyArgs(2) - .ResolveAsync(default, default); - await sutProvider.GetDependency().ReceivedWithAnyArgs(2) - .ReplaceAsync(default); - await sutProvider.GetDependency().ReceivedWithAnyArgs(2) - .LogOrganizationDomainEventAsync(default, EventType.OrganizationDomain_NotVerified, - EventSystemUser.DomainVerification); + await sutProvider.GetDependency().ReceivedWithAnyArgs(2) + .SystemVerifyOrganizationDomainAsync(default); } [Theory, BitAutoData]