1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00

[AC-2206] Fix assigning Manage access to default collection (#3799)

* Fix assigning Manage access to default collection

The previous implementation did not work when creating an org as a
provider because the ownerId is null in OrganizationService.SignUp.
Added a null check and handled assigning access in ProviderService
instead.

* Tweaks
This commit is contained in:
Thomas Rittson 2024-02-15 00:41:51 +10:00 committed by GitHub
parent 97018e2501
commit a07aa8330c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 49 additions and 23 deletions

View File

@ -496,7 +496,7 @@ public class ProviderService : IProviderService
{
ThrowOnInvalidPlanType(organizationSignup.Plan);
var (organization, _) = await _organizationService.SignUpAsync(organizationSignup, true);
var (organization, _, defaultCollection) = await _organizationService.SignUpAsync(organizationSignup, true);
var providerOrganization = new ProviderOrganization
{
@ -508,6 +508,21 @@ public class ProviderService : IProviderService
await _providerOrganizationRepository.CreateAsync(providerOrganization);
await _eventService.LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_Created);
// If using Flexible Collections, give the owner Can Manage access over the default collection
// The orgUser is not available when the org is created so we have to do it here as part of the invite
var defaultOwnerAccess = organization.FlexibleCollections && defaultCollection != null
?
[
new CollectionAccessSelection
{
Id = defaultCollection.Id,
HidePasswords = false,
ReadOnly = false,
Manage = true
}
]
: Array.Empty<CollectionAccessSelection>();
await _organizationService.InviteUsersAsync(organization.Id, user.Id,
new (OrganizationUserInvite, string)[]
{
@ -521,7 +536,7 @@ public class ProviderService : IProviderService
AccessAll = !organization.FlexibleCollections,
Type = OrganizationUserType.Owner,
Permissions = null,
Collections = Array.Empty<CollectionAccessSelection>(),
Collections = defaultOwnerAccess,
},
null
)

View File

@ -523,7 +523,7 @@ public class ProviderServiceTests
sutProvider.GetDependency<IProviderRepository>().GetByIdAsync(provider.Id).Returns(provider);
var providerOrganizationRepository = sutProvider.GetDependency<IProviderOrganizationRepository>();
sutProvider.GetDependency<IOrganizationService>().SignUpAsync(organizationSignup, true)
.Returns(Tuple.Create(organization, null as OrganizationUser));
.Returns((organization, null as OrganizationUser, new Collection()));
var providerOrganization =
await sutProvider.Sut.CreateOrganizationAsync(provider.Id, organizationSignup, clientOwnerEmail, user);
@ -539,20 +539,21 @@ public class ProviderServiceTests
t.First().Item1.Emails.First() == clientOwnerEmail &&
t.First().Item1.Type == OrganizationUserType.Owner &&
t.First().Item1.AccessAll &&
!t.First().Item1.Collections.Any() &&
t.First().Item2 == null));
}
[Theory, OrganizationCustomize(FlexibleCollections = true), BitAutoData]
public async Task CreateOrganizationAsync_WithFlexibleCollections_SetsAccessAllToFalse
(Provider provider, OrganizationSignup organizationSignup, Organization organization, string clientOwnerEmail,
User user, SutProvider<ProviderService> sutProvider)
User user, SutProvider<ProviderService> sutProvider, Collection defaultCollection)
{
organizationSignup.Plan = PlanType.EnterpriseAnnually;
sutProvider.GetDependency<IProviderRepository>().GetByIdAsync(provider.Id).Returns(provider);
var providerOrganizationRepository = sutProvider.GetDependency<IProviderOrganizationRepository>();
sutProvider.GetDependency<IOrganizationService>().SignUpAsync(organizationSignup, true)
.Returns(Tuple.Create(organization, null as OrganizationUser));
.Returns((organization, null as OrganizationUser, defaultCollection));
var providerOrganization =
await sutProvider.Sut.CreateOrganizationAsync(provider.Id, organizationSignup, clientOwnerEmail, user);
@ -568,6 +569,10 @@ public class ProviderServiceTests
t.First().Item1.Emails.First() == clientOwnerEmail &&
t.First().Item1.Type == OrganizationUserType.Owner &&
t.First().Item1.AccessAll == false &&
t.First().Item1.Collections.Single().Id == defaultCollection.Id &&
!t.First().Item1.Collections.Single().HidePasswords &&
!t.First().Item1.Collections.Single().ReadOnly &&
t.First().Item1.Collections.Single().Manage &&
t.First().Item2 == null));
}

View File

@ -20,8 +20,17 @@ public interface IOrganizationService
Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null);
Task<string> AdjustSeatsAsync(Guid organizationId, int seatAdjustment, DateTime? prorationDate = null);
Task VerifyBankAsync(Guid organizationId, int amount1, int amount2);
Task<Tuple<Organization, OrganizationUser>> SignUpAsync(OrganizationSignup organizationSignup, bool provider = false);
Task<Tuple<Organization, OrganizationUser>> SignUpAsync(OrganizationLicense license, User owner,
/// <summary>
/// Create a new organization in a cloud environment
/// </summary>
/// <returns>A tuple containing the new organization, the initial organizationUser (if any) and the default collection (if any)</returns>
#nullable enable
Task<(Organization organization, OrganizationUser? organizationUser, Collection? defaultCollection)> SignUpAsync(OrganizationSignup organizationSignup, bool provider = false);
#nullable disable
/// <summary>
/// Create a new organization on a self-hosted instance
/// </summary>
Task<(Organization organization, OrganizationUser organizationUser)> SignUpAsync(OrganizationLicense license, User owner,
string ownerKey, string collectionName, string publicKey, string privateKey);
Task DeleteAsync(Organization organization);
Task EnableAsync(Guid organizationId, DateTime? expirationDate);

View File

@ -424,7 +424,7 @@ public class OrganizationService : IOrganizationService
/// <summary>
/// Create a new organization in a cloud environment
/// </summary>
public async Task<Tuple<Organization, OrganizationUser>> SignUpAsync(OrganizationSignup signup,
public async Task<(Organization organization, OrganizationUser organizationUser, Collection defaultCollection)> SignUpAsync(OrganizationSignup signup,
bool provider = false)
{
var plan = StaticStore.GetPlan(signup.Plan);
@ -552,7 +552,7 @@ public class OrganizationService : IOrganizationService
/// <summary>
/// Create a new organization on a self-hosted instance
/// </summary>
public async Task<Tuple<Organization, OrganizationUser>> SignUpAsync(
public async Task<(Organization organization, OrganizationUser organizationUser)> SignUpAsync(
OrganizationLicense license, User owner, string ownerKey, string collectionName, string publicKey,
string privateKey)
{
@ -633,14 +633,14 @@ public class OrganizationService : IOrganizationService
Directory.CreateDirectory(dir);
await using var fs = new FileStream(Path.Combine(dir, $"{organization.Id}.json"), FileMode.Create);
await JsonSerializer.SerializeAsync(fs, license, JsonHelpers.Indented);
return result;
return (result.organization, result.organizationUser);
}
/// <summary>
/// Private helper method to create a new organization.
/// This is common code used by both the cloud and self-hosted methods.
/// </summary>
private async Task<Tuple<Organization, OrganizationUser>> SignUpAsync(Organization organization,
private async Task<(Organization organization, OrganizationUser organizationUser, Collection defaultCollection)> SignUpAsync(Organization organization,
Guid ownerId, string ownerKey, string collectionName, bool withPayment)
{
try
@ -655,6 +655,8 @@ public class OrganizationService : IOrganizationService
});
await _applicationCacheService.UpsertOrganizationAbilityAsync(organization);
// ownerId == default if the org is created by a provider - in this case it's created without an
// owner and the first owner is immediately invited afterwards
OrganizationUser orgUser = null;
if (ownerId != default)
{
@ -683,9 +685,10 @@ public class OrganizationService : IOrganizationService
await _pushNotificationService.PushSyncOrgKeysAsync(ownerId);
}
Collection defaultCollection = null;
if (!string.IsNullOrWhiteSpace(collectionName))
{
var defaultCollection = new Collection
defaultCollection = new Collection
{
Name = collectionName,
OrganizationId = organization.Id,
@ -695,7 +698,7 @@ public class OrganizationService : IOrganizationService
// If using Flexible Collections, give the owner Can Manage access over the default collection
List<CollectionAccessSelection> defaultOwnerAccess = null;
if (organization.FlexibleCollections)
if (orgUser != null && organization.FlexibleCollections)
{
defaultOwnerAccess =
[new CollectionAccessSelection { Id = orgUser.Id, HidePasswords = false, ReadOnly = false, Manage = true }];
@ -704,7 +707,7 @@ public class OrganizationService : IOrganizationService
await _collectionRepository.CreateAsync(defaultCollection, null, defaultOwnerAccess);
}
return new Tuple<Organization, OrganizationUser>(organization, orgUser);
return (organization, orgUser, defaultCollection);
}
catch
{

View File

@ -22,7 +22,7 @@ public static class OrganizationTestHelpers
var owner = await userRepository.GetByEmailAsync(ownerEmail);
return await organizationService.SignUpAsync(new OrganizationSignup
var signUpResult = await organizationService.SignUpAsync(new OrganizationSignup
{
Name = name,
BillingEmail = billingEmail,
@ -30,6 +30,8 @@ public static class OrganizationTestHelpers
OwnerKey = ownerKey,
Owner = owner,
});
return new Tuple<Organization, OrganizationUser>(signUpResult.organization, signUpResult.organizationUser);
}
public static async Task<OrganizationUser> CreateUserAsync<T>(

View File

@ -232,10 +232,8 @@ public class OrganizationServiceTests
referenceEvent.Storage == result.Item1.MaxStorageGb));
// TODO: add reference events for SmSeats and Service Accounts - see AC-1481
Assert.NotNull(result);
Assert.NotNull(result.Item1);
Assert.NotNull(result.Item2);
Assert.IsType<Tuple<Organization, OrganizationUser>>(result);
await sutProvider.GetDependency<IPaymentService>().Received(1).PurchaseOrganizationAsync(
Arg.Any<Organization>(),
@ -294,10 +292,8 @@ public class OrganizationServiceTests
!c.HidePasswords &&
c.Manage)));
Assert.NotNull(result);
Assert.NotNull(result.Item1);
Assert.NotNull(result.Item2);
Assert.IsType<Tuple<Organization, OrganizationUser>>(result);
}
[Theory]
@ -323,10 +319,8 @@ public class OrganizationServiceTests
o.UserId == signup.Owner.Id &&
o.AccessAll == true));
Assert.NotNull(result);
Assert.NotNull(result.Item1);
Assert.NotNull(result.Item2);
Assert.IsType<Tuple<Organization, OrganizationUser>>(result);
}
[Theory]
@ -367,10 +361,8 @@ public class OrganizationServiceTests
referenceEvent.Storage == result.Item1.MaxStorageGb));
// TODO: add reference events for SmSeats and Service Accounts - see AC-1481
Assert.NotNull(result);
Assert.NotNull(result.Item1);
Assert.NotNull(result.Item2);
Assert.IsType<Tuple<Organization, OrganizationUser>>(result);
await sutProvider.GetDependency<IPaymentService>().Received(1).PurchaseOrganizationAsync(
Arg.Any<Organization>(),