diff --git a/app/Livewire/Security/PrivateKey/Index.php b/app/Livewire/Security/PrivateKey/Index.php index 950ec152d..1eb66ae3e 100644 --- a/app/Livewire/Security/PrivateKey/Index.php +++ b/app/Livewire/Security/PrivateKey/Index.php @@ -12,7 +12,7 @@ class Index extends Component public function render() { - $privateKeys = PrivateKey::ownedByCurrentTeam(['name', 'uuid', 'is_git_related', 'description'])->get(); + $privateKeys = PrivateKey::ownedByCurrentTeam(['name', 'uuid', 'is_git_related', 'description', 'team_id'])->get(); return view('livewire.security.private-key.index', [ 'privateKeys' => $privateKeys, diff --git a/app/Livewire/Security/PrivateKey/Show.php b/app/Livewire/Security/PrivateKey/Show.php index 9928cfe97..c292d14a3 100644 --- a/app/Livewire/Security/PrivateKey/Show.php +++ b/app/Livewire/Security/PrivateKey/Show.php @@ -79,8 +79,14 @@ private function syncData(bool $toModel = false): void public function mount() { try { - $this->private_key = PrivateKey::ownedByCurrentTeam(['name', 'description', 'private_key', 'is_git_related'])->whereUuid(request()->private_key_uuid)->firstOrFail(); + $this->private_key = PrivateKey::ownedByCurrentTeam(['name', 'description', 'private_key', 'is_git_related', 'team_id'])->whereUuid(request()->private_key_uuid)->firstOrFail(); + + // Explicit authorization check - will throw 403 if not authorized + $this->authorize('view', $this->private_key); + $this->syncData(false); + } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + abort(403, 'You do not have permission to view this private key.'); } catch (\Throwable) { abort(404); } diff --git a/app/Models/PrivateKey.php b/app/Models/PrivateKey.php index 08f3f1ebd..c5cbc6338 100644 --- a/app/Models/PrivateKey.php +++ b/app/Models/PrivateKey.php @@ -82,9 +82,10 @@ public function getPublicKey() public static function ownedByCurrentTeam(array $select = ['*']) { + $teamId = currentTeam()->id; $selectArray = collect($select)->concat(['id']); - return self::whereTeamId(currentTeam()->id)->select($selectArray->all()); + return self::whereTeamId($teamId)->select($selectArray->all()); } public static function validatePrivateKey($privateKey) diff --git a/app/Models/User.php b/app/Models/User.php index 9ab9fefe9..f04b6fa77 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -338,6 +338,39 @@ public function role() return data_get($user, 'pivot.role'); } + /** + * Check if the user is an admin or owner of a specific team + */ + public function isAdminOfTeam(int $teamId): bool + { + $team = $this->teams->where('id', $teamId)->first(); + + if (! $team) { + return false; + } + + $role = $team->pivot->role ?? null; + + return $role === 'admin' || $role === 'owner'; + } + + /** + * Check if the user can access system resources (team_id=0) + * Must be admin/owner of root team + */ + public function canAccessSystemResources(): bool + { + // Check if user is member of root team + $rootTeam = $this->teams->where('id', 0)->first(); + + if (! $rootTeam) { + return false; + } + + // Check if user is admin or owner of root team + return $this->isAdminOfTeam(0); + } + public function requestEmailChange(string $newEmail): void { // Generate 6-digit code diff --git a/app/Policies/PrivateKeyPolicy.php b/app/Policies/PrivateKeyPolicy.php index 996054c95..9f3381faf 100644 --- a/app/Policies/PrivateKeyPolicy.php +++ b/app/Policies/PrivateKeyPolicy.php @@ -20,8 +20,18 @@ public function viewAny(User $user): bool */ public function view(User $user, PrivateKey $privateKey): bool { - // return $user->teams->contains('id', $privateKey->team_id); - return true; + // Handle null team_id + if ($privateKey->team_id === null) { + return false; + } + + // System resource (team_id=0): Only root team admins/owners can access + if ($privateKey->team_id === 0) { + return $user->canAccessSystemResources(); + } + + // Regular resource: Check team membership + return $user->teams->contains('id', $privateKey->team_id); } /** @@ -29,8 +39,9 @@ public function view(User $user, PrivateKey $privateKey): bool */ public function create(User $user): bool { - // return $user->isAdmin(); - return true; + // Only admins/owners can create private keys + // Members should not be able to create SSH keys that could be used for deployments + return $user->isAdmin(); } /** @@ -38,8 +49,19 @@ public function create(User $user): bool */ public function update(User $user, PrivateKey $privateKey): bool { - // return $user->isAdmin() && $user->teams->contains('id', $privateKey->team_id); - return true; + // Handle null team_id + if ($privateKey->team_id === null) { + return false; + } + + // System resource (team_id=0): Only root team admins/owners can update + if ($privateKey->team_id === 0) { + return $user->canAccessSystemResources(); + } + + // Regular resource: Must be admin/owner of the team + return $user->isAdminOfTeam($privateKey->team_id) + && $user->teams->contains('id', $privateKey->team_id); } /** @@ -47,8 +69,19 @@ public function update(User $user, PrivateKey $privateKey): bool */ public function delete(User $user, PrivateKey $privateKey): bool { - // return $user->isAdmin() && $user->teams->contains('id', $privateKey->team_id); - return true; + // Handle null team_id + if ($privateKey->team_id === null) { + return false; + } + + // System resource (team_id=0): Only root team admins/owners can delete + if ($privateKey->team_id === 0) { + return $user->canAccessSystemResources(); + } + + // Regular resource: Must be admin/owner of the team + return $user->isAdminOfTeam($privateKey->team_id) + && $user->teams->contains('id', $privateKey->team_id); } /** diff --git a/package-lock.json b/package-lock.json index fa5ac7aae..ce1097097 100644 --- a/package-lock.json +++ b/package-lock.json @@ -916,8 +916,7 @@ "resolved": "https://registry.npmjs.org/@socket.io/component-emitter/-/component-emitter-3.1.2.tgz", "integrity": "sha512-9BCxFwvbGg/RsZK9tjXd8s4UcwR0MWeFQ1XEKIQVVvAGJyINdrqKMcTRyLoK8Rse1GjzLV9cwjWV1olXRWEXVA==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@tailwindcss/forms": { "version": "0.5.10", @@ -1372,7 +1371,8 @@ "version": "5.5.0", "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-5.5.0.tgz", "integrity": "sha512-hqJHYaQb5OptNunnyAnkHyM8aCjZ1MEIDTQu1iIbbTD/xops91NB5yq1ZK/dC2JDbVWtF23zUtl9JE2NqwT87A==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/asynckit": { "version": "0.4.0", @@ -1535,7 +1535,6 @@ "integrity": "sha512-T0iLjnyNWahNyv/lcjS2y4oE358tVS/SYQNxYXGAJ9/GLgH4VCvOQ/mhTjqU88mLZCQgiG8RIegFHYCdVC+j5w==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@socket.io/component-emitter": "~3.1.0", "debug": "~4.3.1", @@ -1550,7 +1549,6 @@ "integrity": "sha512-Er2nc/H7RrMXZBFCEim6TCmMk02Z8vLC2Rbi1KEBggpo0fS6l0S1nnapwmIi3yW/+GOJap1Krg4w0Hg80oCqgQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ms": "^2.1.3" }, @@ -1569,7 +1567,6 @@ "integrity": "sha512-HqD3yTBfnBxIrbnM1DoD6Pcq8NECnh8d4As1Qgh0z5Gg3jRRIqijury0CL3ghu/edArpUYiYqQiDUQBIs4np3Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10.0.0" } @@ -2331,6 +2328,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -2407,6 +2405,7 @@ "integrity": "sha512-wp3HqIIUc1GRyu1XrP6m2dgyE9MoCsXVsWNlohj0rjSkLf+a0jLvEyVubdg58oMk7bhjBWnFClgp8jfAa6Ak4Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "tweetnacl": "^1.0.3" } @@ -2491,7 +2490,6 @@ "integrity": "sha512-hJVXfu3E28NmzGk8o1sHhN3om52tRvwYeidbj7xKy2eIIse5IoKX3USlS6Tqt3BHAtflLIkCQBkzVrEEfWUyYQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@socket.io/component-emitter": "~3.1.0", "debug": "~4.3.2", @@ -2508,7 +2506,6 @@ "integrity": "sha512-Er2nc/H7RrMXZBFCEim6TCmMk02Z8vLC2Rbi1KEBggpo0fS6l0S1nnapwmIi3yW/+GOJap1Krg4w0Hg80oCqgQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ms": "^2.1.3" }, @@ -2527,7 +2524,6 @@ "integrity": "sha512-/GbIKmo8ioc+NIWIhwdecY0ge+qVBSMdgxGygevmdHj24bsfgtCmcUUcQ5ZzcylGFHsN3k4HB4Cgkl96KVnuew==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@socket.io/component-emitter": "~3.1.0", "debug": "~4.3.1" @@ -2542,7 +2538,6 @@ "integrity": "sha512-Er2nc/H7RrMXZBFCEim6TCmMk02Z8vLC2Rbi1KEBggpo0fS6l0S1nnapwmIi3yW/+GOJap1Krg4w0Hg80oCqgQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ms": "^2.1.3" }, @@ -2591,7 +2586,8 @@ "version": "4.1.10", "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-4.1.10.tgz", "integrity": "sha512-P3nr6WkvKV/ONsTzj6Gb57sWPMX29EPNPopo7+FcpkQaNsrNpZ1pv8QmrYI2RqEKD7mlGqLnGovlcYnBK0IqUA==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/tapable": { "version": "2.3.0", @@ -2660,6 +2656,7 @@ "integrity": "sha512-0msEVHJEScQbhkbVTb/4iHZdJ6SXp/AvxL2sjwYQFfBqleHtnCqv1J3sa9zbWz/6kW1m9Tfzn92vW+kZ1WV6QA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.4.4", @@ -2759,6 +2756,7 @@ "integrity": "sha512-rjOV2ecxMd5SiAmof2xzh2WxntRcigkX/He4YFJ6WdRvVUrbt6DxC1Iujh10XLl8xCDRDtGKMeO3D+pRQ1PP9w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vue/compiler-dom": "3.5.16", "@vue/compiler-sfc": "3.5.16", @@ -2781,7 +2779,6 @@ "integrity": "sha512-6XQFvXTkbfUOZOKKILFG1PDK2NDQs4azKQl26T0YS5CxqWLgXajbPZ+h4gZekJyRqFU8pvnbAbbs/3TgRPy+GQ==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10.0.0" }, @@ -2803,7 +2800,6 @@ "resolved": "https://registry.npmjs.org/xmlhttprequest-ssl/-/xmlhttprequest-ssl-2.1.2.tgz", "integrity": "sha512-TEU+nJVUUnA4CYJFLvK5X9AOeH4KvDvhIfm0vV1GaQRtchnG0hgK5p8hw/xjv8cunWYCsiPCSDzObPyhEwq3KQ==", "dev": true, - "peer": true, "engines": { "node": ">=0.4.0" } diff --git a/resources/views/livewire/security/private-key/index.blade.php b/resources/views/livewire/security/private-key/index.blade.php index 47cfc9b1e..c51c7a00a 100644 --- a/resources/views/livewire/security/private-key/index.blade.php +++ b/resources/views/livewire/security/private-key/index.blade.php @@ -14,22 +14,41 @@
@forelse ($privateKeys as $key) - -
-
- {{ data_get($key, 'name') }} + @can('view', $key) + {{-- Admin/Owner: Clickable link --}} + +
+
+ {{ data_get($key, 'name') }} +
+
+ {{ $key->description }} + @if (!$key->isInUse()) + Unused + @endif +
-
- {{ $key->description }} - @if (!$key->isInUse()) - Unused - @endif + + @else + {{-- Member: Visible but not clickable --}} +
+
+
+ {{ data_get($key, 'name') }} + View Only +
+
+ {{ $key->description }} + @if (!$key->isInUse()) + Unused + @endif +
-
- + @endcan @empty
No private keys found.
@endforelse diff --git a/tests/Unit/Policies/PrivateKeyPolicyTest.php b/tests/Unit/Policies/PrivateKeyPolicyTest.php new file mode 100644 index 000000000..dd0037403 --- /dev/null +++ b/tests/Unit/Policies/PrivateKeyPolicyTest.php @@ -0,0 +1,209 @@ + 0, 'pivot' => (object) ['role' => 'admin']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeTrue(); +}); + +it('allows root team owner to view system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'owner']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeTrue(); +}); + +it('denies regular member of root team to view system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'member']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeFalse(); +}); + +it('denies non-root team member to view system private key', function () { + $teams = collect([ + (object) ['id' => 1, 'pivot' => (object) ['role' => 'owner']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeFalse(); +}); + +it('allows team member to view their own team private key', function () { + $teams = collect([ + (object) ['id' => 1, 'pivot' => (object) ['role' => 'member']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 1; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeTrue(); +}); + +it('denies team member to view another team private key', function () { + $teams = collect([ + (object) ['id' => 1, 'pivot' => (object) ['role' => 'owner']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 2; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->view($user, $privateKey))->toBeFalse(); +}); + +it('allows root team admin to update system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'admin']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->update($user, $privateKey))->toBeTrue(); +}); + +it('denies root team member to update system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'member']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->update($user, $privateKey))->toBeFalse(); +}); + +it('allows team admin to update their own team private key', function () { + $teams = collect([ + (object) ['id' => 1, 'pivot' => (object) ['role' => 'admin']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 1; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->update($user, $privateKey))->toBeTrue(); +}); + +it('denies team member to update their own team private key', function () { + $teams = collect([ + (object) ['id' => 1, 'pivot' => (object) ['role' => 'member']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 1; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->update($user, $privateKey))->toBeFalse(); +}); + +it('allows root team admin to delete system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'admin']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->delete($user, $privateKey))->toBeTrue(); +}); + +it('denies root team member to delete system private key', function () { + $teams = collect([ + (object) ['id' => 0, 'pivot' => (object) ['role' => 'member']], + ]); + + $user = Mockery::mock(User::class)->makePartial(); + $user->shouldReceive('getAttribute')->with('teams')->andReturn($teams); + + $privateKey = new class + { + public $team_id = 0; + }; + + $policy = new PrivateKeyPolicy; + expect($policy->delete($user, $privateKey))->toBeFalse(); +});