From a511bd9b67868848c8d70bed9b8f9ff5c8a18bbc Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:17:55 +0200 Subject: [PATCH 1/2] fix(api): validate token team context --- app/Actions/User/DeleteUserTeams.php | 3 + app/Actions/User/RevokeUserTeamTokens.php | 43 ++++++ app/Http/Kernel.php | 2 + .../EnsureTokenBelongsToCurrentTeamMember.php | 37 +++++ app/Livewire/Team/Member.php | 14 +- app/Mcp/Concerns/ResolvesTeam.php | 10 +- app/Models/Team.php | 3 + app/Models/User.php | 4 + app/Traits/DeletesUserSessions.php | 2 + bootstrap/helpers/api.php | 37 +++-- routes/ai.php | 2 +- routes/api.php | 4 +- .../ApiTokenTeamLifecycleSecurityTest.php | 128 ++++++++++++++++++ tests/Feature/Mcp/McpEndpointTest.php | 11 ++ 14 files changed, 277 insertions(+), 23 deletions(-) create mode 100644 app/Actions/User/RevokeUserTeamTokens.php create mode 100644 app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php create mode 100644 tests/Feature/ApiTokenTeamLifecycleSecurityTest.php diff --git a/app/Actions/User/DeleteUserTeams.php b/app/Actions/User/DeleteUserTeams.php index d572db9e7..b2b06e7ba 100644 --- a/app/Actions/User/DeleteUserTeams.php +++ b/app/Actions/User/DeleteUserTeams.php @@ -137,9 +137,11 @@ class DeleteUserTeams // Update the new owner's role to owner $team->members()->updateExistingPivot($newOwner->id, ['role' => 'owner']); + RevokeUserTeamTokens::forUserTeam($newOwner, $team->id); // Remove the current user from the team $team->members()->detach($this->user->id); + RevokeUserTeamTokens::forUserTeam($this->user, $team->id); $counts['transferred']++; } catch (\Exception $e) { @@ -152,6 +154,7 @@ class DeleteUserTeams foreach ($preview['to_leave'] as $team) { try { $team->members()->detach($this->user->id); + RevokeUserTeamTokens::forUserTeam($this->user, $team->id); $counts['left']++; } catch (\Exception $e) { \Log::error("Failed to remove user from team {$team->id}: ".$e->getMessage()); diff --git a/app/Actions/User/RevokeUserTeamTokens.php b/app/Actions/User/RevokeUserTeamTokens.php new file mode 100644 index 000000000..9aadf1eeb --- /dev/null +++ b/app/Actions/User/RevokeUserTeamTokens.php @@ -0,0 +1,43 @@ +where('tokenable_id', self::userId($user)) + ->where('team_id', $teamId) + ->delete(); + } + + public static function forUser(User|int $user): int + { + return self::baseQuery() + ->where('tokenable_id', self::userId($user)) + ->delete(); + } + + public static function forTeam(int|string $teamId): int + { + return self::baseQuery() + ->where('team_id', $teamId) + ->delete(); + } + + private static function baseQuery(): Builder + { + return PersonalAccessToken::query() + ->where('tokenable_type', User::class); + } + + private static function userId(User|int $user): int + { + return $user instanceof User ? $user->id : $user; + } +} diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index a584bc111..02a49aaa8 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -12,6 +12,7 @@ use App\Http\Middleware\CheckForcePasswordReset; use App\Http\Middleware\DecideWhatToDoWithUser; use App\Http\Middleware\EncryptCookies; use App\Http\Middleware\EnsureMcpEnabled; +use App\Http\Middleware\EnsureTokenBelongsToCurrentTeamMember; use App\Http\Middleware\PreventRequestsDuringMaintenance; use App\Http\Middleware\RedirectIfAuthenticated; use App\Http\Middleware\TrimStrings; @@ -104,6 +105,7 @@ class Kernel extends HttpKernel 'ability' => CheckForAnyAbility::class, 'api.ability' => ApiAbility::class, 'api.sensitive' => ApiSensitiveData::class, + 'api.token.team' => EnsureTokenBelongsToCurrentTeamMember::class, 'can.create.resources' => CanCreateResources::class, 'can.update.resource' => CanUpdateResource::class, 'can.access.terminal' => CanAccessTerminal::class, diff --git a/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php b/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php new file mode 100644 index 000000000..7c858b38b --- /dev/null +++ b/app/Http/Middleware/EnsureTokenBelongsToCurrentTeamMember.php @@ -0,0 +1,37 @@ +user(); + $token = $user?->currentAccessToken(); + $teamId = $token?->team_id; + + if (! $user || ! $token || is_null($teamId)) { + return response()->json(['message' => 'Invalid token.'], 401); + } + + $team = $user->teams() + ->where('teams.id', $teamId) + ->first(); + + if (! $team) { + return response()->json(['message' => 'Invalid token.'], 401); + } + + $role = $team->pivot?->role; + if (($token->can('root') || $token->can('write') || $token->can('write:sensitive')) + && ! in_array($role, ['admin', 'owner'], true)) { + return response()->json(['message' => 'Missing required team role.'], 403); + } + + return $next($request); + } +} diff --git a/app/Livewire/Team/Member.php b/app/Livewire/Team/Member.php index b1f692365..97d492d70 100644 --- a/app/Livewire/Team/Member.php +++ b/app/Livewire/Team/Member.php @@ -2,6 +2,7 @@ namespace App\Livewire\Team; +use App\Actions\User\RevokeUserTeamTokens; use App\Enums\Role; use App\Models\User; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; @@ -23,7 +24,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::ADMIN->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::ADMIN->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -39,7 +42,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::OWNER->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::OWNER->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -55,7 +60,9 @@ class Member extends Component || Role::from($this->getMemberRole())->gt(auth()->user()->role())) { throw new \Exception('You are not authorized to perform this action.'); } - $this->member->teams()->updateExistingPivot(currentTeam()->id, ['role' => Role::MEMBER->value]); + $teamId = currentTeam()->id; + $this->member->teams()->updateExistingPivot($teamId, ['role' => Role::MEMBER->value]); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); $this->dispatch('reloadWindow'); } catch (\Exception $e) { $this->dispatch('error', $e->getMessage()); @@ -73,6 +80,7 @@ class Member extends Component } $teamId = currentTeam()->id; $this->member->teams()->detach(currentTeam()); + RevokeUserTeamTokens::forUserTeam($this->member, $teamId); // Clear cache for the removed user - both old and new key formats Cache::forget("team:{$this->member->id}"); Cache::forget("user:{$this->member->id}:team:{$teamId}"); diff --git a/app/Mcp/Concerns/ResolvesTeam.php b/app/Mcp/Concerns/ResolvesTeam.php index f75219fcf..f6d82453a 100644 --- a/app/Mcp/Concerns/ResolvesTeam.php +++ b/app/Mcp/Concerns/ResolvesTeam.php @@ -28,8 +28,14 @@ trait ResolvesTeam protected function resolveTeamId(Request $request): ?int { - $token = $request->user()?->currentAccessToken(); + $user = $request->user(); + $token = $user?->currentAccessToken(); + $teamId = $token?->team_id; - return $token?->team_id; + if (! $user || is_null($teamId) || ! $user->teams()->where('teams.id', $teamId)->exists()) { + return null; + } + + return (int) $teamId; } } diff --git a/app/Models/Team.php b/app/Models/Team.php index 0fbcfe0c6..f0a50cf69 100644 --- a/app/Models/Team.php +++ b/app/Models/Team.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Actions\User\RevokeUserTeamTokens; use App\Events\ServerReachabilityChanged; use App\Notifications\Channels\SendsDiscord; use App\Notifications\Channels\SendsEmail; @@ -72,6 +73,8 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen }); static::deleting(function (Team $team) { + RevokeUserTeamTokens::forTeam($team->id); + foreach ($team->privateKeys as $key) { $key->delete(); } diff --git a/app/Models/User.php b/app/Models/User.php index cefdf3d3e..9cbe88835 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Actions\User\RevokeUserTeamTokens; use App\Jobs\UpdateStripeCustomerEmailJob; use App\Notifications\Channels\SendsEmail; use App\Notifications\TransactionalEmails\EmailChangeVerification; @@ -121,6 +122,8 @@ class User extends Authenticatable implements SendsEmail static::deleting(function (User $user) { \DB::transaction(function () use ($user) { + RevokeUserTeamTokens::forUser($user); + $teams = $user->teams; foreach ($teams as $team) { $user_alone_in_team = $team->members->count() === 1; @@ -158,6 +161,7 @@ class User extends Authenticatable implements SendsEmail if ($found_other_member_who_is_not_owner) { $found_other_member_who_is_not_owner->pivot->role = 'owner'; $found_other_member_who_is_not_owner->pivot->save(); + RevokeUserTeamTokens::forUserTeam($found_other_member_who_is_not_owner, $team->id); $team->members()->detach($user->id); } else { static::finalizeTeamDeletion($user, $team); diff --git a/app/Traits/DeletesUserSessions.php b/app/Traits/DeletesUserSessions.php index e9ec0d946..44ff5f727 100644 --- a/app/Traits/DeletesUserSessions.php +++ b/app/Traits/DeletesUserSessions.php @@ -2,6 +2,7 @@ namespace App\Traits; +use App\Actions\User\RevokeUserTeamTokens; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Session; @@ -17,6 +18,7 @@ trait DeletesUserSessions Session::invalidate(); Session::regenerateToken(); DB::table('sessions')->where('user_id', $this->id)->delete(); + RevokeUserTeamTokens::forUser($this->id); } /** diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 4b76200d2..85703365d 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -3,15 +3,22 @@ use App\Enums\BuildPackTypes; use App\Enums\RedirectTypes; use App\Enums\StaticImageTypes; +use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Collection; use Illuminate\Http\Request; use Illuminate\Validation\Rule; function getTeamIdFromToken() { - $token = auth()->user()->currentAccessToken(); + $user = auth()->user(); + $token = $user?->currentAccessToken(); + $teamId = data_get($token, 'team_id'); - return data_get($token, 'team_id'); + if (! $user || is_null($teamId) || ! $user->teams()->where('teams.id', $teamId)->exists()) { + return null; + } + + return $teamId; } function invalidTokenResponse() { @@ -95,14 +102,14 @@ function sharedDataApplications() 'git_commit_sha' => ['string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], 'docker_registry_image_name' => 'string|nullable', 'docker_registry_image_tag' => 'string|nullable', - 'install_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'install_command' => ValidationPatterns::shellSafeCommandRules(), + 'build_command' => ValidationPatterns::shellSafeCommandRules(), + 'start_command' => ValidationPatterns::shellSafeCommandRules(), 'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/', 'ports_mappings' => 'string|regex:/^(\d+:\d+)(,\d+:\d+)*$/|nullable', 'custom_network_aliases' => 'string|nullable', - 'base_directory' => \App\Support\ValidationPatterns::directoryPathRules(), - 'publish_directory' => \App\Support\ValidationPatterns::directoryPathRules(), + 'base_directory' => ValidationPatterns::directoryPathRules(), + 'publish_directory' => ValidationPatterns::directoryPathRules(), 'health_check_enabled' => 'boolean', 'health_check_type' => 'string|in:http,cmd', 'health_check_command' => ['nullable', 'string', 'max:1000', 'regex:/^[a-zA-Z0-9 \-_.\/:=@,+]+$/'], @@ -125,24 +132,24 @@ function sharedDataApplications() 'limits_cpuset' => 'string|nullable', 'limits_cpu_shares' => 'numeric', 'custom_labels' => 'string|nullable', - 'custom_docker_run_options' => \App\Support\ValidationPatterns::shellSafeCommandRules(2000), + 'custom_docker_run_options' => ValidationPatterns::shellSafeCommandRules(2000), // Security: deployment commands are intentionally arbitrary shell (e.g. "php artisan migrate"). // Access is gated by API token authentication. Commands run inside the app container, not the host. 'post_deployment_command' => 'string|nullable', - 'post_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'post_deployment_command_container' => ValidationPatterns::containerNameRules(), 'pre_deployment_command' => 'string|nullable', - 'pre_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'pre_deployment_command_container' => ValidationPatterns::containerNameRules(), 'manual_webhook_secret_github' => 'string|nullable', 'manual_webhook_secret_gitlab' => 'string|nullable', 'manual_webhook_secret_bitbucket' => 'string|nullable', 'manual_webhook_secret_gitea' => 'string|nullable', - 'dockerfile_location' => \App\Support\ValidationPatterns::filePathRules(), - 'dockerfile_target_build' => \App\Support\ValidationPatterns::dockerTargetRules(), - 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), + 'dockerfile_location' => ValidationPatterns::filePathRules(), + 'dockerfile_target_build' => ValidationPatterns::dockerTargetRules(), + 'docker_compose_location' => ValidationPatterns::filePathRules(), 'docker_compose' => 'string|nullable', 'docker_compose_domains' => 'array|nullable', - 'docker_compose_custom_start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'docker_compose_custom_build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_start_command' => ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_build_command' => ValidationPatterns::shellSafeCommandRules(), 'is_container_label_escape_enabled' => 'boolean', 'is_preserve_repository_enabled' => 'boolean', ]; diff --git a/routes/ai.php b/routes/ai.php index 7d3a858c5..3a39677ff 100644 --- a/routes/ai.php +++ b/routes/ai.php @@ -4,4 +4,4 @@ use App\Mcp\Servers\CoolifyServer; use Laravel\Mcp\Facades\Mcp; Mcp::web('/mcp', CoolifyServer::class) - ->middleware(['mcp.enabled', 'auth:sanctum']); + ->middleware(['mcp.enabled', 'auth:sanctum', 'api.token.team']); diff --git a/routes/api.php b/routes/api.php index fb3b4bad6..ec6bde2e6 100644 --- a/routes/api.php +++ b/routes/api.php @@ -29,7 +29,7 @@ Route::post('/feedback', [OtherController::class, 'feedback']) ->middleware('throttle:feedback'); Route::group([ - 'middleware' => ['auth:sanctum', 'api.ability:write'], + 'middleware' => ['auth:sanctum', 'api.token.team', 'api.ability:write'], 'prefix' => 'v1', ], function () { Route::get('/enable', [OtherController::class, 'enable_api']); @@ -38,7 +38,7 @@ Route::group([ Route::post('/mcp/disable', [OtherController::class, 'disable_mcp']); }); Route::group([ - 'middleware' => ['auth:sanctum', ApiAllowed::class, 'api.sensitive'], + 'middleware' => ['auth:sanctum', 'api.token.team', ApiAllowed::class, 'api.sensitive'], 'prefix' => 'v1', ], function () { diff --git a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php new file mode 100644 index 000000000..d5ab83af0 --- /dev/null +++ b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php @@ -0,0 +1,128 @@ + InstanceSettings::query()->create([ + 'id' => 0, + 'is_api_enabled' => true, + ])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'admin']); + session(['currentTeam' => $this->team]); +}); + +function bearerJson(string $token): array +{ + return [ + 'Authorization' => 'Bearer '.$token, + 'Content-Type' => 'application/json', + ]; +} + +test('removed member token cannot read team projects', function () { + Project::create(['name' => 'Secret', 'team_id' => $this->team->id]); + $token = $this->user->createToken('read-token', ['read'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $this->withHeaders(bearerJson($token)) + ->getJson('/api/v1/projects') + ->assertUnauthorized(); +}); + +test('removed member token cannot create team projects', function () { + $token = $this->user->createToken('write-token', ['write'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $this->withHeaders(bearerJson($token)) + ->postJson('/api/v1/projects', ['name' => 'Should Not Exist']) + ->assertUnauthorized(); + + expect(Project::where('name', 'Should Not Exist')->exists())->toBeFalse(); +}); + +test('downgraded member old write token cannot create team projects', function () { + $token = $this->user->createToken('write-token', ['write'])->plainTextToken; + + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + + $this->withHeaders(bearerJson($token)) + ->postJson('/api/v1/projects', ['name' => 'Downgrade Bypass']) + ->assertForbidden(); + + expect(Project::where('name', 'Downgrade Bypass')->exists())->toBeFalse(); +}); + +test('admin removal through team member component revokes team tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call('remove'); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('role downgrade through team member component revokes team tokens', function () { + $owner = User::factory()->create(); + $this->team->members()->attach($owner->id, ['role' => 'owner']); + $token = $this->user->createToken('write-token', ['write'])->accessToken; + + $this->actingAs($owner); + session(['currentTeam' => $this->team]); + + Livewire::test(Member::class, ['member' => $this->user]) + ->call('makeReadonly'); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('member cannot create write token through livewire token form', function () { + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + Livewire::test(ApiTokens::class) + ->set('description', 'member-write-token') + ->set('expiresInDays', 30) + ->set('permissions', ['write']) + ->call('addNewToken'); + + expect($this->user->tokens()->where('name', 'member-write-token')->exists())->toBeFalse(); +}); + +test('password change revokes user personal access tokens', function () { + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->user->forceFill(['password' => Hash::make('new-password')])->save(); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); + +test('team deletion revokes team bound personal access tokens', function () { + $token = $this->user->createToken('read-token', ['read'])->accessToken; + + $this->team->delete(); + + expect(DB::table('personal_access_tokens')->where('id', $token->id)->exists())->toBeFalse(); +}); diff --git a/tests/Feature/Mcp/McpEndpointTest.php b/tests/Feature/Mcp/McpEndpointTest.php index 34ae493cc..ae0101547 100644 --- a/tests/Feature/Mcp/McpEndpointTest.php +++ b/tests/Feature/Mcp/McpEndpointTest.php @@ -192,3 +192,14 @@ test('tool calls fail when the token lacks the read ability', function () { expect($response->json('result.isError'))->toBeTrue(); expect($response->json('result.content.0.text'))->toContain('Missing required permissions'); }); + +test('MCP rejects token when user no longer belongs to token team', function () { + Project::create(['name' => 'Hidden', 'team_id' => $this->team->id]); + $token = $this->user->createToken('mcp-read', ['read'])->plainTextToken; + + $this->team->members()->detach($this->user->id); + + $response = mcpCallTool($token, 'list_projects'); + + $response->assertUnauthorized(); +}); From 7193a5d0f646e5df743ed219f135af01a8c6c6a2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:05:51 +0200 Subject: [PATCH 2/2] fix(tests): reuse instance settings in API token team tests --- tests/Feature/ApiTokenTeamLifecycleSecurityTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php index d5ab83af0..c254a26c3 100644 --- a/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php +++ b/tests/Feature/ApiTokenTeamLifecycleSecurityTest.php @@ -14,10 +14,10 @@ use Livewire\Livewire; uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::unguarded(fn () => InstanceSettings::query()->create([ - 'id' => 0, - 'is_api_enabled' => true, - ])); + InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate( + ['id' => 0], + ['is_api_enabled' => true], + )); $this->team = Team::factory()->create(); $this->user = User::factory()->create();