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-14243] Free organization limit is not enforced when editing user #5155

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
๏ปฟ#nullable enable
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Billing.Enums;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
Expand Down Expand Up @@ -72,6 +73,22 @@
throw new NotFoundException();
}

var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId);
if (organization == null)
{
throw new NotFoundException();

Check warning on line 79 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs#L78-L79

Added lines #L78 - L79 were not covered by tests
}

if (organization.PlanType == PlanType.Free && user.Type is OrganizationUserType.Admin or OrganizationUserType.Owner)
{
// Since free organizations only supports a few users there is not much point in avoiding N+1 queries for this.
var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(user.Id);
if (adminCount > 0)
{
throw new BadRequestException("User can only be an admin of one free organization.");
}
}

if (collectionAccess?.Any() == true)
{
await ValidateCollectionAccessAsync(originalUser, collectionAccess.ToList());
Expand Down Expand Up @@ -111,14 +128,13 @@
var additionalSmSeatsRequired = await _countNewSmSeatsRequiredQuery.CountNewSmSeatsRequiredAsync(user.OrganizationId, 1);
if (additionalSmSeatsRequired > 0)
{
var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId);
var update = new SecretsManagerSubscriptionUpdate(organization, true)
.AdjustSeats(additionalSmSeatsRequired);
.AdjustSeats(additionalSmSeatsRequired);

Check warning on line 132 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs#L132

Added line #L132 was not covered by tests
await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update);
}
}

await _organizationUserRepository.ReplaceAsync(user, collectionAccess);

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (win-x64)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (osx-x64)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build MSSQL migrator utility (linux-x64)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Setup, ./util)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Billing, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (MsSqlMigratorUtility, ./util, true)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Notifications, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Identity, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Sso, ./bitwarden_license/src, true)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Events, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Icons, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (EventsProcessor, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Api, ./src)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Scim, ./bitwarden_license/src, true)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Quality scan

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Admin, ./src, true)

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Run tests

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

Check warning on line 137 in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs

View workflow job for this annotation

GitHub Actions / Upload

Possible null reference argument for parameter 'collections' in 'Task IOrganizationUserRepository.ReplaceAsync(OrganizationUser obj, IEnumerable<CollectionAccessSelection> collections)'.

if (groupAccess != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Billing.Enums;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
Expand Down Expand Up @@ -182,6 +183,29 @@ public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsB
exception.Message);
}

[Theory]
[BitAutoData(OrganizationUserType.Admin)]
[BitAutoData(OrganizationUserType.Owner)]
public async Task AcceptOrgUser_AdminOfFreePlanTryingToJoinSecondFreeOrg_ThrowsBadRequest(
OrganizationUserType userType,
SutProvider<AcceptOrgUserCommand> sutProvider,
User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails)
{
// Arrange
SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails);
org.PlanType = PlanType.Free;
orgUser.Type = userType;

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetCountByFreeOrganizationAdminUserAsync(user.Id)
.Returns(1);

// Act & Assert
var exception = await Assert.ThrowsAsync<BadRequestException>(() =>
sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService));

Assert.Equal("You can only be an admin of one free organization.", exception.Message);
}

// AcceptOrgUserByOrgIdAsync tests --------------------------------------------------------------------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Billing.Enums;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
Expand Down Expand Up @@ -144,6 +145,7 @@ public async Task UpdateUserAsync_Passes(
newUserData.Id = oldUserData.Id;
newUserData.UserId = oldUserData.UserId;
newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id;
newUserData.Type = OrganizationUserType.Admin;
newUserData.Permissions = JsonSerializer.Serialize(permissions, new JsonSerializerOptions
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
Expand All @@ -159,6 +161,10 @@ public async Task UpdateUserAsync_Passes(
.Returns(callInfo => callInfo.Arg<IEnumerable<Guid>>()
.Select(guid => new Group { Id = guid, OrganizationId = oldUserData.OrganizationId }).ToList());

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetCountByFreeOrganizationAdminUserAsync(newUserData.Id)
.Returns(0);

await sutProvider.Sut.UpdateUserAsync(newUserData, savingUser.UserId, collections, groups);

var organizationService = sutProvider.GetDependency<IOrganizationService>();
Expand All @@ -175,6 +181,31 @@ await sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>().Received(1).Ha
Arg.Is<IEnumerable<Guid>>(i => i.Contains(newUserData.Id)));
}

[Theory]
[BitAutoData(OrganizationUserType.Admin)]
[BitAutoData(OrganizationUserType.Owner)]
public async Task UpdateUserAsync_WhenUpdatingUserToAdminOrOwner_WithUserAlreadyAdminOfAnotherFreeOrganization_Throws(
OrganizationUserType userType,
OrganizationUser oldUserData,
OrganizationUser newUserData,
Organization organization,
SutProvider<UpdateOrganizationUserCommand> sutProvider)
{
organization.PlanType = PlanType.Free;
newUserData.Type = userType;

Setup(sutProvider, organization, newUserData, oldUserData);

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetCountByFreeOrganizationAdminUserAsync(newUserData.Id)
.Returns(1);

// Assert
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.UpdateUserAsync(newUserData, null, null, null));
Assert.Contains("User can only be an admin of one free organization.", exception.Message);
}

private void Setup(SutProvider<UpdateOrganizationUserCommand> sutProvider, Organization organization,
OrganizationUser newUser, OrganizationUser oldUser)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,27 @@ public async Task SignUpAsync_InvalidateServiceAccount_ShouldThrowException(Orga
() => sutProvider.Sut.SignUpOrganizationAsync(signup));
Assert.Contains("You can't subtract Machine Accounts!", exception.Message);
}

[Theory]
[BitAutoData]
public async Task SignUpAsync_Free_ExistingFreeOrgAdmin_ThrowsBadRequest(
SutProvider<CloudOrganizationSignUpCommand> sutProvider)
{
// Arrange
var signup = new OrganizationSignup
{
Plan = PlanType.Free,
IsFromProvider = false,
Owner = new User { Id = Guid.NewGuid() }
};

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetCountByFreeOrganizationAdminUserAsync(signup.Owner.Id)
.Returns(1);

// Act & Assert
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.SignUpOrganizationAsync(signup));
Assert.Contains("You can only be an admin of one free organization.", exception.Message);
}
}
Loading