Skip to content

Fix project and global invitation access control issues #2894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josephsnyder merged 1 commit into from
May 22, 2025
Merged

Fix project and global invitation access control issues #2894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/Policies/GlobalInvitationPolicy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@

class GlobalInvitationPolicy
{
public function viewInvitations(?User $currentUser): bool
{
return $this->createInvitation($currentUser);
}

public function createInvitation(?User $currentUser): bool
{
if (config('cdash.username_password_authentication_enabled') === false) {
Expand Down
6 changes: 5 additions & 1 deletion app/cdash/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ add_feature_test(/Feature/GraphQL/NoteTypeTest)

add_feature_test(/Feature/GraphQL/CoverageTypeTest)

add_feature_test(/Feature/GraphQL/UserInvitationTypeTest)
add_feature_test(/Feature/GraphQL/ProjectInvitationTypeTest)

add_feature_test(/Feature/GraphQL/LabelTypeTest)

Expand Down Expand Up @@ -312,6 +312,10 @@ add_feature_test(/Feature/GraphQL/Mutations/RevokeGlobalInvitationTest)
add_feature_test(/Feature/GlobalInvitationAcceptanceTest)
set_property(TEST /Feature/GlobalInvitationAcceptanceTest APPEND PROPERTY RUN_SERIAL TRUE)

# Requires exclusive access to the global_invitations table
add_feature_test(/Feature/GraphQL/GlobalInvitationTypeTest)
set_property(TEST /Feature/GraphQL/GlobalInvitationTypeTest APPEND PROPERTY RUN_SERIAL TRUE)

add_feature_test(/Feature/GraphQL/Mutations/RemoveUserTest)

add_browser_test(/Browser/Pages/UsersPageTest)
Expand Down
5 changes: 3 additions & 2 deletions graphql/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type Query {
id: ID @eq
): Site @find

invitations: [GlobalInvitation!]! @paginate(type: CONNECTION) @orderBy(column: "id")
invitations: [GlobalInvitation!]! @canResolved(ability: "viewInvitations") @paginate(type: CONNECTION) @orderBy(column: "id")
}


Expand Down Expand Up @@ -176,8 +176,9 @@ type Project {
"Users with the administrator role for this project."
administrators: [User!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id")

# Can't return null on authorization failure due to Lighthouse bug with connections and can* directives.
"Invitations to this project which have not been accepted yet."
invitations: [ProjectInvitation!]! @hasMany(type: CONNECTION) @orderBy(column: "id")
invitations: [ProjectInvitation!]! @canRoot(ability: "inviteUser") @hasMany(type: CONNECTION) @orderBy(column: "id")
}

enum ProjectVisibility {
Expand Down
45 changes: 30 additions & 15 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29125,6 +29125,16 @@ parameters:
count: 1
path: tests/Feature/GraphQL/FilterTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Testing\\\\TestResponse\\:\\:assertGraphQLErrorMessage\\(\\)\\.$#"
count: 2
path: tests/Feature/GraphQL/GlobalInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\GlobalInvitationTypeTest\\:\\:createInvitation\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/GlobalInvitationTypeTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Relations\\\\BelongsToMany\\<App\\\\Models\\\\User\\>\\:\\:pluck\\(\\)\\.$#"
count: 64
Expand Down Expand Up @@ -29245,6 +29255,26 @@ parameters:
count: 1
path: tests/Feature/GraphQL/Mutations/RevokeProjectInvitationTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Testing\\\\TestResponse\\:\\:assertGraphQLErrorMessage\\(\\)\\.$#"
count: 2
path: tests/Feature/GraphQL/ProjectInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\ProjectInvitationTypeTest\\:\\:testAdminCanViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/ProjectInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\ProjectInvitationTypeTest\\:\\:testAnonymousUserCannotViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/ProjectInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\ProjectInvitationTypeTest\\:\\:testNormalUserCannotViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/ProjectInvitationTypeTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Testing\\\\TestResponse\\:\\:assertGraphQLErrorMessage\\(\\)\\.$#"
count: 5
Expand Down Expand Up @@ -29295,21 +29325,6 @@ parameters:
count: 1
path: tests/Feature/GraphQL/SiteTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\UserInvitationTypeTest\\:\\:testAdminCanViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/UserInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\UserInvitationTypeTest\\:\\:testAnonymousUserCannotViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/UserInvitationTypeTest.php

-
message: "#^Method Tests\\\\Feature\\\\GraphQL\\\\UserInvitationTypeTest\\:\\:testNormalUserCannotViewInvitations\\(\\) throws checked exception OverflowException but it's missing from the PHPDoc @throws tag\\.$#"
count: 1
path: tests/Feature/GraphQL/UserInvitationTypeTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<App\\\\Models\\\\SuccessfulJob\\>\\:\\:count\\(\\)\\.$#"
count: 4
Expand Down
146 changes: 146 additions & 0 deletions tests/Feature/GraphQL/GlobalInvitationTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
<?php

namespace Tests\Feature\GraphQL;

use App\Enums\GlobalRole;
use App\Models\GlobalInvitation;
use App\Models\User;
use Illuminate\Foundation\Testing\DatabaseTruncation;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Str;
use Tests\TestCase;
use Tests\Traits\CreatesUsers;

class GlobalInvitationTypeTest extends TestCase
{
use CreatesUsers;
use DatabaseTruncation;

private User $normalUser;
private User $adminUser;

/** @var array<GlobalInvitation> */
private array $invitations = [];

protected function setUp(): void
{
parent::setUp();

$this->normalUser = $this->makeNormalUser();
$this->adminUser = $this->makeAdminUser();
}

protected function tearDown(): void
{
foreach ($this->invitations as $invitation) {
$invitation->delete();
}
$this->invitations = [];

$this->normalUser->delete();
$this->adminUser->delete();

parent::tearDown();
}

private function createInvitation(): GlobalInvitation
{
/** @var GlobalInvitation $invitation */
$invitation = GlobalInvitation::create([
'email' => fake()->unique()->email(),
'invited_by_id' => $this->adminUser->id,
'role' => GlobalRole::USER,
'invitation_timestamp' => Carbon::now(),
'password' => Hash::make(Str::password()),
]);
$this->invitations[] = $invitation;

return $invitation;
}

public function testAdminCanViewInvitations(): void
{
$invitation = $this->createInvitation();

$this->actingAs($this->adminUser)->graphQL('
query {
invitations {
edges {
node {
id
email
invitedBy {
id
}
role
invitationTimestamp
}
}
}
}
')->assertExactJson([
'data' => [
'invitations' => [
'edges' => [
[
'node' => [
'id' => (string) $invitation->id,
'email' => $invitation->email,
'invitedBy' => [
'id' => (string) $this->adminUser->id,
],
'role' => 'USER',
'invitationTimestamp' => $invitation->invitation_timestamp->toIso8601String(),
],
],
],
],
],
]);
}

public function testNormalUserCannotViewInvitations(): void
{
$this->createInvitation();
$this->actingAs($this->normalUser)->graphQL('
query {
invitations {
edges {
node {
id
email
invitedBy {
id
}
role
invitationTimestamp
}
}
}
}
')->assertGraphQLErrorMessage('This action is unauthorized.');
}

public function testAnonymousUserCannotViewInvitations(): void
{
$this->createInvitation();
$this->graphQL('
query {
invitations {
edges {
node {
id
email
invitedBy {
id
}
role
invitationTimestamp
}
}
}
}
')->assertGraphQLErrorMessage('This action is unauthorized.');
}
}
28 changes: 6 additions & 22 deletions ...eature/GraphQL/UserInvitationTypeTest.php → ...ure/GraphQL/ProjectInvitationTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use Tests\Traits\CreatesProjects;
use Tests\Traits\CreatesUsers;

class UserInvitationTypeTest extends TestCase
class ProjectInvitationTypeTest extends TestCase
{
use CreatesUsers;
use CreatesProjects;
Expand Down Expand Up @@ -71,7 +71,7 @@ public function testAdminCanViewInvitations(): void
}
', [
'id' => $this->project->id,
])->assertJson([
])->assertExactJson([
'data' => [
'project' => [
'invitations' => [
Expand All @@ -94,7 +94,7 @@ public function testAdminCanViewInvitations(): void
],
],
],
], true);
]);
}

public function testNormalUserCannotViewInvitations(): void
Expand All @@ -107,7 +107,7 @@ public function testNormalUserCannotViewInvitations(): void
'invitation_timestamp' => Carbon::now(),
]);

$this->actingAs($this->adminUser)->graphQL('
$this->actingAs($this->normalUser)->graphQL('
query($id: ID) {
project(id: $id) {
invitations {
Expand All @@ -121,15 +121,7 @@ public function testNormalUserCannotViewInvitations(): void
}
', [
'id' => $this->project->id,
])->assertJson([
'data' => [
'project' => [
'invitations' => [
'edges' => [],
],
],
],
], true);
])->assertGraphQLErrorMessage('This action is unauthorized.');
}

public function testAnonymousUserCannotViewInvitations(): void
Expand All @@ -156,14 +148,6 @@ public function testAnonymousUserCannotViewInvitations(): void
}
', [
'id' => $this->project->id,
])->assertJson([
'data' => [
'project' => [
'invitations' => [
'edges' => [],
],
],
],
], true);
])->assertGraphQLErrorMessage('This action is unauthorized.');
}
}