Skip to content

[PM-34822] when lacking permissions return a Forbid() response #7458

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.

Sign up for GitHub

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

Open
voommen-livefront wants to merge 2 commits into
base: main
Choose a base branch
from
+43 −31
Open

[PM-34822] when lacking permissions return a Forbid() response #7458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c172e80
PM-34822 when lacking permissions return a Forbid() response
voommen-livefront Apr 13, 2026
d294408
PM-34822 for consistency update the Get method and unit tests to retu…
voommen-livefront Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions src/Api/Dirt/Controllers/OrganizationIntegrationController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Bit.Api.Dirt.Models.Response;
using Bit.Core.Context;
using Bit.Core.Dirt.EventIntegrations.OrganizationIntegrations.Interfaces;
using Bit.Core.Exceptions;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;

Expand All @@ -18,17 +17,17 @@ public class OrganizationIntegrationController(
IGetOrganizationIntegrationsQuery getQuery) : Controller
{
[HttpGet("")]
public async Task<List<OrganizationIntegrationResponseModel>> GetAsync(Guid organizationId)
public async Task<ActionResult<List<OrganizationIntegrationResponseModel>>> GetAsync(Guid organizationId)
{
if (!await HasPermission(organizationId))
{
throw new NotFoundException();
return Forbid();
}

var integrations = await getQuery.GetManyByOrganizationAsync(organizationId);
return integrations
return Ok(integrations
.Select(integration => new OrganizationIntegrationResponseModel(integration))
.ToList();
.ToList());
}

/// <summary>
Expand All @@ -38,7 +37,7 @@ public async Task<List<OrganizationIntegrationResponseModel>> GetAsync(Guid orga
/// <param name="organizationId"></param>
/// <param name="model"></param>
/// <returns></returns>
/// <exception cref="NotFoundException">Not enough permissions to access the organization.</exception>
/// <exception cref="ForbidResult">Not enough permissions to access the organization.</exception>
/// <exception cref="ConflictResult">When an integration of the same type already exists for the organization.</exception>
[HttpPost("")]
public async Task<ActionResult<OrganizationIntegrationResponseModel>> CreateAsync(Guid organizationId, [FromBody] OrganizationIntegrationRequestModel model)
Expand All @@ -50,7 +49,7 @@ public async Task<ActionResult<OrganizationIntegrationResponseModel>> CreateAsyn

if (!await HasPermission(organizationId))
{
throw new NotFoundException();
return Forbid();
}

var integration = model.ToOrganizationIntegration(organizationId);
Expand All @@ -67,35 +66,36 @@ public async Task<ActionResult<OrganizationIntegrationResponseModel>> CreateAsyn
}

[HttpPut("{integrationId:guid}")]
public async Task<OrganizationIntegrationResponseModel> UpdateAsync(Guid organizationId, Guid integrationId, [FromBody] OrganizationIntegrationRequestModel model)
public async Task<ActionResult<OrganizationIntegrationResponseModel>> UpdateAsync(Guid organizationId, Guid integrationId, [FromBody] OrganizationIntegrationRequestModel model)
{
if (!await HasPermission(organizationId))
{
throw new NotFoundException();
return Forbid();
}

var integration = model.ToOrganizationIntegration(organizationId);
var updated = await updateCommand.UpdateAsync(organizationId, integrationId, integration);

return new OrganizationIntegrationResponseModel(updated);
return Ok(new OrganizationIntegrationResponseModel(updated));
}

[HttpDelete("{integrationId:guid}")]
public async Task DeleteAsync(Guid organizationId, Guid integrationId)
public async Task<IActionResult> DeleteAsync(Guid organizationId, Guid integrationId)
{
if (!await HasPermission(organizationId))
{
throw new NotFoundException();
return Forbid();
}

await deleteCommand.DeleteAsync(organizationId, integrationId);
return NoContent();
}

[HttpPost("{integrationId:guid}/delete")]
[Obsolete("This endpoint is deprecated. Use DELETE method instead")]
public async Task PostDeleteAsync(Guid organizationId, Guid integrationId)
public async Task<IActionResult> PostDeleteAsync(Guid organizationId, Guid integrationId)
{
await DeleteAsync(organizationId, integrationId);
return await DeleteAsync(organizationId, integrationId);
}

private async Task<bool> HasPermission(Guid organizationId)
Expand Down
46 changes: 29 additions & 17 deletions test/Api.Test/Dirt/Controllers/OrganizationIntegrationControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Bit.Core.Dirt.Entities;
using Bit.Core.Dirt.Enums;
using Bit.Core.Dirt.EventIntegrations.OrganizationIntegrations.Interfaces;
using Bit.Core.Exceptions;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using Microsoft.AspNetCore.Mvc;
Expand Down Expand Up @@ -34,7 +33,8 @@ public async Task GetAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
.OrganizationOwner(organizationId)
.Returns(false);

await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.GetAsync(organizationId));
var result = await sutProvider.Sut.GetAsync(organizationId);
Assert.IsType<ForbidResult>(result.Result);
}

[Theory, BitAutoData]
Expand All @@ -56,8 +56,11 @@ public async Task GetAsync_IntegrationsExist_ReturnsIntegrations(
await sutProvider.GetDependency<IGetOrganizationIntegrationsQuery>().Received(1)
.GetManyByOrganizationAsync(organizationId);

Assert.Equal(integrations.Count, result.Count);
Assert.All(result, r => Assert.IsType<OrganizationIntegrationResponseModel>(r));
Assert.IsType<OkObjectResult>(result.Result);
var okResult = result.Result as OkObjectResult;
var returnedIntegrations = okResult.Value as List<OrganizationIntegrationResponseModel>;
Assert.Equal(integrations.Count, returnedIntegrations.Count);
Assert.All(returnedIntegrations, r => Assert.IsType<OrganizationIntegrationResponseModel>(r));
}

[Theory, BitAutoData]
Expand All @@ -71,11 +74,13 @@ public async Task GetAsync_NoIntegrations_ReturnsEmptyList(
.Returns(true);
sutProvider.GetDependency<IGetOrganizationIntegrationsQuery>()
.GetManyByOrganizationAsync(organizationId)
.Returns([]);
.Returns(new List<OrganizationIntegration>());

var result = await sutProvider.Sut.GetAsync(organizationId);

Assert.Empty(result);
var okResult = result.Result as OkObjectResult;
var returnedIntegrations = okResult.Value as List<OrganizationIntegrationResponseModel>;
Assert.Empty(returnedIntegrations);
}

[Theory, BitAutoData]
Expand Down Expand Up @@ -129,7 +134,7 @@ public async Task CreateAsync_TheTypeAlreadyExists_ThrowsConflict(
}

[Theory, BitAutoData]
public async Task CreateAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
public async Task CreateAsync_UserIsNotOrganizationAdmin_ReturnsForbid(
SutProvider<OrganizationIntegrationController> sutProvider,
Guid organizationId)
{
Expand All @@ -138,8 +143,9 @@ public async Task CreateAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
.OrganizationOwner(organizationId)
.Returns(false);

await Assert.ThrowsAsync<NotFoundException>(async () =>
await sutProvider.Sut.CreateAsync(organizationId, _webhookRequestModel));
var response = await sutProvider.Sut.CreateAsync(organizationId, _webhookRequestModel);

Assert.IsType<ForbidResult>(response.Result);
}

[Theory, BitAutoData]
Expand Down Expand Up @@ -178,7 +184,7 @@ await sutProvider.GetDependency<IDeleteOrganizationIntegrationCommand>().Receive
}

[Theory, BitAutoData]
public async Task DeleteAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
public async Task DeleteAsync_UserIsNotOrganizationAdmin_ReturnsForbid(
SutProvider<OrganizationIntegrationController> sutProvider,
Guid organizationId,
Guid integrationId)
Expand All @@ -188,8 +194,9 @@ public async Task DeleteAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
.OrganizationOwner(organizationId)
.Returns(false);

await Assert.ThrowsAsync<NotFoundException>(async () =>
await sutProvider.Sut.DeleteAsync(organizationId, integrationId));
var response = await sutProvider.Sut.DeleteAsync(organizationId, integrationId);

Assert.IsType<ForbidResult>(response);
}

[Theory, BitAutoData]
Expand Down Expand Up @@ -217,12 +224,16 @@ await sutProvider.GetDependency<IUpdateOrganizationIntegrationCommand>().Receive
.UpdateAsync(organizationId, integrationId, Arg.Is<OrganizationIntegration>(i =>
i.OrganizationId == organizationId &&
i.Type == IntegrationType.Webhook));
Assert.IsType<OrganizationIntegrationResponseModel>(response);
Assert.Equal(IntegrationType.Webhook, response.Type);
Assert.IsType<OkObjectResult>(response.Result);
var okResult = response.Result as OkObjectResult;
Assert.NotNull(okResult);
var resultValue = okResult!.Value as OrganizationIntegrationResponseModel;
Assert.NotNull(resultValue);
Assert.Equal(IntegrationType.Webhook, resultValue!.Type);
}

[Theory, BitAutoData]
public async Task UpdateAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
public async Task UpdateAsync_UserIsNotOrganizationAdmin_ReturnsForbid(
SutProvider<OrganizationIntegrationController> sutProvider,
Guid organizationId,
Guid integrationId)
Expand All @@ -232,7 +243,8 @@ public async Task UpdateAsync_UserIsNotOrganizationAdmin_ThrowsNotFound(
.OrganizationOwner(organizationId)
.Returns(false);

await Assert.ThrowsAsync<NotFoundException>(async () =>
await sutProvider.Sut.UpdateAsync(organizationId, integrationId, _webhookRequestModel));
var response = await sutProvider.Sut.UpdateAsync(organizationId, integrationId, _webhookRequestModel);

Assert.IsType<ForbidResult>(response.Result);
}
}
Loading