From e350ef650a5d9237ba57b1bfe779123d67bb6dcf Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Tue, 17 Apr 2018 08:10:17 -0400 Subject: [PATCH] dont cycle security token on re-hash --- src/Api/Controllers/AccountsController.cs | 7 +--- src/Api/Controllers/CiphersController.cs | 5 +-- .../Controllers/OrganizationsController.cs | 7 +--- src/Api/Controllers/TwoFactorController.cs | 4 +- .../ResourceOwnerPasswordValidator.cs | 2 +- src/Core/Services/IUserService.cs | 1 + .../Services/Implementations/UserService.cs | 39 ++++++++++++++++--- 7 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index 1b35b468a..08d62d7c0 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -25,7 +25,6 @@ namespace Bit.Api.Controllers private readonly ICipherService _cipherService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ILicensingService _licenseService; - private readonly UserManager _userManager; private readonly GlobalSettings _globalSettings; public AccountsController( @@ -33,13 +32,11 @@ namespace Bit.Api.Controllers ICipherService cipherService, IOrganizationUserRepository organizationUserRepository, ILicensingService licenseService, - UserManager userManager, GlobalSettings globalSettings) { _userService = userService; _cipherService = cipherService; _organizationUserRepository = organizationUserRepository; - _userManager = userManager; _licenseService = licenseService; _globalSettings = globalSettings; } @@ -79,7 +76,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - if(!await _userManager.CheckPasswordAsync(user, model.MasterPasswordHash)) + if(!await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { await Task.Delay(2000); throw new BadRequestException("MasterPasswordHash", "Invalid password."); @@ -323,7 +320,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - if(!await _userManager.CheckPasswordAsync(user, model.MasterPasswordHash)) + if(!await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { ModelState.AddModelError("MasterPasswordHash", "Invalid password."); await Task.Delay(2000); diff --git a/src/Api/Controllers/CiphersController.cs b/src/Api/Controllers/CiphersController.cs index 792f97fac..35411fca4 100644 --- a/src/Api/Controllers/CiphersController.cs +++ b/src/Api/Controllers/CiphersController.cs @@ -25,7 +25,6 @@ namespace Bit.Api.Controllers private readonly ICollectionCipherRepository _collectionCipherRepository; private readonly ICipherService _cipherService; private readonly IUserService _userService; - private readonly UserManager _userManager; private readonly CurrentContext _currentContext; private readonly GlobalSettings _globalSettings; @@ -34,7 +33,6 @@ namespace Bit.Api.Controllers ICollectionCipherRepository collectionCipherRepository, ICipherService cipherService, IUserService userService, - UserManager userManager, CurrentContext currentContext, GlobalSettings globalSettings) { @@ -42,7 +40,6 @@ namespace Bit.Api.Controllers _collectionCipherRepository = collectionCipherRepository; _cipherService = cipherService; _userService = userService; - _userManager = userManager; _currentContext = currentContext; _globalSettings = globalSettings; } @@ -381,7 +378,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - if(!await _userManager.CheckPasswordAsync(user, model.MasterPasswordHash)) + if(!await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { ModelState.AddModelError("MasterPasswordHash", "Invalid password."); await Task.Delay(2000); diff --git a/src/Api/Controllers/OrganizationsController.cs b/src/Api/Controllers/OrganizationsController.cs index b4614050b..14eaf735e 100644 --- a/src/Api/Controllers/OrganizationsController.cs +++ b/src/Api/Controllers/OrganizationsController.cs @@ -32,7 +32,6 @@ namespace Bit.Api.Controllers private readonly CurrentContext _currentContext; private readonly GlobalSettings _globalSettings; private readonly ApiSettings _apiSettings; - private readonly UserManager _userManager; public OrganizationsController( IOrganizationRepository organizationRepository, @@ -41,15 +40,13 @@ namespace Bit.Api.Controllers IUserService userService, CurrentContext currentContext, GlobalSettings globalSettings, - IOptions apiSettings, - UserManager userManager) + IOptions apiSettings) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _organizationService = organizationService; _userService = userService; _currentContext = currentContext; - _userManager = userManager; _globalSettings = globalSettings; _apiSettings = apiSettings.Value; } @@ -359,7 +356,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - if(!await _userManager.CheckPasswordAsync(user, model.MasterPasswordHash)) + if(!await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { await Task.Delay(2000); throw new BadRequestException("MasterPasswordHash", "Invalid password."); diff --git a/src/Api/Controllers/TwoFactorController.cs b/src/Api/Controllers/TwoFactorController.cs index 5673095d2..bb234f50a 100644 --- a/src/Api/Controllers/TwoFactorController.cs +++ b/src/Api/Controllers/TwoFactorController.cs @@ -246,7 +246,7 @@ namespace Bit.Api.Controllers var user = await _userManager.FindByEmailAsync(model.Email.ToLowerInvariant()); if(user != null) { - if(await _userManager.CheckPasswordAsync(user, model.MasterPasswordHash)) + if(await _userService.CheckPasswordAsync(user, model.MasterPasswordHash)) { await _userService.SendTwoFactorEmailAsync(user); return; @@ -336,7 +336,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - if(!await _userManager.CheckPasswordAsync(user, masterPasswordHash)) + if(!await _userService.CheckPasswordAsync(user, masterPasswordHash)) { await Task.Delay(2000); throw new BadRequestException("MasterPasswordHash", "Invalid password."); diff --git a/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs b/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs index 4931cbb2a..7cf6887b0 100644 --- a/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs +++ b/src/Core/IdentityServer/ResourceOwnerPasswordValidator.cs @@ -69,7 +69,7 @@ namespace Bit.Core.IdentityServer } var user = await _userManager.FindByEmailAsync(context.UserName.ToLowerInvariant()); - if(user == null || !await _userManager.CheckPasswordAsync(user, context.Password)) + if(user == null || !await _userService.CheckPasswordAsync(user, context.Password)) { await BuildErrorResultAsync(false, context, user); return; diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index be87399c5..590e85186 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -50,5 +50,6 @@ namespace Bit.Core.Services Task DisablePremiumAsync(User user, DateTime? expirationDate); Task UpdatePremiumExpirationAsync(Guid userId, DateTime? expirationDate); Task GenerateLicenseAsync(User user, BillingInfo billingInfo = null); + Task CheckPasswordAsync(User user, string password); } } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index dc348df31..aa5e8666c 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -411,7 +411,7 @@ namespace Bit.Core.Services throw new ArgumentNullException(nameof(user)); } - if(await base.CheckPasswordAsync(user, masterPassword)) + if(await CheckPasswordAsync(user, masterPassword)) { var result = await UpdatePasswordHash(user, newMasterPassword); if(!result.Succeeded) @@ -440,7 +440,7 @@ namespace Bit.Core.Services throw new ArgumentNullException(nameof(user)); } - if(await base.CheckPasswordAsync(user, masterPassword)) + if(await CheckPasswordAsync(user, masterPassword)) { user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow; user.SecurityStamp = Guid.NewGuid().ToString(); @@ -469,7 +469,7 @@ namespace Bit.Core.Services throw new ArgumentNullException(nameof(user)); } - if(await base.CheckPasswordAsync(user, masterPassword)) + if(await CheckPasswordAsync(user, masterPassword)) { var result = await base.UpdateSecurityStampAsync(user); if(!result.Succeeded) @@ -527,7 +527,7 @@ namespace Bit.Core.Services return false; } - if(!await base.CheckPasswordAsync(user, masterPassword)) + if(!await CheckPasswordAsync(user, masterPassword)) { return false; } @@ -745,7 +745,31 @@ namespace Bit.Core.Services new UserLicense(user, billingInfo, _licenseService); } - private async Task UpdatePasswordHash(User user, string newPassword, bool validatePassword = true) + public override async Task CheckPasswordAsync(User user, string password) + { + if(user == null) + { + return false; + } + + var result = await base.VerifyPasswordAsync(Store as IUserPasswordStore, user, password); + if(result == PasswordVerificationResult.SuccessRehashNeeded) + { + await UpdatePasswordHash(user, password, false, false); + user.RevisionDate = DateTime.UtcNow; + await _userRepository.ReplaceAsync(user); + } + + var success = result != PasswordVerificationResult.Failed; + if(!success) + { + Logger.LogWarning(0, "Invalid password for user {userId}.", user.Id); + } + return success; + } + + private async Task UpdatePasswordHash(User user, string newPassword, + bool validatePassword = true, bool refreshStamp = true) { if(validatePassword) { @@ -757,7 +781,10 @@ namespace Bit.Core.Services } user.MasterPassword = _passwordHasher.HashPassword(user, newPassword); - user.SecurityStamp = Guid.NewGuid().ToString(); + if(refreshStamp) + { + user.SecurityStamp = Guid.NewGuid().ToString(); + } return IdentityResult.Success; }