From 3911a0305c0177c5bb77659883b3c59709004570 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 13 May 2026 10:11:40 +0200 Subject: [PATCH 1/2] fix(api-tokens): persist expiration warning state Track when expiration warnings are sent on personal access tokens so repeated job runs or cache flushes do not send duplicate notifications. --- app/Jobs/ApiTokenExpirationWarningJob.php | 31 +++++++++++----- app/Models/PersonalAccessToken.php | 8 ++++ ...ent_at_to_personal_access_tokens_table.php | 30 +++++++++++++++ .../Feature/ApiTokenExpirationWarningTest.php | 37 +++++++++++++++++-- 4 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 database/migrations/2026_05_13_000000_add_expiration_warning_sent_at_to_personal_access_tokens_table.php diff --git a/app/Jobs/ApiTokenExpirationWarningJob.php b/app/Jobs/ApiTokenExpirationWarningJob.php index a8f388c85..810919c6a 100644 --- a/app/Jobs/ApiTokenExpirationWarningJob.php +++ b/app/Jobs/ApiTokenExpirationWarningJob.php @@ -12,7 +12,6 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; -use Illuminate\Support\Facades\RateLimiter; use Laravel\Horizon\Contracts\Silenced; class ApiTokenExpirationWarningJob implements ShouldBeEncrypted, ShouldQueue, Silenced @@ -29,20 +28,34 @@ public function handle(): void ->whereNotNull('expires_at') ->where('expires_at', '>', now()) ->where('expires_at', '<=', now()->addDay()) + ->whereNull('api_token_expiration_warning_sent_at') ->where('tokenable_type', User::class) ->chunkById(100, function ($tokens) { foreach ($tokens as $token) { if (! $token->team_id) { continue; } - RateLimiter::attempt( - 'api-token-expiring:'.$token->id, - $maxAttempts = 0, - function () use ($token) { - Team::find($token->team_id)?->notify(new ApiTokenExpiringNotification($token)); - }, - $decaySeconds = 7 * 24 * 3600, - ); + + $team = Team::find($token->team_id); + if (! $team) { + continue; + } + + $warningSentAt = now(); + $markedAsSent = PersonalAccessToken::query() + ->whereKey($token->getKey()) + ->whereNotNull('expires_at') + ->where('expires_at', '>', now()) + ->where('expires_at', '<=', now()->addDay()) + ->whereNull('api_token_expiration_warning_sent_at') + ->update(['api_token_expiration_warning_sent_at' => $warningSentAt]); + + if ($markedAsSent !== 1) { + continue; + } + + $token->forceFill(['api_token_expiration_warning_sent_at' => $warningSentAt]); + $team->notify(new ApiTokenExpiringNotification($token)); } }); } diff --git a/app/Models/PersonalAccessToken.php b/app/Models/PersonalAccessToken.php index 398046a7c..503377bec 100644 --- a/app/Models/PersonalAccessToken.php +++ b/app/Models/PersonalAccessToken.php @@ -11,6 +11,14 @@ class PersonalAccessToken extends SanctumPersonalAccessToken 'token', 'abilities', 'expires_at', + 'api_token_expiration_warning_sent_at', 'team_id', ]; + + protected function casts(): array + { + return [ + 'api_token_expiration_warning_sent_at' => 'datetime', + ]; + } } diff --git a/database/migrations/2026_05_13_000000_add_expiration_warning_sent_at_to_personal_access_tokens_table.php b/database/migrations/2026_05_13_000000_add_expiration_warning_sent_at_to_personal_access_tokens_table.php new file mode 100644 index 000000000..728115482 --- /dev/null +++ b/database/migrations/2026_05_13_000000_add_expiration_warning_sent_at_to_personal_access_tokens_table.php @@ -0,0 +1,30 @@ +timestamp('api_token_expiration_warning_sent_at')->nullable()->after('expires_at'); + $table->index(['expires_at', 'api_token_expiration_warning_sent_at'], 'personal_access_tokens_expiration_warning_index'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('personal_access_tokens', function (Blueprint $table) { + $table->dropIndex('personal_access_tokens_expiration_warning_index'); + $table->dropColumn('api_token_expiration_warning_sent_at'); + }); + } +}; diff --git a/tests/Feature/ApiTokenExpirationWarningTest.php b/tests/Feature/ApiTokenExpirationWarningTest.php index 5255581dd..67380f047 100644 --- a/tests/Feature/ApiTokenExpirationWarningTest.php +++ b/tests/Feature/ApiTokenExpirationWarningTest.php @@ -29,11 +29,12 @@ Notification::fake(); }); -function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt): PersonalAccessToken +function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt, ?Carbon $warningSentAt = null): PersonalAccessToken { $plain = $user->createToken('t-'.uniqid(), ['read'], $expiresAt); $token = $plain->accessToken; $token->team_id = $team->id; + $token->api_token_expiration_warning_sent_at = $warningSentAt; $token->save(); return $token->fresh(); @@ -41,14 +42,15 @@ function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt): Person describe('ApiTokenExpirationWarningJob', function () { test('notifies team when token expires within 24h', function () { - createTokenExpiring($this->user, $this->team, now()->addHours(23)); + $token = createTokenExpiring($this->user, $this->team, now()->addHours(23)); (new ApiTokenExpirationWarningJob)->handle(); Notification::assertSentTo($this->team, ApiTokenExpiringNotification::class); + expect($token->fresh()->api_token_expiration_warning_sent_at)->not->toBeNull(); }); - test('rate limiter prevents duplicate warnings on repeat runs', function () { + test('database marker prevents duplicate warnings on repeat runs', function () { createTokenExpiring($this->user, $this->team, now()->addHours(12)); (new ApiTokenExpirationWarningJob)->handle(); @@ -57,6 +59,35 @@ function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt): Person Notification::assertSentToTimes($this->team, ApiTokenExpiringNotification::class, 1); }); + test('database marker prevents duplicate warnings after cache is flushed', function () { + createTokenExpiring($this->user, $this->team, now()->addHours(12)); + + (new ApiTokenExpirationWarningJob)->handle(); + + Cache::flush(); + + (new ApiTokenExpirationWarningJob)->handle(); + + Notification::assertSentToTimes($this->team, ApiTokenExpiringNotification::class, 1); + }); + + test('skips tokens that already have an expiration warning marker', function () { + createTokenExpiring($this->user, $this->team, now()->addHours(12), now()->subHour()); + + (new ApiTokenExpirationWarningJob)->handle(); + + Notification::assertNothingSent(); + }); + + test('notifies once for each unmarked expiring token', function () { + createTokenExpiring($this->user, $this->team, now()->addHours(12)); + createTokenExpiring($this->user, $this->team, now()->addHours(23)); + + (new ApiTokenExpirationWarningJob)->handle(); + + Notification::assertSentToTimes($this->team, ApiTokenExpiringNotification::class, 2); + }); + test('skips tokens expiring more than 24h out', function () { createTokenExpiring($this->user, $this->team, now()->addDays(3)); From 1522c510cff764c99c341eb4cbbea35382e89962 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 13 May 2026 10:28:32 +0200 Subject: [PATCH 2/2] fix(api-tokens): mark expiration warning after notification Ensure failed token expiration warning notifications do not persist the warning marker, allowing the job to retry later. --- app/Jobs/ApiTokenExpirationWarningJob.php | 4 +++- tests/Feature/ApiTokenExpirationWarningTest.php | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/Jobs/ApiTokenExpirationWarningJob.php b/app/Jobs/ApiTokenExpirationWarningJob.php index 810919c6a..e7b34248e 100644 --- a/app/Jobs/ApiTokenExpirationWarningJob.php +++ b/app/Jobs/ApiTokenExpirationWarningJob.php @@ -42,6 +42,9 @@ public function handle(): void } $warningSentAt = now(); + + $team->notify(new ApiTokenExpiringNotification($token)); + $markedAsSent = PersonalAccessToken::query() ->whereKey($token->getKey()) ->whereNotNull('expires_at') @@ -55,7 +58,6 @@ public function handle(): void } $token->forceFill(['api_token_expiration_warning_sent_at' => $warningSentAt]); - $team->notify(new ApiTokenExpiringNotification($token)); } }); } diff --git a/tests/Feature/ApiTokenExpirationWarningTest.php b/tests/Feature/ApiTokenExpirationWarningTest.php index 67380f047..beea1f126 100644 --- a/tests/Feature/ApiTokenExpirationWarningTest.php +++ b/tests/Feature/ApiTokenExpirationWarningTest.php @@ -6,6 +6,7 @@ use App\Models\User; use App\Notifications\ApiTokenExpiringNotification; use Carbon\Carbon; +use Illuminate\Contracts\Notifications\Dispatcher; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Notification; @@ -50,6 +51,21 @@ function createTokenExpiring(User $user, Team $team, ?Carbon $expiresAt, ?Carbon expect($token->fresh()->api_token_expiration_warning_sent_at)->not->toBeNull(); }); + test('does not mark token as warned when notification fails', function () { + $token = createTokenExpiring($this->user, $this->team, now()->addHours(23)); + $dispatcher = Mockery::mock(Dispatcher::class); + $dispatcher->shouldReceive('send') + ->once() + ->andThrow(new RuntimeException('Notification failed')); + + $this->app->instance(Dispatcher::class, $dispatcher); + + expect(fn () => (new ApiTokenExpirationWarningJob)->handle()) + ->toThrow(RuntimeException::class, 'Notification failed'); + + expect($token->fresh()->api_token_expiration_warning_sent_at)->toBeNull(); + }); + test('database marker prevents duplicate warnings on repeat runs', function () { createTokenExpiring($this->user, $this->team, now()->addHours(12));