bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2026-02-14 15:33:35 +00:00
Code Issues Releases Wiki Activity

[PM-25106] Refactor Misleading Stored Procedure/Repository Language (#6890)

Browse Source 
* Begin migration to appropriately named sprocs

* Update method and parameter names

* Remove incorrect change

* Changes EF to match collection type comparison

* Adds integration test verifying excluded collections

* Changes EF to match collection type comparison

* Fix whitespacing

* Fix dedented if
This commit is contained in:
sven-bitwarden
2026-02-09 09:25:10 -06:00
committed by GitHub
parent 6548737320
commit 70c01bcfb2
18 changed files with 382 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
View File

@@ -155,7 +155,7 @@ public class OrganizationUsersController : BaseAdminConsoleController
[Authorize<ManageUsersRequirement>]
public async Task<OrganizationUserDetailsResponseModel> Get(Guid orgId, Guid id, bool includeGroups = false)
{
var (organizationUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id);
var (organizationUser, collections) = await _organizationUserRepository.GetDetailsByIdWithSharedCollectionsAsync(id);
if (organizationUser == null || organizationUser.OrganizationId != orgId)
{
throw new NotFoundException();

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

@@ -80,7 +80,7 @@ public class MembersController : Controller
[ProducesResponseType((int)HttpStatusCode.NotFound)]
public async Task<IActionResult> Get(Guid id)
{
var (orgUser, collections) = await _organizationUserRepository.GetDetailsByIdWithCollectionsAsync(id);
var (orgUser, collections) = await _organizationUserRepository.GetDetailsByIdWithSharedCollectionsAsync(id);
if (orgUser == null || orgUser.OrganizationId != _currentContext.OrganizationId)
{
return new NotFoundResult();
@@ -123,7 +123,7 @@ public class MembersController : Controller
[ProducesResponseType(typeof(ListResponseModel<MemberResponseModel>), (int)HttpStatusCode.OK)]
public async Task<IActionResult> List()
{
var organizationUserUserDetails = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(_currentContext.OrganizationId!.Value, includeCollections: true);
var organizationUserUserDetails = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(_currentContext.OrganizationId!.Value, includeSharedCollections: true);
var orgUsersTwoFactorIsEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUserUserDetails);
var memberResponses = organizationUserUserDetails.Select(u =>

2
src/Api/Controllers/CollectionsController.cs
View File

@@ -81,7 +81,7 @@ public class CollectionsController : Controller
[HttpGet("details")]
public async Task<ListResponseModel<CollectionAccessDetailsResponseModel>> GetManyWithDetails(Guid orgId)
{
var allOrgCollections = await _collectionRepository.GetManyByOrganizationIdWithPermissionsAsync(
var allOrgCollections = await _collectionRepository.GetManySharedByOrganizationIdWithPermissionsAsync(
orgId, _currentContext.UserId.Value, true);
var readAllAuthorized =

8
src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs
View File

@@ -28,21 +28,21 @@ public interface IOrganizationUserRepository : IRepository<OrganizationUser, Gui
/// </summary>
/// <param name="id">The id of the OrganizationUser</param>
/// <returns>A tuple containing the OrganizationUser and its associated collections</returns>
Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithCollectionsAsync(Guid id);
Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithSharedCollectionsAsync(Guid id);
/// <summary>
/// Returns the OrganizationUsers and their associated collections (excluding DefaultUserCollections).
/// </summary>
/// <param name="organizationId">The id of the organization</param>
/// <param name="includeGroups">Whether to include groups</param>
/// <param name="includeCollections">Whether to include collections</param>
/// <param name="includeSharedCollections">Whether to include shared collections</param>
/// <returns>A list of OrganizationUserUserDetails</returns>
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups = false, bool includeCollections = false);
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups = false, bool includeSharedCollections = false);
/// <inheritdoc cref="GetManyDetailsByOrganizationAsync"/>
/// <remarks>
/// This method is optimized for performance.
/// Reduces database round trips by fetching all data in fewer queries.
/// </remarks>
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups = false, bool includeCollections = false);
Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups = false, bool includeSharedCollections = false);
Task<ICollection<OrganizationUserOrganizationDetails>> GetManyDetailsByUserAsync(Guid userId,
OrganizationUserStatusType? status = null);
Task<OrganizationUserOrganizationDetails?> GetDetailsByUserAsync(Guid userId, Guid organizationId,

2
src/Core/Repositories/ICollectionRepository.cs
View File

@@ -45,7 +45,7 @@ public interface ICollectionRepository : IRepository<Collection, Guid>
/// Optionally, you can include access relationships for other Groups/Users and the collections.
/// Excludes default collections (My Items collections) - used by Admin Console Collections tab.
/// </summary>
Task<ICollection<CollectionAdminDetails>> GetManyByOrganizationIdWithPermissionsAsync(Guid organizationId, Guid userId, bool includeAccessRelationships);
Task<ICollection<CollectionAdminDetails>> GetManySharedByOrganizationIdWithPermissionsAsync(Guid organizationId, Guid userId, bool includeAccessRelationships);
/// <summary>
/// Returns the collection by Id, including permission info for the specified user.

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

@@ -187,12 +187,12 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
return results.SingleOrDefault();
}
}
public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithCollectionsAsync(Guid id)
public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithSharedCollectionsAsync(Guid id)
{
using (var connection = new SqlConnection(ConnectionString))
{
var results = await connection.QueryMultipleAsync(
"[dbo].[OrganizationUserUserDetails_ReadWithCollectionsById]",
"[dbo].[OrganizationUserUserDetails_ReadWithSharedCollectionsById]",
new { Id = id },
commandType: CommandType.StoredProcedure);
@@ -202,7 +202,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
}
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups, bool includeCollections)
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups, bool includeSharedCollections)
{
using (var connection = new SqlConnection(ConnectionString))
{
@@ -216,7 +216,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
var users = results.ToList();
if (!includeCollections && !includeGroups)
if (!includeSharedCollections && !includeGroups)
{
return users;
}
@@ -231,10 +231,10 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
commandType: CommandType.StoredProcedure)).GroupBy(u => u.OrganizationUserId).ToList();
}
if (includeCollections)
if (includeSharedCollections)
{
userCollections = (await connection.QueryAsync<CollectionUser>(
"[dbo].[CollectionUser_ReadByOrganizationUserIds]",
"[dbo].[CollectionUser_ReadSharedCollectionsByOrganizationUserIds]",
new { OrganizationUserIds = orgUserIds },
commandType: CommandType.StoredProcedure)).GroupBy(u => u.OrganizationUserId).ToList();
}
@@ -267,7 +267,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
}
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups, bool includeCollections)
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups, bool includeSharedCollections)
{
using (var connection = new SqlConnection(ConnectionString))
{
@@ -278,7 +278,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
{
OrganizationId = organizationId,
IncludeGroups = includeGroups,
IncludeCollections = includeCollections
IncludeCollections = includeSharedCollections
},
commandType: CommandType.StoredProcedure);
@@ -297,7 +297,7 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
// Read collection associations (third result set, if requested)
Dictionary<Guid, List<CollectionAccessSelection>>? userCollectionMap = null;
if (includeCollections)
if (includeSharedCollections)
{
var collectionUsers = await results.ReadAsync<CollectionUser>();
userCollectionMap = collectionUsers

4
src/Infrastructure.Dapper/Repositories/CollectionRepository.cs
View File

@@ -153,12 +153,12 @@ public class CollectionRepository : Repository<Collection, Guid>, ICollectionRep
}
}
public async Task<ICollection<CollectionAdminDetails>> GetManyByOrganizationIdWithPermissionsAsync(Guid organizationId, Guid userId, bool includeAccessRelationships)
public async Task<ICollection<CollectionAdminDetails>> GetManySharedByOrganizationIdWithPermissionsAsync(Guid organizationId, Guid userId, bool includeAccessRelationships)
{
using (var connection = new SqlConnection(ConnectionString))
{
var results = await connection.QueryMultipleAsync(
$"[{Schema}].[Collection_ReadByOrganizationIdWithPermissions]",
$"[{Schema}].[Collection_ReadSharedCollectionsByOrganizationIdWithPermissions]",
new { OrganizationId = organizationId, UserId = userId, IncludeAccessRelationships = includeAccessRelationships },
commandType: CommandType.StoredProcedure);

16
src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs
View File

@@ -350,7 +350,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
}
#nullable enable
public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithCollectionsAsync(Guid id)
public async Task<(OrganizationUserUserDetails? OrganizationUser, ICollection<CollectionAccessSelection> Collections)> GetDetailsByIdWithSharedCollectionsAsync(Guid id)
{
var organizationUserUserDetails = await GetDetailsByIdAsync(id);
using (var scope = ServiceScopeFactory.CreateScope())
@@ -359,7 +359,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
var query = from ou in dbContext.OrganizationUsers
join cu in dbContext.CollectionUsers on ou.Id equals cu.OrganizationUserId
join c in dbContext.Collections on cu.CollectionId equals c.Id
where ou.Id == id && c.Type != CollectionType.DefaultUserCollection
where ou.Id == id && c.Type == CollectionType.SharedCollection
select cu;
var collections = await query.Select(cu => new CollectionAccessSelection
{
@@ -438,7 +438,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
}
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups, bool includeCollections)
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups, bool includeSharedCollections)
{
using (var scope = ServiceScopeFactory.CreateScope())
{
@@ -448,7 +448,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
where ou.OrganizationId == organizationId
select ou).ToListAsync();
if (!includeCollections && !includeGroups)
if (!includeSharedCollections && !includeGroups)
{
return users;
}
@@ -467,12 +467,12 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
.GroupBy(g => g.OrganizationUserId).ToList();
}
if (includeCollections)
if (includeSharedCollections)
{
collections = (await (from cu in dbContext.CollectionUsers
join ou in userIdEntities on cu.OrganizationUserId equals ou.Id
join c in dbContext.Collections on cu.CollectionId equals c.Id
where c.Type != CollectionType.DefaultUserCollection
where c.Type == CollectionType.SharedCollection
select cu).ToListAsync())
.GroupBy(c => c.OrganizationUserId).ToList();
}
@@ -506,7 +506,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
}
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(
Guid organizationId, bool includeGroups, bool includeCollections)
Guid organizationId, bool includeGroups, bool includeSharedCollections)
{
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);
@@ -541,7 +541,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
? ou.GroupUsers.Select(gu => gu.GroupId).ToList()
: new List<Guid>(),
Collections = includeCollections
Collections = includeSharedCollections
? ou.CollectionUsers
.Where(cu => cu.Collection.Type == CollectionType.SharedCollection)
.Select(cu => new CollectionAccessSelection

2
src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs
View File

@@ -303,7 +303,7 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
}
}
public async Task<ICollection<CollectionAdminDetails>> GetManyByOrganizationIdWithPermissionsAsync(
public async Task<ICollection<CollectionAdminDetails>> GetManySharedByOrganizationIdWithPermissionsAsync(
Guid organizationId, Guid userId, bool includeAccessRelationships)
{
using (var scope = ServiceScopeFactory.CreateScope())

2
src/Infrastructure.EntityFramework/Repositories/Queries/CollectionAdminDetailsQuery.cs
View File

@@ -62,7 +62,7 @@ public class CollectionAdminDetailsQuery : IQuery<CollectionAdminDetails>
{
baseCollectionQuery = baseCollectionQuery.Where(x =>
x.c.OrganizationId == _organizationId &&
x.c.Type != CollectionType.DefaultUserCollection);
x.c.Type == CollectionType.SharedCollection);
}
else if (_collectionId.HasValue)
{

19
src/Sql/dbo/Stored Procedures/CollectionUser_ReadSharedCollectionsByOrganizationUserIds.sql Normal file
View File

@@ -0,0 +1,19 @@
CREATE PROCEDURE [dbo].[CollectionUser_ReadSharedCollectionsByOrganizationUserIds]
@OrganizationUserIds [dbo].[GuidIdArray] READONLY
AS
BEGIN
SET NOCOUNT ON
SELECT
CU.*
FROM
[dbo].[OrganizationUser] OU
INNER JOIN
[dbo].[CollectionUser] CU ON CU.[OrganizationUserId] = OU.[Id]
INNER JOIN
[dbo].[Collection] C ON CU.[CollectionId] = C.[Id]
INNER JOIN
@OrganizationUserIds OUI ON OUI.[Id] = OU.[Id]
WHERE
C.[Type] = 0 -- Only SharedCollection
END

23
src/Sql/dbo/Stored Procedures/OrganizationUserUserDetails_ReadWithSharedCollectionsById.sql Normal file
View File

@@ -0,0 +1,23 @@
CREATE PROCEDURE [dbo].[OrganizationUserUserDetails_ReadWithSharedCollectionsById]
@Id UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON
EXEC [OrganizationUserUserDetails_ReadById] @Id
SELECT
CU.[CollectionId] Id,
CU.[ReadOnly],
CU.[HidePasswords],
CU.[Manage]
FROM
[dbo].[OrganizationUser] OU
INNER JOIN
[dbo].[CollectionUser] CU ON CU.[OrganizationUserId] = [OU].[Id]
INNER JOIN
[dbo].[Collection] C ON CU.[CollectionId] = C.[Id]
WHERE
[OrganizationUserId] = @Id
AND C.[Type] = 0 -- Only SharedCollection
END

86
src/Sql/dbo/Vault/Stored Procedures/Collections/Collection_ReadSharedCollectionsByOrganizationIdWithPermissions.sql Normal file
View File

@@ -0,0 +1,86 @@
CREATE PROCEDURE [dbo].[Collection_ReadSharedCollectionsByOrganizationIdWithPermissions]
@OrganizationId UNIQUEIDENTIFIER,
@UserId UNIQUEIDENTIFIER,
@IncludeAccessRelationships BIT
AS
BEGIN
SET NOCOUNT ON
SELECT
C.*,
MIN(CASE
WHEN
COALESCE(CU.[ReadOnly], CG.[ReadOnly], 0) = 0
THEN 0
ELSE 1
END) AS [ReadOnly],
MIN(CASE
WHEN
COALESCE(CU.[HidePasswords], CG.[HidePasswords], 0) = 0
THEN 0
ELSE 1
END) AS [HidePasswords],
MAX(CASE
WHEN
COALESCE(CU.[Manage], CG.[Manage], 0) = 0
THEN 0
ELSE 1
END) AS [Manage],
MAX(CASE
WHEN
CU.[CollectionId] IS NULL AND CG.[CollectionId] IS NULL
THEN 0
ELSE 1
END) AS [Assigned],
CASE
WHEN
-- No user or group has manage rights
NOT EXISTS(
SELECT 1
FROM [dbo].[CollectionUser] CU2
JOIN [dbo].[OrganizationUser] OU2 ON CU2.[OrganizationUserId] = OU2.[Id]
WHERE
CU2.[CollectionId] = C.[Id] AND
CU2.[Manage] = 1
)
AND NOT EXISTS (
SELECT 1
FROM [dbo].[CollectionGroup] CG2
WHERE
CG2.[CollectionId] = C.[Id] AND
CG2.[Manage] = 1
)
THEN 1
ELSE 0
END AS [Unmanaged]
FROM
[dbo].[CollectionView] C
LEFT JOIN
[dbo].[OrganizationUser] OU ON C.[OrganizationId] = OU.[OrganizationId] AND OU.[UserId] = @UserId
LEFT JOIN
[dbo].[CollectionUser] CU ON CU.[CollectionId] = C.[Id] AND CU.[OrganizationUserId] = [OU].[Id]
LEFT JOIN
[dbo].[GroupUser] GU ON CU.[CollectionId] IS NULL AND GU.[OrganizationUserId] = OU.[Id]
LEFT JOIN
[dbo].[Group] G ON G.[Id] = GU.[GroupId]
LEFT JOIN
[dbo].[CollectionGroup] CG ON CG.[CollectionId] = C.[Id] AND CG.[GroupId] = GU.[GroupId]
WHERE
C.[OrganizationId] = @OrganizationId AND
C.[Type] = 0 -- Only SharedCollection
GROUP BY
C.[Id],
C.[OrganizationId],
C.[Name],
C.[CreationDate],
C.[RevisionDate],
C.[ExternalId],
C.[DefaultUserCollectionEmail],
C.[Type]
IF (@IncludeAccessRelationships = 1)
BEGIN
EXEC [dbo].[CollectionGroup_ReadByOrganizationId] @OrganizationId
EXEC [dbo].[CollectionUser_ReadByOrganizationId] @OrganizationId
END
END