diff --git a/Distribution/user_management_PLAN.md b/Distribution/user_management_PLAN.md new file mode 100644 index 0000000..c0be7b6 --- /dev/null +++ b/Distribution/user_management_PLAN.md @@ -0,0 +1,607 @@ +# User Management Feature Implementation Guide + +## Goal + +Add user management, authentication, role-based permissions, and hierarchy-aware data access to the existing Distribution API. + +This plan is scoped to the current project and its implemented modules: + +- Region +- Route +- Vehicle +- Store +- Warehouse +- Order + +It follows the codebase's actual conventions (verified against `Features/Region`, `Features/Order`, `Data/AppDBContext.cs`, `Models/Template.cs`, `Infra/UoW/`), not generic ASP.NET Core defaults. + +## Hard Constraints + +These rules must remain unchanged while implementing this feature (verified: zero violations exist in the current codebase): + +- Never call `uow.SaveChangesAsync()` in repositories or services. +- Only services call `_uow.CommitAsync(ct)`. +- Use `DateTime.Now`, never `DateTime.UtcNow`. +- Do not add extra modules outside the requested user management scope. +- Follow the existing slice pattern exactly: `I{Feature}Repository.cs`, `{Feature}Repository.cs`, `{Feature}DTOs.cs`, `{Feature}Service.cs`, `{Feature}Controller.cs`. There is no generic repository base (`IGenericRepo`) in this codebase — every repository is hand-written per feature, and the new `User`/`Role`/`Permission` repositories must follow the same pattern. +- All new entities inherit `Models.Template` (`Id`, `CreatedAt`, `IsActive`) exactly like every existing entity (`Region`, `Route`, `Vehicle`, `Store`, `Warehouse`, `Order`, `OrderItem`, `DeliveryRun`) — do not redeclare `Id`/`CreatedAt`/`IsActive`. +- Foreign keys follow the majority existing convention: a `FK_{Name}Id` property plus a `[ForeignKey("FK_{Name}Id")]`-annotated navigation property (as in `Order.FK_StoreId`, `DeliveryRun.FK_VehicleId`/`FK_RouteId`). DTOs expose the plain name without the `FK_` prefix (as `OrderResponse.StoreId` already does). + +## ⚠️ Blocking Prerequisite: Territory Hierarchy Is Not Modeled Yet + +This is the single biggest gap between this plan's authorization design and the current schema, and it must be resolved **before** Phase 1, not discovered mid-implementation. + +Verified against `Models/Store.cs`, `Models/Route.cs`, `Models/Region.cs`, `Models/SaleRoute.cs`, `Models/Order.cs`: + +- `Region` has only a `Name`. Nothing points to it. +- `Route` has no link to `Region`. +- `Store` has no link to `Route` or `Region`. +- `Order` links only to `Store` (`FK_StoreId`) — it has no `RouteId`. +- `SaleRoute` is an empty stub (`public class SaleRoute : Template { }`). It is already flagged in `PLAN.md` as "shape is TBD — excluded until clarified" from the original module build. + +**Consequence**: as written, "restrict a Supervisor to their assigned region/route" or "scope orders by route" cannot be implemented — there is no data path from a `User`'s region/route scope down to a `Store` or `Order`. Only store-level scoping (`UserStoreScope` → `Order.FK_StoreId`) is directly wireable today. + +### Recommended resolution + +Add two small FK fields to the **existing** `Route` and `Store` models (not new modules — minimal additions to unblock the hierarchy): + +- `Route.FK_RegionId` (Guid) — a Route belongs to one Region. +- `Store.FK_RouteId` (Guid) — a Store is served by one Route. + +This gives the chain `Region ← Route ← Store ← Order`, so scope checks can resolve a Store or Order's region/route by joining up from `Store.FK_RouteId` and `Route.FK_RegionId`. It also naturally resolves what `SaleRoute` was probably meant to represent (a store's assigned route), so `SaleRoute` can either be repurposed as this Store↔Route link or left unused and removed later — **do not silently redefine `SaleRoute`'s shape without confirming with the user**, since it was explicitly deferred in the prior plan. + +This must be confirmed before Phase 1 (see Open Decisions, item 0). If declined, the first release should be scoped to **store-level authorization only**, and region/route scoping deferred until the hierarchy exists. + +## Recommended Authorization Model + +Do not model access using role names alone. + +Use three separate concerns: + +1. Authentication +2. Permissions +3. Data scope + +That means: + +- `Role` decides what actions a user can perform. +- `Permission` defines each action explicitly. +- `Scope` defines which regions, routes, or stores a user can access. +- `User.FK_ReportsToUserId` defines organizational hierarchy. + +## Business Hierarchy + +Bottom-to-top hierarchy: + +1. SalesRep +2. Supervisor +3. AreaManager +4. AreaSupervisor +5. RegionalManager +6. RegionalAgency + +Store this in code and data using a stable level value. + +Suggested role levels: + +| Role | Level | +|---|---:| +| SalesRep | 10 | +| Supervisor | 20 | +| AreaManager | 30 | +| AreaSupervisor | 40 | +| RegionalManager | 50 | +| RegionalAgency | 60 | + +## Why Role Alone Is Not Enough + +Two supervisors may both have the same permissions, but one should only access stores and orders under their own territory. + +Because of that, authorization must be: + +- permission-based +- scope-aware +- hierarchy-aware where needed + +## Target Project Structure + +Keep the existing slice pattern and add only the new feature slices required (matches the real layout, not the stale structure described in `AGENTS.md`): + +```text +Distribution/ +├── Features/ +│ ├── Region/ Route/ Vehicle/ Store/ Warehouse/ Order/ (existing) +│ ├── Auth/ (new) +│ ├── User/ (new) +│ └── Role/ (new) +├── Models/ +├── Data/ +│ └── AppDBContext.cs +├── Infra/ +│ └── UoW/ +└── Program.cs +``` + +## New Domain Models + +Add the following entities under `Models/`. All inherit `Template` (`Id`, `CreatedAt`, `IsActive`) — do not add separate `Id`/`CreatedAt`/`IsActive` fields. + +### 1. User + +Purpose: application user account. + +```csharp +public class User : Template +{ + public string Username { get; set; } + public string PasswordHash { get; set; } + public string FullName { get; set; } + + public Guid FK_RoleId { get; set; } + [ForeignKey("FK_RoleId")] + public Role Role { get; set; } + + public Guid? FK_ReportsToUserId { get; set; } + [ForeignKey("FK_ReportsToUserId")] + public User ReportsTo { get; set; } + + public DateTime? UpdatedAt { get; set; } +} +``` + +Notes: +- `FK_ReportsToUserId` is nullable — the top of the hierarchy (`RegionalAgency`) reports to no one. +- `UpdatedAt` has no precedent in `Template` (no existing entity tracks it) — this is a deliberate, scoped addition for audit purposes on `User` only, not a change to `Template` itself. Flagging it explicitly rather than adding it silently. +- Configure `FK_ReportsToUserId` with `OnDelete(DeleteBehavior.Restrict)` in `OnModelCreating` — a self-referencing FK with cascading delete will fail (or silently misbehave) once combined with the other FK relationships being added in this feature. + +### 2. Role + +```csharp +public class Role : Template +{ + public string Name { get; set; } + public int Level { get; set; } +} +``` + +(`IsActive` comes from `Template`; no separate field needed.) + +### 3. Permission + +```csharp +public class Permission : Template +{ + public string Code { get; set; } + public string Description { get; set; } +} +``` + +### 4. RolePermission + +Join entity, same pattern as `OrderItem` (inherits `Template`, composite key configured via Fluent API): + +```csharp +public class RolePermission : Template +{ + public Guid FK_RoleId { get; set; } + [ForeignKey("FK_RoleId")] + public Role Role { get; set; } + + public Guid FK_PermissionId { get; set; } + [ForeignKey("FK_PermissionId")] + public Permission Permission { get; set; } +} +``` + +### 5. UserRegionScope / 6. UserRouteScope / 7. UserStoreScope + +Same shape, one per scope type: + +```csharp +public class UserStoreScope : Template +{ + public Guid FK_UserId { get; set; } + [ForeignKey("FK_UserId")] + public User User { get; set; } + + public Guid FK_StoreId { get; set; } + [ForeignKey("FK_StoreId")] + public Store Store { get; set; } +} +``` + +(Mirror for `UserRegionScope` → `Region`, `UserRouteScope` → `Route`.) + +## DbContext Changes + +Update `Data/AppDBContext.cs` with new `DbSet<>` declarations, following the existing style exactly: + +```csharp +public DbSet Users { get; set; } +public DbSet Roles { get; set; } +public DbSet Permissions { get; set; } +public DbSet RolePermissions { get; set; } +public DbSet UserRegionScopes { get; set; } +public DbSet UserRouteScopes { get; set; } +public DbSet UserStoreScopes { get; set; } +``` + +Extend the existing `OnModelCreating` (currently only configures `OrderItem`'s composite key) with the same pattern for each join table, plus the self-referencing FK behavior: + +```csharp +protected override void OnModelCreating(ModelBuilder modelBuilder) +{ + modelBuilder.Entity() + .HasKey(oi => new { oi.FK_OrderId, oi.FK_ItemId }); + + modelBuilder.Entity() + .HasKey(rp => new { rp.FK_RoleId, rp.FK_PermissionId }); + + modelBuilder.Entity() + .HasKey(s => new { s.FK_UserId, s.FK_RegionId }); + + modelBuilder.Entity() + .HasKey(s => new { s.FK_UserId, s.FK_RouteId }); + + modelBuilder.Entity() + .HasKey(s => new { s.FK_UserId, s.FK_StoreId }); + + modelBuilder.Entity() + .HasOne(u => u.ReportsTo) + .WithMany() + .HasForeignKey(u => u.FK_ReportsToUserId) + .OnDelete(DeleteBehavior.Restrict); +} +``` + +If the territory hierarchy prerequisite above is accepted, also add `Route.FK_RegionId` and `Store.FK_RouteId` FK configuration here (or let EF's convention-based FK discovery handle them, consistent with how `Order.FK_StoreId` is currently handled with zero explicit Fluent config). + +## Authentication Strategy + +Use JWT bearer authentication. + +**Package required**: `Microsoft.AspNetCore.Authentication.JwtBearer` is not currently installed (`Distribution.csproj` only has `Microsoft.AspNetCore.OpenApi`, EF Core, and the MySQL provider) — add it as part of Phase 5. + +**Pipeline gap**: `Program.cs` currently calls `app.UseAuthorization()` (line 47) with no `app.UseAuthentication()` before it and no `AddAuthentication(...)` registered in the service container — the authorization middleware is effectively a no-op today. Phase 5 must add both `builder.Services.AddAuthentication(...).AddJwtBearer(...)` and `app.UseAuthentication()` (before `UseAuthorization()`), not just policies. + +Recommended token claims: + +- `sub` or user id +- `username` +- `role` +- `role_level` + +Do not put full permission lists or all scope data into the token initially. Keep permission and scope validation server-side. + +## Permission Model + +Define permissions per feature action. + +Suggested initial permission set: + +### Region + +- `regions.view` +- `regions.create` +- `regions.update` +- `regions.delete` + +### Route + +- `routes.view` +- `routes.create` +- `routes.update` +- `routes.delete` + +### Vehicle + +- `vehicles.view` +- `vehicles.create` +- `vehicles.update` +- `vehicles.delete` + +### Store + +- `stores.view` +- `stores.create` +- `stores.update` +- `stores.delete` + +### Warehouse + +- `warehouses.view` +- `warehouses.create` +- `warehouses.update` +- `warehouses.delete` + +### Order + +- `orders.view` +- `orders.create` +- `orders.update` +- `orders.delete` + +### User Management + +- `users.view` +- `users.create` +- `users.update` +- `users.deactivate` +- `users.assign_role` +- `users.assign_scope` + +## Suggested Role-to-Permission Mapping + +This is a starting point and can be refined later. + +### SalesRep + +- `orders.view` +- `orders.create` +- `orders.update` +- `stores.view` +- `routes.view` + +### Supervisor + +- all SalesRep permissions +- `orders.delete` +- `stores.create` +- `stores.update` + +### AreaManager + +- all Supervisor permissions +- `vehicles.view` +- `warehouses.view` + +### AreaSupervisor + +- all AreaManager permissions +- `routes.create` +- `routes.update` + +### RegionalManager + +- full access to orders, stores, routes, vehicles, warehouses +- `regions.view` +- `users.view` + +### RegionalAgency + +- full access to all implemented business modules +- full user management permissions + +## Scope Rules + +Permissions are not enough. Apply scope checks in services. + +Recommended scope expectations: + +- `SalesRep` + - limited to assigned stores (route-based scoping requires the territory hierarchy prerequisite above) +- `Supervisor` + - limited to assigned stores and subordinate users; route/region scoping requires the prerequisite +- `AreaManager` / `AreaSupervisor` + - limited to assigned route or area scope — **requires the territory hierarchy prerequisite**; not implementable against the current schema without it +- `RegionalManager` + - limited to assigned regions — **requires the territory hierarchy prerequisite** +- `RegionalAgency` + - global or region-wide based on business decision + +If the prerequisite is deferred, ship `UserStoreScope` first (fully supported today via `Order.FK_StoreId`) and treat `UserRegionScope`/`UserRouteScope` as scaffolded-but-unenforced until `Route.FK_RegionId`/`Store.FK_RouteId` exist. + +## Authorization Behavior for Existing Features + +The current API should eventually enforce both permission and scope. + +### Orders + +- `GET /api/orders` + - return only orders inside the user's store scope (route/region scope needs the prerequisite) +- `GET /api/orders/{id}` + - allow only if the order's store is in scope +- `GET /api/orders/store/{storeId}` + - allow only if `storeId` is in scope +- `POST /api/orders` + - allow only if the target store is in scope +- `PUT /api/orders/{id}` + - allow only if the order's store is in scope +- `DELETE /api/orders/{id}` + - allow only if the order's store is in scope + +### Stores + +- restrict reads and updates to assigned stores directly (`UserStoreScope`); territory-based restriction (by route/region) needs the prerequisite + +### Routes + +- restrict reads and updates to assigned route scope — needs the prerequisite (no user-facing entity currently references a Route except `DeliveryRun`) + +### Regions + +- restrict non-global roles to assigned regions only — needs the prerequisite + +### Warehouses and Vehicles + +- restrict by linked route or regional business rules once those relationships are finalized (already correctly hedged in the original plan — `Warehouse` links only to `Vehicle`, `Vehicle` has no route/region link) + +## Implementation Sequence + +Follow this order to reduce rework. + +### Phase 0: Territory Hierarchy Prerequisite (confirm first) + +1. Confirm with the user whether `Route.FK_RegionId` and `Store.FK_RouteId` should be added (see blocking prerequisite section above). +2. If confirmed, add the two FK fields to the existing `Route`/`Store` models and a migration, before touching user-management code. +3. Resolve or explicitly defer `SaleRoute`'s undefined shape rather than leaving it ambiguous. +4. If declined, proceed with Phase 1 scoped to store-level authorization only. + +### Phase 1: Domain and Data Model + +1. Add user, role, permission, and scope models (all inheriting `Template`). +2. Update `AppDBContext` (DbSets + `OnModelCreating` composite keys + `ReportsTo` self-FK `Restrict` behavior). +3. Create migration. + +### Phase 2: Auth Feature + +Add `Features/Auth/`, following the same file split as every other feature: + +- `AuthDTOs.cs` +- `IAuthRepository` only if needed beyond `IUserRepository` (prefer reusing `IUserRepository` for the username lookup rather than a new interface, to avoid an unnecessary extra abstraction) +- `AuthService.cs` +- `AuthController.cs` + +Recommended endpoints: + +- `POST /api/auth/login` +- `POST /api/auth/refresh` only if refresh tokens are requested later + +Phase 2 scope should at minimum support login with JWT issuance. + +### Phase 3: Role and Permission Seed Data + +Seed: + +- all six business roles +- the initial permission set +- role-permission mappings + +Use EF model seeding (`HasData` in `OnModelCreating`) or a small startup seeding step — keep it simple, consistent with how the rest of this codebase avoids extra abstraction layers. + +### Phase 4: User Feature + +Add `Features/User/`: + +- `UserDTOs.cs` +- `IUserRepository.cs` +- `UserRepository.cs` +- `UserService.cs` +- `UserController.cs` + +Recommended initial endpoints: + +- `GET /api/users` +- `GET /api/users/{id}` +- `POST /api/users` +- `PUT /api/users/{id}` +- `PUT /api/users/{id}/role` +- `PUT /api/users/{id}/scope` +- `DELETE /api/users/{id}` as soft deactivate (set `IsActive = false`, matching how every other feature already soft-filters on `IsActive` — see `OrderService.GetAllAsync`/`GetByIdAsync`) + +Mutating endpoints follow the existing `BeginAsync` / `CommitAsync` / `RollbackAsync` try/catch pattern used in `OrderService` — not a bare `CommitAsync` call. + +### Phase 5: Authorization Infrastructure + +Add: + +- `Microsoft.AspNetCore.Authentication.JwtBearer` package reference +- JWT authentication configuration in `Program.cs` (`AddAuthentication` + `AddJwtBearer`, plus `app.UseAuthentication()` before `app.UseAuthorization()` — both currently absent) +- authorization policies +- a permission-checking service or handler +- a current-user accessor abstraction + +Recommended abstractions: + +- `ICurrentUserService` +- `IPermissionService` +- `IScopeAccessService` + +Register everything in `Program.cs` the same way existing features are registered — one `AddScoped()` per repository and one `AddScoped()` per service (services by concrete type, not behind an interface, matching current `Program.cs`). + +### Phase 6: Protect Existing Endpoints + +Apply authorization incrementally: + +1. Auth endpoints +2. User management endpoints +3. Orders +4. Stores +5. Routes +6. Warehouses and Vehicles +7. Regions + +Do not protect everything in one large change. Make authorization testable and reviewable per module. + +## Repository and Service Rules + +Keep the existing architectural rule: + +- Controllers remain thin (inject the service only, no logic — see `RegionController`) +- Services hold business logic +- Repositories perform data access only, no generic base — one hand-written interface + implementation per feature +- Only services call `_uow.CommitAsync(ct)`, wrapped in `BeginAsync`/try-catch/`RollbackAsync` for any multi-step write (see `OrderService.CreateAsync`) + +Permission and scope validation should happen in services, even if controller policies also exist. + +## DTO Guidance + +Do not expose entities directly. DTOs are `record` types, matching every existing feature (see `OrderDTOs.cs`). + +### Auth DTOs + +- `LoginRequest` +- `LoginResponse` + +### User DTOs + +- `CreateUserRequest` +- `UpdateUserRequest` +- `AssignUserRoleRequest` +- `AssignUserScopeRequest` +- `UserResponse` + +### Role DTOs + +- `RoleResponse` +- `RolePermissionResponse` + +## Password Handling + +Do not store plaintext passwords. + +Use: + +- password hash only +- a standard .NET password hasher implementation + +Recommended: + +- `Microsoft.AspNetCore.Identity.PasswordHasher` — this only requires the `Microsoft.AspNetCore.Identity` package for the hasher utility, not full ASP.NET Core Identity (no `IdentityDbContext`, no Identity tables). Confirm this distinction explicitly since "Identity" is easy to over-adopt by accident. + +There is no need to adopt full ASP.NET Core Identity unless that is explicitly requested later. + +## Recommended Minimal First Release + +To avoid overbuilding, the first implementation should include only: + +1. user entity and role entity +2. permission entity and role-permission map +3. login endpoint with JWT +4. create user and assign role +5. store scope assignment (route/region scope assignment only if Phase 0 is accepted) +6. permission checks on orders and stores first + +This is enough to introduce secure access control without trying to solve every module at once. + +## Open Decisions To Confirm Before Coding + +These should be confirmed before implementation starts: + +0. **Blocking**: Accept adding `Route.FK_RegionId` and `Store.FK_RouteId` to unlock region/route-based scoping (see prerequisite section), or ship store-only scoping for the first release and defer the territory hierarchy? +1. Should `RegionalAgency` have global access, or only access to assigned regions? +2. Should `SalesRep` scope be store-based, route-based, or both? (Route-based depends on decision 0.) +3. Should supervisors see only direct reports, or all users below them in the hierarchy tree? +4. Do you want user creation to include password setup immediately, or use a reset flow later? +5. Do you want refresh tokens in the first version, or JWT login only? + +## Final Recommendation + +For this project, the safest and most maintainable mapping is: + +- hierarchy in `User.FK_ReportsToUserId` +- business power in `Role` +- action control in `Permission` +- data visibility in user scope tables + +That model fits the current distribution modules and will scale better than hardcoding behavior by role name alone — but region/route-level scoping is only as real as the territory hierarchy data model backing it, which does not exist yet. Resolving Open Decision 0 first is what makes the rest of this plan actually buildable rather than aspirational.