Upstream/bitwarden-server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2024-12-31 18:27:49 +01:00
Code Issues Projects Releases Wiki Activity

[PM-13836] Refactor IPolicyService to remove unnecessary IOrganizationService dependency (#4914)

Browse Source
This commit is contained in:
Rui Tomé 2024-10-22 10:38:01 +01:00 committed by GitHub
parent dfa411131d
commit 7b5e0e4a64
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 13 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Api/AdminConsole/Controllers/PoliciesController.cs
View File

@ -25,7 +25,6 @@ public class PoliciesController : Controller
{
private readonly IPolicyRepository _policyRepository;
private readonly IPolicyService _policyService;
private readonly IOrganizationService _organizationService;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IUserService _userService;
private readonly ICurrentContext _currentContext;
@ -36,7 +35,6 @@ public class PoliciesController : Controller
public PoliciesController(
IPolicyRepository policyRepository,
IPolicyService policyService,
IOrganizationService organizationService,
IOrganizationUserRepository organizationUserRepository,
IUserService userService,
ICurrentContext currentContext,
@ -46,7 +44,6 @@ public class PoliciesController : Controller
{
_policyRepository = policyRepository;
_policyService = policyService;
_organizationService = organizationService;
_organizationUserRepository = organizationUserRepository;
_userService = userService;
_currentContext = currentContext;
@ -185,7 +182,7 @@ public class PoliciesController : Controller
}
var userId = _userService.GetProperUserId(User);
await _policyService.SaveAsync(policy, _organizationService, userId);
await _policyService.SaveAsync(policy, userId);
return new PolicyResponseModel(policy);
}
}

6
src/Api/AdminConsole/Public/Controllers/PoliciesController.cs
View File

@ -6,7 +6,6 @@ using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Context;
using Bit.Core.Services;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
@ -18,18 +17,15 @@ public class PoliciesController : Controller
{
private readonly IPolicyRepository _policyRepository;
private readonly IPolicyService _policyService;
private readonly IOrganizationService _organizationService;
private readonly ICurrentContext _currentContext;
public PoliciesController(
IPolicyRepository policyRepository,
IPolicyService policyService,
IOrganizationService organizationService,
ICurrentContext currentContext)
{
_policyRepository = policyRepository;
_policyService = policyService;
_organizationService = organizationService;
_currentContext = currentContext;
}
@ -96,7 +92,7 @@ public class PoliciesController : Controller
{
policy = model.ToPolicy(policy);
}
await _policyService.SaveAsync(policy, _organizationService, null);
await _policyService.SaveAsync(policy, null);
var response = new PolicyResponseModel(policy);
return new JsonResult(response);
}

3
src/Core/AdminConsole/Services/IPolicyService.cs
View File

@ -4,13 +4,12 @@ using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Services;
namespace Bit.Core.AdminConsole.Services;
public interface IPolicyService
{
Task SaveAsync(Policy policy, IOrganizationService organizationService, Guid? savingUserId);
Task SaveAsync(Policy policy, Guid? savingUserId);
/// <summary>
/// Get the combined master password policy options for the specified user.

6
src/Core/AdminConsole/Services/Implementations/PolicyService.cs
View File

@ -61,7 +61,7 @@ public class PolicyService : IPolicyService
_removeOrganizationUserCommand = removeOrganizationUserCommand;
}
public async Task SaveAsync(Policy policy, IOrganizationService organizationService, Guid? savingUserId)
public async Task SaveAsync(Policy policy, Guid? savingUserId)
{
if (_featureService.IsEnabled(FeatureFlagKeys.Pm13322AddPolicyDefinitions))
{
@ -111,7 +111,7 @@ public class PolicyService : IPolicyService
return;
}
await EnablePolicyAsync(policy, org, organizationService, savingUserId);
await EnablePolicyAsync(policy, org, savingUserId);
}
public async Task<MasterPasswordPolicyData> GetMasterPasswordPolicyForUserAsync(User user)
@ -285,7 +285,7 @@ public class PolicyService : IPolicyService
await _eventService.LogPolicyEventAsync(policy, EventType.Policy_Updated);
}
private async Task EnablePolicyAsync(Policy policy, Organization org, IOrganizationService organizationService, Guid? savingUserId)
private async Task EnablePolicyAsync(Policy policy, Organization org, Guid? savingUserId)
{
var currentPolicy = await _policyRepository.GetByIdAsync(policy.Id);
if (!currentPolicy?.Enabled ?? true)

9
src/Core/Auth/Services/Implementations/SsoConfigService.cs
View File

@ -20,7 +20,6 @@ public class SsoConfigService : ISsoConfigService
private readonly IPolicyService _policyService;
private readonly IOrganizationRepository _organizationRepository;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IOrganizationService _organizationService;
private readonly IEventService _eventService;
public SsoConfigService(
@ -29,7 +28,6 @@ public class SsoConfigService : ISsoConfigService
IPolicyService policyService,
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
IOrganizationService organizationService,
IEventService eventService)
{
_ssoConfigRepository = ssoConfigRepository;
@ -37,7 +35,6 @@ public class SsoConfigService : ISsoConfigService
_policyService = policyService;
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
_organizationService = organizationService;
_eventService = eventService;
}
@ -71,20 +68,20 @@ public class SsoConfigService : ISsoConfigService
singleOrgPolicy.Enabled = true;
await _policyService.SaveAsync(singleOrgPolicy, _organizationService, null);
await _policyService.SaveAsync(singleOrgPolicy, null);
var resetPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.ResetPassword) ??
new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.ResetPassword, };
resetPolicy.Enabled = true;
resetPolicy.SetDataModel(new ResetPasswordDataModel { AutoEnrollEnabled = true });
await _policyService.SaveAsync(resetPolicy, _organizationService, null);
await _policyService.SaveAsync(resetPolicy, null);
var ssoRequiredPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.RequireSso) ??
new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.RequireSso, };
ssoRequiredPolicy.Enabled = true;
await _policyService.SaveAsync(ssoRequiredPolicy, _organizationService, null);
await _policyService.SaveAsync(ssoRequiredPolicy, null);
}
await LogEventsAsync(config, oldConfig);

23
test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs
View File

@ -34,7 +34,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Organization not found", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -61,7 +60,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("cannot use policies", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -93,7 +91,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Single Sign-On Authentication policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -124,7 +121,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Maximum Vault Timeout policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -161,7 +157,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Key Connector is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -189,7 +184,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -222,7 +216,7 @@ public class PolicyServiceTests
var utcNow = DateTime.UtcNow;
await sutProvider.Sut.SaveAsync(policy, Substitute.For<IOrganizationService>(), Guid.NewGuid());
await sutProvider.Sut.SaveAsync(policy, Guid.NewGuid());
await sutProvider.GetDependency<IEventService>().Received()
.LogPolicyEventAsync(policy, EventType.Policy_Updated);
@ -252,7 +246,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -353,14 +346,13 @@ public class PolicyServiceTests
(orgUserDetailAdmin, false),
});
var organizationService = Substitute.For<IOrganizationService>();
var removeOrganizationUserCommand = sutProvider.GetDependency<IRemoveOrganizationUserCommand>();
var utcNow = DateTime.UtcNow;
var savingUserId = Guid.NewGuid();
await sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId);
await sutProvider.Sut.SaveAsync(policy, savingUserId);
await removeOrganizationUserCommand.Received()
.RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWithout2FA.Id, savingUserId);
@ -468,13 +460,12 @@ public class PolicyServiceTests
(orgUserDetailAdmin.UserId.Value, false),
});
var organizationService = Substitute.For<IOrganizationService>();
var removeOrganizationUserCommand = sutProvider.GetDependency<IRemoveOrganizationUserCommand>();
var savingUserId = Guid.NewGuid();
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId));
() => sutProvider.Sut.SaveAsync(policy, savingUserId));
Assert.Contains("Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -541,13 +532,11 @@ public class PolicyServiceTests
(orgUserDetail.UserId.Value, false),
});
var organizationService = Substitute.For<IOrganizationService>();
var utcNow = DateTime.UtcNow;
var savingUserId = Guid.NewGuid();
await sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId);
await sutProvider.Sut.SaveAsync(policy, savingUserId);
await sutProvider.GetDependency<IEventService>().Received()
.LogPolicyEventAsync(policy, EventType.Policy_Updated);
@ -590,7 +579,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Trusted device encryption is on and requires this policy.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -626,7 +614,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Trusted device encryption is on and requires this policy.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -659,7 +646,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);
@ -692,7 +678,6 @@ public class PolicyServiceTests
var badRequestException = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SaveAsync(policy,
Substitute.For<IOrganizationService>(),
Guid.NewGuid()));
Assert.Contains("Account recovery policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase);

3
test/Core.Test/Auth/Services/SsoConfigServiceTests.cs
View File

@ -11,7 +11,6 @@ using Bit.Core.Auth.Services;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
@ -342,14 +341,12 @@ public class SsoConfigServiceTests
await sutProvider.GetDependency<IPolicyService>().Received(1)
.SaveAsync(
Arg.Is<Policy>(t => t.Type == PolicyType.SingleOrg),
Arg.Any<IOrganizationService>(),
null
);
await sutProvider.GetDependency<IPolicyService>().Received(1)
.SaveAsync(
Arg.Is<Policy>(t => t.Type == PolicyType.ResetPassword && t.GetDataModel<ResetPasswordDataModel>().AutoEnrollEnabled),
Arg.Any<IOrganizationService>(),
null
);