Skip to content

Conversation

stratoghost
Copy link

@stratoghost stratoghost commented Aug 3, 2025

This PR fixes missing OAuth scopes during the social auth redirect and widens token columns to handle long provider tokens.

Why

  • Providers support a scopes setting, but we weren’t passing it on redirect. That breaks flows that need extra permissions.
  • Some Socialite providers return tokens longer than 400 chars. We were truncating them.

What changed

Social auth

  • SocialController@redirect: reads provider config and applies the configured scopes to the redirect.
  • SocialProvider::getRows: if scopes is an array, store it as a comma-separated string.

Database

Controller update

Model update

Config example

// config/services.php
'vatsim' => [
    'name' => 'VATSIM',
    'scopes' => ['full_name', 'email', 'vatsim_details', 'country'],
    'parameters' => null,
    'stateless' => true,
    'active' => true,
    'socialite' => false,
    'svg' => '...',
    'client_id' => env('VATSIM_CLIENT_ID'),
    'client_secret' => env('VATSIM_CLIENT_SECRET'),
]

Testing

  • ✅ PEST tests passing
  • ⚠️ Dusk has some flaky cases from dev-main
  • 🔄 Manual tests across multiple providers worked as expected

Migration notes

  • Run migrations after deploy.
  • Rolling back will cut tokens to 400 chars. Back up if that’s a concern.
  • Clear config cache if you change scopes: php artisan config:clear

Impact

  • No API changes.
  • Existing configs with array scopes keep working. We store scopes as a comma-separated strings through Sushi.

Reason for the PR

I opened this for DevDojo Auth because scopes existed in provider setup, but the redirect ignored them. This applies the scopes and prevents token truncation seen with some custom Socialite providers.

@Copilot Copilot AI review requested due to automatic review settings August 3, 2025 21:45
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request enhances OAuth functionality by adding support for configurable scopes and increasing token storage capacity. The changes enable dynamic OAuth scope configuration per provider while expanding database storage limits for tokens.

  • Database schema updates to change token columns from varchar to text for increased storage capacity
  • OAuth scope support with dynamic configuration from provider settings
  • Data handling improvements to convert scope arrays to comma-separated strings

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
database/migrations/2025_08_03_213559_change_token_to_text_on_social_provider_users_table.php Migration to change token column from string(400) to text type
database/migrations/2025_08_03_213701_change_refresh_token_to_text_on_social_provider_users_table.php Migration to change refresh_token column from string to text type
src/Http/Controllers/SocialController.php Added dynamic OAuth scope configuration in redirect method
src/Models/SocialProvider.php Added logic to convert scope arrays to comma-separated strings

…text_on_social_provider_users_table.php

Co-authored-by: Copilot <[email protected]>
@msumdev
Copy link

msumdev commented Aug 5, 2025

Finally! Hopefully this gets merged soon.

@tnylea
Copy link
Contributor

tnylea commented Sep 1, 2025

@stratoghost, thanks for the contribution. I'm looking it over and if the tests pass I can get this merged in real soon :)

Thanks!

@tnylea
Copy link
Contributor

tnylea commented Sep 2, 2025

Hey @stratoghost,

It looks like the tests are failing, but that's not your fault. There was a change that was needed to get the pest and dusk tests to pass which I have merged here: #162.

Go ahead and merge main into your branch and that should solve the issue. Hit me up when you've done that and I can get this merged in and in the next release.

Appreciate it 🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants