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

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

Browse Source 
* [AC-2274] Introduce CanEditAnyCiphersAsAdminAsync helper to replace EditAnyCollection usage

* [AC-2274] Add unit tests for CanEditAnyCiphersAsAdmin helper

* [AC-2274] Add Jira ticket

* [AC-2274] Undo change to purge endpoint

* [AC-2274] Update admin checks to account for unassigned ciphers

---------

Co-authored-by: kejaeger <138028972+kejaeger@users.noreply.github.com>
This commit is contained in:
Shane Melton 2024-04-30 10:28:16 -07:00 committed by GitHub
parent 79a4cbaa09
commit 8e7bd79d9a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 227 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -174,7 +174,9 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
{
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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
"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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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,13 +840,19 @@ public class CiphersController : Controller
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 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 cipherIdsToRestore = new HashSet<Guid>(model.Ids.Select(i => new Guid(i)));
var restoredCiphers = await _cipherService.RestoreManyAsync(cipherIdsToRestore, userId, model.OrganizationId, true);
var responses = restoredCiphers.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp));
@ -894,7 +947,7 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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 @@ public class CiphersController : Controller
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();
}

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

@ -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 class CiphersControllerTests
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);
}
}
}