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-12512] Add Endpoint to allow users to request a new device otp #5146

Merged
merged 4 commits into from
Dec 16, 2024
Merged
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
8 changes: 8 additions & 0 deletions src/Api/Auth/Controllers/AccountsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,14 @@
}
}

[RequireFeature(FeatureFlagKeys.NewDeviceVerification)]
[AllowAnonymous]
[HttpPost("resend-new-device-otp")]
public async Task ResendNewDeviceOtpAsync([FromBody] UnauthenticatedSecretVerificatioRequestModel request)
{
await _userService.ResendNewDeviceVerificationEmail(request.Email, request.Secret);
}

Check warning on line 970 in src/Api/Auth/Controllers/AccountsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Controllers/AccountsController.cs#L968-L970

Added lines #L968 - L970 were not covered by tests

private async Task<IEnumerable<Guid>> GetOrganizationIdsManagingUserAsync(Guid userId)
{
var organizationManagingUser = await _userService.GetOrganizationsManagingUserAsync(userId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
๏ปฟusing System.ComponentModel.DataAnnotations;
using Bit.Core.Utilities;

namespace Bit.Api.Auth.Models.Request.Accounts;

public class UnauthenticatedSecretVerificatioRequestModel : SecretVerificationRequestModel
{
[Required]
[StrictEmailAddress]
[StringLength(256)]
public string Email { get; set; }

Check warning on line 11 in src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs#L11

Added line #L11 was not covered by tests
}
2 changes: 1 addition & 1 deletion src/Core/Services/IUserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Task<IdentityResult> UpdatePasswordHash(User user, string newPassword,
Task SendOTPAsync(User user);
Task<bool> VerifyOTPAsync(User user, string token);
Task<bool> VerifySecretAsync(User user, string secret, bool isSettingMFA = false);

Task ResendNewDeviceVerificationEmail(string email, string secret);

void SetTwoFactorProvider(User user, TwoFactorProviderType type, bool setEnabled = true);

Expand Down
16 changes: 15 additions & 1 deletion src/Core/Services/Implementations/UserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
return null;
}

_currentContext.User = await _userRepository.GetByIdAsync(userIdGuid);

Check warning on line 176 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Quality scan

'_currentContext' is null on at least one execution path. (https://rules.sonarsource.com/csharp/RSPEC-2259)
return _currentContext.User;
}

Expand All @@ -184,7 +184,7 @@
return _currentContext.User;
}

_currentContext.User = await _userRepository.GetByIdAsync(userId);

Check warning on line 187 in src/Core/Services/Implementations/UserService.cs

View workflow job for this annotation

GitHub Actions / Quality scan

'_currentContext' is null on at least one execution path. (https://rules.sonarsource.com/csharp/RSPEC-2259)
return _currentContext.User;
}

Expand Down Expand Up @@ -1407,7 +1407,7 @@

public async Task SendOTPAsync(User user)
{
if (user.Email == null)
if (string.IsNullOrEmpty(user.Email))
{
throw new BadRequestException("No user email.");
}
Expand Down Expand Up @@ -1450,6 +1450,20 @@
return isVerified;
}

public async Task ResendNewDeviceVerificationEmail(string email, string secret)
{
var user = await _userRepository.GetByEmailAsync(email);
if (user == null)
{
return;
}

if (await VerifySecretAsync(user, secret))
{
await SendOTPAsync(user);
}
}

private async Task SendAppropriateWelcomeEmailAsync(User user, string initiationPath)
{
var isFromMarketingWebsite = initiationPath.Contains("Secrets Manager trial");
Expand Down
171 changes: 135 additions & 36 deletions test/Core.Test/Services/UserServiceTests.cs
JaredSnider-Bitwarden marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data.Organizations;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
Expand Down Expand Up @@ -240,42 +241,7 @@ public async Task VerifySecretAsync_Works(
});

// HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured
var sut = new UserService(
sutProvider.GetDependency<IUserRepository>(),
sutProvider.GetDependency<ICipherRepository>(),
sutProvider.GetDependency<IOrganizationUserRepository>(),
sutProvider.GetDependency<IOrganizationRepository>(),
sutProvider.GetDependency<IMailService>(),
sutProvider.GetDependency<IPushNotificationService>(),
sutProvider.GetDependency<IUserStore<User>>(),
sutProvider.GetDependency<IOptions<IdentityOptions>>(),
sutProvider.GetDependency<IPasswordHasher<User>>(),
sutProvider.GetDependency<IEnumerable<IUserValidator<User>>>(),
sutProvider.GetDependency<IEnumerable<IPasswordValidator<User>>>(),
sutProvider.GetDependency<ILookupNormalizer>(),
sutProvider.GetDependency<IdentityErrorDescriber>(),
sutProvider.GetDependency<IServiceProvider>(),
sutProvider.GetDependency<ILogger<UserManager<User>>>(),
sutProvider.GetDependency<ILicensingService>(),
sutProvider.GetDependency<IEventService>(),
sutProvider.GetDependency<IApplicationCacheService>(),
sutProvider.GetDependency<IDataProtectionProvider>(),
sutProvider.GetDependency<IPaymentService>(),
sutProvider.GetDependency<IPolicyRepository>(),
sutProvider.GetDependency<IPolicyService>(),
sutProvider.GetDependency<IReferenceEventService>(),
sutProvider.GetDependency<IFido2>(),
sutProvider.GetDependency<ICurrentContext>(),
sutProvider.GetDependency<IGlobalSettings>(),
sutProvider.GetDependency<IAcceptOrgUserCommand>(),
sutProvider.GetDependency<IProviderUserRepository>(),
sutProvider.GetDependency<IStripeSyncService>(),
new FakeDataProtectorTokenFactory<OrgUserInviteTokenable>(),
sutProvider.GetDependency<IFeatureService>(),
sutProvider.GetDependency<IPremiumUserBillingService>(),
sutProvider.GetDependency<IRemoveOrganizationUserCommand>(),
sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
);
var sut = RebuildSut(sutProvider);

var actualIsVerified = await sut.VerifySecretAsync(user, secret);

Expand Down Expand Up @@ -522,6 +488,99 @@ await sutProvider.GetDependency<IMailService>()
.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization2.DisplayName(), user.Email);
}

[Theory, BitAutoData]
public async Task ResendNewDeviceVerificationEmail_UserNull_SendOTPAsyncNotCalled(
SutProvider<UserService> sutProvider, string email, string secret)
{
sutProvider.GetDependency<IUserRepository>()
.GetByEmailAsync(email)
.Returns(null as User);

await sutProvider.Sut.ResendNewDeviceVerificationEmail(email, secret);

await sutProvider.GetDependency<IMailService>()
.DidNotReceive()
.SendOTPEmailAsync(Arg.Any<string>(), Arg.Any<string>());
}

[Theory, BitAutoData]
public async Task ResendNewDeviceVerificationEmail_SecretNotValid_SendOTPAsyncNotCalled(
SutProvider<UserService> sutProvider, string email, string secret)
{
sutProvider.GetDependency<IUserRepository>()
.GetByEmailAsync(email)
.Returns(null as User);

await sutProvider.Sut.ResendNewDeviceVerificationEmail(email, secret);

await sutProvider.GetDependency<IMailService>()
.DidNotReceive()
.SendOTPEmailAsync(Arg.Any<string>(), Arg.Any<string>());
}

[Theory, BitAutoData]
public async Task ResendNewDeviceVerificationEmail_SendsToken_Success(
SutProvider<UserService> sutProvider, User user)
{
// Arrange
var testPassword = "test_password";
var tokenProvider = SetupFakeTokenProvider(sutProvider, user);
SetupUserAndDevice(user, true);

// Setup the fake password verification
var substitutedUserPasswordStore = Substitute.For<IUserPasswordStore<User>>();
substitutedUserPasswordStore
.GetPasswordHashAsync(user, Arg.Any<CancellationToken>())
.Returns((ci) =>
{
return Task.FromResult("hashed_test_password");
});

sutProvider.SetDependency<IUserStore<User>>(substitutedUserPasswordStore, "store");

sutProvider.GetDependency<IPasswordHasher<User>>("passwordHasher")
.VerifyHashedPassword(user, "hashed_test_password", testPassword)
.Returns((ci) =>
{
return PasswordVerificationResult.Success;
});

sutProvider.GetDependency<IUserRepository>()
.GetByEmailAsync(user.Email)
.Returns(user);

// HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured
var sut = RebuildSut(sutProvider);

await sut.ResendNewDeviceVerificationEmail(user.Email, testPassword);

await sutProvider.GetDependency<IMailService>()
.Received(1)
.SendOTPEmailAsync(user.Email, Arg.Any<string>());
}

[Theory]
[BitAutoData("")]
[BitAutoData("null")]
public async Task SendOTPAsync_UserEmailNull_ThrowsBadRequest(
string email,
SutProvider<UserService> sutProvider, User user)
{
user.Email = email == "null" ? null : "";
var expectedMessage = "No user email.";
try
{
await sutProvider.Sut.SendOTPAsync(user);
}
catch (BadRequestException ex)
{
Assert.Equal(ex.Message, expectedMessage);
await sutProvider.GetDependency<IMailService>()
.DidNotReceive()
.SendOTPEmailAsync(Arg.Any<string>(), Arg.Any<string>());
}
}

private static void SetupUserAndDevice(User user,
bool shouldHavePassword)
{
Expand Down Expand Up @@ -573,4 +632,44 @@ private static IUserTwoFactorTokenProvider<User> SetupFakeTokenProvider(SutProvi

return fakeUserTwoFactorProvider;
}

private IUserService RebuildSut(SutProvider<UserService> sutProvider)
{
return new UserService(
sutProvider.GetDependency<IUserRepository>(),
sutProvider.GetDependency<ICipherRepository>(),
sutProvider.GetDependency<IOrganizationUserRepository>(),
sutProvider.GetDependency<IOrganizationRepository>(),
sutProvider.GetDependency<IMailService>(),
sutProvider.GetDependency<IPushNotificationService>(),
sutProvider.GetDependency<IUserStore<User>>(),
sutProvider.GetDependency<IOptions<IdentityOptions>>(),
sutProvider.GetDependency<IPasswordHasher<User>>(),
sutProvider.GetDependency<IEnumerable<IUserValidator<User>>>(),
sutProvider.GetDependency<IEnumerable<IPasswordValidator<User>>>(),
sutProvider.GetDependency<ILookupNormalizer>(),
sutProvider.GetDependency<IdentityErrorDescriber>(),
sutProvider.GetDependency<IServiceProvider>(),
sutProvider.GetDependency<ILogger<UserManager<User>>>(),
sutProvider.GetDependency<ILicensingService>(),
sutProvider.GetDependency<IEventService>(),
sutProvider.GetDependency<IApplicationCacheService>(),
sutProvider.GetDependency<IDataProtectionProvider>(),
sutProvider.GetDependency<IPaymentService>(),
sutProvider.GetDependency<IPolicyRepository>(),
sutProvider.GetDependency<IPolicyService>(),
sutProvider.GetDependency<IReferenceEventService>(),
sutProvider.GetDependency<IFido2>(),
sutProvider.GetDependency<ICurrentContext>(),
sutProvider.GetDependency<IGlobalSettings>(),
sutProvider.GetDependency<IAcceptOrgUserCommand>(),
sutProvider.GetDependency<IProviderUserRepository>(),
sutProvider.GetDependency<IStripeSyncService>(),
new FakeDataProtectorTokenFactory<OrgUserInviteTokenable>(),
sutProvider.GetDependency<IFeatureService>(),
sutProvider.GetDependency<IPremiumUserBillingService>(),
sutProvider.GetDependency<IRemoveOrganizationUserCommand>(),
sutProvider.GetDependency<IRevokeNonCompliantOrganizationUserCommand>()
);
}
}
Loading