Skip to content

Bug: Tenant ID hardcoded in SecretController::store() - needs proper resolution #190

@kevalyq

Description

@kevalyq

📌 Context

In SecretController::store(), the tenant_id is currently hardcoded to use the first available TenantKey:

// TODO: Replace with TenantMiddleware that injects tenant_id into request
// For now, use first available tenant (testing only - NOT production-ready)
$tenantId = \App\Models\TenantKey::first()?->id;
if (! $tenantId) {
    return response()->json([
        'error' => 'Tenant resolution not yet implemented. Please contact system administrator.',
    ], Response::HTTP_SERVICE_UNAVAILABLE);
}

Source: app/Http/Controllers/Api/V1/SecretController.php:155

This is NOT production-ready and violates multi-tenant isolation.


🎯 Goal

Implement proper tenant resolution via middleware that:

  1. Extracts tenant_id from authenticated user context
  2. Injects tenant_id into request
  3. Enforces tenant isolation globally

📋 Implementation Options

Option 1: Middleware Injection (Recommended)

Create: app/Http/Middleware/InjectTenantId.php

class InjectTenantId
{
    public function handle(Request $request, Closure $next): Response
    {
        if ($user = $request->user()) {
            // Assuming User has tenant relationship
            $request->merge(['tenant_id' => $user->tenant_id]);
        }
        
        return $next($request);
    }
}

Register in bootstrap/app.php:

->withMiddleware(function (Middleware $middleware) {
    $middleware->alias([
        'tenant.inject' => \App\Http\Middleware\InjectTenantId::class,
    ]);
})

Use in routes:

Route::middleware(['auth:sanctum', 'tenant.enforce', 'tenant.inject'])->group(function () {
    Route::apiResource('secrets', SecretController::class);
});

Update Controller:

public function store(StoreSecretRequest $request): JsonResponse
{
    $this->authorize('create', Secret::class);

    /** @var \App\Models\User $user */
    $user = $request->user();
    
    $secret = new Secret;
    $secret->tenant_id = $request->input('tenant_id'); // From middleware
    $secret->owner_id = $user->id;
    $secret->version = 1;

    $this->assignFields($secret, $request);
    $secret->save();

    return response()->json([
        'data' => $this->transformSecret($secret),
    ], Response::HTTP_CREATED);
}

Option 2: User Relationship (If User has tenant_id)

If User model already has tenant_id column:

$secret->tenant_id = $user->tenant_id;

Requires validation: User MUST have tenant_id set.


Option 3: TenantKey Resolver Service

Create a TenantResolver service that determines tenant_id based on:

  • User's active tenant
  • Subdomain (if multi-tenant via subdomain)
  • JWT claim

✅ Acceptance Criteria

  • Tenant resolution implemented (choose option)
  • TODO comment removed from SecretController::store()
  • Hardcoded TenantKey::first() removed
  • All existing tests passing
  • New tests for tenant isolation
  • PHPStan level max passing
  • CHANGELOG.md updated

🔗 Related Issues


🧪 Testing

Scenario 1: User from Tenant A cannot create secret with Tenant B's tenant_id
Scenario 2: User without tenant_id receives clear error message
Scenario 3: Middleware correctly injects tenant_id from user context


📝 Notes

  • Current implementation works for single-tenant development only
  • Must be fixed before multi-tenant production deployment
  • Consider adding tenant_id validation in StoreSecretRequest

Type: Bug (Technical Debt)
Priority: Medium
Estimated Effort: 2-3 hours

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    ✅ Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions