-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AC-2971] Draft: Policy strategy pattern #47
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Entities; | ||
using Bit.Core.AdminConsole.Enums; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies; | ||
|
||
public interface IPolicyStrategy | ||
{ | ||
/// <summary> | ||
/// The PolicyType that the strategy is responsible for handling. | ||
/// </summary> | ||
public PolicyType Type { get; } | ||
|
||
/// <summary> | ||
/// A method that is called when the policy state changes from disabled to enabled, before | ||
/// it is saved to the database. | ||
/// For example, this can be used for validation before saving or for side effects. | ||
/// </summary> | ||
/// <param name="policy">The updated policy object.</param> | ||
/// <param name="savingUserId">The current user who is updating the policy.</param> | ||
public Task HandleEnable(Policy policy, Guid? savingUserId); | ||
|
||
/// <summary> | ||
/// A method that is called when the policy state changes from enabled to disabled, before | ||
/// it is saved to the database. | ||
/// For example, this can be used for validation before saving or for side effects. | ||
/// </summary> | ||
/// <param name="policy">The updated policy object.</param> | ||
/// <param name="savingUserId">The current user who is updating the policy.</param> | ||
public Task HandleDisable(Policy policy, Guid? savingUserId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Entities; | ||
using Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.AdminConsole.Repositories; | ||
using Bit.Core.Auth.Enums; | ||
using Bit.Core.Auth.Repositories; | ||
using Bit.Core.Enums; | ||
using Bit.Core.Exceptions; | ||
using Bit.Core.Repositories; | ||
using Bit.Core.Services; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations; | ||
|
||
public class SingleOrgPolicyStrategy : IPolicyStrategy | ||
{ | ||
public PolicyType Type => PolicyType.SingleOrg; | ||
|
||
private readonly IOrganizationUserRepository _organizationUserRepository; | ||
private readonly IMailService _mailService; | ||
private readonly IOrganizationRepository _organizationRepository; | ||
private readonly IPolicyRepository _policyRepository; | ||
private readonly ISsoConfigRepository _ssoConfigRepository; | ||
|
||
public SingleOrgPolicyStrategy( | ||
IOrganizationUserRepository organizationUserRepository, | ||
IMailService mailService, | ||
IOrganizationRepository organizationRepository, | ||
IPolicyRepository policyRepository, | ||
ISsoConfigRepository ssoConfigRepository) | ||
{ | ||
_organizationUserRepository = organizationUserRepository; | ||
_mailService = mailService; | ||
_organizationRepository = organizationRepository; | ||
_policyRepository = policyRepository; | ||
_ssoConfigRepository = ssoConfigRepository; | ||
} | ||
|
||
public async Task HandleEnable(Policy policy, Guid? savingUserId) | ||
{ | ||
// Remove non-compliant users | ||
var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(policy.OrganizationId); | ||
var org = await _organizationRepository.GetByIdAsync(policy.OrganizationId); | ||
if (org == null) | ||
{ | ||
throw new NotFoundException("Organization not found."); | ||
} | ||
|
||
var removableOrgUsers = orgUsers.Where(ou => | ||
ou.Status != OrganizationUserStatusType.Invited && | ||
ou.Status != OrganizationUserStatusType.Revoked && | ||
ou.Type != OrganizationUserType.Owner && | ||
ou.Type != OrganizationUserType.Admin && | ||
ou.UserId != savingUserId | ||
).ToList(); | ||
|
||
var userOrgs = await _organizationUserRepository.GetManyByManyUsersAsync( | ||
removableOrgUsers.Select(ou => ou.UserId!.Value)); | ||
Comment on lines
+55
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: This query might be inefficient for large numbers of users. Consider optimizing or paginating |
||
foreach (var orgUser in removableOrgUsers) | ||
{ | ||
if (userOrgs.Any(ou => ou.UserId == orgUser.UserId | ||
&& ou.OrganizationId != org.Id | ||
&& ou.Status != OrganizationUserStatusType.Invited)) | ||
{ | ||
// TODO: OrganizationService causes a circular dependency here, we either pass it into all handlers | ||
// TODO: like we do with PolicyService at the moment, or we investigate and break that circle | ||
// await _organizationService.DeleteUserAsync(policy.OrganizationId, orgUser.Id, | ||
// savingUserId); | ||
Comment on lines
+63
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Circular dependency issue needs to be resolved before this code can be fully implemented |
||
await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync( | ||
org.DisplayName(), orgUser.Email); | ||
} | ||
} | ||
} | ||
|
||
public async Task HandleDisable(Policy policy, Guid? savingUserId) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: savingUserId parameter is unused in this method. Consider removing if not needed |
||
{ | ||
// Do not allow this policy to be disabled if a dependent policy is still enabled | ||
var policies = await _policyRepository.GetManyByOrganizationIdAsync(policy.OrganizationId); | ||
if (policies.Any(p => p.Type == PolicyType.RequireSso && p.Enabled)) | ||
{ | ||
throw new BadRequestException("Single Sign-On Authentication policy is enabled."); | ||
} | ||
|
||
if (policies.Any(p => p.Type == PolicyType.MaximumVaultTimeout && p.Enabled)) | ||
{ | ||
throw new BadRequestException("Maximum Vault Timeout policy is enabled."); | ||
} | ||
|
||
if (policies.Any(p => p.Type == PolicyType.ResetPassword && p.Enabled)) | ||
{ | ||
throw new BadRequestException("Account Recovery policy is enabled."); | ||
} | ||
Comment on lines
+76
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider refactoring repeated policy checks into a separate method for better maintainability |
||
|
||
// Do not allow this policy to be disabled if Key Connector is being used | ||
var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(policy.OrganizationId); | ||
if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.KeyConnector) | ||
{ | ||
throw new BadRequestException("Key Connector is enabled."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations; | ||
using Bit.Core.AdminConsole.Services; | ||
using Bit.Core.AdminConsole.Services.Implementations; | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies; | ||
|
||
public static class PolicyServiceCollectionExtensions | ||
{ | ||
public static void AddPolicyServices(this IServiceCollection services) | ||
{ | ||
services.AddScoped<IPolicyService, PolicyService>(); | ||
services.AddScoped<IPolicyStrategy, SingleOrgPolicyStrategy>(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider making this property read-only to ensure immutability