Security: Disabled Authorization + Cross-Tenant IDOR in 15+ Services

[lighthousekeeper1212]

Summary

Critical authorization vulnerabilities affect the entire application:

1. [Authorize] attribute is commented out on BaseController, disabling authentication for all endpoints
2. 15+ services have GetAsync/DeleteAsync methods that don't filter by tenant_id, allowing cross-tenant data access

Finding 1: Authentication Disabled (CRITICAL)

File: backend/ModernWMS.Core/Controller/BaseController.cs, line 13

//[Authorize]  // <-- COMMENTED OUT
[Produces("application/json")]
public class BaseController : ControllerBase

All 25+ controllers inherit from BaseController. With [Authorize] commented out, every API endpoint is accessible without authentication. JWT bearer authentication is properly configured in StartupExtensions.cs but never enforced.

Fix: Uncomment [Authorize] on line 13.

Finding 2: Cross-Tenant IDOR in GetAsync (HIGH)

Affected: 15+ services including Company, User, Warehouse, Customer, Supplier, Stock*, ASN, Freight, Category, PrintSolution, etc.

Pattern: GetAsync(int id) fetches records by ID only, without filtering by tenant_id. Meanwhile, AddAsync() in the same service correctly scopes to currentUser.tenant_id.

Example: backend/ModernWMS.WMS/Services/company/CompanyService.cs

// Line 65 - VULNERABLE: No tenant_id filter
public async Task<CompanyViewModel> GetAsync(int id)
{
    var entity = await _dBContext.GetDbSet<CompanyEntity>()
        .AsNoTracking()
        .FirstOrDefaultAsync(t => t.id.Equals(id));  // Fetches ANY tenant's data
}

// Line 83 - CORRECT: Uses currentUser.tenant_id  
public async Task<(int id, string msg)> AddAsync(CompanyViewModel viewModel, CurrentUser currentUser)
{
    if (await DbSet.AnyAsync(t => t.tenant_id.Equals(currentUser.tenant_id) && ...))
}

Impact: User from Tenant A can read any record from Tenant B by ID.

Finding 3: Cross-Tenant Delete Without Scoping (HIGH)

Example: CompanyService.cs, line 145

public async Task<(bool flag, string msg)> DeleteAsync(int id)
{
    var qty = await _dBContext.GetDbSet<CompanyEntity>()
        .Where(t => t.id.Equals(id))  // No tenant_id check
        .ExecuteDeleteAsync();
}

Impact: User from Tenant A can delete records belonging to Tenant B.

Finding 4: Inconsistent Tenant Filtering in UpdateAsync (HIGH)

UpdateAsync methods fetch records by ID without tenant_id, then partially check tenant_id in uniqueness constraints. The initial fetch is unscoped.

Multi-Tenant Confirmation

This IS a multi-tenant system:

• All entities have tenant_id fields
• JWT tokens include tenant_id
• AddAsync() methods properly scope to currentUser.tenant_id
• The GetAllAsync/PageAsync methods in some services correctly filter by tenant_id

The CRUD inconsistency (Add=scoped, Get/Delete/Update=unscoped) indicates incomplete implementation, not by-design.

Recommendations

1. Immediate: Uncomment [Authorize] on BaseController
2. High Priority: Add tenant_id filtering to all GetAsync(), DeleteAsync(), and UpdateAsync() methods
3. Consider: Adding a global query filter in EF Core for tenant_id scoping