1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-24 12:35:25 +01:00

[PM-5963] Fix tde offboarding vault corruption (#4144)

* Attempt to fix tde to mp flow

* Move tde offboarding to dedicated flag

* Add tde offboarding password request

* Validate tde offboarding input

* Correctly check whether tde is active when building trusted device options

* Refactor Tde offboarding into a separate command

* Add unit tests for tde offboarding

* Update tde offboarding request model

* Fix tests

* Fix further tests

* Fix documentation

* Add validation for updatetdepasswordasync key/newmasterpassword

* Add comment explaining test

* Remove unrelated changes
This commit is contained in:
Bernd Schoolmann 2024-07-23 20:53:08 +02:00 committed by GitHub
parent 48f9d09f4e
commit ce185eb3df
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 283 additions and 2 deletions

View File

@ -17,6 +17,7 @@ using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Models.Api.Request.Accounts;
using Bit.Core.Auth.Models.Data;
using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces;
using Bit.Core.Auth.UserFeatures.UserKey;
using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces;
using Bit.Core.Billing.Models;
@ -53,6 +54,7 @@ public class AccountsController : Controller
private readonly IUserService _userService;
private readonly IPolicyService _policyService;
private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand;
private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand;
private readonly IRotateUserKeyCommand _rotateUserKeyCommand;
private readonly IFeatureService _featureService;
private readonly ISubscriberService _subscriberService;
@ -83,6 +85,7 @@ public class AccountsController : Controller
IUserService userService,
IPolicyService policyService,
ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand,
ITdeOffboardingPasswordCommand tdeOffboardingPasswordCommand,
IRotateUserKeyCommand rotateUserKeyCommand,
IFeatureService featureService,
ISubscriberService subscriberService,
@ -106,6 +109,7 @@ public class AccountsController : Controller
_userService = userService;
_policyService = policyService;
_setInitialMasterPasswordCommand = setInitialMasterPasswordCommand;
_tdeOffboardingPasswordCommand = tdeOffboardingPasswordCommand;
_rotateUserKeyCommand = rotateUserKeyCommand;
_featureService = featureService;
_subscriberService = subscriberService;
@ -877,6 +881,29 @@ public class AccountsController : Controller
throw new BadRequestException(ModelState);
}
[HttpPut("update-tde-offboarding-password")]
public async Task PutUpdateTdePasswordAsync([FromBody] UpdateTdeOffboardingPasswordRequestModel model)
{
var user = await _userService.GetUserByPrincipalAsync(User);
if (user == null)
{
throw new UnauthorizedAccessException();
}
var result = await _tdeOffboardingPasswordCommand.UpdateTdeOffboardingPasswordAsync(user, model.NewMasterPasswordHash, model.Key, model.MasterPasswordHint);
if (result.Succeeded)
{
return;
}
foreach (var error in result.Errors)
{
ModelState.AddModelError(string.Empty, error.Description);
}
throw new BadRequestException(ModelState);
}
[HttpPost("request-otp")]
public async Task PostRequestOTP()
{

View File

@ -0,0 +1,14 @@
using System.ComponentModel.DataAnnotations;
namespace Bit.Api.Auth.Models.Request.Accounts;
public class UpdateTdeOffboardingPasswordRequestModel
{
[Required]
[StringLength(300)]
public string NewMasterPasswordHash { get; set; }
[Required]
public string Key { get; set; }
[StringLength(50)]
public string MasterPasswordHint { get; set; }
}

View File

@ -54,18 +54,21 @@ public class TrustedDeviceUserDecryptionOption
public bool HasAdminApproval { get; }
public bool HasLoginApprovingDevice { get; }
public bool HasManageResetPasswordPermission { get; }
public bool IsTdeOffboarding { get; }
public string? EncryptedPrivateKey { get; }
public string? EncryptedUserKey { get; }
public TrustedDeviceUserDecryptionOption(bool hasAdminApproval,
bool hasLoginApprovingDevice,
bool hasManageResetPasswordPermission,
bool isTdeOffboarding,
string? encryptedPrivateKey,
string? encryptedUserKey)
{
HasAdminApproval = hasAdminApproval;
HasLoginApprovingDevice = hasLoginApprovingDevice;
HasManageResetPasswordPermission = hasManageResetPasswordPermission;
IsTdeOffboarding = isTdeOffboarding;
EncryptedPrivateKey = encryptedPrivateKey;
EncryptedUserKey = encryptedUserKey;
}

View File

@ -0,0 +1,14 @@
using Bit.Core.Entities;
using Microsoft.AspNetCore.Identity;
namespace Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces;
/// <summary>
/// <para>Manages the setting of the master password for JIT provisioned TDE <see cref="User"/> in an organization, after the organization disabled TDE.</para>
/// <para>This command is invoked, when the user first logs in after the organization has switched from TDE to master password based decryption.</para>
/// </summary>
public interface ITdeOffboardingPasswordCommand
{
public Task<IdentityResult> UpdateTdeOffboardingPasswordAsync(User user, string masterPassword, string key,
string orgSsoIdentifier);
}

View File

@ -0,0 +1,99 @@
using Bit.Core.Auth.Repositories;
using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Microsoft.AspNetCore.Identity;
namespace Bit.Core.Auth.UserFeatures.UserMasterPassword;
public class TdeOffboardingPasswordCommand : ITdeOffboardingPasswordCommand
{
private readonly IUserService _userService;
private readonly IUserRepository _userRepository;
private readonly IEventService _eventService;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly ISsoUserRepository _ssoUserRepository;
private readonly ISsoConfigRepository _ssoConfigRepository;
private readonly IPushNotificationService _pushService;
public TdeOffboardingPasswordCommand(
IUserService userService,
IUserRepository userRepository,
IEventService eventService,
IOrganizationUserRepository organizationUserRepository,
ISsoUserRepository ssoUserRepository,
ISsoConfigRepository ssoConfigRepository,
IPushNotificationService pushService)
{
_userService = userService;
_userRepository = userRepository;
_eventService = eventService;
_organizationUserRepository = organizationUserRepository;
_ssoUserRepository = ssoUserRepository;
_ssoConfigRepository = ssoConfigRepository;
_pushService = pushService;
}
public async Task<IdentityResult> UpdateTdeOffboardingPasswordAsync(User user, string newMasterPassword, string key, string hint)
{
if (string.IsNullOrWhiteSpace(newMasterPassword))
{
throw new BadRequestException("Master password is required.");
}
if (string.IsNullOrWhiteSpace(key))
{
throw new BadRequestException("Key is required.");
}
if (user.HasMasterPassword())
{
throw new BadRequestException("User already has a master password.");
}
var orgUserDetails = await _organizationUserRepository.GetManyDetailsByUserAsync(user.Id);
orgUserDetails = orgUserDetails.Where(x => x.UseSso).ToList();
if (orgUserDetails.Count == 0)
{
throw new BadRequestException("User is not part of any organization that has SSO enabled.");
}
var orgSSOUsers = await Task.WhenAll(orgUserDetails.Select(async x => await _ssoUserRepository.GetByUserIdOrganizationIdAsync(x.OrganizationId, user.Id)));
if (orgSSOUsers.Length != 1)
{
throw new BadRequestException("User is part of no or multiple SSO configurations.");
}
var orgUser = orgUserDetails.First();
var orgSSOConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(orgUser.OrganizationId);
if (orgSSOConfig == null)
{
throw new BadRequestException("Organization SSO configuration not found.");
}
else if (orgSSOConfig.GetData().MemberDecryptionType != Enums.MemberDecryptionType.MasterPassword)
{
throw new BadRequestException("Organization SSO Member Decryption Type is not Master Password.");
}
var result = await _userService.UpdatePasswordHash(user, newMasterPassword);
if (!result.Succeeded)
{
return result;
}
user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow;
user.ForcePasswordReset = false;
user.Key = key;
user.MasterPasswordHint = hint;
await _userRepository.ReplaceAsync(user);
await _eventService.LogUserEventAsync(user.Id, EventType.User_UpdatedTempPassword);
await _pushService.PushLogOutAsync(user.Id);
return IdentityResult.Success;
}
}

View File

@ -2,6 +2,7 @@
using Bit.Core.Auth.UserFeatures.Registration;
using Bit.Core.Auth.UserFeatures.Registration.Implementations;
using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces;
using Bit.Core.Auth.UserFeatures.UserKey;
using Bit.Core.Auth.UserFeatures.UserKey.Implementations;
using Bit.Core.Auth.UserFeatures.UserMasterPassword;
@ -22,6 +23,7 @@ public static class UserServiceCollectionExtensions
services.AddUserPasswordCommands();
services.AddUserRegistrationCommands();
services.AddWebAuthnLoginCommands();
services.AddTdeOffboardingPasswordCommands();
}
public static void AddUserKeyCommands(this IServiceCollection services, IGlobalSettings globalSettings)
@ -34,6 +36,11 @@ public static class UserServiceCollectionExtensions
services.AddScoped<ISetInitialMasterPasswordCommand, SetInitialMasterPasswordCommand>();
}
private static void AddTdeOffboardingPasswordCommands(this IServiceCollection services)
{
services.AddScoped<ITdeOffboardingPasswordCommand, TdeOffboardingPasswordCommand>();
}
private static void AddUserRegistrationCommands(this IServiceCollection services)
{
services.AddScoped<ISendVerificationEmailForRegistrationCommand, SendVerificationEmailForRegistrationCommand>();

View File

@ -14,6 +14,7 @@ public enum EventType : int
User_UpdatedTempPassword = 1008,
User_MigratedKeyToKeyConnector = 1009,
User_RequestedDeviceApproval = 1010,
User_TdeOffboardingPasswordSet = 1011,
Cipher_Created = 1100,
Cipher_Updated = 1101,

View File

@ -95,8 +95,9 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder
return;
}
var ssoConfigurationData = _ssoConfig.GetData();
if (ssoConfigurationData is not { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption })
var isTdeActive = _ssoConfig.GetData() is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption };
var isTdeOffboarding = _user != null && !_user.HasMasterPassword() && _device != null && _device.IsTrusted() && !isTdeActive;
if (!isTdeActive && !isTdeOffboarding)
{
return;
}
@ -144,6 +145,7 @@ public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder
hasAdminApproval,
hasLoginApprovingDevice,
hasManageResetPasswordPermission,
isTdeOffboarding,
encryptedPrivateKey,
encryptedUserKey);
}

View File

@ -12,6 +12,7 @@ using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Models.Api.Request.Accounts;
using Bit.Core.Auth.Models.Data;
using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces;
using Bit.Core.Auth.UserFeatures.UserKey;
using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces;
using Bit.Core.Billing.Services;
@ -44,6 +45,7 @@ public class AccountsControllerTests : IDisposable
private readonly IPolicyService _policyService;
private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand;
private readonly IRotateUserKeyCommand _rotateUserKeyCommand;
private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand;
private readonly IFeatureService _featureService;
private readonly ISubscriberService _subscriberService;
private readonly IReferenceEventService _referenceEventService;
@ -72,6 +74,7 @@ public class AccountsControllerTests : IDisposable
_policyService = Substitute.For<IPolicyService>();
_setInitialMasterPasswordCommand = Substitute.For<ISetInitialMasterPasswordCommand>();
_rotateUserKeyCommand = Substitute.For<IRotateUserKeyCommand>();
_tdeOffboardingPasswordCommand = Substitute.For<ITdeOffboardingPasswordCommand>();
_featureService = Substitute.For<IFeatureService>();
_subscriberService = Substitute.For<ISubscriberService>();
_referenceEventService = Substitute.For<IReferenceEventService>();
@ -97,6 +100,7 @@ public class AccountsControllerTests : IDisposable
_userService,
_policyService,
_setInitialMasterPasswordCommand,
_tdeOffboardingPasswordCommand,
_rotateUserKeyCommand,
_featureService,
_subscriberService,

View File

@ -0,0 +1,99 @@
using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.Repositories;
using Bit.Core.Auth.UserFeatures.UserMasterPassword;
using Bit.Core.Entities;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using Microsoft.AspNetCore.Identity;
using NSubstitute;
using Xunit;
namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword;
[SutProviderCustomize]
public class TdeOffboardingPasswordTests
{
[Theory]
[BitAutoData]
public async Task TdeOffboardingPasswordCommand_Success(SutProvider<TdeOffboardingPasswordCommand> sutProvider,
User user, string masterPassword, string key, string hint, OrganizationUserOrganizationDetails orgUserDetails, SsoUser ssoUser)
{
// Arrange
user.MasterPassword = null;
sutProvider.GetDependency<IUserService>()
.UpdatePasswordHash(Arg.Any<User>(), Arg.Any<string>())
.Returns(IdentityResult.Success);
orgUserDetails.UseSso = true;
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyDetailsByUserAsync(user.Id)
.Returns(new List<OrganizationUserOrganizationDetails> { orgUserDetails });
sutProvider.GetDependency<ISsoUserRepository>()
.GetByUserIdOrganizationIdAsync(orgUserDetails.OrganizationId, user.Id)
.Returns(ssoUser);
var ssoConfig = new SsoConfig();
var ssoConfigData = ssoConfig.GetData();
ssoConfigData.MemberDecryptionType = MemberDecryptionType.MasterPassword;
ssoConfig.SetData(ssoConfigData);
sutProvider.GetDependency<ISsoConfigRepository>()
.GetByOrganizationIdAsync(orgUserDetails.OrganizationId)
.Returns(ssoConfig);
// Act
var result = await sutProvider.Sut.UpdateTdeOffboardingPasswordAsync(user, masterPassword, key, hint);
// Assert
Assert.Equal(IdentityResult.Success, result);
}
[Theory]
[BitAutoData]
public async Task TdeOffboardingPasswordCommand_RejectWithTdeEnabled(SutProvider<TdeOffboardingPasswordCommand> sutProvider,
User user, string masterPassword, string key, string hint, OrganizationUserOrganizationDetails orgUserDetails, SsoUser ssoUser)
{
// Arrange
user.MasterPassword = null;
sutProvider.GetDependency<IUserService>()
.UpdatePasswordHash(Arg.Any<User>(), Arg.Any<string>(), true, false)
.Returns(IdentityResult.Success);
orgUserDetails.UseSso = true;
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyDetailsByUserAsync(user.Id)
.Returns(new List<OrganizationUserOrganizationDetails> { orgUserDetails });
sutProvider.GetDependency<ISsoUserRepository>()
.GetByUserIdOrganizationIdAsync(orgUserDetails.OrganizationId, user.Id)
.Returns(ssoUser);
var ssoConfig = new SsoConfig();
var ssoConfigData = ssoConfig.GetData();
ssoConfigData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption;
ssoConfig.SetData(ssoConfigData);
sutProvider.GetDependency<ISsoConfigRepository>()
.GetByOrganizationIdAsync(orgUserDetails.OrganizationId)
.Returns(ssoConfig);
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateTdeOffboardingPasswordAsync(user, masterPassword, key, hint));
}
[Theory]
[BitAutoData]
public async Task TdeOffboardingPasswordCommand_RejectWithMasterPassword(SutProvider<TdeOffboardingPasswordCommand> sutProvider,
User user, string masterPassword, string key, string hint)
{
// the user already has a master password, so the off-boarding request should fail, since off-boarding only applies to passwordless TDE users
await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateTdeOffboardingPasswordAsync(user, masterPassword, key, hint));
}
}

View File

@ -178,6 +178,11 @@ public class IdentityServerSsoTests
{
Assert.Equal("HasManageResetPasswordPermission", p.Name);
Assert.Equal(JsonValueKind.False, p.Value.ValueKind);
},
p =>
{
Assert.Equal("IsTdeOffboarding", p.Name);
Assert.Equal(JsonValueKind.False, p.Value.ValueKind);
});
}
@ -219,6 +224,7 @@ public class IdentityServerSsoTests
// "HasAdminApproval": true,
// "HasLoginApprovingDevice": true,
// "HasManageResetPasswordPermission": false
// "IsTdeOffboarding": false
// }
// }
@ -242,6 +248,11 @@ public class IdentityServerSsoTests
{
Assert.Equal("HasManageResetPasswordPermission", p.Name);
Assert.Equal(JsonValueKind.False, p.Value.ValueKind);
},
p =>
{
Assert.Equal("IsTdeOffboarding", p.Name);
Assert.Equal(JsonValueKind.False, p.Value.ValueKind);
});
}