Skip to content

Commit 9863df4

Browse files
jrmccannonjrmccannon
committed
Added transaction isolation and wrapped all calls (cept sm) in transaaction
1 parent 6c8284b commit 9863df4

12 files changed

Lines changed: 133 additions & 120 deletions

File tree

‎bitwarden_license/src/Scim/Users/PostUserCommand.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#nullable enable
22

3+
using System.Data;
34
using Bit.Core;
45
using Bit.Core.AdminConsole.Enums;
56
using Bit.Core.AdminConsole.Models.Business;
@@ -50,7 +51,7 @@ public class PostUserCommand(
5051
Guid organizationId,
5152
ScimProviderType scimProvider)
5253
{
53-
await transactionManager.BeginTransactionAsync();
54+
await transactionManager.BeginTransactionAsync(IsolationLevel.Serializable);
5455

5556
var organization = await organizationRepository.GetByIdAsync(organizationId);
5657

‎src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,14 @@ private async Task<CommandResult<InviteOrganizationUsersResponse>> InviteOrganiz
156156
{
157157
logger.LogError(ex, FailedToInviteUsersError.Code);
158158

159-
await organizationUserRepository.DeleteManyAsync(organizationUserToInviteEntities.Select(x => x.OrganizationUser.Id));
159+
// this should already be done
160+
//await organizationUserRepository.DeleteManyAsync(organizationUserToInviteEntities.Select(x => x.OrganizationUser.Id));
160161

161162
// Do this first so that SmSeats never exceed PM seats (due to current billing requirements)
162163
await RevertSecretsManagerChangesAsync(validatedRequest, organization, validatedRequest.Value.InviteOrganization.SmSeats);
163164

164-
await RevertPasswordManagerChangesAsync(validatedRequest, organization);
165+
//this should already be done
166+
//await RevertPasswordManagerChangesAsync(validatedRequest, organization);
165167

166168
return new Failure<InviteOrganizationUsersResponse>(
167169
new FailedToInviteUsersError(

‎src/Core/Platform/Data/ITransactionManager.cs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace Bit.Core.Platform.Data;
1+
using System.Data;
2+
3+
namespace Bit.Core.Platform.Data;
24

35
/// <summary>
46
/// Manages ambient database transactions that span multiple repository calls.
@@ -10,9 +12,12 @@ public interface ITransactionManager
1012
/// Begins a new ambient transaction. All repository operations on the current
1113
/// async flow will use the same connection and transaction until disposed.
1214
/// Supports nesting: inner calls increment a reference count; only the
13-
/// outermost Dispose/Commit actually affects the database.
15+
/// outermost Dispose/Commit actually affects the database. The isolation level
16+
/// on a nested call is ignored — the inner scope joins the outer transaction.
1417
/// </summary>
15-
Task<ITransactionScope> BeginTransactionAsync(CancellationToken cancellationToken = default);
18+
Task<ITransactionScope> BeginTransactionAsync(
19+
IsolationLevel isolationLevel = IsolationLevel.ReadCommitted,
20+
CancellationToken cancellationToken = default);
1621

1722
/// <summary>
1823
/// Returns true if the current async flow has an active ambient transaction.

‎src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,16 @@ public async Task<ICollection<Organization>> GetManyByIdsAsync(IEnumerable<Guid>
250250

251251
public async Task<OrganizationSeatCounts> GetOccupiedSeatCountByOrganizationIdAsync(Guid organizationId)
252252
{
253-
using (var connection = new SqlConnection(ConnectionString))
253+
return await ExecuteWithConnectionAsync(async (connection, transaction) =>
254254
{
255255
var result = await connection.QueryAsync<OrganizationSeatCounts>(
256256
"[dbo].[Organization_ReadOccupiedSeatCountByOrganizationId]",
257257
new { OrganizationId = organizationId },
258+
transaction: transaction,
258259
commandType: CommandType.StoredProcedure);
259260

260261
return result.SingleOrDefault() ?? new OrganizationSeatCounts();
261-
}
262+
});
262263
}
263264

264265
public async Task<IEnumerable<Organization>> GetOrganizationsForSubscriptionSyncAsync()
@@ -285,11 +286,13 @@ await connection.ExecuteAsync("[dbo].[Organization_UpdateSubscriptionStatus]",
285286

286287
public async Task IncrementSeatCountAsync(Guid organizationId, int increaseAmount, DateTime requestDate)
287288
{
288-
await using var connection = new SqlConnection(ConnectionString);
289-
290-
await connection.ExecuteAsync("[dbo].[Organization_IncrementSeatCount]",
291-
new { OrganizationId = organizationId, SeatsToAdd = increaseAmount, RequestDate = requestDate },
292-
commandType: CommandType.StoredProcedure);
289+
await ExecuteWithConnectionAsync(async (connection, transaction) =>
290+
{
291+
await connection.ExecuteAsync("[dbo].[Organization_IncrementSeatCount]",
292+
new { OrganizationId = organizationId, SeatsToAdd = increaseAmount, RequestDate = requestDate },
293+
transaction: transaction,
294+
commandType: CommandType.StoredProcedure);
295+
});
293296
}
294297

295298
public async Task InitializeOrganizationAsync(Organization organization, Func<DbConnection, DbTransaction, Task> confirmOwnerAction)

‎src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs‎

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,16 @@ public async Task<int> GetCountByOrganizationAsync(Guid organizationId, string e
9494

9595
public async Task<int> GetOccupiedSmSeatCountByOrganizationIdAsync(Guid organizationId)
9696
{
97-
using (var connection = new SqlConnection(ConnectionString))
97+
return await ExecuteWithConnectionAsync(async (connection, transaction) =>
9898
{
9999
var result = await connection.ExecuteScalarAsync<int>(
100100
"[dbo].[OrganizationUser_ReadOccupiedSmSeatCountByOrganizationId]",
101101
new { OrganizationId = organizationId },
102+
transaction: transaction,
102103
commandType: CommandType.StoredProcedure);
103104

104105
return result;
105-
}
106+
});
106107
}
107108

108109
public async Task<ICollection<string>> SelectKnownEmailsAsync(Guid organizationId, IEnumerable<string> emails,
@@ -205,11 +206,12 @@ public async Task<ICollection<OrganizationUser>> GetManyByOrganizationAsync(Guid
205206

206207
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync(Guid organizationId, bool includeGroups, bool includeSharedCollections)
207208
{
208-
using (var connection = new SqlConnection(ConnectionString))
209+
return await ExecuteWithConnectionAsync(async (connection, transaction) =>
209210
{
210211
var results = await connection.QueryAsync<OrganizationUserUserDetails>(
211212
"[dbo].[OrganizationUserUserDetails_ReadByOrganizationId]",
212213
new { OrganizationId = organizationId },
214+
transaction: transaction,
213215
commandType: CommandType.StoredProcedure);
214216

215217
List<IGrouping<Guid, GroupUser>>? userGroups = null;
@@ -229,6 +231,7 @@ public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrga
229231
userGroups = (await connection.QueryAsync<GroupUser>(
230232
"[dbo].[GroupUser_ReadByOrganizationUserIds]",
231233
new { OrganizationUserIds = orgUserIds },
234+
transaction: transaction,
232235
commandType: CommandType.StoredProcedure)).GroupBy(u => u.OrganizationUserId).ToList();
233236
}
234237

@@ -237,6 +240,7 @@ public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrga
237240
userCollections = (await connection.QueryAsync<CollectionUser>(
238241
"[dbo].[CollectionUser_ReadSharedCollectionsByOrganizationUserIds]",
239242
new { OrganizationUserIds = orgUserIds },
243+
transaction: transaction,
240244
commandType: CommandType.StoredProcedure)).GroupBy(u => u.OrganizationUserId).ToList();
241245
}
242246

@@ -265,7 +269,7 @@ public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrga
265269
}
266270

267271
return users;
268-
}
272+
});
269273
}
270274

271275
public async Task<ICollection<OrganizationUserUserDetails>> GetManyDetailsByOrganizationAsync_vNext(Guid organizationId, bool includeGroups, bool includeSharedCollections)
@@ -664,38 +668,40 @@ public async Task<IEnumerable<OrganizationUserUserDetails>> GetManyDetailsByRole
664668

665669
public async Task CreateManyAsync(IEnumerable<CreateOrganizationUser> organizationUserCollection)
666670
{
667-
await using var connection = new SqlConnection(_marsConnectionString);
668-
669671
var organizationUsersList = organizationUserCollection.ToList();
670672
if (organizationUsersList.Count == 0)
671673
{
672674
return;
673675
}
674676

675-
await connection.ExecuteAsync(
676-
$"[{Schema}].[OrganizationUser_CreateManyWithCollectionsAndGroups]",
677-
new
678-
{
679-
OrganizationUserData = JsonSerializer.Serialize(organizationUsersList.Select(x => x.OrganizationUser)),
680-
CollectionData = JsonSerializer.Serialize(organizationUsersList
681-
.SelectMany(x => x.Collections, (user, collection) => new CollectionUser
682-
{
683-
CollectionId = collection.Id,
684-
OrganizationUserId = user.OrganizationUser.Id,
685-
ReadOnly = collection.ReadOnly,
686-
HidePasswords = collection.HidePasswords,
687-
Manage = collection.Manage
688-
})),
689-
GroupData = JsonSerializer.Serialize(organizationUsersList
690-
.SelectMany(x => x.Groups, (user, group) => new GroupUser
691-
{
692-
GroupId = group,
693-
OrganizationUserId = user.OrganizationUser.Id
694-
})),
695-
// Use the same RevisionDate as the created OrganizationUsers
696-
RevisionDate = organizationUsersList.First().OrganizationUser.RevisionDate
697-
},
698-
commandType: CommandType.StoredProcedure);
677+
await ExecuteWithConnectionAsync(async (connection, transaction) =>
678+
{
679+
await connection.ExecuteAsync(
680+
$"[{Schema}].[OrganizationUser_CreateManyWithCollectionsAndGroups]",
681+
new
682+
{
683+
OrganizationUserData =
684+
JsonSerializer.Serialize(organizationUsersList.Select(x => x.OrganizationUser)),
685+
CollectionData = JsonSerializer.Serialize(organizationUsersList
686+
.SelectMany(x => x.Collections,
687+
(user, collection) => new CollectionUser
688+
{
689+
CollectionId = collection.Id,
690+
OrganizationUserId = user.OrganizationUser.Id,
691+
ReadOnly = collection.ReadOnly,
692+
HidePasswords = collection.HidePasswords,
693+
Manage = collection.Manage
694+
})),
695+
GroupData = JsonSerializer.Serialize(organizationUsersList
696+
.SelectMany(x => x.Groups,
697+
(user, group) =>
698+
new GroupUser { GroupId = group, OrganizationUserId = user.OrganizationUser.Id })),
699+
// Use the same RevisionDate as the created OrganizationUsers
700+
RevisionDate = organizationUsersList.First().OrganizationUser.RevisionDate
701+
},
702+
transaction: transaction,
703+
commandType: CommandType.StoredProcedure);
704+
});
699705
}
700706

701707
public async Task<bool> ConfirmOrganizationUserAsync(AcceptedOrganizationUserToConfirm organizationUserToConfirm)

‎src/Infrastructure.Dapper/AdminConsole/Repositories/ProviderRepository.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,16 @@ public ProviderRepository(string connectionString, string readOnlyConnectionStri
4949

5050
public async Task<Provider?> GetByOrganizationIdAsync(Guid organizationId)
5151
{
52-
using (var connection = new SqlConnection(ConnectionString))
52+
return await ExecuteWithConnectionAsync(async (connection, transaction) =>
5353
{
5454
var results = await connection.QueryAsync<Provider>(
5555
"[dbo].[Provider_ReadByOrganizationId]",
5656
new { OrganizationId = organizationId },
57+
transaction: transaction,
5758
commandType: CommandType.StoredProcedure);
5859

5960
return results.FirstOrDefault();
60-
}
61+
});
6162
}
6263

6364
public async Task<ICollection<Provider>> SearchAsync(string name, string userEmail, int skip, int take)

‎src/Infrastructure.Dapper/Data/DapperTransactionManager.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Data.Common;
1+
using System.Data;
22
using Bit.Core.Platform.Data;
33
using Bit.Core.Settings;
44
using Microsoft.Data.SqlClient;
@@ -16,7 +16,9 @@ public DapperTransactionManager(GlobalSettings globalSettings)
1616

1717
public bool HasActiveTransaction => TransactionState.Current is not null;
1818

19-
public async Task<ITransactionScope> BeginTransactionAsync(CancellationToken cancellationToken = default)
19+
public async Task<ITransactionScope> BeginTransactionAsync(
20+
IsolationLevel isolationLevel = IsolationLevel.ReadCommitted,
21+
CancellationToken cancellationToken = default)
2022
{
2123
var existing = TransactionState.Current;
2224
if (existing is not null)
@@ -27,7 +29,7 @@ public async Task<ITransactionScope> BeginTransactionAsync(CancellationToken can
2729

2830
var connection = new SqlConnection(_connectionString);
2931
await connection.OpenAsync(cancellationToken);
30-
var transaction = (DbTransaction)await connection.BeginTransactionAsync(cancellationToken);
32+
var transaction = await connection.BeginTransactionAsync(isolationLevel, cancellationToken);
3133

3234
var holder = new TransactionHolder
3335
{

‎src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -424,26 +424,24 @@ where ids.Contains(organization.Id)
424424

425425
public async Task<OrganizationSeatCounts> GetOccupiedSeatCountByOrganizationIdAsync(Guid organizationId)
426426
{
427-
using (var scope = ServiceScopeFactory.CreateScope())
427+
return await ExecuteWithContextAsync(async dbContext =>
428428
{
429-
var dbContext = GetDatabaseContext(scope);
430429
var users = await dbContext.OrganizationUsers
431430
.Where(ou => ou.OrganizationId == organizationId && ou.Status >= 0)
432431
.CountAsync();
433432

434433
var sponsored = await dbContext.OrganizationSponsorships
435434
.Where(os => os.SponsoringOrganizationId == organizationId &&
436-
os.IsAdminInitiated &&
437-
(os.ToDelete == false || (os.ToDelete == true && os.ValidUntil != null && os.ValidUntil > DateTime.UtcNow)) &&
438-
(os.SponsoredOrganizationId == null || (os.SponsoredOrganizationId != null && (os.ValidUntil == null || os.ValidUntil > DateTime.UtcNow))))
435+
os.IsAdminInitiated &&
436+
(os.ToDelete == false || (os.ToDelete == true && os.ValidUntil != null &&
437+
os.ValidUntil > DateTime.UtcNow)) &&
438+
(os.SponsoredOrganizationId == null || (os.SponsoredOrganizationId != null &&
439+
(os.ValidUntil == null ||
440+
os.ValidUntil > DateTime.UtcNow))))
439441
.CountAsync();
440442

441-
return new OrganizationSeatCounts
442-
{
443-
Users = users,
444-
Sponsored = sponsored
445-
};
446-
}
443+
return new OrganizationSeatCounts { Users = users, Sponsored = sponsored };
444+
});
447445
}
448446

449447
public async Task<IEnumerable<Core.AdminConsole.Entities.Organization>> GetOrganizationsForSubscriptionSyncAsync()
@@ -472,15 +470,15 @@ await dbContext.Organizations
472470

473471
public async Task IncrementSeatCountAsync(Guid organizationId, int increaseAmount, DateTime requestDate)
474472
{
475-
using var scope = ServiceScopeFactory.CreateScope();
476-
await using var dbContext = GetDatabaseContext(scope);
477-
478-
await dbContext.Organizations
479-
.Where(o => o.Id == organizationId)
480-
.ExecuteUpdateAsync(s => s
481-
.SetProperty(o => o.Seats, o => o.Seats + increaseAmount)
482-
.SetProperty(o => o.SyncSeats, true)
483-
.SetProperty(o => o.RevisionDate, requestDate));
473+
await ExecuteWithContextAsync(async dbContext =>
474+
{
475+
await dbContext.Organizations
476+
.Where(o => o.Id == organizationId)
477+
.ExecuteUpdateAsync(s => s
478+
.SetProperty(o => o.Seats, o => o.Seats + increaseAmount)
479+
.SetProperty(o => o.SyncSeats, true)
480+
.SetProperty(o => o.RevisionDate, requestDate));
481+
});
484482
}
485483

486484
public async Task InitializeOrganizationAsync(Core.AdminConsole.Entities.Organization organization, Func<DbConnection, DbTransaction, Task> confirmOwnerAction)

0 commit comments

Comments
 (0)