Skip to content

Commit

Permalink
[AC-1174] Bulk Collection Management (#3229)
Browse files Browse the repository at this point in the history
* [AC-1174] Update SelectionReadOnlyRequestModel to use Guid for Id property

* [AC-1174] Introduce initial bulk-access collection endpoint

* [AC-1174] Introduce BulkAddCollectionAccessCommand and validation logic/tests

* [AC-1174] Add CreateOrUpdateAccessMany method to CollectionRepository

* [AC-1174] Add event logs for bulk add collection access command

* [AC-1174] Add User_BumpAccountRevisionDateByCollectionIds and database migration script

* [AC-1174] Implement EF repository method

* [AC-1174] Improve null checks

* [AC-1174] Remove unnecessary BulkCollectionAccessRequestModel helpers

* [AC-1174] Add unit tests for new controller endpoint

* [AC-1174] Fix formatting

* [AC-1174] Remove comment

* [AC-1174] Remove redundant organizationId parameter

* [AC-1174] Ensure user and group Ids are distinct

* [AC-1174] Cleanup tests based on PR feedback

* [AC-1174] Formatting

* [AC-1174] Update CollectionGroup alias in the sproc

* [AC-1174] Add some additional comments to SQL sproc

* [AC-1174] Add comment explaining additional SaveChangesAsync call

---------

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
  • Loading branch information
shane-melton and eliykat authored Sep 26, 2023
1 parent 2c7d02d commit 5d431ad
Show file tree
Hide file tree
Showing 16 changed files with 943 additions and 5 deletions.
32 changes: 29 additions & 3 deletions src/Api/Controllers/CollectionsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,26 @@ public class CollectionsController : Controller
private readonly ICollectionService _collectionService;
private readonly IDeleteCollectionCommand _deleteCollectionCommand;
private readonly IUserService _userService;
private readonly ICurrentContext _currentContext;
private readonly IAuthorizationService _authorizationService;
private readonly ICurrentContext _currentContext;
private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand;

public CollectionsController(
ICollectionRepository collectionRepository,
ICollectionService collectionService,
IDeleteCollectionCommand deleteCollectionCommand,
IUserService userService,
IAuthorizationService authorizationService,
ICurrentContext currentContext,
IAuthorizationService authorizationService)
IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand)
{
_collectionRepository = collectionRepository;
_collectionService = collectionService;
_deleteCollectionCommand = deleteCollectionCommand;
_userService = userService;
_currentContext = currentContext;
_authorizationService = authorizationService;
_currentContext = currentContext;
_bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand;
}

[HttpGet("{id}")]
Expand Down Expand Up @@ -190,6 +193,29 @@ public async Task PutUsers(Guid orgId, Guid id, [FromBody] IEnumerable<Selection
await _collectionRepository.UpdateUsersAsync(collection.Id, model?.Select(g => g.ToSelectionReadOnly()));
}

[HttpPost("bulk-access")]
public async Task PostBulkCollectionAccess([FromBody] BulkCollectionAccessRequestModel model)
{
var collections = await _collectionRepository.GetManyByManyIdsAsync(model.CollectionIds);

if (collections.Count != model.CollectionIds.Count())
{
throw new NotFoundException("One or more collections not found.");
}

var result = await _authorizationService.AuthorizeAsync(User, collections, CollectionOperations.ModifyAccess);

if (!result.Succeeded)
{
throw new NotFoundException();
}

await _bulkAddCollectionAccessCommand.AddAccessAsync(
collections,
model.Users?.Select(u => u.ToSelectionReadOnly()).ToList(),
model.Groups?.Select(g => g.ToSelectionReadOnly()).ToList());
}

[HttpDelete("{id}")]
[HttpPost("{id}/delete")]
public async Task Delete(Guid orgId, Guid id)
Expand Down
8 changes: 8 additions & 0 deletions src/Api/Models/Request/BulkCollectionAccessRequestModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace Bit.Api.Models.Request;

public class BulkCollectionAccessRequestModel
{
public IEnumerable<Guid> CollectionIds { get; set; }
public IEnumerable<SelectionReadOnlyRequestModel> Groups { get; set; }
public IEnumerable<SelectionReadOnlyRequestModel> Users { get; set; }
}
4 changes: 2 additions & 2 deletions src/Api/Models/Request/SelectionReadOnlyRequestModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Bit.Api.Models.Request;
public class SelectionReadOnlyRequestModel
{
[Required]
public string Id { get; set; }
public Guid Id { get; set; }
public bool ReadOnly { get; set; }
public bool HidePasswords { get; set; }
public bool Manage { get; set; }
Expand All @@ -15,7 +15,7 @@ public CollectionAccessSelection ToSelectionReadOnly()
{
return new CollectionAccessSelection
{
Id = new Guid(Id),
Id = Id,
ReadOnly = ReadOnly,
HidePasswords = HidePasswords,
Manage = Manage,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces;
using Bit.Core.Repositories;
using Bit.Core.Services;

namespace Bit.Core.OrganizationFeatures.OrganizationCollections;

public class BulkAddCollectionAccessCommand : IBulkAddCollectionAccessCommand
{
private readonly ICollectionRepository _collectionRepository;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IGroupRepository _groupRepository;
private readonly IEventService _eventService;

public BulkAddCollectionAccessCommand(
ICollectionRepository collectionRepository,
IOrganizationUserRepository organizationUserRepository,
IGroupRepository groupRepository,
IEventService eventService)
{
_collectionRepository = collectionRepository;
_organizationUserRepository = organizationUserRepository;
_groupRepository = groupRepository;
_eventService = eventService;
}

public async Task AddAccessAsync(ICollection<Collection> collections,
ICollection<CollectionAccessSelection> users,
ICollection<CollectionAccessSelection> groups)
{
await ValidateRequestAsync(collections, users, groups);

await _collectionRepository.CreateOrUpdateAccessForManyAsync(
collections.First().OrganizationId,
collections.Select(c => c.Id),
users,
groups
);

await _eventService.LogCollectionEventsAsync(collections.Select(c =>
(c, EventType.Collection_Updated, (DateTime?)DateTime.UtcNow)));
}

private async Task ValidateRequestAsync(ICollection<Collection> collections, ICollection<CollectionAccessSelection> usersAccess, ICollection<CollectionAccessSelection> groupsAccess)
{
if (collections == null || collections.Count == 0)
{
throw new BadRequestException("No collections were provided.");
}

var orgId = collections.First().OrganizationId;

if (collections.Any(c => c.OrganizationId != orgId))
{
throw new BadRequestException("All collections must belong to the same organization.");
}

var collectionUserIds = usersAccess?.Select(u => u.Id).Distinct().ToList();

if (collectionUserIds is { Count: > 0 })
{
var users = await _organizationUserRepository.GetManyAsync(collectionUserIds);

if (users.Count != collectionUserIds.Count)
{
throw new BadRequestException("One or more users do not exist.");
}

if (users.Any(u => u.OrganizationId != orgId))
{
throw new BadRequestException("One or more users do not belong to the same organization as the collection being assigned.");
}
}

var collectionGroupIds = groupsAccess?.Select(g => g.Id).Distinct().ToList();

if (collectionGroupIds is { Count: > 0 })
{
var groups = await _groupRepository.GetManyByManyIds(collectionGroupIds);

if (groups.Count != collectionGroupIds.Count)
{
throw new BadRequestException("One or more groups do not exist.");
}

if (groups.Any(g => g.OrganizationId != orgId))
{
throw new BadRequestException("One or more groups do not belong to the same organization as the collection being assigned.");
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using Bit.Core.Entities;
using Bit.Core.Models.Data;

namespace Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces;

public interface IBulkAddCollectionAccessCommand
{
Task AddAccessAsync(ICollection<Collection> collections,
ICollection<CollectionAccessSelection> users, ICollection<CollectionAccessSelection> groups);
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ private static void AddOrganizationApiKeyCommandsQueries(this IServiceCollection
public static void AddOrganizationCollectionCommands(this IServiceCollection services)
{
services.AddScoped<IDeleteCollectionCommand, DeleteCollectionCommand>();
services.AddScoped<IBulkAddCollectionAccessCommand, BulkAddCollectionAccessCommand>();
}

private static void AddOrganizationGroupCommands(this IServiceCollection services)
Expand Down
2 changes: 2 additions & 0 deletions src/Core/Repositories/ICollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ public interface ICollectionRepository : IRepository<Collection, Guid>
Task UpdateUsersAsync(Guid id, IEnumerable<CollectionAccessSelection> users);
Task<ICollection<CollectionAccessSelection>> GetManyUsersByIdAsync(Guid id);
Task DeleteManyAsync(IEnumerable<Guid> collectionIds);
Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups);
}
15 changes: 15 additions & 0 deletions src/Infrastructure.Dapper/Repositories/CollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,21 @@ await connection.ExecuteAsync("[dbo].[Collection_DeleteByIds]",
}
}

public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups)
{
using (var connection = new SqlConnection(ConnectionString))
{
var usersArray = users != null ? users.ToArrayTVP() : Enumerable.Empty<CollectionAccessSelection>().ToArrayTVP();
var groupsArray = groups != null ? groups.ToArrayTVP() : Enumerable.Empty<CollectionAccessSelection>().ToArrayTVP();

var results = await connection.ExecuteAsync(
$"[{Schema}].[Collection_CreateOrUpdateAccessForMany]",
new { OrganizationId = organizationId, CollectionIds = collectionIds.ToGuidIdArrayTVP(), Users = usersArray, Groups = groupsArray },
commandType: CommandType.StoredProcedure);
}
}

public async Task CreateUserAsync(Guid collectionId, Guid organizationUserId)
{
using (var connection = new SqlConnection(ConnectionString))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,97 @@ public async Task DeleteManyAsync(IEnumerable<Guid> collectionIds)
}
}

public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups)
{
using (var scope = ServiceScopeFactory.CreateScope())
{
var dbContext = GetDatabaseContext(scope);

var collectionIdsList = collectionIds.ToList();

if (users != null)
{
var existingCollectionUsers = await dbContext.CollectionUsers
.Where(cu => collectionIdsList.Contains(cu.CollectionId))
.ToDictionaryAsync(x => (x.CollectionId, x.OrganizationUserId));

var requestedUsers = users.ToList();

foreach (var collectionId in collectionIdsList)
{
foreach (var requestedUser in requestedUsers)
{
if (!existingCollectionUsers.TryGetValue(
(collectionId, requestedUser.Id),
out var existingCollectionUser)
)
{
// This is a brand new entry
dbContext.CollectionUsers.Add(new CollectionUser
{
CollectionId = collectionId,
OrganizationUserId = requestedUser.Id,
HidePasswords = requestedUser.HidePasswords,
ReadOnly = requestedUser.ReadOnly,
Manage = requestedUser.Manage
});
continue;
}

// It already exists, update it
existingCollectionUser.HidePasswords = requestedUser.HidePasswords;
existingCollectionUser.ReadOnly = requestedUser.ReadOnly;
existingCollectionUser.Manage = requestedUser.Manage;
dbContext.CollectionUsers.Update(existingCollectionUser);
}
}
}

if (groups != null)
{
var existingCollectionGroups = await dbContext.CollectionGroups
.Where(cu => collectionIdsList.Contains(cu.CollectionId))
.ToDictionaryAsync(x => (x.CollectionId, x.GroupId));

var requestedGroups = groups.ToList();

foreach (var collectionId in collectionIdsList)
{
foreach (var requestedGroup in requestedGroups)
{
if (!existingCollectionGroups.TryGetValue(
(collectionId, requestedGroup.Id),
out var existingCollectionGroup)
)
{
// This is a brand new entry
dbContext.CollectionGroups.Add(new CollectionGroup()
{
CollectionId = collectionId,
GroupId = requestedGroup.Id,
HidePasswords = requestedGroup.HidePasswords,
ReadOnly = requestedGroup.ReadOnly,
Manage = requestedGroup.Manage
});
continue;
}

// It already exists, update it
existingCollectionGroup.HidePasswords = requestedGroup.HidePasswords;
existingCollectionGroup.ReadOnly = requestedGroup.ReadOnly;
existingCollectionGroup.Manage = requestedGroup.Manage;
dbContext.CollectionGroups.Update(existingCollectionGroup);
}
}
}
// Need to save the new collection users/groups before running the bump revision code
await dbContext.SaveChangesAsync();
await dbContext.UserBumpAccountRevisionDateByCollectionIdsAsync(collectionIdsList, organizationId);
await dbContext.SaveChangesAsync();
}
}

private async Task ReplaceCollectionGroupsAsync(DatabaseContext dbContext, Core.Entities.Collection collection, IEnumerable<CollectionAccessSelection> groups)
{
var groupsInOrg = dbContext.Groups.Where(g => g.OrganizationId == collection.OrganizationId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,39 @@ from cg in cg_g.DefaultIfEmpty()
UpdateUserRevisionDate(users);
}

public static async Task UserBumpAccountRevisionDateByCollectionIdsAsync(this DatabaseContext context, IEnumerable<Guid> collectionIds, Guid organizationId)
{
var query = from u in context.Users
from c in context.Collections
join ou in context.OrganizationUsers
on u.Id equals ou.UserId
join cu in context.CollectionUsers
on new { ou.AccessAll, OrganizationUserId = ou.Id, CollectionId = c.Id } equals
new { AccessAll = false, cu.OrganizationUserId, cu.CollectionId } into cu_g
from cu in cu_g.DefaultIfEmpty()
join gu in context.GroupUsers
on new { CollectionId = (Guid?)cu.CollectionId, ou.AccessAll, OrganizationUserId = ou.Id } equals
new { CollectionId = (Guid?)null, AccessAll = false, gu.OrganizationUserId } into gu_g
from gu in gu_g.DefaultIfEmpty()
join g in context.Groups
on gu.GroupId equals g.Id into g_g
from g in g_g.DefaultIfEmpty()
join cg in context.CollectionGroups
on new { g.AccessAll, gu.GroupId, CollectionId = c.Id } equals
new { AccessAll = false, cg.GroupId, cg.CollectionId } into cg_g
from cg in cg_g.DefaultIfEmpty()
where ou.OrganizationId == organizationId && collectionIds.Contains(c.Id) &&
ou.Status == OrganizationUserStatusType.Confirmed &&
(cu.CollectionId != null ||

Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Testing

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Identity, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Api, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Events, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 100 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (EventsProcessor, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'
cg.CollectionId != null ||

Check warning on line 101 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Testing

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 101 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Identity, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 101 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Api, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 101 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Events, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'

Check warning on line 101 in src/Infrastructure.EntityFramework/Repositories/DatabaseContextExtensions.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (EventsProcessor, ./src)

The result of the expression is always 'true' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'
ou.AccessAll == true ||
g.AccessAll == true)
select u;

var users = await query.ToListAsync();
UpdateUserRevisionDate(users);
}

public static async Task UserBumpAccountRevisionDateByOrganizationUserIdAsync(this DatabaseContext context, Guid organizationUserId)
{
var query = from u in context.Users
Expand Down
Loading

0 comments on commit 5d431ad

Please sign in to comment.