From a07aa8330c60f8199bda16805840ab5763bc0092 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Thu, 15 Feb 2024 00:41:51 +1000 Subject: [PATCH] [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 --- .../AdminConsole/Services/ProviderService.cs | 19 +++++++++++++++++-- .../Services/ProviderServiceTests.cs | 11 ++++++++--- .../Services/IOrganizationService.cs | 13 +++++++++++-- .../Implementations/OrganizationService.cs | 17 ++++++++++------- .../Helpers/OrganizationTestHelpers.cs | 4 +++- .../Services/OrganizationServiceTests.cs | 8 -------- 6 files changed, 49 insertions(+), 23 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs index b6d53c0ada..e2e5437a55 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs @@ -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(); + 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(), + Collections = defaultOwnerAccess, }, null ) diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs index f9b11cad41..b503d0d5a7 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs @@ -523,7 +523,7 @@ public class ProviderServiceTests sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); sutProvider.GetDependency().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 sutProvider) + User user, SutProvider sutProvider, Collection defaultCollection) { organizationSignup.Plan = PlanType.EnterpriseAnnually; sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); sutProvider.GetDependency().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)); } diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index 768edea9a3..77c92ec4df 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -20,8 +20,17 @@ public interface IOrganizationService Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null); Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment, DateTime? prorationDate = null); Task VerifyBankAsync(Guid organizationId, int amount1, int amount2); - Task> SignUpAsync(OrganizationSignup organizationSignup, bool provider = false); - Task> SignUpAsync(OrganizationLicense license, User owner, + /// + /// Create a new organization in a cloud environment + /// + /// A tuple containing the new organization, the initial organizationUser (if any) and the default collection (if any) +#nullable enable + Task<(Organization organization, OrganizationUser? organizationUser, Collection? defaultCollection)> SignUpAsync(OrganizationSignup organizationSignup, bool provider = false); +#nullable disable + /// + /// Create a new organization on a self-hosted instance + /// + 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); diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 7d0907b0ba..6d547badc6 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -424,7 +424,7 @@ public class OrganizationService : IOrganizationService /// /// Create a new organization in a cloud environment /// - public async Task> 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 /// /// Create a new organization on a self-hosted instance /// - public async Task> 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); } /// /// Private helper method to create a new organization. /// This is common code used by both the cloud and self-hosted methods. /// - private async Task> 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 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, orgUser); + return (organization, orgUser, defaultCollection); } catch { diff --git a/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs b/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs index 39fe8a99d0..2144c6301f 100644 --- a/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs +++ b/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs @@ -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(signUpResult.organization, signUpResult.organizationUser); } public static async Task CreateUserAsync( diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 4ad13d8fd6..a713935dd3 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -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>(result); await sutProvider.GetDependency().Received(1).PurchaseOrganizationAsync( Arg.Any(), @@ -294,10 +292,8 @@ public class OrganizationServiceTests !c.HidePasswords && c.Manage))); - Assert.NotNull(result); Assert.NotNull(result.Item1); Assert.NotNull(result.Item2); - Assert.IsType>(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>(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>(result); await sutProvider.GetDependency().Received(1).PurchaseOrganizationAsync( Arg.Any(),