Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions clients/dashboard/src/components/layout/nav-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ export const sections: NavSection[] = [
caption: "Identity",
icon: Users,
items: [
{ to: "/identity/users", label: "Users", icon: Users },
{ to: "/identity/roles", label: "Roles", icon: ShieldCheck },
{ to: "/identity/groups", label: "Groups", icon: UsersRound },
// Gate the identity-management pages on a manage permission (not View): View Users/Roles/Groups
// are IsBasic so every member holds them (the chat/user picker relies on Users.View), but only
// managers should see these admin pages. Basic lacks the *.Update perms, so the items hide for them.
{ to: "/identity/users", label: "Users", icon: Users, perm: "Permissions.Users.Update" },
{ to: "/identity/roles", label: "Roles", icon: ShieldCheck, perm: "Permissions.Roles.Update" },
{ to: "/identity/groups", label: "Groups", icon: UsersRound, perm: "Permissions.Groups.Update" },
],
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ internal static IServiceCollection ConfigureJwtAuth(this IServiceCollection serv
services.AddAuthorizationBuilder().AddRequiredPermissionPolicy();
services.AddAuthorization(options =>
{
// Permission evaluation lives in the RequiredPermission policy (it reads each
// endpoint's RequiredPermissionAttribute metadata). Wire it as BOTH the default
// AND the fallback policy:
// - FallbackPolicy covers endpoints with no auth metadata at all.
// - DefaultPolicy covers endpoints that opt in via .RequireAuthorization() —
// including the module route-groups (Catalog/Billing/Chat/Files/…). Without
// this, a group-level .RequireAuthorization() applied the built-in
// authenticated-only default, which SUPPRESSED the fallback, so
// .RequirePermission(...) was never evaluated and any authenticated tenant
// member could perform gated writes. Both must point at the permission policy.
options.DefaultPolicy = options.GetPolicy(RequiredPermissionDefaults.PolicyName)!;
options.FallbackPolicy = options.GetPolicy(RequiredPermissionDefaults.PolicyName);
});
return services;
Expand Down
3 changes: 2 additions & 1 deletion src/Tests/Architecture.Tests/Architecture.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<IsPublishable>false</IsPublishable>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<NoWarn>$(NoWarn);CA1515;CA1861;CA1707;CA1307</NoWarn>
<!-- S125: SonarAnalyzer false-positives on descriptive comments with code-like tokens (not dead code). -->
<NoWarn>$(NoWarn);CA1515;CA1861;CA1707;CA1307;S125</NoWarn>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="coverlet.collector">
Expand Down
5 changes: 3 additions & 2 deletions src/Tests/Integration.Tests/Integration.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
CA1515 - internal types; CA1861 - const arrays; CA1707 - underscores in test names;
CA1307 - StringComparison; CA2234 - Uri vs string; CA2000 - dispose (test scope);
CA1062 - null checks in test helpers; CA1031 - broad catch in test setup;
CA1812 - DI-instantiated test doubles; S1481 - deliberately unused locals in tests -->
<NoWarn>$(NoWarn);CA1515;CA1861;CA1707;CA1307;CA2234;CA2000;CA1062;CA1031;CA1812;CA1056;S1481</NoWarn>
CA1812 - DI-instantiated test doubles; S1481 - deliberately unused locals in tests;
S125 - false-positives on descriptive test comments containing code-like tokens -->
<NoWarn>$(NoWarn);CA1515;CA1861;CA1707;CA1307;CA2234;CA2000;CA1062;CA1031;CA1812;CA1056;S1481;S125</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
67 changes: 67 additions & 0 deletions src/Tests/Integration.Tests/Tests/Catalog/ProductsEndpointTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
using Finbuckle.MultiTenant;
using Finbuckle.MultiTenant.Abstractions;
using FSH.Framework.Shared.Multitenancy;
using FSH.Modules.Catalog.Contracts.Dtos;
using FSH.Modules.Identity.Domain;
using Integration.Tests.Infrastructure;
using Integration.Tests.Infrastructure.Extensions;
using Microsoft.AspNetCore.Identity;

namespace Integration.Tests.Tests.Catalog;

Expand Down Expand Up @@ -469,6 +474,68 @@ public async Task ListTrashedProducts_Should_Return401_When_Unauthenticated()
response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
}

[Fact]
public async Task CreateProduct_Should_Return403_When_AuthenticatedUserLacksPermission()
{
// Regression (broken access control): catalog endpoints sit behind a group-level
// .RequireAuthorization(), whose default policy only required authentication. That
// shadowed the permission FallbackPolicy, so .RequirePermission(Products.Create) was
// never evaluated — ANY authenticated tenant member could create products. A seeded
// user holding no catalog-write permission MUST be Forbidden, not fail open.
var (email, password) = await CreateActiveNoPermissionUserAsync($"noperm{Guid.NewGuid():N}"[..14]);
using var userClient = await _auth.CreateAuthenticatedClientAsync(email, password);

using var response = await userClient.PostAsJsonAsync(
$"{TestConstants.CatalogBasePath}/products",
new
{
sku = UniqueSku("Forbidden"),
name = "no-permission user must not create products",
description = (string?)null,
brandId = Guid.NewGuid(),
categoryId = Guid.NewGuid(),
priceAmount = 1m,
priceCurrency = "USD",
stock = 1,
});

response.StatusCode.ShouldBe(HttpStatusCode.Forbidden,
"Catalog permission gates must not fail open: a user without Permissions.Products.Create must be rejected.");
}

/// <summary>
/// Seeds a confirmed + active root-tenant user with NO roles (hence no catalog
/// permissions) via UserManager. Tenant context is set INLINE because it is AsyncLocal —
/// an awaited-helper set would be lost and the tenant query filter would throw.
/// </summary>
private async Task<(string Email, string Password)> CreateActiveNoPermissionUserAsync(string handle)
{
const string password = TestConstants.DefaultPassword;
var email = $"{handle}@example.com";

using var scope = _factory.Services.CreateScope();
var tenant = await scope.ServiceProvider
.GetRequiredService<IMultiTenantStore<AppTenantInfo>>()
.GetAsync(TestConstants.RootTenantId);
scope.ServiceProvider.GetRequiredService<IMultiTenantContextSetter>()
.MultiTenantContext = new MultiTenantContext<AppTenantInfo>(tenant);

var userManager = scope.ServiceProvider.GetRequiredService<UserManager<FshUser>>();
var user = new FshUser
{
FirstName = "No",
LastName = "Perm",
Email = email,
UserName = handle,
EmailConfirmed = true,
IsActive = true,
};
var result = await userManager.CreateAsync(user, password);
result.Succeeded.ShouldBeTrue(
$"Seeding active user failed: {string.Join(", ", result.Errors.Select(e => e.Description))}");
return (email, password);
}

// ─── idempotency ─────────────────────────────────────────────────

[Fact]
Expand Down
Loading