Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-2170] Group modal - limit admin access - collections tab #3998

Merged
merged 39 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a716c3b
Check collection permissions when inviting a member
eliykat Mar 22, 2024
20a0eb5
Update auth handlers to use v1 flag
eliykat Mar 28, 2024
5355232
Initial implementation for PUT
eliykat Apr 4, 2024
f33aac6
Fix existing tests
eliykat Apr 4, 2024
f04dafb
Add tests for PUT
eliykat Apr 5, 2024
4221d94
Improve tests
eliykat Apr 5, 2024
54e59e8
dotnet format
eliykat Apr 5, 2024
3d92618
Add test for invite
eliykat Apr 5, 2024
751db0b
Add test for handler
eliykat Apr 5, 2024
8c50db2
Add missing Manage column to sproc
eliykat Apr 5, 2024
1d692e5
dotnet format
eliykat Apr 5, 2024
31a5be8
Create separate Put_vNext method and update tests
eliykat Apr 17, 2024
554a6c8
Add additional feature flag check for invite method
eliykat Apr 17, 2024
275c39f
Fix test
eliykat Apr 17, 2024
75fd880
Update call to old method
eliykat Apr 23, 2024
4326e7a
Undo unrelated change from feature branch
eliykat Apr 25, 2024
b8faed7
Update old SaveUserAsync reference
eliykat Apr 25, 2024
18d027e
Simplify logic in auth handler
eliykat Apr 25, 2024
f681780
Initial implementation for PUT
eliykat Apr 4, 2024
a33a736
Check collection access when creating or updating group
eliykat Apr 17, 2024
97a5c54
Fix existing tests
eliykat Apr 23, 2024
6cc0178
Add tests for POST collection access
eliykat Apr 23, 2024
7fdd166
Copy collection access tests and get passing
eliykat Apr 23, 2024
5d8517d
dotnet format
eliykat Apr 23, 2024
e45ce79
Remove unintended changes
eliykat Apr 25, 2024
78351a8
Merge remote-tracking branch 'origin/main' into ac/ac-2170/group-moda…
eliykat Apr 29, 2024
a4953cb
Separate group and user collection operations
eliykat Apr 30, 2024
098d44e
Split updating access out of Update operation
eliykat Apr 30, 2024
eb6fab2
Revert "Split updating access out of Update operation"
eliykat Apr 30, 2024
b78d7c6
Fix Handler being too permissive
eliykat Apr 30, 2024
8d71aeb
Add tests for new operations
eliykat Apr 30, 2024
6fb0eed
Make comment wording more consistent
eliykat Apr 30, 2024
433cecc
Merge branch 'ac/ac-2538/server-fix-manageusers-custom-permission' in…
eliykat Apr 30, 2024
2b85e78
Use ModifyGroupAccess operation
eliykat Apr 30, 2024
c2b949b
Merge branch 'main' into ac/ac-2538/server-fix-manageusers-custom-per…
eliykat Apr 30, 2024
c1f6e12
Merge branch 'ac/ac-2538/server-fix-manageusers-custom-permission' in…
eliykat Apr 30, 2024
548f173
Merge branch 'main' into ac/ac-2170/group-modal---collections-tab-cha…
eliykat May 1, 2024
509da4e
Allow ManageGroups to get all collectionDetails
eliykat May 1, 2024
a5683ea
Finish tests
eliykat May 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
89 changes: 79 additions & 10 deletions src/Api/AdminConsole/Controllers/GroupsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Bit.Api.AdminConsole.Models.Response;
using Bit.Api.Models.Response;
using Bit.Api.Utilities;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Api.Vault.AuthorizationHandlers.Groups;
using Bit.Core;
using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
Expand Down Expand Up @@ -32,6 +33,7 @@
private readonly IUserService _userService;
private readonly IFeatureService _featureService;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly ICollectionRepository _collectionRepository;

public GroupsController(
IGroupRepository groupRepository,
Expand All @@ -45,7 +47,8 @@
IApplicationCacheService applicationCacheService,
IUserService userService,
IFeatureService featureService,
IOrganizationUserRepository organizationUserRepository)
IOrganizationUserRepository organizationUserRepository,
ICollectionRepository collectionRepository)
{
_groupRepository = groupRepository;
_groupService = groupService;
Expand All @@ -59,6 +62,7 @@
_userService = userService;
_featureService = featureService;
_organizationUserRepository = organizationUserRepository;
_collectionRepository = collectionRepository;
}

[HttpGet("{id}")]
Expand Down Expand Up @@ -125,16 +129,28 @@
}

[HttpPost("")]
public async Task<GroupResponseModel> Post(string orgId, [FromBody] GroupRequestModel model)
public async Task<GroupResponseModel> Post(Guid orgId, [FromBody] GroupRequestModel model)
{
var orgIdGuid = new Guid(orgId);
if (!await _currentContext.ManageGroups(orgIdGuid))
if (!await _currentContext.ManageGroups(orgId))
{
throw new NotFoundException();
}

var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
var group = model.ToGroup(orgIdGuid);
// Flexible Collections - check the user has permission to grant access to the collections for the new group
if (await FlexibleCollectionsIsEnabledAsync(orgId) && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
{
var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Collections.Select(a => a.Id));
var authorized =
(await _authorizationService.AuthorizeAsync(User, collections, BulkCollectionOperations.ModifyGroupAccess))
.Succeeded;
if (!authorized)
{
throw new NotFoundException("You are not authorized to grant access to these collections.");
}
}

var organization = await _organizationRepository.GetByIdAsync(orgId);
var group = model.ToGroup(orgId);
await _createGroupCommand.CreateGroupAsync(group, organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);

return new GroupResponseModel(group);
Expand All @@ -144,16 +160,40 @@
[HttpPost("{id}")]
public async Task<GroupResponseModel> Put(Guid orgId, Guid id, [FromBody] GroupRequestModel model)
{
if (await FlexibleCollectionsIsEnabledAsync(orgId) && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
{
// Use new Flexible Collections v1 logic
return await Put_vNext(orgId, id, model);
}

// Pre-Flexible Collections v1 logic follows
var group = await _groupRepository.GetByIdAsync(id);
if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
{
throw new NotFoundException();
}

// Flexible Collections v1 - a user may not be able to add themselves to a group
var organization = await _organizationRepository.GetByIdAsync(orgId);

Check warning on line 176 in src/Api/AdminConsole/Controllers/GroupsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Controllers/GroupsController.cs#L176

Added line #L176 was not covered by tests

await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization,
model.Collections.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
return new GroupResponseModel(group);

Check warning on line 180 in src/Api/AdminConsole/Controllers/GroupsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Controllers/GroupsController.cs#L178-L180

Added lines #L178 - L180 were not covered by tests
}

/// <summary>
/// Put logic for Flexible Collections v1
/// </summary>
private async Task<GroupResponseModel> Put_vNext(Guid orgId, Guid id, [FromBody] GroupRequestModel model)
{
var (group, currentAccess) = await _groupRepository.GetByIdWithCollectionsAsync(id);
if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
{
throw new NotFoundException();

Check warning on line 191 in src/Api/AdminConsole/Controllers/GroupsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Controllers/GroupsController.cs#L190-L191

Added lines #L190 - L191 were not covered by tests
}

// Check whether the user is permitted to add themselves to the group
var orgAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
var flexibleCollectionsV1Enabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1);
if (flexibleCollectionsV1Enabled && orgAbility.FlexibleCollections && !orgAbility.AllowAdminAccessToAllCollectionItems)
if (!orgAbility.AllowAdminAccessToAllCollectionItems)
{
var userId = _userService.GetProperUserId(User).Value;
var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId);
Expand All @@ -164,9 +204,38 @@
}
}

// The client only sends collections that the saving user has permissions to edit.
// On the server side, we need to (1) confirm this and (2) concat these with the collections that the user
// can't edit before saving to the database.
var currentCollections = await _collectionRepository
.GetManyByManyIdsAsync(currentAccess.Select(cas => cas.Id));

var readonlyCollectionIds = new HashSet<Guid>();
foreach (var collection in currentCollections)
{
if (!(await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.ModifyGroupAccess))
.Succeeded)
{
readonlyCollectionIds.Add(collection.Id);
}
}

if (model.Collections.Any(c => readonlyCollectionIds.Contains(c.Id)))
{
throw new BadRequestException("You must have Can Manage permissions to edit a collection's membership");
}

var editedCollectionAccess = model.Collections
.Select(c => c.ToSelectionReadOnly());
var readonlyCollectionAccess = currentAccess
.Where(ca => readonlyCollectionIds.Contains(ca.Id));
var collectionsToSave = editedCollectionAccess
.Concat(readonlyCollectionAccess)
.ToList();

var organization = await _organizationRepository.GetByIdAsync(orgId);

await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, collectionsToSave, model.Users);
return new GroupResponseModel(group);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
{ Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or
{ Permissions.EditAnyCollection: true } or
{ Permissions.DeleteAnyCollection: true } or
{ Permissions.ManageUsers: true })
{ Permissions.ManageUsers: true } or
{ Permissions.ManageGroups: true })
{
context.Succeed(requirement);
return;
Expand Down