From fb854a7cff936b25f552f3705e181c2a6fc7414c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:14:02 +0100 Subject: [PATCH] fix(access): hide shared variables and actions that a member cannot use - Check read access on the project or environment for shared variables pages, the canvas, and the sidebar, not only navigation access - Require team-wide read for the server shared variables canvas API - Open the environment variable editor read-only when the value is hidden - Show the integration tokens button only to users who can open it - Show disabled confirmation buttons as disabled, not as danger buttons --- .../Api/SharedVariableCanvasController.php | 6 ++--- .../Shared/EnvironmentVariable/Show.php | 5 ++-- app/Livewire/SharedVariables/Canvas.php | 2 +- .../SharedVariables/Environment/Index.php | 8 +++++- .../SharedVariables/Environment/Show.php | 3 ++- .../SharedVariables/Project/Index.php | 4 ++- app/Livewire/SharedVariables/Project/Show.php | 3 ++- app/Policies/EnvironmentPolicy.php | 9 +++++++ app/Policies/ProjectPolicy.php | 9 +++++++ app/Policies/ServerPolicy.php | 10 +++++++ app/Policies/TeamPolicy.php | 10 +++++++ resources/css/app.css | 2 +- .../components/modal-confirmation.blade.php | 4 +-- resources/views/components/navbar.blade.php | 16 +++++++++--- .../shared/secret-manager-links.blade.php | 26 ++++++++++++------- .../environment/index.blade.php | 2 +- .../shared-variables/project/index.blade.php | 2 +- tests/Feature/Access/AccessPoliciesTest.php | 4 ++- .../ApplicationConfigAuthorizationTest.php | 3 ++- 19 files changed, 98 insertions(+), 30 deletions(-) diff --git a/app/Http/Controllers/Api/SharedVariableCanvasController.php b/app/Http/Controllers/Api/SharedVariableCanvasController.php index 718dc6bb76..5c4483bbef 100644 --- a/app/Http/Controllers/Api/SharedVariableCanvasController.php +++ b/app/Http/Controllers/Api/SharedVariableCanvasController.php @@ -37,7 +37,7 @@ class SharedVariableCanvasController extends Controller if ($owner instanceof JsonResponse) { return $owner; } - $this->authorize('view', $owner); + $this->authorize('viewSharedVariables', $owner); return response()->json($this->canvas->payload($scope, $owner, $this->canEdit($request, $scope, $owner))); } @@ -49,7 +49,7 @@ class SharedVariableCanvasController extends Controller if ($owner instanceof JsonResponse) { return $owner; } - $this->authorize('view', $owner); + $this->authorize('viewSharedVariables', $owner); $return = validateIncomingRequest($request); if ($return instanceof JsonResponse) { @@ -108,7 +108,7 @@ class SharedVariableCanvasController extends Controller if ($owner instanceof JsonResponse) { return $owner; } - $this->authorize('view', $owner); + $this->authorize('viewSharedVariables', $owner); [$source, $target] = array_pad(explode('|', (string) $request->route('link_id'), 2), 2, ''); $resource = $this->canvas->resources($scope, $owner)->get($target); diff --git a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php index a19693a4a5..185d7b10b1 100644 --- a/app/Livewire/Project/Shared/EnvironmentVariable/Show.php +++ b/app/Livewire/Project/Shared/EnvironmentVariable/Show.php @@ -164,9 +164,10 @@ class Show extends Component */ public function loadValues(): void { - $this->authorize('viewSensitive', $this->env); + $this->authorize('view', $this->env); - if ($this->valuesLoaded) { + // Users without viewSensitive open the editor with the value hidden; nothing to decrypt. + if ($this->valuesLoaded || $this->valuesHiddenForUser()) { return; } diff --git a/app/Livewire/SharedVariables/Canvas.php b/app/Livewire/SharedVariables/Canvas.php index 44304cafce..920ba01d08 100644 --- a/app/Livewire/SharedVariables/Canvas.php +++ b/app/Livewire/SharedVariables/Canvas.php @@ -59,7 +59,7 @@ class Canvas extends Component default => 'team', }; $owner = $this->owner(); - $this->authorize('view', $owner); + $this->authorize('viewSharedVariables', $owner); $this->endpoint = $this->scope === 'team' ? '/shared-variables/team/canvas' diff --git a/app/Livewire/SharedVariables/Environment/Index.php b/app/Livewire/SharedVariables/Environment/Index.php index 6685c5c99d..f1d4ed22c0 100644 --- a/app/Livewire/SharedVariables/Environment/Index.php +++ b/app/Livewire/SharedVariables/Environment/Index.php @@ -2,6 +2,7 @@ namespace App\Livewire\SharedVariables\Environment; +use App\Models\Environment; use App\Models\Project; use Illuminate\Support\Collection; use Livewire\Component; @@ -12,7 +13,12 @@ class Index extends Component public function mount() { - $this->projects = Project::ownedByCurrentTeamCached(); + $this->projects = Project::ownedByCurrentTeamCached() + ->each(fn (Project $project) => $project->setRelation('environments', $project->environments + ->filter(fn (Environment $environment) => auth()->user()->can('viewSharedVariables', $environment)) + ->values())) + ->filter(fn (Project $project) => $project->environments->isNotEmpty()) + ->values(); } public function render() diff --git a/app/Livewire/SharedVariables/Environment/Show.php b/app/Livewire/SharedVariables/Environment/Show.php index 5e22376584..1c6a3201cb 100644 --- a/app/Livewire/SharedVariables/Environment/Show.php +++ b/app/Livewire/SharedVariables/Environment/Show.php @@ -62,6 +62,7 @@ class Show extends Component $this->project = Project::ownedByCurrentTeam()->where('uuid', $projectUuid)->firstOrFail(); $this->environment = $this->project->environments()->where('uuid', $environmentUuid)->firstOrFail(); + $this->authorize('viewSharedVariables', $this->environment); if ($this->prefersCanvasView('environment_variables')) { return $this->redirectRoute('shared-variables.environment.canvas', ['project_uuid' => $this->project->uuid, 'environment_uuid' => $this->environment->uuid], navigate: true); } @@ -70,7 +71,7 @@ class Show extends Component public function switch() { - $this->authorize('view', $this->environment); + $this->authorize('viewSharedVariables', $this->environment); $this->view = $this->view === 'normal' ? 'dev' : 'normal'; $this->getDevView(); } diff --git a/app/Livewire/SharedVariables/Project/Index.php b/app/Livewire/SharedVariables/Project/Index.php index 58929bade7..5490e6129b 100644 --- a/app/Livewire/SharedVariables/Project/Index.php +++ b/app/Livewire/SharedVariables/Project/Index.php @@ -12,7 +12,9 @@ class Index extends Component public function mount() { - $this->projects = Project::ownedByCurrentTeamCached(); + $this->projects = Project::ownedByCurrentTeamCached() + ->filter(fn (Project $project) => auth()->user()->can('viewSharedVariables', $project)) + ->values(); } public function render() diff --git a/app/Livewire/SharedVariables/Project/Show.php b/app/Livewire/SharedVariables/Project/Show.php index b2973291ca..a823b1ead5 100644 --- a/app/Livewire/SharedVariables/Project/Show.php +++ b/app/Livewire/SharedVariables/Project/Show.php @@ -55,6 +55,7 @@ class Show extends Component if (! $project) { return redirect()->route('dashboard'); } + $this->authorize('viewSharedVariables', $project); $this->project = $project; if ($this->prefersCanvasView('project_variables')) { return $this->redirectRoute('shared-variables.project.canvas', ['project_uuid' => $project->uuid], navigate: true); @@ -65,7 +66,7 @@ class Show extends Component public function switch() { try { - $this->authorize('view', $this->project); + $this->authorize('viewSharedVariables', $this->project); $this->view = $this->view === 'normal' ? 'dev' : 'normal'; $this->getDevView(); } catch (\Throwable $e) { diff --git a/app/Policies/EnvironmentPolicy.php b/app/Policies/EnvironmentPolicy.php index 1bfd47f8d2..aee3c2ca33 100644 --- a/app/Policies/EnvironmentPolicy.php +++ b/app/Policies/EnvironmentPolicy.php @@ -26,6 +26,15 @@ class EnvironmentPolicy return app(AccessResolver::class)->canOpen($user, $environment); } + /** + * Determine whether the user can read the shared variables of the environment. + * Opening an environment for navigation is not enough; this needs read access on the environment. + */ + public function viewSharedVariables(User $user, Environment $environment): bool + { + return $user->canAccess(AccessLevel::Read, $environment); + } + /** * Determine whether the user can create environments, in the project when one is given. */ diff --git a/app/Policies/ProjectPolicy.php b/app/Policies/ProjectPolicy.php index 687c5e167b..ef52995c41 100644 --- a/app/Policies/ProjectPolicy.php +++ b/app/Policies/ProjectPolicy.php @@ -25,6 +25,15 @@ class ProjectPolicy return app(AccessResolver::class)->canOpen($user, $project); } + /** + * Determine whether the user can read the shared variables of the project. + * Opening a project for navigation is not enough; this needs read access on the project. + */ + public function viewSharedVariables(User $user, Project $project): bool + { + return $user->canAccess(AccessLevel::Read, $project); + } + /** * Determine whether the user can create projects in the current team. */ diff --git a/app/Policies/ServerPolicy.php b/app/Policies/ServerPolicy.php index 2d24355485..67e76ba45a 100644 --- a/app/Policies/ServerPolicy.php +++ b/app/Policies/ServerPolicy.php @@ -2,8 +2,10 @@ namespace App\Policies; +use App\Enums\AccessLevel; use App\Models\Server; use App\Models\User; +use App\Services\Access\AccessResolver; class ServerPolicy { @@ -23,6 +25,14 @@ class ServerPolicy return $user->teams->contains('id', $server->team_id); } + /** + * Determine whether the user can read the shared variables of the server (team-wide read). + */ + public function viewSharedVariables(User $user, Server $server): bool + { + return app(AccessResolver::class)->allowsTeamWide($user, AccessLevel::Read, $server->team_id); + } + /** * Determine whether the user can create models. */ diff --git a/app/Policies/TeamPolicy.php b/app/Policies/TeamPolicy.php index 5b97927601..a17fda20f8 100644 --- a/app/Policies/TeamPolicy.php +++ b/app/Policies/TeamPolicy.php @@ -2,8 +2,10 @@ namespace App\Policies; +use App\Enums\AccessLevel; use App\Models\Team; use App\Models\User; +use App\Services\Access\AccessResolver; class TeamPolicy { @@ -23,6 +25,14 @@ class TeamPolicy return $user->teams->contains('id', $team->id); } + /** + * Determine whether the user can read the team shared variables (team-wide read). + */ + public function viewSharedVariables(User $user, Team $team): bool + { + return app(AccessResolver::class)->allowsTeamWide($user, AccessLevel::Read, $team->id); + } + /** * Determine whether the user can create models. */ diff --git a/resources/css/app.css b/resources/css/app.css index 85995a8452..33cb50af21 100644 --- a/resources/css/app.css +++ b/resources/css/app.css @@ -515,7 +515,7 @@ option { } .button.button-error:disabled { - @apply text-red-800 dark:text-red-300 bg-red-50 dark:bg-red-900/30 border-red-300 dark:border-red-800; + @apply text-neutral-300 dark:text-fg-faint bg-transparent dark:bg-transparent border-neutral-200 dark:border-white/[0.06]; } .button-error:not(:disabled) { diff --git a/resources/views/components/modal-confirmation.blade.php b/resources/views/components/modal-confirmation.blade.php index 50e41d72df..454bd972cb 100644 --- a/resources/views/components/modal-confirmation.blade.php +++ b/resources/views/components/modal-confirmation.blade.php @@ -173,11 +173,11 @@ @else @if ($disabled) @if ($buttonFullWidth) - + {{ $resolvedButtonTitle }} @else - + {{ $resolvedButtonTitle }} @endif diff --git a/resources/views/components/navbar.blade.php b/resources/views/components/navbar.blade.php index 1034c28083..35d2731f17 100644 --- a/resources/views/components/navbar.blade.php +++ b/resources/views/components/navbar.blade.php @@ -109,6 +109,10 @@ @php // Members limited to some projects only see project pages; team-level pages return 403 for them. $teamWide = app(App\Services\Access\AccessResolver::class)->coversWholeTeam(auth()->user(), currentTeam()->id); + // Project and environment variables need read access there; opening a parent of a granted resource is not enough. + $readableProjects = $teamWide ? null : App\Models\Project::ownedByCurrentTeamCached(); + $canReadProjectVariables = $teamWide || $readableProjects->contains(fn ($project) => auth()->user()->can('viewSharedVariables', $project)); + $canReadEnvironmentVariables = $teamWide || $readableProjects->contains(fn ($project) => $project->environments->contains(fn ($environment) => auth()->user()->can('viewSharedVariables', $environment))); // An entry with children opens a submenu in place of the main menu (back button + its pages). $navSections = collect([ 'Workspace' => [ @@ -131,8 +135,8 @@ ['label' => 'Shared Variables', 'icon' => 'variables', 'children' => [ ['label' => 'Overview', 'icon' => 'dashboard', 'href' => route('shared-variables.index'), 'active' => request()->is('shared-variables'), 'visible' => $teamWide], ['label' => 'Team', 'icon' => 'teams', 'href' => route('shared-variables.team.index'), 'active' => request()->is('shared-variables/team*'), 'visible' => $teamWide], - ['label' => 'Projects', 'icon' => 'projects', 'href' => route('shared-variables.project.index'), 'active' => request()->is('shared-variables/projects', 'shared-variables/project/*')], - ['label' => 'Environments', 'icon' => 'layers', 'href' => route('shared-variables.environment.index'), 'active' => request()->is('shared-variables/environments*')], + ['label' => 'Projects', 'icon' => 'projects', 'href' => route('shared-variables.project.index'), 'active' => request()->is('shared-variables/projects', 'shared-variables/project/*'), 'visible' => $canReadProjectVariables], + ['label' => 'Environments', 'icon' => 'layers', 'href' => route('shared-variables.environment.index'), 'active' => request()->is('shared-variables/environments*'), 'visible' => $canReadEnvironmentVariables], ['label' => 'Servers', 'icon' => 'servers', 'href' => route('shared-variables.server.index'), 'active' => request()->is('shared-variables/servers', 'shared-variables/server/*'), 'visible' => $teamWide], ]], ], @@ -178,9 +182,13 @@ } $children = array_values(array_filter($item['children'], fn (array $child): bool => (bool) ($child['visible'] ?? true))); - return [...$item, 'children' => $children, 'href' => $children[0]['href'], 'active' => collect($children)->contains('active', true)]; + return [...$item, 'children' => $children, 'href' => $children[0]['href'] ?? null, 'active' => collect($children)->contains('active', true)]; }) - ->all()); + // A parent whose children are all hidden has nothing to open. + ->reject(fn (array $item): bool => isset($item['children']) && $item['children'] === []) + ->values() + ->all()) + ->reject(fn (array $items): bool => $items === []); $activeSubmenu = $navSections->flatten(1)->first(fn (array $item): bool => isset($item['children']) && $item['active']); // Resource menus by depth, registered by registerMainNavContext() (e.g. from // components/main-nav-menu.blade.php). Each one teleports itself into #main-nav-context-{depth} diff --git a/resources/views/livewire/project/shared/secret-manager-links.blade.php b/resources/views/livewire/project/shared/secret-manager-links.blade.php index 447184d12e..4d124a2fcb 100644 --- a/resources/views/livewire/project/shared/secret-manager-links.blade.php +++ b/resources/views/livewire/project/shared/secret-manager-links.blade.php @@ -6,15 +6,20 @@ @if (! $link && $availableTokens->isEmpty()) - - - - Integration tokens - - - + @can('viewAny', \App\Models\IntegrationToken::class) + + + + Integration tokens + + + + @else + + @endcan @else @can('update', $resource)
@@ -130,6 +135,9 @@ {{ $link->integrationToken->name }} · {{ $link->sourceSummary() }}
+ @else + @endif @endcan @endif diff --git a/resources/views/livewire/shared-variables/environment/index.blade.php b/resources/views/livewire/shared-variables/environment/index.blade.php index ed8e38407a..8ff42f6292 100644 --- a/resources/views/livewire/shared-variables/environment/index.blade.php +++ b/resources/views/livewire/shared-variables/environment/index.blade.php @@ -11,7 +11,7 @@ } }"> @if ($projects->isEmpty()) - + @else diff --git a/resources/views/livewire/shared-variables/project/index.blade.php b/resources/views/livewire/shared-variables/project/index.blade.php index e1ac890a52..8d118eb1fd 100644 --- a/resources/views/livewire/shared-variables/project/index.blade.php +++ b/resources/views/livewire/shared-variables/project/index.blade.php @@ -11,7 +11,7 @@ } }"> @if ($projects->isEmpty()) - @else diff --git a/tests/Feature/Access/AccessPoliciesTest.php b/tests/Feature/Access/AccessPoliciesTest.php index 937e3bad2d..d1e6aadef7 100644 --- a/tests/Feature/Access/AccessPoliciesTest.php +++ b/tests/Feature/Access/AccessPoliciesTest.php @@ -215,7 +215,9 @@ it('shows secret values in the variable editor only with read-sensitive access', if ($expectedValue === null) { expect($component->get('isValueHidden'))->toBeTrue() ->and($component->get('value'))->toBeNull(); - $component->call('loadValues')->assertForbidden(); + $component->call('loadValues')->assertOk(); + expect($component->get('value'))->toBeNull() + ->and($component->get('valuesLoaded'))->toBeFalse(); return; } diff --git a/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php b/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php index 017a865bee..a937935d5d 100644 --- a/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php +++ b/tests/Feature/Authorization/ApplicationConfigAuthorizationTest.php @@ -220,8 +220,9 @@ test('member cannot load application environment variable values', function () { 'type' => 'application', ]) ->call('loadValues') - ->assertForbidden() + ->assertOk() ->assertSet('valuesLoaded', false) + ->assertSet('isValueHidden', true) ->assertSet('value', null); });