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-11405] Account Management: Prevent a verified user from changing their email address #4875

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8aa3915
Add check for managed user before purging account
r-tome Oct 4, 2024
2654a17
Merge branch 'main' into ac/pm-11404/prevent-a-verified-user-from-purโ€ฆ
r-tome Oct 7, 2024
fdb4a2c
Rename IOrganizationRepository.GetByClaimedUserDomainAsync to GetByVeโ€ฆ
r-tome Oct 8, 2024
c8da05d
Merge branch 'main' into ac/pm-11404/prevent-a-verified-user-from-purโ€ฆ
r-tome Oct 8, 2024
0c6107b
Rename the property ManagesActiveUser to UserIsManagedByOrganization
r-tome Oct 9, 2024
6d1859f
Remove whole class #nullable enable and add it to specific places
r-tome Oct 9, 2024
6decb51
[PM-11405] Account Deprovisioning: Prevent a verified user from changโ€ฆ
r-tome Oct 10, 2024
d6a3a10
Remove unnecessary .ToList()
r-tome Oct 10, 2024
ee24a91
Refactor IUserService methods GetOrganizationsManagingUserAsync and Iโ€ฆ
r-tome Oct 11, 2024
4520a1f
Merge branch 'ac/pm-11404/prevent-a-verified-user-from-purging-their-โ€ฆ
r-tome Oct 11, 2024
0214d43
Merge branch 'main' into ac/pm-11404/prevent-a-verified-user-from-purโ€ฆ
r-tome Oct 15, 2024
cfec28e
Update error message when unable to purge vault for managed account
r-tome Oct 15, 2024
8cf01b7
Merge branch 'ac/pm-11404/prevent-a-verified-user-from-purging-their-โ€ฆ
r-tome Oct 15, 2024
7a1cdaa
Update error message when unable to change email for managed account
r-tome Oct 15, 2024
945e67e
Merge branch 'main' into ac/pm-11405/prevent-a-verified-user-from-chaโ€ฆ
r-tome Oct 18, 2024
6c9fe98
Update expected error messages on unit tests
r-tome Oct 18, 2024
e15d6c0
Merge branch 'main' into ac/pm-11405/prevent-a-verified-user-from-chaโ€ฆ
r-tome Oct 23, 2024
28947af
Add TestFeatureService to Api.IntegrationTest.Helpers and use it on Aโ€ฆ
r-tome Oct 23, 2024
4ae6abc
Add CreateVerifiedDomainAsync method to OrganizationTestHelpers
r-tome Oct 23, 2024
0d4db11
Add tests to AccountsControllerTest to prevent changing email for manโ€ฆ
r-tome Oct 23, 2024
8e2e790
Remove setting the feature flag value in ApiApplicationFactory and seโ€ฆ
r-tome Oct 24, 2024
e2c3b66
Remove TestFeatureService class from Api.IntegrationTest.Helpers
r-tome Oct 25, 2024
513e6c4
Merge branch 'main' into ac/pm-11405/prevent-a-verified-user-from-chaโ€ฆ
r-tome Oct 25, 2024
9e2de93
Merge branch 'main' into ac/pm-11405/prevent-a-verified-user-from-chaโ€ฆ
r-tome Oct 28, 2024
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
14 changes: 14 additions & 0 deletions src/Api/Auth/Controllers/AccountsController.cs
ike-kottlowski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ public async Task PostEmailToken([FromBody] EmailTokenRequestModel model)
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);
}

Expand All @@ -165,6 +172,13 @@ public async Task PostEmail([FromBody] EmailRequestModel model)
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,
model.NewMasterPasswordHash, model.Token, model.Key);
if (result.Succeeded)
Expand Down
89 changes: 88 additions & 1 deletion test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs
Original file line number Diff line number Diff line change
@@ -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.Helpers;
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;

namespace Bit.Api.IntegrationTest.Controllers;
Expand Down Expand Up @@ -35,4 +44,82 @@ public async Task GetPublicKey()
Assert.Null(content.PrivateKey);
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;
}
}
18 changes: 18 additions & 0 deletions test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,22 @@ public static async Task<OrganizationUser> CreateUserAsync<T>(

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);
}
}
62 changes: 62 additions & 0 deletions test/Api.Test/Auth/Controllers/AccountsControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Bit.Api.Auth.Validators;
using Bit.Api.Tools.Models.Request;
using Bit.Api.Vault.Models.Request;
using Bit.Core;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Entities;
Expand Down Expand Up @@ -143,6 +144,21 @@ public async Task PostEmailToken_ShouldInitiateEmailChange()
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 = "[email protected]";

await _sut.PostEmailToken(new EmailTokenRequestModel { NewEmail = newEmail });

await _userService.Received(1).InitiateEmailChangeAsync(user, newEmail);
}

[Fact]
public async Task PostEmailToken_WhenNotAuthorized_ShouldThrowUnauthorizedAccessException()
{
Expand All @@ -165,6 +181,22 @@ await Assert.ThrowsAsync<BadRequestException>(
);
}

[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]
public async Task PostEmail_ShouldChangeUserEmail()
{
Expand All @@ -178,6 +210,21 @@ public async Task PostEmail_ShouldChangeUserEmail()
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]
public async Task PostEmail_WhenNotAuthorized_ShouldThrownUnauthorizedAccessException()
{
Expand All @@ -201,6 +248,21 @@ await Assert.ThrowsAsync<BadRequestException>(
);
}

[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]
public async Task PostVerifyEmail_ShouldSendEmailVerification()
{
Expand Down
Loading