nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-05-17 17:45:36 -05:00
Code

[PM-20543] - remove restrict-provider-access feature flag (#5700)

Browse Source 
* remove restrict-provider-access feature flag

* remove feature flag

* re-add flag

* remove unnecessary tests

* fix bad merge

* fix bad merge

* remove RestrictProviderAccess key
This commit is contained in:
Jordan Aasen 2025-05-15 14:00:48 -07:00 committed by GitHub
parent 07de9aa8bc
commit 97fbf21977
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 18 additions and 248 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -315,26 +315,10 @@ public class CiphersController : Controller
{
var org = _currentContext.GetOrganization(organizationId);
// 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 }))
// If we're not an "admin" or if we're not a provider user we don't need to check the ciphers
if (org is not ({ Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or { Permissions.EditAnyCollection: true }) || await _currentContext.ProviderUserForOrgAsync(organizationId))
{
// 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)
@ -350,26 +334,10 @@ public class CiphersController : Controller
var org = _currentContext.GetOrganization(organizationId);
// 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 }))
// If we're not an "admin" or if we're a provider user we don't need to check the ciphers
if (org is not ({ Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or { Permissions.EditAnyCollection: true }) || await _currentContext.ProviderUserForOrgAsync(organizationId))
{
// 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;
}
// If the user can edit all ciphers for the organization, just check they all belong to the org
@ -462,10 +430,10 @@ public class CiphersController : Controller
return true;
}
// Provider users can edit all ciphers if RestrictProviderAccess is disabled
// Provider users cannot edit ciphers
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
return false;
}
return false;
@ -485,10 +453,10 @@ public class CiphersController : Controller
return true;
}
// Provider users can only access organization ciphers if RestrictProviderAccess is disabled
// Provider users cannot access organization ciphers
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
return false;
}
return false;
@ -508,10 +476,10 @@ public class CiphersController : Controller
return true;
}
// Provider users can only access all ciphers if RestrictProviderAccess is disabled
// Provider users cannot access ciphers
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return !_featureService.IsEnabled(FeatureFlagKeys.RestrictProviderAccess);
return false;
}
return false;

1
src/Core/Constants.cs
View File

@ -193,7 +193,6 @@ public static class FeatureFlagKeys
/* Vault Team */
public const string PM8851_BrowserOnboardingNudge = "pm-8851-browser-onboarding-nudge";
public const string PM9111ExtensionPersistAddEditForm = "pm-9111-extension-persist-add-edit-form";
public const string RestrictProviderAccess = "restrict-provider-access";
public const string SecurityTasks = "security-tasks";
public const string CipherKeyEncryption = "cipher-key-encryption";
public const string DesktopCipherForms = "pm-18520-desktop-cipher-forms";

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

@ -193,49 +193,6 @@ public class CiphersControllerTests
}
}
[Theory]
[BitAutoData(false)]
[BitAutoData(false)]
[BitAutoData(true)]
public async Task CanEditCiphersAsAdminAsync_Providers(
bool restrictProviders, CipherDetails cipherDetails, CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
)
{
cipherDetails.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);
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(organization.Id).Returns(new List<Cipher> { cipherDetails });
sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility
{
Id = organization.Id,
AllowAdminAccessToAllCollectionItems = false
});
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(restrictProviders);
// Non restricted providers should succeed
if (!restrictProviders)
{
await sutProvider.Sut.DeleteAdmin(cipherDetails.Id);
await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
.DeleteAsync(default, default);
}
else // Otherwise, they should fail
{
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipherDetails.Id));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.DeleteAsync(default, default);
}
await sutProvider.GetDependency<ICurrentContext>().Received().ProviderUserForOrgAsync(organization.Id);
}
[Theory]
[BitAutoData(OrganizationUserType.Owner)]
[BitAutoData(OrganizationUserType.Admin)]
@ -456,24 +413,7 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task DeleteAdmin_WithProviderUser_DeletesCipher(
CipherDetails cipherDetails, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipherDetails.OrganizationId = Guid.NewGuid();
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipherDetails.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(cipherDetails.OrganizationId.Value).Returns(new List<Cipher> { cipherDetails });
await sutProvider.Sut.DeleteAdmin(cipherDetails.Id);
await sutProvider.GetDependency<ICipherService>().Received(1).DeleteAsync(cipherDetails, userId, true);
}
[Theory]
[BitAutoData]
public async Task DeleteAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task DeleteAdmin_WithProviderUser_ThrowsNotFoundException(
Cipher cipher, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipher.OrganizationId = Guid.NewGuid();
@ -481,7 +421,6 @@ public class CiphersControllerTests
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipher.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipher.Id).Returns(cipher);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipher.Id));
}
@ -737,43 +676,13 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task DeleteManyAdmin_WithProviderUser_DeletesCiphers(
CipherBulkDeleteRequestModel model, Guid userId,
List<Cipher> ciphers, SutProvider<CiphersController> sutProvider)
{
var organizationId = Guid.NewGuid();
model.OrganizationId = organizationId.ToString();
model.Ids = ciphers.Select(c => c.Id.ToString()).ToList();
foreach (var cipher in ciphers)
{
cipher.OrganizationId = organizationId;
}
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organizationId).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(organizationId).Returns(ciphers);
await sutProvider.Sut.DeleteManyAdmin(model);
await sutProvider.GetDependency<ICipherService>()
.Received(1)
.DeleteManyAsync(
Arg.Is<IEnumerable<Guid>>(ids =>
ids.All(id => model.Ids.Contains(id.ToString())) && ids.Count() == model.Ids.Count()),
userId, organizationId, true);
}
[Theory]
[BitAutoData]
public async Task DeleteManyAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task DeleteManyAdmin_WithProviderUser_ThrowsNotFoundException(
CipherBulkDeleteRequestModel model, SutProvider<CiphersController> sutProvider)
{
var organizationId = Guid.NewGuid();
model.OrganizationId = organizationId.ToString();
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organizationId).Returns(true);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteManyAdmin(model));
}
@ -1000,24 +909,7 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task PutDeleteAdmin_WithProviderUser_SoftDeletesCipher(
CipherDetails cipherDetails, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipherDetails.OrganizationId = Guid.NewGuid();
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipherDetails.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(cipherDetails.OrganizationId.Value).Returns(new List<Cipher> { cipherDetails });
await sutProvider.Sut.PutDeleteAdmin(cipherDetails.Id);
await sutProvider.GetDependency<ICipherService>().Received(1).SoftDeleteAsync(cipherDetails, userId, true);
}
[Theory]
[BitAutoData]
public async Task PutDeleteAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task PutDeleteAdmin_WithProviderUser_ThrowsNotFoundException(
Cipher cipher, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipher.OrganizationId = Guid.NewGuid();
@ -1025,7 +917,6 @@ public class CiphersControllerTests
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipher.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipher.Id).Returns(cipher);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PutDeleteAdmin(cipher.Id));
}
@ -1272,43 +1163,13 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task PutDeleteManyAdmin_WithProviderUser_SoftDeletesCiphers(
CipherBulkDeleteRequestModel model, Guid userId,
List<Cipher> ciphers, SutProvider<CiphersController> sutProvider)
{
var organizationId = Guid.NewGuid();
model.OrganizationId = organizationId.ToString();
model.Ids = ciphers.Select(c => c.Id.ToString()).ToList();
foreach (var cipher in ciphers)
{
cipher.OrganizationId = organizationId;
}
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organizationId).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(organizationId).Returns(ciphers);
await sutProvider.Sut.PutDeleteManyAdmin(model);
await sutProvider.GetDependency<ICipherService>()
.Received(1)
.SoftDeleteManyAsync(
Arg.Is<IEnumerable<Guid>>(ids =>
ids.All(id => model.Ids.Contains(id.ToString())) && ids.Count() == model.Ids.Count()),
userId, organizationId, true);
}
[Theory]
[BitAutoData]
public async Task PutDeleteManyAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task PutDeleteManyAdmin_WithProviderUser_ThrowsNotFoundException(
CipherBulkDeleteRequestModel model, SutProvider<CiphersController> sutProvider)
{
var organizationId = Guid.NewGuid();
model.OrganizationId = organizationId.ToString();
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organizationId).Returns(true);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PutDeleteManyAdmin(model));
}
@ -1546,27 +1407,7 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task PutRestoreAdmin_WithProviderUser_RestoresCipher(
CipherDetails cipherDetails, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipherDetails.OrganizationId = Guid.NewGuid();
cipherDetails.Type = CipherType.Login;
cipherDetails.Data = JsonSerializer.Serialize(new CipherLoginData());
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipherDetails.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipherDetails.Id, userId).Returns(cipherDetails);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(cipherDetails.OrganizationId.Value).Returns(new List<Cipher> { cipherDetails });
var result = await sutProvider.Sut.PutRestoreAdmin(cipherDetails.Id);
Assert.IsType<CipherMiniResponseModel>(result);
await sutProvider.GetDependency<ICipherService>().Received(1).RestoreAsync(cipherDetails, userId, true);
}
[Theory]
[BitAutoData]
public async Task PutRestoreAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task PutRestoreAdmin_WithProviderUser_ThrowsNotFoundException(
CipherDetails cipherDetails, Guid userId, SutProvider<CiphersController> sutProvider)
{
cipherDetails.OrganizationId = Guid.NewGuid();
@ -1574,7 +1415,6 @@ public class CiphersControllerTests
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(cipherDetails.OrganizationId.Value).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetOrganizationDetailsByIdAsync(cipherDetails.Id).Returns(cipherDetails);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PutRestoreAdmin(cipherDetails.Id));
}
@ -1896,49 +1736,12 @@ public class CiphersControllerTests
[Theory]
[BitAutoData]
public async Task PutRestoreManyAdmin_WithProviderUser_RestoresCiphers(
CipherBulkRestoreRequestModel model, Guid userId,
List<Cipher> ciphers, SutProvider<CiphersController> sutProvider)
{
model.OrganizationId = Guid.NewGuid();
model.Ids = ciphers.Select(c => c.Id.ToString()).ToList();
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(model.OrganizationId).Returns(true);
sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(model.OrganizationId).Returns(ciphers);
var cipherOrgDetails = ciphers.Select(c => new CipherOrganizationDetails
{
Id = c.Id,
OrganizationId = model.OrganizationId
}).ToList();
sutProvider.GetDependency<ICipherService>()
.RestoreManyAsync(
Arg.Any<HashSet<Guid>>(),
userId, model.OrganizationId, true)
.Returns(cipherOrgDetails);
var result = await sutProvider.Sut.PutRestoreManyAdmin(model);
Assert.NotNull(result);
await sutProvider.GetDependency<ICipherService>()
.Received(1)
.RestoreManyAsync(
Arg.Is<HashSet<Guid>>(ids =>
ids.All(id => model.Ids.Contains(id.ToString())) && ids.Count == model.Ids.Count()),
userId, model.OrganizationId, true);
}
[Theory]
[BitAutoData]
public async Task PutRestoreManyAdmin_WithProviderUser_WithRestrictProviderAccessTrue_ThrowsNotFoundException(
public async Task PutRestoreManyAdmin_WithProviderUser_ThrowsNotFoundException(
CipherBulkRestoreRequestModel model, SutProvider<CiphersController> sutProvider)
{
model.OrganizationId = Guid.NewGuid();
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(model.OrganizationId).Returns(true);
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.RestrictProviderAccess).Returns(true);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PutRestoreManyAdmin(model));
}