diff --git a/app/Actions/Projects/BuildProjectBacklogPayload.php b/app/Actions/Projects/BuildProjectBacklogPayload.php index eb71b01..bff6315 100644 --- a/app/Actions/Projects/BuildProjectBacklogPayload.php +++ b/app/Actions/Projects/BuildProjectBacklogPayload.php @@ -59,6 +59,7 @@ class BuildProjectBacklogPayload )->resolve(), 'users' => UserResource::collection( User::query() + ->visibleToProject($project) ->orderBy('name') ->get(), )->resolve(), diff --git a/app/Actions/Projects/BuildProjectTicketDetailPayload.php b/app/Actions/Projects/BuildProjectTicketDetailPayload.php index 26b4511..25257cb 100644 --- a/app/Actions/Projects/BuildProjectTicketDetailPayload.php +++ b/app/Actions/Projects/BuildProjectTicketDetailPayload.php @@ -116,6 +116,7 @@ class BuildProjectTicketDetailPayload 'ticketTypes' => TicketTypeResource::collection($project->ticketTypes()->get())->resolve(), 'users' => UserResource::collection( User::query() + ->visibleToProject($project) ->orderBy('name') ->get(), )->resolve(), diff --git a/app/Actions/Projects/BuildProjectTicketOptionsPayload.php b/app/Actions/Projects/BuildProjectTicketOptionsPayload.php index a22aea1..7c6a307 100644 --- a/app/Actions/Projects/BuildProjectTicketOptionsPayload.php +++ b/app/Actions/Projects/BuildProjectTicketOptionsPayload.php @@ -38,6 +38,7 @@ class BuildProjectTicketOptionsPayload 'ticketTypes' => TicketTypeResource::collection($project->ticketTypes()->get())->resolve(), 'users' => UserResource::collection( User::query() + ->visibleToProject($project) ->orderBy('name') ->get(), )->resolve(), diff --git a/app/Http/Controllers/Api/ProjectTicketController.php b/app/Http/Controllers/Api/ProjectTicketController.php index 02409e5..337fda9 100644 --- a/app/Http/Controllers/Api/ProjectTicketController.php +++ b/app/Http/Controllers/Api/ProjectTicketController.php @@ -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', '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, diff --git a/app/Http/Controllers/Api/UserAssignedTicketController.php b/app/Http/Controllers/Api/UserAssignedTicketController.php index c750955..cbd01cb 100644 --- a/app/Http/Controllers/Api/UserAssignedTicketController.php +++ b/app/Http/Controllers/Api/UserAssignedTicketController.php @@ -18,10 +18,14 @@ class UserAssignedTicketController extends ApiController { $viewer = $this->authenticatedUser($request); + $assignedTickets = $this->listAssignedTicketsByVisibleProject->handle($viewer, $user); + return response()->json([ - 'data' => $this->listAssignedTicketsByVisibleProject->handle($viewer, $user), + 'data' => $assignedTickets, 'meta' => [ - 'assignee' => UserResource::make($user)->resolve(), + 'assignee' => $assignedTickets === [] + ? null + : UserResource::make($user)->resolve(), ], ]); } diff --git a/app/Models/User.php b/app/Models/User.php index a1cc99d..b3dc0ba 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -3,9 +3,12 @@ namespace App\Models; // use Illuminate\Contracts\Auth\MustVerifyEmail; +use App\Actions\Permissions\PermissionService; +use App\Enums\Permission; use Database\Factories\UserFactory; use Illuminate\Database\Eloquent\Attributes\Fillable; use Illuminate\Database\Eloquent\Attributes\Hidden; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; @@ -76,4 +79,22 @@ class User extends Authenticatable { return $this->hasMany(Ticket::class, 'author_user_id'); } + + public function scopeVisibleToProject(Builder $query, Project $project): Builder + { + $permissionService = app(PermissionService::class); + + $allowedUserIds = User::query() + ->select('id') + ->cursor() + ->filter(fn (User $candidate) => $permissionService->allows( + $candidate, + Permission::ProjectRead, + $project + )) + ->pluck('id') + ->all(); + + return $query->whereKey($allowedUserIds); + } } diff --git a/docs/openapi/kanbangit-api.openapi.json b/docs/openapi/kanbangit-api.openapi.json index 9a1a607..043b565 100644 --- a/docs/openapi/kanbangit-api.openapi.json +++ b/docs/openapi/kanbangit-api.openapi.json @@ -1643,7 +1643,7 @@ } }, "AssignedTicketsPayload": { - "description": "Tickets assigned to a user grouped by projects visible to the authenticated application key owner.", + "description": "Tickets assigned to a user grouped by projects visible to the authenticated application key owner. meta.assignee is null when no visible assigned tickets exist.", "content": { "application/json": { "schema": { @@ -1844,7 +1844,14 @@ "type": "object", "properties": { "assignee": { - "$ref": "#/components/schemas/User" + "anyOf": [ + { + "$ref": "#/components/schemas/User" + }, + { + "type": "null" + } + ] } }, "required": ["assignee"] diff --git a/tests/Feature/Api/AssignedTicketsApiTest.php b/tests/Feature/Api/AssignedTicketsApiTest.php index 196f39e..0b7647f 100644 --- a/tests/Feature/Api/AssignedTicketsApiTest.php +++ b/tests/Feature/Api/AssignedTicketsApiTest.php @@ -77,4 +77,37 @@ class AssignedTicketsApiTest extends TestCase ->assertJsonCount(1, 'data.1.tickets') ->assertJsonCount(2, 'data'); } + + public function test_assignee_metadata_is_hidden_when_no_visible_assigned_tickets_exist(): void + { + $viewer = User::factory()->create(); + $assignee = User::factory()->create([ + 'name' => 'Hidden User', + ]); + + $visibleProject = Project::factory()->create([ + 'key' => 'VIS', + 'name' => 'Visible project', + ]); + $hiddenProject = Project::factory()->create([ + 'key' => 'HID', + 'name' => 'Hidden project', + ]); + + $this->syncProjectPermissions($visibleProject, $viewer, ['view']); + + Ticket::factory()->create([ + 'assignee_user_id' => $assignee->id, + 'project_id' => $hiddenProject->id, + 'sequence_number' => 1, + 'ticket_key' => 'HID-1', + 'title' => 'Hidden ticket', + ]); + + $this->withApplicationKey($viewer) + ->getJson(route('api.users.assigned-tickets', $assignee)) + ->assertOk() + ->assertJsonPath('data', []) + ->assertJsonPath('meta.assignee', null); + } } diff --git a/tests/Feature/Api/OpenApiDocumentationTest.php b/tests/Feature/Api/OpenApiDocumentationTest.php index db2ae00..24418f2 100644 --- a/tests/Feature/Api/OpenApiDocumentationTest.php +++ b/tests/Feature/Api/OpenApiDocumentationTest.php @@ -83,5 +83,13 @@ class OpenApiDocumentationTest extends TestCase $this->assertArrayHasKey('UserGitConnectionRequest', $document['components']['schemas']); $this->assertArrayHasKey('AssignedTicketsPayloadEnvelope', $document['components']['schemas']); $this->assertArrayHasKey('AssignedTicketsProjectGroup', $document['components']['schemas']); + + $assigneeSchema = $document['components']['schemas']['AssignedTicketsPayloadEnvelope']['properties']['meta']['properties']['assignee'] ?? null; + + $this->assertIsArray($assigneeSchema); + $this->assertSame([ + ['$ref' => '#/components/schemas/User'], + ['type' => 'null'], + ], $assigneeSchema['anyOf'] ?? null); } } diff --git a/tests/Feature/Api/SettingsApiTest.php b/tests/Feature/Api/SettingsApiTest.php index 110e656..9747a66 100644 --- a/tests/Feature/Api/SettingsApiTest.php +++ b/tests/Feature/Api/SettingsApiTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Api; use App\Actions\ProjectTicketMetadata\InitializeProjectTicketMetadata; use App\Models\Project; +use App\Models\Team; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; @@ -149,6 +150,45 @@ class SettingsApiTest extends TestCase ->assertNoContent(); } + public function test_project_payloads_only_return_users_who_can_view_the_project(): void + { + [$user, $project] = $this->projectContext(); + $viewer = User::factory()->create(['name' => 'Visible User']); + User::factory()->create(['name' => 'Hidden User']); + $teamViewer = User::factory()->create(['name' => 'Team Viewer']); + $team = Team::factory()->create(); + + $this->syncProjectPermissions($project, $viewer, ['view']); + $team->users()->attach($teamViewer); + $this->syncProjectPermissions($project, $team, ['view']); + + $ticket = $project->tickets()->firstOrCreate([ + 'sequence_number' => 1, + ], [ + 'ticket_key' => 'CORE-1', + 'ticket_priority_id' => $project->ticketPriorities()->firstOrFail()->id, + 'ticket_status_id' => $project->ticketStatuses()->firstOrFail()->id, + 'ticket_type_id' => $project->ticketTypes()->where('is_parent', false)->firstOrFail()->id, + 'title' => 'Scoped users ticket', + ]); + + $this->withApplicationKey($user) + ->getJson(route('api.projects.backlog', $project)) + ->assertOk() + ->assertJsonPath('data.users.0.name', $user->name) + ->assertJsonPath('data.users.1.name', 'Team Viewer') + ->assertJsonPath('data.users.2.name', 'Visible User') + ->assertJsonCount(3, 'data.users'); + + $this->withApplicationKey($user) + ->getJson(route('api.projects.tickets.show', [$project, $ticket])) + ->assertOk() + ->assertJsonPath('data.users.0.name', $user->name) + ->assertJsonPath('data.users.1.name', 'Team Viewer') + ->assertJsonPath('data.users.2.name', 'Visible User') + ->assertJsonCount(3, 'data.users'); + } + public function test_project_repositories_and_user_git_connections_are_available(): void { Http::preventStrayRequests(); diff --git a/tests/Feature/Api/TicketApiTest.php b/tests/Feature/Api/TicketApiTest.php index 5185153..224eadd 100644 --- a/tests/Feature/Api/TicketApiTest.php +++ b/tests/Feature/Api/TicketApiTest.php @@ -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->withApplicationKey($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->withApplicationKey($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();