diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 2f0a48043..b1116b137 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -36,31 +36,29 @@ namespace Bit.Api.Controllers } [HttpGet("{id}")] - public async Task Get(string orgId, string id) + public async Task Get(Guid orgId, Guid id) { if (!await CanViewCollectionAsync(orgId, id)) { throw new NotFoundException(); } - var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); + var collection = await GetCollectionAsync(id, orgId); return new CollectionResponseModel(collection); } [HttpGet("{id}/details")] - public async Task GetDetails(string orgId, string id) + public async Task GetDetails(Guid orgId, Guid id) { - var orgIdGuid = new Guid(orgId); - if (!await ViewAtLeastOneCollectionAsync(orgIdGuid) && !await _currentContext.ManageUsers(orgIdGuid)) + if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId)) { throw new NotFoundException(); } - var idGuid = new Guid(id); - if (await _currentContext.ViewAllCollections(orgIdGuid)) + if (await _currentContext.ViewAllCollections(orgId)) { - var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(idGuid); - if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgIdGuid) + var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(id); + if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgId) { throw new NotFoundException(); } @@ -68,9 +66,9 @@ namespace Bit.Api.Controllers } else { - var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(idGuid, + var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(id, _currentContext.UserId.Value); - if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgIdGuid) + if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgId) { throw new NotFoundException(); } @@ -79,24 +77,23 @@ namespace Bit.Api.Controllers } [HttpGet("")] - public async Task> Get(string orgId) + public async Task> Get(Guid orgId) { - var orgIdGuid = new Guid(orgId); - if (!await _currentContext.ViewAllCollections(orgIdGuid) && !await _currentContext.ManageUsers(orgIdGuid)) + if (!await _currentContext.ViewAllCollections(orgId) && !await _currentContext.ManageUsers(orgId)) { throw new NotFoundException(); } IEnumerable orgCollections; - if (await _currentContext.OrganizationAdmin(orgIdGuid)) + if (await _currentContext.OrganizationAdmin(orgId)) { // Admins, Owners and Providers can access all items even if not assigned to them - orgCollections = await _collectionRepository.GetManyByOrganizationIdAsync(orgIdGuid); + orgCollections = await _collectionRepository.GetManyByOrganizationIdAsync(orgId); } else { var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value); - orgCollections = collections.Where(c => c.OrganizationId == orgIdGuid); + orgCollections = collections.Where(c => c.OrganizationId == orgId); } var responses = orgCollections.Select(c => new CollectionResponseModel(c)); @@ -113,28 +110,27 @@ namespace Bit.Api.Controllers } [HttpGet("{id}/users")] - public async Task> GetUsers(string orgId, string id) + public async Task> GetUsers(Guid orgId, Guid id) { - var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); + var collection = await GetCollectionAsync(id, orgId); var collectionUsers = await _collectionRepository.GetManyUsersByIdAsync(collection.Id); var responses = collectionUsers.Select(cu => new SelectionReadOnlyResponseModel(cu)); return responses; } [HttpPost("")] - public async Task Post(string orgId, [FromBody] CollectionRequestModel model) + public async Task Post(Guid orgId, [FromBody] CollectionRequestModel model) { - var orgIdGuid = new Guid(orgId); - var collection = model.ToCollection(orgIdGuid); + var collection = model.ToCollection(orgId); - if (!await CanCreateCollection(orgIdGuid, collection.Id) && - !await CanEditCollectionAsync(orgIdGuid, collection.Id)) + if (!await CanCreateCollection(orgId, collection.Id) && + !await CanEditCollectionAsync(orgId, collection.Id)) { throw new NotFoundException(); } - var assignUserToCollection = !(await _currentContext.EditAnyCollection(orgIdGuid)) && - await _currentContext.EditAssignedCollections(orgIdGuid); + var assignUserToCollection = !(await _currentContext.EditAnyCollection(orgId)) && + await _currentContext.EditAssignedCollections(orgId); await _collectionService.SaveAsync(collection, model.Groups?.Select(g => g.ToSelectionReadOnly()), assignUserToCollection ? _currentContext.UserId : null); @@ -143,41 +139,41 @@ namespace Bit.Api.Controllers [HttpPut("{id}")] [HttpPost("{id}")] - public async Task Put(string orgId, string id, [FromBody] CollectionRequestModel model) + public async Task Put(Guid orgId, Guid id, [FromBody] CollectionRequestModel model) { if (!await CanEditCollectionAsync(orgId, id)) { throw new NotFoundException(); } - var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); + var collection = await GetCollectionAsync(id, orgId); await _collectionService.SaveAsync(model.ToCollection(collection), model.Groups?.Select(g => g.ToSelectionReadOnly())); return new CollectionResponseModel(collection); } [HttpPut("{id}/users")] - public async Task PutUsers(string orgId, string id, [FromBody] IEnumerable model) + public async Task PutUsers(Guid orgId, Guid id, [FromBody] IEnumerable model) { if (!await CanEditCollectionAsync(orgId, id)) { throw new NotFoundException(); } - var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); + var collection = await GetCollectionAsync(id, orgId); await _collectionRepository.UpdateUsersAsync(collection.Id, model?.Select(g => g.ToSelectionReadOnly())); } [HttpDelete("{id}")] [HttpPost("{id}/delete")] - public async Task Delete(string orgId, string id) + public async Task Delete(Guid orgId, Guid id) { if (!await CanDeleteCollectionAsync(orgId, id)) { throw new NotFoundException(); } - var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); + var collection = await GetCollectionAsync(id, orgId); await _collectionService.DeleteAsync(collection); } @@ -220,8 +216,6 @@ namespace Bit.Api.Controllers return await _currentContext.CreateNewCollections(orgId); } - private async Task CanEditCollectionAsync(string orgId, string collectionId) => - await CanEditCollectionAsync(new Guid(orgId), new Guid(collectionId)); private async Task CanEditCollectionAsync(Guid orgId, Guid collectionId) { if (collectionId == default) @@ -236,14 +230,13 @@ namespace Bit.Api.Controllers if (await _currentContext.EditAssignedCollections(orgId)) { - return null != await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + return collectionDetails != null; } return false; } - private async Task CanDeleteCollectionAsync(string orgId, string collectionId) => - await CanDeleteCollectionAsync(new Guid(orgId), new Guid(collectionId)); private async Task CanDeleteCollectionAsync(Guid orgId, Guid collectionId) { if (collectionId == default) @@ -258,14 +251,13 @@ namespace Bit.Api.Controllers if (await _currentContext.DeleteAssignedCollections(orgId)) { - return null != _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + return collectionDetails != null; } return false; } - private async Task CanViewCollectionAsync(string orgId, string collectionId) => - await CanViewCollectionAsync(new Guid(orgId), new Guid(collectionId)); private async Task CanViewCollectionAsync(Guid orgId, Guid collectionId) { if (collectionId == default) @@ -280,7 +272,8 @@ namespace Bit.Api.Controllers if (await _currentContext.ViewAssignedCollections(orgId)) { - return null != _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value); + return collectionDetails != null; } return false; diff --git a/test/Api.Test/Controllers/CollectionsControllerTests.cs b/test/Api.Test/Controllers/CollectionsControllerTests.cs new file mode 100644 index 000000000..47a6d34d1 --- /dev/null +++ b/test/Api.Test/Controllers/CollectionsControllerTests.cs @@ -0,0 +1,94 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Bit.Api.Controllers; +using Bit.Api.Models.Request; +using Bit.Api.Test.AutoFixture.Attributes; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.Controllers +{ + [ControllerCustomize(typeof(CollectionsController))] + [SutProviderCustomize] + public class CollectionsControllerTests + { + [Theory, BitAutoData] + public async Task Post_Success(Guid orgId, SutProvider sutProvider) + { + sutProvider.GetDependency() + .CreateNewCollections(orgId) + .Returns(true); + + sutProvider.GetDependency() + .EditAnyCollection(orgId) + .Returns(false); + + var collectionRequest = new CollectionRequestModel + { + Name = "encrypted_string", + ExternalId = "my_external_id" + }; + + _ = await sutProvider.Sut.Post(orgId, collectionRequest); + + await sutProvider.GetDependency() + .Received(1) + .SaveAsync(Arg.Any(), Arg.Any>(), null); + } + + [Theory, BitAutoData] + public async Task Put_Success(Guid orgId, Guid collectionId, Guid userId, CollectionRequestModel collectionRequest, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .ViewAssignedCollections(orgId) + .Returns(true); + + sutProvider.GetDependency() + .EditAssignedCollections(orgId) + .Returns(true); + + sutProvider.GetDependency() + .UserId + .Returns(userId); + + sutProvider.GetDependency() + .GetByIdAsync(collectionId, userId) + .Returns(new CollectionDetails + { + OrganizationId = orgId, + }); + + _ = await sutProvider.Sut.Put(orgId, collectionId, collectionRequest); + } + + [Theory, BitAutoData] + public async Task Put_CanNotEditAssignedCollection_ThrowsNotFound(Guid orgId, Guid collectionId, Guid userId, CollectionRequestModel collectionRequest, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .EditAssignedCollections(orgId) + .Returns(true); + + sutProvider.GetDependency() + .UserId + .Returns(userId); + + sutProvider.GetDependency() + .GetByIdAsync(collectionId, userId) + .Returns(Task.FromResult(null)); + + _ = await Assert.ThrowsAsync(async () => await sutProvider.Sut.Put(orgId, collectionId, collectionRequest)); + } + } +}