Compare commits

..

2 Commits

Author SHA1 Message Date
Constantin Graf
c791057f25 Allow only one running time entry per user across organizations
The running time entry check in the store endpoint was scoped to the
member, so a user could start a time entry in each of their
organizations. The active time entry endpoint and the database
consistency check expect only one running time entry per user.
2026-10-09 12:58:15 +02:00
Constantin Graf
4a4acb2214 Lock creation of running time entries to prevent race condition
Concurrent requests could both pass the running time entry check and
create more than one running time entry for the same member.
2026-10-09 12:58:06 +02:00
4 changed files with 96 additions and 8 deletions

View File

@@ -5,7 +5,7 @@ permissions:
jobs:
phpunit-extensions:
runs-on: ubuntu-latest
timeout-minutes: 25
timeout-minutes: 15
strategy:
matrix:
postgres_version: [ 15, 16, 17 ]
@@ -132,8 +132,5 @@ jobs:
php artisan key:generate
php artisan passport:keys
- name: "Run PHPUnit (extensions)"
- name: "Run PHPUnit"
run: php artisan test extensions/Billing/tests extensions/Services/tests extensions/Invoicing/tests --stop-on-failure
- name: "Run PHPUnit (core)"
run: php artisan test --testsuite=Unit,Feature --stop-on-failure

View File

@@ -51,6 +51,7 @@ use Illuminate\Support\Carbon;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Blade;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Storage;
@@ -613,7 +614,24 @@ class TimeEntryController extends Controller
$this->checkPermission($organization, 'time-entries:create:all');
}
if ($request->input('end') === null && TimeEntry::query()->whereBelongsTo($member, 'member')->where('end', null)->exists()) {
// Lock the creation of running time entries per user, so that concurrent requests can not create more than one running time entry
$lock = $request->input('end') === null ? Cache::lock('time-entries:running:'.$member->user_id, 10) : null;
$lock?->block(5);
try {
return $this->storeTimeEntry($organization, $member, $request);
} finally {
$lock?->release();
}
}
/**
* @throws TimeEntryStillRunningApiException
*/
private function storeTimeEntry(Organization $organization, Member $member, TimeEntryStoreRequest $request): JsonResource
{
// A user can only have one running time entry, across all organizations
if ($request->input('end') === null && TimeEntry::query()->where('user_id', $member->user_id)->whereNull('end')->exists()) {
throw new TimeEntryStillRunningApiException;
}

View File

@@ -9,10 +9,10 @@
},
"Invoicing": {
"repository": "solidtime-io/extension-invoicing",
"ref": "v0.0.10"
"ref": "v0.0.9"
},
"Auditing": {
"repository": "solidtime-io/extension-auditing",
"ref": "v0.0.5"
"ref": "v0.0.4"
}
}

View File

@@ -24,7 +24,9 @@ use App\Models\Task;
use App\Models\TimeEntry;
use App\Models\User;
use App\Service\TimeEntryFilter;
use Illuminate\Contracts\Cache\LockTimeoutException;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
@@ -2444,6 +2446,77 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
$response->assertJsonPath('error', true);
}
public function test_store_endpoint_fails_if_user_already_has_active_time_entry_in_another_organization(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:create:own',
]);
$otherOrganization = Organization::factory()->create();
$otherMember = Member::factory()->forOrganization($otherOrganization)->forUser($data->user)->create();
TimeEntry::factory()->forOrganization($otherOrganization)->forMember($otherMember)->active()->create();
Passport::actingAs($data->user);
// Act
$response = $this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [
'billable' => true,
'start' => Carbon::now()->toIso8601ZuluString(),
'end' => null,
'member_id' => $data->member->getKey(),
]);
// Assert
$response->assertStatus(400);
$response->assertJsonPath('key', 'time_entry_still_running');
$this->assertSame(0, TimeEntry::query()->whereBelongsTo($data->organization, 'organization')->count());
}
public function test_store_endpoint_releases_running_time_entry_lock_after_request(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:create:own',
]);
Passport::actingAs($data->user);
// Act
$response = $this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [
'billable' => true,
'start' => Carbon::now()->toIso8601ZuluString(),
'end' => null,
'member_id' => $data->member->getKey(),
]);
// Assert
$response->assertStatus(201);
$this->assertTrue(Cache::lock('time-entries:running:'.$data->user->getKey(), 10)->get());
}
public function test_store_endpoint_waits_for_running_time_entry_lock_and_fails_if_it_is_not_released(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:create:own',
]);
Cache::lock('time-entries:running:'.$data->user->getKey(), 10)->get();
$this->withoutExceptionHandling();
Passport::actingAs($data->user);
// Act
try {
$this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [
'billable' => true,
'start' => Carbon::now()->toIso8601ZuluString(),
'end' => null,
'member_id' => $data->member->getKey(),
]);
$this->fail('Expected LockTimeoutException');
} catch (LockTimeoutException) {
// Assert
$this->assertSame(0, TimeEntry::query()->count());
}
}
public function test_store_endpoint_validation_fails_if_task_id_does_not_belong_to_project_id(): void
{
// Arrange