diff --git a/app/Service/DeletionService.php b/app/Service/DeletionService.php index 2cee272f..872e4996 100644 --- a/app/Service/DeletionService.php +++ b/app/Service/DeletionService.php @@ -11,6 +11,7 @@ use App\Models\Client; use App\Models\Member; use App\Models\Organization; use App\Models\OrganizationInvitation; +use App\Models\Passport\RefreshToken; use App\Models\Project; use App\Models\ProjectMember; use App\Models\Report; @@ -169,6 +170,10 @@ class DeletionService } } + // Refresh tokens are not linked to the user directly, so they need to be deleted via their access tokens. + // Otherwise a still-valid refresh token could be used to issue a new access token for a deleted user, + // which fails with a foreign key violation on oauth_access_tokens.user_id. + RefreshToken::query()->whereIn('access_token_id', $user->accessTokens()->pluck('id'))->delete(); $user->accessTokens()->delete(); $user->authCodes()->delete(); diff --git a/tests/Unit/Service/DeletionServiceTest.php b/tests/Unit/Service/DeletionServiceTest.php index b9dd4d56..30b4ccc9 100644 --- a/tests/Unit/Service/DeletionServiceTest.php +++ b/tests/Unit/Service/DeletionServiceTest.php @@ -10,6 +10,9 @@ use App\Exceptions\Api\CanNotDeleteUserWhoIsOwnerOfOrganizationWithMultipleMembe use App\Models\Client; use App\Models\Member; use App\Models\Organization; +use App\Models\Passport\Client as PassportClient; +use App\Models\Passport\RefreshToken; +use App\Models\Passport\Token; use App\Models\Project; use App\Models\ProjectMember; use App\Models\Report; @@ -23,6 +26,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; use PHPUnit\Framework\Attributes\CoversClass; use Tests\TestCaseWithDatabase; use TiMacDonald\Log\LogEntry; @@ -424,4 +428,45 @@ class DeletionServiceTest extends TestCaseWithDatabase 'role' => Role::Placeholder->value, ]); } + + public function test_delete_user_deletes_access_tokens_and_their_refresh_tokens_but_does_not_delete_tokens_of_other_users(): void + { + // Arrange + $user = User::factory()->create(); + $otherUser = User::factory()->create(); + $passportClient = PassportClient::factory()->create(); + + $userToken = Token::factory()->forUser($user)->forClient($passportClient)->create(); + $userRefreshToken = RefreshToken::query()->create([ + 'id' => Str::random(100), + 'access_token_id' => $userToken->getKey(), + 'revoked' => false, + 'expires_at' => now()->addDays(30), + ]); + + $otherUserToken = Token::factory()->forUser($otherUser)->forClient($passportClient)->create(); + $otherUserRefreshToken = RefreshToken::query()->create([ + 'id' => Str::random(100), + 'access_token_id' => $otherUserToken->getKey(), + 'revoked' => false, + 'expires_at' => now()->addDays(30), + ]); + + // Act + $this->deletionService->deleteUser($user); + + // Assert + $this->assertDatabaseMissing(Token::class, [ + 'id' => $userToken->getKey(), + ]); + $this->assertDatabaseMissing(RefreshToken::class, [ + 'id' => $userRefreshToken->getKey(), + ]); + $this->assertDatabaseHas(Token::class, [ + 'id' => $otherUserToken->getKey(), + ]); + $this->assertDatabaseHas(RefreshToken::class, [ + 'id' => $otherUserRefreshToken->getKey(), + ]); + } }