From cfc7fa071b6a6a0ee63471f89ce4928655803830 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 5 Aug 2021 08:50:41 -0400 Subject: [PATCH] Record when a provider user accesses a clients vault (#1496) * Record when a provider user accesses a clients vault * Do not allow removal from provider unless owner exists * PR Review * Null safe event processing * append `Async` to async methods --- .../src/CommCore/Services/ProviderService.cs | 29 +++++++++++++++++-- .../Services/ProviderServiceTests.cs | 12 ++++---- src/Api/Controllers/CiphersController.cs | 9 ++++++ .../ProviderOrganizationsController.cs | 2 +- src/Core/Enums/EventType.cs | 2 ++ src/Core/Services/IOrganizationService.cs | 2 +- src/Core/Services/IProviderService.cs | 3 +- .../Implementations/OrganizationService.cs | 9 ++++-- .../NoopProviderService.cs | 3 +- 9 files changed, 56 insertions(+), 15 deletions(-) diff --git a/bitwarden_license/src/CommCore/Services/ProviderService.cs b/bitwarden_license/src/CommCore/Services/ProviderService.cs index 081004444c..896c9204f4 100644 --- a/bitwarden_license/src/CommCore/Services/ProviderService.cs +++ b/bitwarden_license/src/CommCore/Services/ProviderService.cs @@ -27,6 +27,7 @@ namespace Bit.CommCore.Services private readonly IProviderRepository _providerRepository; private readonly IProviderUserRepository _providerUserRepository; private readonly IProviderOrganizationRepository _providerOrganizationRepository; + private readonly IOrganizationRepository _organizationRepository; private readonly IUserRepository _userRepository; private readonly IUserService _userService; private readonly IOrganizationService _organizationService; @@ -34,11 +35,13 @@ namespace Bit.CommCore.Services public ProviderService(IProviderRepository providerRepository, IProviderUserRepository providerUserRepository, IProviderOrganizationRepository providerOrganizationRepository, IUserRepository userRepository, IUserService userService, IOrganizationService organizationService, IMailService mailService, - IDataProtectionProvider dataProtectionProvider, IEventService eventService, GlobalSettings globalSettings) + IDataProtectionProvider dataProtectionProvider, IEventService eventService, + IOrganizationRepository organizationRepository, GlobalSettings globalSettings) { _providerRepository = providerRepository; _providerUserRepository = providerUserRepository; _providerOrganizationRepository = providerOrganizationRepository; + _organizationRepository = organizationRepository; _userRepository = userRepository; _userService = userService; _organizationService = organizationService; @@ -402,7 +405,7 @@ namespace Bit.CommCore.Services return providerOrganization; } - public async Task RemoveOrganization(Guid providerId, Guid providerOrganizationId, Guid removingUserId) + public async Task RemoveOrganizationAsync(Guid providerId, Guid providerOrganizationId, Guid removingUserId) { var providerOrganization = await _providerOrganizationRepository.GetByIdAsync(providerOrganizationId); if (providerOrganization == null || providerOrganization.ProviderId != providerId) @@ -410,7 +413,7 @@ namespace Bit.CommCore.Services throw new BadRequestException("Invalid organization."); } - if (!await _organizationService.HasConfirmedOwnersExceptAsync(providerOrganization.OrganizationId, new Guid[] {})) + if (!await _organizationService.HasConfirmedOwnersExceptAsync(providerOrganization.OrganizationId, new Guid[] { }, includeProvider: false)) { throw new BadRequestException("Organization needs to have at least one confirmed owner."); } @@ -419,6 +422,26 @@ namespace Bit.CommCore.Services await _eventService.LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_Removed); } + public async Task LogProviderAccessToOrganizationAsync(Guid organizationId) + { + if (organizationId == default) + { + return; + } + + var providerOrganization = await _providerOrganizationRepository.GetByOrganizationId(organizationId); + var organization = await _organizationRepository.GetByIdAsync(organizationId); + if (providerOrganization != null) + { + await _eventService.LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_VaultAccessed); + } + if (organization != null) + { + await _eventService.LogOrganizationEventAsync(organization, EventType.Organization_VaultAccessed); + } + } + + private async Task SendInviteAsync(ProviderUser providerUser, Provider provider) { var nowMillis = CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow); diff --git a/bitwarden_license/test/CmmCore.Test/Services/ProviderServiceTests.cs b/bitwarden_license/test/CmmCore.Test/Services/ProviderServiceTests.cs index 3798ae926e..1dd41a3e38 100644 --- a/bitwarden_license/test/CmmCore.Test/Services/ProviderServiceTests.cs +++ b/bitwarden_license/test/CmmCore.Test/Services/ProviderServiceTests.cs @@ -465,7 +465,7 @@ namespace Bit.CommCore.Test.Services .ReturnsNull(); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganization(provider.Id, providerOrganization.Id, user.Id)); + () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); Assert.Equal("Invalid organization.", exception.Message); } @@ -478,7 +478,7 @@ namespace Bit.CommCore.Test.Services .Returns(providerOrganization); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganization(provider.Id, providerOrganization.Id, user.Id)); + () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); Assert.Equal("Invalid organization.", exception.Message); } @@ -490,11 +490,11 @@ namespace Bit.CommCore.Test.Services sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); sutProvider.GetDependency().GetByIdAsync(providerOrganization.Id) .Returns(providerOrganization); - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default) + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default, default) .ReturnsForAnyArgs(false); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RemoveOrganization(provider.Id, providerOrganization.Id, user.Id)); + () => sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id)); Assert.Equal("Organization needs to have at least one confirmed owner.", exception.Message); } @@ -506,10 +506,10 @@ namespace Bit.CommCore.Test.Services sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); var providerOrganizationRepository = sutProvider.GetDependency(); providerOrganizationRepository.GetByIdAsync(providerOrganization.Id).Returns(providerOrganization); - sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default) + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync(default, default, default) .ReturnsForAnyArgs(true); - await sutProvider.Sut.RemoveOrganization(provider.Id, providerOrganization.Id, user.Id); + await sutProvider.Sut.RemoveOrganizationAsync(provider.Id, providerOrganization.Id, user.Id); await providerOrganizationRepository.Received().DeleteAsync(providerOrganization); await sutProvider.GetDependency().Received() .LogProviderOrganizationEventAsync(providerOrganization, EventType.ProviderOrganization_Removed); diff --git a/src/Api/Controllers/CiphersController.cs b/src/Api/Controllers/CiphersController.cs index 2419357281..5dbf10dee6 100644 --- a/src/Api/Controllers/CiphersController.cs +++ b/src/Api/Controllers/CiphersController.cs @@ -30,6 +30,7 @@ namespace Bit.Api.Controllers private readonly ICipherService _cipherService; private readonly IUserService _userService; private readonly IAttachmentStorageService _attachmentStorageService; + private readonly IProviderService _providerService; private readonly ICurrentContext _currentContext; private readonly ILogger _logger; private readonly GlobalSettings _globalSettings; @@ -40,6 +41,7 @@ namespace Bit.Api.Controllers ICipherService cipherService, IUserService userService, IAttachmentStorageService attachmentStorageService, + IProviderService providerService, ICurrentContext currentContext, ILogger logger, GlobalSettings globalSettings) @@ -49,6 +51,7 @@ namespace Bit.Api.Controllers _cipherService = cipherService; _userService = userService; _attachmentStorageService = attachmentStorageService; + _providerService = providerService; _currentContext = currentContext; _logger = logger; _globalSettings = globalSettings; @@ -224,6 +227,12 @@ namespace Bit.Api.Controllers var responses = ciphers.Select(c => new CipherMiniDetailsResponseModel(c, _globalSettings, collectionCiphersGroupDict)); + + var providerId = await _currentContext.ProviderIdForOrg(orgIdGuid); + if (providerId.HasValue) + { + await _providerService.LogProviderAccessToOrganizationAsync(orgIdGuid); + } return new ListResponseModel(responses); } diff --git a/src/Api/Controllers/ProviderOrganizationsController.cs b/src/Api/Controllers/ProviderOrganizationsController.cs index 347f8f38b7..326a0a86b7 100644 --- a/src/Api/Controllers/ProviderOrganizationsController.cs +++ b/src/Api/Controllers/ProviderOrganizationsController.cs @@ -91,7 +91,7 @@ namespace Bit.Api.Controllers } var userId = _userService.GetProperUserId(User); - await _providerService.RemoveOrganization(providerId, id, userId.Value); + await _providerService.RemoveOrganizationAsync(providerId, id, userId.Value); } } } diff --git a/src/Core/Enums/EventType.cs b/src/Core/Enums/EventType.cs index 6bbcc74f56..03c58bd8ad 100644 --- a/src/Core/Enums/EventType.cs +++ b/src/Core/Enums/EventType.cs @@ -52,6 +52,7 @@ Organization_Updated = 1600, Organization_PurgedVault = 1601, // Organization_ClientExportedVault = 1602, + Organization_VaultAccessed = 1603, Policy_Updated = 1700, @@ -63,5 +64,6 @@ ProviderOrganization_Created = 1900, ProviderOrganization_Added = 1901, ProviderOrganization_Removed = 1902, + ProviderOrganization_VaultAccessed = 1903, } } diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index e0c68430af..cbee1efdc3 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -59,6 +59,6 @@ namespace Bit.Core.Services Task RotateApiKeyAsync(Organization organization); Task DeleteSsoUserAsync(Guid userId, Guid? organizationId); Task UpdateOrganizationKeysAsync(Guid orgId, string publicKey, string privateKey); - Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId); + Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true); } } diff --git a/src/Core/Services/IProviderService.cs b/src/Core/Services/IProviderService.cs index d54f2a4099..8c2c10131e 100644 --- a/src/Core/Services/IProviderService.cs +++ b/src/Core/Services/IProviderService.cs @@ -27,6 +27,7 @@ namespace Bit.Core.Services Task AddOrganization(Guid providerId, Guid organizationId, Guid addingUserId, string key); Task CreateOrganizationAsync(Guid providerId, OrganizationSignup organizationSignup, string clientOwnerEmail, User user); - Task RemoveOrganization(Guid providerId, Guid providerOrganizationId, Guid removingUserId); + Task RemoveOrganizationAsync(Guid providerId, Guid providerOrganizationId, Guid removingUserId); + Task LogProviderAccessToOrganizationAsync(Guid organizationId); } } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index ea2e758937..4024c399e4 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1689,11 +1689,16 @@ namespace Bit.Core.Services return result; } - public async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId) + public async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnumerable organizationUsersId, bool includeProvider = true) { var confirmedOwners = await GetConfirmedOwnersAsync(organizationId); var confirmedOwnersIds = confirmedOwners.Select(u => u.Id); - return confirmedOwnersIds.Except(organizationUsersId).Any(); + bool hasOtherOwner = confirmedOwnersIds.Except(organizationUsersId).Any(); + if (!hasOtherOwner && includeProvider) + { + return (await _currentContext.ProviderIdForOrg(organizationId)).HasValue; + } + return hasOtherOwner; } public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) diff --git a/src/Core/Services/NoopImplementations/NoopProviderService.cs b/src/Core/Services/NoopImplementations/NoopProviderService.cs index cffea1e73e..53427a9e9a 100644 --- a/src/Core/Services/NoopImplementations/NoopProviderService.cs +++ b/src/Core/Services/NoopImplementations/NoopProviderService.cs @@ -31,6 +31,7 @@ namespace Bit.Core.Services public Task AddOrganization(Guid providerId, Guid organizationId, Guid addingUserId, string key) => throw new NotImplementedException(); public Task CreateOrganizationAsync(Guid providerId, OrganizationSignup organizationSignup, string clientOwnerEmail, User user) => throw new NotImplementedException(); - public Task RemoveOrganization(Guid providerId, Guid providerOrganizationId, Guid removingUserId) => throw new NotImplementedException(); + public Task RemoveOrganizationAsync(Guid providerId, Guid providerOrganizationId, Guid removingUserId) => throw new NotImplementedException(); + public Task LogProviderAccessToOrganizationAsync(Guid organizationId) => throw new NotImplementedException(); } }