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
62 changes: 49 additions & 13 deletions src/Api/Vault/Controllers/CiphersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,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 @@ -223,7 +223,7 @@
ValidateClientVersionForFido2CredentialSupport(cipher);

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

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L226 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -315,6 +315,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 @@ -448,7 +484,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 487 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L487 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -478,7 +514,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 517 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L517 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -511,7 +547,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 550 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L550 was not covered by tests
{
throw new NotFoundException();
}
Expand All @@ -538,7 +574,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 577 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L577 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -567,7 +603,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 606 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L606 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -596,7 +632,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 635 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L635 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -629,7 +665,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 @@ -713,7 +749,7 @@
else
{
var orgId = new Guid(organizationId);
if (!await _currentContext.EditAnyCollection(orgId))
if (!await CanEditAnyCipherAsAdminAsync(orgId))
{
throw new NotFoundException();
}
Expand All @@ -730,7 +766,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 769 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L769 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -834,7 +870,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 873 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L873 was not covered by tests
{
throw new NotFoundException();
}
Expand Down Expand Up @@ -900,7 +936,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 939 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L939 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);
}
}
}