From 12498b4b8648d27c4251f8010082620d9c63eada Mon Sep 17 00:00:00 2001 From: Aditya Tripathi Date: Tue, 25 Aug 2026 12:45:47 +0000 Subject: [PATCH 1/9] feat(teams): persist active team and add team selection screen Add current_team_id to users so the last active team is restored on login instead of always defaulting to the personal team. When a user belongs to multiple teams and has no valid stored choice, redirect them to a new team.select screen (SelectTeam Livewire component) to pick one, rather than silently choosing the first team. Update Fortify and OAuth login flows to use the new resolveStoredTeam() logic. --- app/Http/Controllers/OauthController.php | 8 + .../Middleware/DecideWhatToDoWithUser.php | 19 ++- app/Livewire/SelectTeam.php | 46 ++++++ app/Models/User.php | 27 ++++ app/Providers/FortifyServiceProvider.php | 15 +- bootstrap/helpers/shared.php | 7 + ...006_add_current_team_id_to_users_table.php | 26 ++++ .../views/livewire/select-team.blade.php | 18 +++ routes/web.php | 2 + .../Team/ActiveTeamPersistenceTest.php | 139 ++++++++++++++++++ 10 files changed, 300 insertions(+), 7 deletions(-) create mode 100644 app/Livewire/SelectTeam.php create mode 100644 database/migrations/2026_08_24_131006_add_current_team_id_to_users_table.php create mode 100644 resources/views/livewire/select-team.blade.php create mode 100644 tests/Feature/Team/ActiveTeamPersistenceTest.php diff --git a/app/Http/Controllers/OauthController.php b/app/Http/Controllers/OauthController.php index 4038fe63e2..109f8915a1 100644 --- a/app/Http/Controllers/OauthController.php +++ b/app/Http/Controllers/OauthController.php @@ -38,6 +38,14 @@ class OauthController extends Controller } Auth::login($user); + $team = $user->resolveStoredTeam(); + if (! $team && $user->teams()->count() === 0) { + $team = $user->recreate_personal_team(); + } + if ($team) { + session(['currentTeam' => $user->currentTeam = $team]); + } + return redirect('/'); } catch (\Exception $e) { $errorCode = $e instanceof HttpException ? 'auth.failed' : 'auth.failed.callback'; diff --git a/app/Http/Middleware/DecideWhatToDoWithUser.php b/app/Http/Middleware/DecideWhatToDoWithUser.php index dbf261f4db..6babdb69a0 100644 --- a/app/Http/Middleware/DecideWhatToDoWithUser.php +++ b/app/Http/Middleware/DecideWhatToDoWithUser.php @@ -18,9 +18,24 @@ class DecideWhatToDoWithUser } if (auth()?->user()?->currentTeam()) { refreshSession(auth()->user()->currentTeam()); + // A team is already active; the selection screen no longer applies. + if ($request->routeIs('team.select')) { + return redirect()->route('dashboard'); + } } elseif (auth()?->user()?->teams?->count() > 0) { - // User's session team is invalid (e.g., removed from team), switch to first available team - refreshSession(auth()->user()->teams->first()); + // No active team in the session (fresh login or invalidated selection). + // Restore the last active team, or the sole team of a single-team user. + $resolvedTeam = auth()->user()->resolveStoredTeam(); + if ($resolvedTeam) { + refreshSession($resolvedTeam); + } elseif ($request->routeIs('team.select') || $request->routeIs('*livewire.update')) { + // Ambiguous choice: let the user pick a team on the selection screen. + // Livewire's update endpoint must pass through too, otherwise the + // selection action's AJAX call is redirected to HTML and never runs. + return $next($request); + } else { + return redirect()->route('team.select'); + } } if (! auth()->user() || ! isCloud()) { if (! isCloud() && showBoarding() && ! in_array($request->path(), allowedPathsForBoardingAccounts())) { diff --git a/app/Livewire/SelectTeam.php b/app/Livewire/SelectTeam.php new file mode 100644 index 0000000000..ba8b2a577f --- /dev/null +++ b/app/Livewire/SelectTeam.php @@ -0,0 +1,46 @@ +user(); + + // A team is already active, or the user has at most one team: nothing to pick. + if ($user->currentTeam() || $user->teams->count() <= 1) { + $resolved = $user->resolveStoredTeam(); + if ($resolved) { + refreshSession($resolved); + } + + return redirect()->route('dashboard'); + } + } + + public function selectTeam(int $teamId) + { + $user = auth()->user(); + if (! $user->teams->contains('id', $teamId)) { + return; + } + $team = Team::find($teamId); + if (! $team) { + return; + } + refreshSession($team); + + return redirect()->route('dashboard'); + } + + public function render() + { + return view('livewire.select-team', [ + 'teams' => auth()->user()->teams, + ])->layout('layouts.simple'); + } +} diff --git a/app/Models/User.php b/app/Models/User.php index 5b38473962..47f6f2fd40 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -48,6 +48,7 @@ class User extends Authenticatable implements SendsEmail 'name', 'email', 'password', + 'current_team_id', 'force_password_reset', 'marketing_emails', 'pending_email', @@ -66,6 +67,7 @@ class User extends Authenticatable implements SendsEmail ]; protected $casts = [ + 'current_team_id' => 'integer', 'email_verified_at' => 'datetime', 'force_password_reset' => 'boolean', 'show_boarding' => 'boolean', @@ -374,6 +376,31 @@ class User extends Authenticatable implements SendsEmail }); } + /** + * Resolve the team to activate when the session has no current team + * (fresh login or an invalidated session). + * + * Returns the user's last active team when they still belong to it, or the + * sole team of a single-team user. Returns null when the choice is ambiguous + * (more than one team and no valid stored preference) — the caller must then + * prompt the user to pick a team instead of defaulting silently. + */ + public function resolveStoredTeam(): ?Team + { + if (! is_null($this->current_team_id)) { + $storedTeam = $this->teams->firstWhere('id', $this->current_team_id); + if ($storedTeam) { + return $storedTeam; + } + } + + if ($this->teams->count() === 1) { + return $this->teams->first(); + } + + return null; + } + public function role(): ?string { if (data_get($this, 'pivot')) { diff --git a/app/Providers/FortifyServiceProvider.php b/app/Providers/FortifyServiceProvider.php index ce16e617d7..60d2545a05 100644 --- a/app/Providers/FortifyServiceProvider.php +++ b/app/Providers/FortifyServiceProvider.php @@ -90,14 +90,19 @@ class FortifyServiceProvider extends ServiceProvider } $user->currentTeam = $invitation->team; $invitation->delete(); + session(['currentTeam' => $user->currentTeam]); } else { - // Normal login - use personal team - $user->currentTeam = $user->teams->firstWhere('personal_team', true); - if (! $user->currentTeam) { - $user->currentTeam = $user->recreate_personal_team(); + // Restore the last active team; only fall back when unambiguous. + $team = $user->resolveStoredTeam(); + if (! $team && $user->teams->isEmpty()) { + $team = $user->recreate_personal_team(); } + if ($team) { + session(['currentTeam' => $user->currentTeam = $team]); + } + // Otherwise (multiple teams, no stored choice) leave the session + // team unset so the user is sent to the team-selection screen. } - session(['currentTeam' => $user->currentTeam]); return $user; } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index fbfbc9b566..169132cb70 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -593,6 +593,13 @@ function refreshSession(?Team $team = null): void return $team; }); session(['currentTeam' => $team]); + + // Persist the active team so it can be restored after logout/login. + $user = Auth::user(); + if ($user && $user->current_team_id !== $team->id) { + $user->current_team_id = $team->id; + $user->saveQuietly(); + } } function handleError(?Throwable $error = null, ?Component $livewire = null, ?string $customErrorMessage = null) { diff --git a/database/migrations/2026_08_24_131006_add_current_team_id_to_users_table.php b/database/migrations/2026_08_24_131006_add_current_team_id_to_users_table.php new file mode 100644 index 0000000000..01f53d4221 --- /dev/null +++ b/database/migrations/2026_08_24_131006_add_current_team_id_to_users_table.php @@ -0,0 +1,26 @@ +unsignedBigInteger('current_team_id')->nullable()->after('id'); + }); + } + + public function down(): void + { + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('current_team_id'); + }); + } +}; diff --git a/resources/views/livewire/select-team.blade.php b/resources/views/livewire/select-team.blade.php new file mode 100644 index 0000000000..9eb23c19c5 --- /dev/null +++ b/resources/views/livewire/select-team.blade.php @@ -0,0 +1,18 @@ + +
+ @foreach ($teams as $team) + + @endforeach +
+ diff --git a/routes/web.php b/routes/web.php index cdf8161797..30a9634d4d 100644 --- a/routes/web.php +++ b/routes/web.php @@ -51,6 +51,7 @@ use App\Livewire\Security\CloudProviderToken\Show as SecurityCloudProviderTokenS use App\Livewire\Security\CloudTokens; use App\Livewire\Security\PrivateKey\Index as SecurityPrivateKeyIndex; use App\Livewire\Security\PrivateKey\Show as SecurityPrivateKeyShow; +use App\Livewire\SelectTeam; use App\Livewire\Server\Advanced as ServerAdvanced; use App\Livewire\Server\CaCertificate\Show as CaCertificateShow; use App\Livewire\Server\Charts as ServerCharts; @@ -398,6 +399,7 @@ Route::middleware(['auth', 'verified'])->group(function () { }); Route::middleware(['auth'])->group(function () { + Route::get('/select-team', SelectTeam::class)->name('team.select'); Route::get('/sources', function () { $sources = currentTeam()->sources(); diff --git a/tests/Feature/Team/ActiveTeamPersistenceTest.php b/tests/Feature/Team/ActiveTeamPersistenceTest.php new file mode 100644 index 0000000000..4dc1570cd1 --- /dev/null +++ b/tests/Feature/Team/ActiveTeamPersistenceTest.php @@ -0,0 +1,139 @@ + InstanceSettings::firstOrCreate(['id' => 0])); +}); + +/** + * Create a user that belongs to two teams (their auto-created personal team + * plus a second team). Boarding is disabled so the middleware does not bounce + * the request to the onboarding screen. + */ +function userWithTwoTeams(): array +{ + $user = User::factory()->create(); + $personal = $user->teams->first(); + $personal->update(['show_boarding' => false]); + + $second = Team::factory()->create(['show_boarding' => false]); + $user->teams()->attach($second, ['role' => 'owner']); + $user->refresh(); + + return [$user, $personal, $second]; +} + +it('resolves the stored team when the user still belongs to it', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + + expect($user->resolveStoredTeam()?->id)->toBe($second->id); +}); + +it('resolves the only team for single-team users without a stored choice', function () { + $user = User::factory()->create(); + $user->teams->first()->update(['show_boarding' => false]); + + expect($user->resolveStoredTeam()?->id)->toBe($user->teams->first()->id); +}); + +it('returns null for multi-team users without a valid stored choice', function () { + [$user] = userWithTwoTeams(); + + expect($user->resolveStoredTeam())->toBeNull(); +}); + +it('ignores a stored team the user no longer belongs to', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => 99999]); + + expect($user->resolveStoredTeam())->toBeNull(); + // still ambiguous (2 teams), so must pick again + $user->update(['current_team_id' => $second->id]); + $user->refresh(); + expect($user->resolveStoredTeam()?->id)->toBe($second->id); +}); + +it('persists current_team_id when the active team changes via refreshSession', function () { + [$user, , $second] = userWithTwoTeams(); + $this->actingAs($user); + + refreshSession($second); + + expect($user->fresh()->current_team_id)->toBe($second->id) + ->and(data_get(session('currentTeam'), 'id'))->toBe($second->id); +}); + +it('redirects a multi-team user with no stored team to the select screen', function () { + [$user] = userWithTwoTeams(); + + $this->actingAs($user) + ->get('/') + ->assertRedirect(route('team.select')); +}); + +it('restores the stored team for a returning multi-team user', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + + $this->actingAs($user)->get('/'); + + expect(data_get(session('currentTeam'), 'id'))->toBe($second->id); +}); + +it('does not send a single-team user to the select screen', function () { + $user = User::factory()->create(); + $only = $user->teams->first(); + $only->update(['show_boarding' => false]); + + // A single-team user who lands on the select screen is bounced straight to + // the dashboard with their team activated, never shown a choice. + $this->actingAs($user) + ->get(route('team.select')) + ->assertRedirect(route('dashboard')); + + expect(data_get(session('currentTeam'), 'id'))->toBe($only->id); +}); + +it('persists the choice and activates the team when selected on the screen', function () { + [$user, , $second] = userWithTwoTeams(); + + Livewire::actingAs($user) + ->test(SelectTeam::class) + ->call('selectTeam', $second->id) + ->assertRedirect(route('dashboard')); + + expect($user->fresh()->current_team_id)->toBe($second->id) + ->and(data_get(session('currentTeam'), 'id'))->toBe($second->id); +}); + +it('lets the livewire update endpoint through for an ambiguous user', function () { + [$user] = userWithTwoTeams(); + + // The selection action runs as a Livewire AJAX POST to /livewire/update. + // The team gate must not hijack that request with a redirect to the + // selection screen, or the click silently does nothing (HTML != JSON). + $response = $this->actingAs($user) + ->withHeaders(['X-Livewire' => 'true']) + ->post('/livewire/update', []); + + expect($response->headers->get('Location'))->not->toBe(route('team.select')); +}); + +it('bounces users who already have an active team away from the select screen', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + refreshSession($second); + + Livewire::actingAs($user) + ->test(SelectTeam::class) + ->assertRedirect(route('dashboard')); +}); From 2b92fb86b5c7b88a3c202f16ff068a0f3cb773b7 Mon Sep 17 00:00:00 2001 From: Aditya Tripathi Date: Tue, 25 Aug 2026 13:46:29 +0000 Subject: [PATCH 2/9] fix(teams): clear stale current_team_id when membership ends Reset the user's persisted current_team_id when they are removed from a team, when their team is deleted, or when refreshSession finds no team left, so a dangling reference is never restored on next login. --- app/Actions/Team/DeleteTeam.php | 1 + app/Livewire/Team/Member.php | 1 + app/Models/User.php | 15 +++++++++ bootstrap/helpers/shared.php | 7 +++- .../Team/ActiveTeamPersistenceTest.php | 33 +++++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) diff --git a/app/Actions/Team/DeleteTeam.php b/app/Actions/Team/DeleteTeam.php index be880b7e78..5d0c1e6885 100644 --- a/app/Actions/Team/DeleteTeam.php +++ b/app/Actions/Team/DeleteTeam.php @@ -50,6 +50,7 @@ class DeleteTeam ->get() ->each(function (User $member) use ($team): void { $member->teams()->detach($team); + $member->clearStoredTeamIfMatches($team->id); DB::table('sessions')->where('user_id', $member->id)->delete(); }); diff --git a/app/Livewire/Team/Member.php b/app/Livewire/Team/Member.php index 38c932c39d..ab3f7938a1 100644 --- a/app/Livewire/Team/Member.php +++ b/app/Livewire/Team/Member.php @@ -89,6 +89,7 @@ class Member extends Component DB::transaction(function () use ($teamId): void { $this->member->teams()->detach($teamId); RevokeUserTeamTokens::forUserTeam($this->member, $teamId); + $this->member->clearStoredTeamIfMatches($teamId); }); // Clear cache for the removed user - both old and new key formats Cache::forget("team:{$this->member->id}"); diff --git a/app/Models/User.php b/app/Models/User.php index 47f6f2fd40..ab9a817bfe 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -401,6 +401,21 @@ class User extends Authenticatable implements SendsEmail return null; } + /** + * Reset the persisted active team when it points to the given team. + * + * Called when the user is removed from a team (or the team is deleted) so a + * stale current_team_id can never be trusted after the fact. Read paths + * already re-validate membership; this is defense-in-depth that clears the + * dangling value at the source event instead of relying on self-healing. + */ + public function clearStoredTeamIfMatches(int $teamId): void + { + if ($this->current_team_id === $teamId) { + $this->forceFill(['current_team_id' => null])->saveQuietly(); + } + } + public function role(): ?string { if (data_get($this, 'pivot')) { diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 169132cb70..1c19fd1ca6 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -581,8 +581,13 @@ function refreshSession(?Team $team = null): void if (! $team) { // The user has no team left (e.g. just deleted their current team and // belongs to no other): clear the stale session reference instead of - // dereferencing null. + // dereferencing null, and drop the persisted choice so it is not + // restored on next login. session()->forget('currentTeam'); + $user = Auth::user(); + if ($user && ! is_null($user->current_team_id)) { + $user->forceFill(['current_team_id' => null])->saveQuietly(); + } return; } diff --git a/tests/Feature/Team/ActiveTeamPersistenceTest.php b/tests/Feature/Team/ActiveTeamPersistenceTest.php index 4dc1570cd1..b481cce314 100644 --- a/tests/Feature/Team/ActiveTeamPersistenceTest.php +++ b/tests/Feature/Team/ActiveTeamPersistenceTest.php @@ -1,5 +1,6 @@ headers->get('Location'))->not->toBe(route('team.select')); }); +it('clears the stored team when the member is removed from it', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + + // Simulate the removal event (Team\Member::remove detaches then clears). + $user->teams()->detach($second->id); + $user->clearStoredTeamIfMatches($second->id); + + expect($user->fresh()->current_team_id)->toBeNull(); +}); + +it('keeps the stored team when the member is removed from a different team', function () { + [$user, $personal, $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + + $user->teams()->detach($personal->id); + $user->clearStoredTeamIfMatches($personal->id); + + expect($user->fresh()->current_team_id)->toBe($second->id); +}); + +it('clears the stored team for members when their team is deleted', function () { + [$owner, , $shared] = userWithTwoTeams(); + $member = User::factory()->create(); + $member->teams()->attach($shared, ['role' => 'member']); + $member->update(['current_team_id' => $shared->id]); + + app(DeleteTeam::class)->handle($shared->fresh(), $owner); + + expect($member->fresh()->current_team_id)->toBeNull(); +}); + it('bounces users who already have an active team away from the select screen', function () { [$user, , $second] = userWithTwoTeams(); $user->update(['current_team_id' => $second->id]); From f511921895dc0994262312f4704f31ade9c0b491 Mon Sep 17 00:00:00 2001 From: Aditya Tripathi Date: Tue, 25 Aug 2026 17:25:32 +0000 Subject: [PATCH 3/9] fix(teams): guard current_team_id clear against concurrent writes Make clearStoredTeamIfMatches perform an atomic conditional UPDATE so a concurrent team switch isn't clobbered, and call it for the deleting owner in DeleteTeam so their stored team id doesn't point at a deleted team. refreshSession now falls back to resolveStoredTeam() instead of an arbitrary first team. Add a return type to SelectTeam::render() and tests covering owner deletion and concurrent-selection preservation. --- app/Actions/Team/DeleteTeam.php | 4 ++++ app/Livewire/SelectTeam.php | 6 ++++- app/Models/User.php | 10 ++++++++- bootstrap/helpers/shared.php | 7 ++++-- .../Team/ActiveTeamPersistenceTest.php | 22 +++++++++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/app/Actions/Team/DeleteTeam.php b/app/Actions/Team/DeleteTeam.php index 5d0c1e6885..bde72448d8 100644 --- a/app/Actions/Team/DeleteTeam.php +++ b/app/Actions/Team/DeleteTeam.php @@ -54,6 +54,10 @@ class DeleteTeam DB::table('sessions')->where('user_id', $member->id)->delete(); }); + // The deleting owner is excluded from the loop above; clear their + // stored team too so the deleted id is not restored on next login. + $user->clearStoredTeamIfMatches($team->id); + $team->delete(); return $user->teams()->first(); diff --git a/app/Livewire/SelectTeam.php b/app/Livewire/SelectTeam.php index ba8b2a577f..d0a9328562 100644 --- a/app/Livewire/SelectTeam.php +++ b/app/Livewire/SelectTeam.php @@ -3,10 +3,14 @@ namespace App\Livewire; use App\Models\Team; +use Illuminate\Contracts\View\View; use Livewire\Component; class SelectTeam extends Component { + // mount()/selectTeam() intentionally have no return type: Livewire's + // redirect() returns a Redirector (not an Illuminate RedirectResponse), + // matching the convention in sibling components such as SwitchTeam. public function mount() { $user = auth()->user(); @@ -37,7 +41,7 @@ class SelectTeam extends Component return redirect()->route('dashboard'); } - public function render() + public function render(): View { return view('livewire.select-team', [ 'teams' => auth()->user()->teams, diff --git a/app/Models/User.php b/app/Models/User.php index ab9a817bfe..bb810b30fd 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -411,8 +411,16 @@ class User extends Authenticatable implements SendsEmail */ public function clearStoredTeamIfMatches(int $teamId): void { + // Atomic conditional update: only null the column when the database value + // still points at this team, so a newer team selection made concurrently + // (in another request) is preserved rather than clobbered. + static::query() + ->whereKey($this->getKey()) + ->where('current_team_id', $teamId) + ->update(['current_team_id' => null]); + if ($this->current_team_id === $teamId) { - $this->forceFill(['current_team_id' => null])->saveQuietly(); + $this->current_team_id = null; } } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 1c19fd1ca6..e9d17a4e08 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -570,8 +570,11 @@ function refreshSession(?Team $team = null): void $team = Team::find($currentTeam->id); } if (! $team) { - // Fall back to any team the user still belongs to. - $team = User::query()->find(Auth::id())?->teams()->first(); + // Fall back to the user's resolvable team (stored choice, or their + // sole team). Returns null for a multi-team user with no valid stored + // choice, so an arbitrary first team is never silently persisted — + // the user is sent to the selection screen instead. + $team = User::query()->find(Auth::id())?->resolveStoredTeam(); } } diff --git a/tests/Feature/Team/ActiveTeamPersistenceTest.php b/tests/Feature/Team/ActiveTeamPersistenceTest.php index b481cce314..4c0329e368 100644 --- a/tests/Feature/Team/ActiveTeamPersistenceTest.php +++ b/tests/Feature/Team/ActiveTeamPersistenceTest.php @@ -161,6 +161,28 @@ it('clears the stored team for members when their team is deleted', function () expect($member->fresh()->current_team_id)->toBeNull(); }); +it('clears the deleting owner stored team when they delete that team', function () { + [$owner, $personal, $shared] = userWithTwoTeams(); + $owner->update(['current_team_id' => $shared->id]); + + app(DeleteTeam::class)->handle($shared->fresh(), $owner); + + expect($owner->fresh()->current_team_id)->toBeNull(); +}); + +it('preserves a newer team selection when clearing a stale team', function () { + [$user, $personal, $second] = userWithTwoTeams(); + // In-memory model still points at the team being removed ($second)... + $user->update(['current_team_id' => $second->id]); + // ...but a concurrent request already switched the stored choice to $personal. + User::query()->whereKey($user->id)->update(['current_team_id' => $personal->id]); + + $user->clearStoredTeamIfMatches($second->id); + + // The atomic WHERE guard must not clobber the newer selection. + expect($user->fresh()->current_team_id)->toBe($personal->id); +}); + it('bounces users who already have an active team away from the select screen', function () { [$user, , $second] = userWithTwoTeams(); $user->update(['current_team_id' => $second->id]); From e52390ec03e2c0f2c6d9896cd43d5d33b30e4ca6 Mon Sep 17 00:00:00 2001 From: Aditya Tripathi Date: Tue, 8 Sep 2026 03:53:35 +0000 Subject: [PATCH 4/9] fix(team): resolve stored team on deletion and impersonation Use resolveStoredTeam() instead of teams()->first() when picking the next active team after a team deletion or when an admin switches into a user's account, so a valid stored preference wins over an arbitrary first team. DeleteTeam now returns null when the deletion leaves the owner with multiple teams, deferring to the selection screen instead of guessing. refreshSession also stops persisting current_team_id while impersonating, so viewing another user's account no longer overwrites their real last-active team. --- app/Actions/Team/DeleteTeam.php | 7 ++- app/Livewire/Admin/Index.php | 4 +- bootstrap/helpers/shared.php | 6 ++- .../Team/ActiveTeamPersistenceTest.php | 48 +++++++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/app/Actions/Team/DeleteTeam.php b/app/Actions/Team/DeleteTeam.php index bde72448d8..904460d342 100644 --- a/app/Actions/Team/DeleteTeam.php +++ b/app/Actions/Team/DeleteTeam.php @@ -60,7 +60,12 @@ class DeleteTeam $team->delete(); - return $user->teams()->first(); + // Resolve the next active team the same way login does: the user's + // stored choice when still valid, or their sole remaining team. + // Returns null for a multi-team user whose active team was just + // deleted, so refreshSession sends them to the selection screen + // instead of silently dropping them into an arbitrary first team. + return User::query()->find($user->id)?->resolveStoredTeam(); }); Cache::forget("user:{$user->id}:team:{$team->id}"); diff --git a/app/Livewire/Admin/Index.php b/app/Livewire/Admin/Index.php index 226d2e3329..f54f40ffd0 100644 --- a/app/Livewire/Admin/Index.php +++ b/app/Livewire/Admin/Index.php @@ -33,7 +33,7 @@ class Index extends Component if (session('impersonating')) { session()->forget('impersonating'); $user = User::find(0); - $team_to_switch_to = $user->teams->first(); + $team_to_switch_to = $user->resolveStoredTeam() ?? $user->teams->first(); Auth::login($user); refreshSession($team_to_switch_to); @@ -69,7 +69,7 @@ class Index extends Component if (! $user) { abort(404); } - $team_to_switch_to = $user->teams->first(); + $team_to_switch_to = $user->resolveStoredTeam() ?? $user->teams->first(); Auth::login($user); refreshSession($team_to_switch_to); diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index e9d17a4e08..5c9516b799 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -602,9 +602,11 @@ function refreshSession(?Team $team = null): void }); session(['currentTeam' => $team]); - // Persist the active team so it can be restored after logout/login. + // Persist the active team so it can be restored after logout/login — but + // never while an admin is impersonating, so viewing another user's account + // does not overwrite that user's real last-active team. $user = Auth::user(); - if ($user && $user->current_team_id !== $team->id) { + if ($user && ! session('impersonating') && $user->current_team_id !== $team->id) { $user->current_team_id = $team->id; $user->saveQuietly(); } diff --git a/tests/Feature/Team/ActiveTeamPersistenceTest.php b/tests/Feature/Team/ActiveTeamPersistenceTest.php index 4c0329e368..a0cadd5fdb 100644 --- a/tests/Feature/Team/ActiveTeamPersistenceTest.php +++ b/tests/Feature/Team/ActiveTeamPersistenceTest.php @@ -183,6 +183,54 @@ it('preserves a newer team selection when clearing a stale team', function () { expect($user->fresh()->current_team_id)->toBe($personal->id); }); +it('returns the sole remaining team when the deleting owner has one team left', function () { + [$owner, $personal, $shared] = userWithTwoTeams(); + $owner->update(['current_team_id' => $shared->id]); + + $next = app(DeleteTeam::class)->handle($shared->fresh(), $owner); + + expect($next?->id)->toBe($personal->id); +}); + +it('returns null (picker) when the deleting owner still has multiple teams left', function () { + [$owner, , $shared] = userWithTwoTeams(); + $third = Team::factory()->create(['show_boarding' => false]); + $owner->teams()->attach($third, ['role' => 'owner']); + $owner->update(['current_team_id' => $shared->id]); + + // Deleting the active team leaves personal + third: ambiguous, so no team is + // chosen silently and refreshSession(null) routes to the selection screen. + $next = app(DeleteTeam::class)->handle($shared->fresh(), $owner->fresh()); + + expect($next)->toBeNull(); +}); + +it('keeps the active team when the deleted team was not the active one', function () { + [$owner, $personal, $shared] = userWithTwoTeams(); + $third = Team::factory()->create(['show_boarding' => false]); + $owner->teams()->attach($third, ['role' => 'owner']); + $owner->update(['current_team_id' => $personal->id]); + + // Deleting a non-active team must not move the owner off their active team. + $next = app(DeleteTeam::class)->handle($shared->fresh(), $owner->fresh()); + + expect($next?->id)->toBe($personal->id); +}); + +it('does not persist current_team_id while impersonating', function () { + [$user, , $second] = userWithTwoTeams(); + $user->update(['current_team_id' => $second->id]); + $this->actingAs($user); + session(['impersonating' => true]); + + // Viewing a user's account switches the session team but must never + // overwrite that user's stored last-active team. + refreshSession($user->teams->first()); + + expect(data_get(session('currentTeam'), 'id'))->toBe($user->teams->first()->id) + ->and($user->fresh()->current_team_id)->toBe($second->id); +}); + it('bounces users who already have an active team away from the select screen', function () { [$user, , $second] = userWithTwoTeams(); $user->update(['current_team_id' => $second->id]); From e9bf2551ed5ee54c28a3ac9e75cea273b66076f3 Mon Sep 17 00:00:00 2001 From: Florian Pfitzer Date: Tue, 8 Sep 2026 10:08:36 +0200 Subject: [PATCH 5/9] fix: support generic SSH Git usernames --- .../MatchesManualWebhookApplications.php | 4 ++-- .../Project/New/PublicGitRepository.php | 6 ++--- app/Rules/ValidGitRepositoryUrl.php | 6 ++--- tests/Feature/Webhook/WebhookHmacTest.php | 23 +++++++++++++++++++ tests/Unit/ValidGitRepositoryUrlTest.php | 9 ++++++++ 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php index 0463790eb7..9fb3bb2de7 100644 --- a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php +++ b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php @@ -79,9 +79,9 @@ trait MatchesManualWebhookApplications if (is_array($parts) && isset($parts['scheme'])) { $path = data_get($parts, 'path'); - } elseif (Str::startsWith($gitRepository, 'git@') && str_contains($gitRepository, ':')) { + } elseif (preg_match('/^[A-Za-z0-9._-]+@[^:]+:/', $gitRepository) === 1) { $path = Str::after($gitRepository, ':'); - // scp-style SSH URLs embed a custom port as "git@host:2222/owner/repo". + // scp-style SSH URLs embed a custom port as "user@host:2222/owner/repo". // Strip the leading numeric port segment so the path matches the webhook // payload's owner/repo, consistent with convertGitUrl() in shared.php. $path = preg_replace('#^\d+/#', '', $path) ?? $path; diff --git a/app/Livewire/Project/New/PublicGitRepository.php b/app/Livewire/Project/New/PublicGitRepository.php index 81d65bc857..1ef1855c15 100644 --- a/app/Livewire/Project/New/PublicGitRepository.php +++ b/app/Livewire/Project/New/PublicGitRepository.php @@ -137,10 +137,8 @@ class PublicGitRepository extends Component throw new \RuntimeException('Invalid repository URL: '.$validator->errors()->first('repository_url')); } - if (str($this->repository_url)->startsWith('git@')) { - $github_instance = str($this->repository_url)->after('git@')->before(':'); - $repository = str($this->repository_url)->after(':')->before('.git'); - $this->repository_url = 'https://'.str($github_instance).'/'.$repository; + if (preg_match('/^(?[A-Za-z0-9._-]+)@(?[^:]+):(?.+)$/', $this->repository_url, $matches) === 1) { + $this->repository_url = 'https://'.$matches['host'].'/'.$matches['repository']; } if ( (str($this->repository_url)->startsWith('https://') || diff --git a/app/Rules/ValidGitRepositoryUrl.php b/app/Rules/ValidGitRepositoryUrl.php index ba1aed11b6..7cccfa5e97 100644 --- a/app/Rules/ValidGitRepositoryUrl.php +++ b/app/Rules/ValidGitRepositoryUrl.php @@ -77,15 +77,15 @@ class ValidGitRepositoryUrl implements ValidationRule } // Validate based on URL type - if (str_starts_with($value, 'git@')) { + if (preg_match('/^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+:/', $value)) { if (! $this->allowSSH) { $fail('SSH URLs are not allowed.'); return; } - // Validate SSH URL format (git@host:user/repo.git) - if (! preg_match('/^git@[a-zA-Z0-9\.\-]+:[a-zA-Z0-9\-_\/\.~]+$/', $value)) { + // Validate scp-style SSH URL format (user@host:user/repo.git) + if (! preg_match('/^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+:[a-zA-Z0-9\-_\/.~]+$/', $value)) { $fail('The :attribute is not a valid SSH repository URL.'); return; diff --git a/tests/Feature/Webhook/WebhookHmacTest.php b/tests/Feature/Webhook/WebhookHmacTest.php index 011b36731a..45e0da3776 100644 --- a/tests/Feature/Webhook/WebhookHmacTest.php +++ b/tests/Feature/Webhook/WebhookHmacTest.php @@ -542,6 +542,29 @@ describe('Manual Webhook Repository Matching', function () { expect($response->getContent())->not->toContain('No applications found'); }); + test('github matches an ssh repository URL with a non-git username', function () { + $app = createApplicationWithWebhook(overrides: [ + 'git_repository' => 'custom-user@git.example.com:test-org/test-repo.git', + ]); + $secret = $app->manual_webhook_secret_github; + + $payload = json_encode([ + 'ref' => 'refs/heads/main', + 'repository' => ['full_name' => 'test-org/test-repo'], + 'after' => 'abc123', + 'commits' => [], + ]); + + $response = $this->call('POST', '/webhooks/source/github/events/manual', [], [], [], [ + 'HTTP_X-GitHub-Event' => 'push', + 'HTTP_X-Hub-Signature-256' => 'sha256='.hash_hmac('sha256', $payload, $secret), + 'CONTENT_TYPE' => 'application/json', + ], $payload); + + $response->assertOk(); + expect($response->getContent())->not->toContain('No applications found'); + }); + test('gitlab matches scp-style ssh repository URL with custom port', function () { $app = createApplicationWithWebhook(overrides: [ 'git_repository' => 'git@gitlab.example.com:2222/services/xyz.git', diff --git a/tests/Unit/ValidGitRepositoryUrlTest.php b/tests/Unit/ValidGitRepositoryUrlTest.php index da467dc4d3..ee57657a00 100644 --- a/tests/Unit/ValidGitRepositoryUrlTest.php +++ b/tests/Unit/ValidGitRepositoryUrlTest.php @@ -107,6 +107,7 @@ it('validates SSH URLs when allowed', function () { 'git@github.com:user/repo.git', 'git@gitlab.com:user/repo.git', 'git@bitbucket.org:user/repo.git', + 'custom-user@git.example.com:organization/repository.git', ]; foreach ($validUrls as $url) { @@ -115,6 +116,14 @@ it('validates SSH URLs when allowed', function () { } }); +it('rejects non-SSH email-like repository URLs', function () { + $rule = new ValidGitRepositoryUrl; + + $validator = Validator::make(['url' => 'custom-user@git.example.com'], ['url' => $rule]); + + expect($validator->fails())->toBeTrue(); +}); + it('rejects SSH URLs when not allowed', function () { $rule = new ValidGitRepositoryUrl(allowSSH: false); From d25edb85515d5927abe38e2963985226a7ea9dcf Mon Sep 17 00:00:00 2001 From: peaklabs-dev <122374094+peaklabs-dev@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:43:05 +0200 Subject: [PATCH 6/9] fix(terminal): distinguish application containers across servers --- .../Project/Shared/ExecuteContainerCommand.php | 11 ++++++++--- .../shared/execute-container-command.blade.php | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/Livewire/Project/Shared/ExecuteContainerCommand.php b/app/Livewire/Project/Shared/ExecuteContainerCommand.php index aa26071020..e8202b8547 100644 --- a/app/Livewire/Project/Shared/ExecuteContainerCommand.php +++ b/app/Livewire/Project/Shared/ExecuteContainerCommand.php @@ -151,13 +151,18 @@ class ExecuteContainerCommand extends Component }); if ($this->containers->count() === 1) { - $this->selected_container = data_get($this->containers->first(), 'container.Names'); + $this->selected_container = $this->containerTarget($this->containers->first()); $this->connectToContainer(); } $this->containersLoaded = true; } + private function containerTarget(array $container): string + { + return data_get($container, 'server.uuid').':'.data_get($container, 'container.Names'); + } + public function updatedSelectedContainer() { if ($this->selected_container !== 'default') { @@ -202,12 +207,12 @@ class ExecuteContainerCommand extends Component try { $this->authorize('canAccessTerminal'); // Validate container name format - if (! ValidationPatterns::isValidContainerName($this->selected_container)) { + if (! ValidationPatterns::isValidContainerName(str($this->selected_container)->after(':')->value())) { throw new \InvalidArgumentException('Invalid container name format'); } // Verify container exists in our allowed list - $container = collect($this->containers)->firstWhere('container.Names', $this->selected_container); + $container = $this->containers->first(fn ($candidate) => $this->containerTarget($candidate) === $this->selected_container); if (is_null($container)) { throw new \RuntimeException('Container not found.'); } diff --git a/resources/views/livewire/project/shared/execute-container-command.blade.php b/resources/views/livewire/project/shared/execute-container-command.blade.php index dabd523f94..ef179a4ec9 100644 --- a/resources/views/livewire/project/shared/execute-container-command.blade.php +++ b/resources/views/livewire/project/shared/execute-container-command.blade.php @@ -34,7 +34,7 @@ $consoleThemeNames = collect($consoleThemes)->pluck('name', 'key'); $consoleThemeAccents = collect($consoleThemes)->pluck('accent', 'key'); $containerOptions = $containers->map(fn ($container) => [ - 'value' => data_get($container, 'container.Names'), + 'value' => data_get($container, 'server.uuid').':'.data_get($container, 'container.Names'), 'label' => data_get($container, 'container.Names').' · '.data_get($container, 'server.name'), ])->values(); @endphp From 83714ea395916edaf3adc522364dfd7e77e0f3f2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:33:45 +0200 Subject: [PATCH 7/9] fix(git): parse generic scp-style SSH URLs with custom users Centralize scp-style Git URL parsing so user@host:path (including custom usernames and embedded ports) is accepted and converted to HTTPS for public clones, API create, webhooks, validation, and commit/branch links. --- .../Api/ApplicationsController.php | 14 ++-- .../MatchesManualWebhookApplications.php | 9 +-- .../New/GithubPrivateRepositoryDeployKey.php | 8 +++ .../Project/New/PublicGitRepository.php | 5 +- app/Models/Application.php | 44 +++++++----- app/Rules/ValidGitRepositoryUrl.php | 5 +- bootstrap/helpers/shared.php | 65 +++++++++++++++-- .../Api/PublicApplicationSshUrlApiTest.php | 53 ++++++++++++++ .../Feature/GitHttpTransportCommandsTest.php | 22 ++++++ .../Feature/Helpers/ConvertingGitUrlsTest.php | 8 +++ .../Feature/PublicGitRepositorySshUrlTest.php | 34 +++++++++ tests/Feature/Webhook/WebhookHmacTest.php | 23 ++++++ tests/Unit/ApplicationGitCommitLinkTest.php | 39 +++++++++++ tests/Unit/ScpStyleGitUrlTest.php | 70 +++++++++++++++++++ tests/Unit/ValidGitRepositoryUrlTest.php | 3 + 15 files changed, 361 insertions(+), 41 deletions(-) create mode 100644 tests/Feature/Api/PublicApplicationSshUrlApiTest.php create mode 100644 tests/Feature/PublicGitRepositorySshUrlTest.php create mode 100644 tests/Unit/ScpStyleGitUrlTest.php diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index dbe0f86336..67fd515bcd 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -1460,7 +1460,13 @@ class ApplicationsController extends Controller $application->docker_compose_domains = json_encode($dockerComposeDomainsJson); $application->domain_port_overrides = $domainPortOverrides; } - $repository_url_parsed = Url::fromString($request->git_repository); + $gitRepository = $application->git_repository; + $httpsRepository = scpStyleGitUrlToHttps($gitRepository); + if (is_string($httpsRepository)) { + $gitRepository = $httpsRepository; + $application->git_repository = $httpsRepository; + } + $repository_url_parsed = Url::fromString($gitRepository); $git_host = $repository_url_parsed->getHost(); if ($git_host === 'github.com') { $application->source_type = GithubApp::class; @@ -1622,11 +1628,7 @@ class ApplicationsController extends Controller return response()->json(['message' => 'Failed to generate Github App token.'], 400); } - $gitRepository = $request->git_repository; - if (str($gitRepository)->startsWith('http') || str($gitRepository)->contains('github.com')) { - $gitRepository = str($gitRepository)->replace('https://', '')->replace('http://', '')->replace('github.com/', ''); - } - $gitRepository = str($gitRepository)->trim('/')->replaceEnd('.git', '')->toString(); + $gitRepository = gitRepositorySlug($request->git_repository); // Use direct API call to verify repository access instead of loading all repositories // This is much faster and avoids timeouts for GitHub Apps with many repositories diff --git a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php index 9fb3bb2de7..65c92f1349 100644 --- a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php +++ b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php @@ -5,7 +5,6 @@ namespace App\Http\Controllers\Webhook\Concerns; use App\Models\Application; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; -use Illuminate\Support\Str; trait MatchesManualWebhookApplications { @@ -79,12 +78,8 @@ trait MatchesManualWebhookApplications if (is_array($parts) && isset($parts['scheme'])) { $path = data_get($parts, 'path'); - } elseif (preg_match('/^[A-Za-z0-9._-]+@[^:]+:/', $gitRepository) === 1) { - $path = Str::after($gitRepository, ':'); - // scp-style SSH URLs embed a custom port as "user@host:2222/owner/repo". - // Strip the leading numeric port segment so the path matches the webhook - // payload's owner/repo, consistent with convertGitUrl() in shared.php. - $path = preg_replace('#^\d+/#', '', $path) ?? $path; + } elseif (($scp = parseScpStyleGitUrl($gitRepository)) !== null) { + $path = $scp['path']; } else { $path = $gitRepository; } diff --git a/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php b/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php index 502a69bec4..98c5395bfd 100644 --- a/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php +++ b/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php @@ -216,6 +216,14 @@ class GithubPrivateRepositoryDeployKey extends Component throw new \RuntimeException('Invalid repository URL: '.$validator->errors()->first('repository_url')); } + if (($scp = parseScpStyleGitUrl($this->repository_url)) !== null) { + $this->git_host = $scp['host']; + $this->git_repository = $this->repository_url; + $this->git_source = 'other'; + + return; + } + $this->repository_url_parsed = Url::fromString($this->repository_url); $this->git_host = $this->repository_url_parsed->getHost(); $this->git_repository = $this->repository_url_parsed->getSegment(1).'/'.$this->repository_url_parsed->getSegment(2); diff --git a/app/Livewire/Project/New/PublicGitRepository.php b/app/Livewire/Project/New/PublicGitRepository.php index 1ef1855c15..a031c50c00 100644 --- a/app/Livewire/Project/New/PublicGitRepository.php +++ b/app/Livewire/Project/New/PublicGitRepository.php @@ -137,8 +137,9 @@ class PublicGitRepository extends Component throw new \RuntimeException('Invalid repository URL: '.$validator->errors()->first('repository_url')); } - if (preg_match('/^(?[A-Za-z0-9._-]+)@(?[^:]+):(?.+)$/', $this->repository_url, $matches) === 1) { - $this->repository_url = 'https://'.$matches['host'].'/'.$matches['repository']; + $httpsRepositoryUrl = scpStyleGitUrlToHttps($this->repository_url); + if (is_string($httpsRepositoryUrl)) { + $this->repository_url = $httpsRepositoryUrl; } if ( (str($this->repository_url)->startsWith('https://') || diff --git a/app/Models/Application.php b/app/Models/Application.php index cec8d501ab..2d46291c5b 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -663,15 +663,13 @@ class Application extends BaseModel return "{$this->source->html_url}/{$this->git_repository}/tree/{$this->git_branch}{$base_dir}"; } - // Convert the SSH URL to HTTPS URL - if (strpos($this->git_repository, 'git@') === 0) { - $git_repository = str_replace(['git@', ':', '.git'], ['', '/', ''], $this->git_repository); - + $httpsRepository = $this->httpsUrlFromScpStyleGitRepository(); + if (is_string($httpsRepository)) { if (str($this->git_repository)->contains('bitbucket')) { - return "https://{$git_repository}/src/{$this->git_branch}{$base_dir}"; + return "{$httpsRepository}/src/{$this->git_branch}{$base_dir}"; } - return "https://{$git_repository}/tree/{$this->git_branch}{$base_dir}"; + return "{$httpsRepository}/tree/{$this->git_branch}{$base_dir}"; } return $this->git_repository; @@ -686,11 +684,9 @@ class Application extends BaseModel if (! is_null($this->source?->html_url) && ! is_null($this->git_repository) && ! is_null($this->git_branch)) { return "{$this->source->html_url}/{$this->git_repository}/settings/hooks"; } - // Convert the SSH URL to HTTPS URL - if (strpos($this->git_repository, 'git@') === 0) { - $git_repository = str_replace(['git@', ':', '.git'], ['', '/', ''], $this->git_repository); - - return "https://{$git_repository}/settings/hooks"; + $httpsRepository = $this->httpsUrlFromScpStyleGitRepository(); + if (is_string($httpsRepository)) { + return "{$httpsRepository}/settings/hooks"; } return $this->git_repository; @@ -705,11 +701,9 @@ class Application extends BaseModel if (! is_null($this->source?->html_url) && ! is_null($this->git_repository) && ! is_null($this->git_branch)) { return "{$this->source->html_url}/{$this->git_repository}/commits/{$this->git_branch}"; } - // Convert the SSH URL to HTTPS URL - if (strpos($this->git_repository, 'git@') === 0) { - $git_repository = str_replace(['git@', ':', '.git'], ['', '/', ''], $this->git_repository); - - return "https://{$git_repository}/commits/{$this->git_branch}"; + $httpsRepository = $this->httpsUrlFromScpStyleGitRepository(); + if (is_string($httpsRepository)) { + return "{$httpsRepository}/commits/{$this->git_branch}"; } return $this->git_repository; @@ -728,8 +722,9 @@ class Application extends BaseModel } $git_repository = $this->git_repository; - if (strpos($this->git_repository, 'git@') === 0) { - $git_repository = preg_replace('/^git@([^:]+):/', 'https://$1/', $git_repository); + $httpsRepository = scpStyleGitUrlToHttps($git_repository); + if (is_string($httpsRepository)) { + $git_repository = $httpsRepository; } elseif (str($this->git_repository)->startsWith('ssh://')) { $git_repository = 'https://'.parse_url($git_repository, PHP_URL_HOST).parse_url($git_repository, PHP_URL_PATH); } @@ -746,6 +741,17 @@ class Application extends BaseModel return $url->__toString(); } + private function httpsUrlFromScpStyleGitRepository(): ?string + { + $httpsRepository = scpStyleGitUrlToHttps($this->git_repository); + + if (! is_string($httpsRepository)) { + return null; + } + + return Str::replaceEnd('.git', '', $httpsRepository); + } + public function dockerfileLocation(): Attribute { return Attribute::make( @@ -1477,7 +1483,7 @@ class Application extends BaseModel // Check if .gitmodules file exists before running submodule commands $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && if [ -f .gitmodules ]; then"; if ($public) { - $git_clone_command = "{$git_clone_command} sed -i \"s#git@\(.*\):#https://\\1/#g\" {$escapedBaseDir}/.gitmodules || true &&"; + $git_clone_command = "{$git_clone_command} sed -i \"s#[A-Za-z0-9._-]*@\(.*\):#https://\\1/#g\" {$escapedBaseDir}/.gitmodules || true &&"; } // Add shallow submodules flag if shallow clone is enabled $submoduleFlags = $isShallowCloneEnabled ? '--depth=1' : ''; diff --git a/app/Rules/ValidGitRepositoryUrl.php b/app/Rules/ValidGitRepositoryUrl.php index 7cccfa5e97..29e219bd35 100644 --- a/app/Rules/ValidGitRepositoryUrl.php +++ b/app/Rules/ValidGitRepositoryUrl.php @@ -85,7 +85,8 @@ class ValidGitRepositoryUrl implements ValidationRule } // Validate scp-style SSH URL format (user@host:user/repo.git) - if (! preg_match('/^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+:[a-zA-Z0-9\-_\/.~]+$/', $value)) { + $scp = parseScpStyleGitUrl($value); + if ($scp === null || preg_match('/^[a-zA-Z0-9.-]+$/', $scp['host']) !== 1 || preg_match('/^[a-zA-Z0-9\-_\/.~]+$/', $scp['path']) !== 1) { $fail('The :attribute is not a valid SSH repository URL.'); return; @@ -149,7 +150,7 @@ class ValidGitRepositoryUrl implements ValidationRule return; } } else { - $fail('The :attribute must start with https://, http://, git://, or git@.'); + $fail('The :attribute must start with https://, http://, git://, or be an SSH URL (user@host:path).'); return; } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 5c9516b799..d1f5e4016b 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -4358,6 +4358,62 @@ NGINX; } } +/** + * Parse an scp-style SSH Git URL (`user@host:path` or `user@host:port/path`). + * + * @return array{user: string, host: string, port: ?string, path: string}|null + */ +function parseScpStyleGitUrl(?string $gitRepository): ?array +{ + if (! is_string($gitRepository) || $gitRepository === '') { + return null; + } + + if (preg_match('/^(?[A-Za-z0-9._-]+)@(?[^:]+):(?:(?\d+)\/)?(?.+)$/', $gitRepository, $matches) !== 1) { + return null; + } + + $host = trim($matches['host']); + $path = ltrim($matches['path'], '/'); + + if ($host === '' || $path === '') { + return null; + } + + return [ + 'user' => $matches['user'], + 'host' => $host, + 'port' => ($matches['port'] ?? '') === '' ? null : $matches['port'], + 'path' => $path, + ]; +} + +function scpStyleGitUrlToHttps(?string $gitRepository): ?string +{ + $parts = parseScpStyleGitUrl($gitRepository); + + if ($parts === null) { + return null; + } + + return 'https://'.$parts['host'].'/'.$parts['path']; +} + +function gitRepositorySlug(?string $gitRepository): string +{ + if (! is_string($gitRepository) || $gitRepository === '') { + return ''; + } + + if (($scp = parseScpStyleGitUrl($gitRepository)) !== null) { + $gitRepository = $scp['path']; + } elseif (str($gitRepository)->startsWith('http') || str($gitRepository)->contains('github.com')) { + $gitRepository = str($gitRepository)->replace('https://', '')->replace('http://', '')->replace('github.com/', ''); + } + + return str($gitRepository)->trim('/')->replaceEnd('.git', '')->toString(); +} + function convertGitUrl(string $gitRepository, string $deploymentType, GithubApp|GitlabApp|null $source = null): array { $repository = $gitRepository; @@ -4368,7 +4424,6 @@ function convertGitUrl(string $gitRepository, string $deploymentType, GithubApp| 'repository' => $gitRepository, ]; $sshMatches = []; - $matches = []; // Let's try and parse the string to detect if it's a valid SSH string or not preg_match('/((.*?)\:\/\/)?(.*@.*:.*)/', $gitRepository, $sshMatches); @@ -4403,11 +4458,11 @@ function convertGitUrl(string $gitRepository, string $deploymentType, GithubApp| $providerInfo['port'] = (string) $parsedRepository['port']; } } else { - preg_match('/^(?[^:]+):(?\d+)\/(?.+)$/', $normalizedRepository, $matches); + $scp = parseScpStyleGitUrl($normalizedRepository); - if (! empty($matches['port'])) { - $providerInfo['port'] = $matches['port']; - $repository = "{$matches['host']}:{$matches['path']}"; + if ($scp !== null && $scp['port'] !== null) { + $providerInfo['port'] = $scp['port']; + $repository = "{$scp['user']}@{$scp['host']}:{$scp['path']}"; } } diff --git a/tests/Feature/Api/PublicApplicationSshUrlApiTest.php b/tests/Feature/Api/PublicApplicationSshUrlApiTest.php new file mode 100644 index 0000000000..03c18ab48b --- /dev/null +++ b/tests/Feature/Api/PublicApplicationSshUrlApiTest.php @@ -0,0 +1,53 @@ + 'file']); + Storage::fake('ssh-keys'); + InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $this->bearerToken = $this->user->createToken('public-ssh-url-api-test', ['*'])->plainTextToken; + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->first(); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); +}); + +test('public application api converts scp-style ssh urls to https', function () { + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->postJson('/api/v1/applications/public', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'server_uuid' => $this->server->uuid, + 'git_repository' => 'custom-user@git.example.com:2222/organization/repository.git', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '3000', + 'autogenerate_domain' => false, + ]); + + $response->assertCreated(); + + $application = Application::where('uuid', $response->json('uuid'))->firstOrFail(); + + expect($application->git_repository)->toBe('https://git.example.com/organization/repository.git'); +}); diff --git a/tests/Feature/GitHttpTransportCommandsTest.php b/tests/Feature/GitHttpTransportCommandsTest.php index d6f9f13371..3d0295d078 100644 --- a/tests/Feature/GitHttpTransportCommandsTest.php +++ b/tests/Feature/GitHttpTransportCommandsTest.php @@ -69,6 +69,28 @@ it('applies http 1 transport to https fetches after clone', function () { ->toContain("git -c http.version=HTTP/1.1 -c advice.detachedHead=false checkout 'abc123def456abc123def456abc123def456abc1'"); }); +it('rewrites generic ssh submodule remotes to https for public clones', function () { + $application = applicationWithGitSettings(shallow: false); + $application->settings->is_git_submodules_enabled = true; + + $source = new GithubApp; + $source->forceFill([ + 'html_url' => 'https://github.com', + 'api_url' => 'https://api.github.com', + 'is_public' => true, + ]); + $application->setRelation('source', $source); + + $result = $application->generateGitImportCommands( + deployment_uuid: 'test-deployment', + exec_in_docker: false, + ); + + expect($result['commands']) + ->toContain('sed -i "s#[A-Za-z0-9._-]*@\(.*\):#https://\\1/#g"') + ->not->toContain('s#git@\(.*\):#https://\\1/#g'); +}); + it('does not add http transport config to ssh deploy key clones', function () { $application = applicationWithGitSettings(); $application->private_key_id = 1; diff --git a/tests/Feature/Helpers/ConvertingGitUrlsTest.php b/tests/Feature/Helpers/ConvertingGitUrlsTest.php index 96b19fcc96..1860eceeda 100644 --- a/tests/Feature/Helpers/ConvertingGitUrlsTest.php +++ b/tests/Feature/Helpers/ConvertingGitUrlsTest.php @@ -61,6 +61,14 @@ test('convertGitUrlsForSourceAndSshUrlWithCustomPort', function () { ]); }); +test('convertGitUrlsForSourceAndSshUrlWithCustomUsernameAndPort', function () { + $result = convertGitUrl('custom-user@git.domain.com:766/group/project.git', 'source', null); + expect($result)->toBe([ + 'repository' => 'custom-user@git.domain.com:group/project.git', + 'port' => '766', + ]); +}); + test('convertGitUrlsForSourceAndSshUrlSchemeWithCustomPort', function () { $result = convertGitUrl('ssh://git@192.168.56.11:22222/User/Repo.git', 'source', null); expect($result)->toBe([ diff --git a/tests/Feature/PublicGitRepositorySshUrlTest.php b/tests/Feature/PublicGitRepositorySshUrlTest.php new file mode 100644 index 0000000000..80df73a9f6 --- /dev/null +++ b/tests/Feature/PublicGitRepositorySshUrlTest.php @@ -0,0 +1,34 @@ +create(); + $user = User::factory()->create(); + $team->members()->attach($user->id, ['role' => 'owner']); + + $this->actingAs($user); + session(['currentTeam' => $team]); +}); + +test('converts scp-style ssh urls with custom usernames to https', function () { + Livewire::test(PublicGitRepository::class, ['type' => 'public']) + ->set('repository_url', 'custom-user@git.example.com:organization/repository.git') + ->call('loadBranch') + ->assertSet('repository_url', 'https://git.example.com/organization/repository.git') + ->assertSet('branchFound', true); +}); + +test('strips custom ports when converting scp-style ssh urls to https', function () { + Livewire::test(PublicGitRepository::class, ['type' => 'public']) + ->set('repository_url', 'custom-user@git.example.com:2222/organization/repository.git') + ->call('loadBranch') + ->assertSet('repository_url', 'https://git.example.com/organization/repository.git') + ->assertSet('branchFound', true); +}); diff --git a/tests/Feature/Webhook/WebhookHmacTest.php b/tests/Feature/Webhook/WebhookHmacTest.php index 45e0da3776..dfd7990185 100644 --- a/tests/Feature/Webhook/WebhookHmacTest.php +++ b/tests/Feature/Webhook/WebhookHmacTest.php @@ -565,6 +565,29 @@ describe('Manual Webhook Repository Matching', function () { expect($response->getContent())->not->toContain('No applications found'); }); + test('github matches an ssh repository URL with a non-git username and custom port', function () { + $app = createApplicationWithWebhook(overrides: [ + 'git_repository' => 'custom-user@git.example.com:2222/test-org/test-repo.git', + ]); + $secret = $app->manual_webhook_secret_github; + + $payload = json_encode([ + 'ref' => 'refs/heads/main', + 'repository' => ['full_name' => 'test-org/test-repo'], + 'after' => 'abc123', + 'commits' => [], + ]); + + $response = $this->call('POST', '/webhooks/source/github/events/manual', [], [], [], [ + 'HTTP_X-GitHub-Event' => 'push', + 'HTTP_X-Hub-Signature-256' => 'sha256='.hash_hmac('sha256', $payload, $secret), + 'CONTENT_TYPE' => 'application/json', + ], $payload); + + $response->assertOk(); + expect($response->getContent())->not->toContain('No applications found'); + }); + test('gitlab matches scp-style ssh repository URL with custom port', function () { $app = createApplicationWithWebhook(overrides: [ 'git_repository' => 'git@gitlab.example.com:2222/services/xyz.git', diff --git a/tests/Unit/ApplicationGitCommitLinkTest.php b/tests/Unit/ApplicationGitCommitLinkTest.php index 378384fe8f..6ebbaf8eea 100644 --- a/tests/Unit/ApplicationGitCommitLinkTest.php +++ b/tests/Unit/ApplicationGitCommitLinkTest.php @@ -17,6 +17,14 @@ it('generates commit links for direct repository remotes', function (string $rep 'git@github.com:coollabsio/coolify.git', 'https://github.com/coollabsio/coolify/commit/1234567890abcdef', ], + 'SSH remote with custom username' => [ + 'custom-user@git.example.com:coollabsio/coolify.git', + 'https://git.example.com/coollabsio/coolify/commit/1234567890abcdef', + ], + 'SSH remote with custom username and port' => [ + 'custom-user@git.example.com:2222/coollabsio/coolify.git', + 'https://git.example.com/coollabsio/coolify/commit/1234567890abcdef', + ], 'SSH URL' => [ 'ssh://git@gitlab.com/coollabsio/coolify.git', 'https://gitlab.com/coollabsio/coolify/commit/1234567890abcdef', @@ -37,3 +45,34 @@ it('does not generate commit links from incomplete repository URLs', function (s 'missing host' => 'https://', 'missing scheme' => 'github.com/coollabsio/coolify', ]); + +it('converts scp-style remotes with generic usernames into https repository links', function (string $repository, string $expectedBranch, string $expectedCommits, string $expectedWebhook) { + $application = new Application; + $application->setRelation('source', null); + $application->git_repository = $repository; + $application->git_branch = 'main'; + $application->base_directory = '/'; + + expect($application->gitBranchLocation)->toBe($expectedBranch) + ->and($application->gitCommits)->toBe($expectedCommits) + ->and($application->gitWebhook)->toBe($expectedWebhook); +})->with([ + 'git username' => [ + 'git@github.com:coollabsio/coolify.git', + 'https://github.com/coollabsio/coolify/tree/main/', + 'https://github.com/coollabsio/coolify/commits/main', + 'https://github.com/coollabsio/coolify/settings/hooks', + ], + 'custom username' => [ + 'custom-user@git.example.com:organization/repository.git', + 'https://git.example.com/organization/repository/tree/main/', + 'https://git.example.com/organization/repository/commits/main', + 'https://git.example.com/organization/repository/settings/hooks', + ], + 'custom username and port' => [ + 'custom-user@git.example.com:2222/organization/repository.git', + 'https://git.example.com/organization/repository/tree/main/', + 'https://git.example.com/organization/repository/commits/main', + 'https://git.example.com/organization/repository/settings/hooks', + ], +]); diff --git a/tests/Unit/ScpStyleGitUrlTest.php b/tests/Unit/ScpStyleGitUrlTest.php new file mode 100644 index 0000000000..ec95e341cd --- /dev/null +++ b/tests/Unit/ScpStyleGitUrlTest.php @@ -0,0 +1,70 @@ +toBe($expected); +})->with([ + 'git username' => [ + 'git@github.com:organization/repository.git', + [ + 'user' => 'git', + 'host' => 'github.com', + 'port' => null, + 'path' => 'organization/repository.git', + ], + ], + 'custom username' => [ + 'custom-user@git.example.com:organization/repository.git', + [ + 'user' => 'custom-user', + 'host' => 'git.example.com', + 'port' => null, + 'path' => 'organization/repository.git', + ], + ], + 'custom username and port' => [ + 'custom-user@git.example.com:2222/organization/repository.git', + [ + 'user' => 'custom-user', + 'host' => 'git.example.com', + 'port' => '2222', + 'path' => 'organization/repository.git', + ], + ], +]); + +it('converts scp-style ssh git urls to https without embedding custom ports in the path', function (string $url, string $expected) { + expect(scpStyleGitUrlToHttps($url))->toBe($expected); +})->with([ + 'git username' => [ + 'git@github.com:organization/repository.git', + 'https://github.com/organization/repository.git', + ], + 'custom username' => [ + 'custom-user@git.example.com:organization/repository.git', + 'https://git.example.com/organization/repository.git', + ], + 'custom username and port' => [ + 'custom-user@git.example.com:2222/organization/repository.git', + 'https://git.example.com/organization/repository.git', + ], +]); + +it('rejects non-scp-style git urls', function (string $url) { + expect(parseScpStyleGitUrl($url))->toBeNull() + ->and(scpStyleGitUrlToHttps($url))->toBeNull(); +})->with([ + 'https' => 'https://github.com/organization/repository.git', + 'email without path' => 'custom-user@git.example.com', + 'ssh scheme' => 'ssh://git@github.com/organization/repository.git', + 'empty' => '', +]); + +it('normalizes github app repository slugs from scp-style ssh urls', function (string $url, string $expected) { + expect(gitRepositorySlug($url))->toBe($expected); +})->with([ + 'https' => ['https://github.com/organization/repository.git', 'organization/repository'], + 'owner/repo' => ['organization/repository', 'organization/repository'], + 'git username' => ['git@github.com:organization/repository.git', 'organization/repository'], + 'custom username' => ['custom-user@git.example.com:organization/repository.git', 'organization/repository'], + 'custom username and port' => ['custom-user@git.example.com:2222/organization/repository.git', 'organization/repository'], +]); diff --git a/tests/Unit/ValidGitRepositoryUrlTest.php b/tests/Unit/ValidGitRepositoryUrlTest.php index ee57657a00..b4bcb465aa 100644 --- a/tests/Unit/ValidGitRepositoryUrlTest.php +++ b/tests/Unit/ValidGitRepositoryUrlTest.php @@ -108,6 +108,8 @@ it('validates SSH URLs when allowed', function () { 'git@gitlab.com:user/repo.git', 'git@bitbucket.org:user/repo.git', 'custom-user@git.example.com:organization/repository.git', + 'custom-user@git.example.com:2222/organization/repository.git', + 'enterprise-user@enterprise.ghe.com:organization/repository.git', ]; foreach ($validUrls as $url) { @@ -130,6 +132,7 @@ it('rejects SSH URLs when not allowed', function () { $invalidUrls = [ 'git@github.com:user/repo.git', 'git@gitlab.com:user/repo.git', + 'custom-user@git.example.com:organization/repository.git', ]; foreach ($invalidUrls as $url) { From d71a72a45df1b24aa30701ec061e3d1445744cc9 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:21:32 +0200 Subject: [PATCH 8/9] fix(applications): flatten mobile config nav and pin domain save Replace the collapsible mobile configuration menu with an always-visible responsive grid, stack the mobile heading, and swap the domain editor unsaved bar for a sticky Save footer with a scrollable form body. --- .../configuration-sidebar.blade.php | 94 +++++-------------- .../application/configuration.blade.php | 2 +- .../project/application/domains.blade.php | 71 +++++++------- .../project/application/heading.blade.php | 7 +- tests/Feature/ApplicationDomainsTest.php | 11 ++- .../ResourceHeadingUnifiedNavbarTest.php | 35 ++++--- .../ServiceDatabaseVerticalNavigationTest.php | 18 ++++ .../Browser/ApplicationConfigurationTest.php | 18 ++-- 8 files changed, 124 insertions(+), 132 deletions(-) diff --git a/resources/views/components/application/configuration-sidebar.blade.php b/resources/views/components/application/configuration-sidebar.blade.php index 369d498c96..9a1405affb 100644 --- a/resources/views/components/application/configuration-sidebar.blade.php +++ b/resources/views/components/application/configuration-sidebar.blade.php @@ -238,60 +238,28 @@ ]; @endphp -@php - $activeMenuLabel = collect($groupedMenuItems)->flatMap(fn ($items) => $items)->firstWhere('active', true)['label'] ?? 'Settings'; -@endphp