bitwarden / bitwarden/server

UserRepository EF UpdateUserDataAsync does not run delegates in a transaction

Open
#7,566 3 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
20.1k
Forks
1.8k
Avg merge
4d 8h
Merged PRs (30d)
75

Description

### Steps To Reproduce

1. Deploy the server against an Entity Framework provider (PostgreSQL, MySQL, or SQLite) — i.e. any non-MSSQL self-hosted configuration.
2. As an authenticated user, invoke any code path that batches user-state mutations through `IUserRepository.UpdateUserDataAsync(IEnumerable)` or `IUserRepository.SetV2AccountCryptographicStateAsync(...)`. Master-password set/change, Key Connector key set, and `RegisterFinish` master-password unlock data update all flow through here.
3. Construct a batch of two delegates where the second one will throw — e.g. arrange the second delegate's stored mutation to violate a constraint, or inject a faulting delegate after `IUserRepository.SetMasterPassword(...)` in a test harness.
4. Execute the batch and observe the database state after the exception propagates.

### Expected Result

All delegates in the batch commit atomically or none do. The XML doc on `IUserRepository.UpdateUserDataAsync` and `IUserRepository.SetV2AccountCryptographicStateAsync` explicitly states the operations execute "in a single transaction." On a failure mid-batch, every prior write should roll back, leaving the user's cryptographic state (master password, KDF parameters, master-key-wrapped user key, salt, key-connector key) consistent with the pre-call state.

### Actual Result

On the EF code path, each delegate auto-commits independently and the outer transaction wraps nothing. A mid-batch failure leaves earlier delegates' writes durably committed.

Root cause is a mismatch between where the transaction is opened and where the writes are issued:

### Screenshots or Videos

N/A — backend correctness issue. Reproducible via integration test against the EF provider.

### Additional Context

- [`src/Core/Repositories/IUserRepository.cs`](src/Core/Repositories/IUserRepository.cs) (delegate definition, contract)
- [`src/Infrastructure.Dapper/Repositories/UserRepository.cs`](src/Infrastructure.Dapper/Repositories/UserRepository.cs) lines 433–546 (correct, reference behavior)
- [`src/Infrastructure.EntityFramework/Repositories/UserRepository.cs`](src/Infrastructure.EntityFramework/Repositories/UserRepository.cs) lines 511–610 (broken transactional behavior)
- Callers exercising the broken path include [`src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs`](src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs) and [`src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs`](src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs).

### Build Version

main @ 27ae3d545 (also reproduces on every prior commit since the UpdateUserData delegate was introduced)

### Environment

Self-Hosted

### Environment Details

- Database provider: PostgreSQL, MySQL, or SQLite (any EF-backed deployment). MSSQL/Dapper deployments are not affected.
- Operating system: provider-agnostic
- Environment: Docker / Kubernetes / bare metal — not deployment-shape-dependent
- Hardware: N/A

### Issue Tracking Info

- [x] I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.

Contributor guide

Open the contributing guide

Research direction

Read the transaction contract in src/Core/Repositories/IUserRepository.cs, then compare the reference implementation in src/Infrastructure.Dapper/Repositories/UserRepository.cs (lines 433–546) with the EF implementation in src/Infrastructure.EntityFramework/Repositories/UserRepository.cs (lines 511–610). Reproduce the failure with an EF integration test using two delegates, and consider the work complete when a mid-batch exception leaves all prior writes rolled back.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend, database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.