1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-21 12:05:42 +01:00

[SM-716] Adding ability for service account to have write access (#3021)

* adding ability for service account to have write access

* Suggested changes

* fixing tests

* dotnet format changes

* Adding RunAsServiceAccountWIthPermission logic to ProjectAuthorizationhandlerTests

* Removing logic that prevents deleting and updating a secret. Adding Service Account logic to tests inside of secretAuthorizationhandlerTests.

* Removing Service Account from CanUpdateSecret_NotSupportedClientTypes_DoesNotSuceed because it is a supported client type now :)

* thomas sugested changes

* using Arg.Any<AccessClientType>() instead of default in tests

* merge conflict changes and code updates to remove service account tests that are  outdated

* fixing tests

* removing extra  spaces that lint hates
This commit is contained in:
cd-bitwarden 2023-06-30 13:17:41 -04:00 committed by GitHub
parent b87e6d4a38
commit 3f3f52399b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 113 additions and 70 deletions

View File

@ -52,7 +52,7 @@ public class ProjectAuthorizationHandler : AuthorizationHandler<ProjectOperation
{
AccessClientType.NoAccessCheck => true,
AccessClientType.User => true,
AccessClientType.ServiceAccount => false,
AccessClientType.ServiceAccount => true,
_ => false,
};
@ -67,10 +67,6 @@ public class ProjectAuthorizationHandler : AuthorizationHandler<ProjectOperation
{
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient == AccessClientType.ServiceAccount)
{
return;
}
var access = await _projectRepository.AccessToProjectAsync(resource.Id, userId, accessClient);

View File

@ -74,7 +74,8 @@ public class SecretAuthorizationHandler : AuthorizationHandler<SecretOperationRe
AccessClientType.NoAccessCheck => true,
AccessClientType.User => (await _projectRepository.AccessToProjectAsync(project!.Id, userId, accessClient))
.Write,
AccessClientType.ServiceAccount => false,
AccessClientType.ServiceAccount => (await _projectRepository.AccessToProjectAsync(project!.Id, userId, accessClient))
.Write,
_ => false,
};
@ -84,6 +85,7 @@ public class SecretAuthorizationHandler : AuthorizationHandler<SecretOperationRe
}
}
private async Task CanUpdateSecretAsync(AuthorizationHandlerContext context,
SecretOperationRequirement requirement, Secret resource)
{
@ -106,12 +108,10 @@ public class SecretAuthorizationHandler : AuthorizationHandler<SecretOperationRe
hasAccess = true;
break;
case AccessClientType.User:
var newProject = resource.Projects?.FirstOrDefault();
var access = (await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient)).Write;
var accessToNew = newProject != null &&
(await _projectRepository.AccessToProjectAsync(newProject.Id, userId, accessClient))
.Write;
hasAccess = access && accessToNew;
hasAccess = await GetAccessToUpdateSecretAsync(resource, userId, accessClient);
break;
case AccessClientType.ServiceAccount:
hasAccess = await GetAccessToUpdateSecretAsync(resource, userId, accessClient);
break;
default:
hasAccess = false;
@ -129,11 +129,6 @@ public class SecretAuthorizationHandler : AuthorizationHandler<SecretOperationRe
{
var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient == AccessClientType.ServiceAccount)
{
return;
}
var access = await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient);
if (access.Write)
@ -141,4 +136,14 @@ public class SecretAuthorizationHandler : AuthorizationHandler<SecretOperationRe
context.Succeed(requirement);
}
}
private async Task<bool> GetAccessToUpdateSecretAsync(Secret resource, Guid userId, AccessClientType accessClient)
{
var newProject = resource.Projects?.FirstOrDefault();
var access = (await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient)).Write;
var accessToNew = newProject != null &&
(await _projectRepository.AccessToProjectAsync(newProject.Id, userId, accessClient))
.Write;
return access && accessToNew;
}
}

View File

@ -1,4 +1,7 @@
using Bit.Core.Repositories;
using Bit.Core.Context;
using Bit.Core.Exceptions;
using Bit.Core.Identity;
using Bit.Core.Repositories;
using Bit.Core.SecretsManager.Commands.Projects.Interfaces;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Repositories;
@ -10,23 +13,34 @@ public class CreateProjectCommand : ICreateProjectCommand
private readonly IAccessPolicyRepository _accessPolicyRepository;
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IProjectRepository _projectRepository;
private readonly ICurrentContext _currentContext;
public CreateProjectCommand(
IAccessPolicyRepository accessPolicyRepository,
IOrganizationUserRepository organizationUserRepository,
IProjectRepository projectRepository)
IProjectRepository projectRepository,
ICurrentContext currentContext)
{
_accessPolicyRepository = accessPolicyRepository;
_organizationUserRepository = organizationUserRepository;
_projectRepository = projectRepository;
_currentContext = currentContext;
}
public async Task<Project> CreateAsync(Project project, Guid userId)
public async Task<Project> CreateAsync(Project project, Guid id, ClientType clientType)
{
if (clientType != ClientType.User && clientType != ClientType.ServiceAccount)
{
throw new NotFoundException();
}
var createdProject = await _projectRepository.CreateAsync(project);
var orgUser = await _organizationUserRepository.GetByOrganizationAsync(createdProject.OrganizationId,
userId);
if (clientType == ClientType.User)
{
var orgUser = await _organizationUserRepository.GetByOrganizationAsync(createdProject.OrganizationId, id);
var accessPolicy = new UserProjectAccessPolicy()
{
OrganizationUserId = orgUser.Id,
@ -34,7 +48,23 @@ public class CreateProjectCommand : ICreateProjectCommand
Read = true,
Write = true,
};
await _accessPolicyRepository.CreateManyAsync(new List<BaseAccessPolicy> { accessPolicy });
}
else if (clientType == ClientType.ServiceAccount)
{
var serviceAccountProjectAccessPolicy = new ServiceAccountProjectAccessPolicy()
{
ServiceAccountId = id,
GrantedProjectId = createdProject.Id,
Read = true,
Write = true,
};
await _accessPolicyRepository.CreateManyAsync(new List<BaseAccessPolicy> { serviceAccountProjectAccessPolicy });
}
return createdProject;
}
}

View File

@ -55,7 +55,7 @@ public class DeleteProjectCommand : IDeleteProjectCommand
foreach (var project in projects)
{
var access = await _projectRepository.AccessToProjectAsync(project.Id, userId, accessClient);
if (!access.Write || accessClient == AccessClientType.ServiceAccount)
if (!access.Write)
{
results.Add(new Tuple<Project, string>(project, "access denied"));
}

View File

@ -39,6 +39,11 @@ public class ProjectAuthorizationHandlerTests
.ReturnsForAnyArgs(
(AccessClientType.User, userId));
break;
case PermissionType.RunAsServiceAccountWithPermission:
sutProvider.GetDependency<IAccessClientQuery>().GetAccessClientAsync(default, organizationId)
.ReturnsForAnyArgs(
(AccessClientType.ServiceAccount, userId));
break;
default:
throw new ArgumentOutOfRangeException(nameof(permissionType), permissionType, null);
}
@ -103,7 +108,6 @@ public class ProjectAuthorizationHandlerTests
}
[Theory]
[BitAutoData(AccessClientType.ServiceAccount)]
[BitAutoData(AccessClientType.Organization)]
public async Task CanCreateProject_NotSupportedClientTypes_DoesNotSucceed(AccessClientType clientType,
SutProvider<ProjectAuthorizationHandler> sutProvider, Project project, ClaimsPrincipal claimsPrincipal)
@ -125,6 +129,7 @@ public class ProjectAuthorizationHandlerTests
[Theory]
[BitAutoData(PermissionType.RunAsAdmin)]
[BitAutoData(PermissionType.RunAsUserWithPermission)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission)]
public async Task CanCreateProject_Success(PermissionType permissionType,
SutProvider<ProjectAuthorizationHandler> sutProvider, Project project, ClaimsPrincipal claimsPrincipal)
{
@ -199,6 +204,8 @@ public class ProjectAuthorizationHandlerTests
[Theory]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false)]
public async Task CanUpdateProject_ShouldNotSucceed(PermissionType permissionType, bool read, bool write,
SutProvider<ProjectAuthorizationHandler> sutProvider, Project project, ClaimsPrincipal claimsPrincipal,
Guid userId)
@ -221,6 +228,8 @@ public class ProjectAuthorizationHandlerTests
[BitAutoData(PermissionType.RunAsAdmin, false, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true)]
public async Task CanUpdateProject_Success(PermissionType permissionType, bool read, bool write,
SutProvider<ProjectAuthorizationHandler> sutProvider, Project project, ClaimsPrincipal claimsPrincipal,
Guid userId)

View File

@ -41,6 +41,10 @@ public class SecretAuthorizationHandlerTests
sutProvider.GetDependency<IAccessClientQuery>().GetAccessClientAsync(default, organizationId).ReturnsForAnyArgs(
(clientType, userId));
break;
case PermissionType.RunAsServiceAccountWithPermission:
sutProvider.GetDependency<IAccessClientQuery>().GetAccessClientAsync(default, organizationId).ReturnsForAnyArgs(
(AccessClientType.ServiceAccount, userId));
break;
default:
throw new ArgumentOutOfRangeException(nameof(permissionType), permissionType, null);
}
@ -105,7 +109,6 @@ public class SecretAuthorizationHandlerTests
}
[Theory]
[BitAutoData(AccessClientType.ServiceAccount)]
[BitAutoData(AccessClientType.Organization)]
public async Task CanCreateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessClientType clientType,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret, Guid userId,
@ -114,7 +117,7 @@ public class SecretAuthorizationHandlerTests
var requirement = SecretOperations.Create;
SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType);
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).Returns(
(true, true));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -182,6 +185,8 @@ public class SecretAuthorizationHandlerTests
[Theory]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false)]
public async Task CanCreateSecret_DoesNotSucceed(PermissionType permissionType, bool read, bool write,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret,
Guid userId,
@ -190,7 +195,7 @@ public class SecretAuthorizationHandlerTests
var requirement = SecretOperations.Create;
SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId);
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).ReturnsForAnyArgs(
(read, write));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -207,6 +212,8 @@ public class SecretAuthorizationHandlerTests
[BitAutoData(PermissionType.RunAsAdmin, false, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true)]
public async Task CanCreateSecret_Success(PermissionType permissionType, bool read, bool write,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret,
Guid userId,
@ -215,7 +222,7 @@ public class SecretAuthorizationHandlerTests
var requirement = SecretOperations.Create;
SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId);
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).ReturnsForAnyArgs(
(read, write));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -243,7 +250,6 @@ public class SecretAuthorizationHandlerTests
}
[Theory]
[BitAutoData(AccessClientType.ServiceAccount)]
[BitAutoData(AccessClientType.Organization)]
public async Task CanUpdateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessClientType clientType,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret, Guid userId,
@ -252,7 +258,7 @@ public class SecretAuthorizationHandlerTests
var requirement = SecretOperations.Update;
SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType);
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).Returns(
(true, true));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -327,6 +333,15 @@ public class SecretAuthorizationHandlerTests
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true, true, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true, true, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false, false, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, false, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, false, false)]
public async Task CanUpdateSecret_DoesNotSucceed(PermissionType permissionType, bool read, bool write,
bool projectRead, bool projectWrite,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret,
@ -335,10 +350,10 @@ public class SecretAuthorizationHandlerTests
{
var requirement = SecretOperations.Update;
SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId);
sutProvider.GetDependency<ISecretRepository>().AccessToSecretAsync(secret.Id, userId, default).Returns(
sutProvider.GetDependency<ISecretRepository>().AccessToSecretAsync(secret.Id, userId, Arg.Any<AccessClientType>()).Returns(
(read, write));
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).Returns(
(projectRead, projectWrite));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -355,6 +370,8 @@ public class SecretAuthorizationHandlerTests
[BitAutoData(PermissionType.RunAsAdmin, false, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true)]
public async Task CanUpdateSecret_Success(PermissionType permissionType, bool read, bool write,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret,
Guid userId,
@ -362,10 +379,10 @@ public class SecretAuthorizationHandlerTests
{
var requirement = SecretOperations.Update;
SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId);
sutProvider.GetDependency<ISecretRepository>().AccessToSecretAsync(secret.Id, userId, default).Returns(
sutProvider.GetDependency<ISecretRepository>().AccessToSecretAsync(secret.Id, userId, Arg.Any<AccessClientType>()).Returns(
(read, write));
sutProvider.GetDependency<IProjectRepository>()
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns(
.AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any<AccessClientType>()).Returns(
(read, write));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
@ -409,32 +426,16 @@ public class SecretAuthorizationHandlerTests
Assert.False(authzContext.HasSucceeded);
}
[Theory]
[BitAutoData]
public async Task CanDeleteSecret_ServiceAccountClient_DoesNotSucceed(
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret, Guid userId,
ClaimsPrincipal claimsPrincipal)
{
var requirement = SecretOperations.Delete;
SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId,
AccessClientType.ServiceAccount);
sutProvider.GetDependency<ISecretRepository>()
.AccessToSecretAsync(secret.Id, userId, Arg.Any<AccessClientType>())
.Returns((true, true));
var authzContext = new AuthorizationHandlerContext(new List<IAuthorizationRequirement> { requirement },
claimsPrincipal, secret);
await sutProvider.Sut.HandleAsync(authzContext);
Assert.False(authzContext.HasSucceeded);
}
[Theory]
[BitAutoData(PermissionType.RunAsAdmin, true, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false)]
[BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true, true)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, false)]
[BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true, true)]
public async Task CanDeleteProject_AccessCheck(PermissionType permissionType, bool read, bool write,
bool expected,
SutProvider<SecretAuthorizationHandler> sutProvider, Secret secret,

View File

@ -1,4 +1,5 @@
using Bit.Commercial.Core.SecretsManager.Commands.Projects;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Repositories;
using Bit.Core.SecretsManager.Entities;
@ -29,7 +30,7 @@ public class CreateProjectCommandTests
.CreateAsync(Arg.Any<Project>())
.Returns(data);
await sutProvider.Sut.CreateAsync(data, userId);
await sutProvider.Sut.CreateAsync(data, userId, sutProvider.GetDependency<ICurrentContext>().ClientType);
await sutProvider.GetDependency<IProjectRepository>().Received(1)
.CreateAsync(Arg.Is(data));

View File

@ -4,4 +4,5 @@ public enum PermissionType
{
RunAsAdmin,
RunAsUserWithPermission,
RunAsServiceAccountWithPermission
}

View File

@ -73,9 +73,8 @@ public class ProjectsController : Controller
{
throw new NotFoundException();
}
var userId = _userService.GetProperUserId(User).Value;
var result = await _createProjectCommand.CreateAsync(project, userId);
var result = await _createProjectCommand.CreateAsync(project, userId, _currentContext.ClientType);
// Creating a project means you have read & write permission.
return new ProjectResponseModel(result, true, true);

View File

@ -1,8 +1,9 @@
using Bit.Core.SecretsManager.Entities;
using Bit.Core.Identity;
using Bit.Core.SecretsManager.Entities;
namespace Bit.Core.SecretsManager.Commands.Projects.Interfaces;
public interface ICreateProjectCommand
{
Task<Project> CreateAsync(Project project, Guid userId);
Task<Project> CreateAsync(Project project, Guid userId, ClientType clientType);
}

View File

@ -114,12 +114,12 @@ public class ProjectsControllerTests
var resultProject = data.ToProject(orgId);
sutProvider.GetDependency<ICreateProjectCommand>().CreateAsync(default, default)
sutProvider.GetDependency<ICreateProjectCommand>().CreateAsync(default, default, sutProvider.GetDependency<ICurrentContext>().ClientType)
.ReturnsForAnyArgs(resultProject);
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.CreateAsync(orgId, data));
await sutProvider.GetDependency<ICreateProjectCommand>().DidNotReceiveWithAnyArgs()
.CreateAsync(Arg.Any<Project>(), Arg.Any<Guid>());
.CreateAsync(Arg.Any<Project>(), Arg.Any<Guid>(), sutProvider.GetDependency<ICurrentContext>().ClientType);
}
[Theory]
@ -134,13 +134,13 @@ public class ProjectsControllerTests
var resultProject = data.ToProject(orgId);
sutProvider.GetDependency<ICreateProjectCommand>().CreateAsync(default, default)
sutProvider.GetDependency<ICreateProjectCommand>().CreateAsync(default, default, sutProvider.GetDependency<ICurrentContext>().ClientType)
.ReturnsForAnyArgs(resultProject);
await sutProvider.Sut.CreateAsync(orgId, data);
await sutProvider.GetDependency<ICreateProjectCommand>().Received(1)
.CreateAsync(Arg.Any<Project>(), Arg.Any<Guid>());
.CreateAsync(Arg.Any<Project>(), Arg.Any<Guid>(), sutProvider.GetDependency<ICurrentContext>().ClientType);
}
[Theory]