Merge commit from fork

This commit is contained in:
Gregor Vostrak
2026-04-21 21:12:30 +02:00
committed by GitHub
parent 2d6f9e514f
commit b73aa543fd
2 changed files with 43 additions and 2 deletions
@@ -629,9 +629,9 @@ class TimeEntryController extends Controller
/** @var Member|null $member */
$member = $request->has('member_id') ? Member::query()->findOrFail($request->input('member_id')) : null;
if ($timeEntry->member->user_id === Auth::id() && ($member === null || $member->user_id === Auth::id())) {
$this->checkPermission($organization, 'time-entries:update:own');
$this->checkPermission($organization, 'time-entries:update:own', $timeEntry);
} else {
$this->checkPermission($organization, 'time-entries:update:all');
$this->checkPermission($organization, 'time-entries:update:all', $timeEntry);
}
if ($timeEntry->end !== null && $request->has('end') && $request->input('end') === null) {
@@ -2490,6 +2490,47 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
$response->assertForbidden();
}
public function test_update_endpoint_fails_if_time_entry_belongs_to_different_organization_than_url_even_with_update_all_permission(): void
{
// Arrange
// Attacker: has `time-entries:update:all` in their own organization (orgA).
$data = $this->createUserWithPermission([
'time-entries:update:all',
'projects:view:all',
]);
$attackerProject = Project::factory()->forOrganization($data->organization)->create();
// Victim: entirely separate organization (orgB). Attacker has NO membership in orgB.
$victimOrgData = $this->createUserWithPermission([], true);
$victimTimeEntry = TimeEntry::factory()
->forOrganization($victimOrgData->organization)
->forMember($victimOrgData->ownerMember)
->create([
'description' => 'victim-original',
'project_id' => null,
'task_id' => null,
]);
Passport::actingAs($data->user);
// Act: PUT to /organizations/{orgA}/time-entries/{victim_uuid} — URL org is attacker's
// own org, but the route-bound time entry belongs to orgB.
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $victimTimeEntry->getKey()]), [
'description' => 'attacker-overwrite',
'project_id' => $attackerProject->getKey(),
]);
// Assert: must be rejected. Before the fix this returned 200 and rewrote the
// victim row with attacker's project_id while keeping organization_id = orgB.
$response->assertForbidden();
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $victimTimeEntry->getKey(),
'organization_id' => $victimOrgData->organization->getKey(),
'description' => 'victim-original',
'project_id' => null,
]);
}
public function test_update_endpoint_fails_if_user_has_no_permission_to_update_time_entries_for_other_users_in_organization(): void
{
// Arrange