From 54bd5fa894d98e0f21ca65ef85c5171914bb861d Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Fri, 26 Jul 2024 13:30:47 -0400 Subject: [PATCH] Auth/PM-10130 - Registration with Email Verification - Respect Self-hosted Disable Open Registration flag (#4561) * PM-10130 - Registration with email verification - respect self hosted disable open registration setting properly in non-org invite scenarios. * PM-10130 - Fix unit tests. * PM-10130 - Update integration tests. --- .../Implementations/RegisterUserCommand.cs | 16 ++++-- ...VerificationEmailForRegistrationCommand.cs | 5 ++ .../Registration/RegisterUserCommandTests.cs | 14 +++++ ...icationEmailForRegistrationCommandTests.cs | 27 ++++++++++ .../Controllers/AccountsControllerTests.cs | 54 +++++++++++++++++++ .../Factories/WebApplicationFactoryBase.cs | 1 + 6 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs index 6ca6307a6..9d6a3bb3b 100644 --- a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs +++ b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs @@ -37,6 +37,8 @@ public class RegisterUserCommand : IRegisterUserCommand private readonly IUserService _userService; private readonly IMailService _mailService; + private readonly string _disabledUserRegistrationExceptionMsg = "Open registration has been disabled by the system administrator."; + public RegisterUserCommand( IGlobalSettings globalSettings, IOrganizationUserRepository organizationUserRepository, @@ -124,8 +126,6 @@ public class RegisterUserCommand : IRegisterUserCommand private void ValidateOrgInviteToken(string orgInviteToken, Guid? orgUserId, User user) { - const string disabledUserRegistrationExceptionMsg = "Open registration has been disabled by the system administrator."; - var orgInviteTokenProvided = !string.IsNullOrWhiteSpace(orgInviteToken); if (orgInviteTokenProvided && orgUserId.HasValue) @@ -140,7 +140,7 @@ public class RegisterUserCommand : IRegisterUserCommand if (_globalSettings.DisableUserRegistration) { - throw new BadRequestException(disabledUserRegistrationExceptionMsg); + throw new BadRequestException(_disabledUserRegistrationExceptionMsg); } throw new BadRequestException("Organization invite token is invalid."); @@ -152,7 +152,7 @@ public class RegisterUserCommand : IRegisterUserCommand // as you can't register without them. if (_globalSettings.DisableUserRegistration) { - throw new BadRequestException(disabledUserRegistrationExceptionMsg); + throw new BadRequestException(_disabledUserRegistrationExceptionMsg); } // Open registration is allowed @@ -233,6 +233,14 @@ public class RegisterUserCommand : IRegisterUserCommand public async Task RegisterUserViaEmailVerificationToken(User user, string masterPasswordHash, string emailVerificationToken) { + // We validate open registration on send of initial email and here b/c a user could technically start the + // account creation process while open registration is enabled and then finish it after it has been + // disabled by the self hosted admin. + if (_globalSettings.DisableUserRegistration) + { + throw new BadRequestException(_disabledUserRegistrationExceptionMsg); + } + var tokenable = ValidateRegistrationEmailVerificationTokenable(emailVerificationToken, user.Email); user.EmailVerified = true; diff --git a/src/Core/Auth/UserFeatures/Registration/Implementations/SendVerificationEmailForRegistrationCommand.cs b/src/Core/Auth/UserFeatures/Registration/Implementations/SendVerificationEmailForRegistrationCommand.cs index d1cdca5e5..21a421b9d 100644 --- a/src/Core/Auth/UserFeatures/Registration/Implementations/SendVerificationEmailForRegistrationCommand.cs +++ b/src/Core/Auth/UserFeatures/Registration/Implementations/SendVerificationEmailForRegistrationCommand.cs @@ -39,6 +39,11 @@ public class SendVerificationEmailForRegistrationCommand : ISendVerificationEmai public async Task Run(string email, string? name, bool receiveMarketingEmails) { + if (_globalSettings.DisableUserRegistration) + { + throw new BadRequestException("Open registration has been disabled by the system administrator."); + } + if (string.IsNullOrWhiteSpace(email)) { throw new ArgumentNullException(nameof(email)); diff --git a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs index 05e4e1ca8..d61a34541 100644 --- a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs @@ -367,4 +367,18 @@ public class RegisterUserCommandTests } + [Theory] + [BitAutoData] + public async Task RegisterUserViaEmailVerificationToken_DisabledOpenRegistration_ThrowsBadRequestException(SutProvider sutProvider, User user, string masterPasswordHash, string emailVerificationToken) + { + // Arrange + sutProvider.GetDependency() + .DisableUserRegistration = true; + + // Act & Assert + var result = await Assert.ThrowsAsync(() => sutProvider.Sut.RegisterUserViaEmailVerificationToken(user, masterPasswordHash, emailVerificationToken)); + Assert.Equal("Open registration has been disabled by the system administrator.", result.Message); + + } + } diff --git a/test/Core.Test/Auth/UserFeatures/Registration/SendVerificationEmailForRegistrationCommandTests.cs b/test/Core.Test/Auth/UserFeatures/Registration/SendVerificationEmailForRegistrationCommandTests.cs index 627350483..f4f620f8a 100644 --- a/test/Core.Test/Auth/UserFeatures/Registration/SendVerificationEmailForRegistrationCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/Registration/SendVerificationEmailForRegistrationCommandTests.cs @@ -31,6 +31,9 @@ public class SendVerificationEmailForRegistrationCommandTests sutProvider.GetDependency() .EnableEmailVerification = true; + sutProvider.GetDependency() + .DisableUserRegistration = false; + sutProvider.GetDependency() .SendRegistrationVerificationEmailAsync(email, Arg.Any()) .Returns(Task.CompletedTask); @@ -63,6 +66,9 @@ public class SendVerificationEmailForRegistrationCommandTests sutProvider.GetDependency() .EnableEmailVerification = true; + sutProvider.GetDependency() + .DisableUserRegistration = false; + var mockedToken = "token"; sutProvider.GetDependency>() .Protect(Arg.Any()) @@ -91,6 +97,9 @@ public class SendVerificationEmailForRegistrationCommandTests sutProvider.GetDependency() .EnableEmailVerification = false; + sutProvider.GetDependency() + .DisableUserRegistration = false; + var mockedToken = "token"; sutProvider.GetDependency>() .Protect(Arg.Any()) @@ -103,6 +112,19 @@ public class SendVerificationEmailForRegistrationCommandTests Assert.Equal(mockedToken, result); } + [Theory] + [BitAutoData] + public async Task SendVerificationEmailForRegistrationCommand_WhenOpenRegistrationDisabled_ThrowsBadRequestException(SutProvider sutProvider, + string email, string name, bool receiveMarketingEmails) + { + // Arrange + sutProvider.GetDependency() + .DisableUserRegistration = true; + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.Run(email, name, receiveMarketingEmails)); + } + [Theory] [BitAutoData] public async Task SendVerificationEmailForRegistrationCommand_WhenIsExistingUserAndEnableEmailVerificationFalse_ThrowsBadRequestException(SutProvider sutProvider, @@ -125,6 +147,9 @@ public class SendVerificationEmailForRegistrationCommandTests public async Task SendVerificationEmailForRegistrationCommand_WhenNullEmail_ThrowsArgumentNullException(SutProvider sutProvider, string name, bool receiveMarketingEmails) { + sutProvider.GetDependency() + .DisableUserRegistration = false; + await Assert.ThrowsAsync(async () => await sutProvider.Sut.Run(null, name, receiveMarketingEmails)); } @@ -133,6 +158,8 @@ public class SendVerificationEmailForRegistrationCommandTests public async Task SendVerificationEmailForRegistrationCommand_WhenEmptyEmail_ThrowsArgumentNullException(SutProvider sutProvider, string name, bool receiveMarketingEmails) { + sutProvider.GetDependency() + .DisableUserRegistration = false; await Assert.ThrowsAsync(async () => await sutProvider.Sut.Run("", name, receiveMarketingEmails)); } } diff --git a/test/Identity.IntegrationTest/Controllers/AccountsControllerTests.cs b/test/Identity.IntegrationTest/Controllers/AccountsControllerTests.cs index 36d5891d7..c5f10aacb 100644 --- a/test/Identity.IntegrationTest/Controllers/AccountsControllerTests.cs +++ b/test/Identity.IntegrationTest/Controllers/AccountsControllerTests.cs @@ -62,6 +62,28 @@ public class AccountsControllerTests : IClassFixture Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); } + [Theory, BitAutoData] + public async Task PostRegisterSendEmailVerification_DisabledOpenRegistration_ThrowsBadRequestException(string name, bool receiveMarketingEmails) + { + + // Localize substitutions to this test. + var localFactory = new IdentityApplicationFactory(); + localFactory.UpdateConfiguration("globalSettings:disableUserRegistration", "true"); + + var email = $"test+register+{name}@email.com"; + var model = new RegisterSendVerificationEmailRequestModel + { + Email = email, + Name = name, + ReceiveMarketingEmails = receiveMarketingEmails + }; + + var context = await localFactory.PostRegisterSendEmailVerificationAsync(model); + + Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); + } + + [Theory] [BitAutoData(true)] [BitAutoData(false)] @@ -198,6 +220,38 @@ public class AccountsControllerTests : IClassFixture Assert.Equal(kdfParallelism, user.KdfParallelism); } + + [Theory, BitAutoData] + public async Task RegistrationWithEmailVerification_OpenRegistrationDisabled_ThrowsBadRequestException([Required] string name, string emailVerificationToken, + [StringLength(1000), Required] string masterPasswordHash, [StringLength(50)] string masterPasswordHint, [Required] string userSymmetricKey, + [Required] KeysRequestModel userAsymmetricKeys, int kdfMemory, int kdfParallelism) + { + // Localize substitutions to this test. + var localFactory = new IdentityApplicationFactory(); + localFactory.UpdateConfiguration("globalSettings:disableUserRegistration", "true"); + + var email = $"test+register+{name}@email.com"; + + // Now we call the finish registration endpoint with the email verification token + var registerFinishReqModel = new RegisterFinishRequestModel + { + Email = email, + MasterPasswordHash = masterPasswordHash, + MasterPasswordHint = masterPasswordHint, + EmailVerificationToken = emailVerificationToken, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, + UserSymmetricKey = userSymmetricKey, + UserAsymmetricKeys = userAsymmetricKeys, + KdfMemory = kdfMemory, + KdfParallelism = kdfParallelism + }; + + var postRegisterFinishHttpContext = await localFactory.PostRegisterFinishAsync(registerFinishReqModel); + + Assert.Equal(StatusCodes.Status400BadRequest, postRegisterFinishHttpContext.Response.StatusCode); + } + [Theory, BitAutoData] public async Task RegistrationWithEmailVerification_WithOrgInviteToken_Succeeds( [StringLength(1000)] string masterPasswordHash, [StringLength(50)] string masterPasswordHint, string userSymmetricKey, diff --git a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs index b06226557..e0fcc0e5e 100644 --- a/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs +++ b/test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs @@ -144,6 +144,7 @@ public abstract class WebApplicationFactoryBase : WebApplicationFactory // Email Verification { "globalSettings:enableEmailVerification", "true" }, + { "globalSettings:disableUserRegistration", "false" }, { "globalSettings:launchDarkly:flagValues:email-verification", "true" } }); });