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

src/Api/Vault/Controllers/CiphersController.cs

@@ -174,7 +174,9 @@
  public async Task<CipherMiniResponseModel> PostAdmin([FromBody] CipherCreateRequestModel model)
  {
  var cipher = model.Cipher.ToOrganizationCipher();
- if (!await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
+ // Only users that can edit all ciphers can create new ciphers via the admin endpoint
+ // Other users should use the regular POST/create endpoint
+ if (!await CanEditAllCiphersAsync(cipher.OrganizationId.Value))
  {
  throw new NotFoundException();
  }
@@ -226,7 +228,7 @@
  ValidateClientVersionForFido2CredentialSupport(cipher);
 
  if (cipher == null || !cipher.OrganizationId.HasValue ||
- !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))
+ !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
@@ -318,6 +320,28 @@
  return new ListResponseModel<CipherMiniDetailsResponseModel>(allOrganizationCipherResponses);
  }
 
+ /// <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.
+ /// TODO: Move this to its own authorization handler or equivalent service - AC-2062
+ /// </summary>
+ private async Task<bool> CanEditCipherAsAdminAsync(Guid organizationId, IEnumerable<Guid> cipherIds)
+ {
+ // Pre-Flexible collections V1 only needs to check EditAnyCollection
+ if (!await UseFlexibleCollectionsV1Async(organizationId))
+ {
+ return await _currentContext.EditAnyCollection(organizationId);
+ }
+
+ if (await CanEditCiphersAsync(organizationId, cipherIds))
+ {
+ return true;
+ }
+
+ return false;
+ }
+
  /// <summary>
  /// TODO: Move this to its own authorization handler or equivalent service - AC-2062
  /// </summary>
@@ -584,14 +608,23 @@
  {
  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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
 
- await _cipherService.SaveCollectionsAsync(cipher,
- model.CollectionIds.Select(c => new Guid(c)), userId, true);
+ var collectionIds = model.CollectionIds.Select(c => new Guid(c)).ToList();
+
+ // In V1, we still need to check if the user can edit the collections they're submitting
+ // This should only happen for unassigned ciphers (otherwise restricted admins would use the normal collections endpoint)
+ if (await UseFlexibleCollectionsV1Async(cipher.OrganizationId.Value) && !await CanEditItemsInCollections(cipher.OrganizationId.Value, collectionIds))
+ {
+ throw new NotFoundException();
+ }
+
+ await _cipherService.SaveCollectionsAsync(cipher, collectionIds, userId, true);
  }
 
  [HttpPost("bulk-collections")]
@@ -642,7 +675,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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
@@ -674,14 +707,21 @@
  "Consider using the \"Purge Vault\" option instead.");
  }
 
- if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) ||
- !await _currentContext.EditAnyCollection(new Guid(model.OrganizationId)))
+ if (model == null)
+ {
+ throw new NotFoundException();
+ }
+
+ var cipherIds = model.Ids.Select(i => new Guid(i)).ToList();
+
+ if (string.IsNullOrWhiteSpace(model.OrganizationId) ||
+ !await CanEditCipherAsAdminAsync(new Guid(model.OrganizationId), cipherIds))
  {
  throw new NotFoundException();
  }
 
  var userId = _userService.GetProperUserId(User).Value;
- await _cipherService.DeleteManyAsync(model.Ids.Select(i => new Guid(i)), userId, new Guid(model.OrganizationId), true);
+ await _cipherService.DeleteManyAsync(cipherIds, userId, new Guid(model.OrganizationId), true);
  }
 
  [HttpPut("{id}/delete")]
@@ -702,7 +742,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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
@@ -730,14 +770,21 @@
  throw new BadRequestException("You can only delete up to 500 items at a time.");
  }
 
- if (model == null || string.IsNullOrWhiteSpace(model.OrganizationId) ||
- !await _currentContext.EditAnyCollection(new Guid(model.OrganizationId)))
+ if (model == null)
+ {
+ throw new NotFoundException();
+ }
+
+ var cipherIds = model.Ids.Select(i => new Guid(i)).ToList();
+
+ if (string.IsNullOrWhiteSpace(model.OrganizationId) ||
+ !await CanEditCipherAsAdminAsync(new Guid(model.OrganizationId), cipherIds))
  {
  throw new NotFoundException();
  }
 
  var userId = _userService.GetProperUserId(User).Value;
- await _cipherService.SoftDeleteManyAsync(model.Ids.Select(i => new Guid(i)), userId, new Guid(model.OrganizationId), true);
+ await _cipherService.SoftDeleteManyAsync(cipherIds, userId, new Guid(model.OrganizationId), true);
  }
 
  [HttpPut("{id}/restore")]
@@ -760,7 +807,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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
@@ -793,14 +840,20 @@
  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)
  {
  throw new NotFoundException();
  }
 
- var userId = _userService.GetProperUserId(User).Value;
  var cipherIdsToRestore = new HashSet<Guid>(model.Ids.Select(i => new Guid(i)));
 
+ if (model.OrganizationId == default || !await CanEditCipherAsAdminAsync(model.OrganizationId, cipherIdsToRestore))
+ {
+ throw new NotFoundException();
+ }
+
+ var userId = _userService.GetProperUserId(User).Value;
+
  var restoredCiphers = await _cipherService.RestoreManyAsync(cipherIdsToRestore, userId, model.OrganizationId, true);
  var responses = restoredCiphers.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp));
  return new ListResponseModel<CipherMiniResponseModel>(responses);
@@ -894,7 +947,7 @@
  await GetByIdAsync(id, userId);
 
  if (cipher == null || (request.AdminRequest && (!cipher.OrganizationId.HasValue ||
- !await _currentContext.EditAnyCollection(cipher.OrganizationId.Value))))
+ !await CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))))
  {
  throw new NotFoundException();
  }
@@ -998,7 +1051,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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }
@@ -1064,7 +1117,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 CanEditCipherAsAdminAsync(cipher.OrganizationId.Value, new[] { cipher.Id }))
  {
  throw new NotFoundException();
  }

test/Api.Test/Vault/Controllers/CiphersControllerTests.cs

@@ -1,9 +1,18 @@
 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.Entities;
+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;
@@ -42,4 +51,150 @@ 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 CanEditCiphersAsAdminAsync_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(true, true)]
+ [BitAutoData(false, true)]
+ [BitAutoData(true, false)]
+ [BitAutoData(false, false)]
+ public async Task CanEditCiphersAsAdminAsync_NonFlexibleCollections(
+ bool v1Enabled, bool shouldSucceed, CurrentContextOrganization organization, Guid userId, Cipher cipher, SutProvider<CiphersController> sutProvider
+ )
+ {
+ cipher.OrganizationId = organization.Id;
+ 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 = false,
+ AllowAdminAccessToAllCollectionItems = false
+ });
+ sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(v1Enabled);
+ sutProvider.GetDependency<ICipherRepository>().GetByIdAsync(cipher.Id).Returns(cipher);
+
+ if (shouldSucceed)
+ {
+ await sutProvider.Sut.DeleteAdmin(cipher.Id.ToString());
+ await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
+ .DeleteAsync(default, default);
+ }
+ else
+ {
+ await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()));
+ await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
+ .DeleteAsync(default, default);
+ }
+ }
+
+ [Theory]
+ [BitAutoData(false, true)]
+ [BitAutoData(true, true)]
+ [BitAutoData(false, false)]
+ [BitAutoData(true, false)]
+ public async Task CanEditCiphersAsAdminAsync_Providers(
+ bool fcV1Enabled, bool shouldSucceed, Cipher cipher, CurrentContextOrganization organization, Guid userId, SutProvider<CiphersController> sutProvider
+ )
+ {
+ cipher.OrganizationId = organization.Id;
+ 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<ICipherRepository>().GetByIdAsync(cipher.Id).Returns(cipher);
+ sutProvider.GetDependency<ICipherRepository>().GetManyByOrganizationIdAsync(organization.Id).Returns(new List<Cipher> { cipher });
+
+ 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);
+
+ if (shouldSucceed)
+ {
+ await sutProvider.Sut.DeleteAdmin(cipher.Id.ToString());
+ await sutProvider.GetDependency<ICipherService>().ReceivedWithAnyArgs()
+ .DeleteAsync(default, default);
+ }
+ else
+ {
+ await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteAdmin(cipher.Id.ToString()));
+ await sutProvider.GetDependency<ICipherService>().DidNotReceiveWithAnyArgs()
+ .DeleteAsync(default, default);
+ }
+
+ if (fcV1Enabled)
+ {
+ await sutProvider.GetDependency<ICurrentContext>().Received().ProviderUserForOrgAsync(organization.Id);
+ }
+ else
+ {
+ await sutProvider.GetDependency<ICurrentContext>().Received().EditAnyCollection(organization.Id);
+ }
+ }
 }