mirror of
https://github.com/bitwarden/server.git
synced 2024-11-23 12:25:16 +01:00
[PM-11405] Account Management: Prevent a verified user from changing their email address (#4875)
* Add check for managed user before purging account * Rename IOrganizationRepository.GetByClaimedUserDomainAsync to GetByVerifiedUserEmailDomainAsync and refactor to return a list. Remove ManagedByOrganizationId from ProfileResponseMode. Add ManagesActiveUser to ProfileOrganizationResponseModel * Rename the property ManagesActiveUser to UserIsManagedByOrganization * Remove whole class #nullable enable and add it to specific places * [PM-11405] Account Deprovisioning: Prevent a verified user from changing their email address * Remove unnecessary .ToList() * Refactor IUserService methods GetOrganizationsManagingUserAsync and IsManagedByAnyOrganizationAsync to not return nullable objects. Update ProfileOrganizationResponseModel.UserIsManagedByOrganization to not be nullable * Update error message when unable to purge vault for managed account * Update error message when unable to change email for managed account * Update expected error messages on unit tests * Add TestFeatureService to Api.IntegrationTest.Helpers and use it on ApiApplicationFactory to be able to enable specific features for each test * Add CreateVerifiedDomainAsync method to OrganizationTestHelpers * Add tests to AccountsControllerTest to prevent changing email for managed accounts * Remove setting the feature flag value in ApiApplicationFactory and set it on AccountsControllerTest * Remove TestFeatureService class from Api.IntegrationTest.Helpers
This commit is contained in:
parent
cc6e41b42a
commit
c126fee296
@ -148,6 +148,13 @@ public class AccountsController : Controller
|
|||||||
throw new BadRequestException("MasterPasswordHash", "Invalid password.");
|
throw new BadRequestException("MasterPasswordHash", "Invalid password.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If Account Deprovisioning is enabled, we need to check if the user is managed by any organization.
|
||||||
|
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
|
||||||
|
&& await _userService.IsManagedByAnyOrganizationAsync(user.Id))
|
||||||
|
{
|
||||||
|
throw new BadRequestException("Cannot change emails for accounts owned by an organization. Contact your organization administrator for additional details.");
|
||||||
|
}
|
||||||
|
|
||||||
await _userService.InitiateEmailChangeAsync(user, model.NewEmail);
|
await _userService.InitiateEmailChangeAsync(user, model.NewEmail);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -165,6 +172,13 @@ public class AccountsController : Controller
|
|||||||
throw new BadRequestException("You cannot change your email when using Key Connector.");
|
throw new BadRequestException("You cannot change your email when using Key Connector.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If Account Deprovisioning is enabled, we need to check if the user is managed by any organization.
|
||||||
|
if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
|
||||||
|
&& await _userService.IsManagedByAnyOrganizationAsync(user.Id))
|
||||||
|
{
|
||||||
|
throw new BadRequestException("Cannot change emails for accounts owned by an organization. Contact your organization administrator for additional details.");
|
||||||
|
}
|
||||||
|
|
||||||
var result = await _userService.ChangeEmailAsync(user, model.MasterPasswordHash, model.NewEmail,
|
var result = await _userService.ChangeEmailAsync(user, model.MasterPasswordHash, model.NewEmail,
|
||||||
model.NewMasterPasswordHash, model.Token, model.Key);
|
model.NewMasterPasswordHash, model.Token, model.Key);
|
||||||
if (result.Succeeded)
|
if (result.Succeeded)
|
||||||
|
@ -1,6 +1,15 @@
|
|||||||
using System.Net.Http.Headers;
|
using System.Net;
|
||||||
|
using System.Net.Http.Headers;
|
||||||
|
using Bit.Api.Auth.Models.Request.Accounts;
|
||||||
using Bit.Api.IntegrationTest.Factories;
|
using Bit.Api.IntegrationTest.Factories;
|
||||||
|
using Bit.Api.IntegrationTest.Helpers;
|
||||||
using Bit.Api.Models.Response;
|
using Bit.Api.Models.Response;
|
||||||
|
using Bit.Core;
|
||||||
|
using Bit.Core.Billing.Enums;
|
||||||
|
using Bit.Core.Enums;
|
||||||
|
using Bit.Core.Models.Data;
|
||||||
|
using Bit.Core.Services;
|
||||||
|
using NSubstitute;
|
||||||
using Xunit;
|
using Xunit;
|
||||||
|
|
||||||
namespace Bit.Api.IntegrationTest.Controllers;
|
namespace Bit.Api.IntegrationTest.Controllers;
|
||||||
@ -35,4 +44,82 @@ public class AccountsControllerTest : IClassFixture<ApiApplicationFactory>
|
|||||||
Assert.Null(content.PrivateKey);
|
Assert.Null(content.PrivateKey);
|
||||||
Assert.NotNull(content.SecurityStamp);
|
Assert.NotNull(content.SecurityStamp);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmailToken_WhenAccountDeprovisioningEnabled_WithManagedAccount_ThrowsBadRequest()
|
||||||
|
{
|
||||||
|
var email = await SetupOrganizationManagedAccount();
|
||||||
|
|
||||||
|
var tokens = await _factory.LoginAsync(email);
|
||||||
|
var client = _factory.CreateClient();
|
||||||
|
|
||||||
|
var model = new EmailTokenRequestModel
|
||||||
|
{
|
||||||
|
NewEmail = $"{Guid.NewGuid()}@example.com",
|
||||||
|
MasterPasswordHash = "master_password_hash"
|
||||||
|
};
|
||||||
|
|
||||||
|
using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/email-token")
|
||||||
|
{
|
||||||
|
Content = JsonContent.Create(model)
|
||||||
|
};
|
||||||
|
message.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokens.Token);
|
||||||
|
var response = await client.SendAsync(message);
|
||||||
|
|
||||||
|
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||||
|
var content = await response.Content.ReadAsStringAsync();
|
||||||
|
Assert.Contains("Cannot change emails for accounts owned by an organization", content);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmail_WhenAccountDeprovisioningEnabled_WithManagedAccount_ThrowsBadRequest()
|
||||||
|
{
|
||||||
|
var email = await SetupOrganizationManagedAccount();
|
||||||
|
|
||||||
|
var tokens = await _factory.LoginAsync(email);
|
||||||
|
var client = _factory.CreateClient();
|
||||||
|
|
||||||
|
var model = new EmailRequestModel
|
||||||
|
{
|
||||||
|
NewEmail = $"{Guid.NewGuid()}@example.com",
|
||||||
|
MasterPasswordHash = "master_password_hash",
|
||||||
|
NewMasterPasswordHash = "master_password_hash",
|
||||||
|
Token = "validtoken",
|
||||||
|
Key = "key"
|
||||||
|
};
|
||||||
|
|
||||||
|
using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/email")
|
||||||
|
{
|
||||||
|
Content = JsonContent.Create(model)
|
||||||
|
};
|
||||||
|
message.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokens.Token);
|
||||||
|
var response = await client.SendAsync(message);
|
||||||
|
|
||||||
|
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||||
|
var content = await response.Content.ReadAsStringAsync();
|
||||||
|
Assert.Contains("Cannot change emails for accounts owned by an organization", content);
|
||||||
|
}
|
||||||
|
|
||||||
|
private async Task<string> SetupOrganizationManagedAccount()
|
||||||
|
{
|
||||||
|
_factory.SubstituteService<IFeatureService>(featureService =>
|
||||||
|
featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true));
|
||||||
|
|
||||||
|
// Create the owner account
|
||||||
|
var ownerEmail = $"{Guid.NewGuid()}@bitwarden.com";
|
||||||
|
await _factory.LoginWithNewAccount(ownerEmail);
|
||||||
|
|
||||||
|
// Create the organization
|
||||||
|
var (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, plan: PlanType.EnterpriseAnnually2023,
|
||||||
|
ownerEmail: ownerEmail, passwordManagerSeats: 10, paymentMethod: PaymentMethodType.Card);
|
||||||
|
|
||||||
|
// Create a new organization member
|
||||||
|
var (email, orgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, _organization.Id,
|
||||||
|
OrganizationUserType.Custom, new Permissions { AccessReports = true, ManageScim = true });
|
||||||
|
|
||||||
|
// Add a verified domain
|
||||||
|
await OrganizationTestHelpers.CreateVerifiedDomainAsync(_factory, _organization.Id, "bitwarden.com");
|
||||||
|
|
||||||
|
return email;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -105,4 +105,22 @@ public static class OrganizationTestHelpers
|
|||||||
|
|
||||||
return (email, organizationUser);
|
return (email, organizationUser);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates a VerifiedDomain for the specified organization.
|
||||||
|
/// </summary>
|
||||||
|
public static async Task CreateVerifiedDomainAsync(ApiApplicationFactory factory, Guid organizationId, string domain)
|
||||||
|
{
|
||||||
|
var organizationDomainRepository = factory.GetService<IOrganizationDomainRepository>();
|
||||||
|
|
||||||
|
var verifiedDomain = new OrganizationDomain
|
||||||
|
{
|
||||||
|
OrganizationId = organizationId,
|
||||||
|
DomainName = domain,
|
||||||
|
Txt = "btw+test18383838383"
|
||||||
|
};
|
||||||
|
verifiedDomain.SetVerifiedDate();
|
||||||
|
|
||||||
|
await organizationDomainRepository.CreateAsync(verifiedDomain);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -7,6 +7,7 @@ using Bit.Api.Auth.Models.Request.WebAuthn;
|
|||||||
using Bit.Api.Auth.Validators;
|
using Bit.Api.Auth.Validators;
|
||||||
using Bit.Api.Tools.Models.Request;
|
using Bit.Api.Tools.Models.Request;
|
||||||
using Bit.Api.Vault.Models.Request;
|
using Bit.Api.Vault.Models.Request;
|
||||||
|
using Bit.Core;
|
||||||
using Bit.Core.AdminConsole.Repositories;
|
using Bit.Core.AdminConsole.Repositories;
|
||||||
using Bit.Core.AdminConsole.Services;
|
using Bit.Core.AdminConsole.Services;
|
||||||
using Bit.Core.Auth.Entities;
|
using Bit.Core.Auth.Entities;
|
||||||
@ -143,6 +144,21 @@ public class AccountsControllerTests : IDisposable
|
|||||||
await _userService.Received(1).InitiateEmailChangeAsync(user, newEmail);
|
await _userService.Received(1).InitiateEmailChangeAsync(user, newEmail);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmailToken_WithAccountDeprovisioningEnabled_WhenUserIsNotManagedByAnOrganization_ShouldInitiateEmailChange()
|
||||||
|
{
|
||||||
|
var user = GenerateExampleUser();
|
||||||
|
ConfigureUserServiceToReturnValidPrincipalFor(user);
|
||||||
|
ConfigureUserServiceToAcceptPasswordFor(user);
|
||||||
|
_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true);
|
||||||
|
_userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(false);
|
||||||
|
var newEmail = "example@user.com";
|
||||||
|
|
||||||
|
await _sut.PostEmailToken(new EmailTokenRequestModel { NewEmail = newEmail });
|
||||||
|
|
||||||
|
await _userService.Received(1).InitiateEmailChangeAsync(user, newEmail);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task PostEmailToken_WhenNotAuthorized_ShouldThrowUnauthorizedAccessException()
|
public async Task PostEmailToken_WhenNotAuthorized_ShouldThrowUnauthorizedAccessException()
|
||||||
{
|
{
|
||||||
@ -165,6 +181,22 @@ public class AccountsControllerTests : IDisposable
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmailToken_WithAccountDeprovisioningEnabled_WhenUserIsManagedByAnOrganization_ShouldThrowBadRequestException()
|
||||||
|
{
|
||||||
|
var user = GenerateExampleUser();
|
||||||
|
ConfigureUserServiceToReturnValidPrincipalFor(user);
|
||||||
|
ConfigureUserServiceToAcceptPasswordFor(user);
|
||||||
|
_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true);
|
||||||
|
_userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(true);
|
||||||
|
|
||||||
|
var result = await Assert.ThrowsAsync<BadRequestException>(
|
||||||
|
() => _sut.PostEmailToken(new EmailTokenRequestModel())
|
||||||
|
);
|
||||||
|
|
||||||
|
Assert.Equal("Cannot change emails for accounts owned by an organization. Contact your organization administrator for additional details.", result.Message);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task PostEmail_ShouldChangeUserEmail()
|
public async Task PostEmail_ShouldChangeUserEmail()
|
||||||
{
|
{
|
||||||
@ -178,6 +210,21 @@ public class AccountsControllerTests : IDisposable
|
|||||||
await _userService.Received(1).ChangeEmailAsync(user, default, default, default, default, default);
|
await _userService.Received(1).ChangeEmailAsync(user, default, default, default, default, default);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmail_WithAccountDeprovisioningEnabled_WhenUserIsNotManagedByAnOrganization_ShouldChangeUserEmail()
|
||||||
|
{
|
||||||
|
var user = GenerateExampleUser();
|
||||||
|
ConfigureUserServiceToReturnValidPrincipalFor(user);
|
||||||
|
_userService.ChangeEmailAsync(user, default, default, default, default, default)
|
||||||
|
.Returns(Task.FromResult(IdentityResult.Success));
|
||||||
|
_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true);
|
||||||
|
_userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(false);
|
||||||
|
|
||||||
|
await _sut.PostEmail(new EmailRequestModel());
|
||||||
|
|
||||||
|
await _userService.Received(1).ChangeEmailAsync(user, default, default, default, default, default);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task PostEmail_WhenNotAuthorized_ShouldThrownUnauthorizedAccessException()
|
public async Task PostEmail_WhenNotAuthorized_ShouldThrownUnauthorizedAccessException()
|
||||||
{
|
{
|
||||||
@ -201,6 +248,21 @@ public class AccountsControllerTests : IDisposable
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PostEmail_WithAccountDeprovisioningEnabled_WhenUserIsManagedByAnOrganization_ShouldThrowBadRequestException()
|
||||||
|
{
|
||||||
|
var user = GenerateExampleUser();
|
||||||
|
ConfigureUserServiceToReturnValidPrincipalFor(user);
|
||||||
|
_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true);
|
||||||
|
_userService.IsManagedByAnyOrganizationAsync(user.Id).Returns(true);
|
||||||
|
|
||||||
|
var result = await Assert.ThrowsAsync<BadRequestException>(
|
||||||
|
() => _sut.PostEmail(new EmailRequestModel())
|
||||||
|
);
|
||||||
|
|
||||||
|
Assert.Equal("Cannot change emails for accounts owned by an organization. Contact your organization administrator for additional details.", result.Message);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task PostVerifyEmail_ShouldSendEmailVerification()
|
public async Task PostVerifyEmail_ShouldSendEmailVerification()
|
||||||
{
|
{
|
||||||
|
Loading…
Reference in New Issue
Block a user