diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index cfff7b257..19858ae18 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -174,7 +174,9 @@ public class CiphersController : Controller public async Task PostAdmin([FromBody] CipherCreateRequestModel model) { var cipher = model.Cipher.ToOrganizationCipher(); - if (!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + // Only users that can edit all ciphers can create new ciphers via the admin endpoint + // Other users should use the regular POST/create endpoint + if (!await CanEditAllCiphersAsync(cipher.OrganizationId.Value)) { throw new NotFoundException(); } @@ -226,7 +228,7 @@ public class CiphersController : Controller ValidateClientVersionForFido2CredentialSupport(cipher); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } @@ -318,6 +320,28 @@ public class CiphersController : Controller return new ListResponseModel(allOrganizationCipherResponses); } + /// + /// Permission helper to determine if the current user can use the "/admin" variants of the cipher endpoints. + /// Allowed for custom users with EditAnyCollection, providers, unrestricted owners and admins (allowAdminAccess setting is ON). + /// Falls back to original EditAnyCollection permission check for when V1 flag is disabled. + /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 + /// + private async Task CanEditCipherAsAdminAsync(Guid organizationId, IEnumerable cipherIds) + { + // Pre-Flexible collections V1 only needs to check EditAnyCollection + if (!await UseFlexibleCollectionsV1Async(organizationId)) + { + return await _currentContext.EditAnyCollection(organizationId); + } + + if (await CanEditCiphersAsync(organizationId, cipherIds)) + { + return true; + } + + return false; + } + /// /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 /// @@ -584,14 +608,23 @@ public class CiphersController : Controller { var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetByIdAsync(new Guid(id)); + if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } - await _cipherService.SaveCollectionsAsync(cipher, - model.CollectionIds.Select(c => new Guid(c)), userId, true); + var collectionIds = model.CollectionIds.Select(c => new Guid(c)).ToList(); + + // In V1, we still need to check if the user can edit the collections they're submitting + // This should only happen for unassigned ciphers (otherwise restricted admins would use the normal collections endpoint) + if (await UseFlexibleCollectionsV1Async(cipher.OrganizationId.Value) && !await CanEditItemsInCollections(cipher.OrganizationId.Value, collectionIds)) + { + throw new NotFoundException(); + } + + await _cipherService.SaveCollectionsAsync(cipher, collectionIds, userId, true); } [HttpPost("bulk-collections")] @@ -642,7 +675,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetByIdAsync(new Guid(id)); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } @@ -674,14 +707,21 @@ public class CiphersController : Controller "Consider using the \"Purge Vault\" option instead."); } - if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) || - !await _currentContext.EditAnyCollection(new Guid(model.OrganizationId))) + if (model == null) + { + throw new NotFoundException(); + } + + var cipherIds = model.Ids.Select(i => new Guid(i)).ToList(); + + if (string.IsNullOrWhiteSpace(model.OrganizationId) || + !await CanEditCipherAsAdminAsync(new Guid(model.OrganizationId), cipherIds)) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User).Value; - await _cipherService.DeleteManyAsync(model.Ids.Select(i => new Guid(i)), userId, new Guid(model.OrganizationId), true); + await _cipherService.DeleteManyAsync(cipherIds, userId, new Guid(model.OrganizationId), true); } [HttpPut("{id}/delete")] @@ -702,7 +742,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetByIdAsync(new Guid(id)); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } @@ -730,14 +770,21 @@ public class CiphersController : Controller throw new BadRequestException("You can only delete up to 500 items at a time."); } - if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) || - !await _currentContext.EditAnyCollection(new Guid(model.OrganizationId))) + if (model == null) + { + throw new NotFoundException(); + } + + var cipherIds = model.Ids.Select(i => new Guid(i)).ToList(); + + if (string.IsNullOrWhiteSpace(model.OrganizationId) || + !await CanEditCipherAsAdminAsync(new Guid(model.OrganizationId), cipherIds)) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User).Value; - await _cipherService.SoftDeleteManyAsync(model.Ids.Select(i => new Guid(i)), userId, new Guid(model.OrganizationId), true); + await _cipherService.SoftDeleteManyAsync(cipherIds, userId, new Guid(model.OrganizationId), true); } [HttpPut("{id}/restore")] @@ -760,7 +807,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(new Guid(id)); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } @@ -793,13 +840,19 @@ public class CiphersController : Controller throw new BadRequestException("You can only restore up to 500 items at a time."); } - if (model == null || model.OrganizationId == default || !await _currentContext.EditAnyCollection(model.OrganizationId)) + if (model == null) + { + throw new NotFoundException(); + } + + var cipherIdsToRestore = new HashSet(model.Ids.Select(i => new Guid(i))); + + if (model.OrganizationId == default || !await CanEditCipherAsAdminAsync(model.OrganizationId, cipherIdsToRestore)) { throw new NotFoundException(); } var userId = _userService.GetProperUserId(User).Value; - var cipherIdsToRestore = new HashSet(model.Ids.Select(i => new Guid(i))); var restoredCiphers = await _cipherService.RestoreManyAsync(cipherIdsToRestore, userId, model.OrganizationId, true); var responses = restoredCiphers.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp)); @@ -894,7 +947,7 @@ public class CiphersController : Controller await GetByIdAsync(id, userId); if (cipher == null || (request.AdminRequest && (!cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)))) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })))) { throw new NotFoundException(); } @@ -998,7 +1051,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(idGuid); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } @@ -1064,7 +1117,7 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetByIdAsync(idGuid); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value)) + !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id })) { throw new NotFoundException(); } diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index 78e3d04ef..675bde5da 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -1,9 +1,18 @@ using System.Security.Claims; using Bit.Api.Vault.Controllers; +using Bit.Api.Vault.Models; using Bit.Api.Vault.Models.Request; +using Bit.Core; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations; using Bit.Core.Services; +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Enums; using Bit.Core.Vault.Models.Data; using Bit.Core.Vault.Repositories; +using Bit.Core.Vault.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -42,4 +51,150 @@ public class CiphersControllerTests Assert.Equal(folderId, result.FolderId); Assert.Equal(isFavorite, result.Favorite); } + + [Theory] + [BitAutoData(OrganizationUserType.Admin, true, true)] + [BitAutoData(OrganizationUserType.Owner, true, true)] + [BitAutoData(OrganizationUserType.Custom, false, true)] + [BitAutoData(OrganizationUserType.Custom, true, true)] + [BitAutoData(OrganizationUserType.Admin, false, false)] + [BitAutoData(OrganizationUserType.Owner, false, false)] + [BitAutoData(OrganizationUserType.Custom, false, false)] + public async Task CanEditCiphersAsAdminAsync_FlexibleCollections_Success( + OrganizationUserType userType, bool allowAdminsAccessToAllItems, bool shouldSucceed, + CurrentContextOrganization organization, Guid userId, SutProvider sutProvider + ) + { + organization.Type = userType; + if (userType == OrganizationUserType.Custom) + { + // Assume custom users have EditAnyCollections for success case + organization.Permissions.EditAnyCollection = shouldSucceed; + } + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility + { + Id = organization.Id, + FlexibleCollections = true, + AllowAdminAccessToAllCollectionItems = allowAdminsAccessToAllItems + }); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + + var requestModel = new CipherCreateRequestModel + { + Cipher = new CipherRequestModel { OrganizationId = organization.Id.ToString(), Type = CipherType.Login, Login = new CipherLoginModel() }, + CollectionIds = new List() + }; + + if (shouldSucceed) + { + await sutProvider.Sut.PostAdmin(requestModel); + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .SaveAsync(default, default, default); + } + else + { + await Assert.ThrowsAsync(() => sutProvider.Sut.PostAdmin(requestModel)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .SaveAsync(default, default, default); + } + } + + /// + /// To be removed after FlexibleCollections is fully released + /// + [Theory] + [BitAutoData(true, true)] + [BitAutoData(false, true)] + [BitAutoData(true, false)] + [BitAutoData(false, false)] + public async Task CanEditCiphersAsAdminAsync_NonFlexibleCollections( + bool v1Enabled, bool shouldSucceed, CurrentContextOrganization organization, Guid userId, Cipher cipher, SutProvider sutProvider + ) + { + cipher.OrganizationId = organization.Id; + sutProvider.GetDependency().EditAnyCollection(organization.Id).Returns(shouldSucceed); + + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility + { + Id = organization.Id, + FlexibleCollections = false, + AllowAdminAccessToAllCollectionItems = false + }); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(v1Enabled); + sutProvider.GetDependency().GetByIdAsync(cipher.Id).Returns(cipher); + + if (shouldSucceed) + { + await sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()); + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .DeleteAsync(default, default); + } + else + { + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString())); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .DeleteAsync(default, default); + } + } + + [Theory] + [BitAutoData(false, true)] + [BitAutoData(true, true)] + [BitAutoData(false, false)] + [BitAutoData(true, false)] + public async Task CanEditCiphersAsAdminAsync_Providers( + bool fcV1Enabled, bool shouldSucceed, Cipher cipher, CurrentContextOrganization organization, Guid userId, SutProvider sutProvider + ) + { + cipher.OrganizationId = organization.Id; + if (fcV1Enabled) + { + sutProvider.GetDependency().ProviderUserForOrgAsync(organization.Id).Returns(shouldSucceed); + } + else + { + sutProvider.GetDependency().EditAnyCollection(organization.Id).Returns(shouldSucceed); + } + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); + + sutProvider.GetDependency().GetByIdAsync(cipher.Id).Returns(cipher); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(organization.Id).Returns(new List { cipher }); + + sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility + { + Id = organization.Id, + FlexibleCollections = fcV1Enabled, // Assume FlexibleCollections is enabled if v1 is enabled + AllowAdminAccessToAllCollectionItems = false + }); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(fcV1Enabled); + + if (shouldSucceed) + { + await sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()); + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .DeleteAsync(default, default); + } + else + { + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString())); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .DeleteAsync(default, default); + } + + if (fcV1Enabled) + { + await sutProvider.GetDependency().Received().ProviderUserForOrgAsync(organization.Id); + } + else + { + await sutProvider.GetDependency().Received().EditAnyCollection(organization.Id); + } + } }