codex/api-application-access #52

Merged
Twyco merged 8 commits from codex/api-application-access into main 2026-04-24 08:32:36 +00:00
3 changed files with 91 additions and 2 deletions
Showing only changes of commit 3d3b03ee82 - Show all commits
@@ -38,6 +38,7 @@ class BuildProjectTicketOptionsPayload
'ticketTypes' => TicketTypeResource::collection($project->ticketTypes()->get())->resolve(),
'users' => UserResource::collection(
User::query()
Twyco marked this conversation as resolved
Review

Should fix: this options payload currently returns all users in the system. That bypasses the visible-user scoping added elsewhere in this PR (User::visibleToProject) and leaks unrelated account identities via the create-ticket API. Please scope this query to users who can read the project (and add a focused /ticket-options API test for this behavior).

Should fix: this options payload currently returns all users in the system. That bypasses the visible-user scoping added elsewhere in this PR (`User::visibleToProject`) and leaks unrelated account identities via the create-ticket API. Please scope this query to users who can read the project (and add a focused `/ticket-options` API test for this behavior).
->visibleToProject($project)
->orderBy('name')
->get(),
)->resolve(),
@@ -123,7 +123,8 @@ class ProjectTicketController extends ApiController
? "{$ticket->ticket_key} was updated."
: 'No ticket changes were detected.';
return TicketResource::make($ticket->fresh()->load([
$canManageProject = $this->permissionService->allows($user, Permission::ProjectAdmin, $project);
$displayTicket = $ticket->fresh([
'assignee',
Twyco marked this conversation as resolved
Review

Should fix: show_author is enforced in the ticket detail payload, but this update response always loads/serializes author. A non-admin with write access can send a PATCH (including a no-op update) and recover hidden author data. Please apply the same visibility rule here (omit/null author unless the caller can manage the project) so the setting cannot be bypassed.

Should fix: `show_author` is enforced in the ticket detail payload, but this update response always loads/serializes `author`. A non-admin with write access can send a PATCH (including a no-op update) and recover hidden author data. Please apply the same visibility rule here (omit/null author unless the caller can manage the project) so the setting cannot be bypassed.
'author',
'fieldValues.ticketFieldDefinition',
@@ -132,7 +133,23 @@ class ProjectTicketController extends ApiController
'ticketStatus.boardColumn',
'ticketType',
'worklogEntries.actor',
]))
]) ?? $ticket->load([
'assignee',
'author',
'fieldValues.ticketFieldDefinition',
'fieldValues.userValue',
'ticketPriority',
'ticketStatus.boardColumn',
'ticketType',
'worklogEntries.actor',
]);
if (! $project->show_author && ! $canManageProject) {
$displayTicket->setRelation('author', null);
$displayTicket->author_user_id = null;
}
return TicketResource::make($displayTicket)
->additional([
'meta' => [
'message' => $statusMessage,
+71
View File
@@ -81,6 +81,77 @@ class TicketApiTest extends TestCase
->assertJsonPath('data.ticket.story_points', 5);
}
public function test_ticket_update_hides_author_for_non_admins_when_author_visibility_is_disabled(): void
{
$writer = User::factory()->create();
$author = User::factory()->create([
'name' => 'Author User',
]);
$project = Project::factory()->create([
'key' => 'CORE',
'show_author' => false,
]);
app(InitializeProjectTicketMetadata::class)->handle($project);
$priority = $project->ticketPriorities()->firstOrFail();
$status = $project->ticketStatuses()->firstOrFail();
$type = $project->ticketTypes()->where('is_parent', false)->firstOrFail();
$ticket = Ticket::factory()->create([
'author_user_id' => $author->id,
'project_id' => $project->id,
'sequence_number' => 1,
'ticket_key' => 'CORE-1',
'ticket_priority_id' => $priority->id,
'ticket_status_id' => $status->id,
'ticket_type_id' => $type->id,
'title' => 'API ticket',
]);
$this->syncProjectPermissions($project, $writer, ['write', 'view']);
$this->actingAs($writer)
->patchJson(route('api.projects.tickets.update', [$project, $ticket]), [
'acceptance_criteria' => $ticket->acceptance_criteria,
'blocked_comment' => $ticket->blocked_comment,
'description' => $ticket->description,
'is_blocked' => $ticket->is_blocked,
'parent_ticket_id' => $ticket->parent_ticket_id,
'story_points' => $ticket->story_points,
'ticket_priority_id' => $priority->id,
'ticket_type_id' => $type->id,
'title' => 'Updated API ticket',
])
->assertOk()
->assertJsonPath('data.author', null)
->assertJsonPath('data.author_user_id', null);
}
public function test_ticket_options_only_return_project_visible_users(): void
{
[$user, $project] = $this->projectContext();
$projectViewer = User::factory()->create();
$outsider = User::factory()->create();
$this->syncProjectPermissions($project, $projectViewer, ['view']);
$response = $this->actingAs($user)
->getJson(route('api.projects.ticket-options', [
'project' => $project,
'source' => 'backlog',
]))
->assertOk();
$userIds = collect($response->json('data.users'))
->pluck('id')
->all();
$this->assertContains($user->id, $userIds);
$this->assertContains($projectViewer->id, $userIds);
$this->assertNotContains($outsider->id, $userIds);
}
public function test_tickets_can_return_status_targets_and_move_statuses(): void
{
[$user, $project] = $this->projectContext();