Skip to content

Conversation

@kevalyq
Copy link
Contributor

@kevalyq kevalyq commented Nov 9, 2025

Summary

Fixes guard mismatch in test files and User model by explicitly declaring 'sanctum' guard for all permissions and the User model. Resolves 403 Forbidden errors in RBAC tests.

Related Issues

Fixes #126 (RoleApiTest)
Fixes #127 (PersonApiTest - was already partially fixed with 'web')
Fixes #128 (Covered by RoleApiTest)
Fixes #129 (User Model $guard_name)
Part of: #125 (EPIC: Guard Migration)

Changes

Test Files (Guard Declaration)

  • RoleApiTest.php: Added 'guard_name' => 'sanctum' to 3 Permission::create() + 1 Role::create()
  • PersonApiTest.php: Changed 'guard_name' from 'web' to 'sanctum' (2 permissions)

Model (Guard Declaration)

  • User.php: Added protected $guard_name = 'sanctum' with PHPDoc @var string type hint

Documentation

  • CHANGELOG.md: Entry under [Unreleased] > Fixed

Technical Context

Problem:

  • Frontend uses Bearer tokens (sanctum auth)
  • User model had no explicit $guard_name (defaults to 'web')
  • Tests created permissions without guard_name (defaults to 'web')
  • Spatie's givePermissionTo() uses User model's guard, not permission's guard
  • Result: Guard mismatch → PermissionDoesNotExist exception

Solution:
Both User model AND permissions must explicitly use 'sanctum' guard.

Testing

ddev exec php artisan test tests/Feature/RoleApiTest.php tests/Feature/PersonApiTest.php

Results:

  • ✅ 40 tests, 146 assertions (ALL PASS)
  • ✅ PHPStan Level Max: 0 errors
  • ✅ Laravel Pint: Clean
  • ✅ Full test suite: 207 tests, 623 assertions (ALL PASS)
  • ✅ Preflight script: PASSED

Impact

Architecture Semantically Correct: Token-auth uses sanctum guard
Self-Documenting Code: Explicit guard declaration
Zero Technical Debt: No guard mismatch confusion
Unblocks RBAC Phase 4: Issue #108 can proceed

4-Pass Review

Pass 1 - Comprehensive: PSR-12, PHPDoc, tests, CHANGELOG
Pass 2 - Deep Dive: Domain policy, REUSE 3.3, security
Pass 3 - Best Practices: Hidden files, governance
Pass 4 - Security: No secrets, .gitignore complete

Pre-Merge Checklist

  • All tests passing (207/207)
  • PHPStan Level Max: 0 errors
  • Laravel Pint: Clean
  • CHANGELOG updated
  • REUSE compliant
  • Preflight script: PASSED
  • GPG signed commit
  • 4-pass review completed
  • Copilot review (pending)

- Update RoleApiTest.php: Add guard_name='sanctum' to all Permission and Role creation
- Update PersonApiTest.php: Change guard_name from 'web' to 'sanctum'
- Add $guard_name='sanctum' property to User model with PHPDoc type hint
- Resolves guard mismatch causing 403 Forbidden errors in permission checks
- All 207 tests passing (623 assertions)

Fixes #126
Fixes #127
Fixes #128
Fixes #129
Part of: #125
@kevalyq kevalyq marked this pull request as ready for review November 9, 2025 19:55
Copilot AI review requested due to automatic review settings November 9, 2025 19:55
@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 fixes a critical guard mismatch in the permission system by migrating from 'web' to 'sanctum' guard, resolving 403 Forbidden errors in API authentication. The change ensures that Spatie Laravel-Permission guards align with the Sanctum authentication used in API routes.

Key changes:

  • Added explicit guard_name='sanctum' to all test Role and Permission creations
  • Added $guard_name = 'sanctum' property to the User model
  • Updated CHANGELOG with comprehensive migration details

Reviewed Changes

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

File Description
tests/Feature/RoleApiTest.php Added guard_name='sanctum' to Role and Permission creation in test setup
tests/Feature/PersonApiTest.php Changed Permission guard_name from 'web' to 'sanctum' in test setup
app/Models/User.php Added $guard_name='sanctum' property with documentation explaining Spatie integration
CHANGELOG.md Documented the guard migration fix with technical details and issue references

@kevalyq kevalyq merged commit de403ba into main Nov 9, 2025
33 checks passed
@kevalyq kevalyq deleted the fix/126-127-128-129-sanctum-guard-tests branch November 9, 2025 20:16
@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! 😊

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

Labels

None yet

Projects

None yet

2 participants