From 989603ddd361bac328becd49489649e1c7d9983b Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Tue, 5 Dec 2023 12:05:51 -0500 Subject: [PATCH] [Pm 3797 Part 2] Add emergency access rotations (#3434) ## Type of change ``` - [ ] Bug fix - [ ] New feature development - [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ``` ## Objective See #3425 for part 1 and background. This PR adds emergency access to the rotation. All new code is hidden behind a feature flag. The Accounts controller has also been moved to Auth ownership. ## Code changes * **file.ext:** Description of what was changed and why * **AccountsController.cs:** Moved to Auth ownership. Emergency access validation was added (as well as initializing empty lists to avoid errors). * **EmergencyAccessRotationValidator.cs:** Performs validation on the provided list of new emergency access keys. * **EmergencyAccessRepository.cs:** Adds a method to rotate encryption keys. This is added to a list in the `RotateUserKeyCommand` that the `UserRepository` calls so it doesn't have to know about all the domains. ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - If making database changes - make sure you also update Entity Framework queries and/or migrations - Please add **unit tests** where it makes sense to do so (encouraged but not required) - If this change requires a **documentation update** - notify the documentation team - If this change has particular **deployment requirements** - notify the DevOps team --- .../Controllers/AccountsController.cs | 27 ++-- .../Request/Accounts/UpdateKeyRequestModel.cs | 2 +- ...els.cs => EmergencyAccessRequestModels.cs} | 6 + .../EmergencyAccessRotationValidator.cs | 58 ++++++++ src/Api/Auth/Validators/IRotationValidator.cs | 15 ++ src/Api/Startup.cs | 6 + .../Auth/Models/Data/RotateUserKeyData.cs | 1 - .../IEmergencyAccessRepository.cs | 9 ++ .../UserKey/IRotateUserKeyCommand.cs | 12 +- .../Implementations/RotateUserKeyCommand.cs | 34 +++-- .../Auth/Helpers/EmergencyAccessHelpers.cs | 30 ++++ .../Repositories/EmergencyAccessRepository.cs | 56 ++++++++ src/Infrastructure.Dapper/DapperHelpers.cs | 3 +- .../Repositories/UserRepository.cs | 2 +- .../Repositories/EmergencyAccessRepository.cs | 28 ++++ .../Repositories/UserRepository.cs | 2 +- .../Controllers/AccountsControllerTests.cs | 15 +- .../EmergencyAccessRotationValidatorTests.cs | 136 ++++++++++++++++++ .../UserKey/RotateUserKeyCommandTests.cs | 20 +-- 19 files changed, 423 insertions(+), 39 deletions(-) rename src/Api/{ => Auth}/Controllers/AccountsController.cs (97%) rename src/Api/Auth/Models/Request/{EmergencyAccessRequstModels.cs => EmergencyAccessRequestModels.cs} (91%) create mode 100644 src/Api/Auth/Validators/EmergencyAccessRotationValidator.cs create mode 100644 src/Api/Auth/Validators/IRotationValidator.cs create mode 100644 src/Infrastructure.Dapper/Auth/Helpers/EmergencyAccessHelpers.cs rename test/Api.Test/{ => Auth}/Controllers/AccountsControllerTests.cs (97%) create mode 100644 test/Api.Test/Auth/Validators/EmergencyAccessRotationValidatorTests.cs diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs similarity index 97% rename from src/Api/Controllers/AccountsController.cs rename to src/Api/Auth/Controllers/AccountsController.cs index 8cffe3f396..bf9294709b 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -1,5 +1,7 @@ using Bit.Api.AdminConsole.Models.Response; +using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Validators; using Bit.Api.Models.Request; using Bit.Api.Models.Request.Accounts; using Bit.Api.Models.Response; @@ -8,6 +10,7 @@ using Bit.Core; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Models.Data; @@ -16,6 +19,7 @@ using Bit.Core.Auth.UserFeatures.UserKey; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Auth.Utilities; using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Api.Response; @@ -34,7 +38,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; -namespace Bit.Api.Controllers; +namespace Bit.Api.Auth.Controllers; [Route("accounts")] [Authorize("Application")] @@ -61,6 +65,10 @@ public class AccountsController : Controller private bool UseFlexibleCollections => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + private readonly IRotationValidator, IEnumerable> + _emergencyAccessValidator; + + public AccountsController( GlobalSettings globalSettings, ICipherRepository cipherRepository, @@ -78,7 +86,9 @@ public class AccountsController : Controller ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand, IRotateUserKeyCommand rotateUserKeyCommand, IFeatureService featureService, - ICurrentContext currentContext + ICurrentContext currentContext, + IRotationValidator, IEnumerable> + emergencyAccessValidator ) { _cipherRepository = cipherRepository; @@ -98,6 +108,7 @@ public class AccountsController : Controller _rotateUserKeyCommand = rotateUserKeyCommand; _featureService = featureService; _currentContext = currentContext; + _emergencyAccessValidator = emergencyAccessValidator; } #region DEPRECATED (Moved to Identity Service) @@ -403,14 +414,14 @@ public class AccountsController : Controller MasterPasswordHash = model.MasterPasswordHash, Key = model.Key, PrivateKey = model.PrivateKey, - // Ciphers = await _cipherValidator.ValidateAsync(user, model.Ciphers), - // Folders = await _folderValidator.ValidateAsync(user, model.Folders), - // Sends = await _sendValidator.ValidateAsync(user, model.Sends), - // EmergencyAccessKeys = await _emergencyAccessValidator.ValidateAsync(user, model.EmergencyAccessKeys), - // ResetPasswordKeys = await _accountRecoveryValidator.ValidateAsync(user, model.ResetPasswordKeys), + Ciphers = new List(), + Folders = new List(), + Sends = new List(), + EmergencyAccessKeys = await _emergencyAccessValidator.ValidateAsync(user, model.EmergencyAccessKeys), + ResetPasswordKeys = new List(), }; - result = await _rotateUserKeyCommand.RotateUserKeyAsync(dataModel); + result = await _rotateUserKeyCommand.RotateUserKeyAsync(user, dataModel); } else { diff --git a/src/Api/Auth/Models/Request/Accounts/UpdateKeyRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/UpdateKeyRequestModel.cs index 3a9e6e2fc3..a34ce6fd28 100644 --- a/src/Api/Auth/Models/Request/Accounts/UpdateKeyRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/UpdateKeyRequestModel.cs @@ -17,7 +17,7 @@ public class UpdateKeyRequestModel public IEnumerable Ciphers { get; set; } public IEnumerable Folders { get; set; } public IEnumerable Sends { get; set; } - public IEnumerable EmergencyAccessKeys { get; set; } + public IEnumerable EmergencyAccessKeys { get; set; } public IEnumerable ResetPasswordKeys { get; set; } } diff --git a/src/Api/Auth/Models/Request/EmergencyAccessRequstModels.cs b/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs similarity index 91% rename from src/Api/Auth/Models/Request/EmergencyAccessRequstModels.cs rename to src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs index b6862151a5..8b1d5e883b 100644 --- a/src/Api/Auth/Models/Request/EmergencyAccessRequstModels.cs +++ b/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs @@ -46,3 +46,9 @@ public class EmergencyAccessPasswordRequestModel [Required] public string Key { get; set; } } + +public class EmergencyAccessWithIdRequestModel : EmergencyAccessUpdateRequestModel +{ + [Required] + public Guid Id { get; set; } +} diff --git a/src/Api/Auth/Validators/EmergencyAccessRotationValidator.cs b/src/Api/Auth/Validators/EmergencyAccessRotationValidator.cs new file mode 100644 index 0000000000..dd75f6a761 --- /dev/null +++ b/src/Api/Auth/Validators/EmergencyAccessRotationValidator.cs @@ -0,0 +1,58 @@ +using Bit.Api.Auth.Models.Request; +using Bit.Core.Auth.Entities; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; + +namespace Bit.Api.Auth.Validators; + +public class EmergencyAccessRotationValidator : IRotationValidator, + IEnumerable> +{ + private readonly IEmergencyAccessRepository _emergencyAccessRepository; + private readonly IUserService _userService; + + public EmergencyAccessRotationValidator(IEmergencyAccessRepository emergencyAccessRepository, + IUserService userService) + { + _emergencyAccessRepository = emergencyAccessRepository; + _userService = userService; + } + + public async Task> ValidateAsync(User user, + IEnumerable emergencyAccessKeys) + { + var result = new List(); + if (emergencyAccessKeys == null || !emergencyAccessKeys.Any()) + { + return result; + } + + var existing = await _emergencyAccessRepository.GetManyDetailsByGrantorIdAsync(user.Id); + if (existing == null || !existing.Any()) + { + return result; + } + // Exclude any emergency access that has not been confirmed yet. + existing = existing.Where(ea => ea.KeyEncrypted != null).ToList(); + + foreach (var ea in existing) + { + var emergencyAccess = emergencyAccessKeys.FirstOrDefault(c => c.Id == ea.Id); + if (emergencyAccess == null) + { + throw new BadRequestException("All existing emergency access keys must be included in the rotation."); + } + + if (emergencyAccess.KeyEncrypted == null) + { + throw new BadRequestException("Emergency access keys cannot be set to null during rotation."); + } + + result.Add(emergencyAccess.ToEmergencyAccess(ea)); + } + + return result; + } +} diff --git a/src/Api/Auth/Validators/IRotationValidator.cs b/src/Api/Auth/Validators/IRotationValidator.cs new file mode 100644 index 0000000000..7360a4570c --- /dev/null +++ b/src/Api/Auth/Validators/IRotationValidator.cs @@ -0,0 +1,15 @@ +using Bit.Core.Entities; + +namespace Bit.Api.Auth.Validators; + +/// +/// A consistent interface for domains to validate re-encrypted data before saved to database. Some examples are:
+/// - All available encrypted data is accounted for
+/// - All provided encrypted data belongs to the user +/// +/// Request model +/// Domain model +public interface IRotationValidator +{ + Task ValidateAsync(User user, T data); +} diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index fce3ef4756..20301bdb27 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -7,6 +7,9 @@ using Stripe; using Bit.Core.Utilities; using IdentityModel; using System.Globalization; +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Validators; +using Bit.Core.Auth.Entities; using Bit.Core.IdentityServer; using Bit.SharedWeb.Health; using Microsoft.IdentityModel.Logging; @@ -135,6 +138,9 @@ public class Startup // Key Rotation services.AddScoped(); + services + .AddScoped, IEnumerable>, + EmergencyAccessRotationValidator>(); // Services services.AddBaseServices(globalSettings); diff --git a/src/Core/Auth/Models/Data/RotateUserKeyData.cs b/src/Core/Auth/Models/Data/RotateUserKeyData.cs index ecd1bd6ddb..23b6beb710 100644 --- a/src/Core/Auth/Models/Data/RotateUserKeyData.cs +++ b/src/Core/Auth/Models/Data/RotateUserKeyData.cs @@ -7,7 +7,6 @@ namespace Bit.Core.Auth.Models.Data; public class RotateUserKeyData { - public User User { get; set; } public string MasterPasswordHash { get; set; } public string Key { get; set; } public string PrivateKey { get; set; } diff --git a/src/Core/Auth/Repositories/IEmergencyAccessRepository.cs b/src/Core/Auth/Repositories/IEmergencyAccessRepository.cs index 64e7cd872c..e22e934882 100644 --- a/src/Core/Auth/Repositories/IEmergencyAccessRepository.cs +++ b/src/Core/Auth/Repositories/IEmergencyAccessRepository.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.UserFeatures.UserKey; namespace Bit.Core.Repositories; @@ -11,4 +12,12 @@ public interface IEmergencyAccessRepository : IRepository Task GetDetailsByIdGrantorIdAsync(Guid id, Guid grantorId); Task> GetManyToNotifyAsync(); Task> GetExpiredRecoveriesAsync(); + + /// + /// Updates encrypted data for emergency access during a key rotation + /// + /// The grantor that initiated the key rotation + /// A list of emergency access with updated keys + UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid grantorId, + IEnumerable emergencyAccessKeys); } diff --git a/src/Core/Auth/UserFeatures/UserKey/IRotateUserKeyCommand.cs b/src/Core/Auth/UserFeatures/UserKey/IRotateUserKeyCommand.cs index 28632758fd..4ba59ca487 100644 --- a/src/Core/Auth/UserFeatures/UserKey/IRotateUserKeyCommand.cs +++ b/src/Core/Auth/UserFeatures/UserKey/IRotateUserKeyCommand.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; using Microsoft.AspNetCore.Identity; using Microsoft.Data.SqlClient; @@ -12,7 +13,14 @@ public interface IRotateUserKeyCommand /// All necessary information for rotation. Warning: Any encrypted data not included will be lost. /// An IdentityResult for verification of the master password hash /// User must be provided. - Task RotateUserKeyAsync(RotateUserKeyData model); + Task RotateUserKeyAsync(User user, RotateUserKeyData model); } -public delegate Task UpdateEncryptedDataForKeyRotation(SqlTransaction transaction = null); +/// +/// A type used to implement updates to the database for key rotations. Each domain that requires an update of encrypted +/// data during a key rotation should use this to implement its own database call. The user repository loops through +/// these during a key rotation. +/// Note: connection and transaction are only used for Dapper. They won't be available in EF +/// +public delegate Task UpdateEncryptedDataForKeyRotation(SqlConnection connection = null, + SqlTransaction transaction = null); diff --git a/src/Core/Auth/UserFeatures/UserKey/Implementations/RotateUserKeyCommand.cs b/src/Core/Auth/UserFeatures/UserKey/Implementations/RotateUserKeyCommand.cs index 5eb57d62ee..f1ac72b179 100644 --- a/src/Core/Auth/UserFeatures/UserKey/Implementations/RotateUserKeyCommand.cs +++ b/src/Core/Auth/UserFeatures/UserKey/Implementations/RotateUserKeyCommand.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Identity; @@ -9,37 +10,40 @@ public class RotateUserKeyCommand : IRotateUserKeyCommand { private readonly IUserService _userService; private readonly IUserRepository _userRepository; + private readonly IEmergencyAccessRepository _emergencyAccessRepository; private readonly IPushNotificationService _pushService; private readonly IdentityErrorDescriber _identityErrorDescriber; public RotateUserKeyCommand(IUserService userService, IUserRepository userRepository, + IEmergencyAccessRepository emergencyAccessRepository, IPushNotificationService pushService, IdentityErrorDescriber errors) { _userService = userService; _userRepository = userRepository; + _emergencyAccessRepository = emergencyAccessRepository; _pushService = pushService; _identityErrorDescriber = errors; } /// - public async Task RotateUserKeyAsync(RotateUserKeyData model) + public async Task RotateUserKeyAsync(User user, RotateUserKeyData model) { - if (model.User == null) + if (user == null) { - throw new ArgumentNullException(nameof(model.User)); + throw new ArgumentNullException(nameof(user)); } - if (!await _userService.CheckPasswordAsync(model.User, model.MasterPasswordHash)) + if (!await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); } var now = DateTime.UtcNow; - model.User.RevisionDate = model.User.AccountRevisionDate = now; - model.User.LastKeyRotationDate = now; - model.User.SecurityStamp = Guid.NewGuid().ToString(); - model.User.Key = model.Key; - model.User.PrivateKey = model.PrivateKey; + user.RevisionDate = user.AccountRevisionDate = now; + user.LastKeyRotationDate = now; + user.SecurityStamp = Guid.NewGuid().ToString(); + user.Key = model.Key; + user.PrivateKey = model.PrivateKey; if (model.Ciphers.Any() || model.Folders.Any() || model.Sends.Any() || model.EmergencyAccessKeys.Any() || model.ResetPasswordKeys.Any()) { @@ -48,14 +52,20 @@ public class RotateUserKeyCommand : IRotateUserKeyCommand // { // saveEncryptedDataActions.Add(_cipherRepository.SaveRotatedData); // } - await _userRepository.UpdateUserKeyAndEncryptedDataAsync(model.User, saveEncryptedDataActions); + if (model.EmergencyAccessKeys.Any()) + { + saveEncryptedDataActions.Add( + _emergencyAccessRepository.UpdateForKeyRotation(user.Id, model.EmergencyAccessKeys)); + } + + await _userRepository.UpdateUserKeyAndEncryptedDataAsync(user, saveEncryptedDataActions); } else { - await _userRepository.ReplaceAsync(model.User); + await _userRepository.ReplaceAsync(user); } - await _pushService.PushLogOutAsync(model.User.Id, excludeCurrentContextFromPush: true); + await _pushService.PushLogOutAsync(user.Id, excludeCurrentContextFromPush: true); return IdentityResult.Success; } } diff --git a/src/Infrastructure.Dapper/Auth/Helpers/EmergencyAccessHelpers.cs b/src/Infrastructure.Dapper/Auth/Helpers/EmergencyAccessHelpers.cs new file mode 100644 index 0000000000..34e4fcda69 --- /dev/null +++ b/src/Infrastructure.Dapper/Auth/Helpers/EmergencyAccessHelpers.cs @@ -0,0 +1,30 @@ +using System.Data; +using Bit.Core.Auth.Entities; + +namespace Bit.Infrastructure.Dapper.Auth.Helpers; + +public static class EmergencyAccessHelpers +{ + public static DataTable ToDataTable(this IEnumerable emergencyAccesses) + { + var emergencyAccessTable = new DataTable(); + + var columnData = new List<(string name, Type type, Func getter)> + { + (nameof(EmergencyAccess.Id), typeof(Guid), c => c.Id), + (nameof(EmergencyAccess.GrantorId), typeof(Guid), c => c.GrantorId), + (nameof(EmergencyAccess.GranteeId), typeof(Guid), c => c.GranteeId), + (nameof(EmergencyAccess.Email), typeof(string), c => c.Email), + (nameof(EmergencyAccess.KeyEncrypted), typeof(string), c => c.KeyEncrypted), + (nameof(EmergencyAccess.WaitTimeDays), typeof(int), c => c.WaitTimeDays), + (nameof(EmergencyAccess.Type), typeof(short), c => c.Type), + (nameof(EmergencyAccess.Status), typeof(short), c => c.Status), + (nameof(EmergencyAccess.RecoveryInitiatedDate), typeof(DateTime), c => c.RecoveryInitiatedDate), + (nameof(EmergencyAccess.LastNotificationDate), typeof(DateTime), c => c.LastNotificationDate), + (nameof(EmergencyAccess.CreationDate), typeof(DateTime), c => c.CreationDate), + (nameof(EmergencyAccess.RevisionDate), typeof(DateTime), c => c.RevisionDate), + }; + + return emergencyAccesses.BuildTable(emergencyAccessTable, columnData); + } +} diff --git a/src/Infrastructure.Dapper/Auth/Repositories/EmergencyAccessRepository.cs b/src/Infrastructure.Dapper/Auth/Repositories/EmergencyAccessRepository.cs index 00c9cf67ff..195ebfadae 100644 --- a/src/Infrastructure.Dapper/Auth/Repositories/EmergencyAccessRepository.cs +++ b/src/Infrastructure.Dapper/Auth/Repositories/EmergencyAccessRepository.cs @@ -1,8 +1,10 @@ using System.Data; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.UserFeatures.UserKey; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Infrastructure.Dapper.Auth.Helpers; using Bit.Infrastructure.Dapper.Repositories; using Dapper; using Microsoft.Data.SqlClient; @@ -94,4 +96,58 @@ public class EmergencyAccessRepository : Repository, IEme return results.ToList(); } } + + /// + public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation( + Guid grantorId, IEnumerable emergencyAccessKeys) + { + return async (SqlConnection connection, SqlTransaction transaction) => + { + // Create temp table + var sqlCreateTemp = @" + SELECT TOP 0 * + INTO #TempEmergencyAccess + FROM [dbo].[EmergencyAccess]"; + + await using (var cmd = new SqlCommand(sqlCreateTemp, connection, transaction)) + { + cmd.ExecuteNonQuery(); + } + + // Bulk copy data into temp table + using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.KeepIdentity, transaction)) + { + bulkCopy.DestinationTableName = "#TempEmergencyAccess"; + var emergencyAccessTable = emergencyAccessKeys.ToDataTable(); + foreach (DataColumn col in emergencyAccessTable.Columns) + { + bulkCopy.ColumnMappings.Add(col.ColumnName, col.ColumnName); + } + + emergencyAccessTable.PrimaryKey = new DataColumn[] { emergencyAccessTable.Columns[0] }; + await bulkCopy.WriteToServerAsync(emergencyAccessTable); + } + + // Update emergency access table from temp table + var sql = @" + UPDATE + [dbo].[EmergencyAccess] + SET + [KeyEncrypted] = TE.[KeyEncrypted] + FROM + [dbo].[EmergencyAccess] E + INNER JOIN + #TempEmergencyAccess TE ON E.Id = TE.Id + WHERE + E.[GrantorId] = @GrantorId + + DROP TABLE #TempEmergencyAccess"; + + await using (var cmd = new SqlCommand(sql, connection, transaction)) + { + cmd.Parameters.Add("@GrantorId", SqlDbType.UniqueIdentifier).Value = grantorId; + cmd.ExecuteNonQuery(); + } + }; + } } diff --git a/src/Infrastructure.Dapper/DapperHelpers.cs b/src/Infrastructure.Dapper/DapperHelpers.cs index b5e4dc6e5d..4156c1691b 100644 --- a/src/Infrastructure.Dapper/DapperHelpers.cs +++ b/src/Infrastructure.Dapper/DapperHelpers.cs @@ -107,7 +107,8 @@ public static class DapperHelpers return organizationSponsorships.BuildTable(table, columnData); } - private static DataTable BuildTable(this IEnumerable entities, DataTable table, List<(string name, Type type, Func getter)> columnData) + public static DataTable BuildTable(this IEnumerable entities, DataTable table, + List<(string name, Type type, Func getter)> columnData) { foreach (var (name, type, getter) in columnData) { diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index 675b8e6b82..e827d261f9 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -209,7 +209,7 @@ public class UserRepository : Repository, IUserRepository // Update re-encrypted data foreach (var action in updateDataActions) { - await action(transaction); + await action(connection, transaction); } transaction.Commit(); diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/EmergencyAccessRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/EmergencyAccessRepository.cs index f00ff2eecf..e6a32542fb 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/EmergencyAccessRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/EmergencyAccessRepository.cs @@ -1,10 +1,12 @@ using AutoMapper; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.UserFeatures.UserKey; using Bit.Core.Repositories; using Bit.Infrastructure.EntityFramework.Auth.Models; using Bit.Infrastructure.EntityFramework.Auth.Repositories.Queries; using Bit.Infrastructure.EntityFramework.Repositories; +using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -116,4 +118,30 @@ public class EmergencyAccessRepository : Repository + public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation( + Guid grantorId, IEnumerable emergencyAccessKeys) + { + return async (SqlConnection connection, SqlTransaction transaction) => + { + var newKeys = emergencyAccessKeys.ToList(); + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + var userEmergencyAccess = await GetDbSet(dbContext) + .Where(ea => ea.GrantorId == grantorId) + .ToListAsync(); + var validEmergencyAccess = userEmergencyAccess + .Where(ea => newKeys.Any(eak => eak.Id == ea.Id)); + + foreach (var ea in validEmergencyAccess) + { + var eak = newKeys.First(eak => eak.Id == ea.Id); + ea.KeyEncrypted = eak.KeyEncrypted; + } + + await dbContext.SaveChangesAsync(); + }; + } + } diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index d91cdcd90f..8ccf6dab2c 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -164,7 +164,7 @@ public class UserRepository : Repository, IUserR // Update re-encrypted data foreach (var action in updateDataActions) { - // TODO (jlf0dev): Check if transaction captures these operations + // connection and transaction aren't used in EF await action(); } diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs similarity index 97% rename from test/Api.Test/Controllers/AccountsControllerTests.cs rename to test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index c0c50345a2..99d3ac77ba 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -1,9 +1,12 @@ using System.Security.Claims; +using Bit.Api.Auth.Controllers; +using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.Accounts; -using Bit.Api.Controllers; +using Bit.Api.Auth.Validators; using Bit.Core; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.UserKey; @@ -24,7 +27,7 @@ using Microsoft.AspNetCore.Identity; using NSubstitute; using Xunit; -namespace Bit.Api.Test.Controllers; +namespace Bit.Api.Test.Auth.Controllers; public class AccountsControllerTests : IDisposable { @@ -48,6 +51,9 @@ public class AccountsControllerTests : IDisposable private readonly IFeatureService _featureService; private readonly ICurrentContext _currentContext; + private readonly IRotationValidator, IEnumerable> + _emergencyAccessValidator; + public AccountsControllerTests() { @@ -68,6 +74,8 @@ public class AccountsControllerTests : IDisposable _rotateUserKeyCommand = Substitute.For(); _featureService = Substitute.For(); _currentContext = Substitute.For(); + _emergencyAccessValidator = Substitute.For, + IEnumerable>>(); _sut = new AccountsController( _globalSettings, @@ -86,7 +94,8 @@ public class AccountsControllerTests : IDisposable _setInitialMasterPasswordCommand, _rotateUserKeyCommand, _featureService, - _currentContext + _currentContext, + _emergencyAccessValidator ); } diff --git a/test/Api.Test/Auth/Validators/EmergencyAccessRotationValidatorTests.cs b/test/Api.Test/Auth/Validators/EmergencyAccessRotationValidatorTests.cs new file mode 100644 index 0000000000..7e9cf6a6b3 --- /dev/null +++ b/test/Api.Test/Auth/Validators/EmergencyAccessRotationValidatorTests.cs @@ -0,0 +1,136 @@ +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Validators; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.Auth.Validators; + +[SutProviderCustomize] +public class EmergencyAccessRotationValidatorTests +{ + [Theory] + [BitAutoData] + public async Task ValidateAsync_MissingEmergencyAccess_Throws( + SutProvider sutProvider, User user, + IEnumerable emergencyAccessKeys) + { + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + var userEmergencyAccess = emergencyAccessKeys.Select(e => new EmergencyAccessDetails + { + Id = e.Id, + GrantorName = user.Name, + GrantorEmail = user.Email, + KeyEncrypted = e.KeyEncrypted, + Type = e.Type + }).ToList(); + userEmergencyAccess.Add(new EmergencyAccessDetails { Id = Guid.NewGuid(), KeyEncrypted = "TestKey" }); + sutProvider.GetDependency().GetManyDetailsByGrantorIdAsync(user.Id) + .Returns(userEmergencyAccess); + + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ValidateAsync(user, emergencyAccessKeys)); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_EmergencyAccessDoesNotBelongToUser_NotIncluded( + SutProvider sutProvider, User user, + IEnumerable emergencyAccessKeys) + { + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + var userEmergencyAccess = emergencyAccessKeys.Select(e => new EmergencyAccessDetails + { + Id = e.Id, + GrantorName = user.Name, + GrantorEmail = user.Email, + KeyEncrypted = e.KeyEncrypted, + Type = e.Type + }).ToList(); + userEmergencyAccess.RemoveAt(0); + sutProvider.GetDependency().GetManyDetailsByGrantorIdAsync(user.Id) + .Returns(userEmergencyAccess); + + var result = await sutProvider.Sut.ValidateAsync(user, emergencyAccessKeys); + + Assert.DoesNotContain(result, c => c.Id == emergencyAccessKeys.First().Id); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_UserNotPremium_Success( + SutProvider sutProvider, User user, + IEnumerable emergencyAccessKeys) + { + // We want to allow users who have lost premium to rotate their key for any existing emergency access, as long + // as we restrict it to existing records and don't let them alter data + user.Premium = false; + var userEmergencyAccess = emergencyAccessKeys.Select(e => new EmergencyAccessDetails + { + Id = e.Id, + GrantorName = user.Name, + GrantorEmail = user.Email, + KeyEncrypted = e.KeyEncrypted, + Type = e.Type + }).ToList(); + sutProvider.GetDependency().GetManyDetailsByGrantorIdAsync(user.Id) + .Returns(userEmergencyAccess); + + var result = await sutProvider.Sut.ValidateAsync(user, emergencyAccessKeys); + + Assert.Equal(userEmergencyAccess, result); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_NonConfirmedEmergencyAccess_NotReturned( + SutProvider sutProvider, User user, + IEnumerable emergencyAccessKeys) + { + emergencyAccessKeys.First().KeyEncrypted = null; + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + var userEmergencyAccess = emergencyAccessKeys.Select(e => new EmergencyAccessDetails + { + Id = e.Id, + GrantorName = user.Name, + GrantorEmail = user.Email, + KeyEncrypted = e.KeyEncrypted, + Type = e.Type + }).ToList(); + sutProvider.GetDependency().GetManyDetailsByGrantorIdAsync(user.Id) + .Returns(userEmergencyAccess); + + var result = await sutProvider.Sut.ValidateAsync(user, emergencyAccessKeys); + + Assert.DoesNotContain(result, c => c.Id == emergencyAccessKeys.First().Id); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_AttemptToSetKeyToNull_Throws( + SutProvider sutProvider, User user, + IEnumerable emergencyAccessKeys) + { + sutProvider.GetDependency().CanAccessPremium(user).Returns(true); + var userEmergencyAccess = emergencyAccessKeys.Select(e => new EmergencyAccessDetails + { + Id = e.Id, + GrantorName = user.Name, + GrantorEmail = user.Email, + KeyEncrypted = e.KeyEncrypted, + Type = e.Type + }).ToList(); + sutProvider.GetDependency().GetManyDetailsByGrantorIdAsync(user.Id) + .Returns(userEmergencyAccess); + emergencyAccessKeys.First().KeyEncrypted = null; + + await Assert.ThrowsAsync(async () => + await sutProvider.Sut.ValidateAsync(user, emergencyAccessKeys)); + } +} diff --git a/test/Core.Test/Auth/UserFeatures/UserKey/RotateUserKeyCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserKey/RotateUserKeyCommandTests.cs index 4d664a862c..d95d957915 100644 --- a/test/Core.Test/Auth/UserFeatures/UserKey/RotateUserKeyCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserKey/RotateUserKeyCommandTests.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.UserKey.Implementations; +using Bit.Core.Entities; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -13,36 +14,37 @@ namespace Bit.Core.Test.Auth.UserFeatures.UserKey; public class RotateUserKeyCommandTests { [Theory, BitAutoData] - public async Task RotateUserKeyAsync_Success(SutProvider sutProvider, RotateUserKeyData model) + public async Task RotateUserKeyAsync_Success(SutProvider sutProvider, User user, + RotateUserKeyData model) { - sutProvider.GetDependency().CheckPasswordAsync(model.User, model.MasterPasswordHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.MasterPasswordHash) .Returns(true); - var result = await sutProvider.Sut.RotateUserKeyAsync(model); + var result = await sutProvider.Sut.RotateUserKeyAsync(user, model); Assert.Equal(IdentityResult.Success, result); } [Theory, BitAutoData] public async Task RotateUserKeyAsync_InvalidMasterPasswordHash_ReturnsFailedIdentityResult( - SutProvider sutProvider, RotateUserKeyData model) + SutProvider sutProvider, User user, RotateUserKeyData model) { - sutProvider.GetDependency().CheckPasswordAsync(model.User, model.MasterPasswordHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.MasterPasswordHash) .Returns(false); - var result = await sutProvider.Sut.RotateUserKeyAsync(model); + var result = await sutProvider.Sut.RotateUserKeyAsync(user, model); Assert.False(result.Succeeded); } [Theory, BitAutoData] public async Task RotateUserKeyAsync_LogsOutUser( - SutProvider sutProvider, RotateUserKeyData model) + SutProvider sutProvider, User user, RotateUserKeyData model) { - sutProvider.GetDependency().CheckPasswordAsync(model.User, model.MasterPasswordHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.MasterPasswordHash) .Returns(true); - await sutProvider.Sut.RotateUserKeyAsync(model); + await sutProvider.Sut.RotateUserKeyAsync(user, model); await sutProvider.GetDependency().ReceivedWithAnyArgs() .PushLogOutAsync(default, default);