Upstream/bitwarden-server
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2024-11-27 13:05:23 +01:00
Code Issues Projects Releases Wiki Activity

[PM-10996] Remove restrict-provider-access feature flag and usage

Browse Source
This commit is contained in:
Shane Melton 2024-09-10 15:38:23 -07:00
parent 3f1127489d
commit c37c6c7d65
No known key found for this signature in database
3 changed files with 10 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/Api/Vault/Controllers/CiphersController.cs
View File

@ -37,11 +37,9 @@ public class CiphersController : Controller
private readonly ICipherService _cipherService;
private readonly IUserService _userService;
private readonly IAttachmentStorageService _attachmentStorageService;
private readonly IProviderService _providerService;
private readonly ICurrentContext _currentContext;
private readonly ILogger<CiphersController> _logger;
private readonly GlobalSettings _globalSettings;
private readonly IFeatureService _featureService;
private readonly IOrganizationCiphersQuery _organizationCiphersQuery;
private readonly IApplicationCacheService _applicationCacheService;
private readonly ICollectionRepository _collectionRepository;
@ -52,11 +50,9 @@ public class CiphersController : Controller
ICipherService cipherService,
IUserService userService,
IAttachmentStorageService attachmentStorageService,
IProviderService providerService,
ICurrentContext currentContext,
ILogger<CiphersController> logger,
GlobalSettings globalSettings,
IFeatureService featureService,
IOrganizationCiphersQuery organizationCiphersQuery,
IApplicationCacheService applicationCacheService,
ICollectionRepository collectionRepository)
@ -66,11 +62,9 @@ public class CiphersController : Controller
_cipherService = cipherService;
_userService = userService;
_attachmentStorageService = attachmentStorageService;
_providerService = providerService;
_currentContext = currentContext;
_logger = logger;
_globalSettings = globalSettings;
_featureService = featureService;
_organizationCiphersQuery = organizationCiphersQuery;
_applicationCacheService = applicationCacheService;
_collectionRepository = collectionRepository;
@ -282,8 +276,7 @@ public class CiphersController : Controller
/// <summary>
/// Permission helper to determine if the current user can use the "/admin" variants of the cipher endpoints.
/// Allowed for custom users with EditAnyCollection, providers, unrestricted owners and admins (allowAdminAccess setting is ON).
/// Falls back to original EditAnyCollection permission check for when V1 flag is disabled.
/// Allowed for custom users with EditAnyCollection, unrestricted owners and admins (allowAdminAccess setting is ON).
/// TODO: Move this to its own authorization handler or equivalent service - AC-2062
/// </summary>
private async Task<bool> CanEditCipherAsAdminAsync(Guid organizationId, IEnumerable<Guid> cipherIds)
@ -293,23 +286,7 @@ public class CiphersController : Controller
// If we're not an "admin", we don't need to check the ciphers
if (org is not ({ Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or { Permissions.EditAnyCollection: true }))
{
// Are we a provider user? If so, we need to be sure we're not restricted
// Once the feature flag is removed, this check can be combined with the above
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
// Provider is restricted from editing ciphers, so we're not an "admin"
if (_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess))
{
return false;
}
// Provider is unrestricted, so we're an "admin", don't return early
}
else
{
// Not a provider or admin
return false;
}
return false;
}
// We know we're an "admin", now check the ciphers explicitly (in case admins are restricted)
@ -366,12 +343,6 @@ public class CiphersController : Controller
return true;
}
// Provider users can edit all ciphers if RestrictProviderAccess is disabled
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
}
return false;
}
@ -389,12 +360,6 @@ public class CiphersController : Controller
return true;
}
// Provider users can only access organization ciphers if RestrictProviderAccess is disabled
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
}
return false;
}
@ -412,12 +377,6 @@ public class CiphersController : Controller
return true;
}
// Provider users can only access all ciphers if RestrictProviderAccess is disabled
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
}
return false;
}

1
src/Core/Constants.cs
View File

@ -116,7 +116,6 @@ public static class FeatureFlagKeys
public const string EmailVerificationDisableTimingDelays = "email-verification-disable-timing-delays";
public const string AnhFcmv1Migration = "anh-fcmv1-migration";
public const string ExtensionRefresh = "extension-refresh";
public const string RestrictProviderAccess = "restrict-provider-access";
public const string PM4154BulkEncryptionService = "PM-4154-bulk-encryption-service";
public const string VaultBulkManagementAction = "vault-bulk-management-action";
public const string BulkDeviceApproval = "bulk-device-approval";

32
test/Api.Test/Vault/Controllers/CiphersControllerTests.cs
View File

@ -186,18 +186,14 @@ public class CiphersControllerTests
}
[Theory]
[BitAutoData(false)]
[BitAutoData(false)]
[BitAutoData(true)]
public async Task CanEditCiphersAsAdminAsync_Providers(
bool restrictProviders, Cipher cipher, CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
[BitAutoData]
public async Task CannotEditCiphersAsAdminAsync_Providers(Cipher cipher, CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
)
{
cipher.OrganizationId = organization.Id;
// Simulate that the user is a provider for the organization
sutProvider.GetDependency<ICurrentContext>().EditAnyCollection(organization.Id).Returns(true);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organization.Id).Returns(true);
// Simulate that the user is a provider for the organization (i.e. not a member, so there is no organization available)
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns((CurrentContextOrganization)null);
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
@ -209,22 +205,10 @@ public class CiphersControllerTests
Id = organization.Id,
AllowAdminAccessToAllCollectionItems = false
});
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(restrictProviders);
// Non restricted providers should succeed
if (!restrictProviders)
{
await sutProvider.Sut.DeleteAdmin(cipher.Id.ToString());
await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
.DeleteAsync(default, default);
}
else // Otherwise, they should fail
{
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.DeleteAsync(default, default);
}
await sutProvider.GetDependency<ICurrentContext>().Received().ProviderUserForOrgAsync(organization.Id);
// Providers should not be allowed to edit ciphers as Admin
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.DeleteAsync(default, default);
}
}