Skip to content

Commit 39ae16d

Browse files
andrasbacsaiandrasbacsai
authored
Improve team resource route handling (#10829)
2 parents 74f4d04 + a121386 commit 39ae16d

6 files changed

Lines changed: 238 additions & 52 deletions

File tree

‎app/Http/Middleware/CanUpdateResource.php‎

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Models\Application;
66
use App\Models\Environment;
77
use App\Models\Project;
8+
use App\Models\Server;
89
use App\Models\Service;
910
use App\Models\ServiceApplication;
1011
use App\Models\ServiceDatabase;
@@ -23,53 +24,61 @@
2324

2425
class CanUpdateResource
2526
{
27+
/**
28+
* @var array<string, list<class-string>>
29+
*/
30+
private const ROUTE_RESOURCE_MODELS = [
31+
'application_uuid' => [Application::class],
32+
'database_uuid' => [
33+
StandalonePostgresql::class,
34+
StandaloneMysql::class,
35+
StandaloneMariadb::class,
36+
StandaloneRedis::class,
37+
StandaloneKeydb::class,
38+
StandaloneDragonfly::class,
39+
StandaloneClickhouse::class,
40+
StandaloneMongodb::class,
41+
],
42+
'stack_service_uuid' => [ServiceApplication::class, ServiceDatabase::class],
43+
'service_uuid' => [Service::class],
44+
'server_uuid' => [Server::class],
45+
'environment_uuid' => [Environment::class],
46+
'project_uuid' => [Project::class],
47+
];
48+
2649
public function handle(Request $request, Closure $next): Response
2750
{
51+
$resource = $this->resourceFromRoute($request);
52+
53+
if (! $resource) {
54+
abort(404, 'Resource not found.');
55+
}
56+
57+
if (! Gate::allows('update', $resource)) {
58+
abort(403, 'You do not have permission to update this resource.');
59+
}
60+
2861
return $next($request);
62+
}
2963

30-
// Get resource from route parameters
31-
// $resource = null;
32-
// if ($request->route('application_uuid')) {
33-
// $resource = Application::where('uuid', $request->route('application_uuid'))->first();
34-
// } elseif ($request->route('service_uuid')) {
35-
// $resource = Service::where('uuid', $request->route('service_uuid'))->first();
36-
// } elseif ($request->route('stack_service_uuid')) {
37-
// // Handle ServiceApplication or ServiceDatabase
38-
// $stack_service_uuid = $request->route('stack_service_uuid');
39-
// $resource = ServiceApplication::where('uuid', $stack_service_uuid)->first() ??
40-
// ServiceDatabase::where('uuid', $stack_service_uuid)->first();
41-
// } elseif ($request->route('database_uuid')) {
42-
// // Try different database types
43-
// $database_uuid = $request->route('database_uuid');
44-
// $resource = StandalonePostgresql::where('uuid', $database_uuid)->first() ??
45-
// StandaloneMysql::where('uuid', $database_uuid)->first() ??
46-
// StandaloneMariadb::where('uuid', $database_uuid)->first() ??
47-
// StandaloneRedis::where('uuid', $database_uuid)->first() ??
48-
// StandaloneKeydb::where('uuid', $database_uuid)->first() ??
49-
// StandaloneDragonfly::where('uuid', $database_uuid)->first() ??
50-
// StandaloneClickhouse::where('uuid', $database_uuid)->first() ??
51-
// StandaloneMongodb::where('uuid', $database_uuid)->first();
52-
// } elseif ($request->route('server_uuid')) {
53-
// // For server routes, check if user can manage servers
54-
// if (! auth()->user()->isAdmin()) {
55-
// abort(403, 'You do not have permission to access this resource.');
56-
// }
64+
private function resourceFromRoute(Request $request): ?object
65+
{
66+
foreach (self::ROUTE_RESOURCE_MODELS as $routeParameter => $models) {
67+
$uuid = $request->route($routeParameter);
5768

58-
// return $next($request);
59-
// } elseif ($request->route('environment_uuid')) {
60-
// $resource = Environment::where('uuid', $request->route('environment_uuid'))->first();
61-
// } elseif ($request->route('project_uuid')) {
62-
// $resource = Project::ownedByCurrentTeam()->where('uuid', $request->route('project_uuid'))->first();
63-
// }
69+
if (! $uuid) {
70+
continue;
71+
}
6472

65-
// if (! $resource) {
66-
// abort(404, 'Resource not found.');
67-
// }
73+
foreach ($models as $model) {
74+
$resource = $model::where('uuid', $uuid)->first();
6875

69-
// if (! Gate::allows('update', $resource)) {
70-
// abort(403, 'You do not have permission to update this resource.');
71-
// }
76+
if ($resource) {
77+
return $resource;
78+
}
79+
}
80+
}
7281

73-
// return $next($request);
82+
return null;
7483
}
7584
}

‎app/Policies/TeamPolicy.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,63 +37,58 @@ public function create(User $user): bool
3737
*/
3838
public function update(User $user, Team $team): bool
3939
{
40-
// Only admins and owners can update team settings
4140
if (! $user->teams->contains('id', $team->id)) {
4241
return false;
4342
}
4443

45-
return $user->isAdmin() || $user->isOwner();
44+
return $user->isAdminOfTeam($team->id);
4645
}
4746

4847
/**
4948
* Determine whether the user can delete the model.
5049
*/
5150
public function delete(User $user, Team $team): bool
5251
{
53-
// Only admins and owners can delete teams
5452
if (! $user->teams->contains('id', $team->id)) {
5553
return false;
5654
}
5755

58-
return $user->isAdmin() || $user->isOwner();
56+
return $user->isAdminOfTeam($team->id);
5957
}
6058

6159
/**
6260
* Determine whether the user can manage team members.
6361
*/
6462
public function manageMembers(User $user, Team $team): bool
6563
{
66-
// Only admins and owners can manage team members
6764
if (! $user->teams->contains('id', $team->id)) {
6865
return false;
6966
}
7067

71-
return $user->isAdmin() || $user->isOwner();
68+
return $user->isAdminOfTeam($team->id);
7269
}
7370

7471
/**
7572
* Determine whether the user can view admin panel.
7673
*/
7774
public function viewAdmin(User $user, Team $team): bool
7875
{
79-
// Only admins and owners can view admin panel
8076
if (! $user->teams->contains('id', $team->id)) {
8177
return false;
8278
}
8379

84-
return $user->isAdmin() || $user->isOwner();
80+
return $user->isAdminOfTeam($team->id);
8581
}
8682

8783
/**
8884
* Determine whether the user can manage invitations.
8985
*/
9086
public function manageInvitations(User $user, Team $team): bool
9187
{
92-
// Only admins and owners can manage invitations
9388
if (! $user->teams->contains('id', $team->id)) {
9489
return false;
9590
}
9691

97-
return $user->isAdmin() || $user->isOwner();
92+
return $user->isAdminOfTeam($team->id);
9893
}
9994
}

‎templates/compose/inngest.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,3 @@ services:
6262
timeout: 3s
6363
retries: 5
6464
restart: unless-stopped
65-

‎tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
uses(RefreshDatabase::class);
1919

2020
beforeEach(function () {
21-
InstanceSettings::updateOrCreate(['id' => 0]);
21+
$this->withoutVite();
22+
23+
InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['id' => 0]));
2224

2325
$this->team = Team::factory()->create();
2426

‎tests/Feature/Authorization/CanUpdateResourceMiddlewareTest.php‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?php
2+
3+
use App\Http\Middleware\CanUpdateResource;
4+
use App\Models\InstanceSettings;
5+
use App\Models\Project;
6+
use App\Models\Server;
7+
use App\Models\Team;
8+
use App\Models\User;
9+
use Illuminate\Foundation\Testing\RefreshDatabase;
10+
use Illuminate\Http\Request;
11+
use Symfony\Component\HttpKernel\Exception\HttpException;
12+
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
13+
14+
uses(RefreshDatabase::class);
15+
16+
function requestWithCanUpdateResourceRouteParameter(string $parameter, ?string $value): Request
17+
{
18+
$parameters = [
19+
'application_uuid' => null,
20+
'database_uuid' => null,
21+
'stack_service_uuid' => null,
22+
'service_uuid' => null,
23+
'server_uuid' => null,
24+
'environment_uuid' => null,
25+
'project_uuid' => null,
26+
$parameter => $value,
27+
];
28+
29+
$request = Mockery::mock(Request::class)->makePartial();
30+
$request->shouldReceive('route')->andReturnUsing(fn (string $key): ?string => $parameters[$key] ?? null);
31+
32+
return $request;
33+
}
34+
35+
beforeEach(function () {
36+
InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['id' => 0]));
37+
38+
$this->team = Team::factory()->create();
39+
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
40+
$this->server = Server::factory()->create(['team_id' => $this->team->id]);
41+
42+
$this->admin = User::factory()->create();
43+
$this->admin->teams()->attach($this->team, ['role' => 'admin']);
44+
45+
$this->member = User::factory()->create();
46+
$this->member->teams()->attach($this->team, ['role' => 'member']);
47+
});
48+
49+
it('blocks members from update-only project routes before the page renders', function () {
50+
$this->actingAs($this->member);
51+
session(['currentTeam' => $this->team]);
52+
53+
(new CanUpdateResource)->handle(
54+
requestWithCanUpdateResourceRouteParameter('project_uuid', $this->project->uuid),
55+
fn () => response('ok')
56+
);
57+
})->throws(HttpException::class, 'You do not have permission to update this resource.');
58+
59+
it('allows admins through update-only project routes', function () {
60+
$this->actingAs($this->admin);
61+
session(['currentTeam' => $this->team]);
62+
63+
$response = (new CanUpdateResource)->handle(
64+
requestWithCanUpdateResourceRouteParameter('project_uuid', $this->project->uuid),
65+
fn () => response('ok')
66+
);
67+
68+
expect($response->getContent())->toBe('ok');
69+
});
70+
71+
it('blocks members from update-only server routes before the page renders', function () {
72+
$this->actingAs($this->member);
73+
session(['currentTeam' => $this->team]);
74+
75+
(new CanUpdateResource)->handle(
76+
requestWithCanUpdateResourceRouteParameter('server_uuid', $this->server->uuid),
77+
fn () => response('ok')
78+
);
79+
})->throws(HttpException::class, 'You do not have permission to update this resource.');
80+
81+
it('returns not found when an update-only route references an unknown resource', function () {
82+
$this->actingAs($this->admin);
83+
session(['currentTeam' => $this->team]);
84+
85+
(new CanUpdateResource)->handle(
86+
requestWithCanUpdateResourceRouteParameter('project_uuid', 'not-a-project'),
87+
fn () => response('ok')
88+
);
89+
})->throws(NotFoundHttpException::class, 'Resource not found.');

‎tests/Unit/Policies/TeamPolicyTest.php‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
<?php
2+
3+
use App\Models\Team;
4+
use App\Models\User;
5+
use App\Policies\TeamPolicy;
6+
7+
function teamPolicyUserWithTeams(array $teamIds): User
8+
{
9+
$user = Mockery::mock(User::class)->makePartial();
10+
$user->shouldReceive('getAttribute')->with('teams')->andReturn(collect(
11+
array_map(fn (int $teamId): object => (object) ['id' => $teamId], $teamIds)
12+
));
13+
14+
return $user;
15+
}
16+
17+
function teamPolicyTeam(int $teamId): Team
18+
{
19+
$team = Mockery::mock(Team::class)->makePartial();
20+
$team->shouldReceive('getAttribute')->with('id')->andReturn($teamId);
21+
22+
return $team;
23+
}
24+
25+
it('allows any authenticated user to view any teams list', function () {
26+
$user = Mockery::mock(User::class)->makePartial();
27+
28+
expect((new TeamPolicy)->viewAny($user))->toBeTrue();
29+
});
30+
31+
it('allows authenticated users to create teams', function () {
32+
$user = Mockery::mock(User::class)->makePartial();
33+
34+
expect((new TeamPolicy)->create($user))->toBeTrue();
35+
});
36+
37+
it('allows target team members to view the team', function () {
38+
$user = teamPolicyUserWithTeams([1]);
39+
$team = teamPolicyTeam(1);
40+
41+
expect((new TeamPolicy)->view($user, $team))->toBeTrue();
42+
});
43+
44+
it('denies non-members from viewing the team', function () {
45+
$user = teamPolicyUserWithTeams([2]);
46+
$team = teamPolicyTeam(1);
47+
48+
expect((new TeamPolicy)->view($user, $team))->toBeFalse();
49+
});
50+
51+
it('allows target team admins to perform privileged team actions', function (string $ability) {
52+
$user = teamPolicyUserWithTeams([1]);
53+
$user->shouldReceive('isAdminOfTeam')->with(1)->andReturn(true);
54+
$team = teamPolicyTeam(1);
55+
56+
expect((new TeamPolicy)->{$ability}($user, $team))->toBeTrue();
57+
})->with([
58+
'update',
59+
'delete',
60+
'manageMembers',
61+
'viewAdmin',
62+
'manageInvitations',
63+
]);
64+
65+
it('denies target team members even when their current session role is admin elsewhere', function (string $ability) {
66+
$user = teamPolicyUserWithTeams([1, 2]);
67+
$user->shouldReceive('isAdmin')->andReturn(true);
68+
$user->shouldReceive('isOwner')->andReturn(false);
69+
$user->shouldReceive('isAdminOfTeam')->with(1)->andReturn(false);
70+
$team = teamPolicyTeam(1);
71+
72+
expect((new TeamPolicy)->{$ability}($user, $team))->toBeFalse();
73+
})->with([
74+
'update',
75+
'delete',
76+
'manageMembers',
77+
'viewAdmin',
78+
'manageInvitations',
79+
]);
80+
81+
it('denies non-members from privileged team actions', function (string $ability) {
82+
$user = teamPolicyUserWithTeams([2]);
83+
$team = teamPolicyTeam(1);
84+
85+
expect((new TeamPolicy)->{$ability}($user, $team))->toBeFalse();
86+
})->with([
87+
'update',
88+
'delete',
89+
'manageMembers',
90+
'viewAdmin',
91+
'manageInvitations',
92+
]);

0 commit comments

Comments
 (0)