Skip to content

Commit

Permalink
Realized this would throw a null error from the system domain verific…
Browse files Browse the repository at this point in the history
…ation. Adding unknown type to event system user. Adding optional parameter to SaveAsync in policy service in order to pass in event system user.
  • Loading branch information
jrmccannon committed Nov 15, 2024
1 parent 0014d5c commit 82b65e0
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 53 deletions.
1 change: 1 addition & 0 deletions src/Core/AdminConsole/Enums/EventSystemUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

public enum EventSystemUser : byte
{
Unknown = 0,
SCIM = 1,
DomainVerification = 2,
PublicApi = 3,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
Expand All @@ -12,124 +14,114 @@

namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains;

public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand
public class VerifyOrganizationDomainCommand(
IOrganizationDomainRepository organizationDomainRepository,
IDnsResolverService dnsResolverService,
IEventService eventService,
IGlobalSettings globalSettings,
IPolicyService policyService,
IFeatureService featureService,
ICurrentContext currentContext,
ILogger<VerifyOrganizationDomainCommand> logger)
: IVerifyOrganizationDomainCommand
{
private readonly IOrganizationDomainRepository _organizationDomainRepository;
private readonly IDnsResolverService _dnsResolverService;
private readonly IEventService _eventService;
private readonly IGlobalSettings _globalSettings;
private readonly IPolicyService _policyService;
private readonly IFeatureService _featureService;
private readonly ILogger<VerifyOrganizationDomainCommand> _logger;

public VerifyOrganizationDomainCommand(
IOrganizationDomainRepository organizationDomainRepository,
IDnsResolverService dnsResolverService,
IEventService eventService,
IGlobalSettings globalSettings,
IPolicyService policyService,
IFeatureService featureService,
ILogger<VerifyOrganizationDomainCommand> logger)
{
_organizationDomainRepository = organizationDomainRepository;
_dnsResolverService = dnsResolverService;
_eventService = eventService;
_globalSettings = globalSettings;
_policyService = policyService;
_featureService = featureService;
_logger = logger;
}


public async Task<OrganizationDomain> UserVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain)
{
var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain);
var actingUser = new StandardUser(currentContext.UserId ?? Guid.Empty, await currentContext.OrganizationOwner(organizationDomain.OrganizationId));

await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult,
var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain, actingUser);

await eventService.LogOrganizationDomainEventAsync(domainVerificationResult,
domainVerificationResult.VerifiedDate != null
? EventType.OrganizationDomain_Verified
: EventType.OrganizationDomain_NotVerified);

await _organizationDomainRepository.ReplaceAsync(domainVerificationResult);
await organizationDomainRepository.ReplaceAsync(domainVerificationResult);

return domainVerificationResult;
}

public async Task<OrganizationDomain> SystemVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain)
{
var actingUser = new SystemUser(EventSystemUser.DomainVerification);

organizationDomain.SetJobRunCount();

var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain);
var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain, actingUser);

if (domainVerificationResult.VerifiedDate is not null)
{
_logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain");
logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain");

Check warning on line 56 in src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs#L56

Added line #L56 was not covered by tests

await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult,
await eventService.LogOrganizationDomainEventAsync(domainVerificationResult,

Check warning on line 58 in src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs#L58

Added line #L58 was not covered by tests
EventType.OrganizationDomain_Verified,
EventSystemUser.DomainVerification);
}
else
{
domainVerificationResult.SetNextRunDate(_globalSettings.DomainVerification.VerificationInterval);
domainVerificationResult.SetNextRunDate(globalSettings.DomainVerification.VerificationInterval);

await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult,
await eventService.LogOrganizationDomainEventAsync(domainVerificationResult,
EventType.OrganizationDomain_NotVerified,
EventSystemUser.DomainVerification);

_logger.LogInformation(Constants.BypassFiltersEventId,
logger.LogInformation(Constants.BypassFiltersEventId,
"Verification for organization {OrgId} with domain {Domain} failed",
domainVerificationResult.OrganizationId, domainVerificationResult.DomainName);
}

await _organizationDomainRepository.ReplaceAsync(domainVerificationResult);
await organizationDomainRepository.ReplaceAsync(domainVerificationResult);

return domainVerificationResult;
}

private async Task<OrganizationDomain> VerifyOrganizationDomainAsync(OrganizationDomain domain)
private async Task<OrganizationDomain> VerifyOrganizationDomainAsync(OrganizationDomain domain, IActingUser actingUser)
{
domain.SetLastCheckedDate();

if (domain.VerifiedDate is not null)
{
await _organizationDomainRepository.ReplaceAsync(domain);
await organizationDomainRepository.ReplaceAsync(domain);
throw new ConflictException("Domain has already been verified.");
}

var claimedDomain =
await _organizationDomainRepository.GetClaimedDomainsByDomainNameAsync(domain.DomainName);
await organizationDomainRepository.GetClaimedDomainsByDomainNameAsync(domain.DomainName);

if (claimedDomain.Count > 0)
{
await _organizationDomainRepository.ReplaceAsync(domain);
await organizationDomainRepository.ReplaceAsync(domain);
throw new ConflictException("The domain is not available to be claimed.");
}

try
{
if (await _dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt))
if (await dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt))
{
domain.SetVerifiedDate();

await EnableSingleOrganizationPolicyAsync(domain.OrganizationId);
await EnableSingleOrganizationPolicyAsync(domain.OrganizationId, actingUser);
}
}
catch (Exception e)
{
_logger.LogError("Error verifying Organization domain: {domain}. {errorMessage}",
logger.LogError("Error verifying Organization domain: {domain}. {errorMessage}",

Check warning on line 110 in src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs#L110

Added line #L110 was not covered by tests
domain.DomainName, e.Message);
}

return domain;
}

private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId)
private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId, IActingUser actingUser)
{
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
if (featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
{
await _policyService.SaveAsync(
new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true }, null);
await policyService.SaveAsync(
new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true },
savingUserId: actingUser is StandardUser standardUser ? standardUser.UserId : null,
eventSystemUser: actingUser is SystemUser systemUser ? systemUser.SystemUserType : null);
}
}
}
2 changes: 1 addition & 1 deletion src/Core/AdminConsole/Services/IPolicyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Bit.Core.AdminConsole.Services;

public interface IPolicyService
{
Task SaveAsync(Policy policy, Guid? savingUserId);
Task SaveAsync(Policy policy, Guid? savingUserId, EventSystemUser? eventSystemUser = null);

/// <summary>
/// Get the combined master password policy options for the specified user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public PolicyService(
_currentContext = currentContext;
}

public async Task SaveAsync(Policy policy, Guid? savingUserId)
public async Task SaveAsync(Policy policy, Guid? savingUserId, EventSystemUser? eventSystemUser = null)
{
if (_featureService.IsEnabled(FeatureFlagKeys.Pm13322AddPolicyDefinitions))
{
Expand All @@ -84,7 +84,7 @@ public async Task SaveAsync(Policy policy, Guid? savingUserId)
Data = policy.Data,
PerformedBy = savingUserId.HasValue
? new StandardUser(savingUserId.Value, await _currentContext.OrganizationOwner(policy.OrganizationId))
: null
: new SystemUser(eventSystemUser ?? EventSystemUser.Unknown)

Check warning on line 87 in src/Core/AdminConsole/Services/Implementations/PolicyService.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/Services/Implementations/PolicyService.cs#L84-L87

Added lines #L84 - L87 were not covered by tests
};

await _savePolicyCommand.SaveAsync(policyUpdate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
Expand Down Expand Up @@ -143,7 +144,7 @@ await sutProvider.GetDependency<IEventService>().ReceivedWithAnyArgs(1)

[Theory, BitAutoData]
public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningEnabled_WhenDomainIsVerified_ThenSingleOrgPolicyShouldBeEnabled(
OrganizationDomain domain, SutProvider<VerifyOrganizationDomainCommand> sutProvider)
OrganizationDomain domain, Guid userId, SutProvider<VerifyOrganizationDomainCommand> sutProvider)
{
sutProvider.GetDependency<IOrganizationDomainRepository>()
.GetClaimedDomainsByDomainNameAsync(domain.DomainName)
Expand All @@ -157,11 +158,14 @@ public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithA
.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
.Returns(true);

sutProvider.GetDependency<ICurrentContext>()
.UserId.Returns(userId);

_ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain);

await sutProvider.GetDependency<IPolicyService>()
.Received(1)
.SaveAsync(Arg.Is<Policy>(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), null);
.SaveAsync(Arg.Is<Policy>(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), userId);
}

[Theory, BitAutoData]
Expand Down
1 change: 0 additions & 1 deletion test/Core.Test/AdminConsole/Services/EventServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ public async Task LogOrganizationUserEvent_WithEventSystemUser_LogsRequiredInfo(
new EventMessage()
{
IpAddress = ipAddress,
DeviceType = DeviceType.Server,
OrganizationId = orgUser.OrganizationId,
UserId = orgUser.UserId,
OrganizationUserId = orgUser.Id,
Expand Down

0 comments on commit 82b65e0

Please sign in to comment.