[PM-13706] Add repository + stored procedures for private key regeneration #11
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile
|
|
||
| public class UserAsymmetricKeys | ||
| { | ||
| public Guid UserId { get; set; } |
There was a problem hiding this comment.
style: Consider making UserId a required property for consistency
|
|
||
| public async Task RegenerateUserAsymmetricKeysAsync(UserAsymmetricKeys userAsymmetricKeys) | ||
| { | ||
| await using var connection = new SqlConnection(ConnectionString); |
There was a problem hiding this comment.
style: consider using using instead of await using for better resource management
| await connection.ExecuteAsync("[dbo].[UserAsymmetricKeys_Regenerate]", | ||
| new | ||
| { | ||
| userAsymmetricKeys.UserId, | ||
| userAsymmetricKeys.PublicKey, | ||
| PrivateKey = userAsymmetricKeys.UserKeyEncryptedPrivateKey | ||
| }, commandType: CommandType.StoredProcedure); |
There was a problem hiding this comment.
style: wrap in try-catch to handle potential SQL exceptions
| await using var scope = ServiceScopeFactory.CreateAsyncScope(); | ||
| var dbContext = GetDatabaseContext(scope); | ||
|
|
||
| var entity = await dbContext.Users.FindAsync(userAsymmetricKeys.UserId); |
There was a problem hiding this comment.
style: Consider using FirstOrDefaultAsync with a predicate instead of FindAsync for better performance with large datasets
| entity.PrivateKey = userAsymmetricKeys.UserKeyEncryptedPrivateKey; | ||
| entity.RevisionDate = utcNow; | ||
| entity.AccountRevisionDate = utcNow; | ||
| await dbContext.SaveChangesAsync(); |
There was a problem hiding this comment.
style: Add error handling around SaveChangesAsync
| UPDATE [dbo].[User] | ||
| SET [PublicKey] = @PublicKey, | ||
| [PrivateKey] = @PrivateKey, | ||
| [RevisionDate] = @UtcNow, | ||
| [AccountRevisionDate] = @UtcNow | ||
| WHERE [Id] = @UserId |
There was a problem hiding this comment.
style: Add error handling to check if the update was successful
| @PublicKey VARCHAR(MAX), | ||
| @PrivateKey VARCHAR(MAX) |
There was a problem hiding this comment.
style: Consider using NVARCHAR(MAX) for @PublicKey and @privatekey to support Unicode characters
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-13706
📔 Objective
The purpose of this PR is to add the database stored procedure and repository layer for the private key regeneration project.
This initial phase will only target users not in organizations and without emergency access setup.
In future PRs, the database operations will expand to handle these cases.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changesGreptile Summary
This pull request introduces a new repository and stored procedures for regenerating user asymmetric keys, focusing on users not in organizations and without emergency access setup.
UserAsymmetricKeysmodel in/src/Core/KeyManagement/Models/Data/UserAsymmetricKeys.cswith properties for UserId, PublicKey, and UserKeyEncryptedPrivateKeyIUserAsymmetricKeysRepositoryinterface and its Dapper and Entity Framework implementations for key regenerationUserAsymmetricKeys_Regeneratein SQL and migration scripts to update user keys and revision datesRegenerateUserAsymmetricKeysAsyncmethod in repositories to execute key regeneration logic