From 01c8be1a9ccc54ec6b0d01306f878842fb18e264 Mon Sep 17 00:00:00 2001 From: Ike Kottlowski Date: Mon, 16 Dec 2024 17:41:09 -0800 Subject: [PATCH] feat(NewDeviceVerification) : Adding exception flow to admin project. Added tests for new methods in UserService. --- src/Admin/Controllers/UsersController.cs | 19 ++- src/Admin/Models/UserEditModel.cs | 7 +- src/Admin/Views/Users/Edit.cshtml | 132 +++++++++++------- src/Core/Services/IUserService.cs | 11 ++ .../Services/Implementations/UserService.cs | 30 +++- test/Core.Test/Services/UserServiceTests.cs | 66 ++++++++- 6 files changed, 214 insertions(+), 51 deletions(-) diff --git a/src/Admin/Controllers/UsersController.cs b/src/Admin/Controllers/UsersController.cs index 54e43d8b4f0f..a988cc2af7ac 100644 --- a/src/Admin/Controllers/UsersController.cs +++ b/src/Admin/Controllers/UsersController.cs @@ -107,7 +107,8 @@ public async Task Edit(Guid id) var billingHistoryInfo = await _paymentService.GetBillingHistoryAsync(user); var isTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var verifiedDomain = await AccountDeprovisioningEnabled(user.Id); - return View(new UserEditModel(user, isTwoFactorEnabled, ciphers, billingInfo, billingHistoryInfo, _globalSettings, verifiedDomain)); + var deviceVerificationRequired = await _userService.ActiveNewDeviceVerificationException(user.Id); + return View(new UserEditModel(user, isTwoFactorEnabled, ciphers, billingInfo, billingHistoryInfo, _globalSettings, verifiedDomain, deviceVerificationRequired)); } [HttpPost] @@ -162,6 +163,22 @@ public async Task Delete(Guid id) return RedirectToAction("Index"); } + [HttpPost] + [ValidateAntiForgeryToken] + [RequirePermission(Permission.User_GeneralDetails_View)] + [RequireFeature(FeatureFlagKeys.NewDeviceVerification)] + public async Task ToggleNewDeviceVerification(Guid id) + { + var user = await _userRepository.GetByIdAsync(id); + if (user == null) + { + return RedirectToAction("Index"); + } + + await _userService.ToggleNewDeviceVerificationException(user.Id); + return RedirectToAction("Edit", new { id }); + } + // TODO: Feature flag to be removed in PM-14207 private async Task AccountDeprovisioningEnabled(Guid userId) { diff --git a/src/Admin/Models/UserEditModel.cs b/src/Admin/Models/UserEditModel.cs index ed2d653246fc..2597da6e96ce 100644 --- a/src/Admin/Models/UserEditModel.cs +++ b/src/Admin/Models/UserEditModel.cs @@ -18,10 +18,13 @@ public UserEditModel( BillingInfo billingInfo, BillingHistoryInfo billingHistoryInfo, GlobalSettings globalSettings, - bool? claimedAccount) + bool? claimedAccount, + bool? activeNewDeviceVerificationException) { User = UserViewModel.MapViewModel(user, isTwoFactorEnabled, ciphers, claimedAccount); + ActiveNewDeviceVerificationException = activeNewDeviceVerificationException ?? false; + BillingInfo = billingInfo; BillingHistoryInfo = billingHistoryInfo; BraintreeMerchantId = globalSettings.Braintree.MerchantId; @@ -44,6 +47,8 @@ public UserEditModel( public string RandomLicenseKey => CoreHelpers.SecureRandomString(20); public string OneYearExpirationDate => DateTime.Now.AddYears(1).ToString("yyyy-MM-ddTHH:mm"); public string BraintreeMerchantId { get; init; } + public bool ActiveNewDeviceVerificationException { get; init; } + [Display(Name = "Name")] public string Name { get; init; } diff --git a/src/Admin/Views/Users/Edit.cshtml b/src/Admin/Views/Users/Edit.cshtml index d9fc07884d25..417d9fb9a2ca 100644 --- a/src/Admin/Views/Users/Edit.cshtml +++ b/src/Admin/Views/Users/Edit.cshtml @@ -1,11 +1,14 @@ @using Bit.Admin.Enums; @inject Bit.Admin.Services.IAccessControlService AccessControlService +@inject Bit.Core.Services.IFeatureService FeatureService @inject IWebHostEnvironment HostingEnvironment @model UserEditModel @{ ViewData["Title"] = "User: " + Model.User.Email; var canViewUserInformation = AccessControlService.UserHasPermission(Permission.User_UserInformation_View); + var canViewNewDeviceException = AccessControlService.UserHasPermission(Permission.User_UserInformation_View) && + FeatureService.IsEnabled(Bit.Core.FeatureFlagKeys.NewDeviceVerification); var canViewBillingInformation = AccessControlService.UserHasPermission(Permission.User_BillingInformation_View); var canViewGeneral = AccessControlService.UserHasPermission(Permission.User_GeneralDetails_View); var canViewPremium = AccessControlService.UserHasPermission(Permission.User_Premium_View); @@ -32,7 +35,11 @@ // Premium document.getElementById('@(nameof(Model.MaxStorageGb))').value = '1'; document.getElementById('@(nameof(Model.Premium))').checked = true; + using Stripe.Entitlements; // Licensing + using Bit.Core; + using Stripe.Entitlements; + using Microsoft.Identity.Client.Extensibility; document.getElementById('@(nameof(Model.LicenseKey))').value = '@Model.RandomLicenseKey'; document.getElementById('@(nameof(Model.PremiumExpirationDate))').value = '@Model.OneYearExpirationDate'; @@ -47,13 +54,13 @@ if (gateway.value === '@((byte)Bit.Core.Enums.GatewayType.Stripe)') { const url = '@(HostingEnvironment.IsDevelopment() - ? "https://dashboard.stripe.com/test" - : "https://dashboard.stripe.com")'; + ? "https://dashboard.stripe.com/test" + : "https://dashboard.stripe.com")'; window.open(`${url}/customers/${customerId.value}/`, '_blank'); } else if (gateway.value === '@((byte)Bit.Core.Enums.GatewayType.Braintree)') { const url = '@(HostingEnvironment.IsDevelopment() - ? $"https://www.sandbox.braintreegateway.com/merchants/{Model.BraintreeMerchantId}" - : $"https://www.braintreegateway.com/merchants/{Model.BraintreeMerchantId}")'; + ? $"https://www.sandbox.braintreegateway.com/merchants/{Model.BraintreeMerchantId}" + : $"https://www.braintreegateway.com/merchants/{Model.BraintreeMerchantId}")'; window.open(`${url}/${customerId.value}`, '_blank'); } }); @@ -67,13 +74,13 @@ if (gateway.value === '@((byte)Bit.Core.Enums.GatewayType.Stripe)') { const url = '@(HostingEnvironment.IsDevelopment() || HostingEnvironment.IsEnvironment("QA") - ? "https://dashboard.stripe.com/test" - : "https://dashboard.stripe.com")' + ? "https://dashboard.stripe.com/test" + : "https://dashboard.stripe.com")' window.open(`${url}/subscriptions/${subId.value}`, '_blank'); } else if (gateway.value === '@((byte)Bit.Core.Enums.GatewayType.Braintree)') { const url = '@(HostingEnvironment.IsDevelopment() || HostingEnvironment.IsEnvironment("QA") - ? $"https://www.sandbox.braintreegateway.com/merchants/{Model.BraintreeMerchantId}" - : $"https://www.braintreegateway.com/merchants/{Model.BraintreeMerchantId}")'; + ? $"https://www.sandbox.braintreegateway.com/merchants/{Model.BraintreeMerchantId}" + : $"https://www.braintreegateway.com/merchants/{Model.BraintreeMerchantId}")'; window.open(`${url}/subscriptions/${subId.value}`, '_blank'); } }); @@ -88,11 +95,40 @@

User Information

@await Html.PartialAsync("_ViewInformation", Model.User) } +@if (canViewNewDeviceException) +{ +

New Device Verification

+
+
+
+ @if (Model.ActiveNewDeviceVerificationException) + { +

Status: Bypassed

+ + } + else + { +

Status: Required

+ + } +
+ +
+
+} @if (canViewBillingInformation) {

Billing Information

@await Html.PartialAsync("_BillingInformation", - new BillingInformationModel { BillingInfo = Model.BillingInfo, BillingHistoryInfo = Model.BillingHistoryInfo, UserId = Model.User.Id, Entity = "User" }) + new BillingInformationModel +{ + BillingInfo = Model.BillingInfo, + BillingHistoryInfo = Model.BillingHistoryInfo, + UserId = Model.User.Id, + Entity = "User" +}) } @if (canViewGeneral) { @@ -109,7 +145,7 @@ } -
+ @if (canViewPremium) {

Premium

@@ -139,54 +175,56 @@
- +
} -@if (canViewBilling) -{ -

Billing

-
-
-
- - - - + + +
-
-
-
- -
- - @if (canLaunchGateway) - { - - } +
+
+ +
+ + @if (canLaunchGateway) + { + + } +
-
-
-
- -
- - @if (canLaunchGateway) - { - - } +
+
+ +
+ + @if (canLaunchGateway) + { + + } +
-
-} + }
diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 65bec5ea9f78..7a10bfbc16e3 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -77,6 +77,17 @@ Task UpdatePasswordHash(User user, string newPassword, Task VerifyOTPAsync(User user, string token); Task VerifySecretAsync(User user, string secret, bool isSettingMFA = false); + /// + /// We use this method to check if the user has an active new device verification bypass + /// + /// self + /// returns true if the value is found in the cache + Task ActiveNewDeviceVerificationException(Guid userId); + /// + /// We use this method to toggle the new device verification bypass + /// + /// Id of user bypassing new device verification + Task ToggleNewDeviceVerificationException(Guid userId); void SetTwoFactorProvider(User user, TwoFactorProviderType type, bool setEnabled = true); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 2bc81959b9b3..8b19cd2e59a7 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -30,6 +30,7 @@ using Fido2NetLib.Objects; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using File = System.IO.File; @@ -71,6 +72,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IPremiumUserBillingService _premiumUserBillingService; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IRevokeNonCompliantOrganizationUserCommand _revokeNonCompliantOrganizationUserCommand; + private readonly IDistributedCache _distributedCache; public UserService( IUserRepository userRepository, @@ -106,7 +108,8 @@ public UserService( IFeatureService featureService, IPremiumUserBillingService premiumUserBillingService, IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand) + IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand, + IDistributedCache distributedCache) : base( store, optionsAccessor, @@ -148,6 +151,7 @@ public UserService( _premiumUserBillingService = premiumUserBillingService; _removeOrganizationUserCommand = removeOrganizationUserCommand; _revokeNonCompliantOrganizationUserCommand = revokeNonCompliantOrganizationUserCommand; + _distributedCache = distributedCache; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -1450,6 +1454,30 @@ public async Task VerifySecretAsync(User user, string secret, bool isSetti return isVerified; } + public async Task ActiveNewDeviceVerificationException(Guid userId) + { + var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString()); + var cacheValue = await _distributedCache.GetAsync(cacheKey); + return cacheValue != null; + } + + public async Task ToggleNewDeviceVerificationException(Guid userId) + { + var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, userId.ToString()); + var cacheValue = await _distributedCache.GetAsync(cacheKey); + if (cacheValue != null) + { + await _distributedCache.RemoveAsync(cacheKey); + } + else + { + await _distributedCache.SetAsync(cacheKey, new byte[1], new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromHours(24) + }); + } + } + private async Task SendAppropriateWelcomeEmailAsync(User user, string initiationPath) { var isFromMarketingWebsite = initiationPath.Contains("Secrets Manager trial"); diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index de2a5187171c..fa5e092dbdce 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -30,6 +30,7 @@ using Fido2NetLib; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NSubstitute; @@ -274,7 +275,8 @@ public async Task VerifySecretAsync_Works( sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); var actualIsVerified = await sut.VerifySecretAsync(user, secret); @@ -522,6 +524,68 @@ await sutProvider.GetDependency() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization2.DisplayName(), user.Email); } + [Theory, BitAutoData] + public async Task ActiveNewDeviceVerificationException_UserNotInCache_ReturnsFalseAsync( + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetAsync(Arg.Any()) + .Returns(null as byte[]); + + var result = await sutProvider.Sut.ActiveNewDeviceVerificationException(Guid.NewGuid()); + + Assert.False(result); + } + + [Theory, BitAutoData] + public async Task ActiveNewDeviceVerificationException_UserInCache_ReturnsTrueAsync( + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetAsync(Arg.Any()) + .Returns([1]); + + var result = await sutProvider.Sut.ActiveNewDeviceVerificationException(Guid.NewGuid()); + + Assert.True(result); + } + + [Theory, BitAutoData] + public async Task ToggleNewDeviceVerificationException_UserInCache_RemovesUserFromCache( + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetAsync(Arg.Any()) + .Returns([1]); + + await sutProvider.Sut.ToggleNewDeviceVerificationException(Guid.NewGuid()); + + await sutProvider.GetDependency() + .DidNotReceive() + .SetAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .Received(1) + .RemoveAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ToggleNewDeviceVerificationException_UserNotInCache_AddsUserToCache( + SutProvider sutProvider) + { + sutProvider.GetDependency() + .GetAsync(Arg.Any()) + .Returns(null as byte[]); + + await sutProvider.Sut.ToggleNewDeviceVerificationException(Guid.NewGuid()); + + await sutProvider.GetDependency() + .Received(1) + .SetAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceive() + .RemoveAsync(Arg.Any()); + } + private static void SetupUserAndDevice(User user, bool shouldHavePassword) {