Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

Merged
merged 9 commits into from
Apr 30, 2024
60 changes: 48 additions & 12 deletions src/Api/Vault/Controllers/CiphersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
public async Task<CipherMiniResponseModel> PostAdmin([FromBody] CipherCreateRequestModel model)
{
var cipher = model.Cipher.ToOrganizationCipher();
if (!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
if (!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -226,7 +226,7 @@
ValidateClientVersionForFido2CredentialSupport(cipher);

if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 229 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L229

Added line #L229 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -318,6 +318,42 @@
return new ListResponseModel<CipherMiniDetailsResponseModel>(allOrganizationCipherResponses);
}

/// <summary>
/// Permission helper to determine if the current user can use the "/admin" variants of the cipher endpoints.
/// TODO: Move this to its own authorization handler or equivalent service - AC-2062
/// </summary>
private async Task<bool> CanEditAnyCipherAsAdminAsync(Guid organizationId)
{
// Pre-Flexible collections V1 only needs to check EditAnyCollection
if (!await UseFlexibleCollectionsV1Async(organizationId))
{
return await _currentContext.EditAnyCollection(organizationId);
}

var org = _currentContext.GetOrganization(organizationId);
var orgAbility = await _applicationCacheService.GetOrganizationAbilityAsync(organizationId);

// Custom users with EditAnyCollection permission can always edit any cipher, regardless of the AllowAdminAccessToAllCollectionItems setting
if (org is { Type: OrganizationUserType.Custom, Permissions.EditAnyCollection: true })
{
return true;
}

// Owners and Admins can only edit any cipher if the AllowAdminAccessToAllCollectionItems setting is enabled
if (org is { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } && orgAbility is { AllowAdminAccessToAllCollectionItems: true })
{
return true;
}

// Provider users can always edit ciphers in V1 (to change in AC-1707)
if (await _currentContext.ProviderUserForOrgAsync(organizationId))
{
return true;
}

return false;
}

/// <summary>
/// TODO: Move this to its own authorization handler or equivalent service - AC-2062
/// </summary>
Expand Down Expand Up @@ -581,7 +617,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetByIdAsync(new Guid(id));
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 620 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L620

Added line #L620 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -638,7 +674,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetByIdAsync(new Guid(id));
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 677 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L677

Added line #L677 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -671,7 +707,7 @@
}

if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) ||
!await _currentContext.EditAnyCollection(new Guid(model.OrganizationId)))
!await CanEditAnyCipherAsAdminAsync(new Guid(model.OrganizationId)))

Check warning on line 710 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L710

Added line #L710 was not covered by tests
{
throw new NotFoundException();
}
Expand All @@ -698,7 +734,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetByIdAsync(new Guid(id));
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 737 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L737

Added line #L737 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -727,7 +763,7 @@
}

if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) ||
!await _currentContext.EditAnyCollection(new Guid(model.OrganizationId)))
!await CanEditAnyCipherAsAdminAsync(new Guid(model.OrganizationId)))

Check warning on line 766 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L766

Added line #L766 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -756,7 +792,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(new Guid(id));
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 795 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L795

Added line #L795 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -789,7 +825,7 @@
throw new BadRequestException("You can only restore up to 500 items at a time.");
}

if (model == null || model.OrganizationId == default || !await _currentContext.EditAnyCollection(model.OrganizationId))
if (model == null || model.OrganizationId == default || !await CanEditAnyCipherAsAdminAsync(model.OrganizationId))
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -890,7 +926,7 @@
await GetByIdAsync(id, userId);

if (cipher == null || (request.AdminRequest && (!cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))))

Check warning on line 929 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L929

Added line #L929 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -994,7 +1030,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(idGuid);
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 1033 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L1033

Added line #L1033 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -1060,7 +1096,7 @@
var userId = _userService.GetProperUserId(User).Value;
var cipher = await _cipherRepository.GetByIdAsync(idGuid);
if (cipher == null || !cipher.OrganizationId.HasValue ||
!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
!await CanEditAnyCipherAsAdminAsync(cipher.OrganizationId.Value))

Check warning on line 1099 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L1099

Added line #L1099 was not covered by tests
{
throw new NotFoundException();
}
Expand Down
163 changes: 163 additions & 0 deletions test/Api.Test/Vault/Controllers/CiphersControllerTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
using System.Security.Claims;
using Bit.Api.Vault.Controllers;
using Bit.Api.Vault.Models;
using Bit.Api.Vault.Models.Request;
using Bit.Core;
using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data.Organizations;
using Bit.Core.Services;
using Bit.Core.Vault.Enums;
using Bit.Core.Vault.Models.Data;
using Bit.Core.Vault.Repositories;
using Bit.Core.Vault.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
Expand Down Expand Up @@ -42,4 +50,159 @@ public async Task PutPartialShouldReturnCipherWithGivenFolderAndFavoriteValues(G
Assert.Equal(folderId, result.FolderId);
Assert.Equal(isFavorite, result.Favorite);
}

[Theory]
[BitAutoData(OrganizationUserType.Admin, true, true)]
[BitAutoData(OrganizationUserType.Owner, true, true)]
[BitAutoData(OrganizationUserType.Custom, false, true)]
[BitAutoData(OrganizationUserType.Custom, true, true)]
[BitAutoData(OrganizationUserType.Admin, false, false)]
[BitAutoData(OrganizationUserType.Owner, false, false)]
[BitAutoData(OrganizationUserType.Custom, false, false)]
public async Task CanEditAnyCipherAsAdminAsync_FlexibleCollections_Success(
OrganizationUserType userType, bool allowAdminsAccessToAllItems, bool shouldSucceed,
CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
)
{
organization.Type = userType;
if (userType == OrganizationUserType.Custom)
{
// Assume custom users have EditAnyCollections for success case
organization.Permissions.EditAnyCollection = shouldSucceed;
}
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);

sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility
{
Id = organization.Id,
FlexibleCollections = true,
AllowAdminAccessToAllCollectionItems = allowAdminsAccessToAllItems
});
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);

var requestModel = new CipherCreateRequestModel
{
Cipher = new CipherRequestModel { OrganizationId = organization.Id.ToString(), Type = CipherType.Login, Login = new CipherLoginModel() },
CollectionIds = new List<Guid>()
};

if (shouldSucceed)
{
await sutProvider.Sut.PostAdmin(requestModel);
await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
.SaveAsync(default, default, default);
}
else
{
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PostAdmin(requestModel));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.SaveAsync(default, default, default);
}
}

/// <summary>
/// To be removed after FlexibleCollections is fully released
/// </summary>
[Theory]
[BitAutoData(false, false, true)]
[BitAutoData(false, true, true)]
[BitAutoData(true, false, true)]
[BitAutoData(false, false, false)]
[BitAutoData(false, true, false)]
[BitAutoData(true, false, false)]
public async Task CanEditAnyCipherAsAdminAsync_NonFlexibleCollections(
bool fcV1Enabled, bool fcEnabled, bool shouldSucceed,
CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
)
{
sutProvider.GetDependency<ICurrentContext>().EditAnyCollection(organization.Id).Returns(shouldSucceed);

sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);

sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility
{
Id = organization.Id,
FlexibleCollections = fcEnabled,
AllowAdminAccessToAllCollectionItems = false
});
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(fcV1Enabled);

var requestModel = new CipherCreateRequestModel
{
Cipher = new CipherRequestModel { OrganizationId = organization.Id.ToString(), Type = CipherType.Login, Login = new CipherLoginModel() },
CollectionIds = new List<Guid>()
};

if (shouldSucceed)
{
await sutProvider.Sut.PostAdmin(requestModel);
await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
.SaveAsync(default, default, default);
}
else
{
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PostAdmin(requestModel));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.SaveAsync(default, default, default);
}
}

[Theory]
[BitAutoData(false, true)]
[BitAutoData(true, true)]
[BitAutoData(false, false)]
[BitAutoData(true, false)]
public async Task CanEditAnyCipherAsAdminAsync_Providers(
bool fcV1Enabled, bool shouldSucceed, CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
)
{
if (fcV1Enabled)
{
sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(organization.Id).Returns(shouldSucceed);
}
else
{
sutProvider.GetDependency<ICurrentContext>().EditAnyCollection(organization.Id).Returns(shouldSucceed);
}
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);

sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(new OrganizationAbility
{
Id = organization.Id,
FlexibleCollections = fcV1Enabled, // Assume FlexibleCollections is enabled if v1 is enabled
AllowAdminAccessToAllCollectionItems = false
});
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(fcV1Enabled);

var requestModel = new CipherCreateRequestModel
{
Cipher = new CipherRequestModel { OrganizationId = organization.Id.ToString(), Type = CipherType.Login, Login = new CipherLoginModel() },
CollectionIds = new List<Guid>()
};

if (shouldSucceed)
{
await sutProvider.Sut.PostAdmin(requestModel);
await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
.SaveAsync(default, default, default);
}
else
{
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.PostAdmin(requestModel));
await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
.SaveAsync(default, default, default);
}

if (fcV1Enabled)
{
await sutProvider.GetDependency<ICurrentContext>().Received().ProviderUserForOrgAsync(organization.Id);
}
else
{
await sutProvider.GetDependency<ICurrentContext>().Received().EditAnyCollection(organization.Id);
}
}
}