nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-05-22 20:11:04 -05:00
Code

[PS-899] Fix unawaited Task (#2055)

Browse Source 
* Fix unawaited task and switch to Guid parameters

* Remove whitespace

* Formatting

* Fix unwanted Assert
This commit is contained in:
Justin Baur 2022-06-13 11:18:53 -04:00 committed by GitHub
parent e3699156de
commit 46b4207ab6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 128 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
src/Api/Controllers/CollectionsController.cs
View File

@ -36,31 +36,29 @@ namespace Bit.Api.Controllers
} }
[HttpGet("{id}")] [HttpGet("{id}")]
public async Task<CollectionResponseModel> Get(string orgId, string id) public async Task<CollectionResponseModel> Get(Guid orgId, Guid id)
{ {
if (!await CanViewCollectionAsync(orgId, id)) if (!await CanViewCollectionAsync(orgId, id))
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); var collection = await GetCollectionAsync(id, orgId);
return new CollectionResponseModel(collection); return new CollectionResponseModel(collection);
} }
[HttpGet("{id}/details")] [HttpGet("{id}/details")]
public async Task<CollectionGroupDetailsResponseModel> GetDetails(string orgId, string id) public async Task<CollectionGroupDetailsResponseModel> GetDetails(Guid orgId, Guid id)
{ {
var orgIdGuid = new Guid(orgId); if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId))
if (!await ViewAtLeastOneCollectionAsync(orgIdGuid) && !await _currentContext.ManageUsers(orgIdGuid))
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
var idGuid = new Guid(id); if (await _currentContext.ViewAllCollections(orgId))
if (await _currentContext.ViewAllCollections(orgIdGuid))
{ {
var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(idGuid); var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(id);
if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgIdGuid) if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgId)
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
@ -68,9 +66,9 @@ namespace Bit.Api.Controllers
} }
else else
{ {
var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(idGuid, var collectionDetails = await _collectionRepository.GetByIdWithGroupsAsync(id,
_currentContext.UserId.Value); _currentContext.UserId.Value);
if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgIdGuid) if (collectionDetails?.Item1 == null || collectionDetails.Item1.OrganizationId != orgId)
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
@ -79,24 +77,23 @@ namespace Bit.Api.Controllers
} }
[HttpGet("")] [HttpGet("")]
public async Task<ListResponseModel<CollectionResponseModel>> Get(string orgId) public async Task<ListResponseModel<CollectionResponseModel>> Get(Guid orgId)
{ {
var orgIdGuid = new Guid(orgId); if (!await _currentContext.ViewAllCollections(orgId) && !await _currentContext.ManageUsers(orgId))
if (!await _currentContext.ViewAllCollections(orgIdGuid) && !await _currentContext.ManageUsers(orgIdGuid))
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
IEnumerable<Collection> orgCollections; IEnumerable<Collection> 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 // 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 else
{ {
var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value); 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)); var responses = orgCollections.Select(c => new CollectionResponseModel(c));
@ -113,28 +110,27 @@ namespace Bit.Api.Controllers
} }
[HttpGet("{id}/users")] [HttpGet("{id}/users")]
public async Task<IEnumerable<SelectionReadOnlyResponseModel>> GetUsers(string orgId, string id) public async Task<IEnumerable<SelectionReadOnlyResponseModel>> 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 collectionUsers = await _collectionRepository.GetManyUsersByIdAsync(collection.Id);
var responses = collectionUsers.Select(cu => new SelectionReadOnlyResponseModel(cu)); var responses = collectionUsers.Select(cu => new SelectionReadOnlyResponseModel(cu));
return responses; return responses;
} }
[HttpPost("")] [HttpPost("")]
public async Task<CollectionResponseModel> Post(string orgId, [FromBody] CollectionRequestModel model) public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] CollectionRequestModel model)
{ {
var orgIdGuid = new Guid(orgId); var collection = model.ToCollection(orgId);
var collection = model.ToCollection(orgIdGuid);
if (!await CanCreateCollection(orgIdGuid, collection.Id) && if (!await CanCreateCollection(orgId, collection.Id) &&
!await CanEditCollectionAsync(orgIdGuid, collection.Id)) !await CanEditCollectionAsync(orgId, collection.Id))
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
var assignUserToCollection = !(await _currentContext.EditAnyCollection(orgIdGuid)) && var assignUserToCollection = !(await _currentContext.EditAnyCollection(orgId)) &&
await _currentContext.EditAssignedCollections(orgIdGuid); await _currentContext.EditAssignedCollections(orgId);
await _collectionService.SaveAsync(collection, model.Groups?.Select(g => g.ToSelectionReadOnly()), await _collectionService.SaveAsync(collection, model.Groups?.Select(g => g.ToSelectionReadOnly()),
assignUserToCollection ? _currentContext.UserId : null); assignUserToCollection ? _currentContext.UserId : null);
@ -143,41 +139,41 @@ namespace Bit.Api.Controllers
[HttpPut("{id}")] [HttpPut("{id}")]
[HttpPost("{id}")] [HttpPost("{id}")]
public async Task<CollectionResponseModel> Put(string orgId, string id, [FromBody] CollectionRequestModel model) public async Task<CollectionResponseModel> Put(Guid orgId, Guid id, [FromBody] CollectionRequestModel model)
{ {
if (!await CanEditCollectionAsync(orgId, id)) if (!await CanEditCollectionAsync(orgId, id))
{ {
throw new NotFoundException(); 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), await _collectionService.SaveAsync(model.ToCollection(collection),
model.Groups?.Select(g => g.ToSelectionReadOnly())); model.Groups?.Select(g => g.ToSelectionReadOnly()));
return new CollectionResponseModel(collection); return new CollectionResponseModel(collection);
} }
[HttpPut("{id}/users")] [HttpPut("{id}/users")]
public async Task PutUsers(string orgId, string id, [FromBody] IEnumerable<SelectionReadOnlyRequestModel> model) public async Task PutUsers(Guid orgId, Guid id, [FromBody] IEnumerable<SelectionReadOnlyRequestModel> model)
{ {
if (!await CanEditCollectionAsync(orgId, id)) if (!await CanEditCollectionAsync(orgId, id))
{ {
throw new NotFoundException(); 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())); await _collectionRepository.UpdateUsersAsync(collection.Id, model?.Select(g => g.ToSelectionReadOnly()));
} }
[HttpDelete("{id}")] [HttpDelete("{id}")]
[HttpPost("{id}/delete")] [HttpPost("{id}/delete")]
public async Task Delete(string orgId, string id) public async Task Delete(Guid orgId, Guid id)
{ {
if (!await CanDeleteCollectionAsync(orgId, id)) if (!await CanDeleteCollectionAsync(orgId, id))
{ {
throw new NotFoundException(); throw new NotFoundException();
} }
var collection = await GetCollectionAsync(new Guid(id), new Guid(orgId)); var collection = await GetCollectionAsync(id, orgId);
await _collectionService.DeleteAsync(collection); await _collectionService.DeleteAsync(collection);
} }
@ -220,8 +216,6 @@ namespace Bit.Api.Controllers
return await _currentContext.CreateNewCollections(orgId); return await _currentContext.CreateNewCollections(orgId);
} }
private async Task<bool> CanEditCollectionAsync(string orgId, string collectionId) =>
await CanEditCollectionAsync(new Guid(orgId), new Guid(collectionId));
private async Task<bool> CanEditCollectionAsync(Guid orgId, Guid collectionId) private async Task<bool> CanEditCollectionAsync(Guid orgId, Guid collectionId)
{ {
if (collectionId == default) if (collectionId == default)
@ -236,14 +230,13 @@ namespace Bit.Api.Controllers
if (await _currentContext.EditAssignedCollections(orgId)) 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; return false;
} }
private async Task<bool> CanDeleteCollectionAsync(string orgId, string collectionId) =>
await CanDeleteCollectionAsync(new Guid(orgId), new Guid(collectionId));
private async Task<bool> CanDeleteCollectionAsync(Guid orgId, Guid collectionId) private async Task<bool> CanDeleteCollectionAsync(Guid orgId, Guid collectionId)
{ {
if (collectionId == default) if (collectionId == default)
@ -258,14 +251,13 @@ namespace Bit.Api.Controllers
if (await _currentContext.DeleteAssignedCollections(orgId)) 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; return false;
} }
private async Task<bool> CanViewCollectionAsync(string orgId, string collectionId) =>
await CanViewCollectionAsync(new Guid(orgId), new Guid(collectionId));
private async Task<bool> CanViewCollectionAsync(Guid orgId, Guid collectionId) private async Task<bool> CanViewCollectionAsync(Guid orgId, Guid collectionId)
{ {
if (collectionId == default) if (collectionId == default)
@ -280,7 +272,8 @@ namespace Bit.Api.Controllers
if (await _currentContext.ViewAssignedCollections(orgId)) 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; return false;

94
test/Api.Test/Controllers/CollectionsControllerTests.cs Normal file
View File

@ -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<CollectionsController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>()
.CreateNewCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.EditAnyCollection(orgId)
.Returns(false);
var collectionRequest = new CollectionRequestModel
{
Name = "encrypted_string",
ExternalId = "my_external_id"
};
_ = await sutProvider.Sut.Post(orgId, collectionRequest);
await sutProvider.GetDependency<ICollectionService>()
.Received(1)
.SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<SelectionReadOnly>>(), null);
}
[Theory, BitAutoData]
public async Task Put_Success(Guid orgId, Guid collectionId, Guid userId, CollectionRequestModel collectionRequest,
SutProvider<CollectionsController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>()
.ViewAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.EditAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.UserId
.Returns(userId);
sutProvider.GetDependency<ICollectionRepository>()
.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<CollectionsController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>()
.EditAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.UserId
.Returns(userId);
sutProvider.GetDependency<ICollectionRepository>()
.GetByIdAsync(collectionId, userId)
.Returns(Task.FromResult<CollectionDetails>(null));
_ = await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.Put(orgId, collectionId, collectionRequest));
}
}
}