fix: add idempotency check to role assignment API

app/Http/Controllers/RoleController.php

@@ -52,7 +52,31 @@ public function store(AssignRoleRequest $request, int $user): JsonResponse
         /** @var int|null $tenantId */
         $tenantId = app(\Spatie\Permission\PermissionRegistrar::class)->getPermissionsTeamId();
 
-        DB::transaction(function () use ($targetUser, $role, $validFrom, $validUntil, $request, $tenantId, $authUser) {
+        // Idempotency check: Return 200 OK if role already assigned
+        // This prevents DB unique constraint violations on duplicate assignments
+        /** @var object{valid_from: ?string, valid_until: ?string, auto_revoke: bool, reason: ?string}|null $existingAssignment */
+        $existingAssignment = DB::table('model_has_roles')
+            ->where('model_type', get_class($targetUser))
+            ->where('model_id', $targetUser->id)
+            ->where('role_id', $role->id)
+            ->where('tenant_id', $tenantId)
+            ->first();
+
+        if ($existingAssignment) {
+            return response()->json([
+                'message' => 'Role already assigned to user',
+                'user_id' => $targetUser->id,
+                'role' => $role->name,
+                'valid_from' => $existingAssignment->valid_from
+                    ? Carbon::parse($existingAssignment->valid_from)->toIso8601String()
+                    : null,
+                'valid_until' => $existingAssignment->valid_until
+                    ? Carbon::parse($existingAssignment->valid_until)->toIso8601String()
+                    : null,
+                'auto_revoke' => $existingAssignment->auto_revoke,
+                'reason' => $existingAssignment->reason ?? '',
+            ], Response::HTTP_OK); // 200 OK - Idempotent operation
+        }        DB::transaction(function () use ($targetUser, $role, $validFrom, $validUntil, $request, $tenantId, $authUser) {
             // Direct database insert to bypass Spatie's relationship methods:
             // We require additional temporal and audit fields (valid_from, valid_until, auto_revoke, assigned_by, reason)
             // in the model_has_roles table, which are not supported by Spatie's built-in relationship methods.

tests/Feature/Integration/RbacIntegrationTest.php

@@ -270,16 +270,52 @@
             ])
             ->assertSuccessful();
 
-        // Assign same role again - Phase 3 API does not handle idempotency
-        // This is a known limitation - duplicate assignment causes DB constraint violation
-        // Skip this assertion for now
-        // actingAs($this->admin)
-        //     ->postJson("/api/v1/users/{$user->id}/roles", [
-        //         'role' => 'Manager',
-        //     ])
-        //     ->assertSuccessful();
-
-        // User should have role
+        // Assign same role again - should be idempotent (return 200 OK)
+        actingAs($this->admin)
+            ->postJson("/api/v1/users/{$user->id}/roles", [
+                'role' => 'Manager',
+            ])
+            ->assertOk() // 200 OK (not 201 Created)
+            ->assertJson([
+                'message' => 'Role already assigned to user',
+                'role' => 'Manager',
+            ]);
+
+        // User should still have exactly 1 role (not duplicate)
         expect($user->fresh()->roles)->toHaveCount(1);
-    })->skip('Phase 3 API does not handle idempotent role assignment');
+    });
+
+    test('idempotency returns existing role with different temporal parameters', function (): void {
+        $user = User::factory()->create();
+
+        // Assign permanent role (without temporal constraints)
+        actingAs($this->admin)
+            ->postJson("/api/v1/users/{$user->id}/roles", [
+                'role' => 'Manager',
+            ])
+            ->assertCreated();
+
+        // Try to assign same role again with temporal parameters
+        // Should return 200 OK with existing assignment unchanged
+        $response = actingAs($this->admin)
+            ->postJson("/api/v1/users/{$user->id}/roles", [
+                'role' => 'Manager',
+                'valid_from' => now()->toIso8601String(),
+                'valid_until' => now()->addDays(7)->toIso8601String(),
+            ]);
+
+        $response->assertOk()
+            ->assertJson([
+                'message' => 'Role already assigned to user',
+                'role' => 'Manager',
+            ]);
+
+        // Idempotency: Returns existing assignment, does NOT modify it
+        // Note: valid_from is set to now() by default even for "permanent" roles
+        expect($response->json('valid_from'))->not->toBeNull()
+            ->and($response->json('valid_until'))->toBeNull(); // No expiration
+
+        // User should still have exactly 1 role (not duplicate)
+        expect($user->fresh()->roles)->toHaveCount(1);
+    });
 });