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

[AC-10362] Remove OrganizationUser.AccessAll from code (#4622)

Browse Source 
* Remove OrganizationUser.AccessAll from code

* Add shadow property

* Remove remaining reference

* dotnet format

* Fix tests

* Bump migration dates
This commit is contained in:
Thomas Rittson 2024-08-26 21:03:44 +10:00 committed by GitHub
parent 78beac9f19
commit 22bd755b3c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 8249 additions and 159 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/Core/AdminConsole/Entities/OrganizationUser.cs
View File

@ -20,10 +20,6 @@ public class OrganizationUser : ITableObject<Guid>, IExternal
public OrganizationUserStatusType Status { get; set; }
public OrganizationUserType Type { get; set; }
/// <summary>
/// AccessAll is deprecated and should always be left as false. Scheduled for removal.
/// </summary>
public bool AccessAll { get; set; } = false;
[MaxLength(300)]
public string? ExternalId { get; set; }
public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;

1
src/Core/AdminConsole/Models/Data/Organizations/OrganizationUsers/OrganizationUserUserDetails.cs
View File

@ -19,7 +19,6 @@ public class OrganizationUserUserDetails : IExternal, ITwoFactorProvidersUser
public bool? Premium { get; set; }
public OrganizationUserStatusType Status { get; set; }
public OrganizationUserType Type { get; set; }
public bool AccessAll { get; set; }
public bool AccessSecretsManager { get; set; }
public string ExternalId { get; set; }
public string SsoExternalId { get; set; }

5
src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs
View File

@ -92,11 +92,6 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand
throw new BadRequestException("Organization must have at least one confirmed owner.");
}
if (user.AccessAll)
{
throw new BadRequestException("The AccessAll property has been deprecated by collection enhancements. Assign the user to collections instead.");
}
if (collectionAccess?.Count > 0)
{
var invalidAssociations = collectionAccess.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords));

36
src/Infrastructure.Dapper/AdminConsole/Helpers/OrganizationUserHelpers.cs
View File

@ -1,36 +0,0 @@
using System.Data;
using Bit.Core.Entities;
using Dapper;
#nullable enable
namespace Bit.Infrastructure.Dapper.AdminConsole.Helpers;
public static class OrganizationUserHelpers
{
public static DataTable ToTvp(this IEnumerable<OrganizationUser> orgUsers)
{
var table = new DataTable();
table.SetTypeName("[dbo].[OrganizationUserType2]");
var columnData = new List<(string name, Type type, Func<OrganizationUser, object?> getter)>
{
(nameof(OrganizationUser.Id), typeof(Guid), ou => ou.Id),
(nameof(OrganizationUser.OrganizationId), typeof(Guid), ou => ou.OrganizationId),
(nameof(OrganizationUser.UserId), typeof(Guid), ou => ou.UserId),
(nameof(OrganizationUser.Email), typeof(string), ou => ou.Email),
(nameof(OrganizationUser.Key), typeof(string), ou => ou.Key),
(nameof(OrganizationUser.Status), typeof(byte), ou => ou.Status),
(nameof(OrganizationUser.Type), typeof(byte), ou => ou.Type),
(nameof(OrganizationUser.AccessAll), typeof(bool), ou => ou.AccessAll),
(nameof(OrganizationUser.ExternalId), typeof(string), ou => ou.ExternalId),
(nameof(OrganizationUser.CreationDate), typeof(DateTime), ou => ou.CreationDate),
(nameof(OrganizationUser.RevisionDate), typeof(DateTime), ou => ou.RevisionDate),
(nameof(OrganizationUser.Permissions), typeof(string), ou => ou.Permissions),
(nameof(OrganizationUser.ResetPasswordKey), typeof(string), ou => ou.ResetPasswordKey),
(nameof(OrganizationUser.AccessSecretsManager), typeof(bool), ou => ou.AccessSecretsManager),
};
return orgUsers.BuildTable(table, columnData);
}
}

37
src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs
View File

@ -9,7 +9,6 @@ using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Settings;
using Bit.Infrastructure.Dapper.AdminConsole.Helpers;
using Dapper;
using Microsoft.Data.SqlClient;
@ -420,6 +419,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
public async Task<ICollection<Guid>?> CreateManyAsync(IEnumerable<OrganizationUser> organizationUsers)
{
organizationUsers = organizationUsers.ToList();
if (!organizationUsers.Any())
{
return default;
@ -430,12 +430,11 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
organizationUser.SetNewId();
}
var orgUsersTVP = organizationUsers.ToTvp();
using (var connection = new SqlConnection(_marsConnectionString))
{
var results = await connection.ExecuteAsync(
$"[{Schema}].[{Table}_CreateMany2]",
new { OrganizationUsersInput = orgUsersTVP },
$"[{Schema}].[{Table}_CreateMany]",
new { jsonData = JsonSerializer.Serialize(organizationUsers) },
commandType: CommandType.StoredProcedure);
}
@ -444,17 +443,17 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
public async Task ReplaceManyAsync(IEnumerable<OrganizationUser> organizationUsers)
{
organizationUsers = organizationUsers.ToList();
if (!organizationUsers.Any())
{
return;
}
var orgUsersTVP = organizationUsers.ToTvp();
using (var connection = new SqlConnection(_marsConnectionString))
{
var results = await connection.ExecuteAsync(
$"[{Schema}].[{Table}_UpdateMany2]",
new { OrganizationUsersInput = orgUsersTVP },
$"[{Schema}].[{Table}_UpdateMany]",
new { jsonData = JsonSerializer.Serialize(organizationUsers) },
commandType: CommandType.StoredProcedure);
}
}
@ -539,27 +538,11 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(
Guid userId, IEnumerable<OrganizationUser> resetPasswordKeys)
{
return async (SqlConnection connection, SqlTransaction transaction) =>
{
const string sql = @"
UPDATE
[dbo].[OrganizationUser]
SET
[ResetPasswordKey] = AR.[ResetPasswordKey]
FROM
[dbo].[OrganizationUser] OU
INNER JOIN
@ResetPasswordKeys AR ON OU.Id = AR.Id
WHERE
OU.[UserId] = @UserId";
var organizationUsersTVP = resetPasswordKeys.ToTvp();
return async (connection, transaction) =>
await connection.ExecuteAsync(
sql,
new { UserId = userId, resetPasswordKeys = organizationUsersTVP },
$"[{Schema}].[OrganizationUser_UpdateDataForKeyRotation]",
new { UserId = userId, OrganizationUserJson = JsonSerializer.Serialize(resetPasswordKeys) },
transaction: transaction,
commandType: CommandType.Text);
};
commandType: CommandType.StoredProcedure);
}
}

1
src/Infrastructure.EntityFramework/AdminConsole/Repositories/Queries/OrganizationUserUserViewQuery.cs
View File

@ -24,7 +24,6 @@ public class OrganizationUserUserDetailsViewQuery : IQuery<OrganizationUserUserD
Premium = x.u.Premium,
Status = x.ou.Status,
Type = x.ou.Type,
AccessAll = x.ou.AccessAll,
ExternalId = x.ou.ExternalId,
SsoExternalId = x.su.ExternalId,
Permissions = x.ou.Permissions,

4
src/Infrastructure.EntityFramework/Configurations/OrganizationUserEntityTypeConfiguration.cs
View File

@ -12,10 +12,6 @@ public class OrganizationUserEntityTypeConfiguration : IEntityTypeConfiguration<
.Property(ou => ou.Id)
.ValueGeneratedNever();
NpgsqlIndexBuilderExtensions.IncludeProperties(
builder.HasIndex(ou => new { ou.UserId, ou.OrganizationId, ou.Status }).IsClustered(false),
ou => ou.AccessAll);
builder
.HasIndex(ou => ou.OrganizationId)
.IsClustered(false);

3
src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs
View File

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

1
test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs
View File

@ -69,7 +69,6 @@ public static class OrganizationTestHelpers
Key = null,
Type = type,
Status = OrganizationUserStatusType.Confirmed,
AccessAll = false,
ExternalId = null,
AccessSecretsManager = accessSecretsManager,
};

54
test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs
View File

@ -131,10 +131,6 @@ public class UpdateOrganizationUserCommandTests
{
Setup(sutProvider, organization, newUserData, oldUserData);
// Deprecated with Flexible Collections
oldUserData.AccessAll = false;
newUserData.AccessAll = false;
// Arrange list of collections to make sure Manage is mutually exclusive
for (var i = 0; i < collections.Count; i++)
{
@ -178,56 +174,6 @@ public class UpdateOrganizationUserCommandTests
Arg.Is<IEnumerable<Guid>>(i => i.Contains(newUserData.Id)));
}
[Theory, BitAutoData]
public async Task UpdateUserAsync_WithAccessAll_Throws(
Organization organization,
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData,
[OrganizationUser(type: OrganizationUserType.User)] OrganizationUser newUserData,
[OrganizationUser(type: OrganizationUserType.Owner, status: OrganizationUserStatusType.Confirmed)] OrganizationUser savingUser,
List<CollectionAccessSelection> collections,
List<Guid> groups,
SutProvider<UpdateOrganizationUserCommand> sutProvider)
{
newUserData.Id = oldUserData.Id;
newUserData.UserId = oldUserData.UserId;
newUserData.OrganizationId = oldUserData.OrganizationId = savingUser.OrganizationId = organization.Id;
newUserData.Permissions = CoreHelpers.ClassToJsonData(new Permissions());
newUserData.AccessAll = true;
sutProvider.GetDependency<ICollectionRepository>()
.GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
.Returns(callInfo => callInfo.Arg<IEnumerable<Guid>>()
.Select(guid => new Collection { Id = guid, OrganizationId = oldUserData.OrganizationId }).ToList());
sutProvider.GetDependency<IGroupRepository>()
.GetManyByManyIds(Arg.Any<IEnumerable<Guid>>())
.Returns(callInfo => callInfo.Arg<IEnumerable<Guid>>()
.Select(guid => new Group { Id = guid, OrganizationId = oldUserData.OrganizationId }).ToList());
sutProvider.GetDependency<IOrganizationRepository>()
.GetByIdAsync(organization.Id)
.Returns(organization);
sutProvider.GetDependency<IOrganizationService>()
.HasConfirmedOwnersExceptAsync(
newUserData.OrganizationId,
Arg.Is<IEnumerable<Guid>>(i => i.Contains(newUserData.Id)))
.Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByIdAsync(oldUserData.Id)
.Returns(oldUserData);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner)
.Returns(new List<OrganizationUser> { savingUser });
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.UpdateUserAsync(newUserData, oldUserData.UserId, collections, groups));
Assert.Contains("the accessall property has been deprecated", exception.Message.ToLowerInvariant());
}
private void Setup(SutProvider<UpdateOrganizationUserCommand> sutProvider, Organization organization,
OrganizationUser newUser, OrganizationUser oldUser)
{

14
test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs
View File

@ -255,7 +255,7 @@ public class OrganizationServiceTests
[Theory]
[BitAutoData(PlanType.FamiliesAnnually)]
public async Task SignUp_EnablesFlexibleCollectionsFeatures
public async Task SignUp_AssignsOwnerToDefaultCollection
(PlanType planType, OrganizationSignup signup, SutProvider<OrganizationService> sutProvider)
{
signup.Plan = planType;
@ -271,13 +271,7 @@ public class OrganizationServiceTests
var result = await sutProvider.Sut.SignUpAsync(signup);
// Assert: AccessAll is not used
await sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).CreateAsync(
Arg.Is<OrganizationUser>(o =>
o.UserId == signup.Owner.Id &&
o.AccessAll == false));
// Assert: created a Can Manage association for the default collection instead
// Assert: created a Can Manage association for the default collection
Assert.NotNull(orgUserId);
await sutProvider.GetDependency<ICollectionRepository>().Received(1).CreateAsync(
Arg.Any<Collection>(),
@ -1479,6 +1473,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
orgUser.UserId = user.Id;
orgUser.Type = orgUserType;
orgUser.AccessSecretsManager = false;
organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(orgUser.UserId.Value).Returns(1);
organizationRepository.GetByIdAsync(org.Id).Returns(org);
@ -1567,6 +1562,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
orgUser.Status = OrganizationUserStatusType.Accepted;
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
orgUser.UserId = orgUserAnotherOrg.UserId = user.Id;
orgUser.AccessSecretsManager = true;
organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
organizationUserRepository.GetManyByManyUsersAsync(default).ReturnsForAnyArgs(new[] { orgUserAnotherOrg });
organizationRepository.GetByIdAsync(org.Id).Returns(org);
@ -1575,7 +1571,7 @@ OrganizationUserInvite invite, SutProvider<OrganizationService> sutProvider)
await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService);
await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed);
await sutProvider.GetDependency<IMailService>().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email);
await sutProvider.GetDependency<IMailService>().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email, true);
await organizationUserRepository.Received(1).ReplaceManyAsync(Arg.Is<List<OrganizationUser>>(users => users.Contains(orgUser) && users.Count == 1));
}

1
test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/EqualityComparers/OrganizationUserCompare.cs
View File

@ -10,7 +10,6 @@ public class OrganizationUserCompare : IEqualityComparer<OrganizationUser>
return x.Email == y.Email &&
x.Status == y.Status &&
x.Type == y.Type &&
x.AccessAll == y.AccessAll &&
x.ExternalId == y.ExternalId &&
x.Permissions == y.Permissions;
}

2698
util/MySqlMigrations/Migrations/20240826231342_OrganizationUserAccessAllDefaultValue.Designer.cs generated Normal file
View File

File diff suppressed because it is too large Load Diff

44
util/MySqlMigrations/Migrations/20240826231342_OrganizationUserAccessAllDefaultValue.cs Normal file
View File

@ -0,0 +1,44 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.MySqlMigrations.Migrations;
/// <inheritdoc />
public partial class OrganizationUserAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser");
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "OrganizationUser",
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: "OrganizationUser",
type: "tinyint(1)",
nullable: false,
oldClrType: typeof(bool),
oldType: "tinyint(1)",
oldDefaultValue: false);
migrationBuilder.CreateIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser",
columns: new[] { "UserId", "OrganizationId", "Status" });
}
}

8
util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs
View File

@ -1228,7 +1228,9 @@ namespace Bit.MySqlMigrations.Migrations
.HasColumnType("char(36)");
b.Property<bool>("AccessAll")
.HasColumnType("tinyint(1)");
.ValueGeneratedOnAdd()
.HasColumnType("tinyint(1)")
.HasDefaultValue(false);
b.Property<bool>("AccessSecretsManager")
.HasColumnType("tinyint(1)");
@ -1276,10 +1278,6 @@ namespace Bit.MySqlMigrations.Migrations
b.HasIndex("UserId")
.HasAnnotation("SqlServer:Clustered", false);
b.HasIndex("UserId", "OrganizationId", "Status")
.HasAnnotation("Npgsql:IndexInclude", new[] { "AccessAll" })
.HasAnnotation("SqlServer:Clustered", false);
b.ToTable("OrganizationUser", (string)null);
});

2704
util/PostgresMigrations/Migrations/20240826231350_OrganizationUserAccessAllDefaultValue.Designer.cs generated Normal file
View File

File diff suppressed because it is too large Load Diff

45
util/PostgresMigrations/Migrations/20240826231350_OrganizationUserAccessAllDefaultValue.cs Normal file
View File

@ -0,0 +1,45 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.PostgresMigrations.Migrations;
/// <inheritdoc />
public partial class OrganizationUserAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser");
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "OrganizationUser",
type: "boolean",
nullable: false,
defaultValue: false,
oldClrType: typeof(bool),
oldType: "boolean");
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "OrganizationUser",
type: "boolean",
nullable: false,
oldClrType: typeof(bool),
oldType: "boolean",
oldDefaultValue: false);
migrationBuilder.CreateIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser",
columns: new[] { "UserId", "OrganizationId", "Status" })
.Annotation("Npgsql:IndexInclude", new[] { "AccessAll" });
}
}

9
util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs
View File

@ -1233,7 +1233,9 @@ namespace Bit.PostgresMigrations.Migrations
.HasColumnType("uuid");
b.Property<bool>("AccessAll")
.HasColumnType("boolean");
.ValueGeneratedOnAdd()
.HasColumnType("boolean")
.HasDefaultValue(false);
b.Property<bool>("AccessSecretsManager")
.HasColumnType("boolean");
@ -1281,11 +1283,6 @@ namespace Bit.PostgresMigrations.Migrations
b.HasIndex("UserId")
.HasAnnotation("SqlServer:Clustered", false);
b.HasIndex("UserId", "OrganizationId", "Status")
.HasAnnotation("SqlServer:Clustered", false);
NpgsqlIndexBuilderExtensions.IncludeProperties(b.HasIndex("UserId", "OrganizationId", "Status"), new[] { "AccessAll" });
b.ToTable("OrganizationUser", (string)null);
});

2687
util/SqliteMigrations/Migrations/20240826231356_OrganizationUserAccessAllDefaultValue.Designer.cs generated Normal file
View File

File diff suppressed because it is too large Load Diff

44
util/SqliteMigrations/Migrations/20240826231356_OrganizationUserAccessAllDefaultValue.cs Normal file
View File

@ -0,0 +1,44 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.SqliteMigrations.Migrations;
/// <inheritdoc />
public partial class OrganizationUserAccessAllDefaultValue : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser");
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "OrganizationUser",
type: "INTEGER",
nullable: false,
defaultValue: false,
oldClrType: typeof(bool),
oldType: "INTEGER");
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.AlterColumn<bool>(
name: "AccessAll",
table: "OrganizationUser",
type: "INTEGER",
nullable: false,
oldClrType: typeof(bool),
oldType: "INTEGER",
oldDefaultValue: false);
migrationBuilder.CreateIndex(
name: "IX_OrganizationUser_UserId_OrganizationId_Status",
table: "OrganizationUser",
columns: new[] { "UserId", "OrganizationId", "Status" });
}
}

8
util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs
View File

@ -1217,7 +1217,9 @@ namespace Bit.SqliteMigrations.Migrations
.HasColumnType("TEXT");
b.Property<bool>("AccessAll")
.HasColumnType("INTEGER");
.ValueGeneratedOnAdd()
.HasColumnType("INTEGER")
.HasDefaultValue(false);
b.Property<bool>("AccessSecretsManager")
.HasColumnType("INTEGER");
@ -1265,10 +1267,6 @@ namespace Bit.SqliteMigrations.Migrations
b.HasIndex("UserId")
.HasAnnotation("SqlServer:Clustered", false);
b.HasIndex("UserId", "OrganizationId", "Status")
.HasAnnotation("Npgsql:IndexInclude", new[] { "AccessAll" })
.HasAnnotation("SqlServer:Clustered", false);
b.ToTable("OrganizationUser", (string)null);
});