Upstream/bitwarden-server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2024-11-24 12:35:25 +01:00
Code Issues Projects Releases Wiki Activity

[AC-2317] Public API - remove old permissions code (#4125)

Browse Source 
* Remove FlexibleCollections checks from Public API controllers

* Remove AccessAll from Public API

* Update tests
This commit is contained in:
Thomas Rittson 2024-06-04 08:58:44 +10:00 committed by GitHub
parent 2c40dc0602
commit cae417e2a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 24 additions and 172 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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)).ToList();
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection()).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)).ToList();
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection()).ToList();
await _updateGroupCommand.UpdateGroupAsync(updatedGroup, organization, associations);
var response = new GroupResponseModel(updatedGroup, associations);
return new JsonResult(response);

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

@ -64,9 +64,8 @@ public class MembersController : Controller
{
return new NotFoundResult();
}
var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(orgUser.OrganizationId);
var response = new MemberResponseModel(orgUser, await _userService.TwoFactorIsEnabledAsync(orgUser),
userDetails.Item2, flexibleCollectionsIsEnabled);
userDetails.Item2);
return new JsonResult(response);
}
@ -105,10 +104,9 @@ public class MembersController : Controller
{
var users = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(
_currentContext.OrganizationId.Value);
var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(_currentContext.OrganizationId.Value);
// TODO: Get all CollectionUser associations for the organization and marry them up here for the response.
var memberResponsesTasks = users.Select(async u => new MemberResponseModel(u,
await _userService.TwoFactorIsEnabledAsync(u), null, flexibleCollectionsIsEnabled));
await _userService.TwoFactorIsEnabledAsync(u), null));
var memberResponses = await Task.WhenAll(memberResponsesTasks);
var response = new ListResponseModel<MemberResponseModel>(memberResponses);
return new JsonResult(response);
@ -126,12 +124,11 @@ public class MembersController : Controller
[ProducesResponseType(typeof(ErrorResponseModel), (int)HttpStatusCode.BadRequest)]
public async Task<IActionResult> Post([FromBody] MemberCreateRequestModel model)
{
var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(_currentContext.OrganizationId.Value);
var invite = model.ToOrganizationUserInvite(flexibleCollectionsIsEnabled);
var invite = model.ToOrganizationUserInvite();
var user = await _organizationService.InviteUserAsync(_currentContext.OrganizationId.Value, null,
systemUser: null, invite, model.ExternalId);
var response = new MemberResponseModel(user, invite.Collections, flexibleCollectionsIsEnabled);
var response = new MemberResponseModel(user, invite.Collections);
return new JsonResult(response);
}
@ -156,19 +153,18 @@ public class MembersController : Controller
return new NotFoundResult();
}
var updatedUser = model.ToOrganizationUser(existingUser);
var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(flexibleCollectionsIsEnabled)).ToList();
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection()).ToList();
await _updateOrganizationUserCommand.UpdateUserAsync(updatedUser, null, associations, model.Groups);
MemberResponseModel response = null;
if (existingUser.UserId.HasValue)
{
var existingUserDetails = await _organizationUserRepository.GetDetailsByIdAsync(id);
response = new MemberResponseModel(existingUserDetails,
await _userService.TwoFactorIsEnabledAsync(existingUserDetails), associations, flexibleCollectionsIsEnabled);
await _userService.TwoFactorIsEnabledAsync(existingUserDetails), associations);
}
else
{
response = new MemberResponseModel(updatedUser, associations, flexibleCollectionsIsEnabled);
response = new MemberResponseModel(updatedUser, associations);
}
return new JsonResult(response);
}
@ -239,10 +235,4 @@ public class MembersController : Controller
await _organizationService.ResendInviteAsync(_currentContext.OrganizationId.Value, null, id);
return new OkResult();
}
private async Task<bool> FlexibleCollectionsIsEnabledAsync(Guid organizationId)
{
var organizationAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId);
return organizationAbility?.FlexibleCollections ?? false;
}
}

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

@ -12,12 +12,6 @@ public abstract class GroupBaseModel
[StringLength(100)]
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. If your organization is using
/// the latest collection enhancements, you will not be allowed to set this property to <c>true</c>.
/// </summary>
public bool? AccessAll { get; set; }
/// <summary>
/// External identifier for reference or linking this group to another system, such as a user directory.
/// </summary>
/// <example>external_id_123456</example>

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

@ -10,15 +10,14 @@ public abstract class MemberBaseModel
{
public MemberBaseModel() { }
public MemberBaseModel(OrganizationUser user, bool flexibleCollectionsEnabled)
public MemberBaseModel(OrganizationUser user)
{
if (user == null)
{
throw new ArgumentNullException(nameof(user));
}
Type = flexibleCollectionsEnabled ? GetFlexibleCollectionsUserType(user.Type, user.GetPermissions()) : user.Type;
AccessAll = user.AccessAll;
Type = GetFlexibleCollectionsUserType(user.Type, user.GetPermissions());
ExternalId = user.ExternalId;
ResetPasswordEnrolled = user.ResetPasswordKey != null;
@ -28,15 +27,14 @@ public abstract class MemberBaseModel
}
}
public MemberBaseModel(OrganizationUserUserDetails user, bool flexibleCollectionsEnabled)
public MemberBaseModel(OrganizationUserUserDetails user)
{
if (user == null)
{
throw new ArgumentNullException(nameof(user));
}
Type = flexibleCollectionsEnabled ? GetFlexibleCollectionsUserType(user.Type, user.GetPermissions()) : user.Type;
AccessAll = user.AccessAll;
Type = GetFlexibleCollectionsUserType(user.Type, user.GetPermissions());
ExternalId = user.ExternalId;
ResetPasswordEnrolled = user.ResetPasswordKey != null;
@ -53,12 +51,6 @@ public abstract class MemberBaseModel
[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. If your organization is using
/// the latest collection enhancements, you will not be allowed to set this property to <c>true</c>.
/// </summary>
public bool? AccessAll { get; set; }
/// <summary>
/// External identifier for reference or linking this member to another system, such as a user directory.
/// </summary>
/// <example>external_id_123456</example>

12
src/Api/AdminConsole/Public/Models/Request/AssociationWithPermissionsRequestModel.cs
View File

@ -1,11 +1,10 @@
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data;
namespace Bit.Api.AdminConsole.Public.Models.Request;
public class AssociationWithPermissionsRequestModel : AssociationWithPermissionsBaseModel
{
public CollectionAccessSelection ToCollectionAccessSelection(bool migratedToFlexibleCollections)
public CollectionAccessSelection ToCollectionAccessSelection()
{
var collectionAccessSelection = new CollectionAccessSelection
{
@ -15,13 +14,6 @@ public class AssociationWithPermissionsRequestModel : AssociationWithPermissions
Manage = Manage.GetValueOrDefault()
};
// Throws if the org has not migrated to use FC but has passed in a Manage value in the request
if (!migratedToFlexibleCollections && Manage.GetValueOrDefault())
{
throw new BadRequestException(
"Your organization must be using the latest collection enhancements to use the Manage property.");
}
return collectionAccessSelection;
}
}

1
src/Api/AdminConsole/Public/Models/Request/GroupCreateUpdateRequestModel.cs
View File

@ -20,7 +20,6 @@ public class GroupCreateUpdateRequestModel : GroupBaseModel
public Group ToGroup(Group existingGroup)
{
existingGroup.Name = Name;
existingGroup.AccessAll = AccessAll.Value;
existingGroup.ExternalId = ExternalId;
return existingGroup;
}

5
src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs
View File

@ -22,14 +22,13 @@ public class MemberCreateRequestModel : MemberUpdateRequestModel
throw new NotImplementedException();
}
public OrganizationUserInvite ToOrganizationUserInvite(bool flexibleCollectionsIsEnabled)
public OrganizationUserInvite ToOrganizationUserInvite()
{
var invite = new OrganizationUserInvite
{
Emails = new[] { Email },
Type = Type.Value,
AccessAll = AccessAll.Value,
Collections = Collections?.Select(c => c.ToCollectionAccessSelection(flexibleCollectionsIsEnabled)).ToList(),
Collections = Collections?.Select(c => c.ToCollectionAccessSelection()).ToList(),
Groups = Groups
};

1
src/Api/AdminConsole/Public/Models/Request/MemberUpdateRequestModel.cs
View File

@ -19,7 +19,6 @@ public class MemberUpdateRequestModel : MemberBaseModel, IValidatableObject
public virtual OrganizationUser ToOrganizationUser(OrganizationUser existingUser)
{
existingUser.Type = Type.Value;
existingUser.AccessAll = AccessAll.Value;
existingUser.ExternalId = ExternalId;
// Permissions property is optional for backwards compatibility with existing usage

1
src/Api/AdminConsole/Public/Models/Response/GroupResponseModel.cs
View File

@ -19,7 +19,6 @@ public class GroupResponseModel : GroupBaseModel, IResponseModel
Id = group.Id;
Name = group.Name;
AccessAll = group.AccessAll;
ExternalId = group.ExternalId;
Collections = collections?.Select(c => new AssociationWithPermissionsResponseModel(c));
}

7
src/Api/AdminConsole/Public/Models/Response/MemberResponseModel.cs
View File

@ -16,9 +16,7 @@ public class MemberResponseModel : MemberBaseModel, IResponseModel
[JsonConstructor]
public MemberResponseModel() { }
public MemberResponseModel(OrganizationUser user, IEnumerable<CollectionAccessSelection> collections,
bool flexibleCollectionsEnabled)
: base(user, flexibleCollectionsEnabled)
public MemberResponseModel(OrganizationUser user, IEnumerable<CollectionAccessSelection> collections) : base(user)
{
if (user == null)
{
@ -33,8 +31,7 @@ public class MemberResponseModel : MemberBaseModel, IResponseModel
}
public MemberResponseModel(OrganizationUserUserDetails user, bool twoFactorEnabled,
IEnumerable<CollectionAccessSelection> collections, bool flexibleCollectionsEnabled)
: base(user, flexibleCollectionsEnabled)
IEnumerable<CollectionAccessSelection> collections) : base(user)
{
if (user == null)
{

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

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

7
test/Api.IntegrationTest/AdminConsole/Public/Controllers/MembersControllerTests.cs
View File

@ -135,7 +135,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
Email = email,
Type = OrganizationUserType.Custom,
ExternalId = "myCustomUser",
AccessAll = false,
Collections = [],
Groups = []
};
@ -150,7 +149,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
Assert.Equal(email, result.Email);
Assert.Equal(OrganizationUserType.Custom, result.Type);
Assert.Equal("myCustomUser", result.ExternalId);
Assert.False(result.AccessAll);
Assert.Empty(result.Collections);
// Assert against the database values
@ -160,7 +158,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
Assert.Equal(email, orgUser.Email);
Assert.Equal(OrganizationUserType.Custom, orgUser.Type);
Assert.Equal("myCustomUser", orgUser.ExternalId);
Assert.False(orgUser.AccessAll);
Assert.Equal(OrganizationUserStatusType.Invited, orgUser.Status);
Assert.Equal(_organization.Id, orgUser.OrganizationId);
}
@ -180,7 +177,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
EditAnyCollection = true,
AccessEventLogs = true
},
AccessAll = false,
ExternalId = "example",
Collections = []
};
@ -198,7 +194,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
AssertHelper.AssertPropertyEqual(
new PermissionsModel { DeleteAnyCollection = true, EditAnyCollection = true, AccessEventLogs = true },
result.Permissions);
Assert.False(result.AccessAll);
Assert.Empty(result.Collections);
// Assert against the database values
@ -207,7 +202,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
Assert.Equal(OrganizationUserType.Custom, updatedOrgUser.Type);
Assert.Equal("example", updatedOrgUser.ExternalId);
Assert.False(updatedOrgUser.AccessAll);
Assert.Equal(OrganizationUserStatusType.Confirmed, updatedOrgUser.Status);
Assert.Equal(_organization.Id, updatedOrgUser.OrganizationId);
}
@ -225,7 +219,6 @@ public class MembersControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
var request = new MemberUpdateRequestModel
{
Type = OrganizationUserType.Custom,
AccessAll = false,
ExternalId = "example",
Collections = []
};

109
test/Api.Test/AdminConsole/Public/Controllers/GroupsControllerTests.cs
View File

@ -5,7 +5,6 @@ using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Context;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Repositories;
using Bit.Test.Common.AutoFixture;
@ -22,105 +21,7 @@ public class GroupsControllerTests
{
[Theory]
[BitAutoData]
public async Task Post_Success_BeforeFlexibleCollectionMigration(Organization organization, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has not migrated
organization.FlexibleCollections = false;
// Permissions do not contain Manage property
var expectedPermissions = (groupRequestModel.Collections ?? []).Select(model => new AssociationWithPermissionsRequestModel { Id = model.Id, ReadOnly = model.ReadOnly, HidePasswords = model.HidePasswords.GetValueOrDefault() });
groupRequestModel.Collections = expectedPermissions;
sutProvider.GetDependency<ICurrentContext>().OrganizationId.Returns(organization.Id);
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
var response = await sutProvider.Sut.Post(groupRequestModel) as JsonResult;
var responseValue = response.Value as GroupResponseModel;
await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(
Arg.Is<Group>(g =>
g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
g.AccessAll == groupRequestModel.AccessAll && g.ExternalId == groupRequestModel.ExternalId),
organization,
Arg.Any<ICollection<CollectionAccessSelection>>());
Assert.Equal(groupRequestModel.Name, responseValue.Name);
Assert.Equal(groupRequestModel.AccessAll, responseValue.AccessAll);
Assert.Equal(groupRequestModel.ExternalId, responseValue.ExternalId);
}
[Theory]
[BitAutoData]
public async Task Post_Throws_BadRequestException_BeforeFlexibleCollectionMigration_Manage(Organization organization, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has not migrated
organization.FlexibleCollections = false;
// Contains at least one can manage
groupRequestModel.Collections.First().Manage = true;
sutProvider.GetDependency<ICurrentContext>().OrganizationId.Returns(organization.Id);
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
await sutProvider.GetDependency<ICreateGroupCommand>().DidNotReceiveWithAnyArgs().CreateGroupAsync(default, default, default, default);
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Post(groupRequestModel));
}
[Theory]
[BitAutoData]
public async Task Put_Success_BeforeFlexibleCollectionMigration(Organization organization, Group group, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has not migrated
organization.FlexibleCollections = false;
// Permissions do not contain Manage property
var expectedPermissions = (groupRequestModel.Collections ?? []).Select(model => new AssociationWithPermissionsRequestModel { Id = model.Id, ReadOnly = model.ReadOnly, HidePasswords = model.HidePasswords.GetValueOrDefault() });
groupRequestModel.Collections = expectedPermissions;
group.OrganizationId = organization.Id;
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
sutProvider.GetDependency<ICurrentContext>().OrganizationId.Returns(organization.Id);
var response = await sutProvider.Sut.Put(group.Id, groupRequestModel) as JsonResult;
var responseValue = response.Value as GroupResponseModel;
await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(
Arg.Is<Group>(g =>
g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
g.AccessAll == groupRequestModel.AccessAll && g.ExternalId == groupRequestModel.ExternalId),
Arg.Is<Organization>(o => o.Id == organization.Id),
Arg.Any<ICollection<CollectionAccessSelection>>());
Assert.Equal(groupRequestModel.Name, responseValue.Name);
Assert.Equal(groupRequestModel.AccessAll, responseValue.AccessAll);
Assert.Equal(groupRequestModel.ExternalId, responseValue.ExternalId);
}
[Theory]
[BitAutoData]
public async Task Put_Throws_BadRequestException_BeforeFlexibleCollectionMigration_Manage(Organization organization, Group group, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has not migrated
organization.FlexibleCollections = false;
// Contains at least one can manage
groupRequestModel.Collections.First().Manage = true;
group.OrganizationId = organization.Id;
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
sutProvider.GetDependency<ICurrentContext>().OrganizationId.Returns(organization.Id);
await sutProvider.GetDependency<IUpdateGroupCommand>().DidNotReceiveWithAnyArgs().UpdateGroupAsync(default, default, default, default);
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Put(group.Id, groupRequestModel));
}
[Theory]
[BitAutoData]
public async Task Post_Success_AfterFlexibleCollectionMigration(Organization organization, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
public async Task Post_Success(Organization organization, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has migrated
organization.FlexibleCollections = true;
@ -137,18 +38,17 @@ public class GroupsControllerTests
await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(
Arg.Is<Group>(g =>
g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
g.AccessAll == groupRequestModel.AccessAll && g.ExternalId == groupRequestModel.ExternalId),
g.ExternalId == groupRequestModel.ExternalId),
organization,
Arg.Any<ICollection<CollectionAccessSelection>>());
Assert.Equal(groupRequestModel.Name, responseValue.Name);
Assert.Equal(groupRequestModel.AccessAll, responseValue.AccessAll);
Assert.Equal(groupRequestModel.ExternalId, responseValue.ExternalId);
}
[Theory]
[BitAutoData]
public async Task Put_Success_AfterFlexibleCollectionMigration(Organization organization, Group group, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
public async Task Put_Success(Organization organization, Group group, GroupCreateUpdateRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
{
// Organization has migrated
organization.FlexibleCollections = true;
@ -168,12 +68,11 @@ public class GroupsControllerTests
await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(
Arg.Is<Group>(g =>
g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
g.AccessAll == groupRequestModel.AccessAll && g.ExternalId == groupRequestModel.ExternalId),
g.ExternalId == groupRequestModel.ExternalId),
Arg.Is<Organization>(o => o.Id == organization.Id),
Arg.Any<ICollection<CollectionAccessSelection>>());
Assert.Equal(groupRequestModel.Name, responseValue.Name);
Assert.Equal(groupRequestModel.AccessAll, responseValue.AccessAll);
Assert.Equal(groupRequestModel.ExternalId, responseValue.ExternalId);
}
}