diff --git a/docs/CONSTITUTION.md b/docs/CONSTITUTION.md index 7a90f636..7cdb3a7c 100644 --- a/docs/CONSTITUTION.md +++ b/docs/CONSTITUTION.md @@ -369,11 +369,27 @@ public sealed class CreateProductRequest : FormRequest - `.AllowAnonymous()` for public endpoints. - Never hardcode permission strings -- always use the permission constants. +### Instance-Level Policies + +String permissions answer "can this user perform this kind of action?". Policies answer "can this user perform this action on *this* resource?" — ownership, tenancy, and state-machine rules. + +- Implement `IPolicy` (in `SimpleModule.Core.Authorization.Policies`) in the module that owns the resource (SM0060). Policies in assemblies that map to no module — a host assembly without its own `[Module]` class, or a shared library — are exempt: the composition root may layer host-wide rules on any resource. Policy classes must be effectively `public` (SM0059) and non-generic (SM0061); they are auto-discovered by the source generator — in implementation, contracts, and host assemblies, including nested classes — and registered as scoped services. Type-based manual registrations are deduplicated; factory registrations must use the two-generic `AddScoped(factory)` overload, or opt out with `[ManualContractRegistration]`. +- The resource type must be a contracts DTO — a `[Dto]` type or a type declared in a `.Contracts` assembly (SM0058). Policies guard resources that cross module boundaries. +- Policies **complement** permissions, they do not replace them: keep `.RequirePermission()` on the endpoint as the coarse capability gate, then check the instance rule via `IAuthorizer` (or the declarative `.AuthorizeResource()` filter backed by an `IResourceResolver`). +- Endpoints follow load → authorize → act: fetch the resource, call `IAuthorizer.AuthorizeAsync(user, action, resource)`, then perform the operation. Denial throws `ForbiddenException` (403). For anti-enumeration, return `AuthorizationResult.DenyAsNotFound(...)` from the policy — the decision travels with the policy that knows the resource. The host-level `PolicyAuthorizationOptions.NotFoundActions` set (empty by default) is a blunt host-wide override; modules must not mutate it. +- Use `PolicyActions` constants for CRUD verbs; declare module-specific actions as `public const string` on the policy class. +- Multiple policies may target the same resource type (e.g. a tenancy policy plus an ownership policy); a single deny wins. +- Keep contract methods owner-scoped (defense in depth for in-process callers); unscoped loaders used by the load → authorize → act flow belong on a module-internal interface, never on the public contract. +- Collection scoping stays in queries (`WHERE UserId = @me`) — policies are for single-instance checks, not list filtering. +- Policies do not inherit the Admin permission bypass — admin exemptions are an explicit, per-policy decision. +- Reference implementation: `NotificationPolicy` in `modules/Notifications`. + ### Rules - Permissions are owned by the defining module. - Other modules may reference permission constants from Contracts. - Not every module needs permissions. +- Policies are owned by the module that owns the resource — never write a policy for another module's entity. --- @@ -535,6 +551,15 @@ All SM diagnostics are emitted by the Roslyn source generator at compile time. ` | SM0056 | Error | FormRequest class must be sealed | | SM0057 | Error | FormRequest class must extend `FormRequest` | +### Policies + +| Diagnostic | Severity | Rule | +|------------|----------|------| +| SM0058 | Error | Policy resource type must be a contracts DTO | +| SM0059 | Error | Policy class must be public | +| SM0060 | Error | Policy must be owned by the resource's module | +| SM0061 | Error | Policy class must not be generic | + ### Module Metadata | Diagnostic | Severity | Rule | diff --git a/docs/site/.vitepress/config.ts b/docs/site/.vitepress/config.ts index 6b4e0e26..815de16a 100644 --- a/docs/site/.vitepress/config.ts +++ b/docs/site/.vitepress/config.ts @@ -53,6 +53,7 @@ export default defineConfig({ { text: 'Events', link: '/guide/events' }, { text: 'Broadcasting', link: '/guide/broadcasting' }, { text: 'Permissions', link: '/guide/permissions' }, + { text: 'Policies', link: '/guide/policies' }, { text: 'Menus', link: '/guide/menus' }, { text: 'Settings', link: '/guide/settings' }, { text: 'Inertia.js Integration', link: '/guide/inertia' }, diff --git a/docs/site/guide/permissions.md b/docs/site/guide/permissions.md index daa6b1d6..e2609424 100644 --- a/docs/site/guide/permissions.md +++ b/docs/site/guide/permissions.md @@ -6,6 +6,8 @@ outline: deep SimpleModule includes a claims-based permission system that integrates with ASP.NET Core authorization. Each module defines its own permissions, registers them with the framework, and protects endpoints using attributes or extension methods. +Permissions are coarse capability gates ("can this user update products at all?"). For instance-level rules — ownership, tenancy, state-based access — layer a [policy](/guide/policies) on top. + ## Overview The permission system has three layers: diff --git a/docs/site/guide/policies.md b/docs/site/guide/policies.md new file mode 100644 index 00000000..d8b77f69 --- /dev/null +++ b/docs/site/guide/policies.md @@ -0,0 +1,200 @@ +--- +outline: deep +--- + +# Policies + +Permissions answer "can this user perform this kind of action?" (`Products.Update`). They cannot express instance-level rules like "users may only edit *their own* orders" or "tenant admins may only manage users *in their tenant*". Policies fill that gap. + +A policy is a class that encapsulates every per-resource authorization rule for one entity, inspired by [Laravel's policy classes](https://laravel.com/docs/authorization#creating-policies). Policies **layer on top of** permissions — the permission stays on the endpoint as the coarse capability gate; the policy adds the instance check. + +The framework ships three reference policies: + +| Module | Policy | Rule | +|--------|--------|------| +| Notifications | `NotificationPolicy` | recipient-only; declarative `AuthorizeResource` form | +| FileStorage | `FileStoragePolicy` | uploader-or-admin; imperative `IAuthorizer` form | +| Users | `UserPolicy` | self-or-admin view/update, admin-only delete | + +## Defining a Policy + +Implement `IPolicy` in the module that owns the resource (SM0060). The class must be effectively `public` (SM0059) and non-generic (SM0061), and the resource type must be a contracts DTO — a `[Dto]` type or a type declared in your `.Contracts` assembly (SM0058). + +```csharp +using System.Security.Claims; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Extensions; + +namespace SimpleModule.Notifications; + +public sealed class NotificationPolicy : IPolicy +{ + // Module-specific action beyond the conventional CRUD verbs + public const string MarkRead = "markRead"; + + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Notification resource, + CancellationToken cancellationToken = default + ) + { + var result = action switch + { + PolicyActions.View or MarkRead => AllowOwner(user, resource), + _ => AuthorizationResult.Deny($"Unknown notification action '{action}'."), + }; + return Task.FromResult(result); + } + + private static AuthorizationResult AllowOwner(ClaimsPrincipal user, Notification notification) + { + var userId = user.GetUserId(); + return userId is not null && notification.UserId == UserId.From(userId) + ? AuthorizationResult.Allow() + : AuthorizationResult.DenyAsNotFound("You can only access your own notifications."); + } +} +``` + +Two things worth noting in this example: + +- **Admins are not exempt.** Permission checks bypass for the Admin role; policies do not. If an admin should pass an instance rule, the policy must say so explicitly — here it deliberately doesn't, because marking read mutates the recipient's inbox state. +- **`DenyAsNotFound`** makes the denial surface as 404 instead of 403, so callers cannot probe which notification IDs exist. + +### Registration + +There is no registration step. The source generator discovers every `IPolicy` implementation — in implementation, contracts, and host assemblies, including nested classes — and registers it as a scoped service in the generated `AddModules()`: + +```csharp +// generated +services.TryAddEnumerable(ServiceDescriptor.Scoped, NotificationPolicy>()); +``` + +`TryAddEnumerable` deduplicates **type-based** manual registrations (`AddScoped, XPolicy>()`). If you must register via factory, use the two-generic overload — `AddScoped, XPolicy>(sp => ...)` — so the implementation type is visible to dedup, or opt out of auto-registration entirely with `[ManualContractRegistration]` on the policy class. An opted-out policy is wired by its own module, so the auto-registration rules SM0059 (public) and SM0061 (non-generic) are waived for it; the resource rules (SM0058, SM0060) still apply. + +Use the `PolicyActions` constants (`view`, `create`, `update`, `delete`) for conventional verbs, and declare module-specific actions as `public const string` on the policy class so endpoints never hardcode action strings. + +## Declarative Checks: AuthorizeResource + +When the handler doesn't need the resource beyond authorizing it, declare the check on the endpoint. This is what the reference module does — register an `IResourceResolver` once per resource type: + +```csharp +// modules/Notifications/.../Endpoints/Notifications/NotificationResolver.cs +internal sealed class NotificationResolver(INotificationStore store) + : IResourceResolver +{ + public async ValueTask ResolveAsync( + string routeValue, + CancellationToken cancellationToken = default + ) => + Guid.TryParse(routeValue, out var id) + ? await store.FindAsync(NotificationId.From(id), cancellationToken) + : null; +} + +// In ConfigureServices +services.AddScoped, NotificationResolver>(); +``` + +Then the endpoint declares the check instead of performing it: + +```csharp +app.MapPost( + Route, + async Task (Guid id, HttpContext context, INotificationsContracts notifications) => + { + // AuthorizeResource already loaded the notification and ran the policy; + // the contract call stays owner-scoped (defense in depth). + var ok = await notifications.MarkReadAsync( + NotificationId.From(id), + UserId.From(context.User.GetUserId()!) + ); + return ok ? TypedResults.NoContent() : TypedResults.NotFound(); + } + ) + .RequirePermission(NotificationsPermissions.ViewOwn) // permission gate stays + .AuthorizeResource(NotificationPolicy.MarkRead); // route param "id" by default +``` + +The filter loads the resource (404 when missing — including when an optional route parameter is omitted), authorizes the action, and only then invokes the handler. True misconfiguration fails loudly: a route parameter name that doesn't exist in the template, or a missing resolver registration, throws `InvalidOperationException` rather than masquerading as 404. + +::: warning Ordering with Form Requests +Form Request validation is attached at the route-group level and therefore runs **before** endpoint-level filters like `AuthorizeResource`. An endpoint combining both validates the caller's payload before the instance-level authorization check — harmless for security (validation only reflects the caller's own input), but the validation work happens even for requests the policy will deny. Prefer the imperative `IAuthorizer` flow if that ordering matters. +::: + +## Imperative Checks: IAuthorizer + +When the handler needs the loaded resource (to render it, mutate it in place, or branch on its state), inject `IAuthorizer` and follow **load → authorize → act**: + +```csharp +var order = await store.FindAsync(OrderId.From(id), ct); +if (order is null) +{ + return TypedResults.NotFound(); +} + +// Throws on deny — translated by the global exception handler. +await authorizer.AuthorizeAsync(context.User, PolicyActions.Update, order, ct); + +// ... act on the loaded order +``` + +To branch instead of throwing, use `CheckAsync`, which returns the `AuthorizationResult` (with `IsAllowed`, `Reason`, and `TreatAsNotFound`). + +### Keep contracts owner-scoped + +The policy check protects the *endpoint*, not in-process callers. Methods on your public `I{Module}Contracts` interface should stay scoped (`MarkReadAsync(id, userId)` filters by owner) so another module can never mutate or read a foreign user's data by accident. The unscoped loader the policy flow needs (`FindAsync(id)`) belongs on a **module-internal** interface — in Notifications that is `INotificationStore`. + +## Denial Semantics: 403 vs 404 + +A denied check throws `ForbiddenException` (403) with the policy's denial reason. Reasons are returned verbatim in the response detail — write them for the end user and never include internal identifiers. + +To surface a denial as 404 instead (anti-enumeration), return `AuthorizationResult.DenyAsNotFound(...)` from the policy: the policy knows whether a resource's existence is secret, and the decision travels with it. The host-level `PolicyAuthorizationOptions.NotFoundActions` set (empty by default) additionally maps every denial for the listed action names to 404 — it is a blunt host-wide override that also swallows explicit `Deny(reason)` messages, so prefer `DenyAsNotFound` and configure the option only at the host level; modules must not mutate it. + +Calling `AuthorizeAsync`/`CheckAsync` for a resource type with **no registered policy throws `MissingPolicyException`** — authorization fails closed and loudly rather than silently allowing. + +## Multiple Policies per Resource + +More than one policy may target the same resource type — for example a tenancy-scoping policy plus an ownership policy, both owned by the resource's module (SM0060 rejects policies for other modules' resources). Policies run in registration order and evaluation stops at the first deny: **a single deny wins**, and its reason is surfaced. Allow requires every policy to allow. + +## What Policies Are Not For + +- **List filtering** — collection scoping belongs in queries (`WHERE UserId = @me`), not in per-item policy checks. Policies guard single instances. +- **Coarse capabilities** — "can this user use this feature at all" stays a permission string on the endpoint. +- **Other modules' entities** — a policy lives in the module that owns the resource; SM0060 enforces it for module code. Policies in assemblies that map to no module — a host assembly without its own `[Module]` class, or a shared library — are exempt, since the composition root may layer host-wide rules on any resource. Use that power deliberately: such a policy participates in deny-wins for the module's resource, and if your host declares its own `[Module]`, its policies count as that module's and SM0060 applies. +- **Replacing service-level scoping** — contract methods keep their owner filters; policies add endpoint-level semantics on top, they don't substitute for defense in depth. + +## Rules Summary + +| Rule | Enforced by | +|------|-------------| +| Resource type must be a contracts DTO | SM0058 (build error) | +| Policy class must be effectively public | SM0059 (build error; waived for `[ManualContractRegistration]`) | +| Policy must be owned by the resource's module | SM0060 (build error; module code only — unmapped host/shared assemblies are exempt) | +| Policy class must not be generic | SM0061 (build error; waived for `[ManualContractRegistration]`) | +| Policies auto-registered as scoped services | Source generator (`TryAddEnumerable`) | +| Missing policy at check time fails closed | `MissingPolicyException` | +| Deny wins across multiple policies | `IAuthorizer` | +| Denial as 404 | `DenyAsNotFound` per decision (preferred); `PolicyAuthorizationOptions` host override | + +## Testing Policies + +Policies are plain classes — unit test them directly without any host: + +```csharp +[Fact] +public async Task NonOwner_IsDeniedAsNotFound() +{ + var policy = new NotificationPolicy(); + var user = new ClaimsPrincipal( + new ClaimsIdentity([new Claim("sub", "user-2")], "test") + ); + var notification = new Notification { UserId = UserId.From("user-1") }; + + var result = await policy.AuthorizeAsync(user, PolicyActions.View, notification); + + result.IsAllowed.Should().BeFalse(); + result.TreatAsNotFound.Should().BeTrue(); +} +``` diff --git a/framework/SimpleModule.Core/Authorization/Policies/AuthorizationResult.cs b/framework/SimpleModule.Core/Authorization/Policies/AuthorizationResult.cs new file mode 100644 index 00000000..1c72604e --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/AuthorizationResult.cs @@ -0,0 +1,45 @@ +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Outcome of a policy check. Use , , or +/// to construct. +/// +public sealed class AuthorizationResult +{ + private static readonly AuthorizationResult AllowedResult = new(true, null, false); + + private AuthorizationResult(bool isAllowed, string? reason, bool treatAsNotFound) + { + IsAllowed = isAllowed; + Reason = reason; + TreatAsNotFound = treatAsNotFound; + } + + public bool IsAllowed { get; } + + /// + /// Optional human-readable denial reason, surfaced verbatim in the 403 response + /// detail. Write it for the end user — never include internal identifiers or + /// implementation details. + /// + public string? Reason { get; } + + /// + /// When true, surfaces this denial as 404 + /// instead of 403, hiding the resource's existence from unauthorized callers. + /// + public bool TreatAsNotFound { get; } + + public static AuthorizationResult Allow() => AllowedResult; + + public static AuthorizationResult Deny(string? reason = null) => new(false, reason, false); + + /// + /// Denies and asks the authorizer to respond 404 instead of 403 — use when the + /// caller must not learn whether the resource exists (anti-enumeration). The + /// is kept for + /// consumers but never reaches the HTTP response. + /// + public static AuthorizationResult DenyAsNotFound(string? reason = null) => + new(false, reason, true); +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/Authorizer.cs b/framework/SimpleModule.Core/Authorization/Policies/Authorizer.cs new file mode 100644 index 00000000..54282533 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/Authorizer.cs @@ -0,0 +1,68 @@ +using System.Security.Claims; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using SimpleModule.Core.Exceptions; + +namespace SimpleModule.Core.Authorization.Policies; + +public sealed class Authorizer( + IServiceProvider serviceProvider, + IOptions options +) : IAuthorizer +{ + public async Task CheckAsync( + ClaimsPrincipal user, + string action, + TResource resource, + CancellationToken cancellationToken = default + ) + { + // Fail-closed invariant: if the loop below never runs (no policy registered for + // the resource type), this is a developer error and must throw — never allow. + // The flag avoids copying the already-materialized GetServices array on a path + // that runs once per guarded request. + var evaluatedAnyPolicy = false; + foreach (var policy in serviceProvider.GetServices>()) + { + evaluatedAnyPolicy = true; + var result = await policy + .AuthorizeAsync(user, action, resource, cancellationToken) + .ConfigureAwait(false); + if (!result.IsAllowed) + { + return result; + } + } + + if (!evaluatedAnyPolicy) + { + throw MissingPolicyException.ForResource(typeof(TResource)); + } + + return AuthorizationResult.Allow(); + } + + public async Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + TResource resource, + CancellationToken cancellationToken = default + ) + { + var result = await CheckAsync(user, action, resource, cancellationToken) + .ConfigureAwait(false); + if (result.IsAllowed) + { + return; + } + + if (result.TreatAsNotFound || options.Value.NotFoundActions.Contains(action)) + { + throw new NotFoundException(); + } + + throw result.Reason is null + ? new ForbiddenException() + : new ForbiddenException(result.Reason); + } +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/EndpointPolicyExtensions.cs b/framework/SimpleModule.Core/Authorization/Policies/EndpointPolicyExtensions.cs new file mode 100644 index 00000000..f4a53602 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/EndpointPolicyExtensions.cs @@ -0,0 +1,79 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using SimpleModule.Core.Exceptions; + +namespace SimpleModule.Core.Authorization.Policies; + +public static class EndpointPolicyExtensions +{ + /// + /// Declarative policy check: before the handler runs, loads the resource via the + /// module's using the + /// route value, then authorizes + /// against the registered . + /// Missing resource surfaces 404; denial surfaces 403 (or 404 per + /// ). + /// + public static RouteHandlerBuilder AuthorizeResource( + this RouteHandlerBuilder builder, + string action, + string routeParameterName = "id" + ) + { + return builder.AddEndpointFilter(async (context, next) => + { + var httpContext = context.HttpContext; + var routeValue = httpContext.GetRouteValue(routeParameterName)?.ToString(); + if (string.IsNullOrEmpty(routeValue)) + { + // Distinguish runtime absence from misconfiguration: an optional or + // catch-all parameter that exists in the template but wasn't supplied + // is a 404; a parameter name that isn't in the template at all is a + // developer error and must fail loudly. + var templateHasParameter = + (httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern.GetParameter( + routeParameterName + ) + is not null; + if (templateHasParameter) + { + throw new NotFoundException(); + } + + throw new InvalidOperationException( + $"AuthorizeResource<{typeof(TResource).Name}> found no '{routeParameterName}' " + + "route parameter in the endpoint's route template. Pass the route " + + "parameter name used in the template (default is \"id\")." + ); + } + + var resolver = httpContext.RequestServices.GetService>(); + if (resolver is null) + { + throw new InvalidOperationException( + $"No IResourceResolver<{typeof(TResource).Name}> is registered. " + + "Register one in the owning module's ConfigureServices to use " + + "AuthorizeResource, or perform the check imperatively via IAuthorizer." + ); + } + + var resource = await resolver.ResolveAsync(routeValue, httpContext.RequestAborted); + if (resource is null) + { + throw new NotFoundException(); + } + + var authorizer = httpContext.RequestServices.GetRequiredService(); + await authorizer.AuthorizeAsync( + httpContext.User, + action, + resource, + httpContext.RequestAborted + ); + + return await next(context); + }); + } +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/IAuthorizer.cs b/framework/SimpleModule.Core/Authorization/Policies/IAuthorizer.cs new file mode 100644 index 00000000..e0fa2e23 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/IAuthorizer.cs @@ -0,0 +1,35 @@ +using System.Security.Claims; + +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Dispatches instance-level authorization checks to the registered +/// implementations for the resource type. +/// +public interface IAuthorizer +{ + /// + /// Runs the registered policies for in registration + /// order, stopping at the first deny. A single deny wins; allow requires every policy + /// to allow. Throws when no policy is registered. + /// + Task CheckAsync( + ClaimsPrincipal user, + string action, + TResource resource, + CancellationToken cancellationToken = default + ); + + /// + /// Like but throws on deny: + /// for actions listed in + /// (anti-enumeration), + /// otherwise . + /// + Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + TResource resource, + CancellationToken cancellationToken = default + ); +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/IPolicy.cs b/framework/SimpleModule.Core/Authorization/Policies/IPolicy.cs new file mode 100644 index 00000000..abda5c80 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/IPolicy.cs @@ -0,0 +1,20 @@ +using System.Security.Claims; + +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Encapsulates instance-level authorization rules for a resource type — ownership, +/// tenancy, state-machine rules. Layered on top of string permissions: keep +/// .RequirePermission(...) as the coarse capability gate and use a policy for +/// per-resource decisions. Implementations are auto-discovered by the source generator +/// and registered as scoped services; the resource type must be a contracts DTO (SM0058). +/// +public interface IPolicy +{ + Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + TResource resource, + CancellationToken cancellationToken = default + ); +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/IResourceResolver.cs b/framework/SimpleModule.Core/Authorization/Policies/IResourceResolver.cs new file mode 100644 index 00000000..86030b66 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/IResourceResolver.cs @@ -0,0 +1,15 @@ +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Loads a resource from a route value for the declarative +/// AuthorizeResource<TResource> endpoint filter. Modules that opt into the +/// declarative form register one resolver per resource type in +/// ConfigureServices; return null when the resource does not exist (surfaces 404). +/// +public interface IResourceResolver +{ + ValueTask ResolveAsync( + string routeValue, + CancellationToken cancellationToken = default + ); +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/MissingPolicyException.cs b/framework/SimpleModule.Core/Authorization/Policies/MissingPolicyException.cs new file mode 100644 index 00000000..e504fdc5 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/MissingPolicyException.cs @@ -0,0 +1,25 @@ +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Thrown when an authorization check runs for a resource type that has no registered +/// . This is a developer error (fail closed, loudly) — +/// add a policy class for the resource or remove the check. +/// +public sealed class MissingPolicyException : InvalidOperationException +{ + public MissingPolicyException() + : base("No policy is registered for the requested resource type.") { } + + public MissingPolicyException(string message) + : base(message) { } + + public MissingPolicyException(string message, Exception innerException) + : base(message, innerException) { } + + public static MissingPolicyException ForResource(Type resourceType) => + new( + $"No IPolicy<{resourceType?.Name}> is registered. " + + "Add a policy class implementing IPolicy in the module that owns the resource, " + + "or remove the authorization check." + ); +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/PolicyActions.cs b/framework/SimpleModule.Core/Authorization/Policies/PolicyActions.cs new file mode 100644 index 00000000..9a5bab3a --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/PolicyActions.cs @@ -0,0 +1,14 @@ +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Conventional action names for checks. Policies may +/// define additional module-specific actions; these constants just keep the common CRUD +/// verbs consistent across modules. +/// +public static class PolicyActions +{ + public const string View = "view"; + public const string Create = "create"; + public const string Update = "update"; + public const string Delete = "delete"; +} diff --git a/framework/SimpleModule.Core/Authorization/Policies/PolicyAuthorizationOptions.cs b/framework/SimpleModule.Core/Authorization/Policies/PolicyAuthorizationOptions.cs new file mode 100644 index 00000000..ed7d5482 --- /dev/null +++ b/framework/SimpleModule.Core/Authorization/Policies/PolicyAuthorizationOptions.cs @@ -0,0 +1,21 @@ +namespace SimpleModule.Core.Authorization.Policies; + +/// +/// Host-level options for . The preferred way to surface a +/// denial as 404 is — the policy knows +/// whether a resource's existence is secret, and the decision travels with it. This +/// option is a blunt host-wide override on top of that. +/// +public sealed class PolicyAuthorizationOptions +{ + /// + /// Actions whose denial always throws + /// (404) instead of (403), regardless of + /// the policy's decision — including an explicit Deny(reason), whose reason is + /// then swallowed. Empty by default; action names are not namespaced, so an entry + /// applies to every module in the host. Configure only at the host level — modules + /// must not mutate this set. + /// + public ISet NotFoundActions { get; } = + new HashSet(StringComparer.OrdinalIgnoreCase); +} diff --git a/framework/SimpleModule.Core/ManualContractRegistrationAttribute.cs b/framework/SimpleModule.Core/ManualContractRegistrationAttribute.cs index 03b07a17..1815ed32 100644 --- a/framework/SimpleModule.Core/ManualContractRegistrationAttribute.cs +++ b/framework/SimpleModule.Core/ManualContractRegistrationAttribute.cs @@ -18,6 +18,11 @@ namespace SimpleModule.Core; /// The contract is still considered implemented, so SM0025 "no implementation" /// does not fire. Apply this to every implementation of a contract that the module /// registers manually so the generator never tries to auto-wire any of them. +/// The attribute applies the same way to IPolicy<T> implementations: +/// a policy carrying it is not auto-registered, and the auto-registration rules +/// SM0059 "policy must be public" and SM0061 "policy must not be generic" +/// are waived — the module can register an internal or closed-generic policy itself. +/// The resource rules (SM0058, SM0060) still apply. /// /// /// diff --git a/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md b/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md index 33d43bae..2a683957 100644 --- a/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md +++ b/framework/SimpleModule.Generator/AnalyzerReleases.Unshipped.md @@ -42,3 +42,7 @@ SM0054 | SimpleModule.Generator | Info | Endpoint missing Route const field SM0055 | SimpleModule.Generator | Error | Entity class must live in a Contracts assembly SM0056 | SimpleModule.Generator | Error | FormRequest class must be sealed SM0057 | SimpleModule.Generator | Error | FormRequest class must extend FormRequest +SM0058 | SimpleModule.Generator | Error | Policy resource type must be a contracts DTO +SM0059 | SimpleModule.Generator | Error | Policy class must be public +SM0060 | SimpleModule.Generator | Error | Policy must be owned by the resource's module +SM0061 | SimpleModule.Generator | Error | Policy class must not be generic diff --git a/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs b/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs index b3005417..652b5bfc 100644 --- a/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs +++ b/framework/SimpleModule.Generator/Discovery/CoreSymbols.cs @@ -29,6 +29,7 @@ internal readonly record struct CoreSymbols( INamedTypeSymbol? ModuleOptions, INamedTypeSymbol? FormRequestAttribute, INamedTypeSymbol? FormRequestBase, + INamedTypeSymbol? PolicyInterface, bool HasAgentsAssembly, bool HasRagAssembly ) @@ -88,6 +89,9 @@ bool HasRagAssembly FormRequestBase: compilation.GetTypeByMetadataName( "SimpleModule.Core.FormRequests.FormRequest`1" ), + PolicyInterface: compilation.GetTypeByMetadataName( + "SimpleModule.Core.Authorization.Policies.IPolicy`1" + ), HasAgentsAssembly: compilation.GetTypeByMetadataName( "SimpleModule.Agents.SimpleModuleAgentExtensions" ) diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs index cbbc651c..1cd2aed6 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs @@ -48,6 +48,7 @@ internal readonly record struct DiscoveryData( ImmutableArray AgentToolProviders, ImmutableArray KnowledgeSources, ImmutableArray FormRequests, + ImmutableArray Policies, ImmutableArray ContractsAssemblyNames, bool HasAgentsAssembly, bool HasRagAssembly, @@ -81,6 +82,7 @@ string HostAssemblyName ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -106,6 +108,7 @@ public bool Equals(DiscoveryData other) && AgentToolProviders.SequenceEqual(other.AgentToolProviders) && KnowledgeSources.SequenceEqual(other.KnowledgeSources) && FormRequests.SequenceEqual(other.FormRequests) + && Policies.SequenceEqual(other.Policies) && ContractsAssemblyNames.SequenceEqual(other.ContractsAssemblyNames) && HasAgentsAssembly == other.HasAgentsAssembly && HasRagAssembly == other.HasRagAssembly @@ -132,6 +135,7 @@ public override int GetHashCode() hash = HashHelper.HashArray(hash, AgentToolProviders); hash = HashHelper.HashArray(hash, KnowledgeSources); hash = HashHelper.HashArray(hash, FormRequests); + hash = HashHelper.HashArray(hash, Policies); hash = HashHelper.HashArray(hash, ContractsAssemblyNames); hash = HashHelper.Combine(hash, HasAgentsAssembly.GetHashCode()); hash = HashHelper.Combine(hash, HasRagAssembly.GetHashCode()); diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs index 1a577891..ac78c6e3 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryDataBuilder.cs @@ -30,6 +30,7 @@ internal static DiscoveryData Build( List agentToolProviders, List knowledgeSources, List formRequests, + List policies, Dictionary contractsAssemblyMap, bool hasAgentsAssembly, bool hasRagAssembly, @@ -184,6 +185,20 @@ string hostAssemblyName f.Location )) .ToImmutableArray(), + policies + .Select(p => new PolicyInfoRecord( + p.FullyQualifiedName, + p.ResourceTypeFqn, + p.ModuleName, + p.IsPublic, + p.IsGeneric, + p.IsManuallyRegistered, + p.ResourceIsTypeParameter, + p.ResourceIsContractsDto, + p.ResourceModuleName, + p.Location + )) + .ToImmutableArray(), contractsAssemblyMap.Keys.ToImmutableArray(), hasAgentsAssembly, hasRagAssembly, diff --git a/framework/SimpleModule.Generator/Discovery/Finders/PolicyFinder.cs b/framework/SimpleModule.Generator/Discovery/Finders/PolicyFinder.cs new file mode 100644 index 00000000..385d640b --- /dev/null +++ b/framework/SimpleModule.Generator/Discovery/Finders/PolicyFinder.cs @@ -0,0 +1,241 @@ +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static class PolicyFinder +{ + /// + /// Invariant inputs for one policy scan, bundled so the recursive walk doesn't + /// thread two adjacent same-typed dictionaries through every call site. + /// + private readonly struct PolicyScanContext( + INamedTypeSymbol policyInterface, + INamedTypeSymbol? dtoAttribute, + Dictionary contractsAssemblyMap, + Dictionary moduleAssemblyMap, + List results + ) + { + public INamedTypeSymbol PolicyInterface { get; } = policyInterface; + public INamedTypeSymbol? DtoAttribute { get; } = dtoAttribute; + public Dictionary ContractsAssemblyMap { get; } = contractsAssemblyMap; + public Dictionary ModuleAssemblyMap { get; } = moduleAssemblyMap; + public List Results { get; } = results; + } + + private static void FindPolicyTypes( + INamespaceSymbol namespaceSymbol, + in PolicyScanContext context, + string moduleName + ) + { + foreach (var member in namespaceSymbol.GetMembers()) + { + if (member is INamespaceSymbol childNs) + { + FindPolicyTypes(childNs, context, moduleName); + } + else if (member is INamedTypeSymbol typeSymbol) + { + InspectType(typeSymbol, context, moduleName); + } + } + } + + private static void InspectType( + INamedTypeSymbol typeSymbol, + in PolicyScanContext context, + string moduleName + ) + { + // Policies can only be classes; pruning here also keeps the nested-type + // recursion below from visiting struct/enum/delegate members. + if (typeSymbol.TypeKind != TypeKind.Class) + return; + + if (!typeSymbol.IsAbstract && !typeSymbol.IsStatic) + { + // A class may implement IPolicy for more than one resource type; + // each closed interface becomes its own DI registration. + foreach (var iface in typeSymbol.AllInterfaces) + { + if ( + !SymbolEqualityComparer.Default.Equals( + iface.OriginalDefinition, + context.PolicyInterface + ) + ) + continue; + + var resourceType = iface.TypeArguments[0]; + + context.Results.Add( + new PolicyInfo + { + FullyQualifiedName = typeSymbol.ToDisplayString( + SymbolDisplayFormat.FullyQualifiedFormat + ), + ResourceTypeFqn = resourceType.ToDisplayString( + SymbolDisplayFormat.FullyQualifiedFormat + ), + ModuleName = moduleName, + // Effective accessibility: a public class nested inside a + // non-public outer type is unreachable from generated code. + IsPublic = IsEffectivelyPublic(typeSymbol), + IsGeneric = typeSymbol.IsGenericType, + IsManuallyRegistered = ContractFinder.HasManualRegistrationAttribute( + typeSymbol + ), + ResourceIsTypeParameter = resourceType is ITypeParameterSymbol, + ResourceIsContractsDto = IsContractsDto( + resourceType, + context.DtoAttribute + ), + ResourceModuleName = ResolveResourceModule(resourceType, context), + Location = SymbolHelpers.GetSourceLocation(typeSymbol), + } + ); + } + } + + // Policies may be declared as nested classes — recurse into type members. + foreach (var nested in typeSymbol.GetTypeMembers()) + { + InspectType(nested, context, moduleName); + } + } + + private static bool IsEffectivelyPublic(ITypeSymbol type) + { + for (var current = type; current is not null; current = current.ContainingType) + { + if (current.DeclaredAccessibility != Accessibility.Public) + return false; + } + return true; + } + + /// + /// A valid policy resource is an effectively-public contracts DTO: either marked + /// [Dto] or declared in a .Contracts assembly. Checked symbolically (not via the + /// DtoTypes list) so contracts entities excluded from TS/JSON generation + /// ([NoDtoGeneration], IEvent) still qualify. Non-public resources are rejected + /// because the generated registration could not reference them. The suffix match + /// is case-insensitive to agree with SymbolDiscovery's contracts classification. + /// + private static bool IsContractsDto(ITypeSymbol resourceType, INamedTypeSymbol? dtoAttribute) + { + if (resourceType is not INamedTypeSymbol named || !IsEffectivelyPublic(named)) + return false; + + if ( + named.ContainingAssembly?.Name.EndsWith( + AssemblyConventions.ContractsSuffix, + StringComparison.OrdinalIgnoreCase + ) == true + ) + { + return true; + } + + if (dtoAttribute is null) + return false; + + foreach (var attribute in named.GetAttributes()) + { + if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, dtoAttribute)) + return true; + } + + return false; + } + + /// + /// Resolves which module owns the resource type via its containing assembly. + /// Returns "" when the assembly maps to no known module (host types, framework types). + /// + private static string ResolveResourceModule( + ITypeSymbol resourceType, + in PolicyScanContext context + ) + { + var assemblyName = resourceType.ContainingAssembly?.Name; + if (assemblyName is null) + return ""; + + if (context.ContractsAssemblyMap.TryGetValue(assemblyName, out var contractsModule)) + return contractsModule; + + if (context.ModuleAssemblyMap.TryGetValue(assemblyName, out var implModule)) + return implModule; + + return ""; + } + + /// + /// Scans every module implementation assembly, every contracts assembly (mapped to a + /// module or not), and the compiling (host) assembly for IPolicy<T> implementors, + /// visiting each assembly exactly once. No-op when the policy interface isn't + /// resolvable. + /// + internal static void Discover( + List modules, + Dictionary moduleSymbols, + IReadOnlyList contractsAssemblies, + Dictionary contractsAssemblyMap, + Dictionary moduleAssemblyMap, + IAssemblySymbol hostAssembly, + CoreSymbols symbols, + List policies + ) + { + if (symbols.PolicyInterface is null) + return; + + var context = new PolicyScanContext( + symbols.PolicyInterface, + symbols.DtoAttribute, + contractsAssemblyMap, + moduleAssemblyMap, + policies + ); + + var scanned = new HashSet(SymbolEqualityComparer.Default); + + foreach (var module in modules) + { + if ( + moduleSymbols.TryGetValue(module.FullyQualifiedName, out var typeSymbol) + && scanned.Add(typeSymbol.ContainingAssembly) + ) + { + FindPolicyTypes( + typeSymbol.ContainingAssembly.GlobalNamespace, + context, + module.ModuleName + ); + } + } + + // All contracts assemblies — including ones whose name maps to no module, so a + // policy there is still registered (its SM0060 ownership check is skipped). + foreach (var contractsAssembly in contractsAssemblies) + { + if (scanned.Add(contractsAssembly)) + { + contractsAssemblyMap.TryGetValue(contractsAssembly.Name, out var moduleName); + FindPolicyTypes(contractsAssembly.GlobalNamespace, context, moduleName ?? ""); + } + } + + // The compiling assembly: hosts may declare policies too (already covered when + // the host itself contains a [Module] class). + if (scanned.Add(hostAssembly)) + { + moduleAssemblyMap.TryGetValue(hostAssembly.Name, out var hostModule); + FindPolicyTypes(hostAssembly.GlobalNamespace, context, hostModule ?? ""); + } + } +} diff --git a/framework/SimpleModule.Generator/Discovery/Records/DataRecords.cs b/framework/SimpleModule.Generator/Discovery/Records/DataRecords.cs index 7e5bdcc7..08e0848c 100644 --- a/framework/SimpleModule.Generator/Discovery/Records/DataRecords.cs +++ b/framework/SimpleModule.Generator/Discovery/Records/DataRecords.cs @@ -199,6 +199,19 @@ public override int GetHashCode() } } +internal readonly record struct PolicyInfoRecord( + string FullyQualifiedName, + string ResourceTypeFqn, + string ModuleName, + bool IsPublic, + bool IsGeneric, + bool IsManuallyRegistered, + bool ResourceIsTypeParameter, + bool ResourceIsContractsDto, + string ResourceModuleName, + SourceLocationRecord? Location +); + internal readonly record struct ModuleOptionsRecord( string FullyQualifiedName, string ModuleName, diff --git a/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs b/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs index f70ca313..c58705c6 100644 --- a/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs +++ b/framework/SimpleModule.Generator/Discovery/Records/WorkingTypes.cs @@ -164,6 +164,22 @@ internal sealed class DiscoveredTypeInfo public string ModuleName { get; set; } = ""; } +internal sealed class PolicyInfo +{ + public string FullyQualifiedName { get; set; } = ""; + public string ResourceTypeFqn { get; set; } = ""; + public string ModuleName { get; set; } = ""; + public bool IsPublic { get; set; } + public bool IsGeneric { get; set; } + public bool IsManuallyRegistered { get; set; } + public bool ResourceIsTypeParameter { get; set; } + public bool ResourceIsContractsDto { get; set; } + + /// Module owning the resource type's assembly; "" when unattributable. + public string ResourceModuleName { get; set; } = ""; + public SourceLocationRecord? Location { get; set; } +} + internal sealed class FormRequestInfo { public string FullyQualifiedName { get; set; } = ""; diff --git a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs index 45a42e41..529ac889 100644 --- a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs +++ b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs @@ -211,6 +211,19 @@ CancellationToken cancellationToken moduleOptionsList ); + // Step 3f': IPolicy implementors (instance-level authorization) + var policies = new List(); + PolicyFinder.Discover( + modules, + moduleSymbols, + contractsAssemblies, + contractsAssemblyMap, + moduleAssemblyMap, + compilation.Assembly, + s, + policies + ); + // Step 3g: FormRequest types var formRequests = new List(); FormRequestFinder.Discover( @@ -264,6 +277,7 @@ CancellationToken cancellationToken agentToolProviders, knowledgeSources, formRequests, + policies, contractsAssemblyMap, s.HasAgentsAssembly, s.HasRagAssembly, diff --git a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs index 8260e8c8..db35dc62 100644 --- a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs @@ -13,5 +13,6 @@ public void Emit(SourceProductionContext context, DiscoveryData data) PermissionFeatureChecks.Run(context, data); EndpointChecks.Run(context, data); FormRequestChecks.Run(context, data); + PolicyChecks.Run(context, data); } } diff --git a/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.Policy.cs b/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.Policy.cs new file mode 100644 index 00000000..1da7902e --- /dev/null +++ b/framework/SimpleModule.Generator/Emitters/Diagnostics/DiagnosticDescriptors.Policy.cs @@ -0,0 +1,42 @@ +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static partial class DiagnosticDescriptors +{ + internal static readonly DiagnosticDescriptor PolicyResourceMustBeDto = new( + id: "SM0058", + title: "Policy resource type must be a contracts DTO", + messageFormat: "Policy '{0}' targets resource type '{1}' which is neither marked [Dto] nor declared in a .Contracts assembly. Policies guard resources that cross module boundaries — move the resource type to the module's .Contracts assembly or mark it with [Dto].", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + internal static readonly DiagnosticDescriptor PolicyNotPublic = new( + id: "SM0059", + title: "Policy class must be public", + messageFormat: "Policy '{0}' implements IPolicy<{1}> but is not public, so it cannot be auto-registered. Make the class public — otherwise authorization checks for '{1}' fail at runtime with MissingPolicyException.", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + internal static readonly DiagnosticDescriptor PolicyForForeignResource = new( + id: "SM0060", + title: "Policy must be owned by the resource's module", + messageFormat: "Policy '{0}' in module '{1}' targets resource type '{2}' owned by module '{3}'. Policies run with deny-wins semantics, so a foreign policy would silently veto another module's authorization decisions — move the policy to module '{3}'.", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + internal static readonly DiagnosticDescriptor PolicyMustNotBeGeneric = new( + id: "SM0061", + title: "Policy class must not be generic", + messageFormat: "Policy '{0}' is a generic class. Open generic policies cannot be auto-registered — declare one closed policy class per resource type instead.", + category: "SimpleModule.Generator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); +} diff --git a/framework/SimpleModule.Generator/Emitters/Diagnostics/PolicyChecks.cs b/framework/SimpleModule.Generator/Emitters/Diagnostics/PolicyChecks.cs new file mode 100644 index 00000000..f32d7b9f --- /dev/null +++ b/framework/SimpleModule.Generator/Emitters/Diagnostics/PolicyChecks.cs @@ -0,0 +1,100 @@ +using System.Collections.Generic; +using Microsoft.CodeAnalysis; + +namespace SimpleModule.Generator; + +internal static class PolicyChecks +{ + internal static void Run(SourceProductionContext context, DiscoveryData data) + { + // SM0059/SM0061 are class-level rules but discovery yields one record per + // implemented IPolicy interface — dedup so a multi-resource policy class + // reports each class-level diagnostic once. + var reportedNonPublic = new HashSet(); + var reportedGeneric = new HashSet(); + + foreach (var policy in data.Policies) + { + var policyName = TypeMappingHelpers.StripGlobalPrefix(policy.FullyQualifiedName); + var resourceName = TypeMappingHelpers.StripGlobalPrefix(policy.ResourceTypeFqn); + + // SM0059/SM0061 exist to catch policies the generator cannot register; a + // [ManualContractRegistration] policy is wired by its own module (which can + // reference internal types and close generics), so both rules are waived. + // SM0058/SM0060 are resource rules and still apply below. + var autoRegistered = !policy.IsManuallyRegistered; + + // SM0061: open generic policies cannot be auto-registered. + if (policy.IsGeneric && autoRegistered && reportedGeneric.Add(policy.FullyQualifiedName)) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.PolicyMustNotBeGeneric, + LocationHelper.ToLocation(policy.Location), + policyName + ) + ); + } + + // When the resource is the policy's own type parameter there is no concrete + // type to validate — the resource rules below would only add noise on top of + // SM0061. A generic policy with a CONCRETE resource still gets them. + if (policy.ResourceIsTypeParameter) + continue; + + // SM0058: resource must be an effectively-public contracts DTO ([Dto] or + // in a .Contracts assembly). Determined symbolically at discovery so + // contracts entities excluded from DtoTypes ([NoDtoGeneration], IEvent) + // still qualify. + if (!policy.ResourceIsContractsDto) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.PolicyResourceMustBeDto, + LocationHelper.ToLocation(policy.Location), + policyName, + resourceName + ) + ); + } + + // SM0059: non-public policies are skipped by the registration emitter, + // which would otherwise surface only as a runtime MissingPolicyException. + if ( + autoRegistered + && !policy.IsPublic + && reportedNonPublic.Add(policy.FullyQualifiedName) + ) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.PolicyNotPublic, + LocationHelper.ToLocation(policy.Location), + policyName, + resourceName + ) + ); + } + + // SM0060: deny-wins lets any registered policy veto a decision, so a + // policy for another module's resource is a cross-module backdoor. + if ( + policy.ResourceModuleName.Length > 0 + && policy.ModuleName.Length > 0 + && policy.ResourceModuleName != policy.ModuleName + ) + { + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticDescriptors.PolicyForForeignResource, + LocationHelper.ToLocation(policy.Location), + policyName, + policy.ModuleName, + resourceName, + policy.ResourceModuleName + ) + ); + } + } + } +} diff --git a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs index c6b98d66..94cc7e12 100644 --- a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs @@ -26,6 +26,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) sb.AppendLine("using Microsoft.AspNetCore.Http.Json;"); sb.AppendLine("using Microsoft.Extensions.Configuration;"); sb.AppendLine("using Microsoft.Extensions.DependencyInjection;"); + sb.AppendLine("using Microsoft.Extensions.DependencyInjection.Extensions;"); sb.AppendLine("using Microsoft.Extensions.Hosting;"); sb.AppendLine("using Microsoft.AspNetCore.Authorization;"); sb.AppendLine("using SimpleModule.Core.Authorization;"); @@ -137,6 +138,32 @@ public void Emit(SourceProductionContext context, DiscoveryData data) } } + if (data.Policies.Length > 0) + { + sb.AppendLine(); + sb.AppendLine(" // Auto-discovered resource policies (IPolicy)."); + sb.AppendLine( + " // TryAddEnumerable dedups type-based manual registrations; factory" + ); + sb.AppendLine( + " // registrations must use the two-generic AddScoped(factory)" + ); + sb.AppendLine( + " // overload or opt out entirely with [ManualContractRegistration]." + ); + foreach (var policy in data.Policies) + { + // Non-public (SM0059) and generic (SM0061) policies can't be referenced + // here; manually-registered ones are wired by their module. + if (!policy.IsPublic || policy.IsGeneric || policy.IsManuallyRegistered) + continue; + + sb.AppendLine( + $" services.TryAddEnumerable(global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor.Scoped, {policy.FullyQualifiedName}>());" + ); + } + } + sb.AppendLine(); sb.AppendLine(" var permissionBuilder = new PermissionRegistryBuilder();"); diff --git a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs index c4c36e65..40208852 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs @@ -11,6 +11,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Core.Constants; using SimpleModule.Core.Exceptions; using SimpleModule.Core.Health; @@ -148,6 +149,11 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( builder.Services.AddHttpContextAccessor(); builder.Services.AddScoped(); + // Instance-level authorization (IPolicy dispatch). Policies themselves are + // auto-discovered by the source generator and registered in AddModules(). + builder.Services.AddOptions(); + builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.Services.AddScoped(); diff --git a/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs index 88563087..5531135f 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs @@ -3,6 +3,7 @@ using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Database.Interceptors; using Wolverine; using ZiggyCreatures.Caching.Fusion; @@ -62,6 +63,11 @@ params Assembly[] moduleAssemblies // HttpContextAccessor: EntityInterceptor returns null in non-HTTP contexts. builder.Services.AddHttpContextAccessor(); + // AddModules() registers IPolicy implementations, so the dispatcher must + // resolve here too — background handlers may authorize resources as well. + builder.Services.AddOptions(); + builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.Services.AddScoped(); diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DeleteEndpoint.cs b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DeleteEndpoint.cs index 71b2f7ff..6c3ea2ff 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DeleteEndpoint.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DeleteEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.FileStorage.Contracts; namespace SimpleModule.FileStorage.Endpoints.Files; @@ -15,7 +16,12 @@ public class DeleteEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapDelete( Route, - async (FileStorageId id, HttpContext context, IFileStorageContracts files) => + async ( + FileStorageId id, + HttpContext context, + IFileStorageContracts files, + IAuthorizer authorizer + ) => { var file = await files.GetFileByIdAsync(id); if (file is null) @@ -23,10 +29,13 @@ public void Map(IEndpointRouteBuilder app) => return Results.NotFound(); } - if (!FileOwnershipCheck.CanAccess(file, context.User)) - { - return Results.Forbid(); - } + // Owner-or-admin; FileStoragePolicy denies non-owners as 404. + await authorizer.AuthorizeAsync( + context.User, + PolicyActions.Delete, + file, + context.RequestAborted + ); await files.DeleteFileAsync(file); return Results.NoContent(); diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DownloadEndpoint.cs b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DownloadEndpoint.cs index e3faf1eb..88f36056 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DownloadEndpoint.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/DownloadEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.FileStorage.Contracts; namespace SimpleModule.FileStorage.Endpoints.Files; @@ -15,7 +16,12 @@ public class DownloadEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapGet( Route, - async (FileStorageId id, HttpContext context, IFileStorageContracts files) => + async ( + FileStorageId id, + HttpContext context, + IFileStorageContracts files, + IAuthorizer authorizer + ) => { var file = await files.GetFileByIdAsync(id); if (file is null) @@ -23,10 +29,13 @@ public void Map(IEndpointRouteBuilder app) => return Results.NotFound(); } - if (!FileOwnershipCheck.CanAccess(file, context.User)) - { - return Results.Forbid(); - } + // Owner-or-admin; FileStoragePolicy denies non-owners as 404. + await authorizer.AuthorizeAsync( + context.User, + PolicyActions.View, + file, + context.RequestAborted + ); var stream = await files.DownloadFileAsync(file); return stream is null diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/FileOwnershipCheck.cs b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/FileOwnershipCheck.cs deleted file mode 100644 index 2948a67e..00000000 --- a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/FileOwnershipCheck.cs +++ /dev/null @@ -1,12 +0,0 @@ -using System.Security.Claims; -using SimpleModule.Core.Authorization; -using SimpleModule.Core.Extensions; -using SimpleModule.FileStorage.Contracts; - -namespace SimpleModule.FileStorage.Endpoints.Files; - -internal static class FileOwnershipCheck -{ - internal static bool CanAccess(StoredFile file, ClaimsPrincipal user) => - user.IsInRole(WellKnownRoles.Admin) || file.CreatedByUserId == user.GetUserId(); -} diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/GetByIdEndpoint.cs b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/GetByIdEndpoint.cs index b3e3266c..e8f04e8b 100644 --- a/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/GetByIdEndpoint.cs +++ b/modules/FileStorage/src/SimpleModule.FileStorage/Endpoints/Files/GetByIdEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.FileStorage.Contracts; namespace SimpleModule.FileStorage.Endpoints.Files; @@ -15,7 +16,12 @@ public class GetByIdEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapGet( Route, - async (FileStorageId id, HttpContext context, IFileStorageContracts files) => + async ( + FileStorageId id, + HttpContext context, + IFileStorageContracts files, + IAuthorizer authorizer + ) => { var file = await files.GetFileByIdAsync(id); if (file is null) @@ -23,10 +29,13 @@ public void Map(IEndpointRouteBuilder app) => return Results.NotFound(); } - if (!FileOwnershipCheck.CanAccess(file, context.User)) - { - return Results.Forbid(); - } + // Owner-or-admin; FileStoragePolicy denies non-owners as 404. + await authorizer.AuthorizeAsync( + context.User, + PolicyActions.View, + file, + context.RequestAborted + ); return Results.Ok(file); } diff --git a/modules/FileStorage/src/SimpleModule.FileStorage/FileStoragePolicy.cs b/modules/FileStorage/src/SimpleModule.FileStorage/FileStoragePolicy.cs new file mode 100644 index 00000000..12781328 --- /dev/null +++ b/modules/FileStorage/src/SimpleModule.FileStorage/FileStoragePolicy.cs @@ -0,0 +1,44 @@ +using System.Security.Claims; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Extensions; +using SimpleModule.FileStorage.Contracts; + +namespace SimpleModule.FileStorage; + +/// +/// Instance-level rules for stored files: the uploader (or an admin) may view, download, +/// or delete a file. Ownership denials use DenyAsNotFound so a caller holding the +/// coarse FileStorage.View/Delete permission cannot probe which file IDs +/// belong to other users. Auto-registered by the source generator. +/// +public sealed class FileStoragePolicy : IPolicy +{ + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + StoredFile resource, + CancellationToken cancellationToken = default + ) + { + var result = action switch + { + PolicyActions.View or PolicyActions.Delete => AllowOwnerOrAdmin(user, resource), + _ => AuthorizationResult.Deny($"Unknown file action '{action}'."), + }; + return Task.FromResult(result); + } + + private static AuthorizationResult AllowOwnerOrAdmin(ClaimsPrincipal user, StoredFile file) + { + if (user.IsInRole(WellKnownRoles.Admin)) + { + return AuthorizationResult.Allow(); + } + + var userId = user.GetUserId(); + return userId is not null && file.CreatedByUserId == userId + ? AuthorizationResult.Allow() + : AuthorizationResult.DenyAsNotFound("You can only access your own files."); + } +} diff --git a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/Integration/FileAccessPolicyTests.cs b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/Integration/FileAccessPolicyTests.cs new file mode 100644 index 00000000..b1d3b6d4 --- /dev/null +++ b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/Integration/FileAccessPolicyTests.cs @@ -0,0 +1,90 @@ +using System.Net; +using System.Security.Claims; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using SimpleModule.Core.Authorization; +using SimpleModule.FileStorage.Contracts; +using SimpleModule.Tests.Shared.Fixtures; +using Xunit; + +namespace SimpleModule.FileStorage.Tests.Integration; + +/// +/// Exercises through the real GetById endpoint: +/// owner/admin succeed, a non-owner holding the FileStorage.View permission gets 404 +/// (DenyAsNotFound — no existence leak). +/// +[Collection(TestCollections.Integration)] +public sealed class FileAccessPolicyTests(SimpleModuleWebApplicationFactory factory) +{ + private async Task SeedFileAsync(string ownerId) + { + using var scope = factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var file = new StoredFile + { + FileName = "secret.txt", + StoragePath = $"{ownerId}/secret.txt", + ContentType = "text/plain", + Size = 10, + CreatedByUserId = ownerId, + }; + db.StoredFiles.Add(file); + await db.SaveChangesAsync(); // Id is assigned by the DB (int identity) + return file.Id; + } + + private HttpClient ClientFor(string userId, params string[] permissions) => + factory.CreateAuthenticatedClient( + permissions, + new Claim(ClaimTypes.NameIdentifier, userId) + ); + + [Fact] + public async Task GetById_Owner_ReturnsOk() + { + var id = await SeedFileAsync("file-owner-1"); + var client = ClientFor("file-owner-1", FileStoragePermissions.View); + + var response = await client.GetAsync($"/api/files/{id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task GetById_NonOwnerWithPermission_Returns404() + { + var id = await SeedFileAsync("file-owner-2"); + var client = ClientFor("an-intruder", FileStoragePermissions.View); + + var response = await client.GetAsync($"/api/files/{id.Value}"); + + // DenyAsNotFound: a permitted non-owner cannot tell the file exists. + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task GetById_Admin_CanViewAnyFile() + { + var id = await SeedFileAsync("file-owner-3"); + var admin = factory.CreateAuthenticatedClient( + [FileStoragePermissions.View], + new Claim(ClaimTypes.Role, WellKnownRoles.Admin), + new Claim(ClaimTypes.NameIdentifier, "file-admin") + ); + + var response = await admin.GetAsync($"/api/files/{id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task GetById_NonExistent_Returns404() + { + var client = ClientFor("file-owner-4", FileStoragePermissions.View); + + var response = await client.GetAsync("/api/files/2147483600"); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } +} diff --git a/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/IntegrationTestCollection.cs b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/IntegrationTestCollection.cs new file mode 100644 index 00000000..8c4a61db --- /dev/null +++ b/modules/FileStorage/tests/SimpleModule.FileStorage.Tests/IntegrationTestCollection.cs @@ -0,0 +1,6 @@ +using SimpleModule.Tests.Shared.Fixtures; +using Xunit; + +[CollectionDefinition(TestCollections.Integration)] +public sealed class IntegrationTestCollection + : ICollectionFixture; diff --git a/modules/Notifications/src/SimpleModule.Notifications.Contracts/INotificationsContracts.cs b/modules/Notifications/src/SimpleModule.Notifications.Contracts/INotificationsContracts.cs index c732bd03..e643098d 100644 --- a/modules/Notifications/src/SimpleModule.Notifications.Contracts/INotificationsContracts.cs +++ b/modules/Notifications/src/SimpleModule.Notifications.Contracts/INotificationsContracts.cs @@ -8,6 +8,13 @@ public interface INotificationsContracts Task> ListAsync(UserId userId, QueryNotificationsRequest request); Task GetUnreadCountAsync(UserId userId, CancellationToken cancellationToken = default); Task GetByIdAsync(NotificationId id, UserId userId); + + /// + /// Marks a notification read. Owner-scoped: returns false when the notification + /// does not exist or belongs to another user — cross-module callers cannot mutate + /// other users' notifications. Instance-level authorization with richer semantics + /// (reasons, 404 mapping) lives in NotificationPolicy at the endpoint. + /// Task MarkReadAsync(NotificationId id, UserId userId); Task MarkAllReadAsync(UserId userId); } diff --git a/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/MarkReadEndpoint.cs b/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/MarkReadEndpoint.cs index 185dd49e..003fe6ab 100644 --- a/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/MarkReadEndpoint.cs +++ b/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/MarkReadEndpoint.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Routing; using SimpleModule.Core; using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Core.Extensions; using SimpleModule.Notifications.Contracts; using SimpleModule.Users.Contracts; @@ -23,6 +24,9 @@ async Task ( INotificationsContracts notifications ) => { + // AuthorizeResource already loaded the notification and ran + // NotificationPolicy; the contract call stays owner-scoped + // (defense in depth), so false means it vanished meanwhile. var ok = await notifications.MarkReadAsync( NotificationId.From(id), UserId.From(context.User.GetUserId()!) @@ -30,5 +34,6 @@ INotificationsContracts notifications return ok ? TypedResults.NoContent() : TypedResults.NotFound(); } ) - .RequirePermission(NotificationsPermissions.ViewOwn); + .RequirePermission(NotificationsPermissions.ViewOwn) + .AuthorizeResource(NotificationPolicy.MarkRead); } diff --git a/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/NotificationResolver.cs b/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/NotificationResolver.cs new file mode 100644 index 00000000..bc595047 --- /dev/null +++ b/modules/Notifications/src/SimpleModule.Notifications/Endpoints/Notifications/NotificationResolver.cs @@ -0,0 +1,21 @@ +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Notifications.Contracts; +using SimpleModule.Notifications.Services; + +namespace SimpleModule.Notifications.Endpoints.Notifications; + +/// +/// Loads notifications for the declarative AuthorizeResource<Notification> +/// endpoint filter (see ). +/// +internal sealed class NotificationResolver(INotificationStore store) + : IResourceResolver +{ + public async ValueTask ResolveAsync( + string routeValue, + CancellationToken cancellationToken = default + ) => + Guid.TryParse(routeValue, out var id) + ? await store.FindAsync(NotificationId.From(id), cancellationToken) + : null; +} diff --git a/modules/Notifications/src/SimpleModule.Notifications/NotificationPolicy.cs b/modules/Notifications/src/SimpleModule.Notifications/NotificationPolicy.cs new file mode 100644 index 00000000..4c12a91d --- /dev/null +++ b/modules/Notifications/src/SimpleModule.Notifications/NotificationPolicy.cs @@ -0,0 +1,44 @@ +using System.Security.Claims; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Extensions; +using SimpleModule.Notifications.Contracts; +using SimpleModule.Users.Contracts; + +namespace SimpleModule.Notifications; + +/// +/// Instance-level rules for notifications: only the recipient may view or mark a +/// notification read — admins are deliberately not exempt, since marking read mutates +/// the recipient's inbox state. Ownership denials use DenyAsNotFound so callers +/// cannot probe which notification IDs exist. The permission gate +/// (Notifications.ViewOwn) stays on the endpoint; this policy adds the +/// per-resource check. Auto-registered by the source generator. +/// +public sealed class NotificationPolicy : IPolicy +{ + /// Module-specific action beyond the conventional CRUD verbs. + public const string MarkRead = "markRead"; + + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Notification resource, + CancellationToken cancellationToken = default + ) + { + var result = action switch + { + PolicyActions.View or MarkRead => AllowOwner(user, resource), + _ => AuthorizationResult.Deny($"Unknown notification action '{action}'."), + }; + return Task.FromResult(result); + } + + private static AuthorizationResult AllowOwner(ClaimsPrincipal user, Notification notification) + { + var userId = user.GetUserId(); + return userId is not null && notification.UserId == UserId.From(userId) + ? AuthorizationResult.Allow() + : AuthorizationResult.DenyAsNotFound("You can only access your own notifications."); + } +} diff --git a/modules/Notifications/src/SimpleModule.Notifications/NotificationsModule.cs b/modules/Notifications/src/SimpleModule.Notifications/NotificationsModule.cs index 1c0ea43f..6c897f74 100644 --- a/modules/Notifications/src/SimpleModule.Notifications/NotificationsModule.cs +++ b/modules/Notifications/src/SimpleModule.Notifications/NotificationsModule.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.DependencyInjection; using SimpleModule.BackgroundJobs.Contracts; using SimpleModule.Core; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Core.Settings; using SimpleModule.Database; using SimpleModule.Notifications.Channels; @@ -43,6 +44,12 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.Configure(configuration.GetSection("Notifications")); services.AddScoped(); + services.AddScoped(); + // Loader for the declarative AuthorizeResource endpoint filter. + services.AddScoped< + IResourceResolver, + Endpoints.Notifications.NotificationResolver + >(); services.AddScoped(); // Channels — registered as INotificationChannel so the registry can enumerate them. diff --git a/modules/Notifications/src/SimpleModule.Notifications/Pages/Inbox.tsx b/modules/Notifications/src/SimpleModule.Notifications/Pages/Inbox.tsx index 48e38e9a..f5e08dff 100644 --- a/modules/Notifications/src/SimpleModule.Notifications/Pages/Inbox.tsx +++ b/modules/Notifications/src/SimpleModule.Notifications/Pages/Inbox.tsx @@ -17,21 +17,21 @@ function formatDate(value: string): string { } export default function Inbox({ items, totalCount, unreadCount }: Props) { - const markRead = (id: string) => { - router.post( - `/api/notifications/${id}/read`, - {}, - { preserveScroll: true, onSuccess: () => router.reload({ only: ['items', 'unreadCount'] }) }, - ); + // These are plain JSON API endpoints that return 204 No Content, not Inertia + // responses — call them with fetch and refresh the props on success. (router.post + // expects an Inertia-protocol response and treats a 204 as a server error.) + const postAndReload = async (url: string) => { + const res = await fetch(url, { method: 'POST', credentials: 'same-origin' }); + if (!res.ok) { + console.error(`Notification action failed: ${res.status} ${url}`); + return; + } + router.reload({ only: ['items', 'unreadCount'] }); }; - const markAllRead = () => { - router.post( - '/api/notifications/read-all', - {}, - { preserveScroll: true, onSuccess: () => router.reload({ only: ['items', 'unreadCount'] }) }, - ); - }; + const markRead = (id: string) => postAndReload(`/api/notifications/${id}/read`); + + const markAllRead = () => postAndReload('/api/notifications/read-all'); return ( +/// Module-internal loader for the load → authorize → act flow. Deliberately NOT on +/// : the unscoped read exists only so the policy +/// check can see the resource before the handler acts — cross-module callers keep the +/// owner-scoped contract surface. +/// +internal interface INotificationStore +{ + Task FindAsync( + NotificationId id, + CancellationToken cancellationToken = default + ); +} diff --git a/modules/Notifications/src/SimpleModule.Notifications/Services/NotificationService.cs b/modules/Notifications/src/SimpleModule.Notifications/Services/NotificationService.cs index a035d2e4..01e411f9 100644 --- a/modules/Notifications/src/SimpleModule.Notifications/Services/NotificationService.cs +++ b/modules/Notifications/src/SimpleModule.Notifications/Services/NotificationService.cs @@ -5,7 +5,9 @@ namespace SimpleModule.Notifications.Services; -public class NotificationService(NotificationsDbContext db) : INotificationsContracts +public class NotificationService(NotificationsDbContext db) + : INotificationsContracts, + INotificationStore { public async Task> ListAsync( UserId userId, @@ -61,8 +63,18 @@ await db.Notifications.AsNoTracking().FirstOrDefaultAsync(n => n.Id == id && n.UserId == userId ); + public async Task FindAsync( + NotificationId id, + CancellationToken cancellationToken = default + ) => + await db.Notifications.AsNoTracking() + .FirstOrDefaultAsync(n => n.Id == id, cancellationToken); + public async Task MarkReadAsync(NotificationId id, UserId userId) { + // Deliberately load-modify-save rather than ExecuteUpdateAsync: the SaveChanges + // pipeline runs the framework interceptors (audit log entry, UpdatedAt, + // concurrency stamp rotation) that a set-based update would silently skip. var notification = await db.Notifications.FirstOrDefaultAsync(n => n.Id == id && n.UserId == userId ); @@ -77,7 +89,16 @@ public async Task MarkReadAsync(NotificationId id, UserId userId) } notification.ReadAt = DateTimeOffset.UtcNow; - await db.SaveChangesAsync(); + try + { + await db.SaveChangesAsync(); + } + catch (DbUpdateConcurrencyException) + { + // A concurrent request rotated the concurrency stamp (e.g. a double-click + // marking the same notification read) — the outcome the caller wanted + // already holds, so this is an idempotent success, not an error. + } return true; } diff --git a/modules/Notifications/tests/SimpleModule.Notifications.Tests/Integration/MarkReadEndpointTests.cs b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Integration/MarkReadEndpointTests.cs new file mode 100644 index 00000000..54e7def0 --- /dev/null +++ b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Integration/MarkReadEndpointTests.cs @@ -0,0 +1,133 @@ +using System.Net; +using System.Security.Claims; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using SimpleModule.Core.Authorization; +using SimpleModule.Notifications.Contracts; +using SimpleModule.Tests.Shared.Fixtures; +using SimpleModule.Users.Contracts; + +namespace SimpleModule.Notifications.Tests.Integration; + +/// +/// End-to-end coverage of the policy-based authorization flow: the MarkRead endpoint +/// loads the notification, dispatches to via +/// IAuthorizer, and ownership denials surface as 404 (DenyAsNotFound) so callers +/// cannot probe other users' notification IDs. +/// +[Collection(TestCollections.Integration)] +public sealed class MarkReadEndpointTests(SimpleModuleWebApplicationFactory factory) +{ + private async Task SeedAsync(string ownerId) + { + using var scope = factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var notification = new Notification + { + Id = NotificationId.From(Guid.CreateVersion7()), + UserId = UserId.From(ownerId), + Type = "test.event", + Channel = NotificationsConstants.Channels.Database, + Title = "Title", + DataJson = "{}", + }; + db.Notifications.Add(notification); + await db.SaveChangesAsync(); + return notification; + } + + private async Task GetReadAtAsync(NotificationId id) + { + using var scope = factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var notification = await db.Notifications.AsNoTracking().FirstAsync(n => n.Id == id); + return notification.ReadAt; + } + + private HttpClient CreateClientFor(string userId, params string[] roles) + { + var claims = new List { new("sub", userId) }; + claims.AddRange(roles.Select(r => new Claim(ClaimTypes.Role, r))); + return factory.CreateAuthenticatedClient( + [NotificationsPermissions.ViewOwn], + [.. claims] + ); + } + + [Fact] + public async Task MarkRead_AsOwner_Returns204AndMarksRead() + { + var notification = await SeedAsync("owner-1"); + var client = CreateClientFor("owner-1"); + + var response = await client.PostAsync( + $"/api/notifications/{notification.Id.Value}/read", + null + ); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + (await GetReadAtAsync(notification.Id)).Should().NotBeNull(); + } + + [Fact] + public async Task MarkRead_AsNonOwner_Returns404NotForbidden() + { + var notification = await SeedAsync("owner-1"); + var client = CreateClientFor("intruder-2"); + + var response = await client.PostAsync( + $"/api/notifications/{notification.Id.Value}/read", + null + ); + + // NotificationPolicy denies ownership violations with DenyAsNotFound so + // callers cannot probe other users' notification IDs. + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + (await GetReadAtAsync(notification.Id)).Should().BeNull(); + } + + [Fact] + public async Task MarkRead_AsAdminForOthersNotification_Returns404AndDoesNotMutate() + { + // Admins are not exempt from the ownership rule — marking read mutates the + // recipient's inbox state. + var notification = await SeedAsync("owner-1"); + var client = CreateClientFor("admin-1", WellKnownRoles.Admin); + + var response = await client.PostAsync( + $"/api/notifications/{notification.Id.Value}/read", + null + ); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + (await GetReadAtAsync(notification.Id)).Should().BeNull(); + } + + [Fact] + public async Task MarkRead_MissingNotification_Returns404() + { + var client = CreateClientFor("owner-1"); + + var response = await client.PostAsync( + $"/api/notifications/{Guid.CreateVersion7()}/read", + null + ); + + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task MarkRead_Unauthenticated_Returns401() + { + var notification = await SeedAsync("owner-1"); + var client = factory.CreateClient(); + + var response = await client.PostAsync( + $"/api/notifications/{notification.Id.Value}/read", + null + ); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + } +} diff --git a/modules/Notifications/tests/SimpleModule.Notifications.Tests/IntegrationTestCollection.cs b/modules/Notifications/tests/SimpleModule.Notifications.Tests/IntegrationTestCollection.cs new file mode 100644 index 00000000..8c4a61db --- /dev/null +++ b/modules/Notifications/tests/SimpleModule.Notifications.Tests/IntegrationTestCollection.cs @@ -0,0 +1,6 @@ +using SimpleModule.Tests.Shared.Fixtures; +using Xunit; + +[CollectionDefinition(TestCollections.Integration)] +public sealed class IntegrationTestCollection + : ICollectionFixture; diff --git a/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationPolicyTests.cs b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationPolicyTests.cs new file mode 100644 index 00000000..76a8958b --- /dev/null +++ b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationPolicyTests.cs @@ -0,0 +1,100 @@ +using System.Security.Claims; +using FluentAssertions; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Notifications.Contracts; +using SimpleModule.Users.Contracts; + +namespace SimpleModule.Notifications.Tests.Unit; + +public sealed class NotificationPolicyTests +{ + private readonly NotificationPolicy _sut = new(); + + private static ClaimsPrincipal CreateUser(string userId, params string[] roles) + { + var claims = new List { new("sub", userId) }; + claims.AddRange(roles.Select(r => new Claim(ClaimTypes.Role, r))); + return new ClaimsPrincipal(new ClaimsIdentity(claims, "test", "name", ClaimTypes.Role)); + } + + private static Notification CreateNotification(string ownerId) => + new() + { + Id = NotificationId.From(Guid.CreateVersion7()), + UserId = UserId.From(ownerId), + Type = "test.event", + Channel = NotificationsConstants.Channels.Database, + }; + + [Theory] + [InlineData(PolicyActions.View)] + [InlineData(NotificationPolicy.MarkRead)] + public async Task Owner_IsAllowed(string action) + { + var result = await _sut.AuthorizeAsync( + CreateUser("user-1"), + action, + CreateNotification("user-1") + ); + + result.IsAllowed.Should().BeTrue(); + } + + [Theory] + [InlineData(PolicyActions.View)] + [InlineData(NotificationPolicy.MarkRead)] + public async Task NonOwner_IsDeniedAsNotFound(string action) + { + var result = await _sut.AuthorizeAsync( + CreateUser("user-2"), + action, + CreateNotification("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.TreatAsNotFound.Should().BeTrue(); + result.Reason.Should().NotBeNullOrEmpty(); + } + + [Fact] + public async Task Admin_IsNotExemptFromOwnership() + { + // Marking read mutates the recipient's inbox state — admins get no carve-out. + var result = await _sut.AuthorizeAsync( + CreateUser("admin-1", WellKnownRoles.Admin), + NotificationPolicy.MarkRead, + CreateNotification("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.TreatAsNotFound.Should().BeTrue(); + } + + [Fact] + public async Task UnknownAction_IsDenied() + { + var result = await _sut.AuthorizeAsync( + CreateUser("user-1"), + "transmogrify", + CreateNotification("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.TreatAsNotFound.Should().BeFalse(); + } + + [Fact] + public async Task UserWithoutIdClaim_IsDenied() + { + var anonymous = new ClaimsPrincipal(new ClaimsIdentity()); + + var result = await _sut.AuthorizeAsync( + anonymous, + PolicyActions.View, + CreateNotification("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + } +} diff --git a/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationServiceTests.cs b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationServiceTests.cs index bc744c8d..d37a9c76 100644 --- a/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationServiceTests.cs +++ b/modules/Notifications/tests/SimpleModule.Notifications.Tests/Unit/NotificationServiceTests.cs @@ -113,6 +113,40 @@ public async Task MarkReadAsync_WithDifferentUser_ReturnsFalse() var result = await _sut.MarkReadAsync(n.Id, UserId.From("not-the-owner")); result.Should().BeFalse(); + var refreshed = await _db.Notifications.AsNoTracking().FirstAsync(x => x.Id == n.Id); + refreshed.ReadAt.Should().BeNull(); + } + + [Fact] + public async Task MarkReadAsync_AlreadyRead_ReturnsTrueAndKeepsOriginalReadAt() + { + var n = await SeedAsync(readAt: DateTimeOffset.UtcNow.AddDays(-1)); + var stored = await _db.Notifications.AsNoTracking().FirstAsync(x => x.Id == n.Id); + + var result = await _sut.MarkReadAsync(n.Id, _userId); + + result.Should().BeTrue(); + var refreshed = await _db.Notifications.AsNoTracking().FirstAsync(x => x.Id == n.Id); + refreshed.ReadAt.Should().Be(stored.ReadAt); + } + + [Fact] + public async Task FindAsync_ReturnsNotificationRegardlessOfOwner() + { + var n = await SeedAsync(userId: UserId.From("other-user")); + + var found = await _sut.FindAsync(n.Id); + + found.Should().NotBeNull(); + found!.UserId.Should().Be(UserId.From("other-user")); + } + + [Fact] + public async Task FindAsync_MissingNotification_ReturnsNull() + { + var found = await _sut.FindAsync(NotificationId.From(Guid.CreateVersion7())); + + found.Should().BeNull(); } [Fact] diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ClearHomePageEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ClearHomePageEndpoint.cs index 9be3b3ee..6d7b5ff1 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ClearHomePageEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ClearHomePageEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Services; @@ -21,5 +22,5 @@ public void Map(IEndpointRouteBuilder app) => return TypedResults.NoContent(); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs index 453f8dbb..1d5712ce 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/CreateMenuItemEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.FormRequests; using SimpleModule.Settings.Services; @@ -48,5 +49,5 @@ public void Map(IEndpointRouteBuilder app) => return TypedResults.Created($"/api/settings/menus/{entity.Id}", result); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/DeleteMenuItemEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/DeleteMenuItemEndpoint.cs index e0e144a3..1b229d2c 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/DeleteMenuItemEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/DeleteMenuItemEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Services; @@ -21,5 +22,5 @@ async Task (int id, PublicMenuService service) => return deleted ? TypedResults.NoContent() : TypedResults.NotFound(); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/GetAvailablePagesEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/GetAvailablePagesEndpoint.cs index 96af859a..0a4472ac 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/GetAvailablePagesEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/GetAvailablePagesEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Menu; using SimpleModule.Settings.Contracts; @@ -13,5 +14,5 @@ public class GetAvailablePagesEndpoint : IEndpoint public void Map(IEndpointRouteBuilder app) => app.MapGet(Route, (IReadOnlyList pages) => TypedResults.Ok(pages)) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ReorderMenuItemsEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ReorderMenuItemsEndpoint.cs index 5babeb9b..7e813f1c 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ReorderMenuItemsEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/ReorderMenuItemsEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Services; @@ -21,5 +22,5 @@ public void Map(IEndpointRouteBuilder app) => return TypedResults.NoContent(); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/SetHomePageEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/SetHomePageEndpoint.cs index af41d5ce..144fc9ee 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/SetHomePageEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/SetHomePageEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Services; @@ -21,5 +22,5 @@ public void Map(IEndpointRouteBuilder app) => return TypedResults.NoContent(); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs index 76c8901f..b83447e8 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Menus/UpdateMenuItemEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.FormRequests; using SimpleModule.Settings.Services; @@ -37,5 +38,5 @@ PublicMenuService service return entity is not null ? TypedResults.NoContent() : TypedResults.NotFound(); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.ManageMenus); } diff --git a/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/GetSettingsEndpoint.cs b/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/GetSettingsEndpoint.cs index a36e14ad..94e8af63 100644 --- a/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/GetSettingsEndpoint.cs +++ b/modules/Settings/src/SimpleModule.Settings/Endpoints/Settings/GetSettingsEndpoint.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; using SimpleModule.Core.Settings; using SimpleModule.Settings.Contracts; @@ -23,8 +24,14 @@ public void Map(IEndpointRouteBuilder app) => } var results = await settings.GetSettingValuesAsync(filter); + + // This admin list serves global (System/Application) configuration. + // User-scoped values are per-user and must not be enumerable here — + // they are read/written through the /me endpoints, which bind the + // target user to the caller's claims. + results = results.Where(r => r.Scope != SettingScope.User); return TypedResults.Ok(results); } ) - .RequireAuthorization(); + .RequirePermission(SettingsPermissions.View); } diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs index 1d75e333..b7d3f67b 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/FormRequestEndpointTests.cs @@ -195,7 +195,7 @@ public async Task UpdateSetting_InvalidScopeEnum_Returns422() [Fact] public async Task CreateMenuItem_ValidRequest_Returns201WithLocation() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", @@ -216,7 +216,7 @@ public async Task CreateMenuItem_ValidRequest_Returns201WithLocation() public async Task CreateMenuItem_MinimalRequest_Returns201() { // Only Label is required; all other fields are optional. - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", @@ -229,7 +229,7 @@ public async Task CreateMenuItem_MinimalRequest_Returns201() [Fact] public async Task CreateMenuItem_LabelTrimmedByPrepare_EntityHasTrimmedLabel() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", @@ -248,7 +248,7 @@ public async Task CreateMenuItem_LabelTrimmedByPrepare_EntityHasTrimmedLabel() [Fact] public async Task CreateMenuItem_EmptyLabel_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", @@ -262,7 +262,7 @@ public async Task CreateMenuItem_EmptyLabel_Returns422() [Fact] public async Task CreateMenuItem_LabelTooLong_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", @@ -276,7 +276,7 @@ public async Task CreateMenuItem_LabelTooLong_Returns422() [Fact] public async Task CreateMenuItem_UrlTooLong_Returns422() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.PostAsJsonAsync( "/api/settings/menus", diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/MenuEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/MenuEndpointTests.cs index f179e9fc..75006126 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/MenuEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/MenuEndpointTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Net.Http.Json; using FluentAssertions; +using SimpleModule.Settings; using SimpleModule.Settings.Contracts; using SimpleModule.Tests.Shared.Fixtures; @@ -12,15 +13,16 @@ public class MenuEndpointTests(SimpleModuleWebApplicationFactory factory) [Fact] public async Task GetMenus_Authenticated_Returns200() { + // The menu tree is the public navigation — any authenticated user may read it. var client = factory.CreateAuthenticatedClient(); var response = await client.GetAsync("/api/settings/menus"); response.StatusCode.Should().Be(HttpStatusCode.OK); } [Fact] - public async Task CreateMenuItem_Authenticated_Returns201() + public async Task CreateMenuItem_WithManageMenus_Returns201() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var request = new CreateMenuItemRequest { Label = "Test Menu", @@ -32,9 +34,23 @@ public async Task CreateMenuItem_Authenticated_Returns201() } [Fact] - public async Task UpdateMenuItem_NotFound_Returns404() + public async Task CreateMenuItem_WithoutManageMenus_ReturnsForbidden() { var client = factory.CreateAuthenticatedClient(); + var request = new CreateMenuItemRequest + { + Label = "Nope", + Url = "/nope", + IsVisible = true, + }; + var response = await client.PostAsJsonAsync("/api/settings/menus", request); + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task UpdateMenuItem_NotFound_Returns404() + { + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var request = new UpdateMenuItemRequest { Label = "Updated", IsVisible = true }; var response = await client.PutAsJsonAsync("/api/settings/menus/99999", request); response.StatusCode.Should().Be(HttpStatusCode.NotFound); @@ -43,11 +59,19 @@ public async Task UpdateMenuItem_NotFound_Returns404() [Fact] public async Task DeleteMenuItem_NotFound_Returns404() { - var client = factory.CreateAuthenticatedClient(); + var client = factory.CreateAuthenticatedClient([SettingsPermissions.ManageMenus]); var response = await client.DeleteAsync("/api/settings/menus/99999"); response.StatusCode.Should().Be(HttpStatusCode.NotFound); } + [Fact] + public async Task DeleteMenuItem_WithoutManageMenus_ReturnsForbidden() + { + var client = factory.CreateAuthenticatedClient(); + var response = await client.DeleteAsync("/api/settings/menus/99999"); + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + [Fact] public async Task GetMenus_Unauthenticated_Returns401() { diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs index d4e29fb9..2388a845 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Integration/SettingsEndpointTests.cs @@ -2,6 +2,7 @@ using System.Net.Http.Json; using System.Text.Json; using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; using SimpleModule.Core.Settings; using SimpleModule.Settings; using SimpleModule.Settings.Contracts; @@ -75,6 +76,52 @@ await client.PutAsJsonAsync( afterDelete.StatusCode.Should().Be(HttpStatusCode.NotFound); } + [Fact] + public async Task GetSettings_WithoutViewPermission_ReturnsForbidden() + { + var client = factory.CreateAuthenticatedClient(); + + var response = await client.GetAsync("/api/settings"); + + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task GetSettings_WithViewPermission_Returns200() + { + var client = factory.CreateAuthenticatedClient([SettingsPermissions.View]); + + var response = await client.GetAsync("/api/settings"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task GetSettings_DoesNotExposeUserScopedValues() + { + // Seed a user-scoped value for some other user directly via the service. + using (var scope = factory.Services.CreateScope()) + { + var settings = scope.ServiceProvider.GetRequiredService(); + await settings.SetSettingAsync( + UniqueKey("leak"), + JsonSerializer.Deserialize("\"secret\""), + SettingScope.User, + "some-other-user" + ); + } + + var client = factory.CreateAuthenticatedClient([SettingsPermissions.View]); + + // Even explicitly asking for User scope must not enumerate other users' values. + var response = await client.GetAsync("/api/settings?scope=2"); + response.StatusCode.Should().Be(HttpStatusCode.OK); + + var items = await response.Content.ReadFromJsonAsync>(JsonOptions); + items.Should().NotBeNull(); + items!.Should().NotContain(i => i.Scope == SettingScope.User); + } + [Fact] public async Task GetDefinitions_ReturnsArrayOfDefinitions() { diff --git a/modules/Users/src/SimpleModule.Users/Endpoints/Users/DeleteEndpoint.cs b/modules/Users/src/SimpleModule.Users/Endpoints/Users/DeleteEndpoint.cs index 3e006872..0d9ba6b9 100644 --- a/modules/Users/src/SimpleModule.Users/Endpoints/Users/DeleteEndpoint.cs +++ b/modules/Users/src/SimpleModule.Users/Endpoints/Users/DeleteEndpoint.cs @@ -1,8 +1,11 @@ +using System.Security.Claims; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; -using SimpleModule.Core.Endpoints; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Users.Contracts; namespace SimpleModule.Users.Endpoints.Users; @@ -16,10 +19,27 @@ public void Map(IEndpointRouteBuilder app) { app.MapDelete( Route, - (UserId id, IUserContracts userContracts) => - CrudEndpoints.Delete(() => userContracts.DeleteUserAsync(id)) + async Task> ( + UserId id, + ClaimsPrincipal principal, + IUserContracts userContracts, + IAuthorizer authorizer + ) => + { + // Load → authorize → act: deletion is admin-only (UserPolicy). + var existing = await userContracts.GetUserByIdAsync(id); + if (existing is null) + { + return TypedResults.NotFound(); + } + + await authorizer.AuthorizeAsync(principal, PolicyActions.Delete, existing); + + await userContracts.DeleteUserAsync(id); + return TypedResults.NoContent(); + } ) .WithTags(UsersConstants.ModuleName) - .RequireAuthorization(); + .RequirePermission(UsersPermissions.Delete); } } diff --git a/modules/Users/src/SimpleModule.Users/Endpoints/Users/GetByIdEndpoint.cs b/modules/Users/src/SimpleModule.Users/Endpoints/Users/GetByIdEndpoint.cs index ef70e130..b012f6c2 100644 --- a/modules/Users/src/SimpleModule.Users/Endpoints/Users/GetByIdEndpoint.cs +++ b/modules/Users/src/SimpleModule.Users/Endpoints/Users/GetByIdEndpoint.cs @@ -1,8 +1,11 @@ +using System.Security.Claims; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Users.Contracts; namespace SimpleModule.Users.Endpoints.Users; @@ -17,14 +20,23 @@ public void Map(IEndpointRouteBuilder app) Route, async Task, NotFound>> ( UserId id, - IUserContracts userContracts + ClaimsPrincipal principal, + IUserContracts userContracts, + IAuthorizer authorizer ) => { var user = await userContracts.GetUserByIdAsync(id); - return user is not null ? TypedResults.Ok(user) : TypedResults.NotFound(); + if (user is null) + { + return TypedResults.NotFound(); + } + + // Self-or-admin; UserPolicy denies others as 404. + await authorizer.AuthorizeAsync(principal, PolicyActions.View, user); + return TypedResults.Ok(user); } ) .WithTags(UsersConstants.ModuleName) - .RequireAuthorization(); + .RequirePermission(UsersPermissions.View); } } diff --git a/modules/Users/src/SimpleModule.Users/Endpoints/Users/UpdateEndpoint.cs b/modules/Users/src/SimpleModule.Users/Endpoints/Users/UpdateEndpoint.cs index 48d5f9ce..76a67949 100644 --- a/modules/Users/src/SimpleModule.Users/Endpoints/Users/UpdateEndpoint.cs +++ b/modules/Users/src/SimpleModule.Users/Endpoints/Users/UpdateEndpoint.cs @@ -1,8 +1,11 @@ +using System.Security.Claims; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Routing; using SimpleModule.Core; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; using SimpleModule.Users.Contracts; namespace SimpleModule.Users.Endpoints.Users; @@ -19,14 +22,25 @@ public void Map(IEndpointRouteBuilder app) async Task, NotFound>> ( UserId id, UpdateUserRequest request, - IUserContracts userContracts + ClaimsPrincipal principal, + IUserContracts userContracts, + IAuthorizer authorizer ) => { + // Load → authorize → act: authorize the target before mutating. + var existing = await userContracts.GetUserByIdAsync(id); + if (existing is null) + { + return TypedResults.NotFound(); + } + + await authorizer.AuthorizeAsync(principal, PolicyActions.Update, existing); + var user = await userContracts.UpdateUserAsync(id, request); return TypedResults.Ok(user); } ) .WithTags(UsersConstants.ModuleName) - .RequireAuthorization(); + .RequirePermission(UsersPermissions.Update); } } diff --git a/modules/Users/src/SimpleModule.Users/UserPolicy.cs b/modules/Users/src/SimpleModule.Users/UserPolicy.cs new file mode 100644 index 00000000..9d72272c --- /dev/null +++ b/modules/Users/src/SimpleModule.Users/UserPolicy.cs @@ -0,0 +1,46 @@ +using System.Security.Claims; +using SimpleModule.Core.Authorization; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Extensions; +using SimpleModule.Users.Contracts; + +namespace SimpleModule.Users; + +/// +/// Instance-level rules for user accounts, layered on the Users.View/Update/Delete +/// permission gates: a non-admin may view or update only their own account, and only +/// admins may delete accounts. View denials use DenyAsNotFound so a non-admin +/// cannot probe which account IDs exist. Auto-registered by the source generator. +/// +public sealed class UserPolicy : IPolicy +{ + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + UserDto resource, + CancellationToken cancellationToken = default + ) + { + var isAdmin = user.IsInRole(WellKnownRoles.Admin); + var result = action switch + { + PolicyActions.View => isAdmin || IsSelf(user, resource) + ? AuthorizationResult.Allow() + : AuthorizationResult.DenyAsNotFound("You can only view your own account."), + PolicyActions.Update => isAdmin || IsSelf(user, resource) + ? AuthorizationResult.Allow() + : AuthorizationResult.Deny("You can only update your own account."), + PolicyActions.Delete => isAdmin + ? AuthorizationResult.Allow() + : AuthorizationResult.Deny("Only administrators can delete accounts."), + _ => AuthorizationResult.Deny($"Unknown user action '{action}'."), + }; + return Task.FromResult(result); + } + + private static bool IsSelf(ClaimsPrincipal user, UserDto resource) + { + var userId = user.GetUserId(); + return !string.IsNullOrEmpty(userId) && resource.Id == UserId.From(userId); + } +} diff --git a/modules/Users/tests/SimpleModule.Users.Tests/Integration/UsersEndpointTests.cs b/modules/Users/tests/SimpleModule.Users.Tests/Integration/UsersEndpointTests.cs index a89b3567..195e416d 100644 --- a/modules/Users/tests/SimpleModule.Users.Tests/Integration/UsersEndpointTests.cs +++ b/modules/Users/tests/SimpleModule.Users.Tests/Integration/UsersEndpointTests.cs @@ -4,6 +4,7 @@ using FluentAssertions; using SimpleModule.Tests.Shared.Fakes; using SimpleModule.Tests.Shared.Fixtures; +using SimpleModule.Users; using SimpleModule.Users.Contracts; namespace Users.Tests.Integration; @@ -47,10 +48,20 @@ public async Task GetUserById_Unauthenticated_ReturnsUnauthorized() } [Fact] - public async Task GetUserById_Authenticated_WithInvalidId_ReturnsNotFound() + public async Task GetUserById_WithoutViewPermission_ReturnsForbidden() { var client = _factory.CreateAuthenticatedClient(); + var response = await client.GetAsync("/api/users/some-id"); + + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task GetUserById_Authenticated_WithInvalidId_ReturnsNotFound() + { + var client = _factory.CreateAuthenticatedClient([UsersPermissions.View]); + var response = await client.GetAsync("/api/users/nonexistent-id"); response.StatusCode.Should().Be(HttpStatusCode.NotFound); @@ -128,10 +139,125 @@ public async Task DeleteUser_Unauthenticated_ReturnsUnauthorized() [Fact] public async Task DeleteUser_Authenticated_WithNonExistentId_Returns404() { - var client = _factory.CreateAuthenticatedClient(); + var client = _factory.CreateAuthenticatedClient([UsersPermissions.Delete]); var response = await client.DeleteAsync("/api/users/nonexistent-id"); response.StatusCode.Should().Be(HttpStatusCode.NotFound); } + + // --- Instance-level authorization (UserPolicy) --------------------------------- + + private async Task CreateUserAsync(string email) + { + var admin = _factory.CreateAuthenticatedClient(new Claim(ClaimTypes.Role, "Admin")); + var response = await admin.PostAsJsonAsync( + "/api/users", + new CreateUserRequest + { + Email = email, + DisplayName = "Policy Target", + Password = "TestPass1234", + } + ); + response.StatusCode.Should().Be(HttpStatusCode.Created); + var dto = await response.Content.ReadFromJsonAsync(); + dto.Should().NotBeNull(); + return dto!; + } + + private HttpClient ClientFor(string userId, params string[] permissions) => + _factory.CreateAuthenticatedClient( + permissions, + new Claim(ClaimTypes.NameIdentifier, userId) + ); + + private HttpClient AdminClient() => + _factory.CreateAuthenticatedClient( + new Claim(ClaimTypes.Role, "Admin"), + new Claim(ClaimTypes.NameIdentifier, "admin-tester") + ); + + [Fact] + public async Task GetUserById_NonAdminViewingAnotherUser_Returns404() + { + var target = await CreateUserAsync("view-target@test.com"); + var client = ClientFor("a-different-user", UsersPermissions.View); + + var response = await client.GetAsync($"/api/users/{target.Id.Value}"); + + // DenyAsNotFound — a non-owner cannot tell the account exists. + response.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task GetUserById_NonAdminViewingSelf_ReturnsOk() + { + var target = await CreateUserAsync("view-self@test.com"); + var client = ClientFor(target.Id.Value, UsersPermissions.View); + + var response = await client.GetAsync($"/api/users/{target.Id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task GetUserById_Admin_CanViewAnotherUser() + { + var target = await CreateUserAsync("view-admin@test.com"); + + var response = await AdminClient().GetAsync($"/api/users/{target.Id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task UpdateUser_NonAdminUpdatingAnotherUser_ReturnsForbidden() + { + var target = await CreateUserAsync("update-other@test.com"); + var client = ClientFor("a-different-user", UsersPermissions.Update); + var request = new UpdateUserRequest { Email = target.Email, DisplayName = "Hacked" }; + + var response = await client.PutAsJsonAsync($"/api/users/{target.Id.Value}", request); + + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task UpdateUser_NonAdminUpdatingSelf_ReturnsOk() + { + var target = await CreateUserAsync("update-self@test.com"); + var client = ClientFor(target.Id.Value, UsersPermissions.Update); + var request = new UpdateUserRequest + { + Email = target.Email, + DisplayName = "Renamed Self", + }; + + var response = await client.PutAsJsonAsync($"/api/users/{target.Id.Value}", request); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task DeleteUser_NonAdminWithPermission_ReturnsForbidden() + { + var target = await CreateUserAsync("delete-nonadmin@test.com"); + // Even the owner, with the Delete permission, may not delete — admin-only. + var client = ClientFor(target.Id.Value, UsersPermissions.Delete); + + var response = await client.DeleteAsync($"/api/users/{target.Id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task DeleteUser_Admin_Returns204() + { + var target = await CreateUserAsync("delete-admin@test.com"); + + var response = await AdminClient().DeleteAsync($"/api/users/{target.Id.Value}"); + + response.StatusCode.Should().Be(HttpStatusCode.NoContent); + } } diff --git a/tests/SimpleModule.Core.Tests/Authorization/AuthorizeResourceFilterTests.cs b/tests/SimpleModule.Core.Tests/Authorization/AuthorizeResourceFilterTests.cs new file mode 100644 index 00000000..5717b97e --- /dev/null +++ b/tests/SimpleModule.Core.Tests/Authorization/AuthorizeResourceFilterTests.cs @@ -0,0 +1,158 @@ +using System.Net; +using System.Security.Claims; +using FluentAssertions; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Exceptions; + +namespace SimpleModule.Core.Tests.Authorization; + +public sealed record FilterTestDoc(string Kind); + +public sealed record FilterTestOrphan(string Id); + +/// +/// Shared TestServer host for the AuthorizeResource filter tests — booted once per +/// class. The environment is pinned so Development middleware (developer exception +/// page) can't swallow the exceptions the tests assert on. The Doc policy decides by +/// resource Kind: "allow" → allow, "hide" → DenyAsNotFound, anything else → Deny. +/// +public sealed class AuthorizeResourceAppFixture : IAsyncLifetime +{ + public WebApplication App { get; private set; } = null!; + public HttpClient Client { get; private set; } = null!; + + private sealed class DocPolicy : IPolicy + { + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + FilterTestDoc resource, + CancellationToken cancellationToken = default + ) => + Task.FromResult( + resource.Kind switch + { + "allow" => AuthorizationResult.Allow(), + "hide" => AuthorizationResult.DenyAsNotFound("Hidden"), + _ => AuthorizationResult.Deny("Denied by DocPolicy"), + } + ); + } + + private sealed class DocResolver : IResourceResolver + { + public ValueTask ResolveAsync( + string routeValue, + CancellationToken cancellationToken = default + ) => ValueTask.FromResult(routeValue == "missing" ? null : new FilterTestDoc(routeValue)); + } + + public async ValueTask InitializeAsync() + { + var builder = WebApplication.CreateBuilder( + new WebApplicationOptions { EnvironmentName = Environments.Production } + ); + builder.WebHost.UseTestServer(); + builder.Services.AddOptions(); + builder.Services.AddScoped(); + builder.Services.AddScoped, DocPolicy>(); + builder.Services.AddScoped, DocResolver>(); + + App = builder.Build(); + App.MapGet("/docs/{id}", (string id) => Results.Ok(id)) + .AuthorizeResource(PolicyActions.Update); + App.MapGet("/misnamed/{docId}", (string docId) => Results.Ok(docId)) + .AuthorizeResource(PolicyActions.Update); // route param is NOT "id" + App.MapGet("/optional/{id?}", (string? id) => Results.Ok(id)) + .AuthorizeResource(PolicyActions.Update); + App.MapGet("/orphans/{id}", (string id) => Results.Ok(id)) + .AuthorizeResource(PolicyActions.Update); // no resolver registered + + await App.StartAsync(); + Client = App.GetTestClient(); + } + + public async ValueTask DisposeAsync() + { + Client.Dispose(); + await App.DisposeAsync(); + } +} + +/// +/// Exercises the declarative AuthorizeResource endpoint filter against a real routing +/// pipeline: resolver dispatch, policy outcomes, and the failure paths — 404 for +/// runtime-absent values, loud InvalidOperationException for misconfiguration. +/// +public sealed class AuthorizeResourceFilterTests(AuthorizeResourceAppFixture fixture) + : IClassFixture +{ + [Fact] + public async Task AllowedResource_InvokesHandler() + { + var response = await fixture.Client.GetAsync("/docs/allow"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task DeniedResource_ThrowsForbidden() + { + var act = () => fixture.Client.GetAsync("/docs/deny"); + + await act.Should().ThrowAsync().WithMessage("Denied by DocPolicy"); + } + + [Fact] + public async Task DenyAsNotFoundResource_ThrowsNotFound() + { + var act = () => fixture.Client.GetAsync("/docs/hide"); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task MissingResource_ThrowsNotFound() + { + var act = () => fixture.Client.GetAsync("/docs/missing"); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task OptionalParameterOmitted_ThrowsNotFound() + { + // The parameter exists in the template but wasn't supplied — runtime absence, + // not misconfiguration. + var act = () => fixture.Client.GetAsync("/optional"); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task WrongRouteParameterName_ThrowsInvalidOperation() + { + // Misconfiguration must fail loudly, not surface as 404. + var act = () => fixture.Client.GetAsync("/misnamed/allow"); + + await act.Should() + .ThrowAsync() + .WithMessage("*route parameter*"); + } + + [Fact] + public async Task MissingResolver_ThrowsInvalidOperation() + { + var act = () => fixture.Client.GetAsync("/orphans/anything"); + + await act.Should() + .ThrowAsync() + .WithMessage("*IResourceResolver*"); + } +} diff --git a/tests/SimpleModule.Core.Tests/Authorization/AuthorizerTests.cs b/tests/SimpleModule.Core.Tests/Authorization/AuthorizerTests.cs new file mode 100644 index 00000000..1147cafd --- /dev/null +++ b/tests/SimpleModule.Core.Tests/Authorization/AuthorizerTests.cs @@ -0,0 +1,272 @@ +using System.Security.Claims; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using SimpleModule.Core.Authorization.Policies; +using SimpleModule.Core.Exceptions; + +namespace SimpleModule.Core.Tests.Authorization; + +public class AuthorizerTests +{ + private sealed record Widget(string OwnerId); + + private sealed record Gadget(string Name); + + private sealed class WidgetOwnerPolicy : IPolicy + { + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Widget resource, + CancellationToken cancellationToken = default + ) + { + var result = + resource.OwnerId == user.FindFirstValue("sub") + ? AuthorizationResult.Allow() + : AuthorizationResult.Deny("Not the owner"); + return Task.FromResult(result); + } + } + + private sealed class AlwaysAllowPolicy : IPolicy + { + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Widget resource, + CancellationToken cancellationToken = default + ) => Task.FromResult(AuthorizationResult.Allow()); + } + + private sealed class AlwaysDenyPolicy : IPolicy + { + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Widget resource, + CancellationToken cancellationToken = default + ) => Task.FromResult(AuthorizationResult.Deny("Denied by second policy")); + } + + private sealed class DenyAsNotFoundPolicy : IPolicy + { + public Task AuthorizeAsync( + ClaimsPrincipal user, + string action, + Widget resource, + CancellationToken cancellationToken = default + ) => Task.FromResult(AuthorizationResult.DenyAsNotFound("Hidden")); + } + + private static ClaimsPrincipal CreateUser(string userId) => + new(new ClaimsIdentity([new Claim("sub", userId)], "test")); + + private static Authorizer CreateAuthorizer( + Action? configureServices = null, + Action? configureOptions = null + ) + { + var services = new ServiceCollection(); + configureServices?.Invoke(services); + var provider = services.BuildServiceProvider(); + + var options = new PolicyAuthorizationOptions(); + configureOptions?.Invoke(options); + + return new Authorizer(provider, Options.Create(options)); + } + + [Fact] + public async Task CheckAsync_OwnerMatches_Allows() + { + var authorizer = CreateAuthorizer(s => s.AddScoped, WidgetOwnerPolicy>()); + + var result = await authorizer.CheckAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + result.IsAllowed.Should().BeTrue(); + } + + [Fact] + public async Task CheckAsync_OwnerDiffers_DeniesWithReason() + { + var authorizer = CreateAuthorizer(s => s.AddScoped, WidgetOwnerPolicy>()); + + var result = await authorizer.CheckAsync( + CreateUser("user-2"), + PolicyActions.Update, + new Widget("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.Reason.Should().Be("Not the owner"); + } + + [Fact] + public async Task CheckAsync_NoPolicyRegistered_ThrowsMissingPolicyException() + { + var authorizer = CreateAuthorizer(); + + var act = () => + authorizer.CheckAsync(CreateUser("user-1"), PolicyActions.View, new Gadget("g")); + + await act.Should() + .ThrowAsync() + .WithMessage("*IPolicy*"); + } + + [Fact] + public async Task CheckAsync_MultiplePolicies_DenyWins() + { + var authorizer = CreateAuthorizer(s => + { + s.AddScoped, AlwaysAllowPolicy>(); + s.AddScoped, AlwaysDenyPolicy>(); + }); + + var result = await authorizer.CheckAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.Reason.Should().Be("Denied by second policy"); + } + + [Fact] + public async Task CheckAsync_MultiplePolicies_AllAllow_Allows() + { + var authorizer = CreateAuthorizer(s => + { + s.AddScoped, AlwaysAllowPolicy>(); + s.AddScoped, WidgetOwnerPolicy>(); + }); + + var result = await authorizer.CheckAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + result.IsAllowed.Should().BeTrue(); + } + + [Fact] + public async Task AuthorizeAsync_Allowed_DoesNotThrow() + { + var authorizer = CreateAuthorizer(s => s.AddScoped, WidgetOwnerPolicy>()); + + var act = () => + authorizer.AuthorizeAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + await act.Should().NotThrowAsync(); + } + + [Fact] + public async Task AuthorizeAsync_DeniedNonViewAction_ThrowsForbiddenWithReason() + { + var authorizer = CreateAuthorizer(s => s.AddScoped, WidgetOwnerPolicy>()); + + var act = () => + authorizer.AuthorizeAsync( + CreateUser("user-2"), + PolicyActions.Update, + new Widget("user-1") + ); + + await act.Should().ThrowAsync().WithMessage("Not the owner"); + } + + [Fact] + public async Task AuthorizeAsync_DeniedViewAction_ThrowsForbiddenByDefault() + { + // NotFoundActions is empty by default — DenyAsNotFound is the per-decision way + // to surface 404; a plain Deny keeps its 403 + reason. + var authorizer = CreateAuthorizer(s => s.AddScoped, WidgetOwnerPolicy>()); + + var act = () => + authorizer.AuthorizeAsync( + CreateUser("user-2"), + PolicyActions.View, + new Widget("user-1") + ); + + await act.Should().ThrowAsync().WithMessage("Not the owner"); + } + + [Fact] + public async Task AuthorizeAsync_CustomNotFoundAction_ThrowsNotFound() + { + var authorizer = CreateAuthorizer( + s => s.AddScoped, WidgetOwnerPolicy>(), + o => o.NotFoundActions.Add("archive") + ); + + var act = () => + authorizer.AuthorizeAsync(CreateUser("user-2"), "archive", new Widget("user-1")); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task CheckAsync_DenyAsNotFound_CarriesFlagAndReason() + { + var authorizer = CreateAuthorizer(s => s.AddScoped, DenyAsNotFoundPolicy>()); + + var result = await authorizer.CheckAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + result.IsAllowed.Should().BeFalse(); + result.TreatAsNotFound.Should().BeTrue(); + result.Reason.Should().Be("Hidden"); + } + + [Fact] + public async Task AuthorizeAsync_DenyAsNotFound_ThrowsNotFoundForAnyAction() + { + // The policy's per-decision flag wins even when the action is not in + // NotFoundActions — no host/module options mutation required. + var authorizer = CreateAuthorizer(s => s.AddScoped, DenyAsNotFoundPolicy>()); + + var act = () => + authorizer.AuthorizeAsync( + CreateUser("user-1"), + PolicyActions.Update, + new Widget("user-1") + ); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task AuthorizeAsync_ViewAddedToNotFoundActions_ThrowsNotFound() + { + // Host-level opt-in override: a listed action maps every denial to 404. + var authorizer = CreateAuthorizer( + s => s.AddScoped, WidgetOwnerPolicy>(), + o => o.NotFoundActions.Add(PolicyActions.View) + ); + + var act = () => + authorizer.AuthorizeAsync( + CreateUser("user-2"), + PolicyActions.View, + new Widget("user-1") + ); + + await act.Should().ThrowAsync(); + } +} diff --git a/tests/SimpleModule.Generator.Tests/ContractAutoDiscoveryTests.cs b/tests/SimpleModule.Generator.Tests/ContractAutoDiscoveryTests.cs index 0e54224c..9337611d 100644 --- a/tests/SimpleModule.Generator.Tests/ContractAutoDiscoveryTests.cs +++ b/tests/SimpleModule.Generator.Tests/ContractAutoDiscoveryTests.cs @@ -16,88 +16,12 @@ private static CSharpCompilation CreateMultiAssemblyCompilation( string contractsSource, string hostSource, string contractsAssemblyName = "TestAssembly.Contracts" - ) - { - var coreRef = MetadataReference.CreateFromFile( - typeof(SimpleModule.Core.IModule).Assembly.Location - ); - var runtimeDir = Path.GetDirectoryName(typeof(object).Assembly.Location)!; - - var baseRefs = new List - { - MetadataReference.CreateFromFile(typeof(object).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Attribute).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Console).Assembly.Location), - MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Runtime.dll")), - MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Collections.dll")), - coreRef, - }; - - // Compile the contracts assembly - var contractsTree = CSharpSyntaxTree.ParseText(contractsSource); - var contractsCompilation = CSharpCompilation.Create( - contractsAssemblyName, - [contractsTree], - baseRefs, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) - ); - - // Get the contracts assembly as a reference - using var ms = new MemoryStream(); - var emitResult = contractsCompilation.Emit(ms); - emitResult - .Success.Should() - .BeTrue( - "contracts assembly should compile without errors. Diagnostics: " - + string.Join( - ", ", - emitResult - .Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error) - .Select(d => d.ToString()) - ) - ); - ms.Seek(0, SeekOrigin.Begin); - var contractsRef = MetadataReference.CreateFromImage(ms.ToArray()); - - // Now compile the host assembly referencing contracts - var hostRefs = new List(baseRefs) { contractsRef }; - - // Add DI abstractions - var aspNetDir = Path.GetDirectoryName( - typeof(Microsoft.Extensions.DependencyInjection.IServiceCollection).Assembly.Location - ); - if (aspNetDir is not null) - { - var diAbstractions = Path.Combine( - aspNetDir, - "Microsoft.Extensions.DependencyInjection.Abstractions.dll" - ); - if (File.Exists(diAbstractions)) - hostRefs.Add(MetadataReference.CreateFromFile(diAbstractions)); - } - - hostRefs.Add( - MetadataReference.CreateFromFile( - typeof(Microsoft.Extensions.Configuration.IConfiguration).Assembly.Location - ) + ) => + GeneratorTestHelper.CreateMultiAssemblyCompilation( + [(contractsAssemblyName, contractsSource)], + hostSource ); - // Add ASP.NET Core HTTP abstractions (for IResult) - hostRefs.Add( - MetadataReference.CreateFromFile( - typeof(Microsoft.AspNetCore.Http.IResult).Assembly.Location - ) - ); - - var hostTree = CSharpSyntaxTree.ParseText(hostSource); - return CSharpCompilation.Create( - "TestAssembly", - [hostTree], - hostRefs, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) - ); - } - [Fact] public void SingleImplementation_GeneratesAddScoped() { diff --git a/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs b/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs index 5bb100b7..6d52c737 100644 --- a/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs +++ b/tests/SimpleModule.Generator.Tests/DiagnosticCatalogTests.cs @@ -183,6 +183,22 @@ private static readonly Dictionary< DiagnosticSeverity.Error, "SimpleModule.Generator" ), + ["PolicyResourceMustBeDto"] = ( + "SM0058", + DiagnosticSeverity.Error, + "SimpleModule.Generator" + ), + ["PolicyNotPublic"] = ("SM0059", DiagnosticSeverity.Error, "SimpleModule.Generator"), + ["PolicyForForeignResource"] = ( + "SM0060", + DiagnosticSeverity.Error, + "SimpleModule.Generator" + ), + ["PolicyMustNotBeGeneric"] = ( + "SM0061", + DiagnosticSeverity.Error, + "SimpleModule.Generator" + ), }; [Fact] diff --git a/tests/SimpleModule.Generator.Tests/DtoConventionTests.cs b/tests/SimpleModule.Generator.Tests/DtoConventionTests.cs index 5a442e19..19f0d43f 100644 --- a/tests/SimpleModule.Generator.Tests/DtoConventionTests.cs +++ b/tests/SimpleModule.Generator.Tests/DtoConventionTests.cs @@ -15,83 +15,11 @@ private static CSharpCompilation CreateMultiAssemblyCompilation( string contractsSource, string hostSource, string contractsAssemblyName = "TestAssembly.Contracts" - ) - { - var coreRef = MetadataReference.CreateFromFile( - typeof(SimpleModule.Core.IModule).Assembly.Location + ) => + GeneratorTestHelper.CreateMultiAssemblyCompilation( + [(contractsAssemblyName, contractsSource)], + hostSource ); - var runtimeDir = Path.GetDirectoryName(typeof(object).Assembly.Location)!; - - var baseRefs = new List - { - MetadataReference.CreateFromFile(typeof(object).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Attribute).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Console).Assembly.Location), - MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Runtime.dll")), - MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Collections.dll")), - coreRef, - }; - - // Compile the contracts assembly - var contractsTree = CSharpSyntaxTree.ParseText(contractsSource); - var contractsCompilation = CSharpCompilation.Create( - contractsAssemblyName, - [contractsTree], - baseRefs, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) - ); - - using var ms = new MemoryStream(); - var emitResult = contractsCompilation.Emit(ms); - emitResult - .Success.Should() - .BeTrue( - "contracts assembly should compile without errors. Diagnostics: " - + string.Join( - ", ", - emitResult - .Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error) - .Select(d => d.ToString()) - ) - ); - ms.Seek(0, SeekOrigin.Begin); - var contractsRef = MetadataReference.CreateFromImage(ms.ToArray()); - - var hostRefs = new List(baseRefs) { contractsRef }; - - var aspNetDir = Path.GetDirectoryName( - typeof(Microsoft.Extensions.DependencyInjection.IServiceCollection).Assembly.Location - ); - if (aspNetDir is not null) - { - var diAbstractions = Path.Combine( - aspNetDir, - "Microsoft.Extensions.DependencyInjection.Abstractions.dll" - ); - if (File.Exists(diAbstractions)) - hostRefs.Add(MetadataReference.CreateFromFile(diAbstractions)); - } - - hostRefs.Add( - MetadataReference.CreateFromFile( - typeof(Microsoft.Extensions.Configuration.IConfiguration).Assembly.Location - ) - ); - - hostRefs.Add( - MetadataReference.CreateFromFile( - typeof(Microsoft.AspNetCore.Http.IResult).Assembly.Location - ) - ); - - var hostTree = CSharpSyntaxTree.ParseText(hostSource); - return CSharpCompilation.Create( - "TestAssembly", - [hostTree], - hostRefs, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) - ); - } [Fact] public void PublicContractsType_IncludedInTypeScript() diff --git a/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs b/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs index 581293c8..b390d7dc 100644 --- a/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs +++ b/tests/SimpleModule.Generator.Tests/Helpers/GeneratorTestHelper.cs @@ -77,6 +77,13 @@ public static CSharpCompilation CreateCompilation(params string[] sources) if (File.Exists(tasksPath)) references.Add(MetadataReference.CreateFromFile(tasksPath)); + // Add System.Security.Claims for ClaimsPrincipal (used by IPolicy implementors) + references.Add( + MetadataReference.CreateFromFile( + typeof(System.Security.Claims.ClaimsPrincipal).Assembly.Location + ) + ); + return CSharpCompilation.Create( "TestAssembly", syntaxTrees, @@ -154,6 +161,68 @@ public static CSharpCompilation CreateCompilationWithEfCore(params string[] sour return compilation.AddReferences(efCoreReferences); } + /// + /// Compiles one or more referenced assemblies in order (later sources may reference + /// earlier ones), then a host assembly ("TestAssembly") referencing all of them. + /// Mirrors the real layout where modules and their *.Contracts assemblies are + /// separate compilations. + /// + public static CSharpCompilation CreateMultiAssemblyCompilation( + (string AssemblyName, string Source)[] referencedAssemblies, + string hostSource + ) + { + var runtimeDir = Path.GetDirectoryName(typeof(object).Assembly.Location)!; + var baseRefs = new List + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + MetadataReference.CreateFromFile(typeof(Attribute).Assembly.Location), + MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Runtime.dll")), + MetadataReference.CreateFromFile(Path.Combine(runtimeDir, "System.Collections.dll")), + MetadataReference.CreateFromFile(typeof(SimpleModule.Core.IModule).Assembly.Location), + MetadataReference.CreateFromFile( + typeof(System.Security.Claims.ClaimsPrincipal).Assembly.Location + ), + MetadataReference.CreateFromFile( + typeof(Microsoft.Extensions.DependencyInjection.IServiceCollection) + .Assembly + .Location + ), + MetadataReference.CreateFromFile( + typeof(Microsoft.Extensions.Configuration.IConfiguration).Assembly.Location + ), + }; + + var builtRefs = new List(); + foreach (var (assemblyName, source) in referencedAssemblies) + { + var compilation = CSharpCompilation.Create( + assemblyName, + [CSharpSyntaxTree.ParseText(source)], + [.. baseRefs, .. builtRefs], + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + ); + + using var ms = new MemoryStream(); + var emit = compilation.Emit(ms); + if (!emit.Success) + { + var errors = string.Join( + ", ", + emit.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error) + ); + throw new InvalidOperationException( + $"Referenced assembly {assemblyName} failed to compile: {errors}" + ); + } + builtRefs.Add(MetadataReference.CreateFromImage(ms.ToArray())); + } + + // Compose the host side from CreateCompilation so the two paths share one + // reference list and can't drift. + return CreateCompilation(hostSource).AddReferences(builtRefs); + } + public static GeneratorDriverRunResult RunGenerator(CSharpCompilation compilation) { var generator = new ModuleDiscovererGenerator(); diff --git a/tests/SimpleModule.Generator.Tests/PolicyAutoDiscoveryTests.cs b/tests/SimpleModule.Generator.Tests/PolicyAutoDiscoveryTests.cs new file mode 100644 index 00000000..9684a8f6 --- /dev/null +++ b/tests/SimpleModule.Generator.Tests/PolicyAutoDiscoveryTests.cs @@ -0,0 +1,600 @@ +using FluentAssertions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using SimpleModule.Generator.Tests.Helpers; + +namespace SimpleModule.Generator.Tests; + +public class PolicyAutoDiscoveryTests +{ + private const string PolicyForDtoSource = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + public sealed class ProductPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + private static string GetModuleExtensions(CSharpCompilation compilation) + { + var result = GeneratorTestHelper.RunGenerator(compilation); + return result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + } + + [Fact] + public void PolicyImplementor_GeneratesTryAddEnumerableRegistration() + { + var moduleExt = GetModuleExtensions( + GeneratorTestHelper.CreateCompilation(PolicyForDtoSource) + ); + + moduleExt + .Should() + .Contain( + "services.TryAddEnumerable(global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor.Scoped, global::TestApp.ProductPolicy>());" + ); + } + + [Fact] + public void PolicyForDtoResource_DoesNotReportPolicyDiagnostics() + { + var compilation = GeneratorTestHelper.CreateCompilation(PolicyForDtoSource); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics + .Should() + .NotContain(d => d.Id == "SM0058" || d.Id == "SM0059" || d.Id == "SM0060"); + } + + [Fact] + public void PolicyForNonDtoResource_ReportsSm0058() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + // Not a [Dto] and not in a .Contracts assembly + public class InternalProduct + { + public string OwnerId { get; set; } = ""; + } + + public sealed class InternalProductPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, InternalProduct resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + var sm0058 = diagnostics.Where(d => d.Id == "SM0058").ToList(); + sm0058.Should().ContainSingle(); + sm0058[0].GetMessage(null).Should().Contain("InternalProductPolicy"); + sm0058[0].GetMessage(null).Should().Contain("InternalProduct"); + } + + [Fact] + public void NonPublicPolicy_ReportsSm0059AndIsNotRegistered() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + internal sealed class HiddenPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var (result, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Where(d => d.Id == "SM0059").Should().ContainSingle(); + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().NotContain("HiddenPolicy"); + } + + [Fact] + public void NestedPolicy_IsDiscoveredAndRegistered() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + public class ProductFeature + { + public sealed class NestedPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + } + """; + + var moduleExt = GetModuleExtensions(GeneratorTestHelper.CreateCompilation(source)); + + moduleExt.Should().Contain("global::TestApp.ProductFeature.NestedPolicy"); + } + + [Fact] + public void AbstractPolicy_IsNotRegistered() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + public abstract class BasePolicy : IPolicy + { + public abstract Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default); + } + } + """; + + var moduleExt = GetModuleExtensions(GeneratorTestHelper.CreateCompilation(source)); + + moduleExt.Should().NotContain("BasePolicy"); + } + + [Fact] + public void NoPolicies_OmitsPolicySection() + { + var source = """ + using SimpleModule.Core; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + } + """; + + var moduleExt = GetModuleExtensions(GeneratorTestHelper.CreateCompilation(source)); + + moduleExt.Should().NotContain("Auto-discovered resource policies"); + } + + [Fact] + public void ManuallyRegisteredInternalPolicy_NoDiagnosticsAndNotEmitted() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) + { + services.AddScoped, ManualPolicy>(); + } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + // Internal is fine: the module registers it from within its own assembly. + [ManualContractRegistration] + internal sealed class ManualPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var (result, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics + .Should() + .NotContain(d => d.Id == "SM0059" || d.Id == "SM0061", "manual registration waives the auto-registration rules"); + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().NotContain("TryAddEnumerable(global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor.Scoped, global::TestApp.ManualPolicy>"); + } + + [Fact] + public void GenericPolicy_ReportsSm0061AndIsNotRegistered() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + public class OpenPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, T resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var (result, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Where(d => d.Id == "SM0061").Should().ContainSingle(); + diagnostics.Should().NotContain(d => d.Id == "SM0058"); // suppressed for generics + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().NotContain("OpenPolicy"); + } + + [Fact] + public void PublicPolicyInInternalOuterClass_ReportsSm0059AndIsNotRegistered() + { + var source = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + [Dto] + public class Product + { + public string OwnerId { get; set; } = ""; + } + + internal class ProductFeature + { + // Declared public, but unreachable from generated code because the + // outer class is internal. + public sealed class HiddenNestedPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + } + """; + + var compilation = GeneratorTestHelper.CreateCompilation(source); + var (result, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics.Where(d => d.Id == "SM0059").Should().ContainSingle(); + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().NotContain("HiddenNestedPolicy"); + } + + // --- Multi-assembly scenarios ------------------------------------------------- + + [Fact] + public void PolicyInHostAssembly_IsDiscoveredAndRegistered() + { + // The module lives in a referenced assembly; the policy lives in the compiling + // (host) assembly, which has no [Module] class of its own. + var productsModule = """ + using SimpleModule.Core; + using Microsoft.Extensions.DependencyInjection; + + namespace Products + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + } + """; + + var productsContracts = """ + namespace Products.Contracts + { + public class Product + { + public string OwnerId { get; set; } = ""; + } + } + """; + + var hostSource = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core.Authorization.Policies; + + namespace TestApp + { + public sealed class HostProductPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Products.Contracts.Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var compilation = GeneratorTestHelper.CreateMultiAssemblyCompilation( + [("Products", productsModule), ("Products.Contracts", productsContracts)], + hostSource + ); + var result = GeneratorTestHelper.RunGenerator(compilation); + + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().Contain("global::TestApp.HostProductPolicy"); + } + + [Fact] + public void PolicyInContractsAssembly_IsDiscoveredAndPassesSm0058() + { + // Resource has no [Dto] — living in the .Contracts assembly is sufficient + // (covers [NoDtoGeneration]/IEvent entities excluded from DtoTypes). + var contractsSource = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core.Authorization.Policies; + + namespace TestApp.Contracts + { + public class Product + { + public string OwnerId { get; set; } = ""; + } + + public sealed class ProductPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Allow()); + } + } + """; + + var hostSource = """ + using SimpleModule.Core; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + } + """; + + var compilation = GeneratorTestHelper.CreateMultiAssemblyCompilation( + [("TestAssembly.Contracts", contractsSource)], + hostSource + ); + var (result, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + diagnostics + .Should() + .NotContain(d => d.Id == "SM0058" || d.Id == "SM0059" || d.Id == "SM0060"); + var moduleExt = result + .GeneratedTrees.First(t => + t.FilePath.EndsWith("ModuleExtensions.g.cs", StringComparison.Ordinal) + ) + .GetText() + .ToString(); + moduleExt.Should().Contain("global::TestApp.Contracts.ProductPolicy"); + } + + [Fact] + public void PolicyForForeignModuleResource_ReportsSm0060() + { + // The Products module lives in its own assembly so its contracts assembly maps + // to module "Products"; the host assembly hosts the Notifications module with a + // policy targeting the Products resource — a foreign policy. + var productsModule = """ + using SimpleModule.Core; + using Microsoft.Extensions.DependencyInjection; + + namespace Products + { + [Module("Products")] + public class ProductsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + } + """; + + var productsContracts = """ + namespace Products.Contracts + { + public class Product + { + public string OwnerId { get; set; } = ""; + } + } + """; + + var hostSource = """ + using System.Security.Claims; + using System.Threading; + using System.Threading.Tasks; + using SimpleModule.Core; + using SimpleModule.Core.Authorization.Policies; + using Microsoft.Extensions.DependencyInjection; + + namespace TestApp + { + [Module("Notifications")] + public class NotificationsModule : IModule + { + public void ConfigureServices(IServiceCollection services, Microsoft.Extensions.Configuration.IConfiguration configuration) { } + } + + public sealed class ForeignProductPolicy : IPolicy + { + public Task AuthorizeAsync(ClaimsPrincipal user, string action, Products.Contracts.Product resource, CancellationToken cancellationToken = default) => + Task.FromResult(AuthorizationResult.Deny("foreign veto")); + } + } + """; + + var compilation = GeneratorTestHelper.CreateMultiAssemblyCompilation( + [("Products", productsModule), ("Products.Contracts", productsContracts)], + hostSource + ); + var (_, diagnostics) = GeneratorTestHelper.RunGeneratorWithDiagnostics(compilation); + + var sm0060 = diagnostics.Where(d => d.Id == "SM0060").ToList(); + sm0060.Should().ContainSingle(); + sm0060[0].GetMessage(null).Should().Contain("ForeignProductPolicy"); + sm0060[0].GetMessage(null).Should().Contain("Products"); + } +} diff --git a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs index d016558c..0e065f9a 100644 --- a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs +++ b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs @@ -248,6 +248,7 @@ public void SortModules_WithDependencies_ReordersByDependency() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -325,6 +326,7 @@ public void SortModules_WithCycle_ReturnsOriginalOrder() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false, @@ -418,6 +420,7 @@ public void SortModules_NoDependencies_PreservesOriginalOrder() ImmutableArray.Empty, ImmutableArray.Empty, ImmutableArray.Empty, + ImmutableArray.Empty, ImmutableArray.Empty, false, false,