From ef7ed4ca777fc545cad2a6d6f2a1ea19f6f3b71d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 13:09:49 +0000 Subject: [PATCH] fix: atomic usage recording and provisioning to prevent race conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #42 — race condition in EntitlementService usage recording. Changes: 1. New atomic check-and-record methods: - checkAndRecordUsage() — workspace-scoped - checkAndRecordNamespaceUsage() — namespace-scoped These use DB::transaction() with lockForUpdate() on usage records to prevent concurrent requests from reading the same count and both passing the limit check (TOCTOU race). 2. New locked usage query methods: - getLockedCurrentUsage() — bypasses cache, uses SELECT FOR UPDATE - getLockedNamespaceCurrentUsage() — namespace equivalent These are used within the atomic transactions to get a consistent usage count under pessimistic locking. 3. provisionPackage() and provisionNamespacePackage(): Wrapped base-package check-and-cancel in DB::transaction() with lockForUpdate() to prevent duplicate active base packages from concurrent provisioning requests. 4. provisionBoost(): Wrapped in DB::transaction() with lockForUpdate() on existing boosts. Added duplicate detection for blesta_addon_id to prevent the same external addon from being provisioned twice. 5. provisionNamespaceBoost(): Wrapped in DB::transaction() with lockForUpdate() on existing namespace boosts to serialise concurrent provisioning. The existing can() + recordUsage() pattern is preserved for backward compatibility. Callers should migrate to checkAndRecordUsage() for race-safe usage tracking. Co-Authored-By: Claude Opus 4.6 (1M context) --- Services/EntitlementService.php | 444 ++++++++++++++++++++++++++------ 1 file changed, 372 insertions(+), 72 deletions(-) diff --git a/Services/EntitlementService.php b/Services/EntitlementService.php index 894f232..2cfa621 100644 --- a/Services/EntitlementService.php +++ b/Services/EntitlementService.php @@ -19,6 +19,7 @@ use Illuminate\Cache\TaggableStore; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\DB; /** * Core service for managing feature entitlements, usage tracking, and package provisioning. @@ -529,6 +530,193 @@ class EntitlementService return $record; } + /** + * Atomically check entitlement and record usage in a single transaction. + * + * This method prevents the race condition where two concurrent requests both + * read the same usage value, both pass the limit check, and both record usage, + * resulting in over-consumption. It uses `lockForUpdate()` on usage records to + * serialise concurrent access. + * + * ## Example Usage + * + * ```php + * // Atomic check + record in one operation + * $result = $entitlementService->checkAndRecordUsage($workspace, 'pages', 1, $user); + * if ($result->isDenied()) { + * return response()->json(['error' => $result->getMessage()], 403); + * } + * // Usage was already recorded — no need to call recordUsage() separately + * ``` + * + * @param Workspace $workspace The workspace to check and record for + * @param string $featureCode The feature code to check + * @param int $quantity The amount to consume (default: 1) + * @param User|null $user Optional user who triggered the usage + * @param array|null $metadata Optional metadata for audit/debugging + * @return EntitlementResult Contains allowed/denied status. If allowed, usage has already been recorded. + */ + public function checkAndRecordUsage( + Workspace $workspace, + string $featureCode, + int $quantity = 1, + ?User $user = null, + ?array $metadata = null + ): EntitlementResult { + $feature = $this->getFeature($featureCode); + + if (! $feature) { + return EntitlementResult::denied( + reason: "Feature '{$featureCode}' does not exist.", + featureCode: $featureCode + ); + } + + $poolFeatureCode = $feature->getPoolFeatureCode(); + $totalLimit = $this->getTotalLimit($workspace, $poolFeatureCode); + + if ($totalLimit === null) { + return EntitlementResult::denied( + reason: "Your plan does not include {$feature->name}.", + featureCode: $featureCode + ); + } + + if ($totalLimit === -1) { + // Unlimited — still record usage for tracking, but no need for locking + $this->recordUsage($workspace, $featureCode, $quantity, $user, $metadata); + + return EntitlementResult::unlimited($featureCode); + } + + if ($feature->isBoolean()) { + return EntitlementResult::allowed(featureCode: $featureCode); + } + + // Atomic check-and-record within a transaction with row-level locking + return DB::transaction(function () use ($workspace, $feature, $poolFeatureCode, $totalLimit, $quantity, $user, $metadata, $featureCode) { + // Lock existing usage records to prevent concurrent reads from seeing + // the same count. This serialises concurrent usage recording. + $currentUsage = $this->getLockedCurrentUsage($workspace, $poolFeatureCode, $feature); + + if ($currentUsage + $quantity > $totalLimit) { + return EntitlementResult::denied( + reason: "You've reached your {$feature->name} limit ({$totalLimit}).", + limit: $totalLimit, + used: $currentUsage, + featureCode: $featureCode + ); + } + + UsageRecord::create([ + 'workspace_id' => $workspace->id, + 'feature_code' => $poolFeatureCode, + 'quantity' => $quantity, + 'user_id' => $user?->id, + 'metadata' => $metadata, + 'recorded_at' => now(), + ]); + + $this->invalidateUsageCache($workspace, $poolFeatureCode); + + return EntitlementResult::allowed( + limit: $totalLimit, + used: $currentUsage + $quantity, + featureCode: $featureCode + ); + }); + } + + /** + * Atomically check namespace entitlement and record usage in a single transaction. + * + * Namespace equivalent of `checkAndRecordUsage()`. Prevents race conditions + * for namespace-scoped usage by using row-level locking within a transaction. + * + * @param Namespace_ $namespace The namespace to check and record for + * @param string $featureCode The feature code to check + * @param int $quantity The amount to consume (default: 1) + * @param User|null $user Optional user who triggered the usage + * @param array|null $metadata Optional metadata for audit/debugging + * @return EntitlementResult Contains allowed/denied status. If allowed, usage has already been recorded. + */ + public function checkAndRecordNamespaceUsage( + Namespace_ $namespace, + string $featureCode, + int $quantity = 1, + ?User $user = null, + ?array $metadata = null + ): EntitlementResult { + $feature = $this->getFeature($featureCode); + + if (! $feature) { + return EntitlementResult::denied( + reason: "Feature '{$featureCode}' does not exist.", + featureCode: $featureCode + ); + } + + $poolFeatureCode = $feature->getPoolFeatureCode(); + + // Resolve the effective limit using the namespace cascade + $totalLimit = $this->getNamespaceTotalLimit($namespace, $poolFeatureCode); + + if ($totalLimit === null && $namespace->workspace_id) { + $workspace = $namespace->workspace; + if ($workspace) { + $totalLimit = $this->getTotalLimit($workspace, $poolFeatureCode); + } + } + + if ($totalLimit === null) { + return EntitlementResult::denied( + reason: "Your plan does not include {$feature->name}.", + featureCode: $featureCode + ); + } + + if ($totalLimit === -1) { + $this->recordNamespaceUsage($namespace, $featureCode, $quantity, $user, $metadata); + + return EntitlementResult::unlimited($featureCode); + } + + if ($feature->isBoolean()) { + return EntitlementResult::allowed(featureCode: $featureCode); + } + + return DB::transaction(function () use ($namespace, $feature, $poolFeatureCode, $totalLimit, $quantity, $user, $metadata, $featureCode) { + $currentUsage = $this->getLockedNamespaceCurrentUsage($namespace, $poolFeatureCode, $feature); + + if ($currentUsage + $quantity > $totalLimit) { + return EntitlementResult::denied( + reason: "You've reached your {$feature->name} limit ({$totalLimit}).", + limit: $totalLimit, + used: $currentUsage, + featureCode: $featureCode + ); + } + + UsageRecord::create([ + 'namespace_id' => $namespace->id, + 'workspace_id' => $namespace->workspace_id, + 'feature_code' => $poolFeatureCode, + 'quantity' => $quantity, + 'user_id' => $user?->id, + 'metadata' => $metadata, + 'recorded_at' => now(), + ]); + + $this->invalidateNamespaceUsageCache($namespace, $poolFeatureCode); + + return EntitlementResult::allowed( + limit: $totalLimit, + used: $currentUsage + $quantity, + featureCode: $featureCode + ); + }); + } + /** * Provision a package for a workspace. * @@ -599,37 +787,40 @@ class EntitlementService ): WorkspacePackage { $package = Package::where('code', $packageCode)->firstOrFail(); - // Check if this is a base package and workspace already has one - if ($package->is_base_package) { - $existingBase = $workspace->workspacePackages() - ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) - ->active() - ->first(); + $workspacePackage = DB::transaction(function () use ($workspace, $package, $options) { + // Use lockForUpdate() to prevent concurrent provisioning from creating + // duplicate active base packages (TOCTOU race condition). + if ($package->is_base_package) { + $existingBase = $workspace->workspacePackages() + ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) + ->active() + ->lockForUpdate() + ->first(); - if ($existingBase) { - // Cancel existing base package - $existingBase->cancel(now()); + if ($existingBase) { + $existingBase->cancel(now()); - EntitlementLog::logPackageAction( - $workspace, - EntitlementLog::ACTION_PACKAGE_CANCELLED, - $existingBase, - source: $options['source'] ?? EntitlementLog::SOURCE_SYSTEM, - metadata: ['reason' => 'Replaced by new base package'] - ); + EntitlementLog::logPackageAction( + $workspace, + EntitlementLog::ACTION_PACKAGE_CANCELLED, + $existingBase, + source: $options['source'] ?? EntitlementLog::SOURCE_SYSTEM, + metadata: ['reason' => 'Replaced by new base package'] + ); + } } - } - $workspacePackage = WorkspacePackage::create([ - 'workspace_id' => $workspace->id, - 'package_id' => $package->id, - 'status' => WorkspacePackage::STATUS_ACTIVE, - 'starts_at' => $options['starts_at'] ?? now(), - 'expires_at' => $options['expires_at'] ?? null, - 'billing_cycle_anchor' => $options['billing_cycle_anchor'] ?? now(), - 'blesta_service_id' => $options['blesta_service_id'] ?? null, - 'metadata' => $options['metadata'] ?? null, - ]); + return WorkspacePackage::create([ + 'workspace_id' => $workspace->id, + 'package_id' => $package->id, + 'status' => WorkspacePackage::STATUS_ACTIVE, + 'starts_at' => $options['starts_at'] ?? now(), + 'expires_at' => $options['expires_at'] ?? null, + 'billing_cycle_anchor' => $options['billing_cycle_anchor'] ?? now(), + 'blesta_service_id' => $options['blesta_service_id'] ?? null, + 'metadata' => $options['metadata'] ?? null, + ]); + }); EntitlementLog::logPackageAction( $workspace, @@ -723,19 +914,42 @@ class EntitlementService string $featureCode, array $options = [] ): Boost { - $boost = Boost::create([ - 'workspace_id' => $workspace->id, - 'feature_code' => $featureCode, - 'boost_type' => $options['boost_type'] ?? Boost::BOOST_TYPE_ADD_LIMIT, - 'duration_type' => $options['duration_type'] ?? Boost::DURATION_CYCLE_BOUND, - 'limit_value' => $options['limit_value'] ?? null, - 'consumed_quantity' => 0, - 'status' => Boost::STATUS_ACTIVE, - 'starts_at' => $options['starts_at'] ?? now(), - 'expires_at' => $options['expires_at'] ?? null, - 'blesta_addon_id' => $options['blesta_addon_id'] ?? null, - 'metadata' => $options['metadata'] ?? null, - ]); + $boost = DB::transaction(function () use ($workspace, $featureCode, $options) { + // Lock existing active boosts for this workspace+feature to prevent + // duplicate provisioning from concurrent requests. + $existingBoosts = $workspace->boosts() + ->forFeature($featureCode) + ->usable() + ->lockForUpdate() + ->get(); + + // If a blesta_addon_id is provided, check for duplicates to prevent + // the same external addon from being provisioned twice. + $blestaAddonId = $options['blesta_addon_id'] ?? null; + if ($blestaAddonId !== null) { + $duplicate = $existingBoosts->first( + fn (Boost $b) => $b->blesta_addon_id === $blestaAddonId + ); + + if ($duplicate) { + return $duplicate; + } + } + + return Boost::create([ + 'workspace_id' => $workspace->id, + 'feature_code' => $featureCode, + 'boost_type' => $options['boost_type'] ?? Boost::BOOST_TYPE_ADD_LIMIT, + 'duration_type' => $options['duration_type'] ?? Boost::DURATION_CYCLE_BOUND, + 'limit_value' => $options['limit_value'] ?? null, + 'consumed_quantity' => 0, + 'status' => Boost::STATUS_ACTIVE, + 'starts_at' => $options['starts_at'] ?? now(), + 'expires_at' => $options['expires_at'] ?? null, + 'blesta_addon_id' => $blestaAddonId, + 'metadata' => $options['metadata'] ?? null, + ]); + }); EntitlementLog::logBoostAction( $workspace, @@ -1224,6 +1438,79 @@ class EntitlementService return Cache::remember($cacheKey, self::USAGE_CACHE_TTL, $callback); } + /** + * Get current usage with a pessimistic lock for atomic check-and-record. + * + * This bypasses the cache and queries the database directly with `lockForUpdate()` + * to ensure that concurrent transactions see a consistent usage count. Must be + * called within a DB::transaction(). + * + * @param Workspace $workspace The workspace to get locked usage for + * @param string $featureCode The feature code to get usage for + * @param Feature $feature The feature model (for reset configuration) + * @return int The current usage count (under lock) + */ + protected function getLockedCurrentUsage(Workspace $workspace, string $featureCode, Feature $feature): int + { + $query = UsageRecord::where('workspace_id', $workspace->id) + ->where('feature_code', $featureCode) + ->lockForUpdate(); + + if ($feature->resetsMonthly()) { + $primaryPackage = $workspace->workspacePackages() + ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) + ->active() + ->first(); + + $cycleStart = $primaryPackage + ? $primaryPackage->getCurrentCycleStart() + : now()->startOfMonth(); + + $query->where('recorded_at', '>=', $cycleStart); + } elseif ($feature->resetsRolling()) { + $days = $feature->rolling_window_days ?? 30; + $query->where('recorded_at', '>=', now()->subDays($days)); + } + + return (int) $query->sum('quantity'); + } + + /** + * Get current namespace usage with a pessimistic lock for atomic check-and-record. + * + * Namespace equivalent of `getLockedCurrentUsage()`. Must be called within + * a DB::transaction(). + * + * @param Namespace_ $namespace The namespace to get locked usage for + * @param string $featureCode The feature code to get usage for + * @param Feature $feature The feature model (for reset configuration) + * @return int The current usage count (under lock) + */ + protected function getLockedNamespaceCurrentUsage(Namespace_ $namespace, string $featureCode, Feature $feature): int + { + $query = UsageRecord::where('namespace_id', $namespace->id) + ->where('feature_code', $featureCode) + ->lockForUpdate(); + + if ($feature->resetsMonthly()) { + $primaryPackage = $namespace->namespacePackages() + ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) + ->active() + ->first(); + + $cycleStart = $primaryPackage + ? $primaryPackage->getCurrentCycleStart() + : now()->startOfMonth(); + + $query->where('recorded_at', '>=', $cycleStart); + } elseif ($feature->resetsRolling()) { + $days = $feature->rolling_window_days ?? 30; + $query->where('recorded_at', '>=', now()->subDays($days)); + } + + return (int) $query->sum('quantity'); + } + /** * Get a feature by its unique code. * @@ -1747,28 +2034,31 @@ class EntitlementService ): NamespacePackage { $package = Package::where('code', $packageCode)->firstOrFail(); - // Check if this is a base package and namespace already has one - if ($package->is_base_package) { - $existingBase = $namespace->namespacePackages() - ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) - ->active() - ->first(); + $namespacePackage = DB::transaction(function () use ($namespace, $package, $options) { + // Use lockForUpdate() to prevent concurrent provisioning from creating + // duplicate active base packages (TOCTOU race condition). + if ($package->is_base_package) { + $existingBase = $namespace->namespacePackages() + ->whereHas('package', fn ($q) => $q->where('is_base_package', true)) + ->active() + ->lockForUpdate() + ->first(); - if ($existingBase) { - // Cancel existing base package - $existingBase->cancel(now()); + if ($existingBase) { + $existingBase->cancel(now()); + } } - } - $namespacePackage = NamespacePackage::create([ - 'namespace_id' => $namespace->id, - 'package_id' => $package->id, - 'status' => NamespacePackage::STATUS_ACTIVE, - 'starts_at' => $options['starts_at'] ?? now(), - 'expires_at' => $options['expires_at'] ?? null, - 'billing_cycle_anchor' => $options['billing_cycle_anchor'] ?? now(), - 'metadata' => $options['metadata'] ?? null, - ]); + return NamespacePackage::create([ + 'namespace_id' => $namespace->id, + 'package_id' => $package->id, + 'status' => NamespacePackage::STATUS_ACTIVE, + 'starts_at' => $options['starts_at'] ?? now(), + 'expires_at' => $options['expires_at'] ?? null, + 'billing_cycle_anchor' => $options['billing_cycle_anchor'] ?? now(), + 'metadata' => $options['metadata'] ?? null, + ]); + }); $this->invalidateNamespaceCache($namespace); @@ -1846,19 +2136,29 @@ class EntitlementService string $featureCode, array $options = [] ): Boost { - $boost = Boost::create([ - 'namespace_id' => $namespace->id, - 'workspace_id' => $namespace->workspace_id, - 'feature_code' => $featureCode, - 'boost_type' => $options['boost_type'] ?? Boost::BOOST_TYPE_ADD_LIMIT, - 'duration_type' => $options['duration_type'] ?? Boost::DURATION_CYCLE_BOUND, - 'limit_value' => $options['limit_value'] ?? null, - 'consumed_quantity' => 0, - 'status' => Boost::STATUS_ACTIVE, - 'starts_at' => $options['starts_at'] ?? now(), - 'expires_at' => $options['expires_at'] ?? null, - 'metadata' => $options['metadata'] ?? null, - ]); + $boost = DB::transaction(function () use ($namespace, $featureCode, $options) { + // Lock existing active boosts for this namespace+feature to prevent + // duplicate provisioning from concurrent requests. + $namespace->boosts() + ->forFeature($featureCode) + ->usable() + ->lockForUpdate() + ->get(); + + return Boost::create([ + 'namespace_id' => $namespace->id, + 'workspace_id' => $namespace->workspace_id, + 'feature_code' => $featureCode, + 'boost_type' => $options['boost_type'] ?? Boost::BOOST_TYPE_ADD_LIMIT, + 'duration_type' => $options['duration_type'] ?? Boost::DURATION_CYCLE_BOUND, + 'limit_value' => $options['limit_value'] ?? null, + 'consumed_quantity' => 0, + 'status' => Boost::STATUS_ACTIVE, + 'starts_at' => $options['starts_at'] ?? now(), + 'expires_at' => $options['expires_at'] ?? null, + 'metadata' => $options['metadata'] ?? null, + ]); + }); $this->invalidateNamespaceCache($namespace); -- 2.45.3