From 6729c5f68c911f43b2ad2eabf98568fa37e0afc2 Mon Sep 17 00:00:00 2001 From: Dave Roverts Date: Wed, 7 Oct 2026 21:06:43 +0200 Subject: [PATCH 1/2] fix(security): stop storing OAuth access/refresh tokens on users We didn't use them at all --- app/Models/User.php | 41 ------------------- app/Services/Auth/AuthenticationService.php | 17 +------- app/Services/OAuth/VatsimProvider.php | 13 ------ ...p_oauth_token_columns_from_users_table.php | 29 +++++++++++++ tests/Unit/UserTest.php | 37 ----------------- 5 files changed, 31 insertions(+), 106 deletions(-) create mode 100644 database/migrations/2026_10_07_190017_drop_oauth_token_columns_from_users_table.php delete mode 100644 tests/Unit/UserTest.php diff --git a/app/Models/User.php b/app/Models/User.php index 89bde07e..7d4a017d 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -3,13 +3,11 @@ namespace App\Models; use App\Enums\AirportView; -use App\Services\OAuth\VatsimProvider; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; -use League\OAuth2\Client\Token\AccessToken; use Spatie\Activitylog\LogOptions; use Spatie\Activitylog\Traits\LogsActivity; @@ -22,9 +20,6 @@ * @property AirportView $airport_view * @property bool $use_monospace_font * @property string|null $remember_token - * @property string|null $access_token - * @property string|null $refresh_token - * @property int|null $token_expires * @property \Illuminate\Support\Carbon|null $created_at * @property \Illuminate\Support\Carbon|null $updated_at * @property-read \Illuminate\Database\Eloquent\Collection $activities @@ -39,7 +34,6 @@ * @method static \Illuminate\Database\Eloquent\Builder|User newModelQuery() * @method static \Illuminate\Database\Eloquent\Builder|User newQuery() * @method static \Illuminate\Database\Eloquent\Builder|User query() - * @method static \Illuminate\Database\Eloquent\Builder|User whereAccessToken($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereAirportView($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereCreatedAt($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereEmail($value) @@ -47,9 +41,7 @@ * @method static \Illuminate\Database\Eloquent\Builder|User whereIsAdmin($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereNameFirst($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereNameLast($value) - * @method static \Illuminate\Database\Eloquent\Builder|User whereRefreshToken($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereRememberToken($value) - * @method static \Illuminate\Database\Eloquent\Builder|User whereTokenExpires($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereUpdatedAt($value) * @method static \Illuminate\Database\Eloquent\Builder|User whereUseMonospaceFont($value) * @mixin \Eloquent @@ -73,9 +65,6 @@ class User extends Authenticatable */ protected $hidden = [ 'remember_token', - 'access_token', - 'refresh_token', - 'token_expires', ]; public function getActivitylogOptions(): LogOptions @@ -109,36 +98,6 @@ protected function pic(): Attribute ); } - /** - * Returns a valid access token, refreshing it via the OAuth provider if it has expired. - * Persists updated token fields to the database when a refresh occurs. - */ - public function refreshTokenIfExpired(): ?AccessToken - { - if ($this->access_token === null) { - return null; - } - - $token = new AccessToken([ - 'access_token' => $this->access_token, - 'refresh_token' => $this->refresh_token, - 'expires' => $this->token_expires, - ]); - - if ($token->hasExpired()) { - $refreshedToken = resolve(VatsimProvider::class)->updateToken($token); - $token = $refreshedToken instanceof AccessToken ? $refreshedToken : null; - - $this->update([ - 'access_token' => $token?->getToken(), - 'refresh_token' => $token?->getRefreshToken(), - 'token_expires' => $token?->getExpires(), - ]); - } - - return $token; - } - /** * The attributes that should be cast to native types. * diff --git a/app/Services/Auth/AuthenticationService.php b/app/Services/Auth/AuthenticationService.php index 4d8c90fd..b574ebb8 100644 --- a/app/Services/Auth/AuthenticationService.php +++ b/app/Services/Auth/AuthenticationService.php @@ -42,7 +42,7 @@ public function authenticateFromOAuth(Request $request): ?array return null; } - $user = $this->upsertUser($data, $accessToken); + $user = $this->upsertUser($data); return ['user' => $user, 'data' => $data]; } @@ -50,7 +50,7 @@ public function authenticateFromOAuth(Request $request): ?array /** * Create or update the user record and log them in. */ - protected function upsertUser(array $data, $token): User + protected function upsertUser(array $data): User { $account = User::updateOrCreate( ['id' => $data['cid']], @@ -61,19 +61,6 @@ protected function upsertUser(array $data, $token): User ] ); - if ($token->getToken() !== null) { - $account->access_token = $token->getToken(); - } - - if ($token->getRefreshToken() !== null) { - $account->refresh_token = $token->getRefreshToken(); - } - - if ($token->getExpires() !== null) { - $account->token_expires = $token->getExpires(); - } - - $account->save(); auth()->loginUsingId($data['cid'], true); activity()->log('Login'); diff --git a/app/Services/OAuth/VatsimProvider.php b/app/Services/OAuth/VatsimProvider.php index 0c5ede4d..25ae790f 100644 --- a/app/Services/OAuth/VatsimProvider.php +++ b/app/Services/OAuth/VatsimProvider.php @@ -3,8 +3,6 @@ namespace App\Services\OAuth; use League\OAuth2\Client\Provider\GenericProvider; -use League\OAuth2\Client\Provider\Exception\IdentityProviderException; -use League\OAuth2\Client\Token\AccessTokenInterface; class VatsimProvider extends GenericProvider { @@ -25,17 +23,6 @@ public function __construct() ]); } - public function updateToken(AccessTokenInterface $token): ?AccessTokenInterface - { - try { - return $this->getAccessToken('refresh_token', [ - 'refresh_token' => $token->getRefreshToken(), - ]); - } catch (IdentityProviderException) { - return null; - } - } - public function getOAuthProperty(string $property, mixed $data): mixed { return data_get($data, str_replace('-', '.', $property)) ?: false; diff --git a/database/migrations/2026_10_07_190017_drop_oauth_token_columns_from_users_table.php b/database/migrations/2026_10_07_190017_drop_oauth_token_columns_from_users_table.php new file mode 100644 index 00000000..d2976db0 --- /dev/null +++ b/database/migrations/2026_10_07_190017_drop_oauth_token_columns_from_users_table.php @@ -0,0 +1,29 @@ +dropColumn(['access_token', 'refresh_token', 'token_expires']); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('users', function (Blueprint $table): void { + $table->text('access_token')->after('remember_token')->nullable(); + $table->text('refresh_token')->after('access_token')->nullable(); + $table->unsignedBigInteger('token_expires')->after('refresh_token')->nullable(); + }); + } +}; diff --git a/tests/Unit/UserTest.php b/tests/Unit/UserTest.php deleted file mode 100644 index 2ee26654..00000000 --- a/tests/Unit/UserTest.php +++ /dev/null @@ -1,37 +0,0 @@ -create([ - 'access_token' => null, - 'refresh_token' => null, - 'token_expires' => null, - ]); - - expect($user->refreshTokenIfExpired())->toBeNull(); -}); - -it('returns the existing token when it has not expired', function (): void { - /** @var TestCase $this */ - $user = User::factory()->create([ - 'access_token' => 'valid-access-token', - 'refresh_token' => 'valid-refresh-token', - 'token_expires' => time() + 3600, - ]); - - $token = $user->refreshTokenIfExpired(); - - expect($token)->toBeInstanceOf(AccessToken::class); - expect($token->getToken())->toBe('valid-access-token'); - - // Confirm no DB update occurred for a non-expired token - $this->assertDatabaseHas('users', [ - 'id' => $user->id, - 'access_token' => 'valid-access-token', - ]); -}); From 93649f8124e720f15b5d0f5cf59da875116a3d81 Mon Sep 17 00:00:00 2001 From: Dave Roverts Date: Wed, 7 Oct 2026 21:42:46 +0200 Subject: [PATCH 2/2] chore: fix rector --- app/Services/OAuth/VatsimProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Services/OAuth/VatsimProvider.php b/app/Services/OAuth/VatsimProvider.php index 25ae790f..f8758f26 100644 --- a/app/Services/OAuth/VatsimProvider.php +++ b/app/Services/OAuth/VatsimProvider.php @@ -1,5 +1,7 @@