1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00

[PM-10361] Remove Group.AccessAll from code (#4614)

* Remove Group.AccessAll from code

* Add shadow property config and migration
This commit is contained in:
Thomas Rittson 2024-08-13 08:54:03 +10:00 committed by GitHub
parent e2f05f4b8b
commit f04c3b8e54
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 8238 additions and 104 deletions

View File

@ -38,7 +38,6 @@ public class PutGroupCommandTests
var expectedResult = new Group
{
Id = group.Id,
AccessAll = group.AccessAll,
ExternalId = group.ExternalId,
Name = displayName,
OrganizationId = group.OrganizationId
@ -77,7 +76,6 @@ public class PutGroupCommandTests
var expectedResult = new Group
{
Id = group.Id,
AccessAll = group.AccessAll,
ExternalId = group.ExternalId,
Name = displayName,
OrganizationId = group.OrganizationId

View File

@ -13,7 +13,6 @@ public class Group : ITableObject<Guid>, IExternal
public Guid OrganizationId { get; set; }
[MaxLength(100)]
public string Name { get; set; } = null!;
public bool AccessAll { get; set; }
[MaxLength(300)]
public string? ExternalId { get; set; }
public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;

View File

@ -115,11 +115,6 @@ public class CreateGroupCommand : ICreateGroupCommand
throw new BadRequestException("This organization cannot use groups.");
}
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)
{

View File

@ -136,11 +136,6 @@ public class UpdateGroupCommand : IUpdateGroupCommand
await ValidateMemberAccessAsync(originalGroup, memberAccess.ToList());
}
if (group.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the group to collections instead.");
}
var invalidAssociations = collectionAccess?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));
if (invalidAssociations?.Any() ?? false)
{

View File

@ -102,6 +102,9 @@ public class DatabaseContext : DbContext
var eOrganizationDomain = builder.Entity<OrganizationDomain>();
var aWebAuthnCredential = builder.Entity<WebAuthnCredential>();
// Shadow property configurations
eGroup.Property<bool>("AccessAll").HasDefaultValue(false);
eCipher.Property(c => c.Id).ValueGeneratedNever();
eCollection.Property(c => c.Id).ValueGeneratedNever();
eEmergencyAccess.Property(c => c.Id).ValueGeneratedNever();

View File

@ -3,6 +3,9 @@ using Bit.Infrastructure.EntityFramework.Models;
namespace Bit.Infrastructure.EntityFramework.Repositories.Queries;
/// <summary>
/// Returns all Collections that a user is assigned to in an organization, either directly or via a group.
/// </summary>
public class CollectionsReadByOrganizationIdUserIdQuery : IQuery<Collection>
{
private readonly Guid? _organizationId;

View File

@ -2,7 +2,6 @@
using System.Text.Json.Nodes;
using AutoMapper;
using Bit.Core.Auth.UserFeatures.UserKey;
using Bit.Core.Enums;
using Bit.Core.Utilities;
using Bit.Core.Vault.Enums;
using Bit.Core.Vault.Models.Data;
@ -544,32 +543,10 @@ public class CipherRepository : Repository<Core.Vault.Entities.Cipher, Cipher, G
}
else
{
availableCollectionsQuery = from c in context.Collections
join o in context.Organizations
on c.OrganizationId equals o.Id
join ou in context.OrganizationUsers
on new { OrganizationId = o.Id, UserId = userId } equals
new { ou.OrganizationId, ou.UserId }
join cu in context.CollectionUsers
on new { ou.AccessAll, CollectionId = c.Id, OrganizationUserId = ou.Id } equals
new { AccessAll = false, cu.CollectionId, cu.OrganizationUserId } 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, CollectionId = c.Id, gu.GroupId } equals
new { AccessAll = false, cg.CollectionId, cg.GroupId } into cg_g
from cg in cg_g.DefaultIfEmpty()
where o.Id == organizationId &&
o.Enabled &&
ou.Status == OrganizationUserStatusType.Confirmed &&
(ou.AccessAll || !cu.ReadOnly || g.AccessAll || !cg.ReadOnly)
select c.Id;
availableCollectionsQuery =
new CollectionsReadByOrganizationIdUserIdQuery(organizationId.Value, userId.Value)
.Run(context)
.Select(c => c.Id);
}
var availableCollections = await availableCollectionsQuery.ToListAsync();

View File

@ -23,9 +23,6 @@ public class CreateGroupCommandTests
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task CreateGroup_Success(SutProvider<CreateGroupCommand> sutProvider, Organization organization, Group group)
{
// Deprecated with Flexible Collections
group.AccessAll = false;
await sutProvider.Sut.CreateGroupAsync(group, organization);
await sutProvider.GetDependency<IGroupRepository>().Received(1).CreateAsync(group);
@ -38,9 +35,6 @@ public class CreateGroupCommandTests
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task CreateGroup_WithCollections_Success(SutProvider<CreateGroupCommand> sutProvider, Organization organization, Group group, List<CollectionAccessSelection> collections)
{
// Deprecated with Flexible Collections
group.AccessAll = false;
// Arrange list of collections to make sure Manage is mutually exclusive
for (var i = 0; i < collections.Count; i++)
{
@ -62,9 +56,6 @@ public class CreateGroupCommandTests
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task CreateGroup_WithEventSystemUser_Success(SutProvider<CreateGroupCommand> sutProvider, Organization organization, Group group, EventSystemUser eventSystemUser)
{
// Deprecated with Flexible Collections
group.AccessAll = false;
await sutProvider.Sut.CreateGroupAsync(group, organization, eventSystemUser);
await sutProvider.GetDependency<IGroupRepository>().Received(1).CreateAsync(group);
@ -77,9 +68,6 @@ public class CreateGroupCommandTests
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task CreateGroup_WithNullOrganization_Throws(SutProvider<CreateGroupCommand> sutProvider, Group group, EventSystemUser eventSystemUser)
{
// Deprecated with Flexible Collections
group.AccessAll = false;
var exception = await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.CreateGroupAsync(group, null, eventSystemUser));
Assert.Contains("Organization not found", exception.Message);
@ -92,9 +80,6 @@ public class CreateGroupCommandTests
[Theory, OrganizationCustomize(UseGroups = false), BitAutoData]
public async Task CreateGroup_WithUseGroupsAsFalse_Throws(SutProvider<CreateGroupCommand> sutProvider, Organization organization, Group group, EventSystemUser eventSystemUser)
{
// Deprecated with Flexible Collections
group.AccessAll = false;
var exception = await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.CreateGroupAsync(group, organization, eventSystemUser));
Assert.Contains("This organization cannot use groups", exception.Message);
@ -103,19 +88,4 @@ public class CreateGroupCommandTests
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default);
await sutProvider.GetDependency<IReferenceEventService>().DidNotReceiveWithAnyArgs().RaiseEventAsync(default);
}
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task CreateGroup_WithAccessAll_Throws(
SutProvider<CreateGroupCommand> sutProvider, Organization organization, Group group)
{
group.AccessAll = true;
var exception =
await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.CreateGroupAsync(group, organization));
Assert.Contains("AccessAll property has been deprecated", exception.Message);
await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().CreateAsync(default);
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default);
await sutProvider.GetDependency<IReferenceEventService>().DidNotReceiveWithAnyArgs().RaiseEventAsync(default);
}
}

View File

@ -104,30 +104,10 @@ public class UpdateGroupCommandTests
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default);
}
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task UpdateGroup_WithAccessAll_Throws(
SutProvider<UpdateGroupCommand> sutProvider, Organization organization, Group group, Group oldGroup)
{
ArrangeGroup(sutProvider, group, oldGroup);
ArrangeUsers(sutProvider, group);
ArrangeCollections(sutProvider, group);
group.AccessAll = true;
var exception =
await Assert.ThrowsAsync<BadRequestException>(async () => await sutProvider.Sut.UpdateGroupAsync(group, organization));
Assert.Contains("AccessAll property has been deprecated", exception.Message);
await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().CreateAsync(default);
await sutProvider.GetDependency<IEventService>().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default);
}
[Theory, OrganizationCustomize(UseGroups = true), BitAutoData]
public async Task UpdateGroup_GroupBelongsToDifferentOrganization_Throws(SutProvider<UpdateGroupCommand> sutProvider,
Group group, Group oldGroup, Organization organization)
{
group.AccessAll = false;
ArrangeGroup(sutProvider, group, oldGroup);
ArrangeUsers(sutProvider, group);
ArrangeCollections(sutProvider, group);
@ -142,7 +122,6 @@ public class UpdateGroupCommandTests
public async Task UpdateGroup_CollectionsBelongsToDifferentOrganization_Throws(SutProvider<UpdateGroupCommand> sutProvider,
Group group, Group oldGroup, Organization organization, List<CollectionAccessSelection> collectionAccess)
{
group.AccessAll = false;
ArrangeGroup(sutProvider, group, oldGroup);
ArrangeUsers(sutProvider, group);
@ -159,7 +138,6 @@ public class UpdateGroupCommandTests
public async Task UpdateGroup_CollectionsDoNotExist_Throws(SutProvider<UpdateGroupCommand> sutProvider,
Group group, Group oldGroup, Organization organization, List<CollectionAccessSelection> collectionAccess)
{
group.AccessAll = false;
ArrangeGroup(sutProvider, group, oldGroup);
ArrangeUsers(sutProvider, group);
@ -182,7 +160,6 @@ public class UpdateGroupCommandTests
public async Task UpdateGroup_MemberBelongsToDifferentOrganization_Throws(SutProvider<UpdateGroupCommand> sutProvider,
Group group, Group oldGroup, Organization organization, IEnumerable<Guid> userAccess)
{
group.AccessAll = false;
ArrangeGroup(sutProvider, group, oldGroup);
ArrangeCollections(sutProvider, group);
@ -219,7 +196,6 @@ public class UpdateGroupCommandTests
private void ArrangeGroup(SutProvider<UpdateGroupCommand> sutProvider, Group group, Group oldGroup)
{
group.AccessAll = false;
oldGroup.OrganizationId = group.OrganizationId;
oldGroup.Id = group.Id;
sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(oldGroup);

View File

@ -8,7 +8,6 @@ public class GroupCompare : IEqualityComparer<Group>
public bool Equals(Group x, Group y)
{
return x.Name == y.Name &&
x.AccessAll == y.AccessAll &&
x.ExternalId == y.ExternalId;
}

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,35 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.MySqlMigrations.Migrations;
/// <inheritdoc />
public partial class GroupAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "tinyint(1)",
nullable: false,
defaultValue: false,
oldClrType: typeof(bool),
oldType: "tinyint(1)");
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "tinyint(1)",
nullable: false,
oldClrType: typeof(bool),
oldType: "tinyint(1)",
oldDefaultValue: false);
}
}

View File

@ -1017,7 +1017,9 @@ namespace Bit.MySqlMigrations.Migrations
.HasColumnType("char(36)");
b.Property<bool>("AccessAll")
.HasColumnType("tinyint(1)");
.ValueGeneratedOnAdd()
.HasColumnType("tinyint(1)")
.HasDefaultValue(false);
b.Property<DateTime>("CreationDate")
.HasColumnType("datetime(6)");
@ -2388,7 +2390,7 @@ namespace Bit.MySqlMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ApiKey", b =>
{
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ApiKeys")
.HasForeignKey("ServiceAccountId");
b.Navigation("ServiceAccount");
@ -2526,7 +2528,7 @@ namespace Bit.MySqlMigrations.Migrations
.OnDelete(DeleteBehavior.Cascade);
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ProjectAccessPolicies")
.HasForeignKey("ServiceAccountId");
b.Navigation("GrantedProject");
@ -2676,8 +2678,12 @@ namespace Bit.MySqlMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", b =>
{
b.Navigation("ApiKeys");
b.Navigation("GroupAccessPolicies");
b.Navigation("ProjectAccessPolicies");
b.Navigation("UserAccessPolicies");
});

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,35 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.PostgresMigrations.Migrations;
/// <inheritdoc />
public partial class GroupAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "boolean",
nullable: false,
defaultValue: false,
oldClrType: typeof(bool),
oldType: "boolean");
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "boolean",
nullable: false,
oldClrType: typeof(bool),
oldType: "boolean",
oldDefaultValue: false);
}
}

View File

@ -1022,7 +1022,9 @@ namespace Bit.PostgresMigrations.Migrations
.HasColumnType("uuid");
b.Property<bool>("AccessAll")
.HasColumnType("boolean");
.ValueGeneratedOnAdd()
.HasColumnType("boolean")
.HasDefaultValue(false);
b.Property<DateTime>("CreationDate")
.HasColumnType("timestamp with time zone");
@ -2395,7 +2397,7 @@ namespace Bit.PostgresMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ApiKey", b =>
{
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ApiKeys")
.HasForeignKey("ServiceAccountId");
b.Navigation("ServiceAccount");
@ -2533,7 +2535,7 @@ namespace Bit.PostgresMigrations.Migrations
.OnDelete(DeleteBehavior.Cascade);
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ProjectAccessPolicies")
.HasForeignKey("ServiceAccountId");
b.Navigation("GrantedProject");
@ -2683,8 +2685,12 @@ namespace Bit.PostgresMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", b =>
{
b.Navigation("ApiKeys");
b.Navigation("GroupAccessPolicies");
b.Navigation("ProjectAccessPolicies");
b.Navigation("UserAccessPolicies");
});

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,35 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.SqliteMigrations.Migrations;
/// <inheritdoc />
public partial class GroupAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "INTEGER",
nullable: false,
defaultValue: false,
oldClrType: typeof(bool),
oldType: "INTEGER");
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "Group",
type: "INTEGER",
nullable: false,
oldClrType: typeof(bool),
oldType: "INTEGER",
oldDefaultValue: false);
}
}

View File

@ -1006,7 +1006,9 @@ namespace Bit.SqliteMigrations.Migrations
.HasColumnType("TEXT");
b.Property<bool>("AccessAll")
.HasColumnType("INTEGER");
.ValueGeneratedOnAdd()
.HasColumnType("INTEGER")
.HasDefaultValue(false);
b.Property<DateTime>("CreationDate")
.HasColumnType("TEXT");
@ -2377,7 +2379,7 @@ namespace Bit.SqliteMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ApiKey", b =>
{
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ApiKeys")
.HasForeignKey("ServiceAccountId");
b.Navigation("ServiceAccount");
@ -2515,7 +2517,7 @@ namespace Bit.SqliteMigrations.Migrations
.OnDelete(DeleteBehavior.Cascade);
b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "ServiceAccount")
.WithMany()
.WithMany("ProjectAccessPolicies")
.HasForeignKey("ServiceAccountId");
b.Navigation("GrantedProject");
@ -2665,8 +2667,12 @@ namespace Bit.SqliteMigrations.Migrations
modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", b =>
{
b.Navigation("ApiKeys");
b.Navigation("GroupAccessPolicies");
b.Navigation("ProjectAccessPolicies");
b.Navigation("UserAccessPolicies");
});