Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-13706] Add repository + stored procedures for private key regeneration #11

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Core/KeyManagement/Models/Data/UserAsymmetricKeys.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
๏ปฟ#nullable enable
namespace Bit.Core.KeyManagement.Models.Data;

public class UserAsymmetricKeys
{
public Guid UserId { get; set; }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider making UserId a required property for consistency

public required string PublicKey { get; set; }
public required string UserKeyEncryptedPrivateKey { get; set; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
๏ปฟ#nullable enable
using Bit.Core.KeyManagement.Models.Data;

namespace Bit.Core.KeyManagement.Repositories;

public interface IUserAsymmetricKeysRepository
{
Task RegenerateUserAsymmetricKeysAsync(UserAsymmetricKeys userAsymmetricKeys);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
๏ปฟusing Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.Repositories;
using Bit.Core.Billing.Repositories;
using Bit.Core.KeyManagement.Repositories;
using Bit.Core.NotificationCenter.Repositories;
using Bit.Core.Repositories;
using Bit.Core.SecretsManager.Repositories;
Expand All @@ -9,6 +10,7 @@
using Bit.Infrastructure.Dapper.AdminConsole.Repositories;
using Bit.Infrastructure.Dapper.Auth.Repositories;
using Bit.Infrastructure.Dapper.Billing.Repositories;
using Bit.Infrastructure.Dapper.KeyManagement.Repositories;
using Bit.Infrastructure.Dapper.NotificationCenter.Repositories;
using Bit.Infrastructure.Dapper.Repositories;
using Bit.Infrastructure.Dapper.SecretsManager.Repositories;
Expand Down Expand Up @@ -58,6 +60,7 @@ public static void AddDapperRepositories(this IServiceCollection services, bool
services.AddSingleton<INotificationStatusRepository, NotificationStatusRepository>();
services
.AddSingleton<IClientOrganizationMigrationRecordRepository, ClientOrganizationMigrationRecordRepository>();
services.AddSingleton<IUserAsymmetricKeysRepository, UserAsymmetricKeysRepository>();

if (selfHosted)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
๏ปฟ#nullable enable
using System.Data;
using Bit.Core.KeyManagement.Models.Data;
using Bit.Core.KeyManagement.Repositories;
using Bit.Core.Settings;
using Bit.Infrastructure.Dapper.Repositories;
using Dapper;
using Microsoft.Data.SqlClient;

namespace Bit.Infrastructure.Dapper.KeyManagement.Repositories;

public class UserAsymmetricKeysRepository : BaseRepository, IUserAsymmetricKeysRepository
{
public UserAsymmetricKeysRepository(GlobalSettings globalSettings)
: this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString)
{
}

public UserAsymmetricKeysRepository(string connectionString, string readOnlyConnectionString) : base(
connectionString, readOnlyConnectionString)
{
}

public async Task RegenerateUserAsymmetricKeysAsync(UserAsymmetricKeys userAsymmetricKeys)
{
await using var connection = new SqlConnection(ConnectionString);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Comment on lines +28 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: wrap in try-catch to handle potential SQL exceptions

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Bit.Core.Auth.Repositories;
using Bit.Core.Billing.Repositories;
using Bit.Core.Enums;
using Bit.Core.KeyManagement.Repositories;
using Bit.Core.NotificationCenter.Repositories;
using Bit.Core.Repositories;
using Bit.Core.SecretsManager.Repositories;
Expand All @@ -10,6 +11,7 @@
using Bit.Infrastructure.EntityFramework.AdminConsole.Repositories;
using Bit.Infrastructure.EntityFramework.Auth.Repositories;
using Bit.Infrastructure.EntityFramework.Billing.Repositories;
using Bit.Infrastructure.EntityFramework.KeyManagement.Repositories;
using Bit.Infrastructure.EntityFramework.NotificationCenter.Repositories;
using Bit.Infrastructure.EntityFramework.Repositories;
using Bit.Infrastructure.EntityFramework.SecretsManager.Repositories;
Expand Down Expand Up @@ -95,6 +97,7 @@ public static void AddPasswordManagerEFRepositories(this IServiceCollection serv
services.AddSingleton<INotificationStatusRepository, NotificationStatusRepository>();
services
.AddSingleton<IClientOrganizationMigrationRecordRepository, ClientOrganizationMigrationRecordRepository>();
services.AddSingleton<IUserAsymmetricKeysRepository, UserAsymmetricKeysRepository>();

if (selfHosted)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
๏ปฟ#nullable enable
using AutoMapper;
using Bit.Core.KeyManagement.Models.Data;
using Bit.Core.KeyManagement.Repositories;
using Bit.Infrastructure.EntityFramework.Repositories;
using Microsoft.Extensions.DependencyInjection;

namespace Bit.Infrastructure.EntityFramework.KeyManagement.Repositories;

public class UserAsymmetricKeysRepository : BaseEntityFrameworkRepository, IUserAsymmetricKeysRepository
{
public UserAsymmetricKeysRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) : base(
serviceScopeFactory,
mapper)
{
}

public async Task RegenerateUserAsymmetricKeysAsync(UserAsymmetricKeys userAsymmetricKeys)
{
await using var scope = ServiceScopeFactory.CreateAsyncScope();
var dbContext = GetDatabaseContext(scope);

var entity = await dbContext.Users.FindAsync(userAsymmetricKeys.UserId);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using FirstOrDefaultAsync with a predicate instead of FindAsync for better performance with large datasets

if (entity != null)
{
var utcNow = DateTime.UtcNow;
entity.PublicKey = userAsymmetricKeys.PublicKey;
entity.PrivateKey = userAsymmetricKeys.UserKeyEncryptedPrivateKey;
entity.RevisionDate = utcNow;
entity.AccountRevisionDate = utcNow;
await dbContext.SaveChangesAsync();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add error handling around SaveChangesAsync

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
CREATE PROCEDURE [dbo].[UserAsymmetricKeys_Regenerate]
@UserId UNIQUEIDENTIFIER,
@PublicKey VARCHAR(MAX),
@PrivateKey VARCHAR(MAX)
AS
BEGIN
SET NOCOUNT ON
DECLARE @UtcNow DATETIME2(7) = GETUTCDATE();

UPDATE [dbo].[User]
SET [PublicKey] = @PublicKey,
[PrivateKey] = @PrivateKey,
[RevisionDate] = @UtcNow,
[AccountRevisionDate] = @UtcNow
WHERE [Id] = @UserId
Comment on lines +10 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add error handling to check if the update was successful

END
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
CREATE OR ALTER PROCEDURE [dbo].[UserAsymmetricKeys_Regenerate]
@UserId UNIQUEIDENTIFIER,
@PublicKey VARCHAR(MAX),
@PrivateKey VARCHAR(MAX)
Comment on lines +3 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using NVARCHAR(MAX) for @PublicKey and @privatekey to support Unicode characters

AS
BEGIN
SET NOCOUNT ON
DECLARE @UtcNow DATETIME2(7) = GETUTCDATE();

UPDATE [dbo].[User]
SET [PublicKey] = @PublicKey,
[PrivateKey] = @PrivateKey,
[RevisionDate] = @UtcNow,
[AccountRevisionDate] = @UtcNow
WHERE [Id] = @UserId
END
Loading