diff --git a/bitwarden_license/src/Services/Pam/Api/Endpoints/AccessRuleEndpoints.cs b/bitwarden_license/src/Services/Pam/Api/Endpoints/AccessRuleEndpoints.cs index d5cc8aaa2f1b..4c47f38ef87a 100644 --- a/bitwarden_license/src/Services/Pam/Api/Endpoints/AccessRuleEndpoints.cs +++ b/bitwarden_license/src/Services/Pam/Api/Endpoints/AccessRuleEndpoints.cs @@ -1,4 +1,6 @@ -using Bit.Services.Pam.Api.Endpoints.Handlers; +using Bit.Api.AdminConsole.Authorization; +using Bit.Api.AdminConsole.Authorization.Requirements; +using Bit.Services.Pam.Api.Endpoints.Handlers; using Bit.Services.Pam.Api.Models.Request; namespace Bit.Services.Pam.Api.Endpoints; @@ -7,11 +9,26 @@ namespace Bit.Services.Pam.Api.Endpoints; /// The organizations/{orgId}/access-rules resource: rule CRUD scoped to an organization. orgId is /// bound from the group's route prefix. /// +/// +/// Authorization runs in the middleware rather than the handler. The group requires organization membership and the +/// write endpoints additionally require ; ASP.NET combines the group and +/// endpoint policies, so a write has to satisfy both. Both requirements read orgId off the route, which the +/// group prefix supplies. +/// +/// The group requirement is deliberately and not +/// MemberOrProviderRequirement: providers manage an organization's billing and configuration, but access +/// rules gate who can lease credentials out of it, which is not theirs to change. Note this group gate is the only +/// thing keeping providers out — derives from +/// BasePermissionRequirement, which falls back to authorizing a provider for the organization. Removing or +/// weakening the group requirement would silently readmit them to the write endpoints. +/// +/// internal static class AccessRuleEndpoints { public static RouteGroupBuilder MapAccessRuleEndpoints(this RouteGroupBuilder group) { group.WithTags("AccessRules"); + group.RequireAuthorization(new AuthorizeAttribute()); group.MapGet("", (Guid orgId, AccessRuleEndpointsHandler handler) => handler.GetAll(orgId)) .WithName("Pam_AccessRules_GetAll"); @@ -20,10 +37,12 @@ public static RouteGroupBuilder MapAccessRuleEndpoints(this RouteGroupBuilder gr .WithName("Pam_AccessRules_Get"); group.MapPost("", (Guid orgId, AccessRuleRequestModel model, AccessRuleEndpointsHandler handler) => handler.Post(orgId, model)) - .WithName("Pam_AccessRules_Post"); + .WithName("Pam_AccessRules_Post") + .RequireAuthorization(new AuthorizeAttribute()); group.MapPut("{id:guid}", (Guid orgId, Guid id, AccessRuleRequestModel model, AccessRuleEndpointsHandler handler) => handler.Put(orgId, id, model)) - .WithName("Pam_AccessRules_Put"); + .WithName("Pam_AccessRules_Put") + .RequireAuthorization(new AuthorizeAttribute()); group.MapDelete("{id:guid}", async (Guid orgId, Guid id, AccessRuleEndpointsHandler handler) => @@ -31,7 +50,8 @@ public static RouteGroupBuilder MapAccessRuleEndpoints(this RouteGroupBuilder gr await handler.Delete(orgId, id); return TypedResults.NoContent(); }) - .WithName("Pam_AccessRules_Delete"); + .WithName("Pam_AccessRules_Delete") + .RequireAuthorization(new AuthorizeAttribute()); return group; } diff --git a/bitwarden_license/src/Services/Pam/Pam.csproj b/bitwarden_license/src/Services/Pam/Pam.csproj index 264ff0119b1a..e80fe52e7579 100644 --- a/bitwarden_license/src/Services/Pam/Pam.csproj +++ b/bitwarden_license/src/Services/Pam/Pam.csproj @@ -26,6 +26,7 @@ + diff --git a/bitwarden_license/src/Services/Pam/packages.lock.json b/bitwarden_license/src/Services/Pam/packages.lock.json index c77e4b8f25cd..8b97f591c54a 100644 --- a/bitwarden_license/src/Services/Pam/packages.lock.json +++ b/bitwarden_license/src/Services/Pam/packages.lock.json @@ -910,6 +910,12 @@ "httpextensions": { "type": "Project" }, + "organizationauthorization": { + "type": "Project", + "dependencies": { + "Core": "[2026.7.2, )" + } + }, "serilogfilelogging": { "type": "Project", "dependencies": { diff --git a/bitwarden_license/test/Services/Pam.Test/Api/Endpoints/AccessRuleEndpointsTests.cs b/bitwarden_license/test/Services/Pam.Test/Api/Endpoints/AccessRuleEndpointsTests.cs index 79d8ee82daef..a394786bf6f3 100644 --- a/bitwarden_license/test/Services/Pam.Test/Api/Endpoints/AccessRuleEndpointsTests.cs +++ b/bitwarden_license/test/Services/Pam.Test/Api/Endpoints/AccessRuleEndpointsTests.cs @@ -1,8 +1,11 @@ -using Bit.Core.Models.Api; +using Bit.Api.AdminConsole.Authorization.Requirements; +using Bit.Core.Auth.Identity; +using Bit.Core.Models.Api; using Bit.HttpExtensions; using Bit.Services.Pam.Api.Endpoints; using Bit.Services.Pam.Api.Endpoints.Handlers; using Bit.Services.Pam.Api.Models.Response; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; @@ -84,6 +87,79 @@ public void AccessRuleGroup_DocumentsErrorResponseModel_For400And404() Assert.Contains(produces, p => p.StatusCode == StatusCodes.Status404NotFound && p.Type == typeof(ErrorResponseModel)); } + /// + /// Collects the authorization requirements an endpoint carries. They arrive as two shapes of metadata: + /// AuthorizeAttribute<T> contributes , while a policy + /// built inline contributes an . AuthorizationMiddleware combines both, so a + /// test asking "what must this endpoint satisfy" has to read both. + /// + private static List RequirementsFor(Endpoint endpoint) => + [ + .. endpoint.Metadata.GetOrderedMetadata().SelectMany(policy => policy.Requirements), + .. endpoint.Metadata.GetOrderedMetadata().SelectMany(data => data.GetRequirements()) + ]; + + [Theory] + [InlineData("Pam_AccessRules_GetAll", typeof(MemberRequirement))] + [InlineData("Pam_AccessRules_Get", typeof(MemberRequirement))] + [InlineData("Pam_AccessRules_Post", typeof(ManageAccessRulesRequirement))] + [InlineData("Pam_AccessRules_Put", typeof(ManageAccessRulesRequirement))] + [InlineData("Pam_AccessRules_Delete", typeof(ManageAccessRulesRequirement))] + public void MapPamEndpoints_AuthorizesRouteWithRequirement(string name, Type requirementType) + { + // Reads require membership; writes require authority over rule authorship. The requirements are carried as + // endpoint metadata, which AuthorizationMiddleware combines with the group's Policies.Application. + var endpoint = Assert.Single( + MaterializeEndpoints(), + e => e.Metadata.GetMetadata()?.EndpointName == name); + + Assert.Contains(RequirementsFor(endpoint), r => r.GetType() == requirementType); + + // The per-route requirement adds to the group's policy rather than replacing it. + Assert.Contains(endpoint.Metadata.GetOrderedMetadata(), + data => data.Policy == Policies.Application); + } + + [Fact] + public void MapPamEndpoints_AccessRuleWritesRequireMembershipBesidesThePermission() + { + // ManageAccessRulesRequirement derives from BasePermissionRequirement, whose final arm authorizes any + // provider for the organization. The group's MemberRequirement is the only thing keeping providers out of + // rule authorship, so each write must carry it *in addition to* the permission — the permission alone would + // admit them. + var writeRoutes = new[] { "Pam_AccessRules_Post", "Pam_AccessRules_Put", "Pam_AccessRules_Delete" }; + + var endpoints = MaterializeEndpoints() + .Where(e => writeRoutes.Contains(e.Metadata.GetMetadata()?.EndpointName)) + .ToList(); + + Assert.Equal(writeRoutes.Length, endpoints.Count); + Assert.All(endpoints, endpoint => + { + var requirements = RequirementsFor(endpoint); + Assert.Contains(requirements, r => r is MemberRequirement); + Assert.Contains(requirements, r => r is ManageAccessRulesRequirement); + }); + } + + [Fact] + public void MapPamEndpoints_AccessRulesNeverAuthorizeProvidersByMembership() + { + // Access rules gate who can lease credentials out of an organization, which is not a provider's to read or + // change. MemberOrProviderRequirement would let them in, so no access-rule route may carry it. + var endpoints = MaterializeEndpoints() + .Where(e => e.Metadata.GetMetadata()!.Tags.Contains("AccessRules")) + .ToList(); + + Assert.Equal(5, endpoints.Count); + Assert.All(endpoints, endpoint => + { + var requirements = RequirementsFor(endpoint); + Assert.Contains(requirements, r => r is MemberRequirement); + Assert.DoesNotContain(requirements, r => r is MemberOrProviderRequirement); + }); + } + [Theory] [InlineData(nameof(AccessRuleEndpointsHandler.GetAll), typeof(Task>))] [InlineData(nameof(AccessRuleEndpointsHandler.Get), typeof(Task))] diff --git a/bitwarden_license/test/Services/Pam.Test/packages.lock.json b/bitwarden_license/test/Services/Pam.Test/packages.lock.json index f9f6a0791440..0f5dd369d90b 100644 --- a/bitwarden_license/test/Services/Pam.Test/packages.lock.json +++ b/bitwarden_license/test/Services/Pam.Test/packages.lock.json @@ -1323,11 +1323,18 @@ "httpextensions": { "type": "Project" }, + "organizationauthorization": { + "type": "Project", + "dependencies": { + "Core": "[2026.7.2, )" + } + }, "pam": { "type": "Project", "dependencies": { "Core": "[2026.7.2, )", - "HttpExtensions": "[2026.7.2, )" + "HttpExtensions": "[2026.7.2, )", + "OrganizationAuthorization": "[0.0.1, )" } }, "serilogfilelogging": { diff --git a/src/Api/packages.lock.json b/src/Api/packages.lock.json index 5c8515b0a4fa..ec16c21fcdbc 100644 --- a/src/Api/packages.lock.json +++ b/src/Api/packages.lock.json @@ -1276,7 +1276,8 @@ "type": "Project", "dependencies": { "Core": "[2026.7.2, )", - "HttpExtensions": "[2026.7.2, )" + "HttpExtensions": "[2026.7.2, )", + "OrganizationAuthorization": "[0.0.1, )" } }, "serilogfilelogging": { diff --git a/src/Libraries/OrganizationAuthorization/Organizations/Requirements/PermissionRequirements.cs b/src/Libraries/OrganizationAuthorization/Organizations/Requirements/PermissionRequirements.cs index d50c74b971bb..81b2e1267dba 100644 --- a/src/Libraries/OrganizationAuthorization/Organizations/Requirements/PermissionRequirements.cs +++ b/src/Libraries/OrganizationAuthorization/Organizations/Requirements/PermissionRequirements.cs @@ -3,6 +3,7 @@ public class AccessEventLogsRequirement() : BasePermissionRequirement(p => p.AccessEventLogs); public class AccessImportExportRequirement() : BasePermissionRequirement(p => p.AccessImportExport); public class AccessReportsRequirement() : BasePermissionRequirement(p => p.AccessReports); +public class ManageAccessRulesRequirement() : BasePermissionRequirement(p => p.ManageAccessRules); public class ManageAccountRecoveryRequirement() : BasePermissionRequirement(p => p.ManageResetPassword); public class ManageGroupsRequirement() : BasePermissionRequirement(p => p.ManageGroups); public class ManagePoliciesRequirement() : BasePermissionRequirement(p => p.ManagePolicies); diff --git a/test/Api.IntegrationTest/packages.lock.json b/test/Api.IntegrationTest/packages.lock.json index fb98ea7ad115..f171c0e89520 100644 --- a/test/Api.IntegrationTest/packages.lock.json +++ b/test/Api.IntegrationTest/packages.lock.json @@ -1554,7 +1554,8 @@ "type": "Project", "dependencies": { "Core": "[2026.7.2, )", - "HttpExtensions": "[2026.7.2, )" + "HttpExtensions": "[2026.7.2, )", + "OrganizationAuthorization": "[0.0.1, )" } }, "rustsdk": { diff --git a/test/Api.Test/packages.lock.json b/test/Api.Test/packages.lock.json index 7c5ff5a4451c..d03c7f84177d 100644 --- a/test/Api.Test/packages.lock.json +++ b/test/Api.Test/packages.lock.json @@ -1892,7 +1892,8 @@ "type": "Project", "dependencies": { "Core": "[2026.7.2, )", - "HttpExtensions": "[2026.7.2, )" + "HttpExtensions": "[2026.7.2, )", + "OrganizationAuthorization": "[0.0.1, )" } }, "serilogfilelogging": { diff --git a/test/Billing.IntegrationTest/packages.lock.json b/test/Billing.IntegrationTest/packages.lock.json index 222098031e64..574c8151dd50 100644 --- a/test/Billing.IntegrationTest/packages.lock.json +++ b/test/Billing.IntegrationTest/packages.lock.json @@ -2099,7 +2099,8 @@ "type": "Project", "dependencies": { "Core": "[2026.7.2, )", - "HttpExtensions": "[2026.7.2, )" + "HttpExtensions": "[2026.7.2, )", + "OrganizationAuthorization": "[0.0.1, )" } }, "postgresmigrations": { diff --git a/test/Libraries/OrganizationAuthorization.Test/Organizations/Requirements/PermissionRequirementsTests.cs b/test/Libraries/OrganizationAuthorization.Test/Organizations/Requirements/PermissionRequirementsTests.cs index 1acfbd5be33b..0c8cd105249f 100644 --- a/test/Libraries/OrganizationAuthorization.Test/Organizations/Requirements/PermissionRequirementsTests.cs +++ b/test/Libraries/OrganizationAuthorization.Test/Organizations/Requirements/PermissionRequirementsTests.cs @@ -21,6 +21,7 @@ public class PermissionRequirementsTests new object[] { new AccessEventLogsRequirement(), nameof(Permissions.AccessEventLogs) }, new object[] { new AccessImportExportRequirement(), nameof(Permissions.AccessImportExport) }, new object[] { new AccessReportsRequirement(), nameof(Permissions.AccessReports) }, + new object[] { new ManageAccessRulesRequirement(), nameof(Permissions.ManageAccessRules) }, new object[] { new ManageAccountRecoveryRequirement(), nameof(Permissions.ManageResetPassword) }, new object[] { new ManageGroupsRequirement(), nameof(Permissions.ManageGroups) }, new object[] { new ManagePoliciesRequirement(), nameof(Permissions.ManagePolicies) },