nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-01 08:02:49 -05:00
Code Activity

[AC-1880] Public API - Deprecated properties (#3706)

Browse Source 
* feat: remove required for AccessAll and add xmldoc for usage restrictions, refs AC-1880

* feat: add validation for create group workflow wrt manage property, refs AC-1880

* feat: add validation for update group workflow wrt manage property, refs AC-1880

* feat: add validation for create and update member workflow wrt manage property, refs AC-1880

* feat: add validation for update collection workflow wrt manage property, refs AC-1880

* fix: flaky Public/GroupsControllerTests + more test coverage, refs AC-1880
This commit is contained in:
Vincent Salucci
2024-02-08 07:44:36 -06:00
committed by GitHub
parent 7747744ff9
commit d29755de5a
18 changed files with 221 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/Api/AdminConsole/Controllers/GroupsController.cs
View File

@ -125,7 +125,7 @@ public class GroupsController : Controller
var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
var group = model.ToGroup(orgIdGuid);
await _createGroupCommand.CreateGroupAsync(group, organization, model.Collections?.Select(c => c.ToSelectionReadOnly()), model.Users);
await _createGroupCommand.CreateGroupAsync(group, organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
return new GroupResponseModel(group);
}
@ -143,7 +143,7 @@ public class GroupsController : Controller
var orgIdGuid = new Guid(orgId);
var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, model.Collections?.Select(c => c.ToSelectionReadOnly()), model.Users);
await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
return new GroupResponseModel(group);
}

2
src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
View File

@ -308,7 +308,7 @@ public class OrganizationUsersController : Controller
var userId = _userService.GetProperUserId(User);
await _organizationService.SaveUserAsync(model.ToOrganizationUser(organizationUser), userId.Value,
model.Collections?.Select(c => c.ToSelectionReadOnly()), model.Groups);
model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Groups);
}
[HttpPut("{id}/groups")]

4
src/Api/AdminConsole/Public/Controllers/GroupsController.cs
View File

@ -111,7 +111,7 @@ public class GroupsController : Controller
{
var group = model.ToGroup(_currentContext.OrganizationId.Value);
var organization = await _organizationRepository.GetByIdAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organization.FlexibleCollections));
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organization.FlexibleCollections)).ToList();
await _createGroupCommand.CreateGroupAsync(group, organization, associations);
var response = new GroupResponseModel(group, associations);
return new JsonResult(response);
@ -140,7 +140,7 @@ public class GroupsController : Controller
var updatedGroup = model.ToGroup(existingGroup);
var organization = await _organizationRepository.GetByIdAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organization.FlexibleCollections));
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organization.FlexibleCollections)).ToList();
await _updateGroupCommand.UpdateGroupAsync(updatedGroup, organization, associations);
var response = new GroupResponseModel(updatedGroup, associations);
return new JsonResult(response);

12
src/Api/AdminConsole/Public/Controllers/MembersController.cs
View File

@ -5,7 +5,6 @@ using Bit.Api.Models.Public.Response;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Context;
using Bit.Core.Models.Business;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Microsoft.AspNetCore.Authorization;
@ -123,14 +122,7 @@ public class MembersController : Controller
public async Task<IActionResult> Post([FromBody] MemberCreateRequestModel model)
{
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false));
var invite = new OrganizationUserInvite
{
Emails = new List<string> { model.Email },
Type = model.Type.Value,
AccessAll = model.AccessAll.Value,
Collections = associations
};
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false)).ToList();
var user = await _organizationService.InviteUserAsync(_currentContext.OrganizationId.Value, null,
model.Email, model.Type.Value, model.AccessAll.Value, model.ExternalId, associations, model.Groups);
var response = new MemberResponseModel(user, associations);
@ -159,7 +151,7 @@ public class MembersController : Controller
}
var updatedUser = model.ToOrganizationUser(existingUser);
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false));
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false)).ToList();
await _organizationService.SaveUserAsync(updatedUser, null, associations, model.Groups);
MemberResponseModel response = null;
if (existingUser.UserId.HasValue)

2
src/Api/AdminConsole/Public/Models/AssociationWithPermissionsBaseModel.cs
View File

@ -22,7 +22,7 @@ public abstract class AssociationWithPermissionsBaseModel
public bool? HidePasswords { get; set; }
/// <summary>
/// When true, the manage permission allows a user to both edit the ciphers within a collection and edit the users/groups that are assigned to the collection.
/// This field will not affect behavior until the Flexible Collections functionality is released in Q1, 2024.
/// This field will not affect behavior until your organization is using the latest collection enhancements (Releasing Q1, 2024)
/// </summary>
public bool? Manage { get; set; }
}

4
src/Api/AdminConsole/Public/Models/GroupBaseModel.cs
View File

@ -13,9 +13,9 @@ public abstract class GroupBaseModel
public string Name { get; set; }
/// <summary>
/// Determines if this group can access all collections within the organization, or only the associated
/// collections. If set to <c>true</c>, this option overrides any collection assignments.
/// collections. If set to <c>true</c>, this option overrides any collection assignments. If your organization is using
/// the latest collection enhancements, you will not be allowed to set this property to <c>true</c>.
/// </summary>
[Required]
public bool? AccessAll { get; set; }
/// <summary>
/// External identifier for reference or linking this group to another system, such as a user directory.

7
src/Api/AdminConsole/Public/Models/MemberBaseModel.cs
View File

@ -36,15 +36,16 @@ public abstract class MemberBaseModel
}
/// <summary>
/// The member's type (or role) within the organization.
/// The member's type (or role) within the organization. If your organization has is using the latest collection enhancements,
/// you will not be allowed to assign the Manager role (OrganizationUserType = 3).
/// </summary>
[Required]
public OrganizationUserType? Type { get; set; }
/// <summary>
/// Determines if this member can access all collections within the organization, or only the associated
/// collections. If set to <c>true</c>, this option overrides any collection assignments.
/// collections. If set to <c>true</c>, this option overrides any collection assignments. If your organization is using
/// the latest collection enhancements, you will not be allowed to set this property to <c>true</c>.
/// </summary>
[Required]
public bool? AccessAll { get; set; }
/// <summary>
/// External identifier for reference or linking this member to another system, such as a user directory.

2
src/Api/Public/Controllers/CollectionsController.cs
View File

@ -93,7 +93,7 @@ public class CollectionsController : Controller
}
var updatedCollection = model.ToCollection(existingCollection);
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(_currentContext.OrganizationId.Value);
var associations = model.Groups?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false));
var associations = model.Groups?.Select(c => c.ToCollectionAccessSelection(organizationAbility?.FlexibleCollections ?? false)).ToList();
await _collectionService.SaveAsync(updatedCollection, associations);
var response = new CollectionResponseModel(updatedCollection, associations);
return new JsonResult(response);

23
src/Core/AdminConsole/OrganizationFeatures/Groups/CreateGroupCommand.cs
View File

@ -36,10 +36,10 @@ public class CreateGroupCommand : ICreateGroupCommand
}
public async Task CreateGroupAsync(Group group, Organization organization,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null)
{
Validate(organization, group);
Validate(organization, group, collections);
await GroupRepositoryCreateGroupAsync(group, organization, collections);
if (users != null)
@ -51,10 +51,10 @@ public class CreateGroupCommand : ICreateGroupCommand
}
public async Task CreateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null)
{
Validate(organization, group);
Validate(organization, group, collections);
await GroupRepositoryCreateGroupAsync(group, organization, collections);
if (users != null)
@ -103,7 +103,7 @@ public class CreateGroupCommand : ICreateGroupCommand
}
}
private static void Validate(Organization organization, Group group)
private static void Validate(Organization organization, Group group, IEnumerable<CollectionAccessSelection> collections)
{
if (organization == null)
{
@ -115,9 +115,18 @@ public class CreateGroupCommand : ICreateGroupCommand
throw new BadRequestException("This organization cannot use groups.");
}
if (organization.FlexibleCollections && group.AccessAll)
if (organization.FlexibleCollections)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
if (group.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
}
}

4
src/Core/AdminConsole/OrganizationFeatures/Groups/Interfaces/ICreateGroupCommand.cs
View File

@ -7,10 +7,10 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
public interface ICreateGroupCommand
{
Task CreateGroupAsync(Group group, Organization organization,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null);
Task CreateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null);
}

4
src/Core/AdminConsole/OrganizationFeatures/Groups/Interfaces/IUpdateGroupCommand.cs
View File

@ -7,10 +7,10 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
public interface IUpdateGroupCommand
{
Task UpdateGroupAsync(Group group, Organization organization,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null);
Task UpdateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> users = null);
}

23
src/Core/AdminConsole/OrganizationFeatures/Groups/UpdateGroupCommand.cs
View File

@ -26,10 +26,10 @@ public class UpdateGroupCommand : IUpdateGroupCommand
}
public async Task UpdateGroupAsync(Group group, Organization organization,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> userIds = null)
{
Validate(organization, group);
Validate(organization, group, collections);
await GroupRepositoryUpdateGroupAsync(group, collections);
if (userIds != null)
@ -41,10 +41,10 @@ public class UpdateGroupCommand : IUpdateGroupCommand
}
public async Task UpdateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
IEnumerable<CollectionAccessSelection> collections = null,
ICollection<CollectionAccessSelection> collections = null,
IEnumerable<Guid> userIds = null)
{
Validate(organization, group);
Validate(organization, group, collections);
await GroupRepositoryUpdateGroupAsync(group, collections);
if (userIds != null)
@ -97,7 +97,7 @@ public class UpdateGroupCommand : IUpdateGroupCommand
}
}
private static void Validate(Organization organization, Group group)
private static void Validate(Organization organization, Group group, IEnumerable<CollectionAccessSelection> collections)
{
if (organization == null)
{
@ -109,9 +109,18 @@ public class UpdateGroupCommand : IUpdateGroupCommand
throw new BadRequestException("This organization cannot use groups.");
}
if (organization.FlexibleCollections && group.AccessAll)
if (organization.FlexibleCollections)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
if (group.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
}
}

4
src/Core/AdminConsole/Services/IOrganizationService.cs
View File

@ -36,7 +36,7 @@ public interface IOrganizationService
Task<List<OrganizationUser>> InviteUsersAsync(Guid organizationId, EventSystemUser systemUser,
IEnumerable<(OrganizationUserInvite invite, string externalId)> invites);
Task<OrganizationUser> InviteUserAsync(Guid organizationId, Guid? invitingUserId, string email,
OrganizationUserType type, bool accessAll, string externalId, IEnumerable<CollectionAccessSelection> collections, IEnumerable<Guid> groups);
OrganizationUserType type, bool accessAll, string externalId, ICollection<CollectionAccessSelection> collections, IEnumerable<Guid> groups);
Task<OrganizationUser> InviteUserAsync(Guid organizationId, EventSystemUser systemUser, string email,
OrganizationUserType type, bool accessAll, string externalId, IEnumerable<CollectionAccessSelection> collections, IEnumerable<Guid> groups);
Task<IEnumerable<Tuple<OrganizationUser, string>>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId, IEnumerable<Guid> organizationUsersId);
@ -45,7 +45,7 @@ public interface IOrganizationService
Guid confirmingUserId, IUserService userService);
Task<List<Tuple<OrganizationUser, string>>> ConfirmUsersAsync(Guid organizationId, Dictionary<Guid, string> keys,
Guid confirmingUserId, IUserService userService);
Task SaveUserAsync(OrganizationUser user, Guid? savingUserId, IEnumerable<CollectionAccessSelection> collections, IEnumerable<Guid> groups);
Task SaveUserAsync(OrganizationUser user, Guid? savingUserId, ICollection<CollectionAccessSelection> collections, IEnumerable<Guid> groups);
[Obsolete("IDeleteOrganizationUserCommand should be used instead. To be removed by EC-607.")]
Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId);
[Obsolete("IDeleteOrganizationUserCommand should be used instead. To be removed by EC-607.")]

25
src/Core/AdminConsole/Services/Implementations/OrganizationService.cs
View File

@ -1370,7 +1370,7 @@ public class OrganizationService : IOrganizationService
}
public async Task SaveUserAsync(OrganizationUser user, Guid? savingUserId,
IEnumerable<CollectionAccessSelection> collections,
ICollection<CollectionAccessSelection> collections,
IEnumerable<Guid> groups)
{
if (user.Id.Equals(default(Guid)))
@ -1408,6 +1408,15 @@ public class OrganizationService : IOrganizationService
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead.");
}
if (organizationAbility?.FlexibleCollections == true && collections?.Any() == true)
{
var invalidAssociations = collections.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations.Any())
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
// End Flexible Collections
// Only autoscale (if required) after all validation has passed so that we know it's a valid request before
@ -1625,9 +1634,20 @@ public class OrganizationService : IOrganizationService
}
public async Task<OrganizationUser> InviteUserAsync(Guid organizationId, Guid? invitingUserId, string email,
OrganizationUserType type, bool accessAll, string externalId, IEnumerable<CollectionAccessSelection> collections,
OrganizationUserType type, bool accessAll, string externalId, ICollection<CollectionAccessSelection> collections,
IEnumerable<Guid> groups)
{
// Validate Collection associations if org is using latest collection enhancements
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId);
if (organizationAbility?.FlexibleCollections ?? false)
{
var invalidAssociations = collections?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
}
return await SaveUserSendInviteAsync(organizationId, invitingUserId, systemUser: null, email, type, accessAll, externalId, collections, groups);
}
@ -1635,6 +1655,7 @@ public class OrganizationService : IOrganizationService
OrganizationUserType type, bool accessAll, string externalId, IEnumerable<CollectionAccessSelection> collections,
IEnumerable<Guid> groups)
{
// Collection associations validation not required as they are always an empty list - created via system user (scim)
return await SaveUserSendInviteAsync(organizationId, invitingUserId: null, systemUser, email, type, accessAll, externalId, collections, groups);
}

24
src/Core/Services/Implementations/CollectionService.cs
View File

@ -55,15 +55,25 @@ public class CollectionService : ICollectionService
var groupsList = groups?.ToList();
var usersList = users?.ToList();
// If using Flexible Collections - a collection should always have someone with Can Manage permissions
if (org.FlexibleCollections && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
if (org.FlexibleCollections)
{
var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems)
// Cannot use Manage with ReadOnly/HidePasswords permissions
var invalidAssociations = groupsList?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{
throw new BadRequestException(
"At least one member or group must have can manage permission.");
throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true.");
}
// If using Flexible Collections V1 - a collection should always have someone with Can Manage permissions
if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
{
var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems)
{
throw new BadRequestException(
"At least one member or group must have can manage permission.");
}
}
}