1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00

[AC-1873] Fix: restore logic assigning Managers to new collections server-side (#3498)

* Restore pre-flexible collections logic to assign managers to new collections

* Dont overwrite existing access

* Fix and add tests
This commit is contained in:
Thomas Rittson 2023-12-02 01:28:10 +10:00 committed by GitHub
parent f46ea0bf3b
commit 519b3dea24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 86 additions and 4 deletions

View File

@ -4,7 +4,9 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core; using Bit.Core;
using Bit.Core.Context; using Bit.Core.Context;
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces;
using Bit.Core.Repositories; using Bit.Core.Repositories;
using Bit.Core.Services; using Bit.Core.Services;
@ -26,6 +28,7 @@ public class CollectionsController : Controller
private readonly ICurrentContext _currentContext; private readonly ICurrentContext _currentContext;
private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand; private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand;
private readonly IFeatureService _featureService; private readonly IFeatureService _featureService;
private readonly IOrganizationUserRepository _organizationUserRepository;
public CollectionsController( public CollectionsController(
ICollectionRepository collectionRepository, ICollectionRepository collectionRepository,
@ -35,7 +38,8 @@ public class CollectionsController : Controller
IAuthorizationService authorizationService, IAuthorizationService authorizationService,
ICurrentContext currentContext, ICurrentContext currentContext,
IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand, IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand,
IFeatureService featureService) IFeatureService featureService,
IOrganizationUserRepository organizationUserRepository)
{ {
_collectionRepository = collectionRepository; _collectionRepository = collectionRepository;
_collectionService = collectionService; _collectionService = collectionService;
@ -45,6 +49,7 @@ public class CollectionsController : Controller
_currentContext = currentContext; _currentContext = currentContext;
_bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand; _bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand;
_featureService = featureService; _featureService = featureService;
_organizationUserRepository = organizationUserRepository;
} }
private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
@ -168,7 +173,29 @@ public class CollectionsController : Controller
} }
var groups = model.Groups?.Select(g => g.ToSelectionReadOnly()); var groups = model.Groups?.Select(g => g.ToSelectionReadOnly());
var users = model.Users?.Select(g => g.ToSelectionReadOnly()); var users = model.Users?.Select(g => g.ToSelectionReadOnly()).ToList() ?? new List<CollectionAccessSelection>();
// Pre-flexible collections logic assigned Managers to collections they create
var assignUserToCollection =
!FlexibleCollectionsIsEnabled &&
!await _currentContext.EditAnyCollection(orgId) &&
await _currentContext.EditAssignedCollections(orgId);
var isNewCollection = collection.Id == default;
if (assignUserToCollection && isNewCollection && _currentContext.UserId.HasValue)
{
var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, _currentContext.UserId.Value);
// don't add duplicate access if the user has already specified it themselves
var existingAccess = users.Any(u => u.Id == orgUser.Id);
if (orgUser is { Status: OrganizationUserStatusType.Confirmed } && !existingAccess)
{
users.Add(new CollectionAccessSelection
{
Id = orgUser.Id,
ReadOnly = false
});
}
}
await _collectionService.SaveAsync(collection, groups, users); await _collectionService.SaveAsync(collection, groups, users);
return new CollectionResponseModel(collection); return new CollectionResponseModel(collection);

View File

@ -3,6 +3,7 @@ using Bit.Api.Models.Request;
using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities;
using Bit.Core.Context; using Bit.Core.Context;
using Bit.Core.Entities; using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions; using Bit.Core.Exceptions;
using Bit.Core.Models.Data; using Bit.Core.Models.Data;
using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces;
@ -12,6 +13,8 @@ using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute; using NSubstitute;
using Xunit; using Xunit;
using Collection = Bit.Core.Entities.Collection;
using User = Bit.Core.Entities.User;
namespace Bit.Api.Test.Controllers; namespace Bit.Api.Test.Controllers;
@ -24,8 +27,11 @@ namespace Bit.Api.Test.Controllers;
public class LegacyCollectionsControllerTests public class LegacyCollectionsControllerTests
{ {
[Theory, BitAutoData] [Theory, BitAutoData]
public async Task Post_Success(Guid orgId, SutProvider<CollectionsController> sutProvider) public async Task Post_Manager_AssignsToCollection_Success(Guid orgId, OrganizationUser orgUser, SutProvider<CollectionsController> sutProvider)
{ {
orgUser.Type = OrganizationUserType.Manager;
orgUser.Status = OrganizationUserStatusType.Confirmed;
sutProvider.GetDependency<ICurrentContext>() sutProvider.GetDependency<ICurrentContext>()
.OrganizationManager(orgId) .OrganizationManager(orgId)
.Returns(true); .Returns(true);
@ -34,6 +40,15 @@ public class LegacyCollectionsControllerTests
.EditAnyCollection(orgId) .EditAnyCollection(orgId)
.Returns(false); .Returns(false);
sutProvider.GetDependency<ICurrentContext>()
.EditAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>().UserId = orgUser.UserId;
sutProvider.GetDependency<IOrganizationUserRepository>().GetByOrganizationAsync(orgId, orgUser.UserId.Value)
.Returns(orgUser);
var collectionRequest = new CollectionRequestModel var collectionRequest = new CollectionRequestModel
{ {
Name = "encrypted_string", Name = "encrypted_string",
@ -42,9 +57,49 @@ public class LegacyCollectionsControllerTests
_ = await sutProvider.Sut.Post(orgId, collectionRequest); _ = await sutProvider.Sut.Post(orgId, collectionRequest);
var test = sutProvider.GetDependency<ICollectionService>().ReceivedCalls();
await sutProvider.GetDependency<ICollectionService>() await sutProvider.GetDependency<ICollectionService>()
.Received(1) .Received(1)
.SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(), null); .SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(),
Arg.Is<IEnumerable<CollectionAccessSelection>>(users => users.Any(u => u.Id == orgUser.Id && !u.ReadOnly && !u.HidePasswords && !u.Manage)));
}
[Theory, BitAutoData]
public async Task Post_Owner_DoesNotAssignToCollection_Success(Guid orgId, OrganizationUser orgUser, SutProvider<CollectionsController> sutProvider)
{
orgUser.Type = OrganizationUserType.Owner;
orgUser.Status = OrganizationUserStatusType.Confirmed;
sutProvider.GetDependency<ICurrentContext>()
.OrganizationManager(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.EditAnyCollection(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.EditAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>().UserId = orgUser.UserId;
sutProvider.GetDependency<IOrganizationUserRepository>().GetByOrganizationAsync(orgId, orgUser.UserId.Value)
.Returns(orgUser);
var collectionRequest = new CollectionRequestModel
{
Name = "encrypted_string",
ExternalId = "my_external_id"
};
_ = await sutProvider.Sut.Post(orgId, collectionRequest);
var test = sutProvider.GetDependency<ICollectionService>().ReceivedCalls();
await sutProvider.GetDependency<ICollectionService>()
.Received(1)
.SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(),
Arg.Is<IEnumerable<CollectionAccessSelection>>(users => !users.Any()));
} }
[Theory, BitAutoData] [Theory, BitAutoData]