Skip to content

Comments

fix: resolve audit issues for v2 release#15

Open
soymgomez wants to merge 15 commits intomainfrom
fix/audit-issues
Open

fix: resolve audit issues for v2 release#15
soymgomez wants to merge 15 commits intomainfrom
fix/audit-issues

Conversation

@soymgomez
Copy link
Member

@soymgomez soymgomez commented Feb 20, 2026

Summary

Resolves all issues found during the security and code quality audit. This PR prepares the package for the v1 → v2 major release.

Security

  • Encrypt OAuth tokens at rest — Added encrypted cast to oauth_token and oauth_refresh_token in the Eloquent model. Tokens are now encrypted/decrypted transparently via Laravel's Crypt.
  • Session fixation — Added Session::regenerate() before Auth::guard()->login() in the OAuth callback.
  • Null check on groups$callback['groups'] ?? [] in all 3 places to prevent errors when the OAuth provider does not return groups.
  • Rate limiting — Added throttle:5,1 middleware to OAuth routes.

Bug fixes

  • Middleware ignoring redirectOAuthTokenRenewal now captures and returns the redirect response from renew() instead of silently discarding it.
  • Broken facade — Fixed accessor to return 'oauth' string, singleton now resolves OAuthService, deleted dead src/OAuth.php.
  • Encrypt command bug — Fixed continue that skipped entire records when oauth_token was already encrypted, leaving oauth_refresh_token unencrypted. Each field is now evaluated independently.
  • Regex injection in command — Added preg_quote() when building regex from env variable keys.
  • Relative path in command — Changed to config_path('oauth.php').
  • Config comments — Fixed copy-paste descriptions for guard_name, login_route_name, redirect_route_name_callback_ok.
  • Config default — Changed user_model_name default from test class to App\Models\User.
  • PHPStan type errors — Fixed mixed concatenation in OAuthService, @var type mismatches in OAuthCommand and OAuthController.

Upgrade path (v1 → v2)

  • New alter migration (upgrade_oauth_table_v2) — Changes oauth_refresh_token to longText, oauth_token_expires_at to unsignedBigInteger, adds index on oauth_id. Original create_oauth_table migration preserved for existing installations.
  • oauth:encrypt-tokens command — Encrypts existing plaintext tokens in-place, skips already encrypted values, safe to re-run.
  • Deprecated event aliasEventsOAuthTokenUpdated now extends OAuthTokenUpdated as a deprecated alias (will be removed in v3).
  • UPGRADE.md — Documents all breaking changes and step-by-step migration guide with maintenance mode.

Code quality

  • Constructor property promotion in OAuthController (removed nullable + service locator fallback).
  • Request injection — Replaced Request facade with Illuminate\Http\Request injection in callback().
  • Event renameEventsOAuthTokenUpdatedOAuthTokenUpdated.
  • Variable naming$valorFormateado$formattedValue.
  • Typo fix — "loged" → "logged" in interface and handler.
  • Simplified belongsTo(new $model())::class$model directly.
  • Removed dead codesrc/OAuth.php, loadMigrations() method.

Breaking changes

  • Dropped Laravel 10 support — Laravel 10 reached EOL in Feb 2025. This avoids requiring doctrine/dbal for migration ->change() calls.
  • Constructor signature of OAuthController changed (required params, no more nullable fallback).
  • src/OAuth.php removed — Use Raiolanetworks\OAuth\Facades\OAuth instead.
  • Rate limiting added to OAuth routes (throttle:5,1).

CI / DevOps

  • Updated test matrix — Dropped Laravel 10, added Laravel 12 with testbench ^10.0.
  • Updated dev dependencieslarastan ^2.0|^3.0, orchestra/testbench ^9.4|^10.0, nunomaduro/collision ^8.1.1.
  • PHPStan config — Added ignore for env() false positive in config files (larastan v3 rule), set reportUnmatchedIgnoredErrors: false for cross-version compat.
  • Fixed prefer-lowest — CI matrix now uses ^9.4/^10.0 instead of 9.*/10.* to respect minimum testbench versions.

Test plan

  • All 24 tests passing (75 assertions)
  • PHPStan — no errors (compatible with both larastan v2 and v3)
  • Pint — no formatting issues
  • CI green: L11 + L12, PHP 8.2 + 8.3, prefer-lowest + prefer-stable (8 jobs)
  • Encrypt command tested: plaintext encryption, skip already encrypted, partial encryption, empty table
  • Deprecated event alias tested: EventsOAuthTokenUpdated is instance of OAuthTokenUpdated
  • Middleware tested: redirect response returned, null response passes through

Regenerate session before login to prevent session fixation attacks.
Add null coalescing for groups array from OAuth callback.

Changelog: security
Tokens are now encrypted/decrypted transparently via Laravel's
encrypted cast, preventing plaintext credential exposure in the DB.

Changelog: security
The middleware now returns the redirect response from renew() instead
of always proceeding to the next middleware, which caused expired
token redirects to be silently discarded.

Changelog: fixed
- Remove src/OAuth.php (dead code with misplaced getFacadeAccessor)
- Fix facade accessor to use 'oauth' string key
- Fix singleton to resolve OAuthService instead of ServiceProvider
- Remove unused loadMigrations() method and its call in command

Changelog: fixed
- Use unsignedBigInteger for token expiry (2038 problem)
- Add index on oauth_id column
- Use longText for refresh_token (encrypted values are longer)
- Fix copy-paste comments in config
- Default user_model_name to App\Models\User
- Sanitize regex input with preg_quote in env variable replacement
- Use config_path() instead of relative path

Changelog: fixed
- Add throttle:5,1 middleware to OAuth routes
- Replace Request facade with constructor injection in callback()

Changelog: changed
- Use constructor property promotion in OAuthController
- Remove nullable params and service locator fallback
- Rename EventsOAuthTokenUpdated to OAuthTokenUpdated
- Fix Spanish variable name: valorFormateado -> formattedValue
- Fix typo: loged -> logged in PHPDoc
- Simplify belongsTo: use class string directly

Changelog: changed
- Run Pint formatter on all files
- Fix renew() return type to ?RedirectResponse (removes Redirector)

Changelog: changed
- Restore original migration, add separate alter migration for
  existing installations (longText, unsignedBigInteger, index)
- Add oauth:encrypt-tokens command to migrate plaintext tokens
- Add deprecated EventsOAuthTokenUpdated alias for backwards compat
- Add UPGRADE.md documenting all breaking changes

Changelog: added
The encrypt command had a bug where `continue` skipped the entire
record when oauth_token was already encrypted, leaving
oauth_refresh_token unencrypted. Now each field is evaluated
independently. UPGRADE.md updated to recommend maintenance mode
during migration.

Changelog: fixed
Laravel 10 reached EOL in Feb 2025. Dropping it avoids the
doctrine/dbal dependency for migration ->change() calls.

Changelog: removed
Move "Verify Laravel version" to section 1 since it is a
prerequisite. Add test for EventsOAuthTokenUpdated deprecated alias.

Changelog: fixed
Add Laravel 12 to test matrix with testbench 10.* and larastan 3.*.

Changelog: changed
- Fix PHPStan type errors for strict mode with phpstan v2
- Ignore env() false positive in config/ (larastan v3 rule)
- Use ^9.4/^10.0 testbench in CI matrix to fix prefer-lowest
- Allow larastan ^3.0 for Laravel 12 support

Changelog: fixed
@soymgomez soymgomez requested a review from victore13 February 20, 2026 16:56
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.

2 participants