Skip to content

Conversation

@kevalyq
Copy link
Contributor

@kevalyq kevalyq commented Nov 9, 2025

Summary

Updates config/auth.php to set sanctum as the default guard instead of web, aligning configuration with SecPal's API-only, token-based architecture.

Changes

  • ✅ Changed default guard: 'web''sanctum'
  • ✅ Added explicit sanctum guard configuration to guards array
  • ✅ Updated documentation comments explaining API-only architecture
  • ✅ Self-documenting: Config now clearly shows React PWA + Bearer token architecture

Why This Matters

Before: Config defaulted to web guard (semantically incorrect for API-only app)
After: Config defaults to sanctum guard (matches actual authentication mechanism)

Benefits:

  • Self-documenting configuration (future developers immediately see "API-only, token-based")
  • Consistent with User model $guard_name = 'sanctum' (Add $guard_name property to User model #129)
  • Aligns config with actual authentication mechanism (all routes use auth:sanctum)
  • Prevents confusion about authentication mechanism

Review Notes: Guard Usage in Routes

Question: Are explicit auth:sanctum guard specifications in routes still necessary after this change?

Answer:YES - They should REMAIN.

Reasoning:

  1. Best Practice: Explicit guard specification is recommended in Laravel
  2. Self-Documenting: Makes the intent clear (API token-based authentication)
  3. No Ambiguity: Prevents accidental fallback to wrong guard
  4. Future-Proof: If we add other guards, explicit specification prevents confusion

Current Routes (CORRECT):

Route::middleware('auth:sanctum')->group(function () {
    // Protected API endpoints
});

Don't Change To:

Route::middleware('auth')->group(function () {  // ❌ Implicit - less clear
    // Protected API endpoints
});

Conclusion: Keep explicit auth:sanctum in routes. This PR only aligns the config with reality - it doesn't change the explicit route specifications (which are best practice).

Testing

  • ✅ All 207 tests passing (no behavior change)
  • ✅ PHPStan Level Max: 0 errors
  • ✅ Laravel Pint: Clean
  • ✅ REUSE 3.3: Compliant
  • ✅ Preflight script: Passed

Specific Tests Verified

# Full test suite
ddev exec php artisan test
# Result: 207 passed (623 assertions)

# RoleApiTest (uses permissions with sanctum guard)
ddev exec php artisan test --filter=RoleApiTest
# Result: 24 passed

# AuthTest (token generation)
ddev exec php artisan test --filter=AuthTest
# Result: 18 passed

Related Issues

Implementation Notes

Why keep web guard?

  • Laravel requires it for default password reset flow
  • We only use it for password reset email verification (stateless token-based)
  • NOT used for actual authentication sessions

No Breaking Changes:

  • Config change only - no code behavior change
  • All authentication already uses auth:sanctum middleware
  • User model already has $guard_name = 'sanctum'
  • Tests already create permissions with guard_name='sanctum'

Checklist

  • TDD: Tests written first (no new tests needed - config aligns with existing tests)
  • All tests pass (207 tests, 623 assertions)
  • PHPStan Level Max: 0 errors
  • Laravel Pint: Clean
  • REUSE 3.3: Compliant
  • CHANGELOG.md updated (in same commit)
  • Documentation comments updated
  • No --no-verify bypass used
  • One topic only (configuration change)
  • Self-reviewed using 4-pass review strategy

PR Size

  • Lines Changed: 39 lines (config + CHANGELOG)
  • Complexity: Low (configuration alignment)
  • Risk: Very low (aligns config with existing code)

Category: Configuration / Architecture
Effort: 15 minutes
Impact: Self-documenting configuration, architectural clarity

- Change default guard from 'web' to 'sanctum' (API-only architecture)
- Add explicit sanctum guard configuration to guards array
- Update documentation comments explaining API-only, token-based auth
- Aligns config with actual authentication mechanism (routes use auth:sanctum)
- Self-documenting: Config clearly shows React PWA + Bearer token architecture
- Consistent with User model $guard_name = 'sanctum' property (#129)
- No behavior change: All 207 tests passing

Fixes #134
Part of: #125
@kevalyq kevalyq marked this pull request as ready for review November 9, 2025 20:39
Copilot AI review requested due to automatic review settings November 9, 2025 20:39
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

💡 Tip: Consider Using Draft PRs

Benefits of opening PRs as drafts initially:

  • 💰 Saves CI runtime and Copilot review credits
  • 🎯 Automatically sets linked issues to "🚧 In Progress" status
  • 🚀 Mark "Ready for review" when done to trigger full CI pipeline

How to convert:

  1. Click "Still in progress? Convert to draft" in the sidebar, OR
  2. Use gh pr ready when ready for review

This is just a friendly reminder - feel free to continue as is! 😊

Copy link
Contributor

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 PR updates the default authentication guard configuration to match the actual authentication mechanism used throughout the application. SecPal uses Laravel Sanctum for stateless API token authentication, and this change makes the configuration explicitly reflect that architecture.

Key changes:

  • Changed default guard from 'web' to 'sanctum' in config/auth.php
  • Added explicit sanctum guard configuration to the guards array
  • Updated documentation comments to clearly explain the API-only, token-based architecture

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
config/auth.php Changed default guard to 'sanctum', added sanctum guard configuration, and updated documentation comments to explain API-only architecture
CHANGELOG.md Documented the configuration change with comprehensive details about the alignment with existing authentication patterns

@kevalyq kevalyq merged commit 266e74a into main Nov 9, 2025
33 checks passed
@kevalyq kevalyq deleted the fix/134-set-sanctum-default-guard branch November 9, 2025 20:42
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

💡 Tip: Consider Using Draft PRs

Benefits of opening PRs as drafts initially:

  • 💰 Saves CI runtime and Copilot review credits
  • 🎯 Automatically sets linked issues to "🚧 In Progress" status
  • 🚀 Mark "Ready for review" when done to trigger full CI pipeline

How to convert:

  1. Click "Still in progress? Convert to draft" in the sidebar, OR
  2. Use gh pr ready when ready for review

This is just a friendly reminder - feel free to continue as is! 😊

kevalyq added a commit that referenced this pull request Nov 9, 2025
- New file: docs/GUARD_ARCHITECTURE.md (comprehensive guard guide)
- Explains Laravel Guards: What they are, common types (web vs sanctum)
- Documents SecPal architecture: API-only, token-based, stateless
- Spatie Permission integration: Guard-awareness, mismatch troubleshooting
- Configuration walkthrough: config/auth.php, User model, route middleware
- Developer guidelines: Best practices for permissions/roles with sanctum
- Migration context: EPIC #125 systematic migration from web to sanctum
- Code examples: Correct vs incorrect patterns for tests, seeders, factories
- Troubleshooting: Common errors (PermissionDoesNotExist, 403) with solutions
- Incorporates insights from PR #134/#135: Explicit middleware best practice

Fixes #130
Part of: #125
kevalyq added a commit that referenced this pull request Nov 9, 2025
Copilot review feedback: Distinguish between issue number and PR number
for clarity. Issue #134 was implemented in PR #135.
kevalyq added a commit that referenced this pull request Nov 9, 2025
* docs(rbac): document Guard architecture and sanctum usage

- New file: docs/GUARD_ARCHITECTURE.md (comprehensive guard guide)
- Explains Laravel Guards: What they are, common types (web vs sanctum)
- Documents SecPal architecture: API-only, token-based, stateless
- Spatie Permission integration: Guard-awareness, mismatch troubleshooting
- Configuration walkthrough: config/auth.php, User model, route middleware
- Developer guidelines: Best practices for permissions/roles with sanctum
- Migration context: EPIC #125 systematic migration from web to sanctum
- Code examples: Correct vs incorrect patterns for tests, seeders, factories
- Troubleshooting: Common errors (PermissionDoesNotExist, 403) with solutions
- Incorporates insights from PR #134/#135: Explicit middleware best practice

Fixes #130
Part of: #125

* docs: clarify Issue #134 vs PR #135 in CHANGELOG

Copilot review feedback: Distinguish between issue number and PR number
for clarity. Issue #134 was implemented in PR #135.
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.

Set sanctum as default guard in config/auth.php

2 participants