Add user management implementation plan

2026-08-18 15:43:15 +05:30
parent ac3f34e4f3
commit 878847302e
1 changed files with 607 additions and 0 deletions
@@ -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<T>`) 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<User> Users { get; set; }
+public DbSet<Role> Roles { get; set; }
+public DbSet<Permission> Permissions { get; set; }
+public DbSet<RolePermission> RolePermissions { get; set; }
+public DbSet<UserRegionScope> UserRegionScopes { get; set; }
+public DbSet<UserRouteScope> UserRouteScopes { get; set; }
+public DbSet<UserStoreScope> 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<OrderItem>()
+        .HasKey(oi => new { oi.FK_OrderId, oi.FK_ItemId });
+
+    modelBuilder.Entity<RolePermission>()
+        .HasKey(rp => new { rp.FK_RoleId, rp.FK_PermissionId });
+
+    modelBuilder.Entity<UserRegionScope>()
+        .HasKey(s => new { s.FK_UserId, s.FK_RegionId });
+
+    modelBuilder.Entity<UserRouteScope>()
+        .HasKey(s => new { s.FK_UserId, s.FK_RouteId });
+
+    modelBuilder.Entity<UserStoreScope>()
+        .HasKey(s => new { s.FK_UserId, s.FK_StoreId });
+
+    modelBuilder.Entity<User>()
+        .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<IXRepository, XRepository>()` per repository and one `AddScoped<XService>()` 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<TUser>` — 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.