Skip to content

[PM-33819] Enforce use of authorize attributes #7242

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

Merged
eliykat merged 2 commits into from
Mar 23, 2026
Merged

[PM-33819] Enforce use of authorize attributes #7242

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
5c8cc17
Add tests to ensure we are using authorize attributes.
eliykat Mar 17, 2026
8d97a0b
Fix tests
eliykat Mar 18, 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
62 changes: 5 additions & 57 deletions src/Api/AdminConsole/Controllers/PoliciesController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Tokens;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
Expand All @@ -34,42 +32,32 @@ public class PoliciesController : Controller
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory;
private readonly IPolicyRepository _policyRepository;
private readonly IUserService _userService;
private readonly ISavePolicyCommand _savePolicyCommand;
private readonly IVNextSavePolicyCommand _vNextSavePolicyCommand;
private readonly IPolicyQuery _policyQuery;

public PoliciesController(IPolicyRepository policyRepository,
IOrganizationUserRepository organizationUserRepository,
IUserService userService,
ICurrentContext currentContext,
IDataProtectorTokenFactory<OrgUserInviteTokenable> orgUserInviteTokenDataFactory,
IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery,
IOrganizationRepository organizationRepository,
ISavePolicyCommand savePolicyCommand,
IVNextSavePolicyCommand vNextSavePolicyCommand,
IPolicyQuery policyQuery)
{
_policyRepository = policyRepository;
_organizationUserRepository = organizationUserRepository;
_userService = userService;
_currentContext = currentContext;
_organizationRepository = organizationRepository;
_orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory;
_organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery;
_savePolicyCommand = savePolicyCommand;
_vNextSavePolicyCommand = vNextSavePolicyCommand;
_policyQuery = policyQuery;
}

[HttpGet("{type}")]
[Authorize<ManagePoliciesRequirement>]
public async Task<PolicyStatusResponseModel> Get(Guid orgId, PolicyType type)
{
if (!await _currentContext.ManagePolicies(orgId))
{
throw new NotFoundException();
}

var policy = await _policyQuery.RunAsync(orgId, type);
if (policy.Type is PolicyType.SingleOrg)
{
Expand All @@ -80,15 +68,10 @@ public async Task<PolicyStatusResponseModel> Get(Guid orgId, PolicyType type)
}

[HttpGet("")]
public async Task<ListResponseModel<PolicyResponseModel>> GetAll(string orgId)
[Authorize<ManagePoliciesRequirement>]
public async Task<ListResponseModel<PolicyResponseModel>> GetAll(Guid orgId)
{
var orgIdGuid = new Guid(orgId);
if (!await _currentContext.ManagePolicies(orgIdGuid))
{
throw new NotFoundException();
}

var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgIdGuid);
var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgId);

return new ListResponseModel<PolicyResponseModel>(policies.Select(p => new PolicyResponseModel(p)));
}
Expand Down Expand Up @@ -124,34 +107,8 @@ public async Task<ListResponseModel<PolicyResponseModel>> GetByToken(Guid orgId,
return new ListResponseModel<PolicyResponseModel>(responses);
}

// TODO: PM-4097 - remove GetByInvitedUser once all clients are updated to use the GetMasterPasswordPolicy endpoint below
Copy link
Copy Markdown
Member Author

@eliykat eliykat Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ticket is over 2 years old and I could not find any existing consumers.

[Obsolete("Deprecated API", false)]
[AllowAnonymous]
[HttpGet("invited-user")]
public async Task<ListResponseModel<PolicyResponseModel>> GetByInvitedUser(Guid orgId, [FromQuery] Guid userId)
{
var user = await _userService.GetUserByIdAsync(userId);
if (user == null)
{
throw new UnauthorizedAccessException();
}
var orgUsersByUserId = await _organizationUserRepository.GetManyByUserAsync(user.Id);
var orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgId);
if (orgUser == null)
{
throw new NotFoundException();
}
if (orgUser.Status != OrganizationUserStatusType.Invited)
{
throw new UnauthorizedAccessException();
}

var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgId);
var responses = policies.Where(p => p.Enabled).Select(p => new PolicyResponseModel(p));
return new ListResponseModel<PolicyResponseModel>(responses);
}

[HttpGet("master-password")]
[Authorize<MemberRequirement>]
public async Task<PolicyResponseModel> GetMasterPasswordPolicy(Guid orgId)
{
var organization = await _organizationRepository.GetByIdAsync(orgId);
Expand All @@ -161,15 +118,6 @@ public async Task<PolicyResponseModel> GetMasterPasswordPolicy(Guid orgId)
throw new NotFoundException();
}

var userId = _userService.GetProperUserId(User).Value;

var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId);

if (orgUser == null)
{
throw new NotFoundException();
}

var policy = await _policyRepository.GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword);

if (policy == null || !policy.Enabled)
Expand Down
58 changes: 58 additions & 0 deletions test/Api.Test/AdminConsole/Controllers/AdminConsoleControllersAuthorizationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
ο»Ώusing Bit.Api.AdminConsole.Controllers;
using Bit.Api.Test.Utilities;
using Microsoft.AspNetCore.Mvc;
using Xunit;

namespace Bit.Api.Test.AdminConsole.Controllers;

public class AdminConsoleControllersAuthorizationTests
{
/// <summary>
/// Controllers that have not yet been migrated to use method-level authorization attributes.
/// TODO: Remove controllers from this list as they are migrated to use [Authorize] or [AllowAnonymous] on all methods.
/// </summary>
private static readonly HashSet<Type> _controllersNotYetMigrated =
[
typeof(GroupsController),
typeof(OrganizationAuthRequestsController),
typeof(OrganizationConnectionsController),
typeof(OrganizationDomainController),
typeof(OrganizationsController),
typeof(OrganizationUsersController),
typeof(ProviderClientsController),
typeof(ProviderOrganizationsController),
typeof(ProvidersController),
typeof(ProviderUsersController)
];

public static IEnumerable<object[]> GetAllAdminConsoleControllers()
{
// This is just a convenient way to get the assembly reference - it does
// not actually require that all controllers extend this base class
var assembly = typeof(BaseAdminConsoleController).Assembly;
return assembly.GetTypes()
.Where(t => t.IsClass
&& !t.IsAbstract
&& typeof(ControllerBase).IsAssignableFrom(t)
&& t.Namespace == "Bit.Api.AdminConsole.Controllers")
.Except(_controllersNotYetMigrated)
.Select(t => new object[] { t });
}

/// <summary>
/// Automatically finds all controllers in the Bit.Api.AdminConsole.Controllers namespace
/// and ensures that they have [Authorize] or [AllowAnonymous] attributes on all methods.
/// </summary>
/// <remarks>
/// See <see cref="_controllersNotYetMigrated"/> for an exemption list of existing controllers
/// that aren't using these attributes yet (but should be).
/// See <see cref="ControllerAuthorizationTestHelpers.AssertAllHttpMethodsHaveAuthorization"/>
/// for more information about what this test requires to pass.
/// </remarks>
[Theory]
[MemberData(nameof(GetAllAdminConsoleControllers))]
public void AllControllers_HaveAuthorizationOnAllMethods(Type controllerType)
{
ControllerAuthorizationTestHelpers.AssertAllHttpMethodsHaveAuthorization(controllerType);
}
}
81 changes: 13 additions & 68 deletions ...st/Controllers/PoliciesControllerTests.cs β†’ ...le/Controllers/PoliciesControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
ο»Ώusing System.Security.Claims;
using System.Text.Json;
ο»Ώusing System.Text.Json;
using Bit.Api.AdminConsole.Controllers;
using Bit.Api.AdminConsole.Models.Request;
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Test.Utilities;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
Expand All @@ -15,28 +15,32 @@
using Bit.Core.Entities;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Tokens;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using NSubstitute;
using Xunit;

namespace Bit.Api.Test.Controllers;
namespace Bit.Api.Test.AdminConsole.Controllers;


// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern.
[ControllerCustomize(typeof(PoliciesController))]
[SutProviderCustomize]
public class PoliciesControllerTests
{
[Fact]
public void AllActionMethodsHaveAuthorization()
{
ControllerAuthorizationTestHelpers.AssertAllHttpMethodsHaveAuthorization(
typeof(PoliciesController));
}

[Theory]
[BitAutoData]
public async Task GetMasterPasswordPolicy_WhenCalled_ReturnsMasterPasswordPolicy(
SutProvider<PoliciesController> sutProvider,
Guid orgId, Guid userId,
OrganizationUser orgUser,
Guid orgId,
Policy policy,
MasterPasswordPolicyData mpPolicyData,
Organization organization)
Expand All @@ -47,15 +51,6 @@ public async Task GetMasterPasswordPolicy_WhenCalled_ReturnsMasterPasswordPolicy
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
organizationRepository.GetByIdAsync(orgId).Returns(organization);

sutProvider.GetDependency<IUserService>()
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
.Returns(userId);

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(orgId, userId)
.Returns(orgUser);


policy.Type = PolicyType.MasterPassword;
policy.Enabled = true;
// data should be a JSON serialized version of the mpPolicyData object
Expand Down Expand Up @@ -90,35 +85,17 @@ public async Task GetMasterPasswordPolicy_WhenCalled_ReturnsMasterPasswordPolicy
[Theory]
[BitAutoData]
public async Task GetMasterPasswordPolicy_OrgUserIsNull_ThrowsNotFoundException(
SutProvider<PoliciesController> sutProvider, Guid orgId, Guid userId)
SutProvider<PoliciesController> sutProvider, Guid orgId)
{
// Arrange
sutProvider.GetDependency<IUserService>()
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
.Returns(userId);

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(orgId, userId)
.Returns((OrganizationUser)null);

// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId));
}

[Theory]
[BitAutoData]
public async Task GetMasterPasswordPolicy_PolicyIsNull_ThrowsNotFoundException(
SutProvider<PoliciesController> sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser)
SutProvider<PoliciesController> sutProvider, Guid orgId)
{
// Arrange
sutProvider.GetDependency<IUserService>()
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
.Returns(userId);

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(orgId, userId)
.Returns(orgUser);

sutProvider.GetDependency<IPolicyRepository>()
.GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword)
.Returns((Policy)null);
Expand All @@ -130,17 +107,9 @@ public async Task GetMasterPasswordPolicy_PolicyIsNull_ThrowsNotFoundException(
[Theory]
[BitAutoData]
public async Task GetMasterPasswordPolicy_PolicyNotEnabled_ThrowsNotFoundException(
SutProvider<PoliciesController> sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser, Policy policy)
SutProvider<PoliciesController> sutProvider, Guid orgId, Policy policy)
{
// Arrange
sutProvider.GetDependency<IUserService>()
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
.Returns(userId);

sutProvider.GetDependency<IOrganizationUserRepository>()
.GetByOrganizationAsync(orgId, userId)
.Returns(orgUser);

policy.Enabled = false; // Ensuring the policy is not enabled
sutProvider.GetDependency<IPolicyRepository>()
.GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword)
Expand All @@ -160,7 +129,6 @@ public async Task GetMasterPasswordPolicy_WhenUsePoliciesIsFalse_ThrowsNotFoundE
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
organizationRepository.GetByIdAsync(orgId).Returns((Organization)null);


// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId));
}
Expand All @@ -178,7 +146,6 @@ public async Task GetMasterPasswordPolicy_WhenOrgIsNull_ThrowsNotFoundException(
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
organizationRepository.GetByIdAsync(orgId).Returns(organization);


// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId));
}
Expand Down Expand Up @@ -211,20 +178,6 @@ public async Task Get_WhenUserCanManagePolicies_WithExistingType_ReturnsExisting
Assert.Equal(policy.OrganizationId, result.OrganizationId);
}

[Theory]
[BitAutoData]
public async Task Get_WhenUserCannotManagePolicies_ThrowsNotFoundException(
SutProvider<PoliciesController> sutProvider, Guid orgId, PolicyType type)
{
// Arrange
sutProvider.GetDependency<ICurrentContext>()
.ManagePolicies(orgId)
.Returns(false);

// Act & Assert
await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.Get(orgId, type));
}

[Theory]
[BitAutoData]
public async Task GetByToken_WhenOrganizationUseUsePoliciesIsFalse_ThrowsNotFoundException(
Expand Down Expand Up @@ -473,10 +426,6 @@ await sutProvider.GetDependency<IVNextSavePolicyCommand>()
m.PerformedBy.UserId == userId &&
m.PerformedBy.IsOrganizationOwnerOrProvider == true));

await sutProvider.GetDependency<ISavePolicyCommand>()
.DidNotReceiveWithAnyArgs()
.VNextSaveAsync(default);

Assert.NotNull(result);
Assert.Equal(policy.Id, result.Id);
Assert.Equal(policy.Type, result.Type);
Expand Down Expand Up @@ -515,10 +464,6 @@ await sutProvider.GetDependency<IVNextSavePolicyCommand>()
m.PerformedBy.UserId == userId &&
m.PerformedBy.IsOrganizationOwnerOrProvider == true));

await sutProvider.GetDependency<ISavePolicyCommand>()
.DidNotReceiveWithAnyArgs()
.VNextSaveAsync(default);

Assert.NotNull(result);
Assert.Equal(policy.Id, result.Id);
Assert.Equal(policy.Type, result.Type);
Expand Down
Loading
Loading