diff --git a/app/Http/Middleware/CanUpdateResource.php b/app/Http/Middleware/CanUpdateResource.php index 372af4498..3b28ee07c 100644 --- a/app/Http/Middleware/CanUpdateResource.php +++ b/app/Http/Middleware/CanUpdateResource.php @@ -5,6 +5,7 @@ use App\Models\Application; use App\Models\Environment; use App\Models\Project; +use App\Models\Server; use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; @@ -23,53 +24,61 @@ class CanUpdateResource { + /** + * @var array> + */ + private const ROUTE_RESOURCE_MODELS = [ + 'application_uuid' => [Application::class], + 'database_uuid' => [ + StandalonePostgresql::class, + StandaloneMysql::class, + StandaloneMariadb::class, + StandaloneRedis::class, + StandaloneKeydb::class, + StandaloneDragonfly::class, + StandaloneClickhouse::class, + StandaloneMongodb::class, + ], + 'stack_service_uuid' => [ServiceApplication::class, ServiceDatabase::class], + 'service_uuid' => [Service::class], + 'server_uuid' => [Server::class], + 'environment_uuid' => [Environment::class], + 'project_uuid' => [Project::class], + ]; + public function handle(Request $request, Closure $next): Response { + $resource = $this->resourceFromRoute($request); + + if (! $resource) { + abort(404, 'Resource not found.'); + } + + if (! Gate::allows('update', $resource)) { + abort(403, 'You do not have permission to update this resource.'); + } + return $next($request); + } - // Get resource from route parameters - // $resource = null; - // if ($request->route('application_uuid')) { - // $resource = Application::where('uuid', $request->route('application_uuid'))->first(); - // } elseif ($request->route('service_uuid')) { - // $resource = Service::where('uuid', $request->route('service_uuid'))->first(); - // } elseif ($request->route('stack_service_uuid')) { - // // Handle ServiceApplication or ServiceDatabase - // $stack_service_uuid = $request->route('stack_service_uuid'); - // $resource = ServiceApplication::where('uuid', $stack_service_uuid)->first() ?? - // ServiceDatabase::where('uuid', $stack_service_uuid)->first(); - // } elseif ($request->route('database_uuid')) { - // // Try different database types - // $database_uuid = $request->route('database_uuid'); - // $resource = StandalonePostgresql::where('uuid', $database_uuid)->first() ?? - // StandaloneMysql::where('uuid', $database_uuid)->first() ?? - // StandaloneMariadb::where('uuid', $database_uuid)->first() ?? - // StandaloneRedis::where('uuid', $database_uuid)->first() ?? - // StandaloneKeydb::where('uuid', $database_uuid)->first() ?? - // StandaloneDragonfly::where('uuid', $database_uuid)->first() ?? - // StandaloneClickhouse::where('uuid', $database_uuid)->first() ?? - // StandaloneMongodb::where('uuid', $database_uuid)->first(); - // } elseif ($request->route('server_uuid')) { - // // For server routes, check if user can manage servers - // if (! auth()->user()->isAdmin()) { - // abort(403, 'You do not have permission to access this resource.'); - // } + private function resourceFromRoute(Request $request): ?object + { + foreach (self::ROUTE_RESOURCE_MODELS as $routeParameter => $models) { + $uuid = $request->route($routeParameter); - // return $next($request); - // } elseif ($request->route('environment_uuid')) { - // $resource = Environment::where('uuid', $request->route('environment_uuid'))->first(); - // } elseif ($request->route('project_uuid')) { - // $resource = Project::ownedByCurrentTeam()->where('uuid', $request->route('project_uuid'))->first(); - // } + if (! $uuid) { + continue; + } - // if (! $resource) { - // abort(404, 'Resource not found.'); - // } + foreach ($models as $model) { + $resource = $model::where('uuid', $uuid)->first(); - // if (! Gate::allows('update', $resource)) { - // abort(403, 'You do not have permission to update this resource.'); - // } + if ($resource) { + return $resource; + } + } + } - // return $next($request); + return null; } } diff --git a/app/Policies/TeamPolicy.php b/app/Policies/TeamPolicy.php index 849e23751..cc7745b64 100644 --- a/app/Policies/TeamPolicy.php +++ b/app/Policies/TeamPolicy.php @@ -37,12 +37,11 @@ public function create(User $user): bool */ public function update(User $user, Team $team): bool { - // Only admins and owners can update team settings if (! $user->teams->contains('id', $team->id)) { return false; } - return $user->isAdmin() || $user->isOwner(); + return $user->isAdminOfTeam($team->id); } /** @@ -50,12 +49,11 @@ public function update(User $user, Team $team): bool */ public function delete(User $user, Team $team): bool { - // Only admins and owners can delete teams if (! $user->teams->contains('id', $team->id)) { return false; } - return $user->isAdmin() || $user->isOwner(); + return $user->isAdminOfTeam($team->id); } /** @@ -63,12 +61,11 @@ public function delete(User $user, Team $team): bool */ public function manageMembers(User $user, Team $team): bool { - // Only admins and owners can manage team members if (! $user->teams->contains('id', $team->id)) { return false; } - return $user->isAdmin() || $user->isOwner(); + return $user->isAdminOfTeam($team->id); } /** @@ -76,12 +73,11 @@ public function manageMembers(User $user, Team $team): bool */ public function viewAdmin(User $user, Team $team): bool { - // Only admins and owners can view admin panel if (! $user->teams->contains('id', $team->id)) { return false; } - return $user->isAdmin() || $user->isOwner(); + return $user->isAdminOfTeam($team->id); } /** @@ -89,11 +85,10 @@ public function viewAdmin(User $user, Team $team): bool */ public function manageInvitations(User $user, Team $team): bool { - // Only admins and owners can manage invitations if (! $user->teams->contains('id', $team->id)) { return false; } - return $user->isAdmin() || $user->isOwner(); + return $user->isAdminOfTeam($team->id); } } diff --git a/templates/compose/inngest.yaml b/templates/compose/inngest.yaml index c2578eb81..aaf0070c0 100644 --- a/templates/compose/inngest.yaml +++ b/templates/compose/inngest.yaml @@ -62,4 +62,3 @@ services: timeout: 3s retries: 5 restart: unless-stopped - diff --git a/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php b/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php index 2c3aa123f..31c30c124 100644 --- a/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php +++ b/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php @@ -18,7 +18,9 @@ uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::updateOrCreate(['id' => 0]); + $this->withoutVite(); + + InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['id' => 0])); $this->team = Team::factory()->create(); diff --git a/tests/Feature/Authorization/CanUpdateResourceMiddlewareTest.php b/tests/Feature/Authorization/CanUpdateResourceMiddlewareTest.php new file mode 100644 index 000000000..4d440b41a --- /dev/null +++ b/tests/Feature/Authorization/CanUpdateResourceMiddlewareTest.php @@ -0,0 +1,89 @@ + null, + 'database_uuid' => null, + 'stack_service_uuid' => null, + 'service_uuid' => null, + 'server_uuid' => null, + 'environment_uuid' => null, + 'project_uuid' => null, + $parameter => $value, + ]; + + $request = Mockery::mock(Request::class)->makePartial(); + $request->shouldReceive('route')->andReturnUsing(fn (string $key): ?string => $parameters[$key] ?? null); + + return $request; +} + +beforeEach(function () { + InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['id' => 0])); + + $this->team = Team::factory()->create(); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + + $this->admin = User::factory()->create(); + $this->admin->teams()->attach($this->team, ['role' => 'admin']); + + $this->member = User::factory()->create(); + $this->member->teams()->attach($this->team, ['role' => 'member']); +}); + +it('blocks members from update-only project routes before the page renders', function () { + $this->actingAs($this->member); + session(['currentTeam' => $this->team]); + + (new CanUpdateResource)->handle( + requestWithCanUpdateResourceRouteParameter('project_uuid', $this->project->uuid), + fn () => response('ok') + ); +})->throws(HttpException::class, 'You do not have permission to update this resource.'); + +it('allows admins through update-only project routes', function () { + $this->actingAs($this->admin); + session(['currentTeam' => $this->team]); + + $response = (new CanUpdateResource)->handle( + requestWithCanUpdateResourceRouteParameter('project_uuid', $this->project->uuid), + fn () => response('ok') + ); + + expect($response->getContent())->toBe('ok'); +}); + +it('blocks members from update-only server routes before the page renders', function () { + $this->actingAs($this->member); + session(['currentTeam' => $this->team]); + + (new CanUpdateResource)->handle( + requestWithCanUpdateResourceRouteParameter('server_uuid', $this->server->uuid), + fn () => response('ok') + ); +})->throws(HttpException::class, 'You do not have permission to update this resource.'); + +it('returns not found when an update-only route references an unknown resource', function () { + $this->actingAs($this->admin); + session(['currentTeam' => $this->team]); + + (new CanUpdateResource)->handle( + requestWithCanUpdateResourceRouteParameter('project_uuid', 'not-a-project'), + fn () => response('ok') + ); +})->throws(NotFoundHttpException::class, 'Resource not found.'); diff --git a/tests/Unit/Policies/TeamPolicyTest.php b/tests/Unit/Policies/TeamPolicyTest.php new file mode 100644 index 000000000..3b341d488 --- /dev/null +++ b/tests/Unit/Policies/TeamPolicyTest.php @@ -0,0 +1,92 @@ +makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn(collect( + array_map(fn (int $teamId): object => (object) ['id' => $teamId], $teamIds) + )); + + return $user; +} + +function teamPolicyTeam(int $teamId): Team +{ + $team = Mockery::mock(Team::class)->makePartial(); + $team->shouldReceive('getAttribute')->with('id')->andReturn($teamId); + + return $team; +} + +it('allows any authenticated user to view any teams list', function () { + $user = Mockery::mock(User::class)->makePartial(); + + expect((new TeamPolicy)->viewAny($user))->toBeTrue(); +}); + +it('allows authenticated users to create teams', function () { + $user = Mockery::mock(User::class)->makePartial(); + + expect((new TeamPolicy)->create($user))->toBeTrue(); +}); + +it('allows target team members to view the team', function () { + $user = teamPolicyUserWithTeams([1]); + $team = teamPolicyTeam(1); + + expect((new TeamPolicy)->view($user, $team))->toBeTrue(); +}); + +it('denies non-members from viewing the team', function () { + $user = teamPolicyUserWithTeams([2]); + $team = teamPolicyTeam(1); + + expect((new TeamPolicy)->view($user, $team))->toBeFalse(); +}); + +it('allows target team admins to perform privileged team actions', function (string $ability) { + $user = teamPolicyUserWithTeams([1]); + $user->shouldReceive('isAdminOfTeam')->with(1)->andReturn(true); + $team = teamPolicyTeam(1); + + expect((new TeamPolicy)->{$ability}($user, $team))->toBeTrue(); +})->with([ + 'update', + 'delete', + 'manageMembers', + 'viewAdmin', + 'manageInvitations', +]); + +it('denies target team members even when their current session role is admin elsewhere', function (string $ability) { + $user = teamPolicyUserWithTeams([1, 2]); + $user->shouldReceive('isAdmin')->andReturn(true); + $user->shouldReceive('isOwner')->andReturn(false); + $user->shouldReceive('isAdminOfTeam')->with(1)->andReturn(false); + $team = teamPolicyTeam(1); + + expect((new TeamPolicy)->{$ability}($user, $team))->toBeFalse(); +})->with([ + 'update', + 'delete', + 'manageMembers', + 'viewAdmin', + 'manageInvitations', +]); + +it('denies non-members from privileged team actions', function (string $ability) { + $user = teamPolicyUserWithTeams([2]); + $team = teamPolicyTeam(1); + + expect((new TeamPolicy)->{$ability}($user, $team))->toBeFalse(); +})->with([ + 'update', + 'delete', + 'manageMembers', + 'viewAdmin', + 'manageInvitations', +]);