diff --git a/src/CachetCoreServiceProvider.php b/src/CachetCoreServiceProvider.php index 4b9b8327..67456c54 100644 --- a/src/CachetCoreServiceProvider.php +++ b/src/CachetCoreServiceProvider.php @@ -24,6 +24,8 @@ use Cachet\Models\Schedule; use Cachet\Models\Subscriber; use Cachet\Models\WebhookAttempt; +use Cachet\Policies\SubscriberPolicy; +use Cachet\Policies\UserPolicy; use Cachet\Settings\AppSettings; use Cachet\Settings\MailSettings; use Cachet\View\Composers\MailThemeComposer; @@ -44,6 +46,7 @@ use Illuminate\Routing\Router; use Illuminate\Support\Facades\Blade; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Route; use Illuminate\Support\ServiceProvider; @@ -106,6 +109,7 @@ public function boot(): void } $this->configureTrustedProxies(); + $this->registerPolicies(); Route::middlewareGroup('cachet', config('cachet.middleware', [])); Route::middlewareGroup('cachet:api', config('cachet.api_middleware', [])); @@ -141,6 +145,16 @@ public function boot(): void $this->configureScramble(); } + /** + * Register policies for Cachet's models and the configured user model. + */ + private function registerPolicies(): void + { + Gate::policy(Models\User::class, UserPolicy::class); + Gate::policy(config('cachet.user_model'), UserPolicy::class); + Gate::policy(Subscriber::class, SubscriberPolicy::class); + } + /** * Register the package's resources such as routes, migrations, etc. */ diff --git a/src/Filament/Resources/ApiKeys/ApiKeyResource.php b/src/Filament/Resources/ApiKeys/ApiKeyResource.php index e0abd7d8..d068e2d4 100644 --- a/src/Filament/Resources/ApiKeys/ApiKeyResource.php +++ b/src/Filament/Resources/ApiKeys/ApiKeyResource.php @@ -5,6 +5,7 @@ use Cachet\Cachet; use Cachet\Filament\Resources\ApiKeys\Pages\CreateApiKey; use Cachet\Filament\Resources\ApiKeys\Pages\ListApiKeys; +use Cachet\Models\Subscriber; use Filament\Actions\BulkAction; use Filament\Actions\DeleteAction; use Filament\Forms\Components\CheckboxList; @@ -19,6 +20,7 @@ use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Str; use Laravel\Sanctum\PersonalAccessToken; @@ -145,6 +147,10 @@ private static function getAbilities(): array $abilities = []; foreach (Cachet::getResourceApiAbilities() as $resource => $apiAbilities) { + if ($resource === 'subscribers' && Gate::denies('viewAny', Subscriber::class)) { + continue; + } + foreach ($apiAbilities as $ability) { $key = "{$resource}.{$ability}"; $abilities[$key] = Str::headline(__('cachet::api_key.abilities_label', [ diff --git a/src/Filament/Resources/ApiKeys/Pages/CreateApiKey.php b/src/Filament/Resources/ApiKeys/Pages/CreateApiKey.php index d71de14a..b4c822b5 100644 --- a/src/Filament/Resources/ApiKeys/Pages/CreateApiKey.php +++ b/src/Filament/Resources/ApiKeys/Pages/CreateApiKey.php @@ -5,11 +5,13 @@ namespace Cachet\Filament\Resources\ApiKeys\Pages; use Cachet\Filament\Resources\ApiKeys\ApiKeyResource; +use Cachet\Models\Subscriber; use Cachet\Models\User; use Carbon\Carbon; use Filament\Facades\Filament; use Filament\Resources\Pages\CreateRecord; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Facades\Gate; class CreateApiKey extends CreateRecord { @@ -21,10 +23,20 @@ public function handleRecordCreation(array $data): Model { /** @var User $user */ $user = Filament::auth()->user(); + $abilities = $data['abilities'] ?? []; + + if ($abilities === [] || in_array('*', $abilities, true)) { + Gate::forUser($user)->authorize('issueFullAccessApiToken', $user::class); + $abilities = ['*']; + } + + if (collect($abilities)->contains(fn (string $ability): bool => str_starts_with($ability, 'subscribers.'))) { + Gate::forUser($user)->authorize('viewAny', Subscriber::class); + } $token = $user->createToken( name: $data['name'], - abilities: empty($data['abilities']) ? ['*'] : $data['abilities'], + abilities: $abilities, expiresAt: filled($data['expires_at']) ? Carbon::parse($data['expires_at']) : null, ); diff --git a/src/Filament/Resources/Subscribers/SubscriberResource.php b/src/Filament/Resources/Subscribers/SubscriberResource.php index 587576e3..e79b196c 100644 --- a/src/Filament/Resources/Subscribers/SubscriberResource.php +++ b/src/Filament/Resources/Subscribers/SubscriberResource.php @@ -94,12 +94,14 @@ public static function table(Table $table): Table ]) ->recordActions([ Action::make('verify') + ->authorize('update') ->label(__('cachet::subscriber.list.actions.verify_label')) ->color('warning') ->action(fn (Subscriber $record) => $record->verify()) ->requiresConfirmation() ->hidden(fn (Subscriber $record): bool => $record->hasVerifiedEmail()), Action::make('resend-verification') + ->authorize('update') ->label(__('cachet::subscriber.list.actions.resend_verification_label')) ->color('gray') ->action(function (Subscriber $record) { diff --git a/src/Filament/Resources/Users/UserResource.php b/src/Filament/Resources/Users/UserResource.php index 690699c3..935b94a4 100644 --- a/src/Filament/Resources/Users/UserResource.php +++ b/src/Filament/Resources/Users/UserResource.php @@ -21,56 +21,13 @@ use Filament\Tables\Columns\TextColumn; use Filament\Tables\Columns\ToggleColumn; use Filament\Tables\Table; -use Illuminate\Auth\Access\Response; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Hash; class UserResource extends Resource { protected static string|\BackedEnum|null $navigationIcon = Heroicon::OutlinedUsers; - public static function canAccess(): bool - { - return auth()->user()->isAdmin(); - } - - public static function getEditAuthorizationResponse(Model $record): Response - { - if (Cachet::demoMode()) { - return Response::deny(); - } - - if (auth()->user()->is($record)) { - return Response::allow(); - } - - if (auth()->user()->isAdmin()) { - return Response::allow(); - } - - return Response::deny(); - } - - public static function getDeleteAuthorizationResponse(Model $record): Response - { - $response = parent::getDeleteAuthorizationResponse($record); - - if ($response->denied()) { - return $response; - } - - if (! $record instanceof User || auth()->user()->is($record)) { - return Response::deny(); - } - - if ($record->isAdmin() && static::getModel()::query()->where('is_admin', true)->count() <= 1) { - return Response::deny(); - } - - return $response; - } - public static function form(Schema $schema): Schema { return $schema @@ -147,11 +104,13 @@ public static function table(Table $table): Table ->recordActions([ EditAction::make(), Action::make('verify-email') + ->authorize('update') ->label(__('cachet::user.list.actions.verify_email')) ->icon(Heroicon::OutlinedCheckBadge) ->disabled(fn (User $record): bool => $record->hasVerifiedEmail()) ->action(fn (Builder $query, User $record) => $record->sendEmailVerificationNotification()), Action::make('reset-two-factor') + ->authorize('update') ->label(__('cachet::user.list.actions.reset_two_factor')) ->icon(Heroicon::OutlinedShieldExclamation) ->requiresConfirmation() diff --git a/src/Filament/Widgets/Overview.php b/src/Filament/Widgets/Overview.php index b346c2c8..686862fe 100644 --- a/src/Filament/Widgets/Overview.php +++ b/src/Filament/Widgets/Overview.php @@ -11,6 +11,7 @@ use Filament\Widgets\StatsOverviewWidget as BaseWidget; use Filament\Widgets\StatsOverviewWidget\Stat; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Gate; class Overview extends BaseWidget { @@ -18,7 +19,7 @@ class Overview extends BaseWidget protected function getColumns(): int { - return 3; + return Gate::allows('viewAny', Subscriber::class) ? 3 : 2; } protected function getStats(): array @@ -29,7 +30,7 @@ protected function getStats(): array $operationalComponents = (int) $components->operational; $allOperational = $totalComponents === $operationalComponents; - return [ + $stats = [ Stat::make('open_incidents', $openIncidents) ->label(__('cachet::incident.overview.open_incidents_label')) ->description(__('cachet::incident.overview.open_incidents_description')) @@ -46,17 +47,24 @@ protected function getStats(): array ->color($allOperational ? 'success' : 'warning') ->url(ComponentResource::getUrl('index')), - Stat::make('total_subscribers', Subscriber::count()) - ->label(__('cachet::subscriber.overview.total_subscribers_label')) - ->description(__('cachet::subscriber.overview.verified_subscribers_description', [ - 'count' => Subscriber::query()->whereNotNull('email_verified_at')->count(), - ])) - ->chart($this->dailyCounts('subscribers')) - ->icon('cachet-subscribers') - ->chartColor('info') - ->color('gray') - ->url(SubscriberResource::getUrl('index')), ]; + + if (Gate::denies('viewAny', Subscriber::class)) { + return $stats; + } + + $stats[] = Stat::make('total_subscribers', Subscriber::count()) + ->label(__('cachet::subscriber.overview.total_subscribers_label')) + ->description(__('cachet::subscriber.overview.verified_subscribers_description', [ + 'count' => Subscriber::query()->whereNotNull('email_verified_at')->count(), + ])) + ->chart($this->dailyCounts('subscribers')) + ->icon('cachet-subscribers') + ->chartColor('info') + ->color('gray') + ->url(SubscriberResource::getUrl('index')); + + return $stats; } /** diff --git a/src/Http/Controllers/Api/SubscriberController.php b/src/Http/Controllers/Api/SubscriberController.php index 284b9180..f07c3ceb 100644 --- a/src/Http/Controllers/Api/SubscriberController.php +++ b/src/Http/Controllers/Api/SubscriberController.php @@ -16,6 +16,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Illuminate\Routing\Controller; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Number; use Spatie\QueryBuilder\AllowedFilter; use Spatie\QueryBuilder\QueryBuilder; @@ -37,6 +38,7 @@ class SubscriberController extends Controller public function index(Request $request) { $this->guard('subscribers.manage'); + Gate::authorize('viewAny', Subscriber::class); $subscribers = QueryBuilder::for(Subscriber::class) ->allowedIncludes(['components', 'meta']) @@ -57,6 +59,7 @@ public function index(Request $request) public function store(CreateSubscriberRequestData $data, CreateSubscriber $createSubscriberAction) { $this->guard('subscribers.manage'); + Gate::authorize('create', Subscriber::class); $subscriber = $createSubscriberAction->handle( $data->email, @@ -80,6 +83,7 @@ public function store(CreateSubscriberRequestData $data, CreateSubscriber $creat public function show(Subscriber $subscriber) { $this->guard('subscribers.manage'); + Gate::authorize('view', $subscriber); $subscriberQuery = QueryBuilder::for(Subscriber::class) ->allowedIncludes(['components', 'meta']) @@ -96,6 +100,7 @@ public function show(Subscriber $subscriber) public function update(UpdateSubscriberRequestData $data, Subscriber $subscriber, UpdateSubscriber $updateSubscriberAction) { $this->guard('subscribers.manage'); + Gate::authorize('update', $subscriber); $updateSubscriberAction->handle( $subscriber, @@ -114,6 +119,7 @@ public function update(UpdateSubscriberRequestData $data, Subscriber $subscriber public function destroy(Subscriber $subscriber, UnsubscribeSubscriber $unsubscribeSubscriberAction) { $this->guard('subscribers.delete'); + Gate::authorize('delete', $subscriber); $unsubscribeSubscriberAction->handle($subscriber); diff --git a/src/Mcp/Concerns/GuardsMcpAbilities.php b/src/Mcp/Concerns/GuardsMcpAbilities.php index 8b18d4a4..eea84cba 100644 --- a/src/Mcp/Concerns/GuardsMcpAbilities.php +++ b/src/Mcp/Concerns/GuardsMcpAbilities.php @@ -2,6 +2,7 @@ namespace Cachet\Mcp\Concerns; +use Illuminate\Support\Facades\Gate; use Laravel\Mcp\Response; trait GuardsMcpAbilities @@ -16,6 +17,18 @@ protected function tokenCan(string $ability): bool return $user !== null && $user->tokenCan($ability); } + /** + * Determine whether the token ability and its resource policy both allow access. + */ + protected function tokenCanAnd(string $tokenAbility, string $policyAbility, mixed $arguments): bool + { + $user = auth('sanctum')->user(); + + return $user !== null + && $user->tokenCan($tokenAbility) + && Gate::forUser($user)->allows($policyAbility, $arguments); + } + /** * Create the error response returned when the required token ability is missing. */ diff --git a/src/Mcp/Tools/Subscribers/CreateSubscriber.php b/src/Mcp/Tools/Subscribers/CreateSubscriber.php index aefbb306..c62e64da 100644 --- a/src/Mcp/Tools/Subscribers/CreateSubscriber.php +++ b/src/Mcp/Tools/Subscribers/CreateSubscriber.php @@ -6,6 +6,7 @@ use Cachet\Data\Requests\Subscriber\CreateSubscriberRequestData; use Cachet\Mcp\Concerns\GuardsMcpAbilities; use Cachet\Mcp\Concerns\PresentsResources; +use Cachet\Models\Subscriber; use Illuminate\Contracts\JsonSchema\JsonSchema; use Laravel\Mcp\Request; use Laravel\Mcp\Response; @@ -38,7 +39,7 @@ public function schema(JsonSchema $schema): array public function handle(Request $request, CreateSubscriberAction $action): Response|ResponseFactory { - if (! $this->tokenCan('subscribers.manage')) { + if (! $this->tokenCanAnd('subscribers.manage', 'create', Subscriber::class)) { return $this->missingAbility('subscribers.manage'); } @@ -60,6 +61,6 @@ public function handle(Request $request, CreateSubscriberAction $action): Respon public function shouldRegister(): bool { - return $this->tokenCan('subscribers.manage'); + return $this->tokenCanAnd('subscribers.manage', 'viewAny', Subscriber::class); } } diff --git a/src/Mcp/Tools/Subscribers/ListSubscribers.php b/src/Mcp/Tools/Subscribers/ListSubscribers.php index ec5b73ab..6cf71ca5 100644 --- a/src/Mcp/Tools/Subscribers/ListSubscribers.php +++ b/src/Mcp/Tools/Subscribers/ListSubscribers.php @@ -39,7 +39,7 @@ public function schema(JsonSchema $schema): array public function handle(Request $request): Response|ResponseFactory { - if (! $this->tokenCan('subscribers.manage')) { + if (! $this->tokenCanAnd('subscribers.manage', 'viewAny', Subscriber::class)) { return $this->missingAbility('subscribers.manage'); } @@ -57,6 +57,6 @@ public function handle(Request $request): Response|ResponseFactory public function shouldRegister(): bool { - return $this->tokenCan('subscribers.manage'); + return $this->tokenCanAnd('subscribers.manage', 'viewAny', Subscriber::class); } } diff --git a/src/Mcp/Tools/Subscribers/UnsubscribeSubscriber.php b/src/Mcp/Tools/Subscribers/UnsubscribeSubscriber.php index 4204628b..20462e88 100644 --- a/src/Mcp/Tools/Subscribers/UnsubscribeSubscriber.php +++ b/src/Mcp/Tools/Subscribers/UnsubscribeSubscriber.php @@ -32,7 +32,7 @@ public function schema(JsonSchema $schema): array public function handle(Request $request, UnsubscribeSubscriberAction $action): Response { - if (! $this->tokenCan('subscribers.delete')) { + if (! $this->tokenCanAnd('subscribers.delete', 'viewAny', Subscriber::class)) { return $this->missingAbility('subscribers.delete'); } @@ -42,6 +42,10 @@ public function handle(Request $request, UnsubscribeSubscriberAction $action): R return Response::error("Subscriber [{$id}] not found."); } + if (! $this->tokenCanAnd('subscribers.delete', 'delete', $subscriber)) { + return $this->missingAbility('subscribers.delete'); + } + $action->handle($subscriber); return Response::text("Subscriber [{$id}] unsubscribed."); @@ -49,6 +53,6 @@ public function handle(Request $request, UnsubscribeSubscriberAction $action): R public function shouldRegister(): bool { - return $this->tokenCan('subscribers.delete'); + return $this->tokenCanAnd('subscribers.delete', 'viewAny', Subscriber::class); } } diff --git a/src/Mcp/Tools/Subscribers/UpdateSubscriber.php b/src/Mcp/Tools/Subscribers/UpdateSubscriber.php index f545e49c..713ffa21 100644 --- a/src/Mcp/Tools/Subscribers/UpdateSubscriber.php +++ b/src/Mcp/Tools/Subscribers/UpdateSubscriber.php @@ -41,7 +41,7 @@ public function schema(JsonSchema $schema): array public function handle(Request $request, UpdateSubscriberAction $action): Response|ResponseFactory { - if (! $this->tokenCan('subscribers.manage')) { + if (! $this->tokenCanAnd('subscribers.manage', 'viewAny', Subscriber::class)) { return $this->missingAbility('subscribers.manage'); } @@ -51,6 +51,10 @@ public function handle(Request $request, UpdateSubscriberAction $action): Respon return Response::error("Subscriber [{$id}] not found."); } + if (! $this->tokenCanAnd('subscribers.manage', 'update', $subscriber)) { + return $this->missingAbility('subscribers.manage'); + } + $data = UpdateSubscriberRequestData::validateAndCreate($request->only(['email', 'global', 'components'])); $action->handle($subscriber, email: $data->email, global: $data->global, components: $data->components); @@ -60,6 +64,6 @@ public function handle(Request $request, UpdateSubscriberAction $action): Respon public function shouldRegister(): bool { - return $this->tokenCan('subscribers.manage'); + return $this->tokenCanAnd('subscribers.manage', 'viewAny', Subscriber::class); } } diff --git a/src/Models/User.php b/src/Models/User.php index 4ae2d086..3ebe8828 100644 --- a/src/Models/User.php +++ b/src/Models/User.php @@ -84,7 +84,7 @@ public function canAccessPanel(Panel $panel): bool */ public function isAdmin(): bool { - return $this->is_admin; + return (bool) $this->is_admin; } /** diff --git a/src/Policies/SubscriberPolicy.php b/src/Policies/SubscriberPolicy.php new file mode 100644 index 00000000..47a66ba4 --- /dev/null +++ b/src/Policies/SubscriberPolicy.php @@ -0,0 +1,39 @@ +isAdmin(); + } + + public function view(CachetUser $user, Subscriber $subscriber): bool + { + return $user->isAdmin(); + } + + public function create(CachetUser $user): bool + { + return $user->isAdmin(); + } + + public function update(CachetUser $user, Subscriber $subscriber): bool + { + return $user->isAdmin(); + } + + public function delete(CachetUser $user, Subscriber $subscriber): bool + { + return $user->isAdmin(); + } + + public function deleteAny(CachetUser $user): bool + { + return $user->isAdmin(); + } +} diff --git a/src/Policies/UserPolicy.php b/src/Policies/UserPolicy.php new file mode 100644 index 00000000..952b4599 --- /dev/null +++ b/src/Policies/UserPolicy.php @@ -0,0 +1,57 @@ +isAdmin(); + } + + public function view(Model&CachetUser $user, Model&CachetUser $record): bool + { + return $user->is($record) || $user->isAdmin(); + } + + public function create(CachetUser $user): bool + { + return $user->isAdmin(); + } + + public function update(Model&CachetUser $user, Model&CachetUser $record): bool + { + if (Cachet::demoMode()) { + return false; + } + + return $user->is($record) || $user->isAdmin(); + } + + public function delete(Model&CachetUser $user, Model&CachetUser $record): bool + { + if (! $user->isAdmin() || $user->is($record)) { + return false; + } + + if ($record->isAdmin() && $record->newQuery()->where('is_admin', true)->count() <= 1) { + return false; + } + + return true; + } + + public function deleteAny(CachetUser $user): bool + { + return $user->isAdmin(); + } + + public function issueFullAccessApiToken(CachetUser $user): bool + { + return $user->isAdmin(); + } +} diff --git a/tests/Feature/Api/SubscriberTest.php b/tests/Feature/Api/SubscriberTest.php index 69487544..551466ce 100644 --- a/tests/Feature/Api/SubscriberTest.php +++ b/tests/Feature/Api/SubscriberTest.php @@ -21,7 +21,7 @@ }); it('cannot list subscribers without the token ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); Subscriber::factory(2)->create(); @@ -30,8 +30,16 @@ $response->assertForbidden(); }); +it('cannot list subscribers as a non-administrator with a wildcard token', function () { + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + Subscriber::factory(2)->create(); + + getJson('/status/api/subscribers')->assertForbidden(); +}); + it('can list subscribers', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(2)->create(); @@ -42,7 +50,7 @@ }); it('does not list more than 15 subscribers by default', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(20)->create(); @@ -53,7 +61,7 @@ }); it('can list more than 15 subscribers', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(20)->create(); @@ -64,7 +72,7 @@ }); it('can filter subscribers by email', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(5)->create(); Subscriber::factory()->create(['email' => 'james@alt-three.com']); @@ -85,7 +93,7 @@ }); it('cannot get a subscriber without the token ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); $subscriber = Subscriber::factory()->create(); @@ -94,8 +102,16 @@ $response->assertForbidden(); }); +it('cannot get a subscriber as a non-administrator with a wildcard token', function () { + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + $subscriber = Subscriber::factory()->create(); + + getJson('/status/api/subscribers/'.$subscriber->id)->assertForbidden(); +}); + it('can get a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(5)->create(); $subscriber = Subscriber::factory()->create(); @@ -109,7 +125,7 @@ }); it('can get a subscriber with components', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->hasComponents(2)->create(); @@ -120,7 +136,7 @@ }); it('can filter subscribers by meta', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); Subscriber::factory(5)->create(); $subscriber = Subscriber::factory()->create(); @@ -136,7 +152,7 @@ }); it('can include meta on a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->create(); $subscriber->syncMeta(['region' => 'eu-west', 'priority' => 3, 'critical' => true]); @@ -152,7 +168,7 @@ }); it('does not include meta on a subscriber by default', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->create(); $subscriber->syncMeta(['region' => 'eu-west']); @@ -166,7 +182,7 @@ it('can create a subscriber with meta', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $response = postJson('/status/api/subscribers', [ 'email' => 'james@alt-three.com', @@ -187,7 +203,7 @@ }); it('cannot create a subscriber without the token ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); $response = postJson('/status/api/subscribers', [ 'email' => 'james@alt-three.com', @@ -196,10 +212,23 @@ $response->assertForbidden(); }); +it('cannot create a subscriber as a non-administrator with a wildcard token', function () { + Notification::fake(); + + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + postJson('/status/api/subscribers', [ + 'email' => 'hidden@example.com', + ])->assertForbidden(); + + $this->assertDatabaseMissing('subscribers', ['email' => 'hidden@example.com']); + Notification::assertNothingSent(); +}); + it('can create a subscriber', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $response = postJson('/status/api/subscribers', [ 'email' => 'james@alt-three.com', @@ -223,7 +252,7 @@ it('can create a verified subscriber without sending a verification email', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $response = postJson('/status/api/subscribers', [ 'email' => 'james@alt-three.com', @@ -241,7 +270,7 @@ it('can verify and update an existing subscriber', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->create([ 'email' => 'james@alt-three.com', @@ -265,7 +294,7 @@ it('can create a subscriber with component subscriptions', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $components = Component::factory(2)->create(); @@ -287,7 +316,7 @@ }); it('cannot create a subscriber with an invalid email address', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $response = postJson('/status/api/subscribers', [ 'email' => 'not-an-email', @@ -298,7 +327,7 @@ }); it('cannot create a subscriber with components that do not exist', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $response = postJson('/status/api/subscribers', [ 'email' => 'james@alt-three.com', @@ -320,7 +349,7 @@ }); it('cannot update a subscriber without the token ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); $subscriber = Subscriber::factory()->create(); @@ -331,8 +360,20 @@ $response->assertForbidden(); }); +it('cannot update a subscriber as a non-administrator with a wildcard token', function () { + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + $subscriber = Subscriber::factory()->create(['email' => 'hidden@example.com']); + + putJson('/status/api/subscribers/'.$subscriber->id, [ + 'email' => 'exposed@example.com', + ])->assertForbidden(); + + expect($subscriber->fresh()->email)->toBe('hidden@example.com'); +}); + it('can update a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->verified()->create(); @@ -352,7 +393,7 @@ }); it('can update a subscriber component subscriptions', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->hasComponents(1)->create(); $components = Component::factory(2)->create(); @@ -367,7 +408,7 @@ }); it('does not detach component subscriptions when components are omitted from an update', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->hasComponents(2)->create(); @@ -389,7 +430,7 @@ }); it('cannot delete a subscriber without the token ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); $subscriber = Subscriber::factory()->create(); @@ -398,8 +439,18 @@ $response->assertForbidden(); }); +it('cannot delete a subscriber as a non-administrator with a wildcard token', function () { + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + $subscriber = Subscriber::factory()->create(); + + deleteJson('/status/api/subscribers/'.$subscriber->id)->assertForbidden(); + + expect($subscriber->fresh())->not->toBeNull(); +}); + it('can delete a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.delete']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.delete']); $subscriber = Subscriber::factory()->hasComponents(2)->create(); diff --git a/tests/Feature/Filament/DashboardTest.php b/tests/Feature/Filament/DashboardTest.php index 4e9413a9..0091b070 100644 --- a/tests/Feature/Filament/DashboardTest.php +++ b/tests/Feature/Filament/DashboardTest.php @@ -93,6 +93,7 @@ public function dailyIncidentCounts(): array it('links overview stats to their relevant resources', function () { Filament::setCurrentPanel(Filament::getPanel('cachet')); + actingAs(User::factory()->create(['is_admin' => true])); $overview = new class extends Overview { diff --git a/tests/Feature/Filament/Resources/ApiKeyResourceTest.php b/tests/Feature/Filament/Resources/ApiKeyResourceTest.php new file mode 100644 index 00000000..47e0f7df --- /dev/null +++ b/tests/Feature/Filament/Resources/ApiKeyResourceTest.php @@ -0,0 +1,96 @@ +create(['is_admin' => true]); + + actingAs($admin); + + (new CreateApiKey)->handleRecordCreation([ + 'name' => 'Full access', + 'abilities' => [], + 'expires_at' => null, + ]); + + expect($admin->tokens()->sole()->abilities)->toBe(['*']); +}); + +it('prevents non-administrators from creating full access tokens', function () { + $user = User::factory()->create(['is_admin' => false]); + + actingAs($user); + + expect(fn () => (new CreateApiKey)->handleRecordCreation([ + 'name' => 'Full access', + 'abilities' => [], + 'expires_at' => null, + ]))->toThrow(AuthorizationException::class); + + expect($user->tokens()->exists())->toBeFalse(); +}); + +it('prevents non-administrators from submitting a wildcard ability', function () { + $user = User::factory()->create(['is_admin' => false]); + + actingAs($user); + + expect(fn () => (new CreateApiKey)->handleRecordCreation([ + 'name' => 'Submitted wildcard', + 'abilities' => ['*'], + 'expires_at' => null, + ]))->toThrow(AuthorizationException::class); + + expect($user->tokens()->exists())->toBeFalse(); +}); + +it('prevents non-administrators from creating subscriber tokens', function () { + $user = User::factory()->create(['is_admin' => false]); + + actingAs($user); + + expect(fn () => (new CreateApiKey)->handleRecordCreation([ + 'name' => 'Subscriber access', + 'abilities' => ['subscribers.manage'], + 'expires_at' => null, + ]))->toThrow(AuthorizationException::class); + + expect($user->tokens()->exists())->toBeFalse(); +}); + +it('allows non-administrators to create scoped operational tokens', function () { + $user = User::factory()->create(['is_admin' => false]); + + actingAs($user); + + (new CreateApiKey)->handleRecordCreation([ + 'name' => 'Incident access', + 'abilities' => ['incidents.manage'], + 'expires_at' => null, + ]); + + expect($user->tokens()->sole()->abilities)->toBe(['incidents.manage']); +}); + +it('hides subscriber abilities from non-administrators', function () { + actingAs(User::factory()->create(['is_admin' => false])); + + livewire(CreateApiKey::class) + ->assertFormFieldExists('abilities', fn (CheckboxList $field): bool => ! array_key_exists( + 'subscribers.manage', + $field->getOptions(), + )); +}); diff --git a/tests/Feature/Filament/Resources/SubscriberResourceTest.php b/tests/Feature/Filament/Resources/SubscriberResourceTest.php index c35990ac..542c4503 100644 --- a/tests/Feature/Filament/Resources/SubscriberResourceTest.php +++ b/tests/Feature/Filament/Resources/SubscriberResourceTest.php @@ -5,6 +5,7 @@ use Cachet\Events\Subscribers\SubscriberVerified; use Cachet\Filament\Resources\Subscribers\Pages\CreateSubscriber; use Cachet\Filament\Resources\Subscribers\Pages\ListSubscribers; +use Cachet\Filament\Resources\Subscribers\SubscriberResource; use Cachet\Models\Subscriber; use Cachet\Notifications\VerifySubscriberEmail; use Filament\Actions\Testing\TestAction; @@ -85,3 +86,10 @@ livewire(ListSubscribers::class) ->assertActionHidden(TestAction::make('resend-verification')->table($subscriber)); }); + +it('denies non-administrators direct access to subscribers', function () { + actingAs(User::factory()->create(['is_admin' => false])); + + $this->get(SubscriberResource::getUrl('index')) + ->assertForbidden(); +}); diff --git a/tests/Feature/Filament/Resources/UserResourceTest.php b/tests/Feature/Filament/Resources/UserResourceTest.php index 74c2f9e8..0437bb5f 100644 --- a/tests/Feature/Filament/Resources/UserResourceTest.php +++ b/tests/Feature/Filament/Resources/UserResourceTest.php @@ -61,3 +61,10 @@ expect($this->admin->fresh())->not->toBeNull() ->and($otherAdmin->fresh())->toBeNull(); }); + +it('denies non-administrators direct access to users', function () { + actingAs(User::factory()->create(['is_admin' => false])); + + $this->get(UserResource::getUrl('index')) + ->assertForbidden(); +}); diff --git a/tests/Feature/Filament/Widgets/OverviewTest.php b/tests/Feature/Filament/Widgets/OverviewTest.php index 7a0cfa6d..cacea7b3 100644 --- a/tests/Feature/Filament/Widgets/OverviewTest.php +++ b/tests/Feature/Filament/Widgets/OverviewTest.php @@ -8,7 +8,9 @@ use Cachet\Models\Component; use Cachet\Models\Incident; use Cachet\Models\Subscriber; +use Workbench\App\User; +use function Pest\Laravel\actingAs; use function Pest\Livewire\livewire; it('overview smoke test', function () { @@ -58,6 +60,8 @@ }); it('shows the total and verified subscriber counts', function () { + actingAs(User::factory()->create(['is_admin' => true])); + Subscriber::factory()->count(2)->verified()->create(); Subscriber::factory()->create(); @@ -69,3 +73,13 @@ $component->assertSee('3'); $component->assertSee(__('cachet::subscriber.overview.verified_subscribers_description', ['count' => 2])); }); + +it('hides subscriber counts from non-administrators', function () { + actingAs(User::factory()->create(['is_admin' => false])); + + Subscriber::factory()->create(); + + livewire(Overview::class) + ->assertSuccessful() + ->assertDontSee(__('cachet::subscriber.overview.total_subscribers_label')); +}); diff --git a/tests/Feature/Mcp/McpSettingsTest.php b/tests/Feature/Mcp/McpSettingsTest.php index 571edfd3..a3aefa8f 100644 --- a/tests/Feature/Mcp/McpSettingsTest.php +++ b/tests/Feature/Mcp/McpSettingsTest.php @@ -120,7 +120,7 @@ function enableMcp(bool $protected = true): void it('exposes every tool to tokens with all abilities', function () { enableMcp(protected: false); - Sanctum::actingAs(User::factory()->create(), ['*']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['*']); $tools = collect(postJson('/status/mcp', mcpRequest('tools/list', ['per_page' => 50])) ->assertOk() @@ -130,6 +130,23 @@ function enableMcp(bool $protected = true): void expect($tools)->toHaveCount(41); }); +it('hides subscriber tools from non-administrators with all abilities', function () { + enableMcp(protected: false); + + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + $tools = collect(postJson('/status/mcp', mcpRequest('tools/list', ['per_page' => 50])) + ->assertOk() + ->json('result.tools')) + ->pluck('name'); + + expect($tools) + ->not->toContain('list_subscribers') + ->not->toContain('create_subscriber') + ->not->toContain('update_subscriber') + ->not->toContain('unsubscribe_subscriber'); +}); + it('does not allow guests to call write tools', function () { enableMcp(protected: false); diff --git a/tests/Feature/Mcp/Tools/SubscriberToolsTest.php b/tests/Feature/Mcp/Tools/SubscriberToolsTest.php index 473ae309..781d3f6f 100644 --- a/tests/Feature/Mcp/Tools/SubscriberToolsTest.php +++ b/tests/Feature/Mcp/Tools/SubscriberToolsTest.php @@ -22,7 +22,7 @@ }); it('hides the subscribers list from tokens without the ability', function () { - Sanctum::actingAs(User::factory()->create()); + Sanctum::actingAs(User::factory()->create(['is_admin' => true])); Subscriber::factory(2)->create(); @@ -30,8 +30,29 @@ ->assertHasErrors(); }); +it('hides subscriber tools from non-administrators with wildcard tokens', function () { + Notification::fake(); + + Sanctum::actingAs(User::factory()->create(['is_admin' => false]), ['*']); + + $subscriber = Subscriber::factory()->create(['email' => 'hidden@example.com']); + + CachetServer::tool(ListSubscribers::class)->assertHasErrors(); + CachetServer::tool(CreateSubscriber::class, ['email' => 'new@example.com'])->assertHasErrors(); + CachetServer::tool(UpdateSubscriber::class, [ + 'id' => $subscriber->id, + 'email' => 'exposed@example.com', + ])->assertHasErrors(); + CachetServer::tool(UnsubscribeSubscriber::class, ['id' => $subscriber->id])->assertHasErrors(); + + expect($subscriber->fresh()->email)->toBe('hidden@example.com') + ->and(Subscriber::query()->where('email', 'new@example.com')->exists())->toBeFalse(); + + Notification::assertNothingSent(); +}); + it('lists subscribers with the ability', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $components = Component::factory(2)->create(); $components->each->syncTags(['Public API']); @@ -52,7 +73,7 @@ it('creates a subscriber and sends a verification email', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); CachetServer::tool(CreateSubscriber::class, ['email' => 'james@example.com']) ->assertOk() @@ -68,7 +89,7 @@ it('creates a verified subscriber without sending a verification email', function () { Notification::fake(); - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); CachetServer::tool(CreateSubscriber::class, [ 'email' => 'james@example.com', @@ -79,14 +100,14 @@ }); it('validates the subscriber email', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); CachetServer::tool(CreateSubscriber::class, ['email' => 'not-an-email']) ->assertHasErrors(); }); it('updates a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.manage']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.manage']); $subscriber = Subscriber::factory()->create(['email' => 'old@example.com']); @@ -99,7 +120,7 @@ }); it('unsubscribes a subscriber', function () { - Sanctum::actingAs(User::factory()->create(), ['subscribers.delete']); + Sanctum::actingAs(User::factory()->create(['is_admin' => true]), ['subscribers.delete']); $subscriber = Subscriber::factory()->create(); diff --git a/tests/Feature/Policies/SubscriberPolicyTest.php b/tests/Feature/Policies/SubscriberPolicyTest.php new file mode 100644 index 00000000..446113db --- /dev/null +++ b/tests/Feature/Policies/SubscriberPolicyTest.php @@ -0,0 +1,23 @@ +create(['is_admin' => true]); + $user = User::factory()->create(['is_admin' => false]); + $subscriber = Subscriber::factory()->create(); + + foreach (['viewAny', 'create'] as $ability) { + expect(Gate::forUser($admin)->allows($ability, Subscriber::class))->toBeTrue() + ->and(Gate::forUser($user)->allows($ability, Subscriber::class))->toBeFalse(); + } + + foreach (['view', 'update', 'delete'] as $ability) { + expect(Gate::forUser($admin)->allows($ability, $subscriber))->toBeTrue() + ->and(Gate::forUser($user)->allows($ability, $subscriber))->toBeFalse(); + } +}); diff --git a/tests/Feature/Policies/UserPolicyTest.php b/tests/Feature/Policies/UserPolicyTest.php new file mode 100644 index 00000000..fed90cb3 --- /dev/null +++ b/tests/Feature/Policies/UserPolicyTest.php @@ -0,0 +1,54 @@ +create(['is_admin' => true]); + $user = User::factory()->create(['is_admin' => false]); + $record = User::factory()->create(); + + expect(Gate::forUser($admin)->allows('viewAny', CachetUser::class))->toBeTrue() + ->and(Gate::forUser($admin)->allows('view', $record))->toBeTrue() + ->and(Gate::forUser($admin)->allows('create', CachetUser::class))->toBeTrue() + ->and(Gate::forUser($admin)->allows('update', $record))->toBeTrue() + ->and(Gate::forUser($admin)->allows('delete', $record))->toBeTrue() + ->and(Gate::forUser($user)->allows('viewAny', CachetUser::class))->toBeFalse() + ->and(Gate::forUser($user)->allows('create', CachetUser::class))->toBeFalse() + ->and(Gate::forUser($user)->allows('update', $record))->toBeFalse() + ->and(Gate::forUser($user)->allows('delete', $record))->toBeFalse(); +}); + +it('allows non-administrators to view only themselves', function () { + $user = User::factory()->create(['is_admin' => false]); + $otherUser = User::factory()->create(['is_admin' => false]); + + expect(Gate::forUser($user)->allows('view', $user))->toBeTrue() + ->and(Gate::forUser($user)->allows('view', $otherUser))->toBeFalse(); +}); + +it('prevents administrators from deleting themselves', function () { + $admin = User::factory()->create(['is_admin' => true]); + + expect(Gate::forUser($admin)->allows('delete', $admin))->toBeFalse(); +}); + +it('prevents user updates in demo mode', function () { + config()->set('cachet.demo_mode', true); + + $admin = User::factory()->create(['is_admin' => true]); + $record = User::factory()->create(); + + expect(Gate::forUser($admin)->allows('update', $record))->toBeFalse(); +}); + +it('allows only administrators to issue full access tokens', function () { + $admin = User::factory()->create(['is_admin' => true]); + $user = User::factory()->create(['is_admin' => false]); + + expect(Gate::forUser($admin)->allows('issueFullAccessApiToken', CachetUser::class))->toBeTrue() + ->and(Gate::forUser($user)->allows('issueFullAccessApiToken', CachetUser::class))->toBeFalse(); +});