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

[AC-1597] Revert GetByUserIdWithPolicyDetailsAsync changes to unblock SQL CPU (#3203)

* Revert "[PM-3007] Caching user policies on PolicyService variable (#3117)"

This reverts commit 78588d0246.

* Don't delete old migration script

* Add migration to revert sproc
This commit is contained in:
Thomas Rittson 2023-08-16 13:42:09 +10:00 committed by GitHub
parent fd1ae91d74
commit fc814ff352
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 71 additions and 34 deletions

View File

@ -39,6 +39,6 @@ public interface IOrganizationUserRepository : IRepository<OrganizationUser, Gui
Task<IEnumerable<OrganizationUserUserDetails>> GetManyByMinimumRoleAsync(Guid organizationId, OrganizationUserType minRole); Task<IEnumerable<OrganizationUserUserDetails>> GetManyByMinimumRoleAsync(Guid organizationId, OrganizationUserType minRole);
Task RevokeAsync(Guid id); Task RevokeAsync(Guid id);
Task RestoreAsync(Guid id, OrganizationUserStatusType status); Task RestoreAsync(Guid id, OrganizationUserStatusType status);
Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId); Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType);
Task<int> GetOccupiedSmSeatCountByOrganizationIdAsync(Guid organizationId); Task<int> GetOccupiedSmSeatCountByOrganizationIdAsync(Guid organizationId);
} }

View File

@ -20,8 +20,6 @@ public class PolicyService : IPolicyService
private readonly IMailService _mailService; private readonly IMailService _mailService;
private readonly GlobalSettings _globalSettings; private readonly GlobalSettings _globalSettings;
private IEnumerable<OrganizationUserPolicyDetails> _cachedOrganizationUserPolicyDetails;
public PolicyService( public PolicyService(
IEventService eventService, IEventService eventService,
IOrganizationRepository organizationRepository, IOrganizationRepository organizationRepository,
@ -196,25 +194,18 @@ public class PolicyService : IPolicyService
return result.Any(); return result.Any();
} }
private async Task<IEnumerable<OrganizationUserPolicyDetails>> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType? policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) private async Task<IEnumerable<OrganizationUserPolicyDetails>> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted)
{ {
// Check if the cached policies are available var organizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, policyType);
if (_cachedOrganizationUserPolicyDetails == null)
{
// Cached policies not available, retrieve from the repository
_cachedOrganizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId);
}
var excludedUserTypes = GetUserTypesExcludedFromPolicy(policyType); var excludedUserTypes = GetUserTypesExcludedFromPolicy(policyType);
return _cachedOrganizationUserPolicyDetails.Where(o => return organizationUserPolicyDetails.Where(o =>
(policyType == null || o.PolicyType == policyType) &&
o.PolicyEnabled && o.PolicyEnabled &&
!excludedUserTypes.Contains(o.OrganizationUserType) && !excludedUserTypes.Contains(o.OrganizationUserType) &&
o.OrganizationUserStatus >= minStatus && o.OrganizationUserStatus >= minStatus &&
!o.IsProvider); !o.IsProvider);
} }
private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType? policyType) private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType policyType)
{ {
switch (policyType) switch (policyType)
{ {

View File

@ -505,13 +505,13 @@ public class OrganizationUserRepository : Repository<OrganizationUser, Guid>, IO
} }
} }
public async Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId) public async Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType)
{ {
using (var connection = new SqlConnection(ConnectionString)) using (var connection = new SqlConnection(ConnectionString))
{ {
var results = await connection.QueryAsync<OrganizationUserPolicyDetails>( var results = await connection.QueryAsync<OrganizationUserPolicyDetails>(
$"[{Schema}].[{Table}_ReadByUserIdWithPolicyDetails]", $"[{Schema}].[{Table}_ReadByUserIdWithPolicyDetails]",
new { UserId = userId }, new { UserId = userId, PolicyType = policyType },
commandType: CommandType.StoredProcedure); commandType: CommandType.StoredProcedure);
return results.ToList(); return results.ToList();

View File

@ -588,7 +588,7 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
} }
} }
public async Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId) public async Task<IEnumerable<OrganizationUserPolicyDetails>> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType)
{ {
using (var scope = ServiceScopeFactory.CreateScope()) using (var scope = ServiceScopeFactory.CreateScope())
{ {
@ -604,7 +604,8 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
join ou in dbContext.OrganizationUsers join ou in dbContext.OrganizationUsers
on p.OrganizationId equals ou.OrganizationId on p.OrganizationId equals ou.OrganizationId
let email = dbContext.Users.Find(userId).Email // Invited orgUsers do not have a UserId associated with them, so we have to match up their email let email = dbContext.Users.Find(userId).Email // Invited orgUsers do not have a UserId associated with them, so we have to match up their email
where ou.UserId == userId || ou.Email == email where p.Type == policyType &&
(ou.UserId == userId || ou.Email == email)
select new OrganizationUserPolicyDetails select new OrganizationUserPolicyDetails
{ {
OrganizationUserId = ou.Id, OrganizationUserId = ou.Id,

View File

@ -1,5 +1,6 @@
CREATE PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails] CREATE PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails]
@UserId UNIQUEIDENTIFIER @UserId UNIQUEIDENTIFIER,
@PolicyType TINYINT
AS AS
BEGIN BEGIN
SET NOCOUNT ON SET NOCOUNT ON
@ -12,19 +13,22 @@ SELECT
OU.[Type] AS OrganizationUserType, OU.[Type] AS OrganizationUserType,
OU.[Status] AS OrganizationUserStatus, OU.[Status] AS OrganizationUserStatus,
OU.[Permissions] AS OrganizationUserPermissionsData, OU.[Permissions] AS OrganizationUserPermissionsData,
CASE WHEN PU.[ProviderId] IS NOT NULL THEN 1 ELSE 0 END AS IsProvider CASE WHEN EXISTS (
SELECT 1
FROM [dbo].[ProviderUserView] PU
INNER JOIN [dbo].[ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId]
WHERE PU.[UserId] = OU.[UserId] AND PO.[OrganizationId] = P.[OrganizationId]
) THEN 1 ELSE 0 END AS IsProvider
FROM [dbo].[PolicyView] P FROM [dbo].[PolicyView] P
INNER JOIN [dbo].[OrganizationUserView] OU INNER JOIN [dbo].[OrganizationUserView] OU
ON P.[OrganizationId] = OU.[OrganizationId] ON P.[OrganizationId] = OU.[OrganizationId]
LEFT JOIN [dbo].[ProviderUserView] PU WHERE P.[Type] = @PolicyType AND
ON PU.[UserId] = OU.[UserId] (
LEFT JOIN [dbo].[ProviderOrganizationView] PO (OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId
ON PO.[ProviderId] = PU.[ProviderId] AND PO.[OrganizationId] = P.[OrganizationId] OR EXISTS (
WHERE SELECT 1
(OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId FROM [dbo].[UserView] U
OR EXISTS ( WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email
SELECT 1 )
FROM [dbo].[UserView] U
WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email
) )
END END

View File

@ -624,12 +624,18 @@ public class PolicyServiceTests
private static void SetupUserPolicies(Guid userId, SutProvider<PolicyService> sutProvider) private static void SetupUserPolicies(Guid userId, SutProvider<PolicyService> sutProvider)
{ {
sutProvider.GetDependency<IOrganizationUserRepository>() sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByUserIdWithPolicyDetailsAsync(userId) .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.RequireSso)
.Returns(new List<OrganizationUserPolicyDetails> .Returns(new List<OrganizationUserPolicyDetails>
{ {
new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = false, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false}, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = false, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false},
new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false },
new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true }
});
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.DisableSend)
.Returns(new List<OrganizationUserPolicyDetails>
{
new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = false }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = false },
new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true } new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true }
}); });

View File

@ -274,7 +274,7 @@ public class OrganizationUserRepositoryTests
} }
// Act // Act
var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id); var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id, policy.Type);
results.Add(result.FirstOrDefault()); results.Add(result.FirstOrDefault());
} }

View File

@ -0,0 +1,35 @@
CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails]
@UserId UNIQUEIDENTIFIER,
@PolicyType TINYINT
AS
BEGIN
SET NOCOUNT ON
SELECT
OU.[Id] AS OrganizationUserId,
P.[OrganizationId],
P.[Type] AS PolicyType,
P.[Enabled] AS PolicyEnabled,
P.[Data] AS PolicyData,
OU.[Type] AS OrganizationUserType,
OU.[Status] AS OrganizationUserStatus,
OU.[Permissions] AS OrganizationUserPermissionsData,
CASE WHEN EXISTS (
SELECT 1
FROM [dbo].[ProviderUserView] PU
INNER JOIN [dbo].[ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId]
WHERE PU.[UserId] = OU.[UserId] AND PO.[OrganizationId] = P.[OrganizationId]
) THEN 1 ELSE 0 END AS IsProvider
FROM [dbo].[PolicyView] P
INNER JOIN [dbo].[OrganizationUserView] OU
ON P.[OrganizationId] = OU.[OrganizationId]
WHERE P.[Type] = @PolicyType AND
(
(OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId
OR EXISTS (
SELECT 1
FROM [dbo].[UserView] U
WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email
)
)
END
GO