mirror of
https://github.com/bitwarden/server
synced 2025-12-10 21:33:41 +00:00
[PM-22103] Exclude default collections from admin apis (#6021)
* feat: exclude DefaultUserCollection from GetManyByOrganizationIdWithPermissionsAsync Updated EF implementation, SQL procedure, and unit test to verify that default user collections are filtered from results * Update the public CollectionsController.Get method to return a NotFoundResult for collections of type DefaultUserCollection. * Add unit tests for the public CollectionsController * Update ICollectionRepository.GetManyByOrganizationIdAsync to exclude results of the type DefaultUserCollection Modified the SQL stored procedure and the EF query to reflect this change and added a new integration test to ensure the functionality works as expected. * Refactor CollectionsController to remove unused IApplicationCacheService dependency * Update IOrganizationUserRepository.GetDetailsByIdWithCollectionsAsync to exclude DefaultUserCollections * Update IOrganizationUserRepository.GetManyDetailsByOrganizationAsync to exclude DefaultUserCollections * Undo change to GetByIdWithCollectionsAsync * Update integration test to verify exclusion of DefaultUserCollection in OrganizationUserRepository.GetDetailsByIdWithCollectionsAsync * Clarify documentation in ICollectionRepository to specify that GetManyByOrganizationIdWithAccessAsync returns only shared collections belonging to the organization. * Add Arrange, Act, and Assert comments to CollectionsControllerTests
This commit is contained in:
@@ -296,10 +296,29 @@ public class CollectionRepositoryTests
|
||||
}
|
||||
}, null);
|
||||
|
||||
// Create a default user collection (should be excluded from admin console results)
|
||||
var defaultCollection = new Collection
|
||||
{
|
||||
Name = "My Items Collection",
|
||||
OrganizationId = organization.Id,
|
||||
Type = CollectionType.DefaultUserCollection
|
||||
};
|
||||
|
||||
await collectionRepository.CreateAsync(defaultCollection, null, users: new[]
|
||||
{
|
||||
new CollectionAccessSelection()
|
||||
{
|
||||
Id = orgUser.Id, HidePasswords = false, ReadOnly = false, Manage = true
|
||||
}
|
||||
});
|
||||
|
||||
var collections = await collectionRepository.GetManyByOrganizationIdWithPermissionsAsync(organization.Id, user.Id, true);
|
||||
|
||||
Assert.NotNull(collections);
|
||||
|
||||
// Should return only 3 collections (excluding the default user collection)
|
||||
Assert.Equal(3, collections.Count);
|
||||
|
||||
collections = collections.OrderBy(c => c.Name).ToList();
|
||||
|
||||
Assert.Collection(collections, c1 =>
|
||||
@@ -463,4 +482,69 @@ public class CollectionRepositoryTests
|
||||
Assert.False(c3.Unmanaged);
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Test to ensure collections are properly retrieved by organization
|
||||
/// </summary>
|
||||
[DatabaseTheory, DatabaseData]
|
||||
public async Task GetManyByOrganizationIdAsync_Success(
|
||||
IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
ICollectionRepository collectionRepository,
|
||||
IOrganizationUserRepository organizationUserRepository)
|
||||
{
|
||||
var user = await userRepository.CreateAsync(new User
|
||||
{
|
||||
Name = "Test User",
|
||||
Email = $"test+{Guid.NewGuid()}@email.com",
|
||||
ApiKey = "TEST",
|
||||
SecurityStamp = "stamp",
|
||||
});
|
||||
|
||||
var organization = await organizationRepository.CreateAsync(new Organization
|
||||
{
|
||||
Name = "Test Org",
|
||||
PlanType = PlanType.EnterpriseAnnually,
|
||||
Plan = "Test Plan",
|
||||
BillingEmail = "billing@email.com"
|
||||
});
|
||||
|
||||
var orgUser = await organizationUserRepository.CreateAsync(new OrganizationUser
|
||||
{
|
||||
OrganizationId = organization.Id,
|
||||
UserId = user.Id,
|
||||
Status = OrganizationUserStatusType.Confirmed,
|
||||
});
|
||||
|
||||
var collection1 = new Collection { Name = "Collection 1", OrganizationId = organization.Id, };
|
||||
await collectionRepository.CreateAsync(collection1, null, null);
|
||||
|
||||
var collection2 = new Collection { Name = "Collection 2", OrganizationId = organization.Id, };
|
||||
await collectionRepository.CreateAsync(collection2, null, null);
|
||||
|
||||
var collection3 = new Collection { Name = "Collection 3", OrganizationId = organization.Id, };
|
||||
await collectionRepository.CreateAsync(collection3, null, null);
|
||||
|
||||
// Create a default user collection (should not be returned by this method)
|
||||
var defaultCollection = new Collection
|
||||
{
|
||||
Name = "My Items",
|
||||
OrganizationId = organization.Id,
|
||||
Type = CollectionType.DefaultUserCollection
|
||||
};
|
||||
await collectionRepository.CreateAsync(defaultCollection, null, null);
|
||||
|
||||
var collections = await collectionRepository.GetManyByOrganizationIdAsync(organization.Id);
|
||||
|
||||
Assert.NotNull(collections);
|
||||
Assert.Equal(3, collections.Count); // Should only return the 3 shared collections, excluding the default user collection
|
||||
Assert.All(collections, c => Assert.Equal(organization.Id, c.OrganizationId));
|
||||
Assert.All(collections, c => Assert.NotEqual(CollectionType.DefaultUserCollection, c.Type));
|
||||
|
||||
// Verify specific collections are returned
|
||||
Assert.Contains(collections, c => c.Name == "Collection 1");
|
||||
Assert.Contains(collections, c => c.Name == "Collection 2");
|
||||
Assert.Contains(collections, c => c.Name == "Collection 3");
|
||||
Assert.DoesNotContain(collections, c => c.Name == "My Items");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -184,6 +184,87 @@ public class OrganizationUserRepositoryTests
|
||||
r.EncryptedPrivateKey == "privatekey");
|
||||
}
|
||||
|
||||
[DatabaseTheory, DatabaseData]
|
||||
public async Task GetManyDetailsByOrganizationAsync_WithIncludeCollections_ExcludesDefaultCollections(
|
||||
IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
IOrganizationUserRepository organizationUserRepository,
|
||||
ICollectionRepository collectionRepository)
|
||||
{
|
||||
var user = await userRepository.CreateAsync(new User
|
||||
{
|
||||
Name = "Test User",
|
||||
Email = $"test+{Guid.NewGuid()}@example.com",
|
||||
ApiKey = "TEST",
|
||||
SecurityStamp = "stamp",
|
||||
});
|
||||
|
||||
var organization = await organizationRepository.CreateAsync(new Organization
|
||||
{
|
||||
Name = "Test Org",
|
||||
BillingEmail = user.Email,
|
||||
Plan = "Test",
|
||||
});
|
||||
|
||||
var orgUser = await organizationUserRepository.CreateAsync(new OrganizationUser
|
||||
{
|
||||
OrganizationId = organization.Id,
|
||||
UserId = user.Id,
|
||||
Status = OrganizationUserStatusType.Confirmed,
|
||||
});
|
||||
|
||||
// Create a regular collection
|
||||
var regularCollection = await collectionRepository.CreateAsync(new Collection
|
||||
{
|
||||
OrganizationId = organization.Id,
|
||||
Name = "Regular Collection",
|
||||
Type = CollectionType.SharedCollection
|
||||
});
|
||||
|
||||
// Create a default user collection
|
||||
var defaultCollection = await collectionRepository.CreateAsync(new Collection
|
||||
{
|
||||
OrganizationId = organization.Id,
|
||||
Name = "Default Collection",
|
||||
Type = CollectionType.DefaultUserCollection,
|
||||
DefaultUserCollectionEmail = user.Email
|
||||
});
|
||||
|
||||
// Assign the organization user to both collections
|
||||
await organizationUserRepository.ReplaceAsync(orgUser, new List<CollectionAccessSelection>
|
||||
{
|
||||
new CollectionAccessSelection
|
||||
{
|
||||
Id = regularCollection.Id,
|
||||
ReadOnly = false,
|
||||
HidePasswords = false,
|
||||
Manage = true
|
||||
},
|
||||
new CollectionAccessSelection
|
||||
{
|
||||
Id = defaultCollection.Id,
|
||||
ReadOnly = false,
|
||||
HidePasswords = false,
|
||||
Manage = true
|
||||
}
|
||||
});
|
||||
|
||||
// Get organization users with collections included
|
||||
var organizationUsers = await organizationUserRepository.GetManyDetailsByOrganizationAsync(
|
||||
organization.Id, includeGroups: false, includeCollections: true);
|
||||
|
||||
Assert.NotNull(organizationUsers);
|
||||
Assert.Single(organizationUsers);
|
||||
|
||||
var orgUserWithCollections = organizationUsers.First();
|
||||
Assert.NotNull(orgUserWithCollections.Collections);
|
||||
|
||||
// Should only include the regular collection, not the default collection
|
||||
Assert.Single(orgUserWithCollections.Collections);
|
||||
Assert.Equal(regularCollection.Id, orgUserWithCollections.Collections.First().Id);
|
||||
Assert.DoesNotContain(orgUserWithCollections.Collections, c => c.Id == defaultCollection.Id);
|
||||
}
|
||||
|
||||
[DatabaseTheory, DatabaseData]
|
||||
public async Task GetManyDetailsByUserAsync_Works(IUserRepository userRepository,
|
||||
IOrganizationRepository organizationRepository,
|
||||
@@ -498,6 +579,17 @@ public class OrganizationUserRepositoryTests
|
||||
RevisionDate = requestTime
|
||||
});
|
||||
|
||||
// Create a default user collection that should be excluded from admin results
|
||||
var defaultCollection = await collectionRepository.CreateAsync(new Collection
|
||||
{
|
||||
Id = CoreHelpers.GenerateComb(),
|
||||
OrganizationId = organization.Id,
|
||||
Name = "My Items",
|
||||
Type = CollectionType.DefaultUserCollection,
|
||||
CreationDate = requestTime,
|
||||
RevisionDate = requestTime
|
||||
});
|
||||
|
||||
var group1 = await groupRepository.CreateAsync(new Group
|
||||
{
|
||||
Id = CoreHelpers.GenerateComb(),
|
||||
@@ -544,6 +636,13 @@ public class OrganizationUserRepositoryTests
|
||||
ReadOnly = true,
|
||||
HidePasswords = false,
|
||||
Manage = false
|
||||
},
|
||||
new CollectionAccessSelection
|
||||
{
|
||||
Id = defaultCollection.Id,
|
||||
ReadOnly = false,
|
||||
HidePasswords = false,
|
||||
Manage = true
|
||||
}
|
||||
],
|
||||
Groups = [group1.Id]
|
||||
@@ -605,7 +704,11 @@ public class OrganizationUserRepositoryTests
|
||||
var orgUser1 = await organizationUserRepository.GetDetailsByIdWithCollectionsAsync(orgUserCollection[0].OrganizationUser.Id);
|
||||
var group1Database = await groupRepository.GetManyIdsByUserIdAsync(orgUserCollection[0].OrganizationUser.Id);
|
||||
Assert.Equal(orgUserCollection[0].OrganizationUser.Id, orgUser1.OrganizationUser.Id);
|
||||
|
||||
// Should only return the regular collection, not the default collection (even though both were assigned)
|
||||
Assert.Single(orgUser1.Collections);
|
||||
Assert.Equal(collection1.Id, orgUser1.Collections.First().Id);
|
||||
Assert.DoesNotContain(orgUser1.Collections, c => c.Id == defaultCollection.Id);
|
||||
Assert.Equal(group1.Id, group1Database.First());
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user