Skip to content

Modernize package, harden admin checks and update dependencies - #25

Open
MSnoeren wants to merge 1 commit into
mainfrom
claude/zen-noether-kkyg29
Open

MSnoeren wants to merge 1 commit into
mainfrom
claude/zen-noether-kkyg29

Conversation

@MSnoeren

Copy link
Copy Markdown
Member

Summary

Security

  • Empty admin entries no longer grant admin rights. An empty ADMINS= used to become [''], so a user with an empty email counted as an admin. Empty and whitespace-only entries are now ignored, and users without an email are never admins.
  • Strict email comparison. Previously loose in_array, so e.g. "1e3" matched "1000".
  • No crashes on unexpected users. A user model without the Adminable trait, or with a null email, now simply isn't an admin instead of causing a fatal error / TypeError.
  • Middleware returns 403 for authenticated non-admins; guests still get 401.
  • Updated locked dependencies (Laravel 13.16 → 13.39, Symfony 7.4.x, Guzzle, …). composer audit reports no advisories.

Fixes & improvements

  • Fix the migration stub missing its trailing ; after return new class {...} (parse error when run). Rollback now drops the index before the column.
  • The driver is resolved lazily; config and cache are read per check, so long-running workers (Octane, queues) don't keep a stale admin list. An unknown driver now throws a clear InvalidArgumentException.
  • MySQLDriver::flush() to clear the cached list, plus an optional ADMINS_MYSQL_CONNECTION setting.
  • Use publishesMigrations() so published migrations get a proper timestamp.
  • Require Laravel 12 or 13 (instead of open-ended >=10), declare the illuminate components used, drop unused spatie/macroable.
  • CI: matrix over PHP 8.3–8.5 × Laravel 12/13, composer audit step, read-only token. PHPUnit config updated for v12.
  • New tests for the gate, the @admin Blade directive, the MySQL driver (incl. caching) and the edge cases above.
  • README updated for the Laravel 11+ structure (bootstrap/app.php, AppServiceProvider).

⚠️ Breaking changes (needs a major release)

  • Laravel 10 and 11 are no longer supported.
  • Logged-in non-admins get 403 instead of 401.
  • getAdminEmail() now returns ?string; models overriding it must update the signature.

Testing

  • 18 tests and PHPStan level 9 pass locally on PHP 8.3 / Laravel 13. Laravel 12 combinations are covered by CI.

Generated by Claude Code

Security:
- Ignore empty and whitespace-only entries in the admin list, so a user
  without an email can no longer match an empty ADMINS value.
- Compare emails strictly (no loose numeric-string comparison).
- Users without an email or without the Adminable trait are never
  admins instead of causing a TypeError/fatal error.
- Middleware now responds 403 for authenticated non-admins (401 only
  for guests).
- Update locked dependencies (Laravel 13.39, Symfony 7.4.x, ...);
  composer audit reports no advisories.

Fixes and improvements:
- Fix the migration stub missing its trailing semicolon (parse error).
- Resolve the driver lazily and fail with a clear exception on an
  unknown driver.
- Read config/cache per check so long-running workers (Octane, queues)
  don't keep a stale admin list; add MySQLDriver::flush() and an
  optional database connection setting.
- Use publishesMigrations() so published migrations get a timestamp.
- Require Laravel 12 or 13 and declare the illuminate components used;
  drop the unused spatie/macroable dev dependency.
- CI: test Laravel 12 and 13, run composer audit, read-only token.
- Add tests for the gate, Blade directive, MySQL driver and edge cases.
- Update README for Laravel 11+ application structure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011nsvciJbFZ9rTyYWtbFh3d
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