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.
This commit is contained in:
Aditya Tripathi 2026-08-25 17:25:32 +00:00 committed by Andras Bacsai
parent 2b92fb86b5
commit f511921895
5 changed files with 45 additions and 4 deletions

View file

@ -54,6 +54,10 @@ public function handle(Team $team, User $user): ?Team
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();

View file

@ -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 @@ public function selectTeam(int $teamId)
return redirect()->route('dashboard');
}
public function render()
public function render(): View
{
return view('livewire.select-team', [
'teams' => auth()->user()->teams,

View file

@ -411,8 +411,16 @@ public function resolveStoredTeam(): ?Team
*/
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;
}
}

View file

@ -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();
}
}

View file

@ -161,6 +161,28 @@ function userWithTwoTeams(): array
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]);