nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-06-30 15:42:48 -05:00
Code Activity

[PM-21612] [Unified] Fix unhandled error when editing an invited member (#5817)

Browse Source 
* Check for UserId instead of passing potentially default value to
  bump account revision date method.

* Pass explicit UserId into CipherRepository.CreateAsync method
  used for imports.
This commit is contained in:
Thomas Rittson
2025-05-23 11:45:41 +10:00
committed by GitHub
parent 83478f9c69
commit 198d96e155
16 changed files with 169 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs
View File

@ -49,7 +49,7 @@ public class ImportCiphersAsyncCommandTests
await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, importingUserId);
// Assert
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(ciphers, Arg.Any<List<Folder>>());
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(importingUserId, ciphers, Arg.Any<List<Folder>>());
await sutProvider.GetDependency<IPushNotificationService>().Received(1).PushSyncVaultAsync(importingUserId);
}
@ -77,7 +77,7 @@ public class ImportCiphersAsyncCommandTests
await sutProvider.Sut.ImportIntoIndividualVaultAsync(folders, ciphers, folderRelationships, importingUserId);
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(ciphers, Arg.Any<List<Folder>>());
await sutProvider.GetDependency<ICipherRepository>().Received(1).CreateAsync(importingUserId, ciphers, Arg.Any<List<Folder>>());
await sutProvider.GetDependency<IPushNotificationService>().Received(1).PushSyncVaultAsync(importingUserId);
}

3
test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs
View File

@ -7,6 +7,7 @@ using Bit.Core.Test.AutoFixture.Attributes;
using Bit.Infrastructure.EFIntegration.Test.AutoFixture;
using Bit.Infrastructure.EFIntegration.Test.Repositories.EqualityComparers;
using Bit.Infrastructure.EntityFramework.AdminConsole.Models;
using Bit.Infrastructure.EntityFramework.AdminConsole.Repositories;
using Xunit;
using EfRepo = Bit.Infrastructure.EntityFramework.Repositories;
using Organization = Bit.Core.AdminConsole.Entities.Organization;
@ -161,7 +162,7 @@ public class OrganizationRepositoryTests
[CiSkippedTheory, EfOrganizationUserAutoData]
public async Task SearchUnassignedAsync_Works(OrganizationUser orgUser, User user, Organization org,
List<EfRepo.OrganizationUserRepository> efOrgUserRepos, List<EfRepo.OrganizationRepository> efOrgRepos, List<EfRepo.UserRepository> efUserRepos,
List<OrganizationUserRepository> efOrgUserRepos, List<EfRepo.OrganizationRepository> efOrgRepos, List<EfRepo.UserRepository> efUserRepos,
SqlRepo.OrganizationUserRepository sqlOrgUserRepo, SqlRepo.OrganizationRepository sqlOrgRepo, SqlRepo.UserRepository sqlUserRepo)
{
orgUser.Type = OrganizationUserType.Owner;

8
test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs
View File

@ -24,7 +24,7 @@ public class OrganizationUserRepositoryTests
{
[CiSkippedTheory, EfOrganizationUserAutoData]
public async Task CreateAsync_Works_DataMatches(OrganizationUser orgUser, User user, Organization org,
OrganizationUserCompare equalityComparer, List<EfRepo.OrganizationUserRepository> suts,
OrganizationUserCompare equalityComparer, List<EfAdminConsoleRepo.OrganizationUserRepository> suts,
List<EfRepo.OrganizationRepository> efOrgRepos, List<EfRepo.UserRepository> efUserRepos,
SqlRepo.OrganizationUserRepository sqlOrgUserRepo, SqlRepo.UserRepository sqlUserRepo,
SqlRepo.OrganizationRepository sqlOrgRepo)
@ -67,7 +67,7 @@ public class OrganizationUserRepositoryTests
User user,
Organization org,
OrganizationUserCompare equalityComparer,
List<EfRepo.OrganizationUserRepository> suts,
List<EfAdminConsoleRepo.OrganizationUserRepository> suts,
List<EfRepo.UserRepository> efUserRepos,
List<EfRepo.OrganizationRepository> efOrgRepos,
SqlRepo.OrganizationUserRepository sqlOrgUserRepo,
@ -113,7 +113,7 @@ public class OrganizationUserRepositoryTests
}
[CiSkippedTheory, EfOrganizationUserAutoData]
public async Task DeleteAsync_Works_DataMatches(OrganizationUser orgUser, User user, Organization org, List<EfRepo.OrganizationUserRepository> suts,
public async Task DeleteAsync_Works_DataMatches(OrganizationUser orgUser, User user, Organization org, List<EfAdminConsoleRepo.OrganizationUserRepository> suts,
List<EfRepo.UserRepository> efUserRepos, List<EfRepo.OrganizationRepository> efOrgRepos,
SqlRepo.OrganizationUserRepository sqlOrgUserRepo, SqlRepo.UserRepository sqlUserRepo,
SqlRepo.OrganizationRepository sqlOrgRepo)
@ -188,7 +188,7 @@ public class OrganizationUserRepositoryTests
List<EfAdminConsoleRepo.PolicyRepository> efPolicyRepository,
List<EfRepo.UserRepository> efUserRepository,
List<EfRepo.OrganizationRepository> efOrganizationRepository,
List<EfRepo.OrganizationUserRepository> suts,
List<EfAdminConsoleRepo.OrganizationUserRepository> suts,
List<EfAdminConsoleRepo.ProviderRepository> efProviderRepository,
List<EfAdminConsoleRepo.ProviderOrganizationRepository> efProviderOrganizationRepository,
List<EfAdminConsoleRepo.ProviderUserRepository> efProviderUserRepository,

1
test/Infrastructure.EFIntegration.Test/AutoFixture/OrganizationUserFixtures.cs
View File

@ -7,6 +7,7 @@ using Bit.Core.Entities;
using Bit.Core.Models.Data;
using Bit.Core.Test.AutoFixture.OrganizationUserFixtures;
using Bit.Core.Test.AutoFixture.UserFixtures;
using Bit.Infrastructure.EntityFramework.AdminConsole.Repositories;
using Bit.Infrastructure.EntityFramework.Repositories;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;

1
test/Infrastructure.EFIntegration.Test/Vault/AutoFixture/CipherFixtures.cs
View File

@ -5,6 +5,7 @@ using Bit.Core.Test.AutoFixture.UserFixtures;
using Bit.Core.Vault.Entities;
using Bit.Core.Vault.Models.Data;
using Bit.Infrastructure.EFIntegration.Test.AutoFixture.Relays;
using Bit.Infrastructure.EntityFramework.AdminConsole.Repositories;
using Bit.Infrastructure.EntityFramework.Repositories;
using Bit.Infrastructure.EntityFramework.Vault.Repositories;
using Bit.Test.Common.AutoFixture;

3
test/Infrastructure.EFIntegration.Test/Vault/Repositories/CipherRepositoryTests.cs
View File

@ -9,6 +9,7 @@ using Bit.Infrastructure.EntityFramework.Repositories.Queries;
using Bit.Test.Common.AutoFixture.Attributes;
using LinqToDB;
using Xunit;
using EfAdminConsoleRepo = Bit.Infrastructure.EntityFramework.AdminConsole.Repositories;
using EfRepo = Bit.Infrastructure.EntityFramework.Repositories;
using EfVaultRepo = Bit.Infrastructure.EntityFramework.Vault.Repositories;
using SqlRepo = Bit.Infrastructure.Dapper.Repositories;
@ -112,7 +113,7 @@ public class CipherRepositoryTests
[CiSkippedTheory, EfOrganizationCipherCustomize, BitAutoData]
public async Task CreateAsync_BumpsOrgUserAccountRevisionDates(Cipher cipher, List<User> users,
List<OrganizationUser> orgUsers, Collection collection, Organization org, List<EfVaultRepo.CipherRepository> suts, List<EfRepo.UserRepository> efUserRepos, List<EfRepo.OrganizationRepository> efOrgRepos,
List<EfRepo.OrganizationUserRepository> efOrgUserRepos, List<EfRepo.CollectionRepository> efCollectionRepos)
List<EfAdminConsoleRepo.OrganizationUserRepository> efOrgUserRepos, List<EfRepo.CollectionRepository> efCollectionRepos)
{
var savedCiphers = new List<Cipher>();
foreach (var sut in suts)

21
test/Infrastructure.IntegrationTest/AdminConsole/OrganizationTestHelpers.cs
View File

@ -56,6 +56,17 @@ public static class OrganizationTestHelpers
Type = OrganizationUserType.Owner
});
public static Task<OrganizationUser> CreateTestOrganizationUserInviteAsync(
this IOrganizationUserRepository organizationUserRepository,
Organization organization)
=> organizationUserRepository.CreateAsync(new OrganizationUser
{
OrganizationId = organization.Id,
UserId = null, // Invites are not linked to a UserId
Status = OrganizationUserStatusType.Invited,
Type = OrganizationUserType.Owner
});
public static Task<Group> CreateTestGroupAsync(
this IGroupRepository groupRepository,
Organization organization,
@ -63,4 +74,14 @@ public static class OrganizationTestHelpers
=> groupRepository.CreateAsync(
new Group { OrganizationId = organization.Id, Name = $"{identifier} {Guid.NewGuid()}" }
);
public static Task<Collection> CreateTestCollectionAsync(
this ICollectionRepository collectionRepository,
Organization organization,
string identifier = "test")
=> collectionRepository.CreateAsync(new Collection
{
OrganizationId = organization.Id,
Name = $"{identifier} {Guid.NewGuid()}"
});
}

88
test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserReplaceTests.cs Normal file
View File

@ -0,0 +1,88 @@
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.Repositories;
using Xunit;
namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.OrganizationUserRepository;
public class OrganizationUserReplaceTests
{
/// <summary>
/// Specifically tests OrganizationUsers in the invited state, which is unique because
/// they're not linked to a UserId.
/// </summary>
[DatabaseTheory, DatabaseData]
public async Task ReplaceAsync_WithCollectionAccess_WhenUserIsInvited_Success(
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
ICollectionRepository collectionRepository)
{
var organization = await organizationRepository.CreateTestOrganizationAsync();
var orgUser = await organizationUserRepository.CreateTestOrganizationUserInviteAsync(organization);
// Act: update the user, including collection access so we test this overloaded method
orgUser.Type = OrganizationUserType.Admin;
orgUser.AccessSecretsManager = true;
var collection = await collectionRepository.CreateTestCollectionAsync(organization);
await organizationUserRepository.ReplaceAsync(orgUser, [
new CollectionAccessSelection { Id = collection.Id, Manage = true }
]);
// Assert
var (actualOrgUser, actualCollections) = await organizationUserRepository.GetByIdWithCollectionsAsync(orgUser.Id);
Assert.NotNull(actualOrgUser);
Assert.Equal(OrganizationUserType.Admin, actualOrgUser.Type);
Assert.True(actualOrgUser.AccessSecretsManager);
var collectionAccess = Assert.Single(actualCollections);
Assert.Equal(collection.Id, collectionAccess.Id);
Assert.True(collectionAccess.Manage);
}
/// <summary>
/// Tests OrganizationUsers in the Confirmed status, which is a stand-in for all other
/// non-Invited statuses (which are all linked to a UserId).
/// </summary>
/// <param name="organizationRepository"></param>
/// <param name="organizationUserRepository"></param>
/// <param name="collectionRepository"></param>
[DatabaseTheory, DatabaseData]
public async Task ReplaceAsync_WithCollectionAccess_WhenUserIsConfirmed_Success(
IUserRepository userRepository,
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
ICollectionRepository collectionRepository)
{
var organization = await organizationRepository.CreateTestOrganizationAsync();
var user = await userRepository.CreateTestUserAsync();
// OrganizationUser is linked with the User in the Confirmed status
var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user);
// Act: update the user, including collection access so we test this overloaded method
orgUser.Type = OrganizationUserType.Admin;
orgUser.AccessSecretsManager = true;
var collection = await collectionRepository.CreateTestCollectionAsync(organization);
await organizationUserRepository.ReplaceAsync(orgUser, [
new CollectionAccessSelection { Id = collection.Id, Manage = true }
]);
// Assert
var (actualOrgUser, actualCollections) = await organizationUserRepository.GetByIdWithCollectionsAsync(orgUser.Id);
Assert.NotNull(actualOrgUser);
Assert.Equal(OrganizationUserType.Admin, actualOrgUser.Type);
Assert.True(actualOrgUser.AccessSecretsManager);
var collectionAccess = Assert.Single(actualCollections);
Assert.Equal(collection.Id, collectionAccess.Id);
Assert.True(collectionAccess.Manage);
// Account revision date should be updated to a later date
var actualUser = await userRepository.GetByIdAsync(user.Id);
Assert.NotNull(actualUser);
Assert.True(actualUser.AccountRevisionDate.CompareTo(user.AccountRevisionDate) > 0);
}
}

2
test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepositoryTests.cs → test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationUserRepository/OrganizationUserRepositoryTests.cs
View File

@ -8,7 +8,7 @@ using Bit.Core.Repositories;
using Bit.Core.Utilities;
using Xunit;
namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories;
namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.OrganizationUserRepository;
public class OrganizationUserRepositoryTests
{