Skip to content

Commit

Permalink
[PM-13014] - Add CanToggleStatus property to PolicyRepsonseModel base…
Browse files Browse the repository at this point in the history
…d on Policy Validators (#4940)

* Adding CanToggleState to PoliciesControllers (api/public) endpoints. Added mappings wrapped in feature flag.

* Updated logic for determining CanToggle. Removed setting of toggle from List endpoint. Added new details model for single policy response. Validator now returns after first error.
  • Loading branch information
jrmccannon authored Nov 11, 2024
1 parent 2e635c9 commit 1dec51b
Show file tree
Hide file tree
Showing 14 changed files with 167 additions and 32 deletions.
30 changes: 22 additions & 8 deletions src/Api/AdminConsole/Controllers/PoliciesController.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using Bit.Api.AdminConsole.Models.Request;
using Bit.Api.AdminConsole.Models.Response.Helpers;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Models.Response;
using Bit.Core;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Api.Response;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Models.Business.Tokenables;
Expand All @@ -16,7 +20,6 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.Mvc;
using AdminConsoleEntities = Bit.Core.AdminConsole.Entities;

namespace Bit.Api.AdminConsole.Controllers;

Expand All @@ -32,6 +35,8 @@ public class PoliciesController : Controller
private readonly GlobalSettings _globalSettings;
private readonly IDataProtector _organizationServiceDataProtector;
private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory;
private readonly IFeatureService _featureService;
private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery;

public PoliciesController(
IPolicyRepository policyRepository,
Expand All @@ -41,7 +46,9 @@ public PoliciesController(
ICurrentContext currentContext,
GlobalSettings globalSettings,
IDataProtectionProvider dataProtectionProvider,
IDataProtectorTokenFactory<OrgUserInviteTokenable> orgUserInviteTokenDataFactory)
IDataProtectorTokenFactory<OrgUserInviteTokenable> orgUserInviteTokenDataFactory,
IFeatureService featureService,
IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery)
{
_policyRepository = policyRepository;
_policyService = policyService;
Expand All @@ -53,10 +60,12 @@ public PoliciesController(
"OrganizationServiceDataProtector");

_orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory;
_featureService = featureService;
_organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery;
}

[HttpGet("{type}")]
public async Task<PolicyResponseModel> Get(Guid orgId, int type)
public async Task<PolicyDetailResponseModel> Get(Guid orgId, int type)
{
if (!await _currentContext.ManagePolicies(orgId))
{
Expand All @@ -65,10 +74,15 @@ public async Task<PolicyResponseModel> Get(Guid orgId, int type)
var policy = await _policyRepository.GetByOrganizationIdTypeAsync(orgId, (PolicyType)type);
if (policy == null)
{
return new PolicyResponseModel(new AdminConsoleEntities.Policy() { Type = (PolicyType)type, Enabled = false });
return new PolicyDetailResponseModel(new Policy { Type = (PolicyType)type });
}

return new PolicyResponseModel(policy);
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) && policy.Type is PolicyType.SingleOrg)
{
return await policy.GetSingleOrgPolicyDetailResponseAsync(_organizationHasVerifiedDomainsQuery);
}

return new PolicyDetailResponseModel(policy);
}

[HttpGet("")]
Expand All @@ -81,8 +95,8 @@ public async Task<ListResponseModel<PolicyResponseModel>> Get(string orgId)
}

var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgIdGuid);
var responses = policies.Select(p => new PolicyResponseModel(p));
return new ListResponseModel<PolicyResponseModel>(responses);

return new ListResponseModel<PolicyResponseModel>(policies.Select(p => new PolicyResponseModel(p)));
}

[AllowAnonymous]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;

namespace Bit.Api.AdminConsole.Models.Response.Helpers;

public static class PolicyDetailResponses
{
public static async Task<PolicyDetailResponseModel> GetSingleOrgPolicyDetailResponseAsync(this Policy policy, IOrganizationHasVerifiedDomainsQuery hasVerifiedDomainsQuery)
{
if (policy.Type is not PolicyType.SingleOrg)
{
throw new ArgumentException($"'{nameof(policy)}' must be of type '{nameof(PolicyType.SingleOrg)}'.", nameof(policy));
}

return new PolicyDetailResponseModel(policy, !await hasVerifiedDomainsQuery.HasVerifiedDomainsAsync(policy.OrganizationId));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Bit.Core.AdminConsole.Entities;

namespace Bit.Api.AdminConsole.Models.Response.Organizations;

public class PolicyDetailResponseModel : PolicyResponseModel
{
public PolicyDetailResponseModel(Policy policy, string obj = "policy") : base(policy, obj)
{
}

public PolicyDetailResponseModel(Policy policy, bool canToggleState) : base(policy)
{
CanToggleState = canToggleState;
}

/// <summary>
/// Indicates whether the Policy can be enabled/disabled
/// </summary>
public bool CanToggleState { get; set; } = true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
using Bit.Core.AdminConsole.Enums;
using Bit.Core.Models.Api;

namespace Bit.Core.AdminConsole.Models.Api.Response;
namespace Bit.Api.AdminConsole.Models.Response.Organizations;

public class PolicyResponseModel : ResponseModel
{
Expand Down
12 changes: 5 additions & 7 deletions src/Api/AdminConsole/Public/Controllers/PoliciesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@ public PoliciesController(
[ProducesResponseType((int)HttpStatusCode.NotFound)]
public async Task<IActionResult> Get(PolicyType type)
{
var policy = await _policyRepository.GetByOrganizationIdTypeAsync(
_currentContext.OrganizationId.Value, type);
var policy = await _policyRepository.GetByOrganizationIdTypeAsync(_currentContext.OrganizationId.Value, type);
if (policy == null)
{
return new NotFoundResult();
}
var response = new PolicyResponseModel(policy);
return new JsonResult(response);

return new JsonResult(new PolicyResponseModel(policy));
}

/// <summary>
Expand All @@ -62,9 +61,8 @@ public async Task<IActionResult> Get(PolicyType type)
public async Task<IActionResult> List()
{
var policies = await _policyRepository.GetManyByOrganizationIdAsync(_currentContext.OrganizationId.Value);
var policyResponses = policies.Select(p => new PolicyResponseModel(p));
var response = new ListResponseModel<PolicyResponseModel>(policyResponses);
return new JsonResult(response);

return new JsonResult(new ListResponseModel<PolicyResponseModel>(policies.Select(p => new PolicyResponseModel(p))));
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Api/Auth/Controllers/EmergencyAccessController.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
using Bit.Api.AdminConsole.Models.Request.Organizations;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Auth.Models.Request;
using Bit.Api.Auth.Models.Response;
using Bit.Api.Models.Response;
using Bit.Api.Vault.Models.Response;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Models.Api.Response;
using Bit.Core.Auth.Services;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
Expand Down
4 changes: 2 additions & 2 deletions src/Api/Vault/Models/Response/SyncResponseModel.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using Bit.Api.Models.Response;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Models.Response;
using Bit.Api.Tools.Models.Response;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Models.Api.Response;
using Bit.Core.AdminConsole.Models.Data.Provider;
using Bit.Core.Entities;
using Bit.Core.Models.Api;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand
private readonly IGlobalSettings _globalSettings;
private readonly IPolicyService _policyService;
private readonly IFeatureService _featureService;
private readonly IOrganizationService _organizationService;
private readonly ILogger<VerifyOrganizationDomainCommand> _logger;

public VerifyOrganizationDomainCommand(
Expand All @@ -30,7 +29,6 @@ public VerifyOrganizationDomainCommand(
IGlobalSettings globalSettings,
IPolicyService policyService,
IFeatureService featureService,
IOrganizationService organizationService,
ILogger<VerifyOrganizationDomainCommand> logger)
{
_organizationDomainRepository = organizationDomainRepository;
Expand All @@ -39,7 +37,6 @@ public VerifyOrganizationDomainCommand(
_globalSettings = globalSettings;
_policyService = policyService;
_featureService = featureService;
_organizationService = organizationService;
_logger = logger;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ private async Task RunValidatorAsync(IPolicyValidator validator, PolicyUpdate po
if (currentPolicy is not { Enabled: true } && policyUpdate.Enabled)
{
var missingRequiredPolicyTypes = validator.RequiredPolicies
.Where(requiredPolicyType =>
savedPoliciesDict.GetValueOrDefault(requiredPolicyType) is not { Enabled: true })
.Where(requiredPolicyType => savedPoliciesDict.GetValueOrDefault(requiredPolicyType) is not { Enabled: true })
.ToList();

if (missingRequiredPolicyTypes.Count != 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models;
using Bit.Core.Auth.Enums;
Expand All @@ -23,22 +24,28 @@ public class SingleOrgPolicyValidator : IPolicyValidator
private readonly IOrganizationRepository _organizationRepository;
private readonly ISsoConfigRepository _ssoConfigRepository;
private readonly ICurrentContext _currentContext;
private readonly IFeatureService _featureService;
private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand;
private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery;

public SingleOrgPolicyValidator(
IOrganizationUserRepository organizationUserRepository,
IMailService mailService,
IOrganizationRepository organizationRepository,
ISsoConfigRepository ssoConfigRepository,
ICurrentContext currentContext,
IRemoveOrganizationUserCommand removeOrganizationUserCommand)
IFeatureService featureService,
IRemoveOrganizationUserCommand removeOrganizationUserCommand,
IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery)
{
_organizationUserRepository = organizationUserRepository;
_mailService = mailService;
_organizationRepository = organizationRepository;
_ssoConfigRepository = ssoConfigRepository;
_currentContext = currentContext;
_featureService = featureService;
_removeOrganizationUserCommand = removeOrganizationUserCommand;
_organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery;
}

public IEnumerable<PolicyType> RequiredPolicies => [];
Expand Down Expand Up @@ -93,9 +100,21 @@ public async Task<string> ValidateAsync(PolicyUpdate policyUpdate, Policy? curre
if (policyUpdate is not { Enabled: true })
{
var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(policyUpdate.OrganizationId);
return ssoConfig.ValidateDecryptionOptionsNotEnabled([MemberDecryptionType.KeyConnector]);

var validateDecryptionErrorMessage = ssoConfig.ValidateDecryptionOptionsNotEnabled([MemberDecryptionType.KeyConnector]);

if (!string.IsNullOrWhiteSpace(validateDecryptionErrorMessage))
{
return validateDecryptionErrorMessage;
}

if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
&& await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(policyUpdate.OrganizationId))
{
return "The Single organization policy is required for organizations that have enabled domain verification.";
}
}

return "";
return string.Empty;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ private async Task HasVerifiedDomainsAsync(Organization org)
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
&& await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(org.Id))
{
throw new BadRequestException("Organization has verified domains.");
throw new BadRequestException("The Single organization policy is required for organizations that have enabled domain verification.");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
using AutoFixture;
using Bit.Api.AdminConsole.Models.Response.Helpers;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces;
using NSubstitute;
using Xunit;

namespace Bit.Api.Test.AdminConsole.Models.Response.Helpers;

public class PolicyDetailResponsesTests
{
[Fact]
public async Task GetSingleOrgPolicyDetailResponseAsync_GivenPolicyEntity_WhenIsSingleOrgTypeAndHasVerifiedDomains_ThenShouldNotBeAbleToToggle()
{
var fixture = new Fixture();

var policy = fixture.Build<Policy>()
.Without(p => p.Data)
.With(p => p.Type, PolicyType.SingleOrg)
.Create();

var querySub = Substitute.For<IOrganizationHasVerifiedDomainsQuery>();
querySub.HasVerifiedDomainsAsync(policy.OrganizationId)
.Returns(true);

var result = await policy.GetSingleOrgPolicyDetailResponseAsync(querySub);

Assert.False(result.CanToggleState);
}

[Fact]
public async Task GetSingleOrgPolicyDetailResponseAsync_GivenPolicyEntity_WhenIsNotSingleOrgType_ThenShouldThrowArgumentException()
{
var fixture = new Fixture();

var policy = fixture.Build<Policy>()
.Without(p => p.Data)
.With(p => p.Type, PolicyType.TwoFactorAuthentication)
.Create();

var querySub = Substitute.For<IOrganizationHasVerifiedDomainsQuery>();
querySub.HasVerifiedDomainsAsync(policy.OrganizationId)
.Returns(true);

var action = async () => await policy.GetSingleOrgPolicyDetailResponseAsync(querySub);

await Assert.ThrowsAsync<ArgumentException>("policy", action);
}

[Fact]
public async Task GetSingleOrgPolicyDetailResponseAsync_GivenPolicyEntity_WhenIsSingleOrgTypeAndDoesNotHaveVerifiedDomains_ThenShouldBeAbleToToggle()
{
var fixture = new Fixture();

var policy = fixture.Build<Policy>()
.Without(p => p.Data)
.With(p => p.Type, PolicyType.SingleOrg)
.Create();

var querySub = Substitute.For<IOrganizationHasVerifiedDomainsQuery>();
querySub.HasVerifiedDomainsAsync(policy.OrganizationId)
.Returns(false);

var result = await policy.GetSingleOrgPolicyDetailResponseAsync(querySub);

Assert.True(result.CanToggleState);
}
}
6 changes: 3 additions & 3 deletions test/Api.Test/Controllers/PoliciesControllerTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System.Security.Claims;
using System.Text.Json;
using Bit.Api.AdminConsole.Controllers;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Api.Response;
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Context;
Expand Down Expand Up @@ -157,7 +157,7 @@ public async Task Get_WhenUserCanManagePolicies_WithExistingType_ReturnsExisting
var result = await sutProvider.Sut.Get(orgId, type);

// Assert
Assert.IsType<PolicyResponseModel>(result);
Assert.IsType<PolicyDetailResponseModel>(result);
Assert.Equal(policy.Id, result.Id);
Assert.Equal(policy.Type, result.Type);
Assert.Equal(policy.Enabled, result.Enabled);
Expand All @@ -182,7 +182,7 @@ public async Task Get_WhenUserCanManagePolicies_WithNonExistingType_ReturnsDefau
var result = await sutProvider.Sut.Get(orgId, type);

// Assert
Assert.IsType<PolicyResponseModel>(result);
Assert.IsType<PolicyDetailResponseModel>(result);
Assert.Equal(result.Type, (PolicyType)type);
Assert.False(result.Enabled);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,6 @@ public async Task SaveAsync_GivenOrganizationUsingPoliciesAndHasVerifiedDomains_
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy, null));

Assert.Equal("Organization has verified domains.", badRequestException.Message);
Assert.Equal("The Single organization policy is required for organizations that have enabled domain verification.", badRequestException.Message);
}
}

0 comments on commit 1dec51b

Please sign in to comment.